Merge pull request #68 from urbanairship/fixmodbug

Fix mod bug since 1.4.0
urbanairship · Jun 23, 2017 · 8f6916a · 8f6916a
2 parents bb48abc + 25f5930
commit 8f6916a
Show file tree

Hide file tree

Showing 5 changed files with 37 additions and 11 deletions.
diff --git a/RELEASE_NOTES.txt b/RELEASE_NOTES.txt
@@ -1,3 +1,11 @@
+2.0.0
+=====
+Non compatible update with existing data cubes.  The DataCube constructor in
+previous versions with the useAddressPrefixByteHash parameter set to true is
+compatible with PREFIX_MODE.NO_ADDRESS_PREFIX, but is not compatible with the
+new mode PREFIX_MODE.MOD_ADDRESS_PREFIX because there was a bug since 1.4.0
+that didn't implement this feature correctly.
+
 1.5.0
 =====
 Prevent getId calls from creating a new identifier
@@ -6,7 +14,6 @@ Optimize id creation.
 
 1.4.0
 =====
-
 Add functionality to optionally add a hash in front of row keys, which permits
 users to ignore dimension order when considering performance.
 

diff --git a/pom.xml b/pom.xml
@@ -3,7 +3,7 @@
 
   <groupId>com.urbanairship</groupId>
   <artifactId>datacube</artifactId>
-  <version>1.5.1-SNAPSHOT</version>
+  <version>2.0.0-SNAPSHOT</version>
   <packaging>jar</packaging>
 
   <name>datacube</name>

diff --git a/src/main/java/com/urbanairship/datacube/Address.java b/src/main/java/com/urbanairship/datacube/Address.java
@@ -146,13 +146,25 @@ private Optional<byte[]> toKey(IdService idService, boolean readOnly) throws IOE
         for (byte[] keyElement : keyElemsInOrder) {
             totalKeySize += keyElement.length;
         }
-        ByteBuffer bb = ByteBuffer.allocate(totalKeySize);
-
+        ByteBuffer bb;
+        // Add a place holder for the hash byte if it's required
+        if (this.cube.useAddressPrefixByteHash()) {
+            bb = ByteBuffer.allocate(totalKeySize + 1);
+            bb.put((byte) 0x01);
+        } else {
+            bb = ByteBuffer.allocate(totalKeySize);
+        }
 
         for (byte[] keyElement : keyElemsInOrder) {
             bb.put(keyElement);
         }
 
+        // Update the byte prefix placeholder of the hash of the key contents if required.
+        if (this.cube.useAddressPrefixByteHash()) {
+            byte hashByte = Util.hashByteArray(bb.array(), 1, totalKeySize + 1);
+            bb.put(0, hashByte);
+        }
+
         if (bb.remaining() != 0) {
             throw new AssertionError("Key length calculation was somehow wrong, " +
                     bb.remaining() + " bytes remaining");

diff --git a/src/main/java/com/urbanairship/datacube/DataCube.java b/src/main/java/com/urbanairship/datacube/DataCube.java
@@ -29,6 +29,8 @@
 public class DataCube<T extends Op> {
     private static final Logger log = LoggerFactory.getLogger(DataCube.class);
 
+    public enum PREFIX_MODE { NO_ADDRESS_PREFIX, MOD_ADDRESS_PREFIX}
+
     private final List<Dimension<?>> dims;
     private final List<Rollup> rollups;
     private final Multimap<Dimension<?>,BucketType> bucketsOfInterest;
@@ -42,25 +44,30 @@ public class DataCube<T extends Op> {
      * @param rollups see {@link Rollup}
      */
     public DataCube(List<Dimension<?>> dims, List<Rollup> rollups) {
-        this(dims, rollups, false);
+        this(dims, rollups, PREFIX_MODE.NO_ADDRESS_PREFIX);
     }
 
     /**
      *
      * @param dims see {@link Dimension}
      * @param rollups see {@link Rollup}
-     * @param useAddressPrefixByteHash Prefix the keys by a hash byte (calculated by hashing each element
+     * @param prefixMode use MOD_ADDRESS_PREFIX to prefix the keys by a hash byte (calculated by hashing each element
      *                                 in the key).  This is only a storage artifact to benefit systems
      *                                 like HBase, where monotonically increasing row keys can result in
      *                                 hot spots.
-     *                                 Warning: Do NOT enable or disable this feature for an existing cube or
-     *                                 the keys will not map properly.
+     *                                 Warning: Do NOT switch modes for an existing cube or the keys will
+     *                                 not map properly.  Also, data from versions of datacube before 2.0.0,
+     *                                 with this feature enabled, is not compatible with 2.0.0+.
      */
-    public DataCube(List<Dimension<?>> dims, List<Rollup> rollups, boolean useAddressPrefixByteHash) {
+    public DataCube(List<Dimension<?>> dims, List<Rollup> rollups, PREFIX_MODE prefixMode) {
         this.dims = dims;
         this.rollups = rollups;
         this.validAddressSet = Sets.newHashSet();
-        this.useAddressPrefixByteHash = useAddressPrefixByteHash;
+        if (PREFIX_MODE.MOD_ADDRESS_PREFIX == prefixMode) {
+            this.useAddressPrefixByteHash = true;
+        } else {
+            this.useAddressPrefixByteHash = false;
+        }
 
         bucketsOfInterest = HashMultimap.create();
 

diff --git a/src/test/java/com/urbanairship/datacube/tweetcountexample/TweetCube.java b/src/test/java/com/urbanairship/datacube/tweetcountexample/TweetCube.java
@@ -84,7 +84,7 @@ public TweetCube(DbHarness<LongOp> dbHarness, SyncLevel syncLevel) {
 		 * The DataCube defines the core logic that maps input points to database
 		 * increments.
 		 */
-		dataCube = new DataCube<LongOp>(dimensions, rollups, true);
+		dataCube = new DataCube<LongOp>(dimensions, rollups, DataCube.PREFIX_MODE.MOD_ADDRESS_PREFIX);
 
 		/*
 		 * The DataCubeIo object connects the DataCube logic layer and the