eclipse-ditto#1650: adding support for:

* validating features and properties as part of "modify thing" * handling "ditto:category" correctly * ensuring completeness of defined features * validation on creation/modification * of thing * of attributes * on attribute * on features * on feature Signed-off-by: Thomas Jäckle <[email protected]>
beyonnex-io · Jun 5, 2024 · 5550fa0 · 5550fa0
1 parent e0200da
commit 5550fa0
Show file tree

Hide file tree

Showing 20 changed files with 892 additions and 241 deletions.
diff --git a/...java/org/eclipse/ditto/internal/utils/persistentactors/AbstractPersistenceSupervisor.java b/...java/org/eclipse/ditto/internal/utils/persistentactors/AbstractPersistenceSupervisor.java
@@ -842,6 +842,10 @@ private void handleSignalEnforcementResponse(@Nullable final Object response,
                 log.withCorrelationId(dre)
                         .info("Received DittoRuntimeException during enforcement or " +
                                 "forwarding to target actor, telling sender: {}", dre);
+                if (dre instanceof DittoInternalErrorException) {
+                    log.withCorrelationId(dre)
+                            .error(dre, "Received DittoInternalErrorException during enforcement");
+                }
             }
             sender.tell(dre, getSelf());
         } else if (response instanceof Status.Success success) {

diff --git a/...ipse/ditto/things/service/persistence/actors/strategies/commands/CreateThingStrategy.java b/...ipse/ditto/things/service/persistence/actors/strategies/commands/CreateThingStrategy.java
@@ -125,12 +125,18 @@ protected Result<ThingEvent<?>> doApply(final Context<ThingId> context,
                         .build()
                 );
 
+        // validate based on potentially referenced Thing WoT TM/TD
+        final CompletionStage<Thing> validatedStage = thingStage.thenCompose(createdThing -> wotThingModelValidator
+                .validateThing(createdThing, command.getResourcePath(), command.getDittoHeaders())
+                .thenApply(aVoid -> createdThing)
+        );
+
         final CompletionStage<ThingEvent<?>> eventStage =
-                thingStage.thenApply(newThingWithImplicits ->
+                validatedStage.thenApply(newThingWithImplicits ->
                         ThingCreated.of(newThingWithImplicits, nextRevision, now, commandHeaders, metadata)
                 );
 
-        final CompletionStage<WithDittoHeaders> responseStage = thingStage.thenApply(newThingWithImplicits ->
+        final CompletionStage<WithDittoHeaders> responseStage = validatedStage.thenApply(newThingWithImplicits ->
                 appendETagHeaderIfProvided(command, CreateThingResponse.of(newThingWithImplicits, commandHeaders),
                         newThingWithImplicits)
         );

diff --git a/.../ditto/things/service/persistence/actors/strategies/commands/ModifyAttributeStrategy.java b/.../ditto/things/service/persistence/actors/strategies/commands/ModifyAttributeStrategy.java
@@ -13,6 +13,7 @@
 package org.eclipse.ditto.things.service.persistence.actors.strategies.commands;
 
 import java.util.Optional;
+import java.util.concurrent.CompletionStage;
 
 import javax.annotation.Nullable;
 import javax.annotation.concurrent.Immutable;
@@ -91,13 +92,27 @@ private Result<ThingEvent<?>> getModifyResult(final Context<ThingId> context, fi
         final JsonPointer attributePointer = command.getAttributePointer();
         final DittoHeaders dittoHeaders = command.getDittoHeaders();
 
-        final ThingEvent<?> event =
+        final CompletionStage<Void> validatedStage = getValidatedStage(command, thing);
+        final CompletionStage<ThingEvent<?>> eventStage = validatedStage.thenApply(aVoid ->
                 AttributeModified.of(thingId, attributePointer, command.getAttributeValue(), nextRevision,
-                        getEventTimestamp(), dittoHeaders, metadata);
-        final WithDittoHeaders response = appendETagHeaderIfProvided(command,
-                ModifyAttributeResponse.modified(thingId, attributePointer, dittoHeaders), thing);
+                        getEventTimestamp(), dittoHeaders, metadata)
+        );
+        final CompletionStage<WithDittoHeaders> responseStage = validatedStage.thenApply(aVoid ->
+                appendETagHeaderIfProvided(command,
+                        ModifyAttributeResponse.modified(thingId, attributePointer, dittoHeaders), thing)
+        );
+
+        return ResultFactory.newMutationResult(command, eventStage, responseStage);
+    }
 
-        return ResultFactory.newMutationResult(command, event, response);
+    private CompletionStage<Void> getValidatedStage(final ModifyAttribute command, @Nullable final Thing thing) {
+        return wotThingModelValidator
+                .validateThingAttribute(Optional.ofNullable(thing).flatMap(Thing::getDefinition).orElse(null),
+                        command.getAttributePointer(),
+                        command.getAttributeValue(),
+                        command.getResourcePath(),
+                        command.getDittoHeaders()
+                );
     }
 
     private Result<ThingEvent<?>> getCreateResult(final Context<ThingId> context, final long nextRevision,
@@ -108,13 +123,17 @@ private Result<ThingEvent<?>> getCreateResult(final Context<ThingId> context, fi
         final JsonValue attributeValue = command.getAttributeValue();
         final DittoHeaders dittoHeaders = command.getDittoHeaders();
 
-        final ThingEvent<?> event =
+        final CompletionStage<Void> validatedStage = getValidatedStage(command, thing);
+        final CompletionStage<ThingEvent<?>> eventStage = validatedStage.thenApply(aVoid ->
                 AttributeCreated.of(thingId, attributePointer, attributeValue, nextRevision, getEventTimestamp(),
-                        dittoHeaders, metadata);
-        final WithDittoHeaders response = appendETagHeaderIfProvided(command,
-                ModifyAttributeResponse.created(thingId, attributePointer, attributeValue, dittoHeaders), thing);
-
-        return ResultFactory.newMutationResult(command, event, response);
+                        dittoHeaders, metadata)
+        );
+        final CompletionStage<WithDittoHeaders> responseStage = validatedStage.thenApply(aVoid ->
+                appendETagHeaderIfProvided(command,
+                        ModifyAttributeResponse.created(thingId, attributePointer, attributeValue, dittoHeaders), thing)
+        );
+
+        return ResultFactory.newMutationResult(command, eventStage, responseStage);
     }
 
     @Override

diff --git a/...ditto/things/service/persistence/actors/strategies/commands/ModifyAttributesStrategy.java b/...ditto/things/service/persistence/actors/strategies/commands/ModifyAttributesStrategy.java
@@ -13,6 +13,7 @@
 package org.eclipse.ditto.things.service.persistence.actors.strategies.commands;
 
 import java.util.Optional;
+import java.util.concurrent.CompletionStage;
 
 import javax.annotation.Nullable;
 import javax.annotation.concurrent.Immutable;
@@ -87,27 +88,42 @@ private Result<ThingEvent<?>> getModifyResult(final Context<ThingId> context, fi
         final ThingId thingId = context.getState();
         final DittoHeaders dittoHeaders = command.getDittoHeaders();
 
-        final ThingEvent<?> event =
-                AttributesModified.of(thingId, command.getAttributes(), nextRevision, getEventTimestamp(),
-                        dittoHeaders, metadata);
-        final WithDittoHeaders response = appendETagHeaderIfProvided(command,
-                ModifyAttributesResponse.modified(thingId, dittoHeaders), thing);
+        final CompletionStage<Attributes> validatedStage = getValidatedStage(command, thing);
+        final CompletionStage<ThingEvent<?>> eventStage = validatedStage.thenApply(attributes ->
+                AttributesModified.of(thingId, attributes, nextRevision, getEventTimestamp(), dittoHeaders, metadata)
+        );
+        final CompletionStage<WithDittoHeaders> responseStage = validatedStage.thenApply(attributes ->
+                appendETagHeaderIfProvided(command, ModifyAttributesResponse.modified(thingId, dittoHeaders), thing)
+        );
 
-        return ResultFactory.newMutationResult(command, event, response);
+        return ResultFactory.newMutationResult(command, eventStage, responseStage);
+    }
+
+    private CompletionStage<Attributes> getValidatedStage(final ModifyAttributes command, @Nullable final Thing thing) {
+        return wotThingModelValidator
+                .validateThingAttributes(Optional.ofNullable(thing).flatMap(Thing::getDefinition).orElse(null),
+                        command.getAttributes(),
+                        command.getResourcePath(),
+                        command.getDittoHeaders()
+                )
+                .thenApply(aVoid -> command.getAttributes());
     }
 
     private Result<ThingEvent<?>> getCreateResult(final Context<ThingId> context, final long nextRevision,
             final ModifyAttributes command, @Nullable final Thing thing, @Nullable final Metadata metadata) {
         final ThingId thingId = context.getState();
-        final Attributes attributes = command.getAttributes();
         final DittoHeaders dittoHeaders = command.getDittoHeaders();
 
-        final ThingEvent<?> event =
-                AttributesCreated.of(thingId, attributes, nextRevision, getEventTimestamp(), dittoHeaders, metadata);
-        final WithDittoHeaders response = appendETagHeaderIfProvided(command,
-                ModifyAttributesResponse.created(thingId, attributes, dittoHeaders), thing);
+        final CompletionStage<Attributes> validatedStage = getValidatedStage(command, thing);
+        final CompletionStage<ThingEvent<?>> eventStage = validatedStage.thenApply(attributes ->
+                AttributesCreated.of(thingId, attributes, nextRevision, getEventTimestamp(), dittoHeaders, metadata)
+        );
+        final CompletionStage<WithDittoHeaders> responseStage = validatedStage.thenApply(attributes ->
+                appendETagHeaderIfProvided(command, ModifyAttributesResponse.created(thingId, attributes, dittoHeaders),
+                        thing)
+        );
 
-        return ResultFactory.newMutationResult(command, event, response);
+        return ResultFactory.newMutationResult(command, eventStage, responseStage);
     }
 
     @Override

diff --git a/...hings/service/persistence/actors/strategies/commands/ModifyFeatureDefinitionStrategy.java b/...hings/service/persistence/actors/strategies/commands/ModifyFeatureDefinitionStrategy.java
@@ -55,6 +55,8 @@ protected Result<ThingEvent<?>> doApply(final Context<ThingId> context,
             final ModifyFeatureDefinition command,
             @Nullable final Metadata metadata) {
 
+        // TODO TJ we probably must also validate the current feature properties based on the new feature definition
+
         final String featureId = command.getFeatureId();
 
         return extractFeature(command, thing)

diff --git a/...se/ditto/things/service/persistence/actors/strategies/commands/ModifyFeatureStrategy.java b/...se/ditto/things/service/persistence/actors/strategies/commands/ModifyFeatureStrategy.java
@@ -95,7 +95,12 @@ protected Result<ThingEvent<?>> doApply(final Context<ThingId> context,
                     // validate based on potentially referenced Feature WoT TM
                     final CompletionStage<Void> validatedStage;
                     if (featureDefinition.isPresent()) {
-                        validatedStage = wotThingModelValidator.validateFeature(feature, command.getDittoHeaders());
+                        validatedStage = wotThingModelValidator.validateFeature(
+                                nonNullThing.getDefinition().orElse(null),
+                                feature,
+                                command.getResourcePath(),
+                                command.getDittoHeaders()
+                        );
                     } else {
                         validatedStage = CompletableFuture.completedStage(null);
                     }
@@ -169,7 +174,12 @@ private Result<ThingEvent<?>> getCreateResult(final Context<ThingId> context, fi
                 );
 
         final Function<Feature, CompletionStage<Feature>> validationFunction = feature ->
-                wotThingModelValidator.validateFeature(feature, command.getDittoHeaders())
+                wotThingModelValidator.validateFeature(
+                                Optional.ofNullable(thing).flatMap(Thing::getDefinition).orElse(null),
+                                feature,
+                                command.getResourcePath(),
+                                command.getDittoHeaders()
+                        )
                         .thenApply(aVoid -> feature);
         final CompletionStage<ThingEvent<?>> eventStage =
                 featureStage.thenCompose(validationFunction).thenApply(feature ->

diff --git a/...e/ditto/things/service/persistence/actors/strategies/commands/ModifyFeaturesStrategy.java b/...e/ditto/things/service/persistence/actors/strategies/commands/ModifyFeaturesStrategy.java
@@ -19,6 +19,7 @@
 import java.util.Set;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.CompletionStage;
+import java.util.function.Function;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
@@ -79,7 +80,8 @@ protected Result<ThingEvent<?>> doApply(final Context<ThingId> context,
         ThingCommandSizeValidator.getInstance().ensureValidSize(
                 () -> {
                     final long lengthWithOutFeatures = thingWithoutFeaturesJsonObject.getUpperBoundForStringSize();
-                    final long featuresLength = featuresJsonObject.getUpperBoundForStringSize() + "features".length() + 5L;
+                    final long featuresLength =
+                            featuresJsonObject.getUpperBoundForStringSize() + "features".length() + 5L;
                     return lengthWithOutFeatures + featuresLength;
                 },
                 () -> {
@@ -99,13 +101,30 @@ private Result<ThingEvent<?>> getModifyResult(final Context<ThingId> context, fi
 
         final DittoHeaders dittoHeaders = command.getDittoHeaders();
 
-        final ThingEvent<?> event =
-                FeaturesModified.of(command.getEntityId(), command.getFeatures(), nextRevision,
-                        getEventTimestamp(), dittoHeaders, metadata);
-        final WithDittoHeaders response = appendETagHeaderIfProvided(command,
-                ModifyFeaturesResponse.modified(context.getState(), dittoHeaders), thing);
+        final CompletionStage<Features> validationStage = getValidationStage(command, command.getFeatures(), thing);
+        final CompletionStage<ThingEvent<?>> eventStage = validationStage.thenApply(features ->
+                FeaturesModified.of(command.getEntityId(), features, nextRevision,
+                        getEventTimestamp(), dittoHeaders, metadata)
+        );
+        final CompletionStage<WithDittoHeaders> responseStage = validationStage.thenApply(features ->
+                appendETagHeaderIfProvided(command,
+                        ModifyFeaturesResponse.modified(context.getState(), dittoHeaders), thing)
+        );
 
-        return ResultFactory.newMutationResult(command, event, response);
+        return ResultFactory.newMutationResult(command, eventStage, responseStage);
+    }
+
+    private CompletionStage<Features> getValidationStage(final ModifyFeatures command,
+            final Features features,
+            @Nullable final Thing thing
+    ) {
+        return wotThingModelValidator.validateFeatures(
+                        Optional.ofNullable(thing).flatMap(Thing::getDefinition).orElse(null),
+                        features,
+                        command.getResourcePath(),
+                        command.getDittoHeaders()
+                )
+                .thenApply(aVoid -> command.getFeatures());
     }
 
     private Result<ThingEvent<?>> getCreateResult(final Context<ThingId> context, final long nextRevision,
@@ -159,14 +178,18 @@ private Result<ThingEvent<?>> getCreateResult(final Context<ThingId> context, fi
                                 )
                         );
 
-        final CompletableFuture<ThingEvent<?>> eventStage = featuresStage.thenApply(features ->
-                FeaturesCreated.of(command.getEntityId(), features, nextRevision, getEventTimestamp(),
-                        dittoHeaders, metadata
-                )
-        );
+        final Function<Features, CompletionStage<Features>> validationFunction = features ->
+                getValidationStage(command, features, thing)
+                        .thenApply(aVoid -> features);
 
+        final CompletableFuture<ThingEvent<?>> eventStage =
+                featuresStage.thenCompose(validationFunction).thenApply(features ->
+                        FeaturesCreated.of(command.getEntityId(), features, nextRevision, getEventTimestamp(),
+                                dittoHeaders, metadata
+                        )
+                );
         final CompletableFuture<WithDittoHeaders> responseStage =
-                featuresStage.thenApply(features -> appendETagHeaderIfProvided(command,
+                featuresStage.thenCompose(validationFunction).thenApply(features -> appendETagHeaderIfProvided(command,
                         ModifyFeaturesResponse.created(context.getState(), features, dittoHeaders), thing)
                 );
 

diff --git a/.../things/service/persistence/actors/strategies/commands/ModifyThingDefinitionStrategy.java b/.../things/service/persistence/actors/strategies/commands/ModifyThingDefinitionStrategy.java
@@ -55,6 +55,10 @@ protected Result<ThingEvent<?>> doApply(final Context<ThingId> context,
             final ModifyThingDefinition command,
             @Nullable final Metadata metadata) {
 
+        // TODO TJ when changing the thing "definition", we must also validate the complete thing against the new definition
+        //  and fail the request if it does not match
+        //  only that way, we can support upgrading a WoT model version
+
         return extractDefinition(thing)
                 .map(definition -> getModifyResult(context, nextRevision, command, thing, metadata))
                 .orElseGet(() -> getCreateResult(context, nextRevision, command, thing, metadata));

diff --git a/...ipse/ditto/things/service/persistence/actors/strategies/commands/ModifyThingStrategy.java b/...ipse/ditto/things/service/persistence/actors/strategies/commands/ModifyThingStrategy.java
@@ -99,16 +99,17 @@ private Result<ThingEvent<?>> applyModifyCommand(final Context<ThingId> context,
 
         final Thing modifiedThing = applyThingModifications(command.getThing(), thing, eventTs, nextRevision);
         // validate based on potentially referenced Thing WoT TM/TD
-        final CompletionStage<Void> validatedStage = wotThingModelValidator
-                .validateThing(modifiedThing, command.getDittoHeaders());
+        final CompletionStage<Thing> validatedStage = wotThingModelValidator
+                .validateThing(modifiedThing, command.getResourcePath(), command.getDittoHeaders())
+                .thenApply(aVoid -> modifiedThing);
 
         final CompletionStage<ThingEvent<?>> eventStage =
-                validatedStage.thenApply(aVoid ->
-                        ThingModified.of(modifiedThing, nextRevision, eventTs, dittoHeaders, metadata));
+                validatedStage.thenApply(theThing ->
+                        ThingModified.of(theThing, nextRevision, eventTs, dittoHeaders, metadata));
         final CompletionStage<WithDittoHeaders> responseStage =
-                validatedStage.thenApply(aVoid ->
+                validatedStage.thenApply(theThing ->
                         appendETagHeaderIfProvided(command,
-                                ModifyThingResponse.modified(context.getState(), dittoHeaders), modifiedThing));
+                                ModifyThingResponse.modified(context.getState(), dittoHeaders), theThing));
 
         return ResultFactory.newMutationResult(command, eventStage, responseStage);
     }

diff --git a/things/service/src/main/resources/things.conf b/things/service/src/main/resources/things.conf
@@ -246,7 +246,7 @@ ditto {
       }
 
       tm-model-validation {
-        enabled = true # TODO TJ default should be false
+        enabled = true # TODO TJ default should be false to "opt in" into the WoT validation explicitly
         enabled = ${?THINGS_WOT_TM_MODEL_VALIDATION_ENABLED}
 
         thing {
@@ -261,6 +261,9 @@ ditto {
         }
 
         feature {
+          enforce-presence-of-modeled-features = true
+          allow-non-modeled-features = false
+
           enforce-properties = true
           allow-non-modeled-properties = false