Merge pull request #8412 from vespa-engine/geirst/tensor-add-and-modify-update-cleanup

Geirst/tensor add and modify update cleanup

7 files changed, 44 insertions, 74 deletions

diff --git a/document/abi-spec.json b/document/abi-spec.json
index 01fae7584cb..23c41af6995 100644
--- a/document/abi-spec.json
+++ b/document/abi-spec.json
@@ -5203,6 +5203,7 @@
       "public void setValue(com.yahoo.document.datatypes.TensorFieldValue)",
       "public boolean equals(java.lang.Object)",
       "public int hashCode()",
+      "public java.lang.String toString()",
       "public bridge synthetic void setValue(com.yahoo.document.datatypes.FieldValue)",
       "public bridge synthetic com.yahoo.document.datatypes.FieldValue getValue()"
     ],
diff --git a/document/src/main/java/com/yahoo/document/json/readers/TensorAddUpdateReader.java b/document/src/main/java/com/yahoo/document/json/readers/TensorAddUpdateReader.java
index 2cca11c19f8..ffbfe49347c 100644
--- a/document/src/main/java/com/yahoo/document/json/readers/TensorAddUpdateReader.java
+++ b/document/src/main/java/com/yahoo/document/json/readers/TensorAddUpdateReader.java
@@ -6,6 +6,7 @@ import com.yahoo.document.TensorDataType;
 import com.yahoo.document.datatypes.TensorFieldValue;
 import com.yahoo.document.json.TokenBuffer;
 import com.yahoo.document.update.TensorAddUpdate;
+import com.yahoo.tensor.Tensor;
 import com.yahoo.tensor.TensorType;
 
 import static com.yahoo.document.json.readers.JsonParserHelpers.expectObjectStart;
@@ -28,6 +29,7 @@ public class TensorAddUpdateReader {
         TensorType tensorType = tensorDataType.getTensorType();
         TensorFieldValue tensorFieldValue = new TensorFieldValue(tensorType);
         fillTensor(buffer, tensorFieldValue);
+        expectTensorIsNonEmpty(field, tensorFieldValue.getTensor().get());
         return new TensorAddUpdate(tensorFieldValue);
     }
 
@@ -40,4 +42,11 @@ public class TensorAddUpdateReader {
         }
     }
 
+    private static void expectTensorIsNonEmpty(Field field, Tensor tensor) {
+        if (tensor.isEmpty()) {
+            throw new IllegalArgumentException("Add update for field '" + field.getName() + "' does not contain tensor cells");
+        }
+    }
+
+
 }
diff --git a/document/src/main/java/com/yahoo/document/update/TensorAddUpdate.java b/document/src/main/java/com/yahoo/document/update/TensorAddUpdate.java
index 3a67e2e9de8..7a8137ce0a3 100644
--- a/document/src/main/java/com/yahoo/document/update/TensorAddUpdate.java
+++ b/document/src/main/java/com/yahoo/document/update/TensorAddUpdate.java
@@ -64,4 +64,9 @@ public class TensorAddUpdate extends ValueUpdate<TensorFieldValue> {
     public int hashCode() {
         return Objects.hash(super.hashCode(), tensor);
     }
+
+    @Override
+    public String toString() {
+        return super.toString() + " " + tensor;
+    }
 }
diff --git a/document/src/main/java/com/yahoo/document/update/TensorModifyUpdate.java b/document/src/main/java/com/yahoo/document/update/TensorModifyUpdate.java
index 679df0cf4c0..9983fb01ead 100644
--- a/document/src/main/java/com/yahoo/document/update/TensorModifyUpdate.java
+++ b/document/src/main/java/com/yahoo/document/update/TensorModifyUpdate.java
@@ -50,6 +50,7 @@ public class TensorModifyUpdate extends ValueUpdate<TensorFieldValue> {
     @Override
     public FieldValue applyTo(FieldValue oldValue) {
         if (oldValue instanceof TensorFieldValue) {
+            // TODO: implement
         } else {
             throw new IllegalStateException("Cannot use tensor modify update on non-tensor datatype "+oldValue.getClass().getName());
         }
diff --git a/document/src/test/java/com/yahoo/document/json/DocumentUpdateJsonSerializerTest.java b/document/src/test/java/com/yahoo/document/json/DocumentUpdateJsonSerializerTest.java
index 5f486e9a670..65b51ad34b8 100644
--- a/document/src/test/java/com/yahoo/document/json/DocumentUpdateJsonSerializerTest.java
+++ b/document/src/test/java/com/yahoo/document/json/DocumentUpdateJsonSerializerTest.java
@@ -296,7 +296,7 @@ public class DocumentUpdateJsonSerializerTest {
     }
 
     @Test
-    public void test_tensor_modify_update() {
+    public void test_tensor_modify_update_on_dense_tensor() {
         roundtripSerializeJsonAndMatch(inputJson(
                 "{",
                 "  'update': 'DOCUMENT_ID',",
@@ -316,6 +316,26 @@ public class DocumentUpdateJsonSerializerTest {
     }
 
     @Test
+    public void test_tensor_modify_update_on_sparse_tensor() {
+        roundtripSerializeJsonAndMatch(inputJson(
+                "{",
+                "  'update': 'DOCUMENT_ID',",
+                "  'fields': {",
+                "    'sparse_tensor': {",
+                "      'modify': {",
+                "        'operation': 'add',",
+                "        'cells': [",
+                "          { 'address': { 'x': 'a', 'y': 'b' }, 'value': 2.0 },",
+                "          { 'address': { 'x': 'c', 'y': 'd' }, 'value': 3.0 }",
+                "        ]",
+                "      }",
+                "    }",
+                "  }",
+                "}"
+        ));
+    }
+
+    @Test
     public void test_tensor_add_update() {
         roundtripSerializeJsonAndMatch(inputJson(
                 "{",
diff --git a/document/src/test/java/com/yahoo/document/json/JsonReaderTestCase.java b/document/src/test/java/com/yahoo/document/json/JsonReaderTestCase.java
index ecbd615a633..ec37ebc8295 100644
--- a/document/src/test/java/com/yahoo/document/json/JsonReaderTestCase.java
+++ b/document/src/test/java/com/yahoo/document/json/JsonReaderTestCase.java
@@ -1455,6 +1455,13 @@ public class JsonReaderTestCase {
     }
 
     @Test
+    public void tensor_add_update_without_cells_throws() {
+        exception.expect(IllegalArgumentException.class);
+        exception.expectMessage("Add update for field 'sparse_tensor' does not contain tensor cells");
+        createTensorAddUpdate(inputJson("{}"), "sparse_tensor");
+    }
+
+    @Test
     public void require_that_parser_propagates_datatype_parser_errors_predicate() {
         assertParserErrorMatches(
                 "Error in document 'id:unittest:testpredicate::0' - could not parse field 'boolean' of type 'predicate': " +
diff --git a/document/src/test/java/com/yahoo/document/update/SerializationTestCase.java b/document/src/test/java/com/yahoo/document/update/SerializationTestCase.java
index 104c77ea923..2fc1692d575 100644
--- a/document/src/test/java/com/yahoo/document/update/SerializationTestCase.java
+++ b/document/src/test/java/com/yahoo/document/update/SerializationTestCase.java
@@ -27,20 +27,12 @@ public class SerializationTestCase {
     private DocumentType documentType;
 
     private Field field;
-    private final static TensorType sparseTensorType = new TensorType.Builder().mapped("x").mapped("y").build();
-    private final static TensorType denseTensorType = new TensorType.Builder().indexed("x", 2).indexed("y", 3).build();
-    private Field sparseTensorField;
-    private Field denseTensorField;
 
     @Before
     public void setUp() {
         documentType = new DocumentType("document1");
         field = new Field("field1", DataType.getArray(DataType.STRING));
         documentType.addField(field);
-        sparseTensorField = new Field("sparse_tensor", new TensorDataType(sparseTensorType));
-        denseTensorField = new Field("dense_tensor", new TensorDataType(denseTensorType));
-        documentType.addField(sparseTensorField);
-        documentType.addField(denseTensorField);
     }
 
     @Test
@@ -73,69 +65,4 @@ public class SerializationTestCase {
         assertEquals("'field1' [clear]", deserializedUpdate.toString());
     }
 
-    @Test
-    public void test_tensor_modify_update_serialization_with_dense_tensor() {
-        String tensorString = "{{x:1,y:2}:2}";
-        FieldUpdate update = createTensorModifyUpdate(denseTensorField, denseTensorType, tensorString);
-
-        FieldUpdate deserializedUpdate = roundtripSerialize(update);
-        TensorModifyUpdate modifyUpdate = expectTensorModifyUpdate(deserializedUpdate, "dense_tensor");
-
-        assertEquals(TensorModifyUpdate.Operation.REPLACE, modifyUpdate.getOperation());
-        assertEquals(TensorType.fromSpec("tensor(x{},y{})"), modifyUpdate.getValue().getDataType().getTensorType());
-        assertEquals(createTensor(sparseTensorType, tensorString), modifyUpdate.getValue());
-        assertEquals(update, deserializedUpdate);
-    }
-
-    @Test
-    public void test_tensor_modify_update_serialization_with_sparse_tensor() {
-        String tensorString = "{{x:a,y:b}:2}";
-        FieldUpdate update = createTensorModifyUpdate(sparseTensorField, sparseTensorType, tensorString);
-
-        FieldUpdate deserializedUpdate = roundtripSerialize(update);
-        TensorModifyUpdate modifyUpdate = expectTensorModifyUpdate(deserializedUpdate, "sparse_tensor");
-
-        assertEquals(TensorModifyUpdate.Operation.REPLACE, modifyUpdate.getOperation());
-        assertEquals(TensorType.fromSpec("tensor(x{},y{})"), modifyUpdate.getValue().getDataType().getTensorType());
-        assertEquals(createTensor(sparseTensorType, tensorString), modifyUpdate.getValue());
-        assertEquals(update, deserializedUpdate);
-    }
-
-    private static FieldUpdate createTensorModifyUpdate(Field tensorField, TensorType tensorType, String tensorString) {
-        FieldUpdate result = new FieldUpdate(tensorField);
-        // Note that the tensor type is converted to only have mapped dimensions.
-        TensorFieldValue tensor = createTensor(TensorModifyUpdate.convertToCompatibleType(tensorType), tensorString);
-        result.addValueUpdate(new TensorModifyUpdate(TensorModifyUpdate.Operation.REPLACE, tensor));
-        return result;
-    }
-
-    private static TensorFieldValue createTensor(TensorType type, String tensorCellString) {
-        return new TensorFieldValue(Tensor.from(type, tensorCellString));
-    }
-
-    private static GrowableByteBuffer serializeUpdate(FieldUpdate update) {
-        DocumentSerializer buffer = DocumentSerializerFactory.createHead(new GrowableByteBuffer());
-        update.serialize(buffer);
-        buffer.getBuf().rewind();
-        return buffer.getBuf();
-    }
-
-    private FieldUpdate deserializeUpdate(GrowableByteBuffer buffer) {
-        return new FieldUpdate(DocumentDeserializerFactory.createHead(new DocumentTypeManager(), buffer), documentType, Document.SERIALIZED_VERSION);
-    }
-
-    private FieldUpdate roundtripSerialize(FieldUpdate update) {
-        GrowableByteBuffer buffer = serializeUpdate(update);
-        return deserializeUpdate(buffer);
-    }
-
-    private static TensorModifyUpdate expectTensorModifyUpdate(FieldUpdate update, String tensorFieldName) {
-        assertEquals(tensorFieldName, update.getField().getName());
-        assertEquals(1, update.getValueUpdates().size());
-        ValueUpdate valueUpdate = update.getValueUpdate(0);
-        if (!(valueUpdate instanceof TensorModifyUpdate)) {
-            throw new IllegalStateException("Expected tensorModifyUpdate");
-        }
-        return (TensorModifyUpdate)valueUpdate;
-    }
 }