#1939 fix merge / PATCH regex based deletion during patch

* 2 problems fixed:
  * deletion was not applied correctly for nested JsonObjects
  * chosen regex delimiters `/` were incompatible with HTTP API checks that no slashes may be contained in JsonKeys and feature ids

Signed-off-by: Thomas Jäckle <[email protected]>

Author: thjaeckle

documentation/src/main/resources/pages/ditto/httpapi-concepts.md

@@ -408,14 +408,25 @@ JSON objects using a regular expression **before** applying all other patch valu
 
 The syntax for this function is rather specific (so that no "normally" occurring JSON keys match the same syntax):
 ```json
+{
+  "{%raw%}{{ ~.*~ }}{%endraw%}": null
+}
+```
+
+So the recommended regex delimiter to use is the `~` character, additionally supported delimiters at this point are:
+* `~`
+* `/` (discouraged due to the special meaning of `/` for HTTP paths)
+
+A discouraged and thus deprecated way to define the regex (using `/` as delimiter) is:
+```json
 {
   "{%raw%}{{ /.*/ }}{%endraw%}": null
 }
 ```
 
-When such a `{%raw%}{{ /<regex>/ }}{%endraw%}` with the value `null` is detected in the merge patch, the content between the 2 `/` is 
+When such a `{%raw%}{{ ~<regex>~ }}{%endraw%}` with the value `null` is detected in the merge patch, the content between the 2 delimiters is 
 interpreted as regular expression to apply for finding keys to delete from the target object.  
-As a result, using `"{%raw%}{{ /.*/ }}{%endraw%}": null` would delete all the values inside a JSON object before applying the new 
+As a result, using `"{%raw%}{{ ~.*~ }}{%endraw%}": null` would delete all the values inside a JSON object before applying the new 
 values provided in the patch.
 
 Example:  
@@ -444,7 +455,7 @@ of this year:
   "features": {
     "aggregated-history": {
       "properties": {
-        "{%raw%}{{ /2022-.*/ }}{%endraw%}": null,
+        "{%raw%}{{ ~2022-.*~ }}{%endraw%}": null,
         "2023-03": 105.21
       }
     }

json/src/main/java/org/eclipse/ditto/json/JsonMergePatch.java

@@ -194,8 +194,10 @@ private static JsonObject mergeJsonObjects(final JsonObject jsonObject1, final J
 
         // add fields of jsonObject2 not present in jsonObject1
         jsonObject2.forEach(jsonField -> {
-            if (!jsonObject1.contains(jsonField.getKey()) && toBeNulledKeysByRegex.contains(jsonField.getKey())) {
+            if (toBeNulledKeysByRegex.contains(jsonField.getKey())) {
                 builder.remove(jsonField.getKey());
+                jsonObject1.getValue(jsonField.getKey())
+                        .ifPresent(v -> builder.set(jsonField.getKey(), v));
             }
         });
 
@@ -208,11 +210,11 @@ private static List<JsonKey> determineToBeNulledKeysByRegex(
 
         final List<JsonKey> toBeNulledKeysByRegex = new ArrayList<>();
         final List<JsonKey> keyRegexes = jsonObject1.getKeys().stream()
-                .filter(key -> key.toString().startsWith("{{") && key.toString().endsWith("}}"))
+                .filter(JsonMergePatch::isEnclosedByCurlyBraces)
                 .collect(Collectors.toList());
         keyRegexes.forEach(keyRegex -> {
             final String keyRegexWithoutCurly = keyRegex.toString().substring(2, keyRegex.length() - 2).trim();
-            if (keyRegexWithoutCurly.startsWith("/") && keyRegexWithoutCurly.endsWith("/")) {
+            if (isEnclosedByRegexDelimiter(keyRegexWithoutCurly)) {
                 final String regexStr = keyRegexWithoutCurly.substring(1, keyRegexWithoutCurly.length() - 1);
                 final Pattern pattern = Pattern.compile(regexStr);
                 jsonObject1.getValue(keyRegex)
@@ -227,6 +229,15 @@ private static List<JsonKey> determineToBeNulledKeysByRegex(
         return toBeNulledKeysByRegex;
     }
 
+    private static boolean isEnclosedByCurlyBraces(final JsonKey key) {
+        return key.toString().startsWith("{{") && key.toString().endsWith("}}");
+    }
+
+    private static boolean isEnclosedByRegexDelimiter(final String keyRegexWithoutCurly) {
+        return keyRegexWithoutCurly.startsWith("~") && keyRegexWithoutCurly.endsWith("~") ||
+                keyRegexWithoutCurly.startsWith("/") && keyRegexWithoutCurly.endsWith("/");
+    }
+
     /**
      * Applies this merge patch on the given json value.
      *

json/src/test/java/org/eclipse/ditto/json/JsonMergePatchTest.java

@@ -476,21 +476,21 @@ public void removeFieldsUsingRegexWithNullValue() {
 
         final JsonObject objectToPatch = JsonFactory.newObjectBuilder()
                 .set("a", JsonFactory.newObjectBuilder()
-                        .set("{{ /2023-04-.*/ }}", JsonValue.nullLiteral())
+                        .set("{{ ~2023-04-.*~ }}", JsonValue.nullLiteral())
                         .set("2023-05-01", JsonValue.of("new"))
                         .set("2023-05-02", JsonValue.of("catch"))
                         .set("2023-05-03", JsonValue.of("phrase"))
                         .build())
                 .set("b", JsonFactory.newObjectBuilder()
-                        .set("{{ /2023-04-01/ }}", JsonValue.nullLiteral())
-                        .set("{{ /^2023-04-03$/ }}", JsonValue.nullLiteral())
-                        .set("{{ /[0-9]{4}-04-.+4/ }}", JsonValue.nullLiteral())
+                        .set("{{ ~2023-04-01~ }}", JsonValue.nullLiteral())
+                        .set("{{ ~^2023-04-03$~ }}", JsonValue.nullLiteral())
+                        .set("{{ ~[0-9]{4}-04-.+4~ }}", JsonValue.nullLiteral())
                         .set("2023-05-01", JsonValue.of("new"))
                         .set("2023-05-02", JsonValue.of("catch"))
                         .set("2023-05-03", JsonValue.of("phrase"))
                         .build())
                 .set("c", JsonFactory.newObjectBuilder()
-                        .set("{{ /.*/ }}", JsonValue.nullLiteral())
+                        .set("{{ ~.*~ }}", JsonValue.nullLiteral())
                         .set("2023-05-01", JsonValue.of("new"))
                         .set("2023-05-02", JsonValue.of("catch"))
                         .set("2023-05-03", JsonValue.of("phrase"))
@@ -523,5 +523,50 @@ public void removeFieldsUsingRegexWithNullValue() {
         Assertions.assertThat(mergedObject).isEqualTo(expectedObject);
     }
 
+    @Test
+    public void removeFieldsUsingRegexWithNullValueWithHierarchy() {
+        final JsonObject originalObject = JsonFactory.newObjectBuilder()
+                .set("first", JsonFactory.newObjectBuilder()
+                        .set("second", JsonFactory.newObjectBuilder()
+                                .set("third", JsonFactory.newObjectBuilder()
+                                        .set("something-on-third", "foobar3")
+                                        .set("another-on-third", false)
+                                        .build())
+                                .set("something-on-second", "foobar2")
+                                .set("another-on-second", false)
+                                .build())
+                        .set("something-on-first", "foobar1")
+                        .set("another-on-first", 42)
+                        .build()
+                )
+                .build();
+
+        final JsonObject objectToPatch = JsonFactory.newObjectBuilder()
+                .set("first", JsonFactory.newObjectBuilder()
+                        .set("{{ ~seco.*~ }}", JsonValue.nullLiteral())
+                        .set("second", JsonFactory.newObjectBuilder()
+                                .set("another-on-second", true)
+                                .build()
+                        )
+                        .build()
+                )
+                .build();
+
+        final JsonValue expectedObject = JsonFactory.newObjectBuilder()
+                .set("first", JsonFactory.newObjectBuilder()
+                        .set("second", JsonFactory.newObjectBuilder()
+                                .set("another-on-second", true)
+                                .build())
+                        .set("something-on-first", "foobar1")
+                        .set("another-on-first", 42)
+                        .build()
+                )
+                .build();
+
+        final JsonValue mergedObject = JsonMergePatch.of(objectToPatch).applyOn(originalObject);
+
+        Assertions.assertThat(mergedObject).isEqualTo(expectedObject);
+    }
+
 
 }