Brand new KeyStrategy work

Md5KeyStrategy and Sha1KeyStrategy now both work independently of the HashCodeKeyStrategy. StringKeyStrategy and HashCodeKeyStrategy will now both throw exceptions if the key is too long, no more auto truncated for people doing bad things.
raykrueger · Apr 6, 2011 · 70881ae · 70881ae
1 parent 67dcd2a
commit 70881ae
Show file tree

Hide file tree

Showing 9 changed files with 54 additions and 62 deletions.
diff --git a/src/main/java/com/googlecode/hibernate/memcached/AbstractKeyStrategy.java b/src/main/java/com/googlecode/hibernate/memcached/AbstractKeyStrategy.java
@@ -10,31 +10,27 @@
  * KeyStrategy base class that handles concatenation, cleaning, and truncating the final cache key.
  * <p/>
  * Concatenates the three key components; regionName, clearIndex and key.<br/>
- * Subclasses are responsible for transforming the Key object into something identifyable.<br/>
- * If the key total length, including region and clearIndex, are greater than the maxKeyLength, the key's hashCode
- * will be used as the key. Subclasses can override this behavior.
+ * Subclasses are responsible for transforming the Key object into something identifyable.
  *
  * @author Ray Krueger
  */
 public abstract class AbstractKeyStrategy implements KeyStrategy {
 
-    public static final int DEFAULT_MAX_KEY_LENGTH = 250;
+    public static final int MAX_KEY_LENGTH = 250;
 
     protected final Logger log = LoggerFactory.getLogger(getClass());
 
     private static final Pattern CLEAN_PATTERN = Pattern.compile("\\s");
 
-    private int maxKeyLength = DEFAULT_MAX_KEY_LENGTH;
-
     public String toKey(String regionName, long clearIndex, Object key) {
         if (key == null) {
             throw new IllegalArgumentException("key must not be null");
         }
 
         String keyString = concatenateKey(regionName, clearIndex, transformKeyObject(key));
 
-        if (keyString.length() > maxKeyLength) {
-            return truncateKey(keyString);
+        if (keyString.length() > MAX_KEY_LENGTH) {
+            throw new IllegalArgumentException("Key is longer than " + MAX_KEY_LENGTH + " characters, try using the Sha1KeyStrategy: " + keyString);
         }
 
         String finalKey = CLEAN_PATTERN.matcher(keyString).replaceAll("");
@@ -44,26 +40,6 @@ public String toKey(String regionName, long clearIndex, Object key) {
 
     protected abstract String transformKeyObject(Object key);
 
-    protected String truncateKey(String key) {
-
-        String keyHashCode = StringUtils.md5Hex(key);
-
-        log.warn("Encoded key [{}] to md5 hash [{}]. " +
-                "Be sure to set cache region names whenever possible as the names Hibernate generates are really long.",
-                key, keyHashCode
-        );
-
-        return keyHashCode;
-    }
-
-    public int getMaxKeyLength() {
-        return maxKeyLength;
-    }
-
-    public void setMaxKeyLength(int maxKeyLength) {
-        this.maxKeyLength = maxKeyLength;
-    }
-
     protected String concatenateKey(String regionName, long clearIndex, Object key) {
         return new StringBuilder()
                 .append(regionName)

diff --git a/src/main/java/com/googlecode/hibernate/memcached/DigestKeyStrategy.java b/src/main/java/com/googlecode/hibernate/memcached/DigestKeyStrategy.java
@@ -0,0 +1,20 @@
+package com.googlecode.hibernate.memcached;
+
+import com.googlecode.hibernate.memcached.utils.StringUtils;
+
+/**
+ * @author Ray Krueger
+ */
+public abstract class DigestKeyStrategy extends AbstractKeyStrategy {
+
+  protected String transformKeyObject(Object key) {
+    return key.toString() + ":" + key.hashCode();
+  }
+
+  protected String concatenateKey(String regionName, long clearIndex, Object key) {
+    String longKey = super.concatenateKey(regionName, clearIndex, key);
+    return digest(longKey);
+  }
+
+  protected abstract String digest(String string);
+}
diff --git a/src/main/java/com/googlecode/hibernate/memcached/Md5KeyStrategy.java b/src/main/java/com/googlecode/hibernate/memcached/Md5KeyStrategy.java
@@ -5,9 +5,8 @@
 /**
  * @author Ray Krueger
  */
-public class Md5KeyStrategy extends HashCodeKeyStrategy {
-    protected String concatenateKey(String regionName, long clearIndex, Object key) {
-        String longKey = super.concatenateKey(regionName, clearIndex, key);
-        return StringUtils.md5Hex(longKey);
-    }
+public class Md5KeyStrategy extends DigestKeyStrategy {
+  protected String digest(String key) {
+    return StringUtils.md5Hex(key);
+  }
 }
diff --git a/src/main/java/com/googlecode/hibernate/memcached/Sha1KeyStrategy.java b/src/main/java/com/googlecode/hibernate/memcached/Sha1KeyStrategy.java
@@ -5,9 +5,8 @@
 /**
  * @author Ray Krueger
  */
-public class Sha1KeyStrategy extends HashCodeKeyStrategy {
-    protected String concatenateKey(String regionName, long clearIndex, Object key) {
-        String longKey = super.concatenateKey(regionName, clearIndex, key);
-        return StringUtils.sha1Hex(longKey);
-    }
-}
+public class Sha1KeyStrategy extends DigestKeyStrategy {
+  protected String digest(String key) {
+    return StringUtils.sha1Hex(key);
+  }
+}
diff --git a/src/test/groovy/com/googlecode/hibernate/memcached/AbstractKeyStrategyTestCase.groovy b/src/test/groovy/com/googlecode/hibernate/memcached/AbstractKeyStrategyTestCase.groovy
@@ -26,4 +26,4 @@ abstract class AbstractKeyStrategyTestCase extends BaseTestCase {
 
     abstract KeyStrategy getKeyStrategy();
 
-}
+}
diff --git a/src/test/groovy/com/googlecode/hibernate/memcached/HashCodeKeyStrategyTest.groovy b/src/test/groovy/com/googlecode/hibernate/memcached/HashCodeKeyStrategyTest.groovy
@@ -27,10 +27,12 @@ class HashCodeKeyStrategyTest extends AbstractKeyStrategyTestCase {
         assert_cache_key_equals "Ihavespaces:0:-2100783816", "I have spaces", 0, "so do I"
     }
 
-    void test_really_long_keys_get_truncated() {
+    void test_really_long_key_throws_exception() {
         String regionName = ""
         250.times {regionName += "x"}
-        assert_cache_key_equals "e2e82011e3d56dd6be564fdcb72a8d64", regionName, 0, "blah blah blah"
+        shouldFail(IllegalArgumentException) {
+          getKeyStrategy().toKey(regionName, 0, "blah blah blah")
+        }
     }
 
-}
+}
diff --git a/src/test/groovy/com/googlecode/hibernate/memcached/Md5KeyStrategyTest.groovy b/src/test/groovy/com/googlecode/hibernate/memcached/Md5KeyStrategyTest.groovy
@@ -12,25 +12,25 @@ class Md5KeyStrategyTest extends AbstractKeyStrategyTestCase {
     }
 
     void test() {
-        assert_cache_key_equals "dfbb1717f813ecccac747d5076e2a6d5", "test", 0, "boing"
+        assert_cache_key_equals "a088ce3b48a12c8a8f26058240f4518d", "test", 0, "boing"
     }
 
     void test_null_region() {
-        assert_cache_key_equals "71b3dae5a0a8d765658a6c27bed071fd", null, 0, "boing"
+        assert_cache_key_equals "cf23c7bb0c99979d4be1129adc959e6f", null, 0, "boing"
     }
 
     void test_null_key_does_not_validate() {
         assert_null_key_does_not_validate()
     }
 
     void test_spaces() {
-        assert_cache_key_equals "23c5e5682b9a9fad5b30a95fae4ff299", "I have spaces", 0, "so do I"
+        assert_cache_key_equals "0564810c2fd4e86dc6f355ad99e7d01b", "I have spaces", 0, "so do I"
     }
 
     void test_really_long_keys_get_truncated() {
         String regionName = ""
         250.times {regionName += "x"}
-        assert_cache_key_equals "e2e82011e3d56dd6be564fdcb72a8d64", regionName, 0, "blah blah blah"
+        assert_cache_key_equals "16df3d87c2f8bde43fcdbb545be10626", regionName, 0, "blah blah blah"
     }
 
-}
+}
diff --git a/src/test/groovy/com/googlecode/hibernate/memcached/Sha1KeyStrategyTest.groovy b/src/test/groovy/com/googlecode/hibernate/memcached/Sha1KeyStrategyTest.groovy
@@ -12,25 +12,25 @@ class Sha1KeyStrategyTest extends AbstractKeyStrategyTestCase {
     }
 
     void test() {
-        assert_cache_key_equals "5c2adf57badcd5d923228b96dd1aee3bf0d5bf2c", "test", 0, "boing"
+        assert_cache_key_equals "cd23e26dd7ab1d052e1c0a04daa27a03f6cd5d1c", "test", 0, "boing"
     }
 
     void test_null_region() {
-        assert_cache_key_equals "a0e96499b9522edc2807f4189e1cfdd65a4dad0d", null, 0, "boing"
+        assert_cache_key_equals "6afcec5614479d46a1ec6d73dabbc2cea154da3c", null, 0, "boing"
     }
 
     void test_null_key_does_not_validate() {
         assert_null_key_does_not_validate()
     }
 
     void test_spaces() {
-        assert_cache_key_equals "3344a9dadb9f405a39924d593592be1bf400e978", "I have spaces", 0, "so do I"
+        assert_cache_key_equals "949b2a6fce917d85bd56e6197c93b3affa694e50", "I have spaces", 0, "so do I"
     }
 
     void test_really_long_keys_get_truncated() {
         String regionName = ""
         250.times {regionName += "x"}
-        assert_cache_key_equals "3c64cd962343bc26ea73c78ba59eeed88491f439", regionName, 0, "blah blah blah"
+        assert_cache_key_equals "7f00c6faf1fefaf62cabb512285cc60ce641d5c8", regionName, 0, "blah blah blah"
     }
 
-}
+}
diff --git a/src/test/groovy/com/googlecode/hibernate/memcached/StringKeyStrategyTest.groovy b/src/test/groovy/com/googlecode/hibernate/memcached/StringKeyStrategyTest.groovy
@@ -15,12 +15,6 @@ class StringKeyStrategyTest extends AbstractKeyStrategyTestCase {
         assert_cache_key_equals "test:0:boing", "test", 0, "boing"
     }
 
-    void test_config() {
-        assertEquals(StringKeyStrategy.DEFAULT_MAX_KEY_LENGTH, strategy.maxKeyLength)
-        strategy.maxKeyLength = 1
-        assertEquals(1, strategy.maxKeyLength)
-    }
-
     void test_null_region() {
         assert_cache_key_equals "null:0:boing", null, 0, "boing"
     }
@@ -33,10 +27,12 @@ class StringKeyStrategyTest extends AbstractKeyStrategyTestCase {
         assert_cache_key_equals "Ihavespaces:0:sodoI", "I have spaces", 0, "so do I"
     }
 
-    void test_really_long_keys_get_truncated() {
+    void test_really_long_key_throws_exception() {
         String regionName = ""
         250.times {regionName += "x"}
-        assert_cache_key_equals "fe009b44a903277f4b8e07f2cb03e96f", regionName, 0, "blah blah blah"
+        shouldFail(IllegalArgumentException) {
+          getKeyStrategy().toKey(regionName, 0, "blah blah blah")
+        }
     }
 
-}
+}