From 37fc3e6c653675c3516cfd156bf090c44bd231ea Mon Sep 17 00:00:00 2001 From: Oleg Zhurakousky Date: Wed, 23 Aug 2017 08:27:55 -0400 Subject: [PATCH] Removed EmptyIterable - Removed EmptyIterable as it's only used by _MemoryBasedJavaFileManager_ to ensure the contract of the _list(..)_ operation that must not return null. The same contract is ensured with _new IterableClasspath(classpath, packageName, recurse)_ while making _MemoryBasedJavaFileManager.list(..)_ simpler and more consistent. - Untill this fix the AbstractByteCodeLoadingProxy was building FQCN of the byte-array defined class using Resource.getFileName() and then some, which is not very reliable since if such name does not match the actual name contained in the byte code, class loading will result in exception. So, this fix reads FQCN from the actual byte code.. - Reduced visibility of AbstractByteCodeLoadingProxy - Simplified ByteCodeLoadingFunctionTests Closes gh-99 --- .../function/compiler/java/EmptyIterable.java | 47 ------------------- .../java/MemoryBasedJavaFileManager.java | 37 ++++++--------- .../proxy/AbstractByteCodeLoadingProxy.java | 42 +++++++---------- .../proxy/ByteCodeLoadingConsumer.java | 3 +- .../proxy/ByteCodeLoadingFunction.java | 3 +- .../proxy/ByteCodeLoadingSupplier.java | 3 +- .../proxy/ByteCodeLoadingFunctionTests.java | 31 ++---------- 7 files changed, 43 insertions(+), 123 deletions(-) delete mode 100644 spring-cloud-function-compiler/src/main/java/org/springframework/cloud/function/compiler/java/EmptyIterable.java diff --git a/spring-cloud-function-compiler/src/main/java/org/springframework/cloud/function/compiler/java/EmptyIterable.java b/spring-cloud-function-compiler/src/main/java/org/springframework/cloud/function/compiler/java/EmptyIterable.java deleted file mode 100644 index 4741a87b5..000000000 --- a/spring-cloud-function-compiler/src/main/java/org/springframework/cloud/function/compiler/java/EmptyIterable.java +++ /dev/null @@ -1,47 +0,0 @@ -/* - * Copyright 2016 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.cloud.function.compiler.java; - -import java.util.Iterator; - -import javax.tools.JavaFileObject; - -import org.apache.commons.collections.IteratorUtils; - -/** - * Simple iterable that can be used to return an iterator over no values. - * - * @author Andy Clement - */ -class EmptyIterable extends CloseableFilterableJavaFileObjectIterable { - - static EmptyIterable instance = new EmptyIterable(); - - private EmptyIterable() { - super(null,false); - } - - public void close() { - } - - @SuppressWarnings("unchecked") - @Override - public Iterator iterator() { - return IteratorUtils.emptyIterator(); - } - -} \ No newline at end of file diff --git a/spring-cloud-function-compiler/src/main/java/org/springframework/cloud/function/compiler/java/MemoryBasedJavaFileManager.java b/spring-cloud-function-compiler/src/main/java/org/springframework/cloud/function/compiler/java/MemoryBasedJavaFileManager.java index eaf35312d..40330832d 100644 --- a/spring-cloud-function-compiler/src/main/java/org/springframework/cloud/function/compiler/java/MemoryBasedJavaFileManager.java +++ b/spring-cloud-function-compiler/src/main/java/org/springframework/cloud/function/compiler/java/MemoryBasedJavaFileManager.java @@ -1,12 +1,12 @@ /* - * Copyright 2016 the original author or authors. - * + * Copyright 2016-2017 the original author or authors. + * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. @@ -44,6 +44,7 @@ import org.slf4j.LoggerFactory; * as a lookup mechanism for resolving types. * * @author Andy Clement + * @author Oleg Zhurakousky */ public class MemoryBasedJavaFileManager implements JavaFileManager { @@ -55,7 +56,7 @@ public class MemoryBasedJavaFileManager implements JavaFileManager { private List toClose = new ArrayList<>(); private Map resolvedAdditionalDependencies = new LinkedHashMap<>(); - + public MemoryBasedJavaFileManager() { outputCollector = new CompilationOutputCollector(); } @@ -78,14 +79,11 @@ public class MemoryBasedJavaFileManager implements JavaFileManager { public Iterable list(Location location, String packageName, Set kinds, boolean recurse) throws IOException { logger.debug("list({},{},{},{})", location, packageName, kinds, recurse); - CloseableFilterableJavaFileObjectIterable resultIterable = null; + String classpath = ""; if (location == StandardLocation.PLATFORM_CLASS_PATH && (kinds == null || kinds.contains(Kind.CLASS))) { - String sunBootClassPath = System.getProperty("sun.boot.class.path"); - logger.debug("Creating iterable for boot class path: {}", sunBootClassPath); - resultIterable = new IterableClasspath(sunBootClassPath, packageName, - recurse); - toClose.add(resultIterable); + classpath = System.getProperty("sun.boot.class.path"); + logger.debug("Creating iterable for boot class path: {}", classpath); } else if (location == StandardLocation.CLASS_PATH && (kinds == null || kinds.contains(Kind.CLASS))) { @@ -95,18 +93,11 @@ public class MemoryBasedJavaFileManager implements JavaFileManager { javaClassPath += File.pathSeparatorChar + resolvedAdditionalDependency.toURI().toString().substring("file:".length()); } } - logger.debug("Creating iterable for class path: {}", javaClassPath); - resultIterable = new IterableClasspath(javaClassPath, packageName, recurse); - toClose.add(resultIterable); - } - else if (location == StandardLocation.SOURCE_PATH) { - // There are no 'extra sources' - resultIterable = EmptyIterable.instance; - } - else { - // Nothing to list - resultIterable = EmptyIterable.instance; + classpath = javaClassPath; + logger.debug("Creating iterable for class path: {}", classpath); } + CloseableFilterableJavaFileObjectIterable resultIterable = new IterableClasspath(classpath, packageName, recurse); + toClose.add(resultIterable); return resultIterable; } @@ -224,7 +215,7 @@ public class MemoryBasedJavaFileManager implements JavaFileManager { try { File resolved = engine.resolve(new Dependency(new DefaultArtifact(coordinates), "runtime")); // Example: - // dependency = maven://org.springframework:spring-expression:4.3.9.RELEASE + // dependency = maven://org.springframework:spring-expression:4.3.9.RELEASE // resolved.toURI() = file:/Users/aclement/.m2/repository/org/springframework/spring-expression/4.3.9.RELEASE/spring-expression-4.3.9.RELEASE.jar resolvedAdditionalDependencies.put(dependency, resolved); } catch (RuntimeException re) { diff --git a/spring-cloud-function-compiler/src/main/java/org/springframework/cloud/function/compiler/proxy/AbstractByteCodeLoadingProxy.java b/spring-cloud-function-compiler/src/main/java/org/springframework/cloud/function/compiler/proxy/AbstractByteCodeLoadingProxy.java index d8a8dbd88..4c6641277 100644 --- a/spring-cloud-function-compiler/src/main/java/org/springframework/cloud/function/compiler/proxy/AbstractByteCodeLoadingProxy.java +++ b/spring-cloud-function-compiler/src/main/java/org/springframework/cloud/function/compiler/proxy/AbstractByteCodeLoadingProxy.java @@ -19,9 +19,9 @@ package org.springframework.cloud.function.compiler.proxy; import java.lang.reflect.Method; import java.util.concurrent.atomic.AtomicReference; +import org.springframework.asm.ClassReader; import org.springframework.beans.factory.InitializingBean; import org.springframework.cloud.function.compiler.CompilationResultFactory; -import org.springframework.cloud.function.compiler.FunctionCompiler; import org.springframework.cloud.function.compiler.java.SimpleClassLoader; import org.springframework.cloud.function.support.FunctionFactoryMetadata; import org.springframework.core.io.Resource; @@ -30,36 +30,30 @@ import org.springframework.util.ReflectionUtils; /** * @author Mark Fisher + * @author Oleg Zhurakousky */ -public abstract class AbstractByteCodeLoadingProxy implements InitializingBean, FunctionFactoryMetadata { +abstract class AbstractByteCodeLoadingProxy implements InitializingBean, FunctionFactoryMetadata { private final Resource resource; - private final Class type; - - private CompilationResultFactory factory; - private final SimpleClassLoader classLoader = new SimpleClassLoader(AbstractByteCodeLoadingProxy.class.getClassLoader()); + private T target; + private Method method; - public AbstractByteCodeLoadingProxy(Resource resource, Class type) { + AbstractByteCodeLoadingProxy(Resource resource) { this.resource = resource; - this.type = type; } @Override @SuppressWarnings("unchecked") public void afterPropertiesSet() throws Exception { byte[] bytes = FileCopyUtils.copyToByteArray(this.resource.getInputStream()); - String filename = this.resource.getFilename(); - String functionName = filename == null ? type.getSimpleName() : filename.replaceAll(".fun$", ""); - String firstLetter = functionName.substring(0, 1).toUpperCase(); - String upperCasedName = (functionName.length() > 1) ? firstLetter + functionName.substring(1) : firstLetter; - String className = String.format("%s.%s%sFactory", FunctionCompiler.class.getPackage().getName(), upperCasedName, this.type.getSimpleName()); + String className = new ClassReader(bytes).getClassName().replace("/", "."); Class factoryClass = this.classLoader.defineClass(className, bytes); try { - this.factory = (CompilationResultFactory) factoryClass.newInstance(); + this.target = ((CompilationResultFactory) factoryClass.newInstance()).getResult(); this.method = findFactoryMethod(factoryClass); } catch (InstantiationException | IllegalAccessException e) { @@ -67,6 +61,16 @@ public abstract class AbstractByteCodeLoadingProxy implements InitializingBea } } + @Override + public final T getTarget() { + return this.target; + } + + @Override + public Method getFactoryMethod() { + return this.method; + } + private Method findFactoryMethod(Class clazz) { AtomicReference method = new AtomicReference<>(); ReflectionUtils.doWithLocalMethods(clazz, m -> { @@ -77,14 +81,4 @@ public abstract class AbstractByteCodeLoadingProxy implements InitializingBea }); return method.get(); } - - @Override - public final T getTarget() { - return this.factory.getResult(); - } - - @Override - public Method getFactoryMethod() { - return this.method; - } } diff --git a/spring-cloud-function-compiler/src/main/java/org/springframework/cloud/function/compiler/proxy/ByteCodeLoadingConsumer.java b/spring-cloud-function-compiler/src/main/java/org/springframework/cloud/function/compiler/proxy/ByteCodeLoadingConsumer.java index 4489c484d..3d682cb10 100644 --- a/spring-cloud-function-compiler/src/main/java/org/springframework/cloud/function/compiler/proxy/ByteCodeLoadingConsumer.java +++ b/spring-cloud-function-compiler/src/main/java/org/springframework/cloud/function/compiler/proxy/ByteCodeLoadingConsumer.java @@ -23,13 +23,14 @@ import org.springframework.core.io.Resource; /** * @author Mark Fisher + * @author Oleg Zhurakousky * * @param type */ public class ByteCodeLoadingConsumer extends AbstractByteCodeLoadingProxy> implements FunctionFactoryMetadata>, Consumer { public ByteCodeLoadingConsumer(Resource resource) { - super(resource, Consumer.class); + super(resource); } @Override diff --git a/spring-cloud-function-compiler/src/main/java/org/springframework/cloud/function/compiler/proxy/ByteCodeLoadingFunction.java b/spring-cloud-function-compiler/src/main/java/org/springframework/cloud/function/compiler/proxy/ByteCodeLoadingFunction.java index eb8a50b48..c730b7cc0 100644 --- a/spring-cloud-function-compiler/src/main/java/org/springframework/cloud/function/compiler/proxy/ByteCodeLoadingFunction.java +++ b/spring-cloud-function-compiler/src/main/java/org/springframework/cloud/function/compiler/proxy/ByteCodeLoadingFunction.java @@ -23,6 +23,7 @@ import org.springframework.core.io.Resource; /** * @author Mark Fisher + * @author Oleg Zhurakousky * * @param Function input type * @param Function result type @@ -30,7 +31,7 @@ import org.springframework.core.io.Resource; public class ByteCodeLoadingFunction extends AbstractByteCodeLoadingProxy> implements FunctionFactoryMetadata>, Function { public ByteCodeLoadingFunction(Resource resource) { - super(resource, Function.class); + super(resource); } @Override diff --git a/spring-cloud-function-compiler/src/main/java/org/springframework/cloud/function/compiler/proxy/ByteCodeLoadingSupplier.java b/spring-cloud-function-compiler/src/main/java/org/springframework/cloud/function/compiler/proxy/ByteCodeLoadingSupplier.java index dbb4e4f12..ce05223bd 100644 --- a/spring-cloud-function-compiler/src/main/java/org/springframework/cloud/function/compiler/proxy/ByteCodeLoadingSupplier.java +++ b/spring-cloud-function-compiler/src/main/java/org/springframework/cloud/function/compiler/proxy/ByteCodeLoadingSupplier.java @@ -23,13 +23,14 @@ import org.springframework.core.io.Resource; /** * @author Mark Fisher + * @author Oleg Zhurakousky * * @param type */ public class ByteCodeLoadingSupplier extends AbstractByteCodeLoadingProxy> implements FunctionFactoryMetadata>, Supplier { public ByteCodeLoadingSupplier(Resource resource) { - super(resource, Supplier.class); + super(resource); } @Override diff --git a/spring-cloud-function-compiler/src/test/java/org/springframework/cloud/function/compiler/proxy/ByteCodeLoadingFunctionTests.java b/spring-cloud-function-compiler/src/test/java/org/springframework/cloud/function/compiler/proxy/ByteCodeLoadingFunctionTests.java index 63b6dce92..9ff41f9a3 100644 --- a/spring-cloud-function-compiler/src/test/java/org/springframework/cloud/function/compiler/proxy/ByteCodeLoadingFunctionTests.java +++ b/spring-cloud-function-compiler/src/test/java/org/springframework/cloud/function/compiler/proxy/ByteCodeLoadingFunctionTests.java @@ -36,7 +36,7 @@ import reactor.core.publisher.Flux; /** * @author Dave Syer - * + * @author Oleg Zhurakousky */ public class ByteCodeLoadingFunctionTests { @@ -44,12 +44,7 @@ public class ByteCodeLoadingFunctionTests { public void compileConsumer() throws Exception { CompiledFunctionFactory> compiled = new ConsumerCompiler( String.class.getName()).compile("foos", "System.out::println", "String"); - ByteArrayResource resource = new ByteArrayResource(compiled.getGeneratedClassBytes(), "foos") { - @Override - public String getFilename() { - return "foos.fun"; - } - }; + ByteArrayResource resource = new ByteArrayResource(compiled.getGeneratedClassBytes(), "foos"); ByteCodeLoadingConsumer consumer = new ByteCodeLoadingConsumer<>(resource); consumer.afterPropertiesSet(); assertThat(consumer instanceof FunctionFactoryMetadata); @@ -61,12 +56,7 @@ public class ByteCodeLoadingFunctionTests { public void compileSupplier() throws Exception { CompiledFunctionFactory> compiled = new SupplierCompiler( String.class.getName()).compile("foos", "() -> \"foo\"", "String"); - ByteArrayResource resource = new ByteArrayResource(compiled.getGeneratedClassBytes(), "foos") { - @Override - public String getFilename() { - return "foos.fun"; - } - }; + ByteArrayResource resource = new ByteArrayResource(compiled.getGeneratedClassBytes(), "foos"); ByteCodeLoadingSupplier supplier = new ByteCodeLoadingSupplier<>(resource); supplier.afterPropertiesSet(); assertThat(supplier instanceof FunctionFactoryMetadata); @@ -78,12 +68,7 @@ public class ByteCodeLoadingFunctionTests { public void compileFunction() throws Exception { CompiledFunctionFactory> compiled = new FunctionCompiler( String.class.getName()).compile("foos", "v -> v.toUpperCase()", "String", "String"); - ByteArrayResource resource = new ByteArrayResource(compiled.getGeneratedClassBytes(), "foos") { - @Override - public String getFilename() { - return "foos.fun"; - } - }; + ByteArrayResource resource = new ByteArrayResource(compiled.getGeneratedClassBytes(), "foos"); ByteCodeLoadingFunction function = new ByteCodeLoadingFunction<>(resource); function.afterPropertiesSet(); assertThat(function instanceof FunctionFactoryMetadata); @@ -95,17 +80,11 @@ public class ByteCodeLoadingFunctionTests { public void compileFluxFunction() throws Exception { CompiledFunctionFactory, Flux>> compiled = new FunctionCompiler, Flux>( String.class.getName()).compile("foos", "flux -> flux.map(v -> v.toUpperCase())", "Flux", "Flux"); - ByteArrayResource resource = new ByteArrayResource(compiled.getGeneratedClassBytes(), "foos") { - @Override - public String getFilename() { - return "foos.fun"; - } - }; + ByteArrayResource resource = new ByteArrayResource(compiled.getGeneratedClassBytes(), "foos"); ByteCodeLoadingFunction, Flux> function = new ByteCodeLoadingFunction<>(resource); function.afterPropertiesSet(); assertThat(function instanceof FunctionFactoryMetadata); assertThat(FunctionFactoryUtils.isFluxFunction(function.getFactoryMethod())).isTrue(); assertThat(function.apply(Flux.just("foo")).blockFirst()).isEqualTo("FOO"); } - }