Review round 1 fixes

11 files changed, 179 insertions, 138 deletions

diff --git a/vespajlib/src/main/java/com/yahoo/slime/ArrayInserter.java b/vespajlib/src/main/java/com/yahoo/slime/ArrayInserter.java
index d1711bb8313..4ea9239798a 100644
--- a/vespajlib/src/main/java/com/yahoo/slime/ArrayInserter.java
+++ b/vespajlib/src/main/java/com/yahoo/slime/ArrayInserter.java
@@ -7,10 +7,15 @@ package com.yahoo.slime;
  **/
 public final class ArrayInserter implements Inserter {
     private Cursor target;
+
+    public ArrayInserter() {}
+    public ArrayInserter(Cursor c) { adjust(c); }
+
     public final ArrayInserter adjust(Cursor c) {
         target = c;
         return this;
     }
+
     public final Cursor insertNIX()                { return target.addNix(); }
     public final Cursor insertBOOL(boolean value)  { return target.addBool(value); }
     public final Cursor insertLONG(long value)     { return target.addLong(value); }
diff --git a/vespajlib/src/main/java/com/yahoo/slime/BinaryDecoder.java b/vespajlib/src/main/java/com/yahoo/slime/BinaryDecoder.java
index 3fe35ef2098..2281890bbeb 100644
--- a/vespajlib/src/main/java/com/yahoo/slime/BinaryDecoder.java
+++ b/vespajlib/src/main/java/com/yahoo/slime/BinaryDecoder.java
@@ -8,7 +8,7 @@ final class BinaryDecoder {
 
     private final SlimeInserter slimeInserter = new SlimeInserter();
     private final ArrayInserter arrayInserter = new ArrayInserter();
-    private final ObjectInserter objectInserter = new ObjectInserter();
+    private final ObjectSymbolInserter objectInserter = new ObjectSymbolInserter();
 
     public BinaryDecoder() {}
 
diff --git a/vespajlib/src/main/java/com/yahoo/slime/Injector.java b/vespajlib/src/main/java/com/yahoo/slime/Injector.java
index 3e507cf29c2..95198c16031 100644
--- a/vespajlib/src/main/java/com/yahoo/slime/Injector.java
+++ b/vespajlib/src/main/java/com/yahoo/slime/Injector.java
@@ -25,17 +25,29 @@ public class Injector {
     }
 
     private void injectValue(Inserter inserter, Inspector inspector, Inspector guard) {
-        switch (inspector.type()) {
-            case NIX:    inserter.insertNIX(); break;
-            case BOOL:   inserter.insertBOOL(inspector.asBool()); break;
-            case LONG:   inserter.insertLONG(inspector.asLong()); break;
-            case DOUBLE: inserter.insertDOUBLE(inspector.asDouble()); break;
-            case STRING: inserter.insertSTRING(inspector.asString()); break;
-            case DATA:   inserter.insertDATA(inspector.asData()); break;
-            case ARRAY:  injectArray(inserter, inspector, guard); break;
-            case OBJECT: injectObject(inserter, inspector, guard); break;
-            default:     throw new IllegalArgumentException("Unknown type " + inspector.type());
-        }
+        inspector.accept(new Visitor() {
+            @Override public void visitInvalid()           { }
+            @Override public void visitNix()               { inserter.insertNIX(); }
+            @Override public void visitBool(boolean bit)   { inserter.insertBOOL(bit); }
+            @Override public void visitLong(long l)        { inserter.insertLONG(l); }
+            @Override public void visitDouble(double d)    { inserter.insertDOUBLE(d); }
+            @Override public void visitString(String str)  { inserter.insertSTRING(str); }
+            @Override public void visitString(byte[] utf8) { inserter.insertSTRING(utf8); }
+            @Override public void visitData(byte[] data)   { inserter.insertDATA(data); }
+
+            @Override
+            public void visitArray(Inspector arr) {
+                Cursor cursor = inserter.insertARRAY();
+                ArrayTraverser arrayTraverser = new NestedInjector(cursor, guard != null ? guard : cursor);
+                arr.traverse(arrayTraverser);
+            }
+            @Override
+            public void visitObject(Inspector obj) {
+                Cursor cursor = inserter.insertOBJECT();
+                ObjectTraverser objectTraverser = new NestedInjector(cursor, guard != null ? guard : cursor);
+                obj.traverse(objectTraverser);
+            }
+        });
     }
 
     private void injectArray(Inserter inserter, Inspector inspector, Inspector guard) {
@@ -65,9 +77,7 @@ public class Injector {
                 return;
             }
 
-            var inserter = new ArrayInserter();
-            inserter.adjust(cursor);
-            injectValue(inserter, inspector, guard);
+            injectValue(new ArrayInserter(cursor), inspector, guard);
         }
 
         @Override
@@ -76,9 +86,7 @@ public class Injector {
                 return;
             }
 
-            var inserter = new ObjectInserter();
-            inserter.adjust(cursor, name);
-            injectValue(inserter, inspector, guard);
+            injectValue(new ObjectInserter(cursor, name), inspector, guard);
         }
     }
 }
diff --git a/vespajlib/src/main/java/com/yahoo/slime/Inspector.java b/vespajlib/src/main/java/com/yahoo/slime/Inspector.java
index ca2e678c152..7af7accaf49 100644
--- a/vespajlib/src/main/java/com/yahoo/slime/Inspector.java
+++ b/vespajlib/src/main/java/com/yahoo/slime/Inspector.java
@@ -134,6 +134,8 @@ public interface Inspector {
     /**
      * Tests whether this is equal to Inspector.
      *
+     * Since equality of two Inspectors is subtle, {@link Object#equals(Object)} is not used.
+     *
      * @param that inspector.
      * @return true if they are equal.
      */
diff --git a/vespajlib/src/main/java/com/yahoo/slime/JsonDecoder.java b/vespajlib/src/main/java/com/yahoo/slime/JsonDecoder.java
index 71eaa7f8b25..15bd8948861 100644
--- a/vespajlib/src/main/java/com/yahoo/slime/JsonDecoder.java
+++ b/vespajlib/src/main/java/com/yahoo/slime/JsonDecoder.java
@@ -4,9 +4,6 @@ package com.yahoo.slime;
 import com.yahoo.text.Utf8;
 
 import java.io.ByteArrayOutputStream;
-import java.nio.ByteBuffer;
-import java.util.ArrayList;
-import java.util.List;
 
 /**
  * A port of the C++ json decoder intended to be fast.
@@ -20,7 +17,7 @@ public class JsonDecoder {
 
     private final SlimeInserter slimeInserter = new SlimeInserter();
     private final ArrayInserter arrayInserter = new ArrayInserter();
-    private final JsonObjectInserter objectInserter = new JsonObjectInserter();
+    private final ObjectInserter objectInserter = new ObjectInserter();
     private final ByteArrayOutputStream buf = new ByteArrayOutputStream();
 
     private static final byte[] TRUE = {'t', 'r', 'u', 'e'};
@@ -284,23 +281,4 @@ public class JsonDecoder {
         }
     }
 
-    private static final class JsonObjectInserter implements Inserter {
-        private Cursor target;
-        private String key;
-        public final JsonObjectInserter adjust(Cursor c, String key) {
-            target = c;
-            this.key = key;
-            return this;
-        }
-        public final Cursor insertNIX()                { return target.setNix(key); }
-        public final Cursor insertBOOL(boolean value)  { return target.setBool(key, value); }
-        public final Cursor insertLONG(long value)     { return target.setLong(key, value); }
-        public final Cursor insertDOUBLE(double value) { return target.setDouble(key, value); }
-        public final Cursor insertSTRING(String value) { return target.setString(key, value); }
-        public final Cursor insertSTRING(byte[] utf8)  { return target.setString(key, utf8); }
-        public final Cursor insertDATA(byte[] value)   { return target.setData(key, value); }
-        public final Cursor insertARRAY()              { return target.setArray(key); }
-        public final Cursor insertOBJECT()             { return target.setObject(key); }
-    }
-
 }
diff --git a/vespajlib/src/main/java/com/yahoo/slime/ObjectInserter.java b/vespajlib/src/main/java/com/yahoo/slime/ObjectInserter.java
index 203bdb3f561..96b7a4e4a15 100644
--- a/vespajlib/src/main/java/com/yahoo/slime/ObjectInserter.java
+++ b/vespajlib/src/main/java/com/yahoo/slime/ObjectInserter.java
@@ -1,73 +1,32 @@
-// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.slime;
 
-import java.util.Optional;
-
 /**
- * Helper class for inserting values into an ObjectValue.
- * For justification read Inserter documentation.
- **/
+ * @author hakonhall
+ */
 public final class ObjectInserter implements Inserter {
     private Cursor target;
-    private int symbol;
-    private Optional<String> symbolName = Optional.empty();
+    private String key;
 
-    public final ObjectInserter adjust(Cursor c, int sym) {
-        target = c;
-        symbol = sym;
-        symbolName = Optional.empty();
-        return this;
+    public ObjectInserter() {}
+
+    public ObjectInserter(Cursor c, String key) {
+        adjust(c, key);
     }
 
-    public final ObjectInserter adjust(Cursor c, String name) {
+    public final ObjectInserter adjust(Cursor c, String key) {
         target = c;
-        symbol = -1;
-        symbolName = Optional.of(name);
+        this.key = key;
         return this;
     }
 
-    public final Cursor insertNIX()                {
-        return symbolName.map(name -> target.setNix(name))
-                     .orElseGet(() -> target.setNix(symbol));
-    }
-
-    public final Cursor insertBOOL(boolean value)  {
-        return symbolName.map(name -> target.setBool(name, value))
-                     .orElseGet(() -> target.setBool(symbol, value));
-    }
-
-    public final Cursor insertLONG(long value)  {
-        return symbolName.map(name -> target.setLong(name, value))
-                     .orElseGet(() -> target.setLong(symbol, value));
-    }
-
-    public final Cursor insertDOUBLE(double value)  {
-        return symbolName.map(name -> target.setDouble(name, value))
-                     .orElseGet(() -> target.setDouble(symbol, value));
-    }
-
-    public final Cursor insertSTRING(String value)  {
-        return symbolName.map(name -> target.setString(name, value))
-                     .orElseGet(() -> target.setString(symbol, value));
-    }
-
-    public final Cursor insertSTRING(byte[] utf8)  {
-        return symbolName.map(name -> target.setString(name, utf8))
-                     .orElseGet(() -> target.setString(symbol, utf8));
-    }
-
-    public final Cursor insertDATA(byte[] value)  {
-        return symbolName.map(name -> target.setData(name, value))
-                     .orElseGet(() -> target.setData(symbol, value));
-    }
-
-    public final Cursor insertARRAY()  {
-        return symbolName.map(name -> target.setArray(name))
-                     .orElseGet(() -> target.setArray(symbol));
-    }
-
-    public final Cursor insertOBJECT()  {
-        return symbolName.map(name -> target.setObject(name))
-                     .orElseGet(() -> target.setObject(symbol));
-    }
+    public final Cursor insertNIX()                { return target.setNix(key); }
+    public final Cursor insertBOOL(boolean value)  { return target.setBool(key, value); }
+    public final Cursor insertLONG(long value)     { return target.setLong(key, value); }
+    public final Cursor insertDOUBLE(double value) { return target.setDouble(key, value); }
+    public final Cursor insertSTRING(String value) { return target.setString(key, value); }
+    public final Cursor insertSTRING(byte[] utf8)  { return target.setString(key, utf8); }
+    public final Cursor insertDATA(byte[] value)   { return target.setData(key, value); }
+    public final Cursor insertARRAY()              { return target.setArray(key); }
+    public final Cursor insertOBJECT()             { return target.setObject(key); }
 }
diff --git a/vespajlib/src/main/java/com/yahoo/slime/ObjectSymbolInserter.java b/vespajlib/src/main/java/com/yahoo/slime/ObjectSymbolInserter.java
new file mode 100644
index 00000000000..9f50e1ae7df
--- /dev/null
+++ b/vespajlib/src/main/java/com/yahoo/slime/ObjectSymbolInserter.java
@@ -0,0 +1,33 @@
+// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.slime;
+
+/**
+ * Helper class for inserting values into an ObjectValue.
+ * For justification read Inserter documentation.
+ **/
+public final class ObjectSymbolInserter implements Inserter {
+    private Cursor target;
+    private int symbol;
+
+    public ObjectSymbolInserter() {}
+
+    public ObjectSymbolInserter(Cursor cursor, int sym) {
+        adjust(cursor, sym);
+    }
+
+    public final ObjectSymbolInserter adjust(Cursor c, int sym) {
+        target = c;
+        symbol = sym;
+        return this;
+    }
+
+    public final Cursor insertNIX()                { return target.setNix(symbol); }
+    public final Cursor insertBOOL(boolean value)  { return target.setBool(symbol, value); }
+    public final Cursor insertLONG(long value)     { return target.setLong(symbol, value); }
+    public final Cursor insertDOUBLE(double value) { return target.setDouble(symbol, value); }
+    public final Cursor insertSTRING(String value) { return target.setString(symbol, value); }
+    public final Cursor insertSTRING(byte[] utf8)  { return target.setString(symbol, utf8); }
+    public final Cursor insertDATA(byte[] value)   { return target.setData(symbol, value); }
+    public final Cursor insertARRAY()              { return target.setArray(symbol); }
+    public final Cursor insertOBJECT()             { return target.setObject(symbol); }
+}
diff --git a/vespajlib/src/main/java/com/yahoo/slime/Slime.java b/vespajlib/src/main/java/com/yahoo/slime/Slime.java
index 84f193caa4d..8357e3035c0 100644
--- a/vespajlib/src/main/java/com/yahoo/slime/Slime.java
+++ b/vespajlib/src/main/java/com/yahoo/slime/Slime.java
@@ -158,4 +158,20 @@ public final class Slime {
         return wrap(names.insert(name));
     }
 
+    /**
+     * Tests whether this is equal to Inspector.
+     *
+     * Since equality of two Inspectors is subtle, {@link Object#equals(Object)} is not used.
+     *
+     * @param that inspector.
+     * @return true if they are equal.
+     */
+    public boolean equalTo(Slime that) {
+        return get().equalTo(that.get());
+    }
+
+    @Override
+    public String toString() {
+        return get().toString();
+    }
 }
diff --git a/vespajlib/src/main/java/com/yahoo/slime/SlimeInserter.java b/vespajlib/src/main/java/com/yahoo/slime/SlimeInserter.java
index 2607f762f66..62b7cb06b6d 100644
--- a/vespajlib/src/main/java/com/yahoo/slime/SlimeInserter.java
+++ b/vespajlib/src/main/java/com/yahoo/slime/SlimeInserter.java
@@ -5,7 +5,7 @@ package com.yahoo.slime;
  * Helper class for inserting values into a Slime object.
  * For justification read Inserter documentation.
  **/
-final class SlimeInserter implements Inserter {
+public final class SlimeInserter implements Inserter {
     private Slime target;
     public final SlimeInserter adjust(Slime slime) {
         target = slime;
diff --git a/vespajlib/src/main/java/com/yahoo/slime/Value.java b/vespajlib/src/main/java/com/yahoo/slime/Value.java
index 183c659c35a..9d325fc7fb6 100644
--- a/vespajlib/src/main/java/com/yahoo/slime/Value.java
+++ b/vespajlib/src/main/java/com/yahoo/slime/Value.java
@@ -125,6 +125,7 @@ abstract class Value implements Cursor {
         if (equal) {
             switch (type()) {
                 case NIX:
+                    equal = valid() == that.valid();
                     break;
                 case BOOL:
                     equal = asBool() == that.asBool();
diff --git a/vespajlib/src/test/java/com/yahoo/slime/InjectorTest.java b/vespajlib/src/test/java/com/yahoo/slime/InjectorTest.java
index cfafe5ede04..c1e7fd4eece 100644
--- a/vespajlib/src/test/java/com/yahoo/slime/InjectorTest.java
+++ b/vespajlib/src/test/java/com/yahoo/slime/InjectorTest.java
@@ -1,7 +1,6 @@
 // Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.slime;
 
-import org.junit.Before;
 import org.junit.Test;
 
 import java.nio.charset.StandardCharsets;
@@ -59,10 +58,6 @@ public class InjectorTest {
 
     private final Injector injector = new Injector();
 
-    @Before
-    public void setUp() {
-    }
-
     private void inject(Inspector inspector, Inserter inserter) {
         injector.inject(inspector, inserter);
     }
@@ -73,16 +68,8 @@ public class InjectorTest {
         return inserter;
     }
 
-    private Inserter arrayInserter(Cursor cursor) {
-        var inserter = new ArrayInserter();
-        inserter.adjust(cursor);
-        return inserter;
-    }
-
-    private Inserter objectInserter(Cursor cursor, String name) {
-        var inserter = new ObjectInserter();
-        inserter.adjust(cursor, name);
-        return inserter;
+    private void assertEqualTo(Slime left, Slime right) {
+        assertTrue("'" + left + "' not equal to '" + right + "'", left.equalTo(right));
     }
 
     private void assertEqualTo(Inspector left, Inspector right) {
@@ -118,15 +105,15 @@ public class InjectorTest {
     @Test
     public void injectIntoArray() {
         f2.slime1.setArray();
-        inject(f1.empty.get(), arrayInserter(f2.slime1.get()));
-        inject(f1.nixValue.get(), arrayInserter(f2.slime1.get()));
-        inject(f1.boolValue.get(), arrayInserter(f2.slime1.get()));
-        inject(f1.longValue.get(), arrayInserter(f2.slime1.get()));
-        inject(f1.doubleValue.get(), arrayInserter(f2.slime1.get()));
-        inject(f1.stringValue.get(), arrayInserter(f2.slime1.get()));
-        inject(f1.dataValue.get(), arrayInserter(f2.slime1.get()));
-        inject(f1.arrayValue.get(), arrayInserter(f2.slime1.get()));
-        inject(f1.objectValue.get(), arrayInserter(f2.slime1.get()));
+        inject(f1.empty.get(), new ArrayInserter(f2.slime1.get()));
+        inject(f1.nixValue.get(), new ArrayInserter(f2.slime1.get()));
+        inject(f1.boolValue.get(), new ArrayInserter(f2.slime1.get()));
+        inject(f1.longValue.get(), new ArrayInserter(f2.slime1.get()));
+        inject(f1.doubleValue.get(), new ArrayInserter(f2.slime1.get()));
+        inject(f1.stringValue.get(), new ArrayInserter(f2.slime1.get()));
+        inject(f1.dataValue.get(), new ArrayInserter(f2.slime1.get()));
+        inject(f1.arrayValue.get(), new ArrayInserter(f2.slime1.get()));
+        inject(f1.objectValue.get(), new ArrayInserter(f2.slime1.get()));
 
         assertEquals(f1.empty.get().toString(), f2.slime1.get().entry(0).toString());
         assertEquals(f1.nixValue.get().toString(), f2.slime1.get().entry(1).toString());
@@ -142,15 +129,15 @@ public class InjectorTest {
     @Test
     public void injectIntoObject() {
         f2.slime1.setObject();
-        inject(f1.empty.get(), objectInserter(f2.slime1.get(), "a"));
-        inject(f1.nixValue.get(), objectInserter(f2.slime1.get(), "b"));
-        inject(f1.boolValue.get(), objectInserter(f2.slime1.get(), "c"));
-        inject(f1.longValue.get(), objectInserter(f2.slime1.get(), "d"));
-        inject(f1.doubleValue.get(), objectInserter(f2.slime1.get(), "e"));
-        inject(f1.stringValue.get(), objectInserter(f2.slime1.get(), "f"));
-        inject(f1.dataValue.get(), objectInserter(f2.slime1.get(), "g"));
-        inject(f1.arrayValue.get(), objectInserter(f2.slime1.get(), "h"));
-        inject(f1.objectValue.get(), objectInserter(f2.slime1.get(), "i"));
+        inject(f1.empty.get(), new ObjectInserter(f2.slime1.get(), "a"));
+        inject(f1.nixValue.get(), new ObjectInserter(f2.slime1.get(), "b"));
+        inject(f1.boolValue.get(), new ObjectInserter(f2.slime1.get(), "c"));
+        inject(f1.longValue.get(), new ObjectInserter(f2.slime1.get(), "d"));
+        inject(f1.doubleValue.get(), new ObjectInserter(f2.slime1.get(), "e"));
+        inject(f1.stringValue.get(), new ObjectInserter(f2.slime1.get(), "f"));
+        inject(f1.dataValue.get(), new ObjectInserter(f2.slime1.get(), "g"));
+        inject(f1.arrayValue.get(), new ObjectInserter(f2.slime1.get(), "h"));
+        inject(f1.objectValue.get(), new ObjectInserter(f2.slime1.get(), "i"));
 
         assertEquals(f1.empty.get().toString(), f2.slime1.get().field("a").toString());
         assertEquals(f1.nixValue.get().toString(), f2.slime1.get().field("b").toString());
@@ -167,11 +154,63 @@ public class InjectorTest {
     public void invalidInjectionIsIgnored() {
         inject(f1.arrayValue.get(), slimeInserter(f2.slime1));
         assertEquals(3, f2.slime1.get().entries());
-        inject(f1.longValue.get(), arrayInserter(f2.slime1.get()));
+        inject(f1.longValue.get(), new ArrayInserter(f2.slime1.get()));
         assertEquals(4, f2.slime1.get().entries());
-        inject(f1.doubleValue.get(), arrayInserter(f2.slime1.get()));
+        inject(f1.doubleValue.get(), new ArrayInserter(f2.slime1.get()));
         assertEquals(5, f2.slime1.get().entries());
-        inject(f1.nixValue.get().field("bogus"), arrayInserter(f2.slime1.get()));
+        inject(f1.nixValue.get().field("bogus"), new ArrayInserter(f2.slime1.get()));
         assertEquals(5, f2.slime1.get().entries());
     }
+
+    @Test
+    public void recursiveArrayInject() {
+        Slime expect = new Slime();
+        {
+            Cursor arr = expect.setArray();
+            arr.addLong(1);
+            arr.addLong(2);
+            arr.addLong(3);
+            {
+                Cursor arrCpy = arr.addArray();
+                arrCpy.addLong(1);
+                arrCpy.addLong(2);
+                arrCpy.addLong(3);
+            }
+        }
+        Slime data = new Slime();
+        {
+            Cursor arr = data.setArray();
+            arr.addLong(1);
+            arr.addLong(2);
+            arr.addLong(3);
+        }
+        inject(data.get(), new ArrayInserter(data.get()));
+        assertEquals(expect.toString(), data.toString());
+    }
+
+    @Test
+    public void recursiveObjectInject() {
+        Slime expect = new Slime();
+        {
+            Cursor obj = expect.setObject();
+            obj.setLong("a", 1);
+            obj.setLong("b", 2);
+            obj.setLong("c", 3);
+            {
+                Cursor obj_cpy = obj.setObject("d");
+                obj_cpy.setLong("a", 1);
+                obj_cpy.setLong("b", 2);
+                obj_cpy.setLong("c", 3);
+            }
+        }
+        Slime data = new Slime();
+        {
+            Cursor obj = data.setObject();
+            obj.setLong("a", 1);
+            obj.setLong("b", 2);
+            obj.setLong("c", 3);
+        }
+        inject(data.get(), new ObjectInserter(data.get(), "d"));
+        assertEqualTo(expect, data);
+    }
 }
\ No newline at end of file