Fix #880 by sorting the custom hash map before creating the storage

JSON so that the CAS calls can complete successfully. Thanks @dpellegrino! Signed-off-by: Chris Larsen <[email protected]>
OpenTSDB · Dec 13, 2016 · cc861b0 · cc861b0
1 parent 7f4f98d
commit cc861b0
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 6 deletions.
diff --git a/src/meta/Annotation.java b/src/meta/Annotation.java
@@ -19,6 +19,7 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.TreeMap;
 
 import org.hbase.async.Bytes;
 import org.hbase.async.DeleteRequest;
@@ -44,6 +45,7 @@
 import com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility;
 import com.fasterxml.jackson.annotation.JsonInclude.Include;
 import com.fasterxml.jackson.core.JsonGenerator;
+import com.google.common.annotations.VisibleForTesting;
 import com.stumbleupon.async.Callback;
 import com.stumbleupon.async.Deferred;
 
@@ -512,7 +514,8 @@ public static byte PREFIX() {
    * successful CAS calls
    * @return The serialized object as a byte array
    */
-  private byte[] getStorageJSON() {
+  @VisibleForTesting
+  byte[] getStorageJSON() {
     // TODO - precalculate size
     final ByteArrayOutputStream output = new ByteArrayOutputStream();
     try {
@@ -528,11 +531,9 @@ private byte[] getStorageJSON() {
       if (custom == null) {
         json.writeNullField("custom");
       } else {
-        json.writeObjectFieldStart("custom");
-        for (Map.Entry<String, String> entry : custom.entrySet()) {
-          json.writeStringField(entry.getKey(), entry.getValue());
-        }
-        json.writeEndObject();
+        final TreeMap<String, String> sorted_custom = 
+            new TreeMap<String, String>(custom);
+        json.writeObjectField("custom", sorted_custom);
       }
 
       json.writeEndObject(); 

diff --git a/test/meta/TestAnnotation.java b/test/meta/TestAnnotation.java
@@ -15,8 +15,10 @@
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
 
 import java.util.List;
+import java.util.Map;
 
 import net.opentsdb.core.BaseTsdbTest;
 import net.opentsdb.core.Const;
@@ -34,6 +36,8 @@
 import org.powermock.core.classloader.annotations.PrepareForTest;
 import org.powermock.modules.junit4.PowerMockRunner;
 
+import com.google.common.collect.Maps;
+
 @PowerMockIgnore({"javax.management.*", "javax.xml.*",
   "ch.qos.*", "org.slf4j.*",
   "com.sum.*", "org.xml.*"})
@@ -334,6 +338,21 @@ public void syncToStorageNoChanges() throws Exception {
     note.syncToStorage(tsdb, false).joinUninterruptibly();
   }
 
+  @Test
+  public void getStorageJSONTags() throws Exception {
+    Map<String, String> custom = Maps.newHashMap();
+    custom.put("C", "C");
+    custom.put("P", "P");
+    custom.put("E", "E");
+    System.out.println(custom);
+
+    note.setTSUID(TSUID);
+    note.setStartTime(1388450562L);
+    note.setCustom(custom);
+    final String json = new String(note.getStorageJSON());
+    assertTrue(json.contains("{\"C\":\"C\",\"E\":\"E\",\"P\":\"P\"}"));
+  }
+
   @Test
   public void delete() throws Exception {
     setupStorage(false);