diff --git a/CHANGELOG.md b/CHANGELOG.md index c7922bd32..00be947d7 100644 --- a/CHANGELOG.md +++ b/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: diff --git a/here-naksha-app-service/src/main/java/com/here/naksha/app/service/http/tasks/WriteFeatureApiTask.java b/here-naksha-app-service/src/main/java/com/here/naksha/app/service/http/tasks/WriteFeatureApiTask.java index 628630f59..4ab53e69e 100644 --- a/here-naksha-app-service/src/main/java/com/here/naksha/app/service/http/tasks/WriteFeatureApiTask.java +++ b/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 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; } diff --git a/here-naksha-lib-core/src/main/java/com/here/naksha/lib/core/util/diff/PatcherUtils.java b/here-naksha-lib-core/src/main/java/com/here/naksha/lib/core/util/diff/PatcherUtils.java index ebc53b139..53e6d4d27 100644 --- a/here-naksha-lib-core/src/main/java/com/here/naksha/lib/core/util/diff/PatcherUtils.java +++ b/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 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> iterator = mapdiff.entrySet().iterator(); while (iterator.hasNext()) { Entry 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 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; } } diff --git a/here-naksha-lib-core/src/test/java/com/here/naksha/lib/core/util/diff/PatcherTest.java b/here-naksha-lib-core/src/test/java/com/here/naksha/lib/core/util/diff/PatcherTest.java index 4b0955425..c30fc5322 100644 --- a/here-naksha-lib-core/src/test/java/com/here/naksha/lib/core/util/diff/PatcherTest.java +++ b/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 = diff --git a/here-naksha-lib-core/src/test/resources/patcher/feature_3_patched_to_4_no_remove.json b/here-naksha-lib-core/src/test/resources/patcher/feature_3_patched_to_4_no_remove.json index ef6767850..a5d12d73b 100644 --- a/here-naksha-lib-core/src/test/resources/patcher/feature_3_patched_to_4_no_remove.json +++ b/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": [ { diff --git a/here-naksha-lib-core/src/test/resources/patcher/topology/existing.json b/here-naksha-lib-core/src/test/resources/patcher/topology/existing.json new file mode 100644 index 000000000..57f6ae503 --- /dev/null +++ b/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 + ] + } + } +} \ No newline at end of file diff --git a/here-naksha-lib-core/src/test/resources/patcher/topology/expected.json b/here-naksha-lib-core/src/test/resources/patcher/topology/expected.json new file mode 100644 index 000000000..390e16cb6 --- /dev/null +++ b/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" +} \ No newline at end of file diff --git a/here-naksha-lib-core/src/test/resources/patcher/topology/input.json b/here-naksha-lib-core/src/test/resources/patcher/topology/input.json new file mode 100644 index 000000000..bfa527fa4 --- /dev/null +++ b/here-naksha-lib-core/src/test/resources/patcher/topology/input.json @@ -0,0 +1,92 @@ +{ + "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": { + "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" + } + } + ] + }, + "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" +} \ No newline at end of file