ISPN-4220 : RemoteCacheManager.getCache may ignore the forceReturnVal…

…ue flag RemoteCacheManager's internal cacheName2RemoteCache Map now uses a RemoteCacheKey instead of the cacheName String. This allows retrieval of different RemoteCache instances for different forceReturnValues
infinispan · Aug 4, 2015 · c5dc1ef · c5dc1ef
1 parent ea653fd
commit c5dc1ef
Show file tree

Hide file tree

Showing 2 changed files with 62 additions and 4 deletions.
diff --git a/client/hotrod-client/src/main/java/org/infinispan/client/hotrod/RemoteCacheManager.java b/client/hotrod-client/src/main/java/org/infinispan/client/hotrod/RemoteCacheManager.java
@@ -143,7 +143,7 @@ public class RemoteCacheManager implements BasicCacheContainer {
 
 
    private volatile boolean started = false;
-   private final Map<String, RemoteCacheHolder> cacheName2RemoteCache = new HashMap<String, RemoteCacheHolder>();
+   private final Map<RemoteCacheKey, RemoteCacheHolder> cacheName2RemoteCache = new HashMap<>();
    private final AtomicInteger defaultCacheTopologyId = new AtomicInteger(HotRodConstants.DEFAULT_CACHE_TOPOLOGY);
    private Configuration configuration;
    private Codec codec;
@@ -620,7 +620,8 @@ private Properties loadFromStream(InputStream stream) {
    @SuppressWarnings("unchecked")
    private <K, V> RemoteCache<K, V> createRemoteCache(String cacheName, Boolean forceReturnValueOverride) {
       synchronized (cacheName2RemoteCache) {
-         if (!cacheName2RemoteCache.containsKey(cacheName)) {
+         RemoteCacheKey key = new RemoteCacheKey(cacheName, forceReturnValueOverride);
+         if (!cacheName2RemoteCache.containsKey(key)) {
             RemoteCacheImpl<K, V> result = createRemoteCache(cacheName);
             RemoteCacheHolder rcc = new RemoteCacheHolder(result, forceReturnValueOverride == null ? configuration.forceReturnValues() : forceReturnValueOverride);
             AtomicInteger topologyId = cacheName.isEmpty() ? defaultCacheTopologyId : new AtomicInteger(-1);
@@ -635,10 +636,10 @@ private <K, V> RemoteCache<K, V> createRemoteCache(String cacheName, Boolean for
             }
             result.start();
             // If ping on startup is disabled, or cache is defined in server
-            cacheName2RemoteCache.put(cacheName, rcc);
+            cacheName2RemoteCache.put(key, rcc);
             return result;
          } else {
-            return (RemoteCache<K, V>) cacheName2RemoteCache.get(cacheName).remoteCache;
+            return (RemoteCache<K, V>) cacheName2RemoteCache.get(key).remoteCache;
          }
       }
    }
@@ -688,6 +689,35 @@ public static byte[] cacheNameBytes() {
 
 }
 
+class RemoteCacheKey {
+
+   final String cacheName;
+   final boolean forceReturnValue;
+
+   RemoteCacheKey(String cacheName, boolean forceReturnValue) {
+      this.cacheName = cacheName;
+      this.forceReturnValue = forceReturnValue;
+   }
+
+   @Override
+   public boolean equals(Object o) {
+      if (this == o) return true;
+      if (!(o instanceof RemoteCacheKey)) return false;
+
+      RemoteCacheKey that = (RemoteCacheKey) o;
+
+      if (forceReturnValue != that.forceReturnValue) return false;
+      return !(cacheName != null ? !cacheName.equals(that.cacheName) : that.cacheName != null);
+   }
+
+   @Override
+   public int hashCode() {
+      int result = cacheName != null ? cacheName.hashCode() : 0;
+      result = 31 * result + (forceReturnValue ? 1 : 0);
+      return result;
+   }
+}
+
 class RemoteCacheHolder {
    final RemoteCacheImpl<?, ?> remoteCache;
    final boolean forceReturnValue;

diff --git a/client/hotrod-client/src/test/java/org/infinispan/client/hotrod/ForceReturnValuesTest.java b/client/hotrod-client/src/test/java/org/infinispan/client/hotrod/ForceReturnValuesTest.java
@@ -12,6 +12,7 @@
 import java.util.Properties;
 
 import static org.infinispan.server.hotrod.test.HotRodTestingUtil.hotRodCacheConfiguration;
+import static org.testng.AssertJUnit.*;
 
 @Test(testName = "client.hotrod.ForceReturnValuesTest", groups = "functional")
 @CleanupAfterMethod
@@ -54,4 +55,31 @@ public void testForceReturnValues() {
       assert rv != null;
       assert "Value".equals(rv);
    }
+
+   public void testSameInstanceForSameForceReturnValues() {
+      RemoteCache<String, String> rcDontForceReturn = remoteCacheManager.getCache(false);
+      RemoteCache<String, String> rcDontForceReturn2 = remoteCacheManager.getCache(false);
+      assertSame("RemoteCache instances should be the same", rcDontForceReturn, rcDontForceReturn2);
+
+      RemoteCache<String, String> rcForceReturn = remoteCacheManager.getCache(true);
+      RemoteCache<String, String> rcForceReturn2 = remoteCacheManager.getCache(true);
+      assertSame("RemoteCache instances should be the same", rcForceReturn, rcForceReturn2);
+   }
+
+   public void testDifferentInstancesForDifferentForceReturnValues() {
+      RemoteCache<String, String> rcDontForceReturn = remoteCacheManager.getCache(false);
+      RemoteCache<String, String> rcForceReturn = remoteCacheManager.getCache(true);
+      assertNotSame("RemoteCache instances should not be the same", rcDontForceReturn, rcForceReturn);
+
+      String rv = rcDontForceReturn.put("Key", "Value");
+      assertNull(rv);
+      rv = rcDontForceReturn.put("Key", "Value2");
+      assertNull(rv);
+
+      rv = rcForceReturn.put("Key2", "Value");
+      assertNull(rv);
+      rv = rcForceReturn.put("Key2", "Value2");
+      assertNotNull(rv);
+      assertEquals("Previous value should be 'Value'", "Value", rv);
+   }
 }