Merge pull request eclipse-ditto#1738 from eclipse-ditto/bugfix/implicit-policy-creation-error-message

fix wrong exception message when policy could not be implicitly created when creating thing

Author: thjaeckle

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

@@ -49,6 +49,9 @@ public final class ThingNotCreatableException extends DittoRuntimeException impl
             "The Thing with ID ''{0}'' could not be created because creation of its " +
                     "implicit Policy ID ''{1}'' failed.";
 
+    static final String MESSAGE_TEMPLATE_CUSTOM_REASON = "The Thing with ID ''{0}'' could not be created as the " +
+            "Policy with ID ''{1}'' could not be created due to: ''{2}''";
+
     static final String DEFAULT_DESCRIPTION_NOT_EXISTING =
             "Check if the ID of the Policy you created the Thing with is correct and that the Policy is existing.";
 
@@ -101,6 +104,19 @@ public static Builder newBuilderForPolicyExisting(final ThingId thingId, final P
         return new Builder(thingId, policyId, false);
     }
 
+    /**
+     * A mutable builder for a {@code ThingNotCreatableException} thrown if a Thing could not be created because
+     * the creation of its implicit Policy failed due to a reason passed as {@code reason}.
+     *
+     * @param thingId the ID of the Thing.
+     * @param policyId the ID of the Policy which was used when creating the Thing.
+     * @param reason the reason why the implicit policy creation failed.
+     * @return the builder.
+     */
+    public static Builder newBuilderForOtherReason(final ThingId thingId, final PolicyId policyId, final String reason) {
+        return new Builder(thingId, policyId, reason);
+    }
+
     /**
      * Returns a new instance of {@code ThingNotCreatableException} that is caused by using the unsupported "live"
      * channel for sending a {@code CreateThing} command.
@@ -221,6 +237,11 @@ private Builder(final ThingId thingId, final PolicyId policyId, final boolean po
             }
         }
 
+        private Builder(final ThingId thingId, final PolicyId policyId, final String reason) {
+            this();
+            message(MessageFormat.format(MESSAGE_TEMPLATE_CUSTOM_REASON, String.valueOf(thingId), policyId, reason));
+        }
+
         private Builder httpStatus(final HttpStatus httpStatus) {
             this.httpStatus = httpStatus;
             return this;

things/service/src/main/java/org/eclipse/ditto/things/service/enforcement/ThingEnforcerActor.java

@@ -51,7 +51,6 @@
 import org.eclipse.ditto.policies.model.SubjectId;
 import org.eclipse.ditto.policies.model.signals.commands.PolicyErrorResponse;
 import org.eclipse.ditto.policies.model.signals.commands.exceptions.PolicyConflictException;
-import org.eclipse.ditto.policies.model.signals.commands.exceptions.PolicyNotAccessibleException;
 import org.eclipse.ditto.policies.model.signals.commands.exceptions.PolicyUnavailableException;
 import org.eclipse.ditto.policies.model.signals.commands.modify.CreatePolicy;
 import org.eclipse.ditto.policies.model.signals.commands.modify.CreatePolicyResponse;
@@ -383,19 +382,17 @@ private Policy handleCreatePolicyResponse(final CreatePolicy createPolicy, final
                     .ifPresent(policy -> getContext().getParent().tell(new ThingPolicyCreated(createThing.getEntityId(),
                             createPolicyResponse.getEntityId(), createPolicy.getDittoHeaders()), getSelf()));
             return createPolicyResponse.getPolicyCreated().orElseThrow();
+        } else if (isAskTimeoutException(policyResponse, null)) {
+            throw PolicyUnavailableException.newBuilder(createPolicy.getEntityId())
+                    .dittoHeaders(createThing.getDittoHeaders())
+                    .build();
+        } else if (policyResponse instanceof DittoRuntimeException policyException) {
+            throw reportInitialPolicyCreationFailure(createPolicy.getEntityId(), createThing, policyException);
         } else {
-            if (shouldReportInitialPolicyCreationFailure(policyResponse)) {
-                throw reportInitialPolicyCreationFailure(createPolicy.getEntityId(), createThing);
-            } else if (isAskTimeoutException(policyResponse, null)) {
-                throw PolicyUnavailableException.newBuilder(createPolicy.getEntityId())
-                        .dittoHeaders(createThing.getDittoHeaders())
-                        .build();
-            } else {
-                final var hint = String.format("creating initial policy during creation of Thing <%s>",
-                        createThing.getEntityId());
-                throw AbstractEnforcementReloaded.reportErrorOrResponse(hint, policyResponse, null,
-                        createThing.getDittoHeaders());
-            }
+            final var hint = String.format("creating initial policy during creation of Thing <%s>",
+                    createThing.getEntityId());
+            throw AbstractEnforcementReloaded.reportErrorOrResponse(hint, policyResponse, null,
+                    createThing.getDittoHeaders());
         }
     }
 
@@ -410,22 +407,30 @@ private static boolean isAskTimeoutException(final Object response, @Nullable fi
         return error instanceof AskTimeoutException || response instanceof AskTimeoutException;
     }
 
-    private static boolean shouldReportInitialPolicyCreationFailure(final Object policyResponse) {
-        return policyResponse instanceof PolicyConflictException ||
-                policyResponse instanceof PolicyNotAccessibleException ||
-                policyResponse instanceof NamespaceBlockedException;
-    }
-
     private ThingNotCreatableException reportInitialPolicyCreationFailure(final PolicyId policyId,
-            final CreateThing command) {
+            final CreateThing command, final DittoRuntimeException policyException) {
 
         log.withCorrelationId(command)
-                .info("Failed to create Policy with ID <{}> because it already exists." +
+                .info("Failed to create Policy with ID <{}> due to: <{}: {}>." +
                         " The CreateThing command which would have created a Policy for the Thing with ID <{}>" +
-                        " is therefore not handled.", policyId, command.getEntityId());
-        return ThingNotCreatableException.newBuilderForPolicyExisting(command.getEntityId(), policyId)
-                .dittoHeaders(command.getDittoHeaders())
-                .build();
+                        " is therefore not handled.", policyId,
+                        policyException.getClass().getSimpleName(), policyException.getMessage(),
+                        command.getEntityId()
+                );
+        if (policyException instanceof PolicyConflictException) {
+            return ThingNotCreatableException.newBuilderForPolicyExisting(command.getEntityId(), policyId)
+                    .dittoHeaders(command.getDittoHeaders())
+                    .build();
+        } else if (policyException instanceof NamespaceBlockedException) {
+            return ThingNotCreatableException.newBuilderForPolicyMissing(command.getEntityId(), policyId)
+                    .dittoHeaders(command.getDittoHeaders())
+                    .build();
+        } else {
+            return ThingNotCreatableException.newBuilderForOtherReason(command.getEntityId(), policyId,
+                            policyException.getMessage())
+                    .dittoHeaders(command.getDittoHeaders())
+                    .build();
+        }
     }
 
     @Override