You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by ds...@apache.org on 2019/01/23 01:39:53 UTC

[geode] branch develop updated: GEODE-6290: change PdxInstance.equals for empty class name (#3091)

This is an automated email from the ASF dual-hosted git repository.

dschneider 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 8d97b01  GEODE-6290: change PdxInstance.equals for empty class name (#3091)
8d97b01 is described below

commit 8d97b01293b54f18526824cee5c4776b5a6df066
Author: Darrel Schneider <ds...@pivotal.io>
AuthorDate: Tue Jan 22 17:39:43 2019 -0800

    GEODE-6290: change PdxInstance.equals for empty class name (#3091)
    
    PdxInstance.equals will only act as if an instance has a field
    with the default value if the class name is not empty.
    If the class name is empty it will now require that both instances
    have all the same identity fields.
    If the class name is empty then neverDeserialize will be called automatically.
---
 .../geode/pdx/PdxInstanceFactoryJUnitTest.java     | 54 ++++++++++++++++++++++
 .../java/org/apache/geode/cache/RegionService.java |  5 +-
 .../java/org/apache/geode/pdx/PdxInstance.java     | 10 ++--
 .../geode/pdx/internal/PdxInstanceFactoryImpl.java |  3 ++
 .../apache/geode/pdx/internal/PdxInstanceImpl.java |  3 ++
 5 files changed, 71 insertions(+), 4 deletions(-)

diff --git a/geode-core/src/integrationTest/java/org/apache/geode/pdx/PdxInstanceFactoryJUnitTest.java b/geode-core/src/integrationTest/java/org/apache/geode/pdx/PdxInstanceFactoryJUnitTest.java
index 5f3ea52..5e850f0 100644
--- a/geode-core/src/integrationTest/java/org/apache/geode/pdx/PdxInstanceFactoryJUnitTest.java
+++ b/geode-core/src/integrationTest/java/org/apache/geode/pdx/PdxInstanceFactoryJUnitTest.java
@@ -1306,6 +1306,34 @@ public class PdxInstanceFactoryJUnitTest {
   }
 
   @Test
+  public void pdxInstanceEqualsReturnsTrueIfSameClassAndMissingFieldHasDefaultValue() {
+    PdxInstanceFactory factory =
+        cache.createPdxInstanceFactory("myPdxInstanceType").neverDeserialize();
+    factory.writeString("fieldOne", "valueOne");
+    factory.writeInt("fieldTwo", 0);
+    PdxInstance instance = factory.create();
+    factory = cache.createPdxInstanceFactory("myPdxInstanceType").neverDeserialize();
+    factory.writeString("fieldOne", "valueOne");
+    PdxInstance instance2 = factory.create();
+
+    assertThat(instance).isEqualTo(instance2);
+  }
+
+  @Test
+  public void pdxInstanceEqualsReturnsFalseIfNoClassAndMissingFieldHasDefaultValue() {
+    PdxInstanceFactory factory =
+        cache.createPdxInstanceFactory("").neverDeserialize();
+    factory.writeString("fieldOne", "valueOne");
+    factory.writeInt("fieldTwo", 0);
+    PdxInstance instance = factory.create();
+    factory = cache.createPdxInstanceFactory("").neverDeserialize();
+    factory.writeString("fieldOne", "valueOne");
+    PdxInstance instance2 = factory.create();
+
+    assertThat(instance).isNotEqualTo(instance2);
+  }
+
+  @Test
   public void undeserializablePdxInstanceWithMultipleEqualFieldsInDifferentOrderAreEqual() {
     PdxInstanceFactory factory =
         cache.createPdxInstanceFactory("myPdxInstanceType").neverDeserialize();
@@ -1328,6 +1356,21 @@ public class PdxInstanceFactoryJUnitTest {
   }
 
   @Test
+  public void pdxInstanceWithNoCallsAndMultipleEqualFieldsInDifferentOrderAreEqual() {
+    PdxInstanceFactory factory =
+        cache.createPdxInstanceFactory("");
+    factory.writeString("fieldOne", "valueOne");
+    factory.writeString("fieldTwo", "valueTwo");
+    PdxInstance instance = factory.create();
+    factory = cache.createPdxInstanceFactory("");
+    factory.writeString("fieldTwo", "valueTwo");
+    factory.writeString("fieldOne", "valueOne");
+    PdxInstance instance2 = factory.create();
+
+    assertThat(instance).isEqualTo(instance2);
+  }
+
+  @Test
   public void normalPdxInstanceAddedToRegionWithPdxReadSerializedFalseAndABadClassThrowsClassNotFoundWhenRegionGet() {
     // make sure the cache has pdx-read-serialized set to false
     this.cache.close();
@@ -1354,6 +1397,17 @@ public class PdxInstanceFactoryJUnitTest {
   }
 
   @Test
+  public void pdxInstanceWithNoClassReturnsFalseFromIsDeserializable() {
+    PdxInstanceFactory factory =
+        cache.createPdxInstanceFactory("");
+    factory.writeString("fieldOne", "valueOne");
+    factory.writeString("fieldTwo", "valueTwo");
+    PdxInstance instance = factory.create();
+
+    assertThat(instance.isDeserializable()).isFalse();
+  }
+
+  @Test
   public void normalPdxInstanceReturnsTrueFromIsDeserializable() {
     PdxInstanceFactory factory = cache.createPdxInstanceFactory("className");
     factory.writeString("fieldOne", "valueOne");
diff --git a/geode-core/src/main/java/org/apache/geode/cache/RegionService.java b/geode-core/src/main/java/org/apache/geode/cache/RegionService.java
index cdf1535..2d1d028 100644
--- a/geode-core/src/main/java/org/apache/geode/cache/RegionService.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/RegionService.java
@@ -74,7 +74,10 @@ public interface RegionService extends AutoCloseable {
    * Returns a factory that can create a {@link PdxInstance}.
    *
    * @param className the fully qualified class name that the PdxInstance will become when it is
-   *        fully deserialized.
+   *        fully deserialized unless {@link PdxInstanceFactory#neverDeserialize()} is called.
+   *        If className is the empty string then no class versioning will be done for that
+   *        PdxInstance and {@link PdxInstanceFactory#neverDeserialize()} will be automatically
+   *        called on the returned factory.
    * @return the factory
    * @throws IllegalArgumentException if className is <code>null</code>.
    * @since GemFire 6.6.2
diff --git a/geode-core/src/main/java/org/apache/geode/pdx/PdxInstance.java b/geode-core/src/main/java/org/apache/geode/pdx/PdxInstance.java
index abd4942..e1201fe 100644
--- a/geode-core/src/main/java/org/apache/geode/pdx/PdxInstance.java
+++ b/geode-core/src/main/java/org/apache/geode/pdx/PdxInstance.java
@@ -47,6 +47,7 @@ public interface PdxInstance extends java.io.Serializable {
 
   /**
    * Return the full name of the class that this pdx instance represents.
+   * An empty string will be returned if no class is associated with this instance.
    *
    * @return the name of the class that this pdx instance represents.
    * @since GemFire 6.6.2
@@ -133,8 +134,10 @@ public interface PdxInstance extends java.io.Serializable {
    * <li>The domain class name must be equal for both PdxInstances
    * <li>Each identity field must be equal.
    * </ol>
-   * If one of the instances does not have a field that the other one does then equals will assume
-   * it has the field with a default value. If a PdxInstance has marked identity fields using
+   * If the domain class is not the empty string then
+   * if one of the instances does not have a field that the other one does
+   * then equals will assume it has the field with a default value.
+   * If a PdxInstance has marked identity fields using
    * {@link PdxWriter#markIdentityField(String) markIdentityField} then only the marked identity
    * fields are its identity fields. Otherwise all its fields are identity fields.
    * <P>
@@ -216,7 +219,8 @@ public interface PdxInstance extends java.io.Serializable {
   /**
    * Returns false if this instance will never be deserialized to a domain class.
    * Instances that never deserialize can be created using
-   * {@link PdxInstanceFactory#neverDeserialize}.
+   * {@link PdxInstanceFactory#neverDeserialize} or by creating a factory with
+   * an empty string as the class name.
    *
    * @return false if this instance will never be deserialized to a domain class.
    *
diff --git a/geode-core/src/main/java/org/apache/geode/pdx/internal/PdxInstanceFactoryImpl.java b/geode-core/src/main/java/org/apache/geode/pdx/internal/PdxInstanceFactoryImpl.java
index 3a9163c..624df7f 100644
--- a/geode-core/src/main/java/org/apache/geode/pdx/internal/PdxInstanceFactoryImpl.java
+++ b/geode-core/src/main/java/org/apache/geode/pdx/internal/PdxInstanceFactoryImpl.java
@@ -42,6 +42,9 @@ public class PdxInstanceFactoryImpl implements PdxInstanceFactory {
       throw new IllegalArgumentException(
           "Class name can not be null when creating a PdxInstanceFactory");
     }
+    if (name.isEmpty()) {
+      expectDomainClass = false;
+    }
     PdxOutputStream pdxOutputStream = new PdxOutputStream();
     this.pdxType = new PdxType(name, expectDomainClass);
     this.writer = new PdxWriterImpl(pdxType, pdxRegistry, pdxOutputStream);
diff --git a/geode-core/src/main/java/org/apache/geode/pdx/internal/PdxInstanceImpl.java b/geode-core/src/main/java/org/apache/geode/pdx/internal/PdxInstanceImpl.java
index f18c53c..be7d8f8 100644
--- a/geode-core/src/main/java/org/apache/geode/pdx/internal/PdxInstanceImpl.java
+++ b/geode-core/src/main/java/org/apache/geode/pdx/internal/PdxInstanceImpl.java
@@ -342,6 +342,9 @@ public class PdxInstanceImpl extends PdxReaderImpl implements InternalPdxInstanc
     SortedSet<PdxField> myFields = ur1.getPdxType().getSortedIdentityFields();
     SortedSet<PdxField> otherFields = ur2.getPdxType().getSortedIdentityFields();
     if (!myFields.equals(otherFields)) {
+      if (ur1.getPdxType().getClassName().isEmpty()) {
+        return false;
+      }
       // It is not ok to modify myFields and otherFields in place so make copies
       myFields = new TreeSet<PdxField>(myFields);
       otherFields = new TreeSet<PdxField>(otherFields);