From 805c70fbfac25c50ee92cf8b44d5ecb4e2523ab3 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Wed, 12 Apr 2017 07:47:32 +0200 Subject: [PATCH] DATACMNS-993 - Polishing. Removed custom JUnit integration as we can just create HidingClassLoader instances in the test and the integration actually causes more code being needed (additional JUnit rule, method level annotations etc.). Tweaked ShadowingClassLoader to make obvious what has been changed over the Spring Framework variant. Created upstream ticket [0] to ask for the tweaks that would allow us to remove the class again. Original pull request: #202. Related ticket: SPR-15439 [0] https://jira.spring.io/browse/SPR-15439 --- .../ClassLoaderConfiguration.java | 60 ---------- .../classloadersupport/ClassLoaderRule.java | 113 ------------------ .../classloadersupport/HidingClassLoader.java | 51 ++++++-- .../ShadowingClassLoader.java | 51 ++++---- ...eSpringDataWebSupportIntegrationTests.java | 22 ++-- 5 files changed, 72 insertions(+), 225 deletions(-) delete mode 100644 src/test/java/org/springframework/data/classloadersupport/ClassLoaderConfiguration.java delete mode 100644 src/test/java/org/springframework/data/classloadersupport/ClassLoaderRule.java diff --git a/src/test/java/org/springframework/data/classloadersupport/ClassLoaderConfiguration.java b/src/test/java/org/springframework/data/classloadersupport/ClassLoaderConfiguration.java deleted file mode 100644 index 731487413..000000000 --- a/src/test/java/org/springframework/data/classloadersupport/ClassLoaderConfiguration.java +++ /dev/null @@ -1,60 +0,0 @@ -/* - * Copyright 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. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.springframework.data.classloadersupport; - -import java.lang.annotation.ElementType; -import java.lang.annotation.Retention; -import java.lang.annotation.RetentionPolicy; -import java.lang.annotation.Target; - -/** - * configures the {@link ClassLoaderRule}. - * - * @author Jens Schauder - */ -@Target({ElementType.METHOD, ElementType.TYPE}) -@Retention(RetentionPolicy.RUNTIME) -public @interface ClassLoaderConfiguration { - - /** - * classes of which the package that contains them will be loaded by the {@link HidingClassLoader} not by - * the normal {@code ClassLoader}. - * - * @return list of classes not to shadow. - */ - Class[] shadowPackage() default {}; - - /** - * prefixes of class names that will be loaded by the {@link HidingClassLoader}. - * - * @return list of class prefixes which will not be shadowed. - */ - String[] shadowByPrefix() default {}; - - /** - * classes of which the package that contains them will be hidden by the {@link HidingClassLoader}. - * - * @return list of classes to hidePackage. - */ - Class[] hidePackage() default {}; - - /** - * classes from packages that will be hidden by the {@link HidingClassLoader}. - * - * @return list of classes of which the package will be hidden. - */ - String[] hideByPrefix() default {}; -} diff --git a/src/test/java/org/springframework/data/classloadersupport/ClassLoaderRule.java b/src/test/java/org/springframework/data/classloadersupport/ClassLoaderRule.java deleted file mode 100644 index 1def36324..000000000 --- a/src/test/java/org/springframework/data/classloadersupport/ClassLoaderRule.java +++ /dev/null @@ -1,113 +0,0 @@ -/* - * Copyright 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. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.springframework.data.classloadersupport; - -import static java.util.Arrays.*; - -import java.util.ArrayList; -import java.util.List; -import org.junit.rules.MethodRule; -import org.junit.runners.model.FrameworkMethod; -import org.junit.runners.model.Statement; - -/** - * supports creation of tests that need to load classes with a {@link HidingClassLoader}. - * - * @author Jens Schauder - */ -public class ClassLoaderRule implements MethodRule { - - public HidingClassLoader classLoader; - - @Override - public Statement apply(final Statement base, FrameworkMethod method, Object target) { - - CombinedClassLoaderConfiguration combinedConfiguration = new CombinedClassLoaderConfiguration( - method.getAnnotation(ClassLoaderConfiguration.class), - method.getDeclaringClass().getAnnotation(ClassLoaderConfiguration.class) - ); - - classLoader = createClassLoader(combinedConfiguration); - - return new Statement() { - - @Override - public void evaluate() throws Throwable { - - try { - base.evaluate(); - } finally { - classLoader = null; - } - } - }; - } - - private static HidingClassLoader createClassLoader(CombinedClassLoaderConfiguration configuration) { - - HidingClassLoader classLoader = new HidingClassLoader(mergeHidden(configuration)); - - for (Class shadow : configuration.shadowPackages) { - classLoader.excludeClass(shadow.getPackage().getName()); - } - - for (String shadow : configuration.shadowByPrefix) { - classLoader.excludePackage(shadow); - } - - return classLoader; - } - - private static List mergeHidden(CombinedClassLoaderConfiguration configuration) { - - List hidden = new ArrayList(); - - for (Class aClass : configuration.hidePackages) { - hidden.add(aClass.getPackage().getName()); - } - - for (String aPackage : configuration.hideByPrefix) { - hidden.add(aPackage); - } - - return hidden; - } - - private static class CombinedClassLoaderConfiguration { - - final List shadowPackages = new ArrayList(); - final List shadowByPrefix = new ArrayList(); - final List hidePackages = new ArrayList(); - final List hideByPrefix = new ArrayList(); - - CombinedClassLoaderConfiguration(ClassLoaderConfiguration methodAnnotation, ClassLoaderConfiguration classAnnotation) { - - mergeAnnotation(methodAnnotation); - mergeAnnotation(classAnnotation); - } - - private void mergeAnnotation(ClassLoaderConfiguration methodAnnotation) { - - if (methodAnnotation != null) { - - shadowPackages.addAll(asList(methodAnnotation.shadowPackage())); - shadowByPrefix.addAll(asList(methodAnnotation.shadowByPrefix())); - hidePackages.addAll(asList(methodAnnotation.hidePackage())); - hideByPrefix.addAll(asList(methodAnnotation.hideByPrefix())); - } - } - } -} diff --git a/src/test/java/org/springframework/data/classloadersupport/HidingClassLoader.java b/src/test/java/org/springframework/data/classloadersupport/HidingClassLoader.java index 27e5ca46b..cfa2dddc0 100644 --- a/src/test/java/org/springframework/data/classloadersupport/HidingClassLoader.java +++ b/src/test/java/org/springframework/data/classloadersupport/HidingClassLoader.java @@ -16,29 +16,60 @@ package org.springframework.data.classloadersupport; import java.net.URLClassLoader; +import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; +import java.util.List; +import java.util.stream.Collectors; + +import org.springframework.util.Assert; /** - * is intended for testing code that depends on the presence/absence of certain classes. - * - * Classes can be: - *
    - *
  • shadowed: reloaded by this classloader no matter if they are loaded already by the SystemClassLoader
    • - *
  • hidden: not loaded by this classloader no matter if they are loaded already by the SystemClassLoader. Trying to load these classes results in a {@link ClassNotFoundException}
    • - *
  • all other classes get loaded by the SystemClassLoader
    • - *
+ * is intended for testing code that depends on the presence/absence of certain classes. Classes can be: + *
    + *
  • shadowed: reloaded by this classloader no matter if they are loaded already by the SystemClassLoader
    • + *
  • hidden: not loaded by this classloader no matter if they are loaded already by the SystemClassLoader. Trying to + * load these classes results in a {@link ClassNotFoundException}
    • + *
  • all other classes get loaded by the SystemClassLoader
    • + *
* * @author Jens Schauder + * @author Oliver Gierke */ public class HidingClassLoader extends ShadowingClassLoader { private final Collection hidden; HidingClassLoader(Collection hidden) { + super(URLClassLoader.getSystemClassLoader()); + this.hidden = hidden; } + /** + * Creates a new {@link HidingClassLoader} with the packages of the given classes hidden. + * + * @param packages must not be {@literal null}. + * @return + */ + public static HidingClassLoader hide(Class... packages) { + + Assert.notNull(packages, "Packages must not be null!"); + + List result = new ArrayList(packages.length); + + for (Class type : packages) { + result.add(type.getPackage().getName()); + } + + return new HidingClassLoader(result); + } + + /* + * (non-Javadoc) + * @see org.springframework.data.classloadersupport.ShadowingClassLoader#loadClass(java.lang.String) + */ @Override public Class loadClass(String name) throws ClassNotFoundException { @@ -46,6 +77,10 @@ public class HidingClassLoader extends ShadowingClassLoader { return super.loadClass(name); } + /* + * (non-Javadoc) + * @see java.lang.ClassLoader#findClass(java.lang.String) + */ @Override protected Class findClass(String name) throws ClassNotFoundException { diff --git a/src/test/java/org/springframework/data/classloadersupport/ShadowingClassLoader.java b/src/test/java/org/springframework/data/classloadersupport/ShadowingClassLoader.java index 6011f424b..0da59f9e7 100644 --- a/src/test/java/org/springframework/data/classloadersupport/ShadowingClassLoader.java +++ b/src/test/java/org/springframework/data/classloadersupport/ShadowingClassLoader.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-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. @@ -33,34 +33,31 @@ import org.springframework.util.FileCopyUtils; import org.springframework.util.StringUtils; /** - * ClassLoader decorator that shadows an enclosing ClassLoader, - * applying registered transformers to all affected classes. + * Copy of Spring Frameworks {@link org.springframework.instrument.classloading.ShadowingClassLoader} tweaked to not + * register default exclusions. ClassLoader decorator that shadows an enclosing ClassLoader, applying registered + * transformers to all affected classes. * * @author Rob Harrop * @author Juergen Hoeller * @author Costin Leau + * @author Jens Schauder + * @author Oliver Gierke * @since 2.0 * @see #addTransformer * @see org.springframework.core.OverridingClassLoader + * @see https://jira.spring.io/browse/SPR-15439 */ public class ShadowingClassLoader extends DecoratingClassLoader { - /** Packages that are excluded by default */ - public static final String[] DEFAULT_EXCLUDED_PACKAGES = - new String[] {"java.", "javax.", "sun.", "oracle.", "com.sun.", "com.ibm.", "COM.ibm.", - "org.w3c.", "org.xml.", "org.dom4j.", "org.eclipse", "org.aspectj.", "net.sf.cglib", - "org.springframework.cglib", "org.apache.xerces.", "org.apache.commons.logging."}; - - private final ClassLoader enclosingClassLoader; private final List classFileTransformers = new LinkedList(); private final Map> classCache = new HashMap>(); - /** * Create a new ShadowingClassLoader, decorating the given ClassLoader. + * * @param enclosingClassLoader the ClassLoader to decorate */ public ShadowingClassLoader(ClassLoader enclosingClassLoader) { @@ -68,10 +65,9 @@ public class ShadowingClassLoader extends DecoratingClassLoader { this.enclosingClassLoader = enclosingClassLoader; } - /** - * Add the given ClassFileTransformer to the list of transformers that this - * ClassLoader will apply. + * Add the given ClassFileTransformer to the list of transformers that this ClassLoader will apply. + * * @param transformer the ClassFileTransformer */ public void addTransformer(ClassFileTransformer transformer) { @@ -80,8 +76,9 @@ public class ShadowingClassLoader extends DecoratingClassLoader { } /** - * Copy all ClassFileTransformers from the given ClassLoader to the list of - * transformers that this ClassLoader will apply. + * Copy all ClassFileTransformers from the given ClassLoader to the list of transformers that this ClassLoader will + * apply. + * * @param other the ClassLoader to copy from */ public void copyTransformers(ShadowingClassLoader other) { @@ -89,7 +86,6 @@ public class ShadowingClassLoader extends DecoratingClassLoader { this.classFileTransformers.addAll(other.classFileTransformers); } - @Override public Class loadClass(String name) throws ClassNotFoundException { if (shouldShadow(name)) { @@ -99,25 +95,25 @@ public class ShadowingClassLoader extends DecoratingClassLoader { return cls; } return doLoadClass(name); - } - else { + } else { return this.enclosingClassLoader.loadClass(name); } } /** * Determine whether the given class should be excluded from shadowing. + * * @param className the name of the class * @return whether the specified class should be shadowed */ private boolean shouldShadow(String className) { - return (!className.equals(getClass().getName()) && !className.endsWith("ShadowingClassLoader") && - isEligibleForShadowing(className)); + return (!className.equals(getClass().getName()) && !className.endsWith("ShadowingClassLoader") + && isEligibleForShadowing(className)); } /** - * Determine whether the specified class is eligible for shadowing - * by this class loader. + * Determine whether the specified class is eligible for shadowing by this class loader. + * * @param className the class name to check * @return whether the specified class is eligible * @see #isExcluded @@ -126,7 +122,6 @@ public class ShadowingClassLoader extends DecoratingClassLoader { return isExcluded(className); } - private Class doLoadClass(String name) throws ClassNotFoundException { String internalName = StringUtils.replace(name, ".", "/") + ".class"; InputStream is = this.enclosingClassLoader.getResourceAsStream(internalName); @@ -147,8 +142,7 @@ public class ShadowingClassLoader extends DecoratingClassLoader { } this.classCache.put(name, cls); return cls; - } - catch (IOException ex) { + } catch (IOException ex) { throw new ClassNotFoundException("Cannot load resource for class [" + name + "]", ex); } } @@ -161,13 +155,11 @@ public class ShadowingClassLoader extends DecoratingClassLoader { bytes = (transformed != null ? transformed : bytes); } return bytes; - } - catch (IllegalClassFormatException ex) { + } catch (IllegalClassFormatException ex) { throw new IllegalStateException(ex); } } - @Override public URL getResource(String name) { return this.enclosingClassLoader.getResource(name); @@ -182,5 +174,4 @@ public class ShadowingClassLoader extends DecoratingClassLoader { public Enumeration getResources(String name) throws IOException { return this.enclosingClassLoader.getResources(name); } - } diff --git a/src/test/java/org/springframework/data/web/config/EnableSpringDataWebSupportIntegrationTests.java b/src/test/java/org/springframework/data/web/config/EnableSpringDataWebSupportIntegrationTests.java index 60e3e350f..54a5497df 100644 --- a/src/test/java/org/springframework/data/web/config/EnableSpringDataWebSupportIntegrationTests.java +++ b/src/test/java/org/springframework/data/web/config/EnableSpringDataWebSupportIntegrationTests.java @@ -25,14 +25,12 @@ import java.util.Arrays; import java.util.List; import org.hamcrest.Matcher; -import org.junit.Rule; import org.junit.Test; import org.springframework.context.ApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.core.convert.ConversionService; -import org.springframework.data.classloadersupport.ClassLoaderConfiguration; -import org.springframework.data.classloadersupport.ClassLoaderRule; +import org.springframework.data.classloadersupport.HidingClassLoader; import org.springframework.data.geo.Distance; import org.springframework.data.geo.Point; import org.springframework.data.web.PageableHandlerMethodArgumentResolver; @@ -45,10 +43,11 @@ import org.springframework.test.web.servlet.setup.MockMvcBuilders; import org.springframework.web.context.WebApplicationContext; import org.springframework.web.method.support.HandlerMethodArgumentResolver; import org.springframework.web.servlet.config.annotation.EnableWebMvc; -import org.springframework.web.servlet.config.annotation.WebMvcConfigurationSupport; import org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter; import org.springframework.web.util.UriComponentsBuilder; +import com.fasterxml.jackson.databind.ObjectMapper; + /** * Integration tests for {@link EnableSpringDataWebSupport}. * @@ -56,9 +55,6 @@ import org.springframework.web.util.UriComponentsBuilder; * @author Jens Schauder * @see DATACMNS-330 */ - -@ClassLoaderConfiguration(shadowPackage = { WebMvcConfigurationSupport.class, EnableSpringDataWebSupport.class, - EnableSpringDataWebSupportIntegrationTests.SampleConfig.class }) public class EnableSpringDataWebSupportIntegrationTests { @Configuration @@ -71,8 +67,6 @@ public class EnableSpringDataWebSupportIntegrationTests { } } - @Rule public ClassLoaderRule classLoaderRule = new ClassLoaderRule(); - @Test public void registersBasicBeanDefinitions() throws Exception { @@ -96,10 +90,11 @@ public class EnableSpringDataWebSupportIntegrationTests { } @Test - @ClassLoaderConfiguration(hidePackage = Link.class) public void doesNotRegisterHateoasSpecificComponentsIfHateoasNotPresent() throws Exception { - ApplicationContext context = WebTestUtils.createApplicationContext(classLoaderRule.classLoader, SampleConfig.class); + HidingClassLoader classLoader = HidingClassLoader.hide(Link.class); + + ApplicationContext context = WebTestUtils.createApplicationContext(classLoader, SampleConfig.class); List names = Arrays.asList(context.getBeanDefinitionNames()); @@ -122,11 +117,10 @@ public class EnableSpringDataWebSupportIntegrationTests { /** * @see DATACMNS-475 */ - @Test - @ClassLoaderConfiguration(hidePackage = com.fasterxml.jackson.databind.ObjectMapper.class) public void doesNotRegisterJacksonSpecificComponentsIfJacksonNotPresent() throws Exception { - ApplicationContext context = WebTestUtils.createApplicationContext(classLoaderRule.classLoader, SampleConfig.class); + ApplicationContext context = WebTestUtils.createApplicationContext(HidingClassLoader.hide(ObjectMapper.class), + SampleConfig.class); List names = Arrays.asList(context.getBeanDefinitionNames());