You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by ud...@apache.org on 2016/06/15 16:17:46 UTC

[13/33] incubator-geode git commit: GEODE-1508: fix IllegalStateException from importNewObject

GEODE-1508: fix IllegalStateException from importNewObject

The code in EntryEventImpl that calls importNewObject
is now careful to never call importNewObject with isSerialized false
if isUnretainedNewReferenceOk is false.
The same bug was fixed for importOldObject.
Unit tests have been added for the code in EntryEventImpl
that calls both importNewObject and importOldObject.


Project: http://git-wip-us.apache.org/repos/asf/incubator-geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-geode/commit/d07c966b
Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/d07c966b
Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/d07c966b

Branch: refs/heads/feature/GEODE-420
Commit: d07c966bb6e0a961963433a81d2fa49c913c552d
Parents: be0f7cf
Author: Darrel Schneider <ds...@pivotal.io>
Authored: Tue Jun 7 16:18:33 2016 -0700
Committer: Darrel Schneider <ds...@pivotal.io>
Committed: Mon Jun 13 16:11:28 2016 -0700

----------------------------------------------------------------------
 .../gemfire/internal/cache/EntryEventImpl.java  |  42 +--
 .../internal/cache/EntryEventImplTest.java      | 327 ++++++++++++++++++-
 2 files changed, 331 insertions(+), 38 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/d07c966b/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/EntryEventImpl.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/EntryEventImpl.java b/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/EntryEventImpl.java
index c4849be..f559e2a 100644
--- a/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/EntryEventImpl.java
+++ b/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/EntryEventImpl.java
@@ -1150,12 +1150,10 @@ public class EntryEventImpl
       if (getCachedSerializedNewValue() != null) {
         importer.importNewBytes(getCachedSerializedNewValue(), true);
         return;
-      } else {
-      if (this.newValueBytes != null && this.newValue instanceof CachedDeserializable) {
+      } else if (this.newValueBytes != null && this.newValue instanceof CachedDeserializable) {
         importer.importNewBytes(this.newValueBytes, true);
         return;
       }
-      }
     }
     @Unretained(ENTRY_EVENT_NEW_VALUE) 
     final Object nv = getRawNewValue();
@@ -1163,22 +1161,16 @@ public class EntryEventImpl
       @Unretained(ENTRY_EVENT_NEW_VALUE)
       final StoredObject so = (StoredObject) nv;
       final boolean isSerialized = so.isSerialized();
-      if (so.hasRefCount()) {
-        if (importer.isUnretainedNewReferenceOk()) {
-          importer.importNewObject(nv, isSerialized);
-        } else {
-          if (!isSerialized || prefersSerialized) {
-            byte[] bytes = so.getValueAsHeapByteArray();
-            importer.importNewBytes(bytes, isSerialized);
-            if (isSerialized) {
-              setCachedSerializedNewValue(bytes);
-            }
-          } else {
-            importer.importNewObject(so.getValueAsDeserializedHeapObject(), true);
-          }
+      if (importer.isUnretainedNewReferenceOk()) {
+        importer.importNewObject(nv, isSerialized);
+      } else if (!isSerialized || prefersSerialized) {
+        byte[] bytes = so.getValueAsHeapByteArray();
+        importer.importNewBytes(bytes, isSerialized);
+        if (isSerialized) {
+          setCachedSerializedNewValue(bytes);
         }
       } else {
-        importer.importNewObject(nv, isSerialized);
+        importer.importNewObject(so.getValueAsDeserializedHeapObject(), true);
       }
     } else if (nv instanceof byte[]) {
       importer.importNewBytes((byte[])nv, false);
@@ -1248,18 +1240,12 @@ public class EntryEventImpl
     if (ov instanceof StoredObject) {
       final StoredObject so = (StoredObject) ov;
       final boolean isSerialized = so.isSerialized();
-      if (so.hasRefCount()) {
-        if (importer.isUnretainedOldReferenceOk()) {
-          importer.importOldObject(ov, isSerialized);
-        } else {
-          if (!isSerialized || prefersSerialized) {
-            importer.importOldBytes(so.getValueAsHeapByteArray(), isSerialized);
-          } else {
-           importer.importOldObject(so.getValueAsDeserializedHeapObject(), true);
-          }
-        }
-      } else {
+      if (importer.isUnretainedOldReferenceOk()) {
         importer.importOldObject(ov, isSerialized);
+      } else if (!isSerialized || prefersSerialized) {
+        importer.importOldBytes(so.getValueAsHeapByteArray(), isSerialized);
+      } else {
+        importer.importOldObject(so.getValueAsDeserializedHeapObject(), true);
       }
     } else if (ov instanceof byte[]) {
       importer.importOldBytes((byte[])ov, false);

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/d07c966b/geode-core/src/test/java/com/gemstone/gemfire/internal/cache/EntryEventImplTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/com/gemstone/gemfire/internal/cache/EntryEventImplTest.java b/geode-core/src/test/java/com/gemstone/gemfire/internal/cache/EntryEventImplTest.java
index 800527f..265795f 100644
--- a/geode-core/src/test/java/com/gemstone/gemfire/internal/cache/EntryEventImplTest.java
+++ b/geode-core/src/test/java/com/gemstone/gemfire/internal/cache/EntryEventImplTest.java
@@ -24,33 +24,33 @@ import org.junit.Test;
 import org.junit.experimental.categories.Category;
 
 import com.gemstone.gemfire.cache.Operation;
+import com.gemstone.gemfire.internal.cache.EntryEventImpl.NewValueImporter;
+import com.gemstone.gemfire.internal.cache.EntryEventImpl.OldValueImporter;
+import com.gemstone.gemfire.internal.offheap.StoredObject;
 import com.gemstone.gemfire.test.junit.categories.UnitTest;
 
 @Category(UnitTest.class)
 public class EntryEventImplTest {
 
-  private String expectedRegionName;
   private String key;
-  private String value;
-  private KeyInfo keyInfo;
 
   @Before
   public void setUp() throws Exception {
-    expectedRegionName = "ExpectedFullRegionPathName";
     key = "key1";
-    value = "value1";
-    keyInfo = new KeyInfo(key, value, null);
   }
 
   @Test
   public void verifyToStringOutputHasRegionName() {
     // mock a region object
     LocalRegion region = mock(LocalRegion.class);
+    String expectedRegionName = "ExpectedFullRegionPathName";
+    String value = "value1";
+    KeyInfo keyInfo = new KeyInfo(key, value, null);
     doReturn(expectedRegionName).when(region).getFullPath();
     doReturn(keyInfo).when(region).getKeyInfo(any(), any(), any());
 
-    // create entryevent for the region
-    EntryEventImpl e = createEntryEvent(region);
+    // create entry event for the region
+    EntryEventImpl e = createEntryEvent(region, value);
     
     // The name of the region should be in the toString text
     String toStringValue = e.toString();
@@ -59,15 +59,322 @@ public class EntryEventImplTest {
     // verify that toString called getFullPath method of region object
     verify(region, times(1)).getFullPath();
   }
+  
+  @Test
+  public void verifyExportNewValueWithUnserializedStoredObject() {
+    LocalRegion region = mock(LocalRegion.class);
+    StoredObject newValue = mock(StoredObject.class);
+    byte[] newValueBytes = new byte[]{1,2,3};
+    when(newValue.getValueAsHeapByteArray()).thenReturn(newValueBytes);
+    NewValueImporter nvImporter = mock(NewValueImporter.class);
+    EntryEventImpl e = createEntryEvent(region, newValue);
+    
+    e.exportNewValue(nvImporter);
+    
+    verify(nvImporter).importNewBytes(newValueBytes, false);
+  }
+
+  @Test
+  public void verifyExportNewValueWithByteArray() {
+    LocalRegion region = mock(LocalRegion.class);
+    byte[] newValue = new byte[]{1,2,3};
+    NewValueImporter nvImporter = mock(NewValueImporter.class);
+    EntryEventImpl e = createEntryEvent(region, newValue);
+    
+    e.exportNewValue(nvImporter);
+    
+    verify(nvImporter).importNewBytes(newValue, false);
+  }
+
+  @Test
+  public void verifyExportNewValueWithStringIgnoresNewValueBytes() {
+    LocalRegion region = mock(LocalRegion.class);
+    String newValue = "newValue";
+    NewValueImporter nvImporter = mock(NewValueImporter.class);
+    when(nvImporter.prefersNewSerialized()).thenReturn(true);
+    EntryEventImpl e = createEntryEvent(region, newValue);
+    byte[] newValueBytes = new byte[] {1,2};
+    e.newValueBytes = newValueBytes;
+    
+    e.exportNewValue(nvImporter);
+    
+    verify(nvImporter).importNewObject(newValue, true);
+  }
+  
+  @Test
+  public void verifyExportNewValueWithByteArrayCachedDeserializable() {
+    LocalRegion region = mock(LocalRegion.class);
+    CachedDeserializable newValue = mock(CachedDeserializable.class);
+    byte[] newValueBytes = new byte[] {1,2,3};
+    when(newValue.getValue()).thenReturn(newValueBytes);
+    NewValueImporter nvImporter = mock(NewValueImporter.class);
+    EntryEventImpl e = createEntryEvent(region, newValue);
+    
+    e.exportNewValue(nvImporter);
+    
+    verify(nvImporter).importNewBytes(newValueBytes, true);
+  }
+  
+  @Test
+  public void verifyExportNewValueWithStringCachedDeserializable() {
+    LocalRegion region = mock(LocalRegion.class);
+    CachedDeserializable newValue = mock(CachedDeserializable.class);
+    Object newValueObj = "newValueObj";
+    when(newValue.getValue()).thenReturn(newValueObj);
+    NewValueImporter nvImporter = mock(NewValueImporter.class);
+    EntryEventImpl e = createEntryEvent(region, newValue);
+    byte[] newValueBytes = new byte[] {1,2};
+    e.newValueBytes = newValueBytes;
+    e.setCachedSerializedNewValue(newValueBytes);
+    
+    e.exportNewValue(nvImporter);
+    
+    verify(nvImporter).importNewObject(newValueObj, true);
+  }
+
+  @Test
+  public void verifyExportNewValueWithStringCachedDeserializablePrefersNewValueBytes() {
+    LocalRegion region = mock(LocalRegion.class);
+    CachedDeserializable newValue = mock(CachedDeserializable.class);
+    Object newValueObj = "newValueObj";
+    when(newValue.getValue()).thenReturn(newValueObj);
+    NewValueImporter nvImporter = mock(NewValueImporter.class);
+    when(nvImporter.prefersNewSerialized()).thenReturn(true);
+    EntryEventImpl e = createEntryEvent(region, newValue);
+    byte[] newValueBytes = new byte[] {1,2};
+    e.newValueBytes = newValueBytes;
+    
+    e.exportNewValue(nvImporter);
+    
+    verify(nvImporter).importNewBytes(newValueBytes, true);
+  }
+
+  @Test
+  public void verifyExportNewValueWithStringCachedDeserializablePrefersCachedSerializedNewValue() {
+    LocalRegion region = mock(LocalRegion.class);
+    CachedDeserializable newValue = mock(CachedDeserializable.class);
+    Object newValueObj = "newValueObj";
+    when(newValue.getValue()).thenReturn(newValueObj);
+    NewValueImporter nvImporter = mock(NewValueImporter.class);
+    when(nvImporter.prefersNewSerialized()).thenReturn(true);
+    EntryEventImpl e = createEntryEvent(region, newValue);
+    byte[] newValueBytes = new byte[] {1,2};
+    e.setCachedSerializedNewValue(newValueBytes);
+    
+    e.exportNewValue(nvImporter);
+    
+    verify(nvImporter).importNewBytes(newValueBytes, true);
+  }
+
+  @Test
+  public void verifyExportNewValueWithSerializedStoredObjectAndImporterPrefersSerialized() {
+    LocalRegion region = mock(LocalRegion.class);
+    StoredObject newValue = mock(StoredObject.class);
+    when(newValue.isSerialized()).thenReturn(true);
+    byte[] newValueBytes = new byte[]{1,2,3};
+    when(newValue.getValueAsHeapByteArray()).thenReturn(newValueBytes);
+    NewValueImporter nvImporter = mock(NewValueImporter.class);
+    when(nvImporter.prefersNewSerialized()).thenReturn(true);
+    EntryEventImpl e = createEntryEvent(region, newValue);
+    
+    e.exportNewValue(nvImporter);
+    
+    verify(nvImporter).importNewBytes(newValueBytes, true);
+  }
+
+  @Test
+  public void verifyExportNewValueWithSerializedStoredObject() {
+    LocalRegion region = mock(LocalRegion.class);
+    StoredObject newValue = mock(StoredObject.class);
+    when(newValue.isSerialized()).thenReturn(true);
+    Object newValueDeserialized = "newValueDeserialized";
+    when(newValue.getValueAsDeserializedHeapObject()).thenReturn(newValueDeserialized);
+    NewValueImporter nvImporter = mock(NewValueImporter.class);
+    EntryEventImpl e = createEntryEvent(region, newValue);
+    
+    e.exportNewValue(nvImporter);
+    
+    verify(nvImporter).importNewObject(newValueDeserialized, true);
+  }
+
+  @Test
+  public void verifyExportNewValueWithSerializedStoredObjectAndUnretainedNewReferenceOk() {
+    LocalRegion region = mock(LocalRegion.class);
+    StoredObject newValue = mock(StoredObject.class);
+    when(newValue.isSerialized()).thenReturn(true);
+    Object newValueDeserialized = "newValueDeserialized";
+    when(newValue.getValueAsDeserializedHeapObject()).thenReturn(newValueDeserialized);
+    NewValueImporter nvImporter = mock(NewValueImporter.class);
+    when(nvImporter.isUnretainedNewReferenceOk()).thenReturn(true);
+    EntryEventImpl e = createEntryEvent(region, newValue);
+    
+    e.exportNewValue(nvImporter);
+    
+    verify(nvImporter).importNewObject(newValue, true);
+  }
+
+  @Test
+  public void verifyExportOldValueWithUnserializedStoredObject() {
+    LocalRegion region = mock(LocalRegion.class);
+    StoredObject oldValue = mock(StoredObject.class);
+    byte[] oldValueBytes = new byte[]{1,2,3};
+    when(oldValue.getValueAsHeapByteArray()).thenReturn(oldValueBytes);
+    OldValueImporter ovImporter = mock(OldValueImporter.class);
+    EntryEventImpl e = createEntryEvent(region, null);
+    e.setOldValue(oldValue);
+    
+    e.exportOldValue(ovImporter);
+    
+    verify(ovImporter).importOldBytes(oldValueBytes, false);
+  }
+
+  @Test
+  public void verifyExportOldValueWithByteArray() {
+    LocalRegion region = mock(LocalRegion.class);
+    byte[] oldValue = new byte[]{1,2,3};
+    OldValueImporter ovImporter = mock(OldValueImporter.class);
+    EntryEventImpl e = createEntryEvent(region, null);
+    e.setOldValue(oldValue);
+    
+    e.exportOldValue(ovImporter);
+    
+    verify(ovImporter).importOldBytes(oldValue, false);
+  }
+
+  @Test
+  public void verifyExportOldValueWithStringIgnoresOldValueBytes() {
+    LocalRegion region = mock(LocalRegion.class);
+    String oldValue = "oldValue";
+    OldValueImporter ovImporter = mock(OldValueImporter.class);
+    when(ovImporter.prefersOldSerialized()).thenReturn(true);
+    EntryEventImpl e = createEntryEvent(region, null);
+    byte[] oldValueBytes = new byte[]{1,2,3};
+    e.setSerializedOldValue(oldValueBytes);
+    e.setOldValue(oldValue);
+    
+    e.exportOldValue(ovImporter);
+    
+    verify(ovImporter).importOldObject(oldValue, true);
+  }
+
+  @Test
+  public void verifyExportOldValuePrefersOldValueBytes() {
+    LocalRegion region = mock(LocalRegion.class);
+    OldValueImporter ovImporter = mock(OldValueImporter.class);
+    when(ovImporter.prefersOldSerialized()).thenReturn(true);
+    EntryEventImpl e = createEntryEvent(region, null);
+    byte[] oldValueBytes = new byte[]{1,2,3};
+    e.setSerializedOldValue(oldValueBytes);
+    
+    e.exportOldValue(ovImporter);
+    
+    verify(ovImporter).importOldBytes(oldValueBytes, true);
+  }
+
+  @Test
+  public void verifyExportOldValueWithCacheDeserializableByteArray() {
+    LocalRegion region = mock(LocalRegion.class);
+    CachedDeserializable oldValue = mock(CachedDeserializable.class);
+    byte[] oldValueBytes = new byte[]{1,2,3};
+    when(oldValue.getValue()).thenReturn(oldValueBytes);
+    OldValueImporter ovImporter = mock(OldValueImporter.class);
+    EntryEventImpl e = createEntryEvent(region, null);
+    e.setOldValue(oldValue);
+    
+    e.exportOldValue(ovImporter);
+    
+    verify(ovImporter).importOldBytes(oldValueBytes, true);
+  }
+
+  @Test
+  public void verifyExportOldValueWithCacheDeserializableString() {
+    LocalRegion region = mock(LocalRegion.class);
+    CachedDeserializable oldValue = mock(CachedDeserializable.class);
+    Object oldValueObj = "oldValueObj";
+    when(oldValue.getValue()).thenReturn(oldValueObj);
+    OldValueImporter ovImporter = mock(OldValueImporter.class);
+    EntryEventImpl e = createEntryEvent(region, null);
+    e.setOldValue(oldValue);
+    
+    e.exportOldValue(ovImporter);
+    
+    verify(ovImporter).importOldObject(oldValueObj, true);
+  }
+
+  @Test
+  public void verifyExportOldValueWithCacheDeserializableOk() {
+    LocalRegion region = mock(LocalRegion.class);
+    CachedDeserializable oldValue = mock(CachedDeserializable.class);
+    Object oldValueObj = "oldValueObj";
+    when(oldValue.getValue()).thenReturn(oldValueObj);
+    OldValueImporter ovImporter = mock(OldValueImporter.class);
+    when(ovImporter.isCachedDeserializableValueOk()).thenReturn(true);
+    EntryEventImpl e = createEntryEvent(region, null);
+    e.setOldValue(oldValue);
+    
+    e.exportOldValue(ovImporter);
+    
+    verify(ovImporter).importOldObject(oldValue, true);
+  }
+
+  @Test
+  public void verifyExportOldValueWithSerializedStoredObjectAndImporterPrefersSerialized() {
+    LocalRegion region = mock(LocalRegion.class);
+    StoredObject oldValue = mock(StoredObject.class);
+    when(oldValue.isSerialized()).thenReturn(true);
+    byte[] oldValueBytes = new byte[]{1,2,3};
+    when(oldValue.getValueAsHeapByteArray()).thenReturn(oldValueBytes);
+    OldValueImporter ovImporter = mock(OldValueImporter.class);
+    when(ovImporter.prefersOldSerialized()).thenReturn(true);
+    EntryEventImpl e = createEntryEvent(region, null);
+    e.setOldValue(oldValue);
+    
+    e.exportOldValue(ovImporter);
+    
+    verify(ovImporter).importOldBytes(oldValueBytes, true);
+  }
+
+  @Test
+  public void verifyExportOldValueWithSerializedStoredObject() {
+    LocalRegion region = mock(LocalRegion.class);
+    StoredObject oldValue = mock(StoredObject.class);
+    when(oldValue.isSerialized()).thenReturn(true);
+    Object oldValueDeserialized = "newValueDeserialized";
+    when(oldValue.getValueAsDeserializedHeapObject()).thenReturn(oldValueDeserialized);
+    OldValueImporter ovImporter = mock(OldValueImporter.class);
+    EntryEventImpl e = createEntryEvent(region, null);
+    e.setOldValue(oldValue);
+    
+    e.exportOldValue(ovImporter);
+    
+    verify(ovImporter).importOldObject(oldValueDeserialized, true);
+  }
+
+  @Test
+  public void verifyExportOewValueWithSerializedStoredObjectAndUnretainedOldReferenceOk() {
+    LocalRegion region = mock(LocalRegion.class);
+    StoredObject oldValue = mock(StoredObject.class);
+    when(oldValue.isSerialized()).thenReturn(true);
+    Object oldValueDeserialized = "oldValueDeserialized";
+    when(oldValue.getValueAsDeserializedHeapObject()).thenReturn(oldValueDeserialized);
+    OldValueImporter ovImporter = mock(OldValueImporter.class);
+    when(ovImporter.isUnretainedOldReferenceOk()).thenReturn(true);
+    EntryEventImpl e = createEntryEvent(region, null);
+    e.setOldValue(oldValue);
+    
+    e.exportOldValue(ovImporter);
+    
+    verify(ovImporter).importOldObject(oldValue, true);
+  }
 
-  private EntryEventImpl createEntryEvent(LocalRegion l) {
+  private EntryEventImpl createEntryEvent(LocalRegion l, Object newValue) {
     // create a dummy event id
     byte[] memId = { 1,2,3 };
     EventID eventId = new EventID(memId, 11, 12, 13);
 
     // create an event
     EntryEventImpl event = EntryEventImpl.create(l, Operation.CREATE, key,
-        value, null,  false /* origin remote */, null,
+        newValue, null,  false /* origin remote */, null,
         false /* generateCallbacks */,
         eventId);
     // avoid calling invokeCallbacks