From 59f94c153395a90ba5823e65375b261d5c4d979b Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Wed, 25 Apr 2018 07:27:33 +0100 Subject: [PATCH] Fix potential issue when Message is not available but not needed If an isolated function doesn't have Message in its classpath, we will never actually need to instantiate that class. This change makes sure we check first. --- .../context/message/MessageUtils.java | 9 ++++-- .../function-sample-pojo/.attach_pid9535 | 0 .../stream/function/ClassLoaderUtils.java | 29 +++++++++++++++++-- .../stream/function/MessageUtilsTests.java | 17 ++++++++++- 4 files changed, 48 insertions(+), 7 deletions(-) delete mode 100644 spring-cloud-function-samples/function-sample-pojo/.attach_pid9535 diff --git a/spring-cloud-function-context/src/main/java/org/springframework/cloud/function/context/message/MessageUtils.java b/spring-cloud-function-context/src/main/java/org/springframework/cloud/function/context/message/MessageUtils.java index 64c9c975a..f8f9cde9e 100644 --- a/spring-cloud-function-context/src/main/java/org/springframework/cloud/function/context/message/MessageUtils.java +++ b/spring-cloud-function-context/src/main/java/org/springframework/cloud/function/context/message/MessageUtils.java @@ -90,10 +90,12 @@ public abstract class MessageUtils { return MessageBuilder.withPayload(message).build(); } ClassLoader classLoader = ((Isolated) handler).getClassLoader(); - Class type = ClassUtils.resolveClassName(Message.class.getName(), classLoader); + Class type = ClassUtils.isPresent(Message.class.getName(), classLoader) + ? ClassUtils.resolveClassName(Message.class.getName(), classLoader) + : null; Object payload; Map headers; - if (type.isAssignableFrom(message.getClass())) { + if (type != null && type.isAssignableFrom(message.getClass())) { Method getPayload = ClassUtils.getMethod(type, "getPayload"); Method getHeaders = ClassUtils.getMethod(type, "getHeaders"); payload = ReflectionUtils.invokeMethod(getPayload, message); @@ -101,7 +103,8 @@ public abstract class MessageUtils { Map map = (Map) ReflectionUtils .invokeMethod(getHeaders, message); headers = map; - } else { + } + else { payload = message; headers = Collections.emptyMap(); } diff --git a/spring-cloud-function-samples/function-sample-pojo/.attach_pid9535 b/spring-cloud-function-samples/function-sample-pojo/.attach_pid9535 deleted file mode 100644 index e69de29bb..000000000 diff --git a/spring-cloud-function-stream/src/test/java/org/springframework/cloud/function/stream/function/ClassLoaderUtils.java b/spring-cloud-function-stream/src/test/java/org/springframework/cloud/function/stream/function/ClassLoaderUtils.java index 6ebbe69b9..c3b901e30 100644 --- a/spring-cloud-function-stream/src/test/java/org/springframework/cloud/function/stream/function/ClassLoaderUtils.java +++ b/spring-cloud-function-stream/src/test/java/org/springframework/cloud/function/stream/function/ClassLoaderUtils.java @@ -17,6 +17,7 @@ package org.springframework.cloud.function.stream.function; import java.io.File; +import java.net.MalformedURLException; import java.net.URL; import java.net.URLClassLoader; import java.util.ArrayList; @@ -38,19 +39,40 @@ import reactor.core.publisher.Flux; * */ public class ClassLoaderUtils { - + @Test public void fluxIsShared() { - Class flux = ClassUtils.resolveClassName(Flux.class.getName(), createClassLoader()); + Class flux = ClassUtils.resolveClassName(Flux.class.getName(), + createClassLoader()); assertThat(flux).isEqualTo(Flux.class); } @Test public void messageIsNotShared() { - Class flux = ClassUtils.resolveClassName(Message.class.getName(), createClassLoader()); + Class flux = ClassUtils.resolveClassName(Message.class.getName(), + createClassLoader()); assertThat(flux).isNotEqualTo(Message.class); } + @Test(expected = IllegalArgumentException.class) + public void messageIsNotAvailable() { + Class flux = ClassUtils.resolveClassName(Message.class.getName(), + createMinimalClassLoader()); + assertThat(flux).isNotEqualTo(Message.class); + } + + public static ClassLoader createMinimalClassLoader() { + ClassLoader base = ClassLoaderUtils.class.getClassLoader(); + try { + return new URLClassLoader( + new URL[] { new File("target/test-classes").toURI().toURL() }, + base.getParent()); + } + catch (MalformedURLException e) { + throw new IllegalStateException(e); + } + } + public static ClassLoader createClassLoader() { URL[] urls = findClassPath(); if (urls.length == 1) { @@ -171,4 +193,5 @@ public class ClassLoaderUtils { } } } + } diff --git a/spring-cloud-function-stream/src/test/java/org/springframework/cloud/function/stream/function/MessageUtilsTests.java b/spring-cloud-function-stream/src/test/java/org/springframework/cloud/function/stream/function/MessageUtilsTests.java index bd537b4bc..e2efbab17 100644 --- a/spring-cloud-function-stream/src/test/java/org/springframework/cloud/function/stream/function/MessageUtilsTests.java +++ b/spring-cloud-function-stream/src/test/java/org/springframework/cloud/function/stream/function/MessageUtilsTests.java @@ -73,7 +73,18 @@ public class MessageUtilsTests { Object output = MessageUtils.unpack(function, "foo"); assertThat(output).isInstanceOf(Message.class); @SuppressWarnings("unchecked") - Message message = (Message)output; + Message message = (Message) output; + assertThat(message.getPayload()).isEqualTo("foo"); + } + + @Test + public void testUnpackIsolatedMessageNotAvailable() throws Exception { + Object function = create(Uppercase.class, + ClassLoaderUtils.createMinimalClassLoader()); + Object output = MessageUtils.unpack(function, "foo"); + assertThat(output).isInstanceOf(Message.class); + @SuppressWarnings("unchecked") + Message message = (Message) output; assertThat(message.getPayload()).isEqualTo("foo"); } @@ -89,6 +100,10 @@ public class MessageUtilsTests { } private Object create(Class type) { + return create(type, loader); + } + + private Object create(Class type, ClassLoader loader) { return new IsolatedFunction<>((Function) BeanUtils .instantiate(ClassUtils.resolveClassName(type.getName(), loader))); }