The use of timestamps to replace SNAPSHOT is an old oddity of

bnd-maven-plugin. So we use `-snapshot: SNAPSHOT` to correct this.

We also use `-noextraheaders: true` to remove headers which can break
reproducible builds. This elides `Bnd-LastModified` so we can remove it
from `-removeheaders`.

See https://github.com/bndtools/bnd/blob/master/maven/bnd-maven-plugin/README.md#reproducible-builds

Signed-off-by: BJ Hargrave <bj@bjhargrave.com>
Handle soft proxies for custom assert classes in OSGi bundles (#1979)

When using a composite class loader for defining soft proxies when the
assert class is from a different class loader than the AssertJ classes,
we now use the composite class loader as the ClassLoadingStrategy for
ByteBuddy. This is necessary for Java 9+ to avoid the
ClassLoadingStrategy.UsingLookup default strategy used by AssertJ for
Java 9+. That strategy always defines the proxy classes in the class
loader defining the assert class ignoring the specified class loader.
This change is also beneficial for Java 8 as it avoids the need to
reflectively call the composite class loader to define the proxy
classes.

We refactor the CompositeClassLoader into the
ClassLoadingStrategyFactory class so that we can centralize the logic
for class loader and class loading strategy determination. Assumptions
is updated to use these changes which prepares it for a future where it
could support defining custom assumptions.

Finally, we update the class loader used for the ByteBuddy class caches
to use the class loader of the assert class rather than AssertJ's class
loader as the key. This is important in the OSGi case where multiple
bundles could define custom assertions with the same class name and we
must allow for unique proxy class generation for each bundle.

Fixes #1979

Signed-off-by: BJ Hargrave <bj@bjhargrave.com>

pom.xml

@@ -338,7 +338,9 @@
                   !org.assertj.core.internal.*,\
                   org.assertj.core.*
                 -noclassforname: true
-                -removeheaders: Bnd-LastModified,Private-Package
+                -noextraheaders: true
+                -snapshot: SNAPSHOT
+                -removeheaders: Private-Package
                 -fixupmessages: \
                   "Classes found in the wrong directory...";is:=ignore
               ]]></bnd>
@@ -355,7 +357,9 @@
               <includeClassesDir>false</includeClassesDir>
               <bnd><![CDATA[
                 -includepackage: org.assertj.core.osgi.*
-                -removeheaders: Bnd-LastModified,Private-Package
+                -noextraheaders: true
+                -snapshot: SNAPSHOT
+                -removeheaders: Private-Package
               ]]></bnd>
             </configuration>
           </execution>

src/main/java/org/assertj/core/api/Assumptions.java

@@ -70,6 +70,7 @@
 import java.util.stream.LongStream;
 import java.util.stream.Stream;
 
+import org.assertj.core.api.ClassLoadingStrategyFactory.ClassLoadingStrategyPair;
 import org.assertj.core.api.ThrowableAssert.ThrowingCallable;
 import org.assertj.core.api.recursive.comparison.RecursiveComparisonConfiguration;
 import org.assertj.core.util.CheckReturnValue;
@@ -1277,18 +1278,19 @@ private static <ASSERTION> ASSERTION asAssumption(Class<ASSERTION> assertionType
   @SuppressWarnings("unchecked")
   private static <ASSERTION> Class<? extends ASSERTION> createAssumptionClass(Class<ASSERTION> assertClass) {
     SimpleKey cacheKey = new SimpleKey(assertClass);
-    return (Class<ASSERTION>) CACHE.findOrInsert(Assumptions.class.getClassLoader(),
+    return (Class<ASSERTION>) CACHE.findOrInsert(assertClass.getClassLoader(),
                                                  cacheKey,
                                                  () -> generateAssumptionClass(assertClass));
   }
 
   protected static <ASSERTION> Class<? extends ASSERTION> generateAssumptionClass(Class<ASSERTION> assertionType) {
+    ClassLoadingStrategyPair strategy = classLoadingStrategy(assertionType);
     return BYTE_BUDDY.subclass(assertionType)
                      // TODO ignore non assertion methods ?
                      .method(any())
                      .intercept(ASSUMPTION)
                      .make()
-                     .load(Assumptions.class.getClassLoader(), classLoadingStrategy(assertionType))
+                     .load(strategy.getClassLoader(), strategy.getClassLoadingStrategy())
                      .getLoaded();
   }
 

src/main/java/org/assertj/core/api/ClassLoadingStrategyFactory.java

@@ -12,16 +12,24 @@
  */
 package org.assertj.core.api;
 
+import java.io.IOException;
 import java.lang.invoke.MethodHandles;
 import java.lang.reflect.Method;
+import java.net.URL;
+import java.util.Enumeration;
+import java.util.LinkedHashMap;
+import java.util.Map;
 
+import net.bytebuddy.description.type.TypeDescription;
 import net.bytebuddy.dynamic.loading.ClassInjector;
 import net.bytebuddy.dynamic.loading.ClassLoadingStrategy;
 
 class ClassLoadingStrategyFactory {
 
   private static final MethodHandles.Lookup LOOKUP = MethodHandles.lookup();
   private static final Method PRIVATE_LOOKUP_IN;
+  // Class loader of AssertJ
+  static final ClassLoader ASSERTJ_CLASS_LOADER = ClassLoadingStrategyFactory.class.getClassLoader();
 
   static {
     Method privateLookupIn;
@@ -33,12 +41,25 @@ class ClassLoadingStrategyFactory {
     PRIVATE_LOOKUP_IN = privateLookupIn;
   }
 
-  static ClassLoadingStrategy<ClassLoader> classLoadingStrategy(Class<?> assertClass) {
+  static ClassLoadingStrategyPair classLoadingStrategy(Class<?> assertClass) {
+    // Use ClassLoader of assertion class to allow ByteBuddy to always find it.
+    // This is needed in an OSGi runtime when a custom assertion class is
+    // defined in a different OSGi bundle.
+    ClassLoader assertClassLoader = assertClass.getClassLoader();
+    if (assertClassLoader != ASSERTJ_CLASS_LOADER) {
+      // Return a new CompositeClassLoader if the assertClass is from a
+      // different class loader than AssertJ. Otherwise return the class
+      // loader of AssertJ since there is no need to use a composite class
+      // loader.
+      CompositeClassLoader compositeClassLoader = new CompositeClassLoader(assertClassLoader);
+      return new ClassLoadingStrategyPair(compositeClassLoader, compositeClassLoader);
+    }
     if (ClassInjector.UsingReflection.isAvailable()) {
-      return ClassLoadingStrategy.Default.INJECTION;
+      return new ClassLoadingStrategyPair(assertClassLoader, ClassLoadingStrategy.Default.INJECTION);
     } else if (ClassInjector.UsingLookup.isAvailable()) {
       try {
-        return ClassLoadingStrategy.UsingLookup.of(PRIVATE_LOOKUP_IN.invoke(null, assertClass, LOOKUP));
+        return new ClassLoadingStrategyPair(assertClassLoader,
+            ClassLoadingStrategy.UsingLookup.of(PRIVATE_LOOKUP_IN.invoke(null, assertClass, LOOKUP)));
       } catch (Exception e) {
         throw new IllegalStateException("Could not access package of " + assertClass, e);
       }
@@ -47,4 +68,71 @@ static ClassLoadingStrategy<ClassLoader> classLoadingStrategy(Class<?> assertCla
     }
   }
 
+  // Pair holder of class loader and class loading strategy to use
+  // for ByteBuddy class generation.
+  static class ClassLoadingStrategyPair {
+    private final ClassLoader classLoader;
+    private final ClassLoadingStrategy<ClassLoader> classLoadingStrategy;
+
+    ClassLoadingStrategyPair(ClassLoader classLoader, ClassLoadingStrategy<ClassLoader> classLoadingStrategy) {
+      this.classLoader = classLoader;
+      this.classLoadingStrategy = classLoadingStrategy;
+    }
+
+    ClassLoader getClassLoader() {
+      return classLoader;
+    }
+
+    ClassLoadingStrategy<ClassLoader> getClassLoadingStrategy() {
+      return classLoadingStrategy;
+    }
+  }
+
+  // Composite class loader for when the assert class is from a different
+  // class loader than AssertJ. This can occur in OSGi when the assert class is
+  // from a bundle. The composite class loader provides access to the internal,
+  // non-exported types of AssertJ. ByteBuddy will define the proxy class in the
+  // CompositeClassLoader rather than in the class loader of the assert class.
+  // This means the assert class cannot assume package private access to super
+  // types, interfaces, etc. since the proxy class is defined in a different
+  // class loader (the CompositeClassLoader) than the assert class.
+  static class CompositeClassLoader extends ClassLoader implements ClassLoadingStrategy<ClassLoader> {
+    CompositeClassLoader(ClassLoader parent) {
+      super(parent);
+    }
+
+    @Override
+    protected Class<?> findClass(String name) throws ClassNotFoundException {
+      return ASSERTJ_CLASS_LOADER.loadClass(name);
+    }
+
+    @Override
+    protected URL findResource(String name) {
+      return ASSERTJ_CLASS_LOADER.getResource(name);
+    }
+
+    @Override
+    protected Enumeration<URL> findResources(String name) throws IOException {
+      return ASSERTJ_CLASS_LOADER.getResources(name);
+    }
+
+    @Override
+    public Map<TypeDescription, Class<?>> load(ClassLoader classLoader, Map<TypeDescription, byte[]> types) {
+      Map<TypeDescription, Class<?>> result = new LinkedHashMap<>();
+      for (Map.Entry<TypeDescription, byte[]> entry : types.entrySet()) {
+        TypeDescription typeDescription = entry.getKey();
+        String name = typeDescription.getName();
+        synchronized (getClassLoadingLock(name)) {
+          Class<?> type = findLoadedClass(name);
+          if (type != null) {
+            throw new IllegalStateException("Cannot define already loaded type: " + type);
+          }
+          byte[] typeDefinition = entry.getValue();
+          type = defineClass(name, typeDefinition, 0, typeDefinition.length);
+          result.put(typeDescription, type);
+        }
+      }
+      return result;
+    }
+  }
 }

src/main/java/org/assertj/core/api/SoftProxies.java

@@ -18,13 +18,11 @@
 import static net.bytebuddy.matcher.ElementMatchers.not;
 import static org.assertj.core.api.ClassLoadingStrategyFactory.classLoadingStrategy;
 
-import java.io.IOException;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.InvocationTargetException;
-import java.net.URL;
-import java.util.Enumeration;
 import java.util.List;
 
+import org.assertj.core.api.ClassLoadingStrategyFactory.ClassLoadingStrategyPair;
 import org.assertj.core.api.recursive.comparison.RecursiveComparisonConfiguration;
 
 import net.bytebuddy.ByteBuddy;
@@ -139,7 +137,7 @@ List<AssertionError> errorsCollected() {
   @SuppressWarnings("unchecked")
   private static <ASSERT extends Assert<?, ?>> Class<ASSERT> createSoftAssertionProxyClass(Class<ASSERT> assertClass) {
     SimpleKey cacheKey = new SimpleKey(assertClass);
-    return (Class<ASSERT>) CACHE.findOrInsert(SoftProxies.class.getClassLoader(), cacheKey,
+    return (Class<ASSERT>) CACHE.findOrInsert(assertClass.getClassLoader(), cacheKey,
                                               () -> generateProxyClass(assertClass));
   }
 
@@ -183,6 +181,7 @@ RecursiveComparisonAssert<?> createRecursiveComparisonAssertProxy(RecursiveCompa
   }
 
   static <V> Class<? extends V> generateProxyClass(Class<V> assertClass) {
+    ClassLoadingStrategyPair strategy = classLoadingStrategy(assertClass);
     return BYTE_BUDDY.subclass(assertClass)
                      .defineField(ProxifyMethodChangingTheObjectUnderTest.FIELD_NAME,
                                   ProxifyMethodChangingTheObjectUnderTest.class,
@@ -198,58 +197,11 @@ static <V> Class<? extends V> generateProxyClass(Class<V> assertClass) {
                      .intercept(FieldAccessor.ofField(ProxifyMethodChangingTheObjectUnderTest.FIELD_NAME).setsArgumentAt(0)
                                              .andThen(FieldAccessor.ofField(ErrorCollector.FIELD_NAME).setsArgumentAt(1)))
                      .make()
-                     // Use ClassLoader of soft assertion class to allow ByteBuddy to always find it.
-                     // This is needed in an OSGi runtime when a custom soft assertion class is defined
-                     // in a different OSGi bundle.
-                     .load(CompositeClassLoader.getClassLoader(assertClass), classLoadingStrategy(assertClass))
+                     .load(strategy.getClassLoader(), strategy.getClassLoadingStrategy())
                      .getLoaded();
   }
 
   private static Junction<MethodDescription> methodsNamed(String name) {
     return named(name);
   }
-
-  // Composite class loader for when the assert class is from a different
-  // class loader than AssertJ. This can occur in OSGi when the assert class is
-  // from a bundle. The composite class loader provides access to the internal,
-  // non-exported types of AssertJ. ByteBuddy will define the proxy class in the
-  // CompositeClassLoader rather than in the class loader of the assert class.
-  // This means the assert class cannot assume package private access to super
-  // types, interfaces, etc. since the proxy class is defined in a different
-  // class loader (the CompositeClassLoader) than the assert class.
-  static class CompositeClassLoader extends ClassLoader {
-    // Class loader of AssertJ
-    private static final ClassLoader assertj = SoftProxies.class.getClassLoader();
-
-    // Return a new CompositeClassLoader if the assertClass is from a
-    // different class loader than AssertJ. Otherwise return the class
-    // loader of AssertJ since there is no need to use a composite class
-    // loader.
-    static ClassLoader getClassLoader(Class<?> assertClass) {
-      final ClassLoader other = assertClass.getClassLoader();
-      if (other == assertj) {
-        return other;
-      }
-      return new CompositeClassLoader(other);
-    }
-
-    private CompositeClassLoader(ClassLoader other) {
-      super(other);
-    }
-
-    @Override
-    protected Class<?> findClass(String name) throws ClassNotFoundException {
-      return assertj.loadClass(name);
-    }
-
-    @Override
-    protected URL findResource(String name) {
-      return assertj.getResource(name);
-    }
-
-    @Override
-    protected Enumeration<URL> findResources(String name) throws IOException {
-      return assertj.getResources(name);
-    }
-  }
 }

src/test/java/org/assertj/core/osgi/AssumptionsTest.java

@@ -0,0 +1,49 @@
+/*
+ * 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.
+ *
+ * Copyright 2012-2020 the original author or authors.
+ */
+package org.assertj.core.osgi;
+
+import static java.util.Arrays.asList;
+import static org.assertj.core.api.Assertions.assertThatNoException;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.assertj.core.api.Assumptions.assumeThat;
+import static org.assertj.core.presentation.UnicodeRepresentation.UNICODE_REPRESENTATION;
+
+import org.junit.jupiter.api.Test;
+import org.opentest4j.TestAbortedException;
+
+class AssumptionsTest {
+
+  @Test
+  void should_ignore_test_when_one_of_the_assumption_fails() {
+    assumeThat("foo").isNotEmpty();
+    assertThatThrownBy(() -> assumeThat("bar").isEmpty()).isInstanceOf(TestAbortedException.class);
+  }
+
+  @Test
+  void should_run_test_when_all_assumptions_are_met() {
+    assertThatNoException().isThrownBy(() -> {
+      assumeThat("foo").isNotNull()
+                       .isNotEmpty()
+                       .isEqualTo("foo");
+      assumeThat("bar").contains("ar")
+                       .isNotBlank();
+      assumeThat(asList("John", "Doe", "Jane", "Doe")).as("test description")
+                                                      .withFailMessage("error message")
+                                                      .withRepresentation(UNICODE_REPRESENTATION)
+                                                      .usingElementComparator(String.CASE_INSENSITIVE_ORDER)
+                                                      .filteredOn(string -> string.length() == 4)
+                                                      .containsExactly("JOHN", "JANE");
+    });
+  }
+
+}