From 127b0d5c0eaa2701bfe3d2e8899251a6270314ee Mon Sep 17 00:00:00 2001
From: Henning Baldersheim <balder@yahoo-inc.com>
Date: Fri, 20 Jul 2018 13:28:05 +0200
Subject: Add a prune method that can be used to remove updates that will have
 no effect when applied. Currently will only affect ASSIGN updates.

---
 .../java/com/yahoo/document/DocumentUpdate.java    | 32 ++++++++++++
 .../com/yahoo/document/DocumentUpdateTestCase.java | 58 ++++++++++++++++++++++
 2 files changed, 90 insertions(+)

diff --git a/document/src/main/java/com/yahoo/document/DocumentUpdate.java b/document/src/main/java/com/yahoo/document/DocumentUpdate.java
index 2a9ab9e6169..ef5fdc290c7 100644
--- a/document/src/main/java/com/yahoo/document/DocumentUpdate.java
+++ b/document/src/main/java/com/yahoo/document/DocumentUpdate.java
@@ -1,11 +1,14 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.document;
 
+import com.yahoo.document.datatypes.FieldValue;
 import com.yahoo.document.fieldpathupdate.FieldPathUpdate;
 import com.yahoo.document.serialization.DocumentSerializerFactory;
 import com.yahoo.document.serialization.DocumentUpdateReader;
 import com.yahoo.document.serialization.DocumentUpdateWriter;
+import com.yahoo.document.update.AssignValueUpdate;
 import com.yahoo.document.update.FieldUpdate;
+import com.yahoo.document.update.ValueUpdate;
 import com.yahoo.io.GrowableByteBuffer;
 
 import java.util.ArrayList;
@@ -116,6 +119,35 @@ public class DocumentUpdate extends DocumentOperation implements Iterable<FieldP
         return this;
     }
 
+    /**
+     * Will prune away any field update that will not modify any field in the document.
+     * @param doc
+     * @return a reference to itself
+     * @throws IllegalArgumentException if the document does not have the same document type as this update
+     */
+    public DocumentUpdate prune(Document doc) {
+        if ( ! fieldPathUpdates.isEmpty()) return this;
+
+        if (!documentType.equals(doc.getDataType())) {
+            throw new IllegalArgumentException(
+                    "Document " + doc + " must have same type as update, which is type " + documentType);
+        }
+
+        for (Iterator<FieldUpdate> iter = fieldUpdates.iterator(); iter.hasNext();) {
+            FieldUpdate update = iter.next();
+            if (!update.isEmpty()) {
+                ValueUpdate last = update.getValueUpdate(update.size() - 1);
+                if (last instanceof AssignValueUpdate) {
+                    FieldValue currentValue = doc.getFieldValue(update.getField());
+                    if ((currentValue != null) && (currentValue.compareTo(last.getValue()) == 0)) {
+                        iter.remove();
+                    }
+                }
+            }
+        }
+        return this;
+    }
+
     /**
      * Get an unmodifiable list of all field updates that this document update specifies.
      *
diff --git a/document/src/test/java/com/yahoo/document/DocumentUpdateTestCase.java b/document/src/test/java/com/yahoo/document/DocumentUpdateTestCase.java
index 4392ea932b2..4f3d7d3b820 100644
--- a/document/src/test/java/com/yahoo/document/DocumentUpdateTestCase.java
+++ b/document/src/test/java/com/yahoo/document/DocumentUpdateTestCase.java
@@ -651,6 +651,64 @@ public class DocumentUpdateTestCase {
         assertNull(doc.getFieldValue(tensorField));
     }
 
+    @Test
+    public void testThatNonIdenticalAssignCanNotBePrunedAway() {
+        Field field = docType.getField("strfoo");
+        String expected = "some other value";
+        Document doc = createDocument();
+        doc.setFieldValue(field, "some value");
+        DocumentUpdate update = new DocumentUpdate(docType, new DocumentId(documentId));
+        update.addFieldUpdate(FieldUpdate.createAssign(field, new StringFieldValue(expected)));
+        update.prune(doc);
+        assertEquals(1, update.size());
+        update.applyTo(doc);
+        assertEquals(expected, doc.getFieldValue(field).getWrappedValue());
+    }
+
+    @Test
+    public void testThatIdenticalAssignCanBePrunedAway() {
+        Field field = docType.getField("strfoo");
+        String expected = "some value";
+        Document doc = createDocument();
+        doc.setFieldValue(field, "some value");
+        DocumentUpdate update = new DocumentUpdate(docType, new DocumentId(documentId));
+        update.addFieldUpdate(FieldUpdate.createAssign(field,new StringFieldValue(expected)));
+        update.prune(doc);
+        assertEquals(0, update.size());
+        update.applyTo(doc);
+        assertEquals(expected, doc.getFieldValue(field).getWrappedValue());
+    }
+
+    @Test
+    public void testThatIdenticalAssignCanBePrunedAwayIfLast() {
+        Field field = docType.getField("strfoo");
+        String expected = "some value";
+        Document doc = createDocument();
+        doc.setFieldValue(field, "some value");
+        DocumentUpdate update = new DocumentUpdate(docType, new DocumentId(documentId));
+        update.addFieldUpdate(FieldUpdate.createClearField(field));
+        update.addFieldUpdate(FieldUpdate.createAssign(field, new StringFieldValue(expected)));
+        update.prune(doc);
+        assertEquals(0, update.size());
+        update.applyTo(doc);
+        assertEquals(expected, doc.getFieldValue(field).getWrappedValue());
+    }
+
+    @Test
+    public void testThatIdenticalAssignCanNotBePrunedAwayIfNotLast() {
+        Field field = docType.getField("strfoo");
+        String expected = "some random value";
+        Document doc = createDocument();
+        doc.setFieldValue(field, "some value");
+        DocumentUpdate update = new DocumentUpdate(docType, new DocumentId(documentId));
+        update.addFieldUpdate(FieldUpdate.createAssign(field, new StringFieldValue("some value")));
+        update.addFieldUpdate(FieldUpdate.createAssign(field, new StringFieldValue(expected)));
+        update.prune(doc);
+        assertEquals(1, update.size());
+        update.applyTo(doc);
+        assertEquals(expected, doc.getFieldValue(field).getWrappedValue());
+    }
+
     private static TensorFieldValue createTensorFieldValue(String tensor) {
         return new TensorFieldValue(Tensor.from(tensor));
     }
-- 
cgit v1.2.3


From e4eeaf0806416d5e15cfe0e9300f246e6b3093e7 Mon Sep 17 00:00:00 2001
From: Henning Baldersheim <balder@yahoo-inc.com>
Date: Fri, 20 Jul 2018 14:05:26 +0200
Subject: Update javadoc. Improve error message and produce it in one place.

---
 .../main/java/com/yahoo/document/DocumentUpdate.java | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/document/src/main/java/com/yahoo/document/DocumentUpdate.java b/document/src/main/java/com/yahoo/document/DocumentUpdate.java
index ef5fdc290c7..91e4a7091b8 100644
--- a/document/src/main/java/com/yahoo/document/DocumentUpdate.java
+++ b/document/src/main/java/com/yahoo/document/DocumentUpdate.java
@@ -97,6 +97,12 @@ public class DocumentUpdate extends DocumentOperation implements Iterable<FieldP
         docId = id;
     }
 
+    private void verifyType(Document doc) {
+        if (!documentType.equals(doc.getDataType())) {
+            throw new IllegalArgumentException(
+                    "Document " + doc.getId() + " with type " + doc.getDataType() + " must have same type as update, which is type " + documentType);
+        }
+    }
     /**
      * Applies this document update.
      *
@@ -105,10 +111,7 @@ public class DocumentUpdate extends DocumentOperation implements Iterable<FieldP
      * @throws IllegalArgumentException if the document does not have the same document type as this update
      */
     public DocumentUpdate applyTo(Document doc) {
-        if (!documentType.equals(doc.getDataType())) {
-            throw new IllegalArgumentException(
-                    "Document " + doc + " must have same type as update, which is type " + documentType);
-        }
+        verifyType(doc);
 
         for (FieldUpdate fieldUpdate : fieldUpdates) {
             fieldUpdate.applyTo(doc);
@@ -120,18 +123,15 @@ public class DocumentUpdate extends DocumentOperation implements Iterable<FieldP
     }
 
     /**
-     * Will prune away any field update that will not modify any field in the document.
-     * @param doc
+     * Prune away any field update that will not modify any field in the document.
+     * @param doc document to check against
      * @return a reference to itself
      * @throws IllegalArgumentException if the document does not have the same document type as this update
      */
     public DocumentUpdate prune(Document doc) {
         if ( ! fieldPathUpdates.isEmpty()) return this;
 
-        if (!documentType.equals(doc.getDataType())) {
-            throw new IllegalArgumentException(
-                    "Document " + doc + " must have same type as update, which is type " + documentType);
-        }
+        verifyType(doc);
 
         for (Iterator<FieldUpdate> iter = fieldUpdates.iterator(); iter.hasNext();) {
             FieldUpdate update = iter.next();
-- 
cgit v1.2.3


From e2dff4c7357ea5fcafb68a1677035c19bdde8f8a Mon Sep 17 00:00:00 2001
From: Henning Baldersheim <balder@yahoo-inc.com>
Date: Fri, 20 Jul 2018 14:19:11 +0200
Subject: Remove a restriction that is not necessary since FieldUpdates are
 applied first.

---
 document/src/main/java/com/yahoo/document/DocumentUpdate.java | 2 --
 1 file changed, 2 deletions(-)

diff --git a/document/src/main/java/com/yahoo/document/DocumentUpdate.java b/document/src/main/java/com/yahoo/document/DocumentUpdate.java
index 91e4a7091b8..70c5410534e 100644
--- a/document/src/main/java/com/yahoo/document/DocumentUpdate.java
+++ b/document/src/main/java/com/yahoo/document/DocumentUpdate.java
@@ -129,8 +129,6 @@ public class DocumentUpdate extends DocumentOperation implements Iterable<FieldP
      * @throws IllegalArgumentException if the document does not have the same document type as this update
      */
     public DocumentUpdate prune(Document doc) {
-        if ( ! fieldPathUpdates.isEmpty()) return this;
-
         verifyType(doc);
 
         for (Iterator<FieldUpdate> iter = fieldUpdates.iterator(); iter.hasNext();) {
-- 
cgit v1.2.3