fix: cache hashed keys per hashing algorithm, length and encoder in EVCacheKey

evcache-core/src/main/java/com/netflix/evcache/EVCacheKey.java

@@ -20,10 +20,8 @@ public class EVCacheKey {
     private final String key;
     private final String canonicalKey;
     private String canonicalKeyForDuet;
-    // Note that this we cache hashed keys based on Hashing Algorithm alone, but not based on other hashing properties
-    // like max.hash.length. So changing max.hash.length alone would not necessarily trigger hash recalculation, but
-    // one would have to change the hashing algorithm in order to having hashing properties taken into account.
-    // This is to make such a hashing property change very obvious and not subtle.
+    // Hashed keys are cached by hashing algorithm, max digest bytes, max hash length and encoder, so clients
+    // (server groups) with different hashing properties each get a hash computed with their own properties.
     private final Map<String, String> hashedKeysByAlgorithm;
     private final Map<String, String> hashedKeysByAlgorithmForDuet;
     private final String encoder;
@@ -122,7 +120,7 @@ private String getHashKey(HashingAlgorithm hashingAlgorithm, Boolean shouldEncod
             return null;
         }
 
-        final String key = hashingAlgorithm.toString()+ maxDigestBytes != null ? maxDigestBytes.toString() : "-" + maxHashLength != null ? maxHashLength.toString() : "-" + encoder != null ? encoder : "-";
+        final String key = getHashCacheKey(hashingAlgorithm, maxDigestBytes, maxHashLength, encoder);
         String val = hashedKeysByAlgorithm.get(key);
         if(val == null) {
             val = KeyHasher.getHashedKeyEncoded(getCanonicalKey(false), hashingAlgorithm, maxDigestBytes, maxHashLength, encoder);
@@ -138,7 +136,7 @@ private String getHashKeyForDuet(HashingAlgorithm hashingAlgorithm, Boolean shou
             return null;
         }
 
-        final String key = hashingAlgorithm.toString()+ maxDigestBytes != null ? maxDigestBytes.toString() : "-" + maxHashLength != null ? maxHashLength.toString() : "-" + encoder != null ? encoder : "-";
+        final String key = getHashCacheKey(hashingAlgorithm, maxDigestBytes, maxHashLength, encoder);
         String val = hashedKeysByAlgorithmForDuet.get(key);
         if(val == null) {
             val = KeyHasher.getHashedKeyEncoded(getCanonicalKeyForDuet(), hashingAlgorithm, maxDigestBytes, maxHashLength, encoder);
@@ -149,6 +147,10 @@ private String getHashKeyForDuet(HashingAlgorithm hashingAlgorithm, Boolean shou
         return val;
     }
 
+    private static String getHashCacheKey(HashingAlgorithm hashingAlgorithm, Integer maxDigestBytes, Integer maxHashLength, String encoder) {
+        return hashingAlgorithm + ":" + maxDigestBytes + ":" + maxHashLength + ":" + encoder;
+    }
+
     @Override
     public int hashCode() {
         final int prime = 31;

evcache-core/src/test/java/com/netflix/evcache/EVCacheKeyTest.java

@@ -0,0 +1,66 @@
+package com.netflix.evcache;
+
+import com.netflix.archaius.api.Property;
+import com.netflix.evcache.util.KeyHasher;
+import com.netflix.evcache.util.KeyHasher.HashingAlgorithm;
+import org.testng.annotations.Test;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertNotEquals;
+
+public class EVCacheKeyTest {
+
+    private static final String APP = "EVCACHE_TEST";
+    private static final String KEY = "key";
+
+    @SuppressWarnings("unchecked")
+    private static <T> Property<T> property(T value) {
+        Property<T> property = mock(Property.class);
+        when(property.get()).thenReturn(value);
+        return property;
+    }
+
+    private static EVCacheKey newKey() {
+        return new EVCacheKey(APP, KEY, KEY, HashingAlgorithm.siphash24, property(Boolean.TRUE),
+                property(-1), property(-1), "base64");
+    }
+
+    @Test
+    public void hashKeyIsRecomputedForEachHashingAlgorithm() {
+        EVCacheKey evcKey = newKey();
+
+        String siphash = evcKey.getHashKey(false, HashingAlgorithm.siphash24, null, -1, -1, "base64");
+        String md5 = evcKey.getHashKey(false, HashingAlgorithm.md5, null, -1, -1, "base64");
+
+        assertEquals(siphash, KeyHasher.getHashedKeyEncoded(KEY, HashingAlgorithm.siphash24, -1, -1, "base64"));
+        assertEquals(md5, KeyHasher.getHashedKeyEncoded(KEY, HashingAlgorithm.md5, -1, -1, "base64"));
+        assertNotEquals(md5, siphash);
+    }
+
+    @Test
+    public void hashKeyIsRecomputedForEachMaxHashLengthAndEncoder() {
+        EVCacheKey evcKey = newKey();
+
+        String full = evcKey.getHashKey(false, HashingAlgorithm.md5, null, -1, -1, "base64");
+        String truncated = evcKey.getHashKey(false, HashingAlgorithm.md5, null, -1, 8, "base64");
+        String ascii85 = evcKey.getHashKey(false, HashingAlgorithm.md5, null, -1, -1, "ascii85");
+
+        assertEquals(full, KeyHasher.getHashedKeyEncoded(KEY, HashingAlgorithm.md5, -1, -1, "base64"));
+        assertEquals(truncated, KeyHasher.getHashedKeyEncoded(KEY, HashingAlgorithm.md5, -1, 8, "base64"));
+        assertEquals(ascii85, KeyHasher.getHashedKeyEncoded(KEY, HashingAlgorithm.md5, -1, -1, "ascii85"));
+    }
+
+    @Test
+    public void duetHashKeyIsRecomputedForEachHashingAlgorithm() {
+        EVCacheKey evcKey = newKey();
+        String duetKey = APP + ":" + KEY;
+
+        String siphash = evcKey.getHashKey(true, HashingAlgorithm.siphash24, null, -1, -1, "base64");
+        String md5 = evcKey.getHashKey(true, HashingAlgorithm.md5, null, -1, -1, "base64");
+
+        assertEquals(siphash, KeyHasher.getHashedKeyEncoded(duetKey, HashingAlgorithm.siphash24, -1, -1, "base64"));
+        assertEquals(md5, KeyHasher.getHashedKeyEncoded(duetKey, HashingAlgorithm.md5, -1, -1, "base64"));
+    }
+}

evcache-core/src/test/java/test-suite.xml

@@ -4,6 +4,7 @@
     <classes>
       <class name="com.netflix.evcache.pool.NodeLocatorLookupTest" />
       <class name="com.netflix.evcache.EVCacheInternalImplTest" />
+      <class name="com.netflix.evcache.EVCacheKeyTest" />
       <class name="com.netflix.evcache.pool.EVCacheValueSerdeTest" />
       <class name="com.netflix.evcache.config.EVCacheTranscoderPropertiesTest" />
     </classes>