fix bug that showed up when deleting a complex attribute with a path …

…that only matched until a primitive value. Signed-off-by: Florian Fendt <Florian.Fendt@bosch-si.com>
eclipse-ditto · Oct 30, 2019 · b195579 · b195579
1 parent b3db20b
commit b195579
Show file tree

Hide file tree

Showing 3 changed files with 72 additions and 2 deletions.
diff --git a/json/src/main/java/org/eclipse/ditto/json/ImmutableJsonObject.java b/json/src/main/java/org/eclipse/ditto/json/ImmutableJsonObject.java
@@ -212,8 +212,9 @@ public boolean contains(final CharSequence key) {
         } else {
             result = pointer.getRoot()
                     .flatMap(this::getValueForKey)
-                    .map(jsonValue -> !jsonValue.isObject() ||
-                            jsonValue.asObject().contains(pointer.nextLevel())) // Recursion
+                    .filter(JsonValue::isObject)
+                    .map(JsonValue::asObject)
+                    .map(jsonObject -> jsonObject.contains(pointer.nextLevel()))
                     .orElse(false);
         }
 

diff --git a/json/src/test/java/org/eclipse/ditto/json/ImmutableJsonObjectTest.java b/json/src/test/java/org/eclipse/ditto/json/ImmutableJsonObjectTest.java
@@ -985,6 +985,16 @@ public void containsJsonPointerReturnsExpected() {
         assertThat(underTest.contains(jsonPointer)).isTrue();
     }
 
+    @Test
+    public void containsShouldReturnFalseOnPointerDeeperThanObject() {
+        // When JsonObject
+        final ImmutableJsonObject underTest = ImmutableJsonObject.of(toMap(KNOWN_KEY_FOO, KNOWN_VALUE_BAR));
+        // pointer that goes deeper
+        final JsonPointer deeperThanObject = KNOWN_KEY_FOO.asPointer().append(JsonPointer.of("/foo/not/known/path"));
+
+        assertThat(underTest.contains(deeperThanObject)).isFalse();
+    }
+
     @Test(expected = NullPointerException.class)
     public void tryToGetJsonObjectWithNullJsonPointer() {
         final JsonObject underTest = ImmutableJsonObject.empty();

diff --git a/...o/services/things/persistence/actors/strategies/commands/DeleteAttributeStrategyTest.java b/...o/services/things/persistence/actors/strategies/commands/DeleteAttributeStrategyTest.java
@@ -17,9 +17,12 @@
 import static org.mutabilitydetector.unittesting.MutabilityMatchers.areImmutable;
 
 import org.eclipse.ditto.json.JsonFactory;
+import org.eclipse.ditto.json.JsonObject;
 import org.eclipse.ditto.json.JsonPointer;
+import org.eclipse.ditto.json.JsonValue;
 import org.eclipse.ditto.model.base.exceptions.DittoRuntimeException;
 import org.eclipse.ditto.model.base.headers.DittoHeaders;
+import org.eclipse.ditto.model.things.Thing;
 import org.eclipse.ditto.model.things.ThingId;
 import org.eclipse.ditto.services.utils.persistentactors.commands.CommandStrategy;
 import org.eclipse.ditto.signals.commands.things.modify.DeleteAttribute;
@@ -79,4 +82,60 @@ public void deleteAttributeFromThingWithoutThatAttribute() {
         assertErrorResult(underTest, THING_V2.removeAttribute(attrPointer), command, expectedException);
     }
 
+    /**
+     * Having attributes:
+     * <code>
+     * "attributes": {
+     *     "complex": {
+     *         "nested": "foo"
+     *     }
+     * }
+     * </code>
+     * a delete on <code>/complex/nested/non/existent/path</code> should result in an error
+     */
+    @Test
+    public void deleteFromComplexNestedAttributeWithoutThatPath() {
+        // the last known object key must be still part of the pointer, or the test will not work. Since "nested"
+        // is the last known part of the pointer that is part of the attributes, it has to be a primitive value.
+        // if it was an object and the object did not contain "non", then the test would fail trivially.
+        final JsonPointer attrPointer = JsonFactory.newPointer("/complex/nested/non/existent/path");
+        final CommandStrategy.Context<ThingId> context = getDefaultContext();
+        final DeleteAttribute command = DeleteAttribute.of(context.getState(), attrPointer, DittoHeaders.empty());
+        final DittoRuntimeException expectedException =
+                ExceptionFactory.attributeNotFound(context.getState(), attrPointer, command.getDittoHeaders());
+
+        final Thing thing = THING_V2.toBuilder()
+                .removeAllAttributes()
+                .setAttribute(JsonPointer.of("/complex"), JsonObject.of("{\"nested\": \"foo\"}"))
+                .build();
+
+        assertErrorResult(underTest, thing, command, expectedException);
+    }
+
+    /**
+     * Having attributes:
+     * <code>
+     * "attributes": {
+     *     "flat":"foobar"
+     * }
+     * </code>
+     * a delete on <code>/flat/non/existent/path</code> should result in an error
+     */
+    @Test
+    public void deleteFromFlatAttributeWithoutThatPath() {
+
+        final JsonPointer attrPointer = JsonFactory.newPointer("/flat/non/existent/path");
+        final CommandStrategy.Context<ThingId> context = getDefaultContext();
+        final DeleteAttribute command = DeleteAttribute.of(context.getState(), attrPointer, DittoHeaders.empty());
+        final DittoRuntimeException expectedException =
+                ExceptionFactory.attributeNotFound(context.getState(), attrPointer, command.getDittoHeaders());
+
+        final Thing thing = THING_V2.toBuilder()
+                .removeAllAttributes()
+                .setAttribute(JsonPointer.of("/flat"), JsonValue.of("foobar"))
+                .build();
+
+        assertErrorResult(underTest, thing, command, expectedException);
+    }
+
 }