Fixed array patching (#365) (#367)

* Fixed array patching
* corrected patcher test expectation

CHANGELOG.md

@@ -1,3 +1,9 @@
+## Naksha_2.1.4
+
+- Fixes:
+  - Patch API (POST /features) fixed to `replace` entire array instead of `append` nodes during patch operation, which otherwise prevents removal of the node even-though is desired 
+
+
 ## Naksha_1.1.1
 
 - Fixes:

here-naksha-app-service/src/main/java/com/here/naksha/app/service/http/tasks/WriteFeatureApiTask.java

@@ -20,7 +20,7 @@
 
 import static com.here.naksha.app.service.http.apis.ApiParams.*;
 import static com.here.naksha.common.http.apis.ApiParamsConst.*;
-import static com.here.naksha.lib.core.util.diff.PatcherUtils.removeAllRemoveOp;
+import static com.here.naksha.lib.core.util.diff.PatcherUtils.removeAllRemoveOpExceptForList;
 import static com.here.naksha.lib.core.util.storage.ResultHelper.readFeaturesFromResult;
 
 import com.fasterxml.jackson.core.JsonProcessingException;
@@ -389,7 +389,7 @@ private List<XyzFeature> performInMemoryPatching(
           if (inputFeature.getId().equals(storageFeature.getId())) {
             // we found matching feature in storage, so we take patched version of the feature
             final Difference difference = Patcher.getDifference(storageFeature, inputFeature);
-            final Difference diffNoRemoveOp = removeAllRemoveOp(difference);
+            final Difference diffNoRemoveOp = removeAllRemoveOpExceptForList(difference);
             featureToPatch = Patcher.patch(storageFeature, diffNoRemoveOp);
             break;
           }

here-naksha-lib-core/src/main/java/com/here/naksha/lib/core/util/diff/PatcherUtils.java

@@ -22,30 +22,33 @@
 import java.util.Map.Entry;
 
 public class PatcherUtils {
-  public static Difference removeAllRemoveOp(Difference difference) {
+  public static Difference removeAllRemoveOpExceptForList(Difference difference) {
     if (difference instanceof RemoveOp) {
       return null;
-    } else if (difference instanceof ListDiff) {
-      final ListDiff listdiff = (ListDiff) difference;
-      final Iterator<Difference> iterator = listdiff.iterator();
-      while (iterator.hasNext()) {
-        Difference next = iterator.next();
-        if (next == null) continue;
-        next = removeAllRemoveOp(next);
-        if (next == null) iterator.remove();
-      }
-      return listdiff;
     } else if (difference instanceof MapDiff) {
       final MapDiff mapdiff = (MapDiff) difference;
       final Iterator<Entry<Object, Difference>> iterator =
           mapdiff.entrySet().iterator();
       while (iterator.hasNext()) {
         Entry<Object, Difference> next = iterator.next();
-        next.setValue(removeAllRemoveOp(next.getValue()));
+        next.setValue(removeAllRemoveOpExceptForList(next.getValue()));
         if (next.getValue() == null) iterator.remove();
       }
       return mapdiff;
     }
+    // NOTE - we avoid removal of nodes for ListDiff, to ensure List is always replaced during patch and not
+    // appended
+    /*else if (difference instanceof ListDiff) {
+    final ListDiff listdiff = (ListDiff) difference;
+    final Iterator<Difference> iterator = listdiff.iterator();
+    while (iterator.hasNext()) {
+    Difference next = iterator.next();
+    if (next == null) continue;
+    next = removeAllRemoveOpExceptForList(next);
+    if (next == null) iterator.remove();
+    }
+    return listdiff;
+    }*/
     return difference;
   }
 }

here-naksha-lib-core/src/test/java/com/here/naksha/lib/core/util/diff/PatcherTest.java

@@ -24,15 +24,16 @@
 import com.here.naksha.lib.core.util.IoHelp;
 import com.here.naksha.lib.core.util.json.JsonObject;
 import com.here.naksha.lib.core.util.json.JsonSerializable;
-import java.util.ArrayList;
+
 import java.util.Arrays;
 import java.util.List;
 import java.util.stream.Stream;
+
+import com.here.naksha.test.common.FileUtil;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 import org.json.JSONException;
 import org.junit.jupiter.api.Assertions;
-import org.junit.jupiter.api.Named;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
@@ -42,7 +43,7 @@
 
 import java.util.Map;
 
-import static com.here.naksha.lib.core.util.diff.PatcherUtils.removeAllRemoveOp;
+import static com.here.naksha.lib.core.util.diff.PatcherUtils.removeAllRemoveOpExceptForList;
 import static org.junit.jupiter.api.Assertions.*;
 import static org.junit.jupiter.params.provider.Arguments.arguments;
 
@@ -113,7 +114,7 @@ void testCompareBasicNestedJson() throws JSONException {
     assertTrue(((MapDiff) nestedArrayDiff34.get(2)).get("willBeDeletedProperty") instanceof RemoveOp);
 
     // Modify the whole difference to get rid of all RemoveOp
-    Difference newDiff34 = removeAllRemoveOp(mapDiff34);
+    Difference newDiff34 = removeAllRemoveOpExceptForList(mapDiff34);
     final JsonObject patchedf3 = Patcher.patch(f3,newDiff34);
 
     assertNotNull(patchedf3);
@@ -158,6 +159,28 @@ void testCompareSameArrayDifferentOrder() throws JSONException {
     assertNull(newDiff);
   }
 
+  @Test
+  void testCompareArrayRemovalOfNode() throws JSONException {
+    final XyzFeature input =
+            FileUtil.parseJsonFileOrFail("src/test/resources/", "patcher/topology/input.json", XyzFeature.class);
+            //JsonSerializable.deserialize(IoHelp.readResource("patcher/topology/input.json"), XyzFeature.class);
+    assertNotNull(input);
+    final XyzFeature existing =
+            FileUtil.parseJsonFileOrFail("src/test/resources/", "patcher/topology/existing.json", XyzFeature.class);
+            //JsonSerializable.deserialize(IoHelp.readResource("patcher/topology/existing.json"), XyzFeature.class);
+    assertNotNull(existing);
+    final XyzFeature expected =
+            FileUtil.parseJsonFileOrFail("src/test/resources/", "patcher/topology/expected.json", XyzFeature.class);
+    assertNotNull(expected);
+
+    final Difference difference = Patcher.getDifference(existing, input);
+    final Difference diffNoRemoveOp = removeAllRemoveOpExceptForList(difference);
+    final XyzFeature patchedFeature = Patcher.patch(existing, diffNoRemoveOp);
+    assertNotNull(patchedFeature);
+
+    JSONAssert.assertEquals(expected.toString(), patchedFeature.toString(), JSONCompareMode.STRICT);
+  }
+
   @Test
   void testPatchingOnlyShuffledArrayProvided() throws JSONException {
     final JsonObject f3 =
@@ -171,7 +194,7 @@ void testPatchingOnlyShuffledArrayProvided() throws JSONException {
     final Difference diff36 = Patcher.getDifference(f3, f6);
     assertNotNull(diff36);
     // Simulate REST API behaviour, ignore all RemoveOp type of Difference
-    final Difference diff36NoRemove = removeAllRemoveOp(diff36);
+    final Difference diff36NoRemove = removeAllRemoveOpExceptForList(diff36);
     final JsonObject patchedf3Tof6 = Patcher.patch(f3, diff36NoRemove);
 
     final JsonObject expectedPatchedf3 =

here-naksha-lib-core/src/test/resources/patcher/feature_3_patched_to_4_no_remove.json

@@ -15,10 +15,8 @@
     {
       "id": "element1",
       "nestedShouldBeUpdated": "updated",
-      "willBeDeletedProperty": "not yet",
       "isAddedProperty": "yes"
-    },
-    "willBeDeletedElement"
+    }
   ],
   "speedLimit": [
     {

here-naksha-lib-core/src/test/resources/patcher/topology/existing.json

@@ -0,0 +1,109 @@
+{
+  "id": "778407118",
+  "bbox": [
+    14.20594,
+    50.02377,
+    14.20667,
+    50.02386
+  ],
+  "type": "Feature",
+  "momType": "Topology",
+  "properties": {
+    "rightAdmin": [
+      {
+        "range": {
+          "endOffset": 1.0,
+          "startOffset": 0.0
+        },
+        "value": {
+          "id": "urn:here::here:Admin:20445433",
+          "featureType": "Admin",
+          "referenceName": "adminId"
+        },
+        "confidence": {
+          "score": 0.0,
+          "scoreType": "EXISTENCE",
+          "updatedOn": "2021-02-17T21:44:27.863Z",
+          "featureType": "ROAD_ADMIN_INFORMATION",
+          "updateSource": "INJECTION_TMOB"
+        }
+      }
+    ],
+    "isLongHaul": [
+      {
+        "range": {
+          "endOffset": 1.0,
+          "startOffset": 0.0
+        },
+        "value": false
+      }
+    ],
+    "isMotorway": [
+      {
+        "range": {
+          "endOffset": 1,
+          "startOffset": 0
+        },
+        "value": false,
+        "confidence": {
+          "score": 0.9,
+          "sources": [
+            "BMW_RSD"
+          ],
+          "scoreType": "GENERIC",
+          "updatedOn": "2024-10-15T16:39:51.329Z",
+          "featureType": "UNDEFINED",
+          "updateSource": "MODERATED",
+          "triggeringReferences": [
+            {
+              "momFeatureType": "LogicalRoadSign",
+              "genericFeatureId": "urn:here::here:logicalroadsign:1622677077480690247"
+            }
+          ]
+        }
+      },
+      {
+        "range": {
+          "endOffset": 1.0,
+          "startOffset": 0.6918789974069862
+        },
+        "value": true,
+        "confidence": {
+          "score": 0.75,
+          "sources": [
+            "BMW_RSD"
+          ],
+          "scoreType": "GENERIC",
+          "updatedOn": "2024-10-05T20:59:55.69448Z",
+          "featureType": "UNDEFINED",
+          "updateSource": "WALLE",
+          "triggeringReferences": [
+            {
+              "momFeatureType": "LogicalRoadSign",
+              "genericFeatureId": "urn:here::here:logicalroadsign:1622677077480690247"
+            },
+            {
+              "momFeatureType": "LogicalRoadSign",
+              "genericFeatureId": "urn:here::here:logicalroadsign:1622677077990856712"
+            }
+          ]
+        }
+      },
+      {
+        "range": {
+          "endOffset": 0.6873084947544703,
+          "startOffset": 0.0
+        },
+        "value": false
+      }
+    ],
+    "referencePoint": {
+      "type": "Point",
+      "coordinates": [
+        14.20667,
+        50.02379,
+        416.38
+      ]
+    }
+  }
+}

here-naksha-lib-core/src/test/resources/patcher/topology/expected.json

@@ -0,0 +1,109 @@
+{
+  "momType": "Topology",
+  "referencePoint": {
+    "type": "Point",
+    "coordinates": [
+      14.20667,
+      50.02379,
+      416.38
+    ]
+  },
+  "id": "778407118",
+  "type": "Feature",
+  "bbox": [
+    14.20594,
+    50.02377,
+    14.20667,
+    50.02386
+  ],
+  "properties": {
+    "isLongHaul": [
+      {
+        "range": {
+          "endOffset": 1.0,
+          "startOffset": 0.0
+        },
+        "value": false
+      }
+    ],
+    "isMotorway": [
+      {
+        "range": {
+          "endOffset": 1,
+          "startOffset": 0
+        },
+        "value": false,
+        "confidence": {
+          "score": 0.75,
+          "sources": [
+            "BMW_RSD"
+          ],
+          "scoreType": "GENERIC",
+          "updatedOn": "2024-10-15T16:39:51.329Z",
+          "featureType": "UNDEFINED",
+          "updateSource": "MODERATED",
+          "triggeringReferences": [
+            {
+              "momFeatureType": "LogicalRoadSign",
+              "genericFeatureId": "urn:here::here:logicalroadsign:1622677077480690247"
+            }
+          ]
+        }
+      }
+    ],
+    "rightAdmin": [
+      {
+        "range": {
+          "endOffset": 1,
+          "startOffset": 0
+        },
+        "value": {
+          "id": "urn:here::here:Admin:20445433",
+          "featureType": "Admin",
+          "referenceName": "adminId"
+        },
+        "confidence": {
+          "score": 0,
+          "scoreType": "EXISTENCE",
+          "updatedOn": "2021-02-17T21:44:27.863Z",
+          "featureType": "ROAD_ADMIN_INFORMATION",
+          "updateSource": "INJECTION_TMOB"
+        }
+      }
+    ],
+    "referencePoint": {
+      "type": "Point",
+      "coordinates": [
+        14.20667,
+        50.02379,
+        416.38
+      ]
+    }
+  },
+  "geometry": {
+    "type": "LineString",
+    "coordinates": [
+      [
+        14.20667,
+        50.02379,
+        416.38
+      ],
+      [
+        14.20636,
+        50.02377,
+        417.17
+      ],
+      [
+        14.20617,
+        50.02379,
+        417.38
+      ],
+      [
+        14.20594,
+        50.02386,
+        417.16
+      ]
+    ]
+  },
+  "class": "NAVLINK"
+}