TABQA-7386: Don't return VersionedValue around a null when calling getAllVersioned() on a removed key.

git-svn-id: https://svn.terracotta.org/repo/tc/dso/trunk@24908 7fc7bbf3-cf45-46d4-be06-341739edd864

Author: Tim Wu

dso-l1/src/main/java/com/tc/object/TCObjectServerMapImpl.java

@@ -766,7 +766,7 @@ private void getAllValuesForKeyFromServer(final SetMultimap<ObjectID, Object> ma
           // update the local cache of corresponding TCServerMap
           map.updateLocalCacheIfNecessary(key, data);
           if (versioned) {
-            rv.put(key, new VersionedObject(data, value.getVersion()));
+            rv.put(key, data == null ? null : new VersionedObject(data, value.getVersion()));
           } else {
             rv.put(key, data);
           }

dso-l1/src/test/java/com/tc/object/TCObjectServerMapImplTest.java

@@ -52,6 +52,7 @@ public Void answer(final InvocationOnMock invocation) throws Throwable {
         Map<Object, Object> result = (Map<Object, Object>)invocation.getArguments()[1];
         result.put("foo", new CompoundResponse(new ObjectID(3), 1, 2, 3, 4, 5));
         result.put("bar", new CompoundResponse(new ObjectID(3), 1, 2, 3, 4, 4));
+        result.put("baz", new CompoundResponse(ObjectID.NULL_ID, 0, 0, 0, 0, 0));
         return null;
       }
     }).when(serverMapManager).getMappingForAllKeys(anyMap(), anyMap());
@@ -62,10 +63,12 @@ public Void answer(final InvocationOnMock invocation) throws Throwable {
     SetMultimap<ObjectID, Object> request = HashMultimap.create();
     request.put(objectID, "foo");
     request.put(new ObjectID(2), "bar");
+    request.put(objectID, "baz");
 
     Map<Object, VersionedObject> result = tcObjectServerMap.getAllVersioned(request);
 
     assertThat(result, hasEntry((Object) "foo", new VersionedObject(expirableMapEntry, 5)));
     assertThat(result, hasEntry((Object) "bar", new VersionedObject(expirableMapEntry, 4)));
+    assertThat(result, hasEntry((Object) "baz", null));
   }
 }

toolkit-impl/src/main/java/com/terracotta/toolkit/collections/map/ServerMap.java

@@ -23,7 +23,6 @@
 import org.terracotta.toolkit.store.ToolkitConfigFields.Consistency;
 
 import com.google.common.base.Preconditions;
-import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Maps;
 import com.google.common.collect.SetMultimap;
 import com.tc.abortable.AbortedOperationException;
@@ -1285,16 +1284,24 @@ public VersionedValue<V> getVersionedValue(Object key) {
   @Override
   public Map<K, VersionedValue<V>> getAllVersioned(final SetMultimap<ObjectID, K> mapIdToKeysMap) {
     try {
-      return ImmutableMap.copyOf(Maps.transformEntries((Map)tcObjectServerMap.getAllVersioned((SetMultimap) mapIdToKeysMap),
-          new Maps.EntryTransformer<Object, VersionedObject, VersionedValue<V>>() {
+      return Collections.unmodifiableMap(new HashMap<K, VersionedValue<V>>(
+          Maps.transformEntries((Map)tcObjectServerMap.getAllVersioned((SetMultimap) mapIdToKeysMap),
+              new Maps.EntryTransformer<Object, VersionedObject, VersionedValue<V>>() {
             @Override
             public VersionedValue<V> transformEntry(final Object key, final VersionedObject value) {
+              if (value == null) {
+                return null;
+              }
               V nonExpiredValue = checkAndGetNonExpiredValue((K)key,
                   value.getObject(), GetType.UNLOCKED, true);
-              return new VersionedValueImpl<V>(nonExpiredValue, value.getVersion());
+              if (nonExpiredValue == null) {
+                return null;
+              } else {
+                return new VersionedValueImpl<V>(nonExpiredValue, value.getVersion());
+              }
             }
           }
-      ));
+      )));
     } catch (AbortedOperationException e) {
       throw new ToolkitAbortableOperationException(e);
     }

toolkit-impl/src/test/java/com/terracotta/toolkit/collections/map/ServerMapTest.java

@@ -8,6 +8,7 @@
 import org.terracotta.toolkit.builder.ToolkitCacheConfigBuilder;
 import org.terracotta.toolkit.config.Configuration;
 import org.terracotta.toolkit.internal.cache.VersionedValue;
+import org.terracotta.toolkit.internal.store.ConfigFieldsInternal;
 
 import com.google.common.collect.HashMultimap;
 import com.google.common.collect.Maps;
@@ -24,6 +25,7 @@
 import com.terracotta.toolkit.bulkload.BufferedOperation;
 import com.terracotta.toolkit.collections.servermap.api.ServerMapLocalStoreFactory;
 import com.terracotta.toolkit.factory.impl.ToolkitCacheDistributedTypeFactory;
+import com.terracotta.toolkit.object.serialization.CustomLifespanSerializedMapValue;
 import com.terracotta.toolkit.object.serialization.SerializationStrategy;
 import com.terracotta.toolkit.object.serialization.SerializedMapValue;
 import com.terracotta.toolkit.rejoin.PlatformServiceProvider;
@@ -40,6 +42,7 @@
 import static org.junit.Assert.assertEquals;
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyBoolean;
+import static org.mockito.Matchers.anyInt;
 import static org.mockito.Matchers.eq;
 import static org.mockito.Matchers.isNull;
 import static org.mockito.Mockito.mock;
@@ -76,8 +79,7 @@ public void setUp() throws Exception {
 
   @Test
   public void testCreateRemoveBufferedOperation() throws Exception {
-    ServerMap serverMap = new ServerMap(configuration, "foo");
-    serverMap.__tc_managed(tcObjectServerMap);
+    ServerMap serverMap = getServerMap();
 
     BufferedOperation bo = serverMap.createBufferedOperation(BufferedOperation.Type.REMOVE, "foo", null, 1, 2, 3, 4);
     assertThat(bo.getType(), is(BufferedOperation.Type.REMOVE));
@@ -87,8 +89,7 @@ public void testCreateRemoveBufferedOperation() throws Exception {
 
   @Test
   public void testCreatePutIfAbsentBufferedOperation() throws Exception {
-    ServerMap serverMap = new ServerMap(configuration, "foo");
-    serverMap.__tc_managed(tcObjectServerMap);
+    ServerMap serverMap = getServerMap();
 
     BufferedOperation<String> bo = serverMap.createBufferedOperation(BufferedOperation.Type.PUT_IF_ABSENT, "foo", "bar", 1, 2, 3, 4);
     assertThat(bo.getType(), is(BufferedOperation.Type.PUT_IF_ABSENT));
@@ -101,8 +102,7 @@ public void testCreatePutIfAbsentBufferedOperation() throws Exception {
 
   @Test
   public void testCreatePutBufferedOperation() throws Exception {
-    ServerMap serverMap = new ServerMap(configuration, "foo");
-    serverMap.__tc_managed(tcObjectServerMap);
+    ServerMap serverMap = getServerMap();
 
     BufferedOperation<String> bo = serverMap.createBufferedOperation(BufferedOperation.Type.PUT, "foo", "bar", 4, 3, 2, 1);
     assertThat(bo.getType(), is(BufferedOperation.Type.PUT));
@@ -115,8 +115,7 @@ public void testCreatePutBufferedOperation() throws Exception {
 
   @Test
   public void testDrain() throws Exception {
-    ServerMap serverMap = new ServerMap(configuration, "foo");
-    serverMap.__tc_managed(tcObjectServerMap);
+    ServerMap serverMap = getServerMap();
 
     Map<String, BufferedOperation<String>> operations = new HashMap<String, BufferedOperation<String>>();
     operations.put("foo", serverMap.createBufferedOperation(BufferedOperation.Type.PUT, "foo", "bar", 4, 3, 2, 1));
@@ -131,27 +130,37 @@ public void testDrain() throws Exception {
 
   @Test
   public void testGetAllVersioned() throws Exception {
-    ServerMap serverMap = new ServerMap(configuration, "foo");
-    serverMap.__tc_managed(tcObjectServerMap);
+    ServerMap serverMap = getServerMap();
 
     SetMultimap<ObjectID, Object> request = HashMultimap.create();
     request.putAll(new ObjectID(1), Sets.<Object>newHashSet("a", "b"));
-    request.putAll(new ObjectID(2), Sets.<Object>newHashSet("c", "d"));
+    request.putAll(new ObjectID(2), Sets.<Object>newHashSet("c", "d", "e", "f"));
 
     Map<Object, Object> response = Maps.newHashMap();
     response.put("a", new VersionedObject(mockSerializedMapValue("1"), 1));
     response.put("b", new VersionedObject(mockSerializedMapValue("2"), 2));
     response.put("c", new VersionedObject(mockSerializedMapValue("3"), 3));
     response.put("d", new VersionedObject(mockSerializedMapValue("4"), 4));
+    response.put("e", null);
+    response.put("f", new VersionedObject(mockSerializedMapValue("5", true), 5));
 
     when(tcObjectServerMap.getAllVersioned(request)).thenReturn(response);
 
     Map<String, VersionedValue<String>> result = serverMap.getAllVersioned(request);
-    assertEquals(4, result.size());
+    assertEquals(6, result.size());
     assertThat(result, hasEntry("a", versionedValue("1", 1)));
     assertThat(result, hasEntry("b", versionedValue("2", 2)));
     assertThat(result, hasEntry("c", versionedValue("3", 3)));
     assertThat(result, hasEntry("d", versionedValue("4", 4)));
+    assertThat(result, hasEntry("e", null));
+    assertThat(result, hasEntry("f", null));
+  }
+
+  private ServerMap getServerMap() {
+    ServerMap serverMap = new ServerMap(configuration, "foo");
+    serverMap.__tc_managed(tcObjectServerMap);
+    serverMap.setLockStrategy(ConfigFieldsInternal.LOCK_STRATEGY.LONG_LOCK_STRATEGY);
+    return serverMap;
   }
 
   private static <T> VersionedValue<T> versionedValue(T value, long version) {
@@ -170,4 +179,12 @@ private static SerializedMapValue<String> mockSerializedMapValue(String value) t
         .thenReturn(value);
     return smv;
   }
+
+  private static SerializedMapValue<String> mockSerializedMapValue(String value, boolean expired) throws IOException, ClassNotFoundException {
+    SerializedMapValue<String> smv = mock(CustomLifespanSerializedMapValue.class);
+    when(smv.getDeserializedValue(any(SerializationStrategy.class), anyBoolean(), any(L1ServerMapLocalCacheStore.class), any(), anyBoolean()))
+        .thenReturn(value);
+    when(smv.isExpired(anyInt(), anyInt(), anyInt())).thenReturn(expired);
+    return smv;
+  }
 }