This is an automated email from the ASF dual-hosted git repository.
bschuchardt pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/develop by this push:
new 9f6eca3 GEODE-5183 require toData/fromData methods to correspond to a
known version
9f6eca3 is described below
commit 9f6eca31659560a9b34f952b5465ee844c8a66a3
Author: Bruce Schuchardt <[email protected]>
AuthorDate: Thu May 10 08:54:07 2018 -0700
GEODE-5183 require toData/fromData methods to correspond to a known version
Methods beginning with toData or fromData in DataSerializable classes
must now correspond to a valid Version or AnalyzeSerializablesJUnitTest
will reject them. I've renamed a few methods and adjusted
sanctionedDataSerializables.txt to remove them (fromDataProblem,
fromData662)
This closes #1928
---
.../geode/distributed/internal/StartupMessage.java | 6 ++--
.../internal/StartupResponseMessage.java | 9 ------
.../geode/internal/InternalInstantiator.java | 12 ++++---
.../cache/persistence/DiskInitFileParser.java | 2 +-
.../cache/persistence/PersistentMemberID.java | 2 +-
.../geode/codeAnalysis/CompiledClassUtils.java | 37 +++++++++++++++++++---
.../codeAnalysis/sanctionedDataSerializables.txt | 12 +++----
7 files changed, 48 insertions(+), 32 deletions(-)
diff --git
a/geode-core/src/main/java/org/apache/geode/distributed/internal/StartupMessage.java
b/geode-core/src/main/java/org/apache/geode/distributed/internal/StartupMessage.java
index 2d326e8..fca3f4e 100644
---
a/geode-core/src/main/java/org/apache/geode/distributed/internal/StartupMessage.java
+++
b/geode-core/src/main/java/org/apache/geode/distributed/internal/StartupMessage.java
@@ -364,7 +364,7 @@ public class StartupMessage extends
HighPriorityDistributionMessage implements A
/**
* Notes a problem that occurs while invoking {@link #fromData}.
*/
- private void fromDataProblem(String s) {
+ private void recordFromDataProblem(String s) {
if (this.fromDataProblems == null) {
this.fromDataProblems = new StringBuffer();
}
@@ -392,7 +392,7 @@ public class StartupMessage extends
HighPriorityDistributionMessage implements A
InternalDataSerializer.register(cName, false, null, null, id);
}
} catch (IllegalArgumentException ex) {
- fromDataProblem(
+ recordFromDataProblem(
LocalizedStrings.StartupMessage_ILLEGALARGUMENTEXCEPTION_WHILE_REGISTERING_A_DATASERIALIZER_0
.toLocalizedString(ex));
}
@@ -409,7 +409,7 @@ public class StartupMessage extends
HighPriorityDistributionMessage implements A
InternalInstantiator.register(instantiatorClassName,
instantiatedClassName, id, false);
}
} catch (IllegalArgumentException ex) {
- fromDataProblem(
+ recordFromDataProblem(
LocalizedStrings.StartupMessage_ILLEGALARGUMENTEXCEPTION_WHILE_REGISTERING_AN_INSTANTIATOR_0
.toLocalizedString(ex));
}
diff --git
a/geode-core/src/main/java/org/apache/geode/distributed/internal/StartupResponseMessage.java
b/geode-core/src/main/java/org/apache/geode/distributed/internal/StartupResponseMessage.java
index bff2d5e..80df0f3 100644
---
a/geode-core/src/main/java/org/apache/geode/distributed/internal/StartupResponseMessage.java
+++
b/geode-core/src/main/java/org/apache/geode/distributed/internal/StartupResponseMessage.java
@@ -193,15 +193,6 @@ public class StartupResponseMessage extends
HighPriorityDistributionMessage
return STARTUP_RESPONSE_MESSAGE;
}
- private void fromDataProblem(String s) {
- if (this.fromDataProblems == null) {
- this.fromDataProblems = new StringBuffer();
- }
-
- this.fromDataProblems.append(s);
- this.fromDataProblems.append(System.getProperty("line.separator", "\n"));
- }
-
@Override
public Version[] getSerializationVersions() {
return null;
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/InternalInstantiator.java
b/geode-core/src/main/java/org/apache/geode/internal/InternalInstantiator.java
index fea2c4c..a2a46db 100644
---
a/geode-core/src/main/java/org/apache/geode/internal/InternalInstantiator.java
+++
b/geode-core/src/main/java/org/apache/geode/internal/InternalInstantiator.java
@@ -889,7 +889,7 @@ public class InternalInstantiator {
DataSerializer.writeObject(this.eventId, out);
}
- private void fromDataProblem(String s) {
+ private void recordFromDataProblem(String s) {
if (this.fromDataProblems == null) {
this.fromDataProblems = new StringBuffer();
}
@@ -913,8 +913,9 @@ public class InternalInstantiator {
InternalDataSerializer.getCachedClass(this.instantiatorClassName); // fix for
bug
// 41206
} catch (ClassNotFoundException ex) {
-
fromDataProblem(LocalizedStrings.InternalInstantiator_COULD_NOT_LOAD_INSTANTIATOR_CLASS_0
- .toLocalizedString(ex));
+ recordFromDataProblem(
+
LocalizedStrings.InternalInstantiator_COULD_NOT_LOAD_INSTANTIATOR_CLASS_0
+ .toLocalizedString(ex));
this.instantiatorClass = null;
}
try {
@@ -922,8 +923,9 @@ public class InternalInstantiator {
InternalDataSerializer.getCachedClass(this.instantiatedClassName); // fix for
bug
// 41206
} catch (ClassNotFoundException ex) {
-
fromDataProblem(LocalizedStrings.InternalInstantiator_COULD_NOT_LOAD_INSTANTIATED_CLASS_0
- .toLocalizedString(ex));
+ recordFromDataProblem(
+
LocalizedStrings.InternalInstantiator_COULD_NOT_LOAD_INSTANTIATED_CLASS_0
+ .toLocalizedString(ex));
this.instantiatedClass = null;
}
}
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/cache/persistence/DiskInitFileParser.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/persistence/DiskInitFileParser.java
index 177274a..56bc038 100644
---
a/geode-core/src/main/java/org/apache/geode/internal/cache/persistence/DiskInitFileParser.java
+++
b/geode-core/src/main/java/org/apache/geode/internal/cache/persistence/DiskInitFileParser.java
@@ -584,7 +584,7 @@ public class DiskInitFileParser {
DataInputStream dis = new DataInputStream(bais);
PersistentMemberID result = new PersistentMemberID();
if (Version.GFE_70.compareTo(gfversion) > 0) {
- result.fromData662(dis);
+ result._fromData662(dis);
} else {
InternalDataSerializer.invokeFromData(result, dis);
}
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/cache/persistence/PersistentMemberID.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/persistence/PersistentMemberID.java
index 7baae21..64d3c08 100644
---
a/geode-core/src/main/java/org/apache/geode/internal/cache/persistence/PersistentMemberID.java
+++
b/geode-core/src/main/java/org/apache/geode/internal/cache/persistence/PersistentMemberID.java
@@ -79,7 +79,7 @@ public class PersistentMemberID implements DataSerializable {
this.name = DataSerializer.readString(in);
}
- public void fromData662(DataInput in) throws IOException,
ClassNotFoundException {
+ public void _fromData662(DataInput in) throws IOException,
ClassNotFoundException {
long diskStoreIdHigh = in.readLong();
long diskStoreIdLow = in.readLong();
this.diskStoreId = new DiskStoreID(diskStoreIdHigh, diskStoreIdLow);
diff --git
a/geode-core/src/test/java/org/apache/geode/codeAnalysis/CompiledClassUtils.java
b/geode-core/src/test/java/org/apache/geode/codeAnalysis/CompiledClassUtils.java
index 468eff9..0de2bf5 100644
---
a/geode-core/src/test/java/org/apache/geode/codeAnalysis/CompiledClassUtils.java
+++
b/geode-core/src/test/java/org/apache/geode/codeAnalysis/CompiledClassUtils.java
@@ -26,18 +26,32 @@ import java.util.ArrayList;
import java.util.Collections;
import java.util.Enumeration;
import java.util.HashMap;
+import java.util.HashSet;
import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
+import java.util.Set;
import java.util.jar.JarEntry;
import java.util.jar.JarFile;
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.internal.Version;
public class CompiledClassUtils {
+
+ static Set<String> allowedDataSerializerMethods;
+
+ static {
+ allowedDataSerializerMethods = new HashSet<>();
+ Version.getAllVersions().iterator().forEachRemaining((version) -> {
+ allowedDataSerializerMethods.add("toDataPre_" +
version.getMethodSuffix());
+ allowedDataSerializerMethods.add("fromDataPre_" +
version.getMethodSuffix());
+ });
+ }
+
/**
* Parse the given class files and return a map of name->Dclass. Any IO
exceptions are consumed by
* this method and written to stderr.
@@ -191,14 +205,27 @@ public class CompiledClassUtils {
ClassAndMethods nc = newclass;
newclass = null;
if (gold.methods.size() != nc.numMethods()) {
- changedClassesSb.append(nc).append(": method count\n");
+ changedClassesSb.append(nc).append(": method count (expected " +
gold.methods.size()
+ + " but found " + nc.numMethods() + ")\n");
continue;
}
boolean comma = false;
for (Map.Entry<String, CompiledMethod> entry : nc.methods.entrySet()) {
CompiledMethod method = entry.getValue();
- String name = method.name();
- Integer goldCode = gold.methods.get(name);
+ String methodName = method.name();
+ if (!methodName.equals("toData") && !methodName.equals("fromData")
+ && !allowedDataSerializerMethods.contains(methodName)) {
+ if (comma) {
+ changedClassesSb.append(", and ");
+ } else {
+ changedClassesSb.append(nc).append(": ");
+ comma = true;
+ }
+ changedClassesSb.append(methodName)
+ .append(" is not a valid method name - doesn't match any
Version");
+ continue;
+ }
+ Integer goldCode = gold.methods.get(methodName);
if (goldCode == null) {
if (comma) {
changedClassesSb.append(", and ");
@@ -206,7 +233,7 @@ public class CompiledClassUtils {
changedClassesSb.append(nc).append(": ");
comma = true;
}
- changedClassesSb.append(name).append(" was added");
+ changedClassesSb.append(methodName).append(" was added");
continue; // only report one diff per class
}
if (goldCode != method.getCode().code.length) {
@@ -216,7 +243,7 @@ public class CompiledClassUtils {
changedClassesSb.append(nc).append(": ");
comma = true;
}
- changedClassesSb.append(name)
+ changedClassesSb.append(methodName)
.append(" (len=" + method.getCode().code.length + ",expected="
+ goldCode + ")");
continue;
}
diff --git
a/geode-core/src/test/resources/org/apache/geode/codeAnalysis/sanctionedDataSerializables.txt
b/geode-core/src/test/resources/org/apache/geode/codeAnalysis/sanctionedDataSerializables.txt
index c4ad894..a6e37a7 100644
---
a/geode-core/src/test/resources/org/apache/geode/codeAnalysis/sanctionedDataSerializables.txt
+++
b/geode-core/src/test/resources/org/apache/geode/codeAnalysis/sanctionedDataSerializables.txt
@@ -186,13 +186,11 @@ org/apache/geode/distributed/internal/ShutdownMessage,2
fromData,27
toData,24
-org/apache/geode/distributed/internal/StartupMessage,3
-fromDataProblem,38
+org/apache/geode/distributed/internal/StartupMessage,2
fromData,293
toData,318
-org/apache/geode/distributed/internal/StartupResponseMessage,3
-fromDataProblem,43
+org/apache/geode/distributed/internal/StartupResponseMessage,2
fromData,220
toData,170
@@ -392,8 +390,7 @@
org/apache/geode/internal/InternalInstantiator$RegistrationContextMessage,2
fromData,14
toData,14
-org/apache/geode/internal/InternalInstantiator$RegistrationMessage,3
-fromDataProblem,38
+org/apache/geode/internal/InternalInstantiator$RegistrationMessage,2
fromData,125
toData,46
@@ -1823,9 +1820,8 @@
org/apache/geode/internal/cache/persistence/MembershipViewRequest$MembershipView
fromData,36
toData,38
-org/apache/geode/internal/cache/persistence/PersistentMemberID,3
+org/apache/geode/internal/cache/persistence/PersistentMemberID,2
fromData,74
-fromData662,66
toData,71
org/apache/geode/internal/cache/persistence/PersistentMemberPattern,2
--
To stop receiving notification emails like this one, please contact
[email protected].
