GEODE-9817: Enable customized source set paths for ClassAnalysisRule (a…

…pache#7121)

Adds support for customizing source set paths of ClassAnalysisRule.

PROBLEM

Modules external to Geode must be structured the same as Geode
source code in order to use ClassAnalysisRule and the
Analyze*Serializables tests. This is necessary to better facilitate
pluggability of modules that need to provide sanctioned serializable
lists.

SOLUTION

Add source set path customization to ClassAnalysisRule, introduce
a new layer of Analyze*Serializables test base classes that can be
directly extended in order to customize source set paths in
ClassAnalysisRule. Also includes improvements to some iterating
of classes during analysis.

geode-connectors/src/integrationTest/java/org/apache/geode/codeAnalysis/AnalyzeConnectorsSerializablesIntegrationTest.java

@@ -23,7 +23,7 @@
 
 @Category(SerializationTest.class)
 public class AnalyzeConnectorsSerializablesIntegrationTest
-    extends AnalyzeSerializablesJUnitTestBase {
+    extends AnalyzeSerializablesWithClassAnalysisRuleTestBase {
 
   @Override
   protected String getModuleName() {

geode-core/src/integrationTest/java/org/apache/geode/codeAnalysis/AnalyzeCoreSerializablesIntegrationTest.java

@@ -22,7 +22,8 @@
 import org.apache.geode.test.junit.categories.SerializationTest;
 
 @Category(SerializationTest.class)
-public class AnalyzeCoreSerializablesIntegrationTest extends AnalyzeSerializablesJUnitTestBase {
+public class AnalyzeCoreSerializablesIntegrationTest
+    extends AnalyzeSerializablesWithClassAnalysisRuleTestBase {
 
   @Override
   protected String getModuleName() {

geode-cq/src/integrationTest/java/org/apache/geode/codeAnalysis/AnalyzeCQSerializablesIntegrationTest.java

@@ -23,7 +23,8 @@
 import org.apache.geode.test.junit.categories.SerializationTest;
 
 @Category({ClientSubscriptionTest.class, SerializationTest.class})
-public class AnalyzeCQSerializablesIntegrationTest extends AnalyzeSerializablesJUnitTestBase {
+public class AnalyzeCQSerializablesIntegrationTest
+    extends AnalyzeSerializablesWithClassAnalysisRuleTestBase {
 
   @Override
   protected String getModuleName() {

geode-dunit/src/integrationTest/java/org/apache/geode/codeAnalysis/AnalyzeDUnitSerializablesIntegrationTest.java

@@ -27,7 +27,9 @@
 import org.apache.geode.test.junit.categories.SerializationTest;
 
 @Category(SerializationTest.class)
-public class AnalyzeDUnitSerializablesIntegrationTest extends AnalyzeSerializablesJUnitTestBase {
+public class AnalyzeDUnitSerializablesIntegrationTest
+    extends AnalyzeSerializablesWithClassAnalysisRuleTestBase {
+
   private static final Logger logger = LogService.getLogger();
 
   private static final Set<Class<?>> IGNORE_CLASSES = new HashSet<>();

geode-for-redis/src/integrationTest/java/org/apache/geode/codeAnalysis/AnalyzeRedisSerializablesIntegrationTest.java

@@ -22,7 +22,8 @@
 import org.apache.geode.test.junit.categories.SerializationTest;
 
 @Category(SerializationTest.class)
-public class AnalyzeRedisSerializablesIntegrationTest extends AnalyzeSerializablesJUnitTestBase {
+public class AnalyzeRedisSerializablesIntegrationTest
+    extends AnalyzeSerializablesWithClassAnalysisRuleTestBase {
 
   @Override
   protected String getModuleName() {

geode-gfsh/src/integrationTest/java/org/apache/geode/codeAnalysis/AnalyzeGfshSerializablesIntegrationTest.java

@@ -22,7 +22,8 @@
 import org.apache.geode.test.junit.categories.SerializationTest;
 
 @Category(SerializationTest.class)
-public class AnalyzeGfshSerializablesIntegrationTest extends AnalyzeSerializablesJUnitTestBase {
+public class AnalyzeGfshSerializablesIntegrationTest
+    extends AnalyzeSerializablesWithClassAnalysisRuleTestBase {
 
   @Override
   protected String getModuleName() {

geode-junit/src/integrationTest/java/org/apache/geode/codeAnalysis/AnalyzeJUnitSerializablesIntegrationTest.java

@@ -28,7 +28,8 @@
 import org.apache.geode.test.junit.internal.JUnitSanctionedSerializablesService;
 
 @Category(SerializationTest.class)
-public class AnalyzeJUnitSerializablesIntegrationTest extends AnalyzeSerializablesJUnitTestBase {
+public class AnalyzeJUnitSerializablesIntegrationTest
+    extends AnalyzeSerializablesWithClassAnalysisRuleTestBase {
 
   private static final Set<Class<?>> IGNORE_CLASSES = new HashSet<>(asList(
       PortfolioNoDS.class, PortfolioPdx.class));

geode-junit/src/main/java/org/apache/geode/codeAnalysis/AnalyzeDataSerializablesJUnitTestBase.java → geode-junit/src/main/java/org/apache/geode/codeAnalysis/AnalyzeDataSerializablesTestBase.java

@@ -16,6 +16,7 @@
 
 import static java.util.Collections.emptyList;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.catchThrowable;
 import static org.assertj.core.api.Assertions.fail;
 import static org.junit.Assert.assertTrue;
 
@@ -36,7 +37,6 @@
 import java.util.Map;
 import java.util.Optional;
 
-import org.junit.AfterClass;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
@@ -47,18 +47,17 @@
 import org.apache.geode.internal.serialization.BufferDataOutputStream;
 import org.apache.geode.internal.serialization.KnownVersion;
 import org.apache.geode.test.junit.categories.SerializationTest;
-import org.apache.geode.test.junit.rules.ClassAnalysisRule;
 
 /**
  * This abstract test class is the basis for all of our AnalyzeModuleNameSerializables tests.
  * Subclasses must provide serialization/deserialization methods.
  *
  * <p>
- * Most tests should subclass {@link AnalyzeSerializablesJUnitTestBase} instead of this
- * class because it ties into geode-core serialization and saves a lot of work.
+ * Most tests should subclass {@link AnalyzeSerializablesWithClassAnalysisRuleTestBase} instead of
+ * this class because it ties into geode-core serialization and saves a lot of work.
  */
 @Category({SerializationTest.class})
-public abstract class AnalyzeDataSerializablesJUnitTestBase {
+public abstract class AnalyzeDataSerializablesTestBase {
 
   private static final Path MODULE_ROOT = Paths.get("..", "..", "..").toAbsolutePath().normalize();
   private static final Path SOURCE_ROOT = MODULE_ROOT.resolve(Paths.get("src"));
@@ -100,14 +99,6 @@ public abstract class AnalyzeDataSerializablesJUnitTestBase {
   @Rule
   public TestName testName = new TestName();
 
-  @Rule
-  public ClassAnalysisRule classProvider = new ClassAnalysisRule(getModuleName());
-
-  @AfterClass
-  public static void afterClass() {
-    ClassAnalysisRule.clearCache();
-  }
-
   /**
    * implement this to return your module's name, such as "geode-core"
    */
@@ -147,8 +138,10 @@ private void loadExpectedDataSerializables() throws Exception {
     }
   }
 
+  protected abstract Map<String, CompiledClass> loadClasses();
+
   public void findClasses() throws Exception {
-    classes = classProvider.getClasses();
+    classes = loadClasses();
 
     List<String> excludedClasses = loadExcludedClasses(getResourceAsFile(EXCLUDED_CLASSES_TXT));
     List<String> openBugs = loadOpenBugs(getResourceAsFile(OPEN_BUGS_TXT));
@@ -209,9 +202,11 @@ public void testExcludedClassesExistAndDoNotDeserialize() throws Exception {
         final Object excludedInstance;
         try {
           excludedInstance = excludedClass.newInstance();
-        } catch (InstantiationException | IllegalAccessException e) {
+        } catch (IllegalAccessException | InstantiationException | NullPointerException e) {
           // okay - it's in the excludedClasses.txt file after all
-          // IllegalAccessException means that the constructor is private.
+          // IllegalAccessException: thrown when non-private constructor does not exist
+          // InstantiationException: thrown when no-arg constructor does not exist
+          // NullPointerException: thrown by constructors that fetch RMI dependencies
           continue;
         }
         serializeAndDeserializeObject(excludedInstance);
@@ -230,17 +225,33 @@ private void serializeAndDeserializeObject(Object object) throws Exception {
     BufferDataOutputStream outputStream = new BufferDataOutputStream(KnownVersion.CURRENT);
     try {
       serializeObject(object, outputStream);
-    } catch (IOException e) {
+    } catch (IOException | NullPointerException e) {
       // some classes, such as BackupLock, are Serializable because the extend something
       // like ReentrantLock but we never serialize them & it doesn't work to try to do so
       System.out.println("Not Serializable: " + object.getClass().getName());
+      return;
+    } catch (Exception e) {
+      /*
+       * NOTE:
+       * this block catches Exception instead of CacheClosedException so that modules without
+       * dependency on geode-core can be tested with Analyze Serializables.
+       *
+       * ex: geode-membership does not depend on geode-core, so it cannot load CacheClosedException
+       */
+      if ("org.apache.geode.cache.CacheClosedException".equals(e.getClass().getName())) {
+        System.out
+            .println("I was unable to serialize " + object.getClass().getName() + " due to " + e);
+        e.printStackTrace();
+        return;
+      }
+      throw e;
     }
-    try {
-      deserializeObject(outputStream);
-      fail("I was able to deserialize " + object.getClass().getName());
-    } catch (InvalidClassException e) {
-      // expected
-    }
+
+    Throwable thrown = catchThrowable(() -> deserializeObject(outputStream));
+
+    assertThat(thrown)
+        .withFailMessage("I was able to deserialize " + object.getClass().getName())
+        .isInstanceOf(InvalidClassException.class);
   }
 
   private String toBuildPathString(File file) {
@@ -338,18 +349,7 @@ private List<ClassAndMethods> findToDatasAndFromDatas() {
   }
 
   File createEmptyFile(String fileName) throws IOException {
-    final String workingDir = System.getProperty("user.dir");
-    final String filePath;
-    if (isIntelliJDir(workingDir)) {
-      String buildDir = workingDir.replace(getModuleName(), "");
-      buildDir =
-          Paths.get(buildDir, "out", "production", "geode." + getModuleName() + ".integrationTest")
-              .toString();
-      filePath = buildDir + File.separator + fileName;
-    } else {
-      filePath = fileName;
-    }
-    File file = new File(filePath);
+    File file = new File(fileName);
     if (file.exists()) {
       assertThat(file.delete()).isTrue();
     }
@@ -358,10 +358,6 @@ File createEmptyFile(String fileName) throws IOException {
     return file;
   }
 
-  private boolean isIntelliJDir(final String workingDir) {
-    return !workingDir.contains("build");
-  }
-
   /**
    * Use this method to get a resource stored in the test's resource directory
    */

geode-junit/src/main/java/org/apache/geode/codeAnalysis/AnalyzeDataSerializablesWithClassAnalysisRuleTestBase.java

@@ -0,0 +1,40 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You 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.apache.geode.codeAnalysis;
+
+import java.util.Map;
+
+import org.junit.AfterClass;
+import org.junit.Rule;
+
+import org.apache.geode.codeAnalysis.decode.CompiledClass;
+import org.apache.geode.test.junit.rules.ClassAnalysisRule;
+
+public abstract class AnalyzeDataSerializablesWithClassAnalysisRuleTestBase
+    extends AnalyzeDataSerializablesTestBase {
+
+  @Rule
+  public ClassAnalysisRule classProvider = new ClassAnalysisRule(getModuleName());
+
+  @AfterClass
+  public static void afterClass() {
+    ClassAnalysisRule.clearCache();
+  }
+
+  @Override
+  protected Map<String, CompiledClass> loadClasses() {
+    return classProvider.getClasses();
+  }
+}

geode-junit/src/main/java/org/apache/geode/codeAnalysis/AnalyzeSerializablesJUnitTestBase.java → geode-junit/src/main/java/org/apache/geode/codeAnalysis/AnalyzeSerializablesTestBase.java

@@ -26,10 +26,10 @@
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
-import java.io.InvalidClassException;
 import java.io.Serializable;
 import java.io.UncheckedIOException;
 import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Modifier;
 import java.nio.file.Path;
 import java.rmi.RemoteException;
@@ -42,8 +42,10 @@
 import java.util.Set;
 
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
+import org.junit.rules.ErrorCollector;
 
 import org.apache.geode.CancelException;
 import org.apache.geode.DataSerializable;
@@ -64,8 +66,8 @@
  * InternalDataSerializer. It also performs initialization of the Geode TypeRegistry
  */
 @Category(SerializationTest.class)
-public abstract class AnalyzeSerializablesJUnitTestBase
-    extends AnalyzeDataSerializablesJUnitTestBase {
+public abstract class AnalyzeSerializablesTestBase
+    extends AnalyzeDataSerializablesTestBase {
 
   private static final String ACTUAL_SERIALIZABLES_DAT = "actualSerializables.dat";
 
@@ -74,6 +76,9 @@ public abstract class AnalyzeSerializablesJUnitTestBase
 
   protected final List<ClassAndVariableDetails> expectedSerializables = new ArrayList<>();
 
+  @Rule
+  public ErrorCollector errorCollector = new ErrorCollector();
+
   @Before
   public void setUp() throws Exception {
     TypeRegistry.init();
@@ -202,12 +207,19 @@ public void testSanctionedClassesExistAndDoDeserialize() throws Exception {
             isThrowable ? sanctionedClass.getDeclaredConstructor(String.class)
                 : sanctionedClass.getDeclaredConstructor((Class<?>[]) null);
         constructor.setAccessible(true);
-        sanctionedInstance =
-            isThrowable ? constructor.newInstance("test throwable") : constructor.newInstance();
+        if (isThrowable) {
+          sanctionedInstance = constructor.newInstance("test throwable");
+        } else {
+          sanctionedInstance = constructor.newInstance();
+        }
         serializeAndDeserializeSanctionedObject(sanctionedInstance);
         continue;
-      } catch (NoSuchMethodException | InstantiationException | IllegalAccessException e) {
+      } catch (NoSuchMethodException | InstantiationException | IllegalAccessException
+          | NullPointerException | InvocationTargetException e) {
         // fall through
+      } catch (Exception e) {
+        errorCollector.addError(e);
+        continue;
       }
       try {
         Class<?> superClass = sanctionedClass;
@@ -339,22 +351,23 @@ protected void serializeObject(Object object, BufferDataOutputStream outputStrea
 
   private void serializeAndDeserializeSanctionedObject(Object object) throws Exception {
     BufferDataOutputStream outputStream = new BufferDataOutputStream(KnownVersion.CURRENT);
+
     try {
       serializeObject(object, outputStream);
     } catch (RemoteException e) {
-      fail(object.getClass().getName() +
-          " is a java.rmi.server.RemoteObject which is not supported by AnalyzeSerializables", e);
-    } catch (IOException e) {
-      // some classes, such as BackupLock, are Serializable because the extend something
-      // like ReentrantLock but we never serialize them & it doesn't work to try to do so
-      throw new AssertionError("Not Serializable: " + object.getClass().getName(), e);
+      // java.rmi.server.RemoteObject which is not supported by AnalyzeSerializables
+    } catch (Exception e) {
+      errorCollector.addError(
+          new AssertionError("I was unable to serialize " + object.getClass().getName(), e));
     }
+
     try {
       deserializeObject(outputStream);
     } catch (CancelException e) {
       // PDX classes fish for a PDXRegistry and find that there is no cache
-    } catch (InvalidClassException e) {
-      fail("I was unable to deserialize " + object.getClass().getName(), e);
+    } catch (Exception e) {
+      errorCollector.addError(
+          new AssertionError("I was unable to deserialize " + object.getClass().getName(), e));
     }
   }