From 11a6e923ee08aaaa123747d9a5f6b2e1dfbd85c9 Mon Sep 17 00:00:00 2001 From: Oleg Zhurakousky Date: Sun, 22 Sep 2019 17:15:19 -0400 Subject: [PATCH] GH-409 Fix default discovery of functions Fixed discovery of functions to ensure that even in cases where default function is found but it's type can not be determined such function is discarded. This effectively ensures that if the actual instance does not match the declared type such function is not treated as function. Resolves #409 --- .../BeanFactoryAwareFunctionRegistry.java | 24 +++++--- .../context/catalog/FunctionTypeUtils.java | 8 --- ...BeanFactoryAwareFunctionRegistryTests.java | 56 ++++++++++++++++++- 3 files changed, 72 insertions(+), 16 deletions(-) diff --git a/spring-cloud-function-context/src/main/java/org/springframework/cloud/function/context/catalog/BeanFactoryAwareFunctionRegistry.java b/spring-cloud-function-context/src/main/java/org/springframework/cloud/function/context/catalog/BeanFactoryAwareFunctionRegistry.java index 842adbcf1..9c7e63860 100644 --- a/spring-cloud-function-context/src/main/java/org/springframework/cloud/function/context/catalog/BeanFactoryAwareFunctionRegistry.java +++ b/spring-cloud-function-context/src/main/java/org/springframework/cloud/function/context/catalog/BeanFactoryAwareFunctionRegistry.java @@ -164,10 +164,6 @@ public class BeanFactoryAwareFunctionRegistry return this.registrationsByFunction.get(function); } - public FunctionType getFunctionType(String name) { - return FunctionType.of(FunctionTypeUtils.getFunctionType(this.lookup(name), this)); - } - private Object locateFunction(String name) { Object function = null; if (this.applicationContext.containsBean(name)) { @@ -214,6 +210,7 @@ public class BeanFactoryAwareFunctionRegistry .filter(n -> !n.endsWith(FunctionRegistration.REGISTRATION_NAME_SUFFIX) && !n.equals(RoutingFunction.FUNCTION_NAME)).toArray(String[]::new); String[] supplierNames = Stream.of(this.applicationContext.getBeanNamesForType(Supplier.class)) .filter(n -> !n.endsWith(FunctionRegistration.REGISTRATION_NAME_SUFFIX) && !n.equals(RoutingFunction.FUNCTION_NAME)).toArray(String[]::new); + /* * we may need to add BiFunction and BiConsumer at some point */ @@ -221,7 +218,8 @@ public class BeanFactoryAwareFunctionRegistry .concat(Stream.of(functionNames), Stream.concat(Stream.of(consumerNames), Stream.of(supplierNames))).collect(Collectors.toList()); if (!ObjectUtils.isEmpty(names)) { - Assert.isTrue(names.size() == 1, "Found more then one function in BeanFactory: " + names); + Assert.isTrue(names.size() == 1, "Found more then one function in BeanFactory: " + names + + ". Consider providing 'spring.cloud.function.definition' property."); definition = names.get(0); } else { @@ -230,6 +228,15 @@ public class BeanFactoryAwareFunctionRegistry definition = this.registrationsByName.keySet().iterator().next(); } } + + if (StringUtils.hasText(definition)) { + Type functionType = discoverFunctionType(this.applicationContext.getBean(definition), definition); + if (!FunctionTypeUtils.isSupplier(functionType) && !FunctionTypeUtils.isFunction(functionType) && !FunctionTypeUtils.isConsumer(functionType)) { + logger.info("Discovered functional instance of bean '" + definition + "' as a default function, however its " + + "function argument types can not be determined. Discarding."); + definition = null; + } + } } return definition; } @@ -576,8 +583,11 @@ public class BeanFactoryAwareFunctionRegistry List acceptedContentTypes = MimeTypeUtils.parseMimeTypes(acceptedOutputMimeTypes[0].toString()); convertedValue = acceptedContentTypes.stream() - .map(acceptedContentType -> messageConverter - .toMessage(value, new MessageHeaders(Collections.singletonMap(MessageHeaders.CONTENT_TYPE, acceptedContentType)))) + .map(acceptedContentType -> { + Object v = value instanceof Message ? ((Message) value).getPayload() : value; + return messageConverter + .toMessage(v, new MessageHeaders(Collections.singletonMap(MessageHeaders.CONTENT_TYPE, acceptedContentType))); + }) .filter(v -> v != null) .findFirst().orElse(null); } diff --git a/spring-cloud-function-context/src/main/java/org/springframework/cloud/function/context/catalog/FunctionTypeUtils.java b/spring-cloud-function-context/src/main/java/org/springframework/cloud/function/context/catalog/FunctionTypeUtils.java index ac09be1ca..f7a942019 100644 --- a/spring-cloud-function-context/src/main/java/org/springframework/cloud/function/context/catalog/FunctionTypeUtils.java +++ b/spring-cloud-function-context/src/main/java/org/springframework/cloud/function/context/catalog/FunctionTypeUtils.java @@ -128,14 +128,6 @@ public final class FunctionTypeUtils { } } - public static Type getFunctionType(Object function, FunctionInspector inspector) { - FunctionRegistration registration = inspector.getRegistration(function); - if (registration != null) { - return registration.getType().getType(); - } - return null; - } - public static Type unwrapActualTypeByIndex(Type type, int index) { if (isMessage(type) || isPublisher(type)) { if (isPublisher(type)) { diff --git a/spring-cloud-function-context/src/test/java/org/springframework/cloud/function/context/catalog/BeanFactoryAwareFunctionRegistryTests.java b/spring-cloud-function-context/src/test/java/org/springframework/cloud/function/context/catalog/BeanFactoryAwareFunctionRegistryTests.java index ed3033880..55a064f9e 100644 --- a/spring-cloud-function-context/src/test/java/org/springframework/cloud/function/context/catalog/BeanFactoryAwareFunctionRegistryTests.java +++ b/spring-cloud-function-context/src/test/java/org/springframework/cloud/function/context/catalog/BeanFactoryAwareFunctionRegistryTests.java @@ -17,6 +17,7 @@ package org.springframework.cloud.function.context.catalog; +import java.io.Serializable; import java.nio.charset.StandardCharsets; import java.util.List; import java.util.Map; @@ -53,7 +54,11 @@ import static org.assertj.core.api.Assertions.assertThat; public class BeanFactoryAwareFunctionRegistryTests { private FunctionCatalog configureCatalog() { - ApplicationContext context = new SpringApplicationBuilder(SampleFunctionConfiguration.class) + return this.configureCatalog(SampleFunctionConfiguration.class); + } + + private FunctionCatalog configureCatalog(Class... configClass) { + ApplicationContext context = new SpringApplicationBuilder(configClass) .run("--logging.level.org.springframework.cloud.function=DEBUG", "--spring.main.lazy-initialization=true"); FunctionCatalog catalog = context.getBean(FunctionCatalog.class); @@ -268,6 +273,15 @@ public class BeanFactoryAwareFunctionRegistryTests { result.getT3().subscribe(v -> System.out.println("=> 3: " + v)); } + @Test + public void SCF_GH_409ConfigurationTests() { + FunctionCatalog catalog = this.configureCatalog(SCF_GH_409ConfigurationAsSupplier.class); + assertThat((Object) catalog.lookup("")).isNull(); + + catalog = this.configureCatalog(SCF_GH_409ConfigurationAsFunction.class); + assertThat((Object) catalog.lookup("")).isNull(); + } + @EnableAutoConfiguration @Configuration @@ -431,6 +445,46 @@ public class BeanFactoryAwareFunctionRegistryTests { } } + @EnableAutoConfiguration + public static class SCF_GH_409ConfigurationAsSupplier { + + @Bean + public Serializable blah() { + return new Foo(); + } + + private static class Foo implements Supplier, Serializable { + + @Override + public Object get() { + // TODO Auto-generated method stub + return null; + } + + } + } + + @EnableAutoConfiguration + public static class SCF_GH_409ConfigurationAsFunction { + + @Bean + public Serializable blah() { + return new Foo(); + } + + private static class Foo implements Function, Serializable { + + @Override + public Object apply(Object t) { + // TODO Auto-generated method stub + return null; + } + + + + } + } + public static class Person { private String name; private int id;