#1690 use correct HTTP status code for "if-equal": skip on equality

connectivity/model/src/main/java/org/eclipse/ditto/connectivity/model/signals/commands/exceptions/ConnectionPreconditionFailedException.java

@@ -57,6 +57,15 @@ private ConnectionPreconditionFailedException(final DittoHeaders dittoHeaders,
         super(ERROR_CODE, HttpStatus.PRECONDITION_FAILED, dittoHeaders, message, description, cause, href);
     }
 
+    /**
+     * A mutable builder for a {@link ConnectionPreconditionFailedException}.
+     *
+     * @return the builder.
+     */
+    public static Builder newBuilder() {
+        return new Builder();
+    }
+
     /**
      * A mutable builder for a {@link ConnectionPreconditionFailedException}.
      *

connectivity/service/src/main/java/org/eclipse/ditto/connectivity/service/messaging/persistence/strategies/commands/ConnectionsConditionalHeadersValidatorProvider.java

@@ -60,8 +60,8 @@ public DittoRuntimeExceptionBuilder<?> createPreconditionNotModifiedExceptionBui
         }
 
         @Override
-        public DittoRuntimeExceptionBuilder<?> createPreconditionNotModifiedForEqualityExceptionBuilder() {
-            return ConnectionPreconditionNotModifiedException.newBuilder()
+        public DittoRuntimeExceptionBuilder<?> createPreconditionFailedForEqualityExceptionBuilder() {
+            return ConnectionPreconditionFailedException.newBuilder()
                     .message("The previous value was equal to the new value and the 'if-equal' header was set to 'skip'.")
                     .description("Your changes were not applied, which is probably the expected outcome.");
         }

internal/utils/conditional-headers/src/main/java/org/eclipse/ditto/internal/utils/headers/conditional/ConditionalHeadersValidator.java

@@ -63,14 +63,14 @@ DittoRuntimeExceptionBuilder<?> createPreconditionNotModifiedExceptionBuilder(St
                 String matched);
 
         /**
-         * Returns a builder for a {@link DittoRuntimeException} in case status {@code 304 (Not Modified)} should be
-         * returned for when an updated value was equal to its previous value and the {@code if-equal} condition was
+         * Returns a builder for a {@link DittoRuntimeException} in case status {@code 412 (Precondition failed)} should
+         * be returned for when an updated value was equal to its previous value and the {@code if-equal} condition was
          * set to "skip".
          *
          * @return the builder.
          * @since 3.3.0
          */
-        DittoRuntimeExceptionBuilder<?> createPreconditionNotModifiedForEqualityExceptionBuilder();
+        DittoRuntimeExceptionBuilder<?> createPreconditionFailedForEqualityExceptionBuilder();
     }
 
     private final ValidationSettings validationSettings;

internal/utils/conditional-headers/src/main/java/org/eclipse/ditto/internal/utils/headers/conditional/IfEqualPreconditionHeader.java

@@ -111,43 +111,10 @@ public boolean meetsConditionFor(@Nullable final Entity<?> entity) {
         if (ifEqual == IfEqual.SKIP) {
             if (command.getCategory() == Command.Category.MODIFY &&
                     command instanceof WithOptionalEntity withOptionalEntity) {
-                return withOptionalEntity.getEntity()
-                        .map(newValue -> {
-                            final Predicate<JsonField> nonHiddenAndNamespace =
-                                    FieldType.notHidden()
-                                            .or(jsonField -> jsonField.getKey().equals(JsonKey.of("_namespace")));
-                            final Optional<JsonValue> previousValue = entity.toJson(JsonSchemaVersion.LATEST, nonHiddenAndNamespace)
-                                    .getValue(command.getResourcePath());
-                            return previousValue.filter(jsonValue -> jsonValue.equals(newValue))
-                                    .isPresent();
-                        })
-                        .orElse(false);
+                return meetsConditionForModifyCommand(entity, withOptionalEntity);
             } else if (command.getCategory() == Command.Category.MERGE &&
                     command instanceof WithOptionalEntity withOptionalEntity) {
-                return withOptionalEntity.getEntity()
-                        .map(newValue -> {
-                            final Optional<JsonValue> previousValue = entity.toJson()
-                                    .getValue(command.getResourcePath());
-                            if (newValue.isObject()) {
-                                return previousValue.filter(JsonValue::isObject)
-                                        .map(JsonValue::asObject)
-                                        .filter(previousObject -> {
-                                            final JsonObject patchedAndSortedNewObject =
-                                                    JsonFactory.mergeJsonValues(newValue.asObject(), previousObject)
-                                                            .asObject().stream()
-                                                            .sorted(Comparator.comparing(j -> j.getKey().toString()))
-                                                            .collect(JsonCollectors.fieldsToObject());
-                                            final JsonObject sortedOldObject = previousObject.stream()
-                                                    .sorted(Comparator.comparing(j -> j.getKey().toString()))
-                                                    .collect(JsonCollectors.fieldsToObject());
-                                            return patchedAndSortedNewObject.equals(sortedOldObject);
-                                        }).isPresent();
-                            } else {
-                                return previousValue.filter(jsonValue -> jsonValue.equals(newValue))
-                                        .isPresent();
-                            }
-                        })
-                        .orElse(false);
+                return meetsConditionForMergeCommand(entity, withOptionalEntity);
             } else {
                 // other commands to "MODIFY" and "MERGE" do never match the "if-equal" precondition header
                 return false;
@@ -158,6 +125,60 @@ public boolean meetsConditionFor(@Nullable final Entity<?> entity) {
         }
     }
 
+    private Boolean meetsConditionForModifyCommand(final Entity<?> entity,
+            final WithOptionalEntity withOptionalEntity) {
+
+        return withOptionalEntity.getEntity()
+                .map(newValue -> {
+                    final Predicate<JsonField> nonHiddenAndNamespace =
+                            FieldType.notHidden()
+                                    .or(jsonField -> jsonField.getKey().equals(JsonKey.of("_namespace")));
+                    final Optional<JsonValue> previousValue =
+                            entity.toJson(JsonSchemaVersion.LATEST, nonHiddenAndNamespace)
+                                    .getValue(command.getResourcePath());
+                    return previousValue.filter(jsonValue -> jsonValue.equals(newValue))
+                            .isPresent();
+                })
+                .orElse(false);
+    }
+
+    private Boolean meetsConditionForMergeCommand(final Entity<?> entity, final WithOptionalEntity withOptionalEntity) {
+
+        return withOptionalEntity.getEntity()
+                .map(newValue -> {
+                    final Optional<JsonValue> previousValue = entity.toJson().getValue(command.getResourcePath());
+                    if (newValue.isObject()) {
+                        final JsonObject newObject;
+                        if (command.getResourcePath().isEmpty()) {
+                            // filter "special fields" for e.g. on thing level the inline "_policy":
+                            newObject = newValue.asObject()
+                                    .stream()
+                                    .filter(jsonField -> !jsonField.getKeyName().startsWith("_"))
+                                    .collect(JsonCollectors.fieldsToObject());
+                        } else {
+                            newObject = newValue.asObject();
+                        }
+                        return previousValue.filter(JsonValue::isObject)
+                                .map(JsonValue::asObject)
+                                .filter(previousObject -> {
+                                    final JsonObject patchedAndSortedNewObject =
+                                            JsonFactory.mergeJsonValues(newObject, previousObject)
+                                                    .asObject().stream()
+                                                    .sorted(Comparator.comparing(j -> j.getKey().toString()))
+                                                    .collect(JsonCollectors.fieldsToObject());
+                                    final JsonObject sortedOldObject = previousObject.stream()
+                                            .sorted(Comparator.comparing(j -> j.getKey().toString()))
+                                            .collect(JsonCollectors.fieldsToObject());
+                                    return patchedAndSortedNewObject.equals(sortedOldObject);
+                                }).isPresent();
+                    } else {
+                        return previousValue.filter(jsonValue -> jsonValue.equals(newValue))
+                                .isPresent();
+                    }
+                })
+                .orElse(false);
+    }
+
     /**
      * Handles the {@link #command} field of this class by invoking the passed {@code isCompletelyEqualSupplier} to
      * check whether the affected entity would be completely equal after applying the {@link #command}.
@@ -176,7 +197,7 @@ C handleCommand(final BooleanSupplier isCompletelyEqualSupplier) {
             final Command.Category category = command.getCategory();
             if (completelyEqual &&
                     (category == Command.Category.MODIFY || category == Command.Category.MERGE)) {
-                potentiallyAdjustedCommand = respondWithNotModified();
+                potentiallyAdjustedCommand = respondWithPreconditionFailed();
             } else {
                 potentiallyAdjustedCommand = command;
             }
@@ -186,9 +207,9 @@ C handleCommand(final BooleanSupplier isCompletelyEqualSupplier) {
         return potentiallyAdjustedCommand;
     }
 
-    private C respondWithNotModified() {
+    private C respondWithPreconditionFailed() {
         throw validationSettings
-                .createPreconditionNotModifiedForEqualityExceptionBuilder()
+                .createPreconditionFailedForEqualityExceptionBuilder()
                 .dittoHeaders(command.getDittoHeaders())
                 .build();
     }

policies/model/src/main/java/org/eclipse/ditto/policies/model/signals/commands/exceptions/PolicyPreconditionFailedException.java

@@ -19,12 +19,12 @@
 import javax.annotation.concurrent.Immutable;
 import javax.annotation.concurrent.NotThreadSafe;
 
-import org.eclipse.ditto.json.JsonObject;
 import org.eclipse.ditto.base.model.common.HttpStatus;
 import org.eclipse.ditto.base.model.exceptions.DittoRuntimeException;
 import org.eclipse.ditto.base.model.exceptions.DittoRuntimeExceptionBuilder;
 import org.eclipse.ditto.base.model.headers.DittoHeaders;
 import org.eclipse.ditto.base.model.json.JsonParsableException;
+import org.eclipse.ditto.json.JsonObject;
 import org.eclipse.ditto.policies.model.PolicyException;
 
 /**
@@ -55,6 +55,15 @@ private PolicyPreconditionFailedException(final DittoHeaders dittoHeaders,
         super(ERROR_CODE, HttpStatus.PRECONDITION_FAILED, dittoHeaders, message, description, cause, href);
     }
 
+    /**
+     * A mutable builder for a {@link org.eclipse.ditto.policies.model.signals.commands.exceptions.PolicyPreconditionFailedException}.
+     *
+     * @return the builder.
+     */
+    public static Builder newBuilder() {
+        return new Builder();
+    }
+
     /**
      * A mutable builder for a {@link org.eclipse.ditto.policies.model.signals.commands.exceptions.PolicyPreconditionFailedException}.
      *

policies/service/src/main/java/org/eclipse/ditto/policies/service/persistence/actors/strategies/commands/PoliciesConditionalHeadersValidatorProvider.java

@@ -58,8 +58,8 @@ public DittoRuntimeExceptionBuilder<?> createPreconditionNotModifiedExceptionBui
         }
 
         @Override
-        public DittoRuntimeExceptionBuilder<?> createPreconditionNotModifiedForEqualityExceptionBuilder() {
-            return PolicyPreconditionNotModifiedException.newBuilder()
+        public DittoRuntimeExceptionBuilder<?> createPreconditionFailedForEqualityExceptionBuilder() {
+            return PolicyPreconditionFailedException.newBuilder()
                     .message("The previous value was equal to the new value and the 'if-equal' header was set to 'skip'.")
                     .description("Your changes were not applied, which is probably the expected outcome.");
         }

policies/service/src/test/java/org/eclipse/ditto/policies/service/persistence/actors/strategies/commands/PoliciesConditionalHeadersValidatorTest.java

@@ -151,7 +151,7 @@ public void ifEqualDoesThrowExceptionWhenIfEqualSkipAndValueIsEqual() {
         final ModifyPolicyImport command = ModifyPolicyImport.of(policyId, policyImport,
                 DittoHeaders.newBuilder().ifEqual(IfEqual.SKIP).build());
 
-        assertThatExceptionOfType(PolicyPreconditionNotModifiedException.class)
+        assertThatExceptionOfType(PolicyPreconditionFailedException.class)
                 .isThrownBy(() -> SUT.applyIfEqualHeader(command, policy))
                 .withMessage("The previous value was equal to the new value and the 'if-equal' header was set to 'skip'.");
     }

things/model/src/main/java/org/eclipse/ditto/things/model/signals/commands/exceptions/ThingPreconditionFailedException.java

@@ -19,12 +19,12 @@
 import javax.annotation.concurrent.Immutable;
 import javax.annotation.concurrent.NotThreadSafe;
 
-import org.eclipse.ditto.json.JsonObject;
 import org.eclipse.ditto.base.model.common.HttpStatus;
 import org.eclipse.ditto.base.model.exceptions.DittoRuntimeException;
 import org.eclipse.ditto.base.model.exceptions.DittoRuntimeExceptionBuilder;
 import org.eclipse.ditto.base.model.headers.DittoHeaders;
 import org.eclipse.ditto.base.model.json.JsonParsableException;
+import org.eclipse.ditto.json.JsonObject;
 import org.eclipse.ditto.things.model.ThingException;
 
 /**
@@ -55,6 +55,15 @@ private ThingPreconditionFailedException(final DittoHeaders dittoHeaders,
         super(ERROR_CODE, HttpStatus.PRECONDITION_FAILED, dittoHeaders, message, description, cause, href);
     }
 
+    /**
+     * A mutable builder for a {@link ThingPreconditionFailedException}.
+     *
+     * @return the builder.
+     */
+    public static Builder newBuilder() {
+        return new Builder();
+    }
+
     /**
      * A mutable builder for a {@link ThingPreconditionFailedException}.
      *

things/service/src/main/java/org/eclipse/ditto/things/service/persistence/actors/strategies/commands/ThingsConditionalHeadersValidatorProvider.java

@@ -70,8 +70,8 @@ public DittoRuntimeExceptionBuilder<?> createPreconditionNotModifiedExceptionBui
         }
 
         @Override
-        public DittoRuntimeExceptionBuilder<?> createPreconditionNotModifiedForEqualityExceptionBuilder() {
-            return ThingPreconditionNotModifiedException.newBuilder()
+        public DittoRuntimeExceptionBuilder<?> createPreconditionFailedForEqualityExceptionBuilder() {
+            return ThingPreconditionFailedException.newBuilder()
                     .message("The previous value was equal to the new value and the 'if-equal' header was set to 'skip'.")
                     .description("Your changes were not applied, which is probably the expected outcome.");
         }

things/service/src/test/java/org/eclipse/ditto/things/service/persistence/actors/strategies/commands/ThingsConditionalHeadersValidatorTest.java

@@ -179,7 +179,7 @@ public void ifEqualDoesThrowExceptionWhenIfEqualSkipAndValueIsEqual() {
         final ModifyAttribute command = ModifyAttribute.of(thingId, attributePath, attributeValue,
                 DittoHeaders.newBuilder().ifEqual(IfEqual.SKIP).build());
 
-        assertThatExceptionOfType(ThingPreconditionNotModifiedException.class)
+        assertThatExceptionOfType(ThingPreconditionFailedException.class)
                 .isThrownBy(() -> SUT.applyIfEqualHeader(command, thing))
                 .withMessage("The previous value was equal to the new value and the 'if-equal' header was set to 'skip'.");
     }
@@ -196,7 +196,7 @@ public void ifEqualDoesThrowExceptionWhenIfEqualSkipAndValueIsEqualUsingMerge()
         final MergeThing command = MergeThing.of(thingId, JsonPointer.empty(), thing.toJson(),
                 DittoHeaders.newBuilder().ifEqual(IfEqual.SKIP).build());
 
-        assertThatExceptionOfType(ThingPreconditionNotModifiedException.class)
+        assertThatExceptionOfType(ThingPreconditionFailedException.class)
                 .isThrownBy(() -> SUT.applyIfEqualHeader(command, thing))
                 .withMessage("The previous value was equal to the new value and the 'if-equal' header was set to 'skip'.");
     }