From 415f883ee0fd4bd4ed722d6f9605ca26aebbe33e Mon Sep 17 00:00:00 2001 From: Thomas Jaeckle Date: Wed, 4 May 2022 13:57:43 +0200 Subject: [PATCH] stabilize ThingPersistenceOperationsActorIT * added some missing javadocs Signed-off-by: Thomas Jaeckle --- .../AbstractEnforcementReloaded.java | 28 +++++++---- .../enforcement/EnforcementReloaded.java | 46 +++++++++++++------ .../enforcement/ThingCommandEnforcement.java | 5 +- .../ThingPersistenceOperationsActorIT.java | 9 +++- 4 files changed, 60 insertions(+), 28 deletions(-) diff --git a/policies/enforcement/src/main/java/org/eclipse/ditto/policies/enforcement/AbstractEnforcementReloaded.java b/policies/enforcement/src/main/java/org/eclipse/ditto/policies/enforcement/AbstractEnforcementReloaded.java index 12abdc76b0..63074e5a3c 100644 --- a/policies/enforcement/src/main/java/org/eclipse/ditto/policies/enforcement/AbstractEnforcementReloaded.java +++ b/policies/enforcement/src/main/java/org/eclipse/ditto/policies/enforcement/AbstractEnforcementReloaded.java @@ -31,10 +31,12 @@ import akka.pattern.AskTimeoutException; /** - * TODO TJ doc + * Abstract implementation of {@link EnforcementReloaded} providing common functionality of all entity specific + * enforcement implementations. * - * @param - * @param + * @param the type of the Signal to enforce/authorize. + * @param the type of the CommandResponse to filter. + * @since 3.0.0 */ public abstract class AbstractEnforcementReloaded, R extends CommandResponse> implements EnforcementReloaded { @@ -78,7 +80,15 @@ protected DittoRuntimeException reportError(final String hint, @Nullable final T } /** - * Report unexpected error or unknown response. TODO TJ fix javadoc + * Reports an error or a response based on type of the error and whether a response was present or not. + * If the error is of type {@link org.eclipse.ditto.base.model.exceptions.DittoRuntimeException}, it is returned as + * is (without modification), otherwise it is wrapped inside a {@link DittoInternalErrorException}. + * + * @param hint hint about the nature of the error. + * @param response the (optional) response. + * @param error the (optional) error. + * @param dittoHeaders the DittoHeaders to use for the DittoRuntimeException. + * @return DittoRuntimeException suitable for transmission of the error. */ protected DittoRuntimeException reportErrorOrResponse(final String hint, @Nullable final Object response, @Nullable final Throwable error, final DittoHeaders dittoHeaders) { @@ -95,12 +105,12 @@ protected DittoRuntimeException reportErrorOrResponse(final String hint, @Nullab } /** - * Report unknown response. + * Reports an unknown response as a DittoInternalErrorException. * - * @param hint - * @param response - * @param dittoHeaders - * @return TODO TJ + * @param hint hint about the nature of the error. + * @param response the unknown response. + * @param dittoHeaders the DittoHeaders to use for the DittoRuntimeException. + * @return DittoInternalErrorException */ protected DittoRuntimeException reportUnknownResponse(final String hint, final Object response, final DittoHeaders dittoHeaders) { diff --git a/policies/enforcement/src/main/java/org/eclipse/ditto/policies/enforcement/EnforcementReloaded.java b/policies/enforcement/src/main/java/org/eclipse/ditto/policies/enforcement/EnforcementReloaded.java index 8d8f03385f..a00763c4ae 100644 --- a/policies/enforcement/src/main/java/org/eclipse/ditto/policies/enforcement/EnforcementReloaded.java +++ b/policies/enforcement/src/main/java/org/eclipse/ditto/policies/enforcement/EnforcementReloaded.java @@ -22,32 +22,45 @@ import org.eclipse.ditto.policies.model.PolicyId; /** - * TODO TJ doc + * Interface providing enforcement/authorization of {@code Signal}s and filtering of {@code CommandResponse}s with the + * help of a concrete {@link PolicyEnforcer} instance. * - * @param - * @param + * @param the type of the Signal to enforce/authorize. + * @param the type of the CommandResponse to filter. + * @since 3.0.0 */ public interface EnforcementReloaded, R extends CommandResponse> { /** + * Authorizes the passed in {@code signal} using the passed in {@code policyEnforcer}. * - * @param signal - * @param enforcer - * @return + * @param signal the signal to authorize/enforce. + * @param policyEnforcer the PolicyEnforcer to use for authorizing the signal. + * @return a CompletionStage with the authorized Signal or a failed stage with a DittoRuntimeException in case of + * an authorization error. + * @throws org.eclipse.ditto.base.model.exceptions.DittoRuntimeException for any authorization related errors, e.g. + * missing access rights. Those have to be caught an interpreted as a command being "unauthorized". */ - CompletionStage authorizeSignal(S signal, PolicyEnforcer enforcer); + CompletionStage authorizeSignal(S signal, PolicyEnforcer policyEnforcer); /** + * Authorizes the passed in {@code signal} when no {@code policyEnforcer} is present, e.g. may be used for + * "creation" commands. * - * @param signal - * @return + * @param signal the signal to authorize/enforce. + * @return a CompletionStage with the authorized Signal or a failed stage with a DittoRuntimeException in case of + * an authorization error. + * @throws org.eclipse.ditto.base.model.exceptions.DittoRuntimeException for any authorization related errors, e.g. + * missing access rights. Those have to be caught an interpreted as a command being "unauthorized". */ CompletionStage authorizeSignalWithMissingEnforcer(S signal); /** - * TODO TJ doc - * @param commandResponse - * @return + * Checks if for the passed in {@code commandResponse} a filtering should be done at all before trying to filter. + * Some responses shall e.g. never be filtered - or other implementations may not apply response filtering at all. + * + * @param commandResponse the CommandResponse to check if it should be filtered at all. + * @return {@code true} if the passed in {@code commandResponse} should be filtered. */ boolean shouldFilterCommandResponse(R commandResponse); @@ -56,13 +69,16 @@ public interface EnforcementReloaded, R extends CommandRespo * * @param commandResponse the command response that needs to be filtered. * @param enforcer the enforcer that should be used for filtering. - * @return the filtered command response. + * @return a CompletionStage with the filtered command response or a failed stage with a DittoRuntimeException. */ CompletionStage filterResponse(R commandResponse, PolicyEnforcer enforcer); /** - * TODO TJ doc - * @param policyEnforcerLoader + * Registers a "loader" of additional {@link PolicyEnforcer}s by providing a function which can load a + * PolicyEnforcer using the passed in {@link PolicyId}. + * There is only one "loader" registered, so the last registered loader wins. + * + * @param policyEnforcerLoader the PolicyEnforcer loader function to register. */ void registerPolicyEnforcerLoader(Function> policyEnforcerLoader); diff --git a/things/service/src/main/java/org/eclipse/ditto/things/service/enforcement/ThingCommandEnforcement.java b/things/service/src/main/java/org/eclipse/ditto/things/service/enforcement/ThingCommandEnforcement.java index 45773cd6a8..9589c0ddbe 100644 --- a/things/service/src/main/java/org/eclipse/ditto/things/service/enforcement/ThingCommandEnforcement.java +++ b/things/service/src/main/java/org/eclipse/ditto/things/service/enforcement/ThingCommandEnforcement.java @@ -214,7 +214,7 @@ public EnforcementConfig getEnforcementConfig() { @Override public CompletionStage> authorizeSignal(final ThingCommand thingCommand, - final PolicyEnforcer enforcer) { + final PolicyEnforcer policyEnforcer) { final ThingCommand authorizedCommand; if (isWotTdRequestingThingQueryCommand(thingCommand)) { @@ -224,7 +224,8 @@ public CompletionStage> authorizeSignal(final ThingCommand th // for retrieving the WoT TD, assume that full TD gets returned unfiltered: authorizedCommand = prepareThingCommandBeforeSendingToPersistence(thingCommand); } else { - final ThingCommand commandWithReadSubjects = authorizeByPolicyOrThrow(enforcer.getEnforcer(), thingCommand); + final ThingCommand commandWithReadSubjects = authorizeByPolicyOrThrow(policyEnforcer.getEnforcer(), + thingCommand); if (commandWithReadSubjects instanceof ThingQueryCommand thingQueryCommand) { authorizedCommand = ensureTwinChannel(thingQueryCommand); } else if (commandWithReadSubjects.getDittoHeaders().getLiveChannelCondition().isPresent()) { diff --git a/things/service/src/test/java/org/eclipse/ditto/things/service/persistence/actors/ThingPersistenceOperationsActorIT.java b/things/service/src/test/java/org/eclipse/ditto/things/service/persistence/actors/ThingPersistenceOperationsActorIT.java index ead55b264c..600a7edb71 100644 --- a/things/service/src/test/java/org/eclipse/ditto/things/service/persistence/actors/ThingPersistenceOperationsActorIT.java +++ b/things/service/src/test/java/org/eclipse/ditto/things/service/persistence/actors/ThingPersistenceOperationsActorIT.java @@ -14,6 +14,7 @@ import java.util.concurrent.CompletableFuture; +import org.eclipse.ditto.base.model.headers.DittoHeaderDefinition; import org.eclipse.ditto.base.model.headers.DittoHeaders; import org.eclipse.ditto.internal.utils.persistence.mongo.ops.eventsource.MongoEventSourceITAssertions; import org.eclipse.ditto.internal.utils.pubsub.DistributedPub; @@ -70,7 +71,9 @@ protected Object getCreateEntityCommand(final ThingId id) { return CreateThing.of(Thing.newBuilder() .setId(id) .setPolicyId(PolicyId.of(id)) - .build(), null, DittoHeaders.empty()); + .build(), null, DittoHeaders.newBuilder() + .putHeader(DittoHeaderDefinition.DITTO_SUDO.getKey(), "true") // required for a stable test - which does not try to load policies from the policiesShardRegion for enforcement + .build()); } @Override @@ -80,7 +83,9 @@ protected Class getCreateEntityResponseClass() { @Override protected Object getRetrieveEntityCommand(final ThingId id) { - return RetrieveThing.of(id, DittoHeaders.empty()); + return RetrieveThing.of(id, DittoHeaders.newBuilder() + .putHeader(DittoHeaderDefinition.DITTO_SUDO.getKey(), "true") // required for a stable test - which does not try to load policies from the policiesShardRegion for enforcement + .build()); } @Override