This is an automated email from the ASF dual-hosted git repository. bschuchardt pushed a commit to branch feature/GEODE-7874 in repository https://gitbox.apache.org/repos/asf/geode.git
commit ba7c5b849786a425c0dfcc05939cfd509d83264e Author: Bruce Schuchardt <[email protected]> AuthorDate: Thu Mar 12 15:02:09 2020 -0700 GEODE-7874 write a serialization backward-compatibility test for geode-membership Split the TestBase class into two classes, one which analyzes DataSerializable-like classes and a subclass that analyzes java-serializable classes. Added an AnalyzeMembershipSerializablesJUnitTest that implements the first TestBase and left other AnalyzeSerializables tests as subclasses of the second (AnalyzeSerializablesJUnitTestBase). I also finally moved the AnalyzeManagementSerializablesJUnitTest into the geode-management module, where it belongs. It wasn't properly testing files in that module before and I had to update its sanctioned list with a couple of new Exceptions that Owen created a while ago. k --- ...java => AnalyzeCoreSerializablesJUnitTest.java} | 2 +- .../AnalyzeDataSerializablesJUnitTestBase.java | 322 +++++++++++++++++ .../AnalyzeSerializablesJUnitTestBase.java | 385 ++++++--------------- .../geode/codeAnalysis/decode/CompiledClass.java | 87 ----- geode-management/build.gradle | 5 + .../AnalyzeManagementSerializablesJUnitTest.java | 5 - .../apache/geode/codeAnalysis/excludedClasses.txt | 0 .../org/apache/geode/codeAnalysis/openBugs.txt | 0 .../codeAnalysis/sanctionedDataSerializables.txt | 0 .../sanctioned-geode-management-serializables.txt | 2 + .../AnalyzeMembershipSerializablesJUnitTest.java | 68 ++++ .../apache/geode/codeAnalysis/excludedClasses.txt | 1 + .../org/apache/geode/codeAnalysis/openBugs.txt | 0 .../codeAnalysis/sanctionedDataSerializables.txt | 72 ++++ 14 files changed, 568 insertions(+), 381 deletions(-) diff --git a/geode-core/src/integrationTest/java/org/apache/geode/codeAnalysis/AnalyzeSerializablesJUnitTest.java b/geode-core/src/integrationTest/java/org/apache/geode/codeAnalysis/AnalyzeCoreSerializablesJUnitTest.java similarity index 92% rename from geode-core/src/integrationTest/java/org/apache/geode/codeAnalysis/AnalyzeSerializablesJUnitTest.java rename to geode-core/src/integrationTest/java/org/apache/geode/codeAnalysis/AnalyzeCoreSerializablesJUnitTest.java index 6d8fec7..52e76e4 100644 --- a/geode-core/src/integrationTest/java/org/apache/geode/codeAnalysis/AnalyzeSerializablesJUnitTest.java +++ b/geode-core/src/integrationTest/java/org/apache/geode/codeAnalysis/AnalyzeCoreSerializablesJUnitTest.java @@ -19,7 +19,7 @@ import org.junit.experimental.categories.Category; import org.apache.geode.test.junit.categories.SerializationTest; @Category({SerializationTest.class}) -public class AnalyzeSerializablesJUnitTest extends AnalyzeSerializablesJUnitTestBase { +public class AnalyzeCoreSerializablesJUnitTest extends AnalyzeSerializablesJUnitTestBase { @Override protected String getModuleName() { diff --git a/geode-junit/src/main/java/org/apache/geode/codeAnalysis/AnalyzeDataSerializablesJUnitTestBase.java b/geode-junit/src/main/java/org/apache/geode/codeAnalysis/AnalyzeDataSerializablesJUnitTestBase.java new file mode 100644 index 0000000..8283725 --- /dev/null +++ b/geode-junit/src/main/java/org/apache/geode/codeAnalysis/AnalyzeDataSerializablesJUnitTestBase.java @@ -0,0 +1,322 @@ +/* + * 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 static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.fail; +import static org.junit.Assert.assertTrue; + +import java.io.BufferedReader; +import java.io.File; +import java.io.FileReader; +import java.io.IOException; +import java.io.InputStream; +import java.io.InvalidClassException; +import java.io.Serializable; +import java.util.ArrayList; +import java.util.Collections; +import java.util.LinkedList; +import java.util.List; +import java.util.Map; + +import org.junit.AfterClass; +import org.junit.Rule; +import org.junit.Test; +import org.junit.experimental.categories.Category; +import org.junit.rules.TestName; + +import org.apache.geode.codeAnalysis.decode.CompiledClass; +import org.apache.geode.codeAnalysis.decode.CompiledMethod; +import org.apache.geode.internal.serialization.BufferDataOutputStream; +import org.apache.geode.internal.serialization.Version; +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. + */ +@Category({SerializationTest.class}) +public abstract class AnalyzeDataSerializablesJUnitTestBase { + + private static final String NEW_LINE = System.getProperty("line.separator"); + + protected static final String FAIL_MESSAGE = NEW_LINE + NEW_LINE + + "If the class is not persisted or sent over the wire add it to the file " + NEW_LINE + "%s" + + NEW_LINE + "Otherwise if this doesn't break backward compatibility, copy the file " + + NEW_LINE + "%s to " + NEW_LINE + "%s."; + protected static final String EXCLUDED_CLASSES_TXT = "excludedClasses.txt"; + private static final String ACTUAL_DATA_SERIALIZABLES_DAT = "actualDataSerializables.dat"; + protected static final String OPEN_BUGS_TXT = "openBugs.txt"; + + /** + * all loaded classes + */ + protected Map<String, CompiledClass> classes; + + private File expectedDataSerializablesFile; + + private List<ClassAndMethodDetails> expectedDataSerializables; + @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" + */ + protected abstract String getModuleName(); + + /** + * Implement this method to return a production class in your module that corresponds to where + * you have put your sanctioned-modulename-serializables.txt file in the production resources + * tree. + */ + protected abstract Class getModuleClass(); + + /** + * Implement this to deserialize an object that was serialized with serializeObject() + */ + protected abstract void deserializeObject(BufferDataOutputStream outputStream) + throws IOException, ClassNotFoundException; + + /** + * Implement this to serialize the given object to the given output stream + */ + protected abstract void serializeObject(Object object, BufferDataOutputStream outputStream) + throws IOException; + + /** + * Prepare your serialization service for use + */ + protected abstract void initializeSerializationService(); + + + + private void loadExpectedDataSerializables() throws Exception { + this.expectedDataSerializablesFile = getResourceAsFile("sanctionedDataSerializables.txt"); + assertThat(this.expectedDataSerializablesFile).exists().canRead(); + + this.expectedDataSerializables = + CompiledClassUtils.loadClassesAndMethods(this.expectedDataSerializablesFile); + } + + public void findClasses() throws Exception { + classes = classProvider.getClasses(); + + List<String> excludedClasses = loadExcludedClasses(getResourceAsFile(EXCLUDED_CLASSES_TXT)); + List<String> openBugs = loadOpenBugs(getResourceAsFile(OPEN_BUGS_TXT)); + + excludedClasses.addAll(openBugs); + removeExclusions(classes, excludedClasses); + } + + + @Test + public void testDataSerializables() throws Exception { + // assumeTrue("Ignoring this test when java version is 9 and above", + // !SystemUtils.isJavaVersionAtLeast(JavaVersion.JAVA_9)); + System.out.println(this.testName.getMethodName() + " starting"); + findClasses(); + loadExpectedDataSerializables(); + + File actualDataSerializablesFile = createEmptyFile(ACTUAL_DATA_SERIALIZABLES_DAT); + System.out.println(this.testName.getMethodName() + " actualDataSerializablesFile=" + + actualDataSerializablesFile.getAbsolutePath()); + + List<ClassAndMethods> actualDataSerializables = findToDatasAndFromDatas(); + CompiledClassUtils.storeClassesAndMethods(actualDataSerializables, actualDataSerializablesFile); + + String diff = + CompiledClassUtils + .diffSortedClassesAndMethods(this.expectedDataSerializables, actualDataSerializables); + if (!diff.isEmpty()) { + System.out.println( + "++++++++++++++++++++++++++++++testDataSerializables found discrepancies++++++++++++++++++++++++++++++++++++"); + System.out.println(diff); + fail(diff + FAIL_MESSAGE, getSrcPathFor(getResourceAsFile(EXCLUDED_CLASSES_TXT)), + actualDataSerializablesFile.getAbsolutePath(), + getSrcPathFor(this.expectedDataSerializablesFile)); + } + } + + @Test + public void testExcludedClassesExistAndDoNotDeserialize() throws Exception { + List<String> excludedClasses = loadExcludedClasses(getResourceAsFile(EXCLUDED_CLASSES_TXT)); + + initializeSerializationService(); + + for (String filePath : excludedClasses) { + String className = filePath.replaceAll("/", "."); + System.out.println("testing class " + className); + + Class excludedClass = Class.forName(className); + assertTrue( + excludedClass.getName() + + " is not Serializable and should be removed from excludedClasses.txt", + Serializable.class.isAssignableFrom(excludedClass)); + + if (!excludedClass.isEnum()) { + final Object excludedInstance; + try { + excludedInstance = excludedClass.newInstance(); + } catch (InstantiationException | IllegalAccessException e) { + // okay - it's in the excludedClasses.txt file after all + // IllegalAccessException means that the constructor is private. + continue; + } + serializeAndDeserializeObject(excludedInstance); + } + } + } + + + + protected void serializeAndDeserializeObject(Object object) throws Exception { + BufferDataOutputStream outputStream = new BufferDataOutputStream(Version.CURRENT); + try { + serializeObject(object, outputStream); + } 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 + System.out.println("Not Serializable: " + object.getClass().getName()); + } + try { + deserializeObject(outputStream); + fail("I was able to deserialize " + object.getClass().getName()); + } catch (InvalidClassException e) { + // expected + } + } + + protected String getSrcPathFor(File file) { + return getSrcPathFor(file, "test"); + } + + private String getSrcPathFor(File file, String testOrMain) { + return file.getAbsolutePath().replace( + "build" + File.separator + "resources" + File.separator + "test", + "src" + File.separator + testOrMain + File.separator + "resources"); + } + + protected List<String> loadExcludedClasses(File exclusionsFile) throws IOException { + List<String> excludedClasses = new LinkedList<>(); + FileReader fr = new FileReader(exclusionsFile); + BufferedReader br = new BufferedReader(fr); + try { + String line; + while ((line = br.readLine()) != null) { + line = line.trim(); + if (!line.isEmpty() && !line.startsWith("#")) { + excludedClasses.add(line); + } + } + } finally { + fr.close(); + } + return excludedClasses; + } + + protected List<String> loadOpenBugs(File exclusionsFile) throws IOException { + List<String> excludedClasses = new LinkedList<>(); + FileReader fr = new FileReader(exclusionsFile); + BufferedReader br = new BufferedReader(fr); + try { + String line; + // each line should have bug#,full-class-name + while ((line = br.readLine()) != null) { + line = line.trim(); + if (!line.isEmpty() && !line.startsWith("#")) { + String[] split = line.split(","); + if (split.length != 2) { + fail("unable to load classes due to malformed line in openBugs.txt: " + line); + } + excludedClasses.add(line.split(",")[1].trim()); + } + } + } finally { + fr.close(); + } + return excludedClasses; + } + + private void removeExclusions(Map<String, CompiledClass> classes, List<String> exclusions) { + for (String exclusion : exclusions) { + exclusion = exclusion.replace('.', '/'); + classes.remove(exclusion); + } + } + + private List<ClassAndMethods> findToDatasAndFromDatas() { + List<ClassAndMethods> result = new ArrayList<>(); + for (Map.Entry<String, CompiledClass> entry : this.classes.entrySet()) { + CompiledClass compiledClass = entry.getValue(); + ClassAndMethods classAndMethods = null; + + for (int i = 0; i < compiledClass.methods.length; i++) { + CompiledMethod method = compiledClass.methods[i]; + + if (!method.isAbstract() && method.descriptor().equals("void")) { + String name = method.name(); + if (name.startsWith("toData") || name.startsWith("fromData")) { + if (classAndMethods == null) { + classAndMethods = new ClassAndMethods(compiledClass); + } + classAndMethods.methods.put(method.name(), method); + } + } + } + if (classAndMethods != null) { + result.add(classAndMethods); + } + } + Collections.sort(result); + return result; + } + + protected File createEmptyFile(String fileName) throws IOException { + File file = new File(fileName); + if (file.exists()) { + assertThat(file.delete()).isTrue(); + } + assertThat(file.createNewFile()).isTrue(); + assertThat(file).exists().canWrite(); + return file; + } + + /** + * Use this method to get a resource stored in the test's resource directory + */ + protected File getResourceAsFile(String resourceName) { + return new File(getClass().getResource(resourceName).getFile()); + } + + /** + * Use this method to get a resource that might be in a JAR file + */ + protected InputStream getResourceAsStream(Class associatedClass, String resourceName) + throws IOException { + return associatedClass.getResource(resourceName).openStream(); + } +} diff --git a/geode-junit/src/main/java/org/apache/geode/codeAnalysis/AnalyzeSerializablesJUnitTestBase.java b/geode-junit/src/main/java/org/apache/geode/codeAnalysis/AnalyzeSerializablesJUnitTestBase.java index fab4dd8..3386628 100644 --- a/geode-junit/src/main/java/org/apache/geode/codeAnalysis/AnalyzeSerializablesJUnitTestBase.java +++ b/geode-junit/src/main/java/org/apache/geode/codeAnalysis/AnalyzeSerializablesJUnitTestBase.java @@ -14,17 +14,14 @@ */ package org.apache.geode.codeAnalysis; -import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.fail; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; -import java.io.BufferedReader; import java.io.ByteArrayInputStream; import java.io.DataInputStream; import java.io.Externalizable; import java.io.File; -import java.io.FileReader; import java.io.IOException; import java.io.InputStream; import java.io.InvalidClassException; @@ -34,153 +31,59 @@ import java.lang.reflect.Modifier; import java.util.ArrayList; import java.util.Collections; import java.util.HashSet; -import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Properties; import java.util.ServiceLoader; import java.util.Set; -import org.junit.AfterClass; import org.junit.Before; -import org.junit.Rule; import org.junit.Test; import org.junit.experimental.categories.Category; -import org.junit.rules.TestName; import org.apache.geode.CancelException; +import org.apache.geode.DataSerializable; import org.apache.geode.DataSerializer; import org.apache.geode.codeAnalysis.decode.CompiledClass; import org.apache.geode.codeAnalysis.decode.CompiledField; -import org.apache.geode.codeAnalysis.decode.CompiledMethod; import org.apache.geode.distributed.ConfigurationProperties; import org.apache.geode.distributed.internal.DistributedSystemService; import org.apache.geode.distributed.internal.DistributionConfig; import org.apache.geode.distributed.internal.DistributionConfigImpl; -import org.apache.geode.internal.HeapDataOutputStream; import org.apache.geode.internal.InternalDataSerializer; +import org.apache.geode.internal.serialization.BufferDataOutputStream; +import org.apache.geode.internal.serialization.DataSerializableFixedID; import org.apache.geode.internal.serialization.Version; import org.apache.geode.pdx.internal.TypeRegistry; import org.apache.geode.test.junit.categories.SerializationTest; -import org.apache.geode.test.junit.rules.ClassAnalysisRule; import org.apache.geode.unsafe.internal.sun.reflect.ReflectionFactory; +/** + * This subclass of AbstractAnalyzeSerializablesTestBase uses DataSerializer and + * InternalDataSerializer. It also performs initialization of the Geode TypeRegistry + */ @Category({SerializationTest.class}) -public abstract class AnalyzeSerializablesJUnitTestBase { - - private static final String NEW_LINE = System.getProperty("line.separator"); - - private static final String FAIL_MESSAGE = NEW_LINE + NEW_LINE - + "If the class is not persisted or sent over the wire add it to the file " + NEW_LINE + "%s" - + NEW_LINE + "Otherwise if this doesn't break backward compatibility, copy the file " - + NEW_LINE + "%s to " + NEW_LINE + "%s."; - private static final String EXCLUDED_CLASSES_TXT = "excludedClasses.txt"; - private static final String ACTUAL_DATA_SERIALIZABLES_DAT = "actualDataSerializables.dat"; - private static final String ACTUAL_SERIALIZABLES_DAT = "actualSerializables.dat"; - private static final String OPEN_BUGS_TXT = "openBugs.txt"; - - /** - * all loaded classes - */ - private Map<String, CompiledClass> classes; - - private File expectedDataSerializablesFile; - private String expectedSerializablesFileName = - "sanctioned-" + getModuleName() + "-serializables.txt"; - - private List<ClassAndMethodDetails> expectedDataSerializables; - private List<ClassAndVariableDetails> expectedSerializables; - - @Rule - public TestName testName = new TestName(); - - @Rule - public ClassAnalysisRule classProvider = new ClassAnalysisRule(getModuleName()); - - private void loadExpectedDataSerializables() throws Exception { - this.expectedDataSerializablesFile = getResourceAsFile("sanctionedDataSerializables.txt"); - assertThat(this.expectedDataSerializablesFile).exists().canRead(); - - this.expectedDataSerializables = - CompiledClassUtils.loadClassesAndMethods(this.expectedDataSerializablesFile); - } - - public void loadExpectedSerializables() throws Exception { - try (InputStream expectedSerializablesStream = - getResourceAsStream(getModuleClass(), expectedSerializablesFileName)) { - // the expectedSerializablesStream will be automatically closed when we exit this block - this.expectedSerializables = - CompiledClassUtils.loadClassesAndVariables(expectedSerializablesStream); - } - } +public abstract class AnalyzeSerializablesJUnitTestBase extends + AnalyzeDataSerializablesJUnitTestBase { + protected static final String ACTUAL_SERIALIZABLES_DAT = "actualSerializables.dat"; - public void findClasses() throws Exception { - classes = classProvider.getClasses(); - - List<String> excludedClasses = loadExcludedClasses(getResourceAsFile(EXCLUDED_CLASSES_TXT)); - List<String> openBugs = loadOpenBugs(getResourceAsFile(OPEN_BUGS_TXT)); + protected String expectedSerializablesFileName = + "sanctioned-" + getModuleName() + "-serializables.txt"; + protected List<ClassAndVariableDetails> expectedSerializables; - excludedClasses.addAll(openBugs); - removeExclusions(classes, excludedClasses); - } @Before public void setUp() throws Exception { TypeRegistry.init(); } - @AfterClass - public static void afterClass() { - ClassAnalysisRule.clearCache(); - } - - private List<DistributedSystemService> initializeServices() { - ServiceLoader<DistributedSystemService> loader = - ServiceLoader.load(DistributedSystemService.class); - List<DistributedSystemService> services = new ArrayList<>(); - for (DistributedSystemService service : loader) { - services.add(service); - } - return services; - } - - - /** - * Override only this one method in sub-classes - */ - protected abstract String getModuleName(); - + @Override protected Class getModuleClass() { + // subclasses should override this if their sanctioned-serializables.txt file is not + // in org.apache.geode.internal return InternalDataSerializer.class; } - @Test - public void testDataSerializables() throws Exception { - // assumeTrue("Ignoring this test when java version is 9 and above", - // !SystemUtils.isJavaVersionAtLeast(JavaVersion.JAVA_9)); - System.out.println(this.testName.getMethodName() + " starting"); - findClasses(); - loadExpectedDataSerializables(); - - File actualDataSerializablesFile = createEmptyFile(ACTUAL_DATA_SERIALIZABLES_DAT); - System.out.println(this.testName.getMethodName() + " actualDataSerializablesFile=" - + actualDataSerializablesFile.getAbsolutePath()); - - List<ClassAndMethods> actualDataSerializables = findToDatasAndFromDatas(); - CompiledClassUtils.storeClassesAndMethods(actualDataSerializables, actualDataSerializablesFile); - - String diff = - CompiledClassUtils - .diffSortedClassesAndMethods(this.expectedDataSerializables, actualDataSerializables); - if (!diff.isEmpty()) { - System.out.println( - "++++++++++++++++++++++++++++++testDataSerializables found discrepancies++++++++++++++++++++++++++++++++++++"); - System.out.println(diff); - fail(diff + FAIL_MESSAGE, getSrcPathFor(getResourceAsFile(EXCLUDED_CLASSES_TXT)), - actualDataSerializablesFile.getAbsolutePath(), - getSrcPathFor(this.expectedDataSerializablesFile)); - } - } @Test public void testSerializables() throws Exception { @@ -208,66 +111,11 @@ public abstract class AnalyzeSerializablesJUnitTestBase { } @Test - public void testExcludedClassesExistAndDoNotDeserialize() throws Exception { - List<String> excludedClasses = loadExcludedClasses(getResourceAsFile(EXCLUDED_CLASSES_TXT)); - Properties properties = new Properties(); - properties.put(ConfigurationProperties.VALIDATE_SERIALIZABLE_OBJECTS, "true"); - properties.put(ConfigurationProperties.SERIALIZABLE_OBJECT_FILTER, "!*"); - DistributionConfig distributionConfig = new DistributionConfigImpl(properties); - InternalDataSerializer.initialize(distributionConfig, initializeServices()); - - for (String filePath : excludedClasses) { - String className = filePath.replaceAll("/", "."); - System.out.println("testing class " + className); - - Class excludedClass = Class.forName(className); - assertTrue( - excludedClass.getName() - + " is not Serializable and should be removed from excludedClasses.txt", - Serializable.class.isAssignableFrom(excludedClass)); - - if (!excludedClass.isEnum()) { - final Object excludedInstance; - try { - excludedInstance = excludedClass.newInstance(); - } catch (InstantiationException | IllegalAccessException e) { - // okay - it's in the excludedClasses.txt file after all - // IllegalAccessException means that the constructor is private. - continue; - } - serializeAndDeserializeObject(excludedInstance); - } - } - } - - - private void serializeAndDeserializeObject(Object object) throws Exception { - HeapDataOutputStream outputStream = new HeapDataOutputStream(Version.CURRENT); - try { - DataSerializer.writeObject(object, outputStream); - } 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 - System.out.println("Not Serializable: " + object.getClass().getName()); - } - try { - DataSerializer - .readObject(new DataInputStream(new ByteArrayInputStream(outputStream.toByteArray()))); - fail("I was able to deserialize " + object.getClass().getName()); - } catch (InvalidClassException e) { - // expected - } - } - - @Test public void testSanctionedClassesExistAndDoDeserialize() throws Exception { loadExpectedSerializables(); Set<String> openBugs = new HashSet<>(loadOpenBugs(getResourceAsFile(OPEN_BUGS_TXT))); - DistributionConfig distributionConfig = new DistributionConfigImpl(new Properties()); - distributionConfig.setValidateSerializableObjects(true); - distributionConfig.setSerializableObjectFilter("!*"); - InternalDataSerializer.initialize(distributionConfig, initializeServices()); + initializeSerializationService(); for (ClassAndVariableDetails details : expectedSerializables) { if (openBugs.contains(details.className)) { @@ -390,109 +238,15 @@ public abstract class AnalyzeSerializablesJUnitTestBase { } } - private void serializeAndDeserializeSanctionedObject(Object object) throws Exception { - HeapDataOutputStream outputStream = new HeapDataOutputStream(Version.CURRENT); - try { - DataSerializer.writeObject(object, outputStream); - } 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); - } - try { - Object instance = DataSerializer - .readObject(new DataInputStream(new ByteArrayInputStream(outputStream.toByteArray()))); - } 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); - } - } - - private String getSrcPathFor(File file) { - return getSrcPathFor(file, "test"); - } - - private String getSrcPathFor(File file, String testOrMain) { - return file.getAbsolutePath().replace( - "build" + File.separator + "resources" + File.separator + "test", - "src" + File.separator + testOrMain + File.separator + "resources"); - } - - private List<String> loadExcludedClasses(File exclusionsFile) throws IOException { - List<String> excludedClasses = new LinkedList<>(); - FileReader fr = new FileReader(exclusionsFile); - BufferedReader br = new BufferedReader(fr); - try { - String line; - while ((line = br.readLine()) != null) { - line = line.trim(); - if (!line.isEmpty() && !line.startsWith("#")) { - excludedClasses.add(line); - } - } - } finally { - fr.close(); - } - return excludedClasses; - } - - private List<String> loadOpenBugs(File exclusionsFile) throws IOException { - List<String> excludedClasses = new LinkedList<>(); - FileReader fr = new FileReader(exclusionsFile); - BufferedReader br = new BufferedReader(fr); - try { - String line; - // each line should have bug#,full-class-name - while ((line = br.readLine()) != null) { - line = line.trim(); - if (!line.isEmpty() && !line.startsWith("#")) { - String[] split = line.split(","); - if (split.length != 2) { - fail("unable to load classes due to malformed line in openBugs.txt: " + line); - } - excludedClasses.add(line.split(",")[1].trim()); - } - } - } finally { - fr.close(); - } - return excludedClasses; - } - - private void removeExclusions(Map<String, CompiledClass> classes, List<String> exclusions) { - for (String exclusion : exclusions) { - exclusion = exclusion.replace('.', '/'); - classes.remove(exclusion); + public void loadExpectedSerializables() throws Exception { + try (InputStream expectedSerializablesStream = + getResourceAsStream(getModuleClass(), expectedSerializablesFileName)) { + // the expectedSerializablesStream will be automatically closed when we exit this block + this.expectedSerializables = + CompiledClassUtils.loadClassesAndVariables(expectedSerializablesStream); } } - private List<ClassAndMethods> findToDatasAndFromDatas() { - List<ClassAndMethods> result = new ArrayList<>(); - for (Map.Entry<String, CompiledClass> entry : this.classes.entrySet()) { - CompiledClass compiledClass = entry.getValue(); - ClassAndMethods classAndMethods = null; - - for (int i = 0; i < compiledClass.methods.length; i++) { - CompiledMethod method = compiledClass.methods[i]; - - if (!method.isAbstract() && method.descriptor().equals("void")) { - String name = method.name(); - if (name.startsWith("toData") || name.startsWith("fromData")) { - if (classAndMethods == null) { - classAndMethods = new ClassAndMethods(compiledClass); - } - classAndMethods.methods.put(method.name(), method); - } - } - } - if (classAndMethods != null) { - result.add(classAndMethods); - } - } - Collections.sort(result); - return result; - } private List<ClassAndVariables> findSerializables() throws IOException { List<ClassAndVariables> result = new ArrayList<>(2000); @@ -507,7 +261,7 @@ public abstract class AnalyzeSerializablesJUnitTestBase { } // System.out.println("processing class " + compiledClass.fullyQualifiedName()); - if (!compiledClass.isInterface() && compiledClass.isSerializableAndNotDataSerializable()) { + if (!compiledClass.isInterface() && isSerializableAndNotDataSerializable(compiledClass)) { ClassAndVariables classAndVariables = new ClassAndVariables(compiledClass); for (int i = 0; i < compiledClass.fields_count; i++) { CompiledField compiledField = compiledClass.fields[i]; @@ -522,28 +276,83 @@ public abstract class AnalyzeSerializablesJUnitTestBase { return result; } - private File createEmptyFile(String fileName) throws IOException { - File file = new File(fileName); - if (file.exists()) { - assertThat(file.delete()).isTrue(); - } - assertThat(file.createNewFile()).isTrue(); - assertThat(file).exists().canWrite(); - return file; + + + @Override + protected void initializeSerializationService() { + Properties properties = new Properties(); + properties.put(ConfigurationProperties.VALIDATE_SERIALIZABLE_OBJECTS, "true"); + properties.put(ConfigurationProperties.SERIALIZABLE_OBJECT_FILTER, "!*"); + DistributionConfig distributionConfig = new DistributionConfigImpl(properties); + InternalDataSerializer.initialize(distributionConfig, initializeServices()); } - /** - * Use this method to get a resource stored in the test's resource directory - */ - private File getResourceAsFile(String resourceName) { - return new File(getClass().getResource(resourceName).getFile()); + @Override + protected void deserializeObject(BufferDataOutputStream outputStream) + throws IOException, ClassNotFoundException { + DataSerializer + .readObject(new DataInputStream(new ByteArrayInputStream(outputStream.toByteArray()))); } - /** - * Use this method to get a resource that might be in a JAR file - */ - private InputStream getResourceAsStream(Class associatedClass, String resourceName) + @Override + protected void serializeObject(Object object, BufferDataOutputStream outputStream) throws IOException { - return associatedClass.getResource(resourceName).openStream(); + DataSerializer.writeObject(object, outputStream); + } + + + private void serializeAndDeserializeSanctionedObject(Object object) throws Exception { + BufferDataOutputStream outputStream = new BufferDataOutputStream(Version.CURRENT); + try { + serializeObject(object, outputStream); + } 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); + } + 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); + } } + + private List<DistributedSystemService> initializeServices() { + ServiceLoader<DistributedSystemService> loader = + ServiceLoader.load(DistributedSystemService.class); + List<DistributedSystemService> services = new ArrayList<>(); + for (DistributedSystemService service : loader) { + services.add(service); + } + return services; + } + + + public boolean isSerializableAndNotDataSerializable(CompiledClass compiledClass) { + // these classes throw exceptions or log ugly messages when you try to load them + // in junit + String name = compiledClass.fullyQualifiedName().replace('/', '.'); + if (name.startsWith("org.apache.geode.internal.shared.NativeCallsJNAImpl") + || name.startsWith("org.apache.geode.internal.statistics.HostStatHelper")) { + return false; + } + try { + Class realClass = Class.forName(name); + return Serializable.class.isAssignableFrom(realClass) + && !DataSerializable.class.isAssignableFrom(realClass) + && !DataSerializableFixedID.class.isAssignableFrom(realClass); + } catch (UnsatisfiedLinkError e) { + System.out.println("Unable to load actual class " + name + " external JNI dependencies"); + } catch (NoClassDefFoundError e) { + System.out.println("Unable to load actual class " + name + " not in JUnit classpath"); + } catch (Throwable e) { + System.out.println("Unable to load actual class " + name + ": " + e); + } + return false; + } + + + } diff --git a/geode-junit/src/main/java/org/apache/geode/codeAnalysis/decode/CompiledClass.java b/geode-junit/src/main/java/org/apache/geode/codeAnalysis/decode/CompiledClass.java index 4b46cd0..dcd87d9 100644 --- a/geode-junit/src/main/java/org/apache/geode/codeAnalysis/decode/CompiledClass.java +++ b/geode-junit/src/main/java/org/apache/geode/codeAnalysis/decode/CompiledClass.java @@ -17,22 +17,14 @@ package org.apache.geode.codeAnalysis.decode; import java.io.DataInputStream; import java.io.File; import java.io.FileInputStream; -import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; -import java.io.PrintStream; -import java.io.Serializable; -import org.apache.geode.DataSerializable; -import org.apache.geode.LogWriter; import org.apache.geode.codeAnalysis.decode.cp.Cp; import org.apache.geode.codeAnalysis.decode.cp.CpClass; import org.apache.geode.codeAnalysis.decode.cp.CpDouble; import org.apache.geode.codeAnalysis.decode.cp.CpLong; import org.apache.geode.codeAnalysis.decode.cp.CpMethodref; -import org.apache.geode.internal.ExitCode; -import org.apache.geode.internal.logging.PureLogWriter; -import org.apache.geode.internal.serialization.DataSerializableFixedID; /** @@ -65,18 +57,6 @@ public class CompiledClass implements Comparable { public int attributes_count; public CompiledAttribute attributes[]; - static LogWriter debugLog; - static PrintStream debugStream; - - static { - try { - debugStream = new PrintStream("loadedClasses.log"); - debugLog = new PureLogWriter(PureLogWriter.ALL_LEVEL, debugStream); - } catch (FileNotFoundException e) { - e.printStackTrace(); - } - } - public static CompiledClass getInstance(File classFile) throws IOException { FileInputStream fstream = new FileInputStream(classFile); DataInputStream source = new DataInputStream(fstream); @@ -181,31 +161,6 @@ public class CompiledClass implements Comparable { return (access_flags & 0x0200) != 0; } - public boolean isSerializableAndNotDataSerializable() { - // these classes throw exceptions or log ugly messages when you try to load them - // in junit - String name = fullyQualifiedName().replace('/', '.'); - if (name.startsWith("org.apache.geode.internal.shared.NativeCallsJNAImpl") - || name.startsWith("org.apache.geode.internal.statistics.HostStatHelper")) { - return false; - } - try { - debugLog.info("isSerializableAndNotDataSerializable loading class " + name); - debugStream.flush(); - Class realClass = Class.forName(name); - return Serializable.class.isAssignableFrom(realClass) - && !DataSerializable.class.isAssignableFrom(realClass) - && !DataSerializableFixedID.class.isAssignableFrom(realClass); - } catch (UnsatisfiedLinkError e) { - System.out.println("Unable to load actual class " + name + " external JNI dependencies"); - } catch (NoClassDefFoundError e) { - System.out.println("Unable to load actual class " + name + " not in JUnit classpath"); - } catch (Throwable e) { - System.out.println("Unable to load actual class " + name + ": " + e); - } - return false; - } - public String fullyQualifiedName() { return ((CpClass) constant_pool[this_class]).className(this); } @@ -223,48 +178,6 @@ public class CompiledClass implements Comparable { return this.fullyQualifiedName().compareTo(otherName); } - public static void main(String argv[]) { - File classFile; - CompiledClass instance; - int idx; - - classFile = null; - try { - classFile = new File(argv[0]); - } catch (NullPointerException e) { - System.err.println("You must give the name of a class file on the command line"); - System.exit(3); - } - if (!classFile.canRead()) { - System.err.println("Unable to read " + argv[0]); - System.exit(3); - } - try { - instance = getInstance(classFile); - System.out.println("Class name is " + instance.fullyQualifiedName()); - System.out.println("Class access is " + instance.accessString()); - System.out.println("Superclass name is " + instance.superClassName()); - System.out.println("Fields:"); - for (idx = 0; idx < instance.fields_count; idx++) { - System.out.println(" " + instance.fields[idx].signature()); - } - System.out.println("Methods:"); - for (idx = 0; idx < instance.methods_count; idx++) { - System.out.println(" " + instance.methods[idx].signature()); - // if (idx == 0) { - System.out.println("..method attributes"); - for (int i = 0; i < instance.methods[idx].attributes_count; i++) { - System.out.println(".." + instance.methods[idx].attributes[i].name(instance)); - } - // } - } - } catch (Throwable e) { - System.err.println("Error reading file: " + e.getMessage()); - System.exit(3); - } - ExitCode.NORMAL.doSystemExit(); - } - public boolean refersToClass(String name) { for (Cp constantPoolEntry : constant_pool) { if (constantPoolEntry instanceof CpClass && diff --git a/geode-management/build.gradle b/geode-management/build.gradle index ccc3270..9ad55ef 100755 --- a/geode-management/build.gradle +++ b/geode-management/build.gradle @@ -43,4 +43,9 @@ dependencies { testCompileOnly(platform(project(':boms:geode-all-bom'))) testCompileOnly('io.swagger:swagger-annotations') + + integrationTestImplementation(project(':geode-dunit')) + integrationTestImplementation(project(':geode-junit')) + integrationTestImplementation('junit:junit') + integrationTestImplementation('org.awaitility:awaitility') } diff --git a/geode-core/src/integrationTest/java/org/apache/geode/codeAnalysis/AnalyzeManagementSerializablesJUnitTest.java b/geode-management/src/integrationTest/java/org/apache/geode/codeAnalysis/AnalyzeManagementSerializablesJUnitTest.java similarity index 85% rename from geode-core/src/integrationTest/java/org/apache/geode/codeAnalysis/AnalyzeManagementSerializablesJUnitTest.java rename to geode-management/src/integrationTest/java/org/apache/geode/codeAnalysis/AnalyzeManagementSerializablesJUnitTest.java index 29f7843..8b8793f 100644 --- a/geode-core/src/integrationTest/java/org/apache/geode/codeAnalysis/AnalyzeManagementSerializablesJUnitTest.java +++ b/geode-management/src/integrationTest/java/org/apache/geode/codeAnalysis/AnalyzeManagementSerializablesJUnitTest.java @@ -25,9 +25,4 @@ public class AnalyzeManagementSerializablesJUnitTest extends AnalyzeSerializable protected String getModuleName() { return "geode-management"; } - - @Override - // Override this method because all geode-management classes will be scanned by the core's - // data serializable test - public void testDataSerializables() throws Exception {} } diff --git a/geode-management/src/integrationTest/resources/org/apache/geode/codeAnalysis/excludedClasses.txt b/geode-management/src/integrationTest/resources/org/apache/geode/codeAnalysis/excludedClasses.txt new file mode 100644 index 0000000..e69de29 diff --git a/geode-management/src/integrationTest/resources/org/apache/geode/codeAnalysis/openBugs.txt b/geode-management/src/integrationTest/resources/org/apache/geode/codeAnalysis/openBugs.txt new file mode 100644 index 0000000..e69de29 diff --git a/geode-management/src/integrationTest/resources/org/apache/geode/codeAnalysis/sanctionedDataSerializables.txt b/geode-management/src/integrationTest/resources/org/apache/geode/codeAnalysis/sanctionedDataSerializables.txt new file mode 100644 index 0000000..e69de29 diff --git a/geode-core/src/main/resources/org/apache/geode/internal/sanctioned-geode-management-serializables.txt b/geode-management/src/main/resources/org/apache/geode/internal/sanctioned-geode-management-serializables.txt similarity index 94% rename from geode-core/src/main/resources/org/apache/geode/internal/sanctioned-geode-management-serializables.txt rename to geode-management/src/main/resources/org/apache/geode/internal/sanctioned-geode-management-serializables.txt index 55886bc..a3d2499 100644 --- a/geode-core/src/main/resources/org/apache/geode/internal/sanctioned-geode-management-serializables.txt +++ b/geode-management/src/main/resources/org/apache/geode/internal/sanctioned-geode-management-serializables.txt @@ -1,3 +1,5 @@ +org/apache/geode/management/api/ClusterManagementException,false,result:org/apache/geode/management/api/ClusterManagementResult +org/apache/geode/management/api/ClusterManagementRealizationException,false,result:org/apache/geode/management/api/ClusterManagementRealizationResult org/apache/geode/management/api/ClusterManagementResult$StatusCode,false org/apache/geode/management/api/CommandType,false org/apache/geode/management/api/RealizationResult,false,memberName:java/lang/String,message:java/lang/String,success:boolean diff --git a/geode-membership/src/integrationTest/java/org/apache/geode/codeAnalysis/AnalyzeMembershipSerializablesJUnitTest.java b/geode-membership/src/integrationTest/java/org/apache/geode/codeAnalysis/AnalyzeMembershipSerializablesJUnitTest.java new file mode 100644 index 0000000..dd3f66c --- /dev/null +++ b/geode-membership/src/integrationTest/java/org/apache/geode/codeAnalysis/AnalyzeMembershipSerializablesJUnitTest.java @@ -0,0 +1,68 @@ +/* + * 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.io.ByteArrayInputStream; +import java.io.DataInputStream; +import java.io.IOException; + +import org.junit.experimental.categories.Category; + +import org.apache.geode.distributed.internal.membership.gms.Services; +import org.apache.geode.internal.serialization.BufferDataOutputStream; +import org.apache.geode.internal.serialization.DSFIDSerializer; +import org.apache.geode.internal.serialization.DSFIDSerializerFactory; +import org.apache.geode.test.junit.categories.MembershipTest; + + +/** + * AnalyzeMembershipSerializablesJUnitTest analyzes the DataSerializableFixedID and + * BasicSerializable implementations in the membership package. It does not test + * java Serializable objects because the DSFIDSerializer that is used by default in the + * membership module does not support java Serializables. + */ +@Category({MembershipTest.class}) +public class AnalyzeMembershipSerializablesJUnitTest extends AnalyzeDataSerializablesJUnitTestBase { + + private final DSFIDSerializer dsfidSerializer = new DSFIDSerializerFactory().create(); + + @Override + protected String getModuleName() { + return "geode-membership"; + } + + @Override + protected Class getModuleClass() { + return Services.class; + } + + @Override + protected void deserializeObject(BufferDataOutputStream outputStream) + throws IOException, ClassNotFoundException { + dsfidSerializer.getObjectDeserializer() + .readObject(new DataInputStream(new ByteArrayInputStream(outputStream.toByteArray()))); + } + + @Override + protected void serializeObject(Object object, BufferDataOutputStream outputStream) + throws IOException { + dsfidSerializer.getObjectSerializer().writeObject(object, outputStream); + } + + @Override + protected void initializeSerializationService() { + Services.registerSerializables(dsfidSerializer); + } +} diff --git a/geode-membership/src/integrationTest/resources/org/apache/geode/codeAnalysis/excludedClasses.txt b/geode-membership/src/integrationTest/resources/org/apache/geode/codeAnalysis/excludedClasses.txt new file mode 100644 index 0000000..8b13789 --- /dev/null +++ b/geode-membership/src/integrationTest/resources/org/apache/geode/codeAnalysis/excludedClasses.txt @@ -0,0 +1 @@ + diff --git a/geode-membership/src/integrationTest/resources/org/apache/geode/codeAnalysis/openBugs.txt b/geode-membership/src/integrationTest/resources/org/apache/geode/codeAnalysis/openBugs.txt new file mode 100644 index 0000000..e69de29 diff --git a/geode-membership/src/integrationTest/resources/org/apache/geode/codeAnalysis/sanctionedDataSerializables.txt b/geode-membership/src/integrationTest/resources/org/apache/geode/codeAnalysis/sanctionedDataSerializables.txt new file mode 100644 index 0000000..c9c9799 --- /dev/null +++ b/geode-membership/src/integrationTest/resources/org/apache/geode/codeAnalysis/sanctionedDataSerializables.txt @@ -0,0 +1,72 @@ +org/apache/geode/distributed/internal/membership/gms/GMSMembershipView,2 +fromData,121 +toData,72 + +org/apache/geode/distributed/internal/membership/gms/MemberIdentifierImpl,6 +fromData,39 +fromDataPre_GFE_7_1_0_0,308 +fromDataPre_GFE_9_0_0_0,308 +toData,35 +toDataPre_GFE_7_1_0_0,290 +toDataPre_GFE_9_0_0_0,280 + +org/apache/geode/distributed/internal/membership/gms/locator/FindCoordinatorRequest,2 +fromData,112 +toData,132 + +org/apache/geode/distributed/internal/membership/gms/locator/FindCoordinatorResponse,2 +fromData,130 +toData,114 + +org/apache/geode/distributed/internal/membership/gms/locator/GetViewRequest,2 +fromData,1 +toData,1 + +org/apache/geode/distributed/internal/membership/gms/locator/GetViewResponse,2 +fromData,20 +toData,17 + +org/apache/geode/distributed/internal/membership/gms/messages/FinalCheckPassedMessage,2 +fromData,20 +toData,17 + +org/apache/geode/distributed/internal/membership/gms/messages/HeartbeatMessage,2 +fromData,11 +toData,11 + +org/apache/geode/distributed/internal/membership/gms/messages/HeartbeatRequestMessage,2 +fromData,30 +toData,27 + +org/apache/geode/distributed/internal/membership/gms/messages/InstallViewMessage,2 +fromData,60 +toData,56 + +org/apache/geode/distributed/internal/membership/gms/messages/JoinRequestMessage,2 +fromData,63 +toData,60 + +org/apache/geode/distributed/internal/membership/gms/messages/JoinResponseMessage,2 +fromData,63 +toData,57 + +org/apache/geode/distributed/internal/membership/gms/messages/LeaveRequestMessage,2 +fromData,28 +toData,25 + +org/apache/geode/distributed/internal/membership/gms/messages/NetworkPartitionMessage,2 +fromData,1 +toData,1 + +org/apache/geode/distributed/internal/membership/gms/messages/RemoveMemberMessage,2 +fromData,28 +toData,25 + +org/apache/geode/distributed/internal/membership/gms/messages/SuspectMembersMessage,2 +fromData,63 +toData,92 + +org/apache/geode/distributed/internal/membership/gms/messages/ViewAckMessage,2 +fromData,40 +toData,37 +
