Review: PolicyEnforcerProvider

Signed-off-by: David Schwilk <david.schwilk@bosch.io>
eclipse-ditto · Jul 5, 2022 · ec92d3d · ec92d3d
1 parent 66950ca
commit ec92d3d
Show file tree

Hide file tree

Showing 4 changed files with 8 additions and 5 deletions.
diff --git a/...t/src/main/java/org/eclipse/ditto/policies/enforcement/CachingPolicyEnforcerProvider.java b/...t/src/main/java/org/eclipse/ditto/policies/enforcement/CachingPolicyEnforcerProvider.java
@@ -32,7 +32,6 @@
 import akka.actor.AbstractActor;
 import akka.actor.ActorRef;
 import akka.actor.ActorRefFactory;
-import akka.actor.ActorSystem;
 import akka.actor.Props;
 import akka.cluster.ddata.ORSet;
 import akka.cluster.ddata.Replicator;
@@ -53,6 +52,7 @@ final class CachingPolicyEnforcerProvider implements PolicyEnforcerProvider {
             final PolicyEnforcerProvider delegate,
             @Nullable final BlockedNamespaces blockedNamespaces,
             final ActorRef pubSubMediator) {
+
         this.cachingPolicyEnforcerProviderActor = actorRefFactory.actorOf(
                 CachingPolicyEnforcerProviderActor.props(policyEnforcerCache, delegate, blockedNamespaces,
                         pubSubMediator));
@@ -115,6 +115,7 @@ private static Props props(final Cache<PolicyId, Entry<PolicyEnforcer>> policyEn
                 final PolicyEnforcerProvider delegate,
                 @Nullable final BlockedNamespaces blockedNamespaces,
                 final ActorRef pubSubMediator) {
+
             return Props.create(CachingPolicyEnforcerProviderActor.class,
                     () -> new CachingPolicyEnforcerProviderActor(policyEnforcerCache, delegate, blockedNamespaces,
                             pubSubMediator));

diff --git a/...t/src/main/java/org/eclipse/ditto/policies/enforcement/DefaultPolicyEnforcerProvider.java b/...t/src/main/java/org/eclipse/ditto/policies/enforcement/DefaultPolicyEnforcerProvider.java
@@ -127,7 +127,8 @@ public CompletionStage<Optional<PolicyEnforcer>> getPolicyEnforcer(@Nullable fin
                         .exceptionally(error -> Optional.empty());
             } catch (final Exception e) {
                 LOGGER.warn(
-                        "Got exception when trying to load the policy enforcer via cache loader. This is unexpected"
+                        "Got exception when trying to load the policy enforcer via cache loader. This is " +
+                                "unexpected", e
                 );
                 return CompletableFuture.completedStage(Optional.empty());
             }
@@ -137,7 +138,7 @@ public CompletionStage<Optional<PolicyEnforcer>> getPolicyEnforcer(@Nullable fin
     private PolicyEnforcerProvider withCaching(final ActorSystem actorSystem) {
         final var dispatchers = actorSystem.dispatchers();
         final var cacheConfig = DefaultCacheConfig.of(actorSystem.settings().config(), "ditto.policies-enforcer-cache");
-        final var cacheDispatcher = dispatchers.lookup("enforcement-cache-dispatcher");
+        final var cacheDispatcher = dispatchers.lookup(PolicyEnforcerCacheLoader.ENFORCEMENT_CACHE_DISPATCHER);
         final var policyEnforcerCache =
                 CacheFactory.<PolicyId, Entry<PolicyEnforcer>>createCache(cacheConfig, "policy_enforcer_cache",
                         cacheDispatcher);

diff --git a/...e/src/main/java/org/eclipse/ditto/things/service/enforcement/ThingCommandEnforcement.java b/...e/src/main/java/org/eclipse/ditto/things/service/enforcement/ThingCommandEnforcement.java
@@ -61,7 +61,6 @@
 import org.eclipse.ditto.rql.query.things.FieldNamesPredicateVisitor;
 import org.eclipse.ditto.things.model.Thing;
 import org.eclipse.ditto.things.model.ThingConstants;
-import org.eclipse.ditto.things.model.ThingId;
 import org.eclipse.ditto.things.model.signals.commands.ThingCommand;
 import org.eclipse.ditto.things.model.signals.commands.ThingCommandResponse;
 import org.eclipse.ditto.things.model.signals.commands.exceptions.LiveChannelConditionNotAllowedException;
@@ -70,7 +69,6 @@
 import org.eclipse.ditto.things.model.signals.commands.exceptions.ThingConditionFailedException;
 import org.eclipse.ditto.things.model.signals.commands.exceptions.ThingConditionInvalidException;
 import org.eclipse.ditto.things.model.signals.commands.exceptions.ThingNotAccessibleException;
-import org.eclipse.ditto.things.model.signals.commands.exceptions.ThingNotModifiableException;
 import org.eclipse.ditto.things.model.signals.commands.modify.CreateThing;
 import org.eclipse.ditto.things.model.signals.commands.modify.MergeThing;
 import org.eclipse.ditto.things.model.signals.commands.modify.ThingModifyCommand;
@@ -366,6 +364,7 @@ static <T extends ThingCommand<T>> T authorizeByPolicyOrThrow(final Enforcer enf
     private CompletionStage<Done> handleFailedCreateThing(
             final CreateThing createThing,
             final PolicyEnforcer policyEnforcer) {
+
         if (shouldDeletePolicy(createThing)) {
             return deletePolicy(policyEnforcer.getPolicy().flatMap(Policy::getEntityId).orElseThrow(), createThing);
         }

diff --git a/...ervice/src/main/java/org/eclipse/ditto/things/service/enforcement/ThingEnforcerActor.java b/...ervice/src/main/java/org/eclipse/ditto/things/service/enforcement/ThingEnforcerActor.java
@@ -209,6 +209,7 @@ private CompletionStage<PolicyEnforcer> loadPolicyEnforcerForCreateThing(final C
 
     private CompletionStage<Policy> getCopiedPolicy(final String policyIdOrPlaceholder,
             final DittoHeaders dittoHeaders, final PolicyId policyIdForCopiedPolicy) {
+
         return ReferencePlaceholder.fromCharSequence(policyIdOrPlaceholder)
                 .map(referencePlaceholder -> {
                     log.withCorrelationId(dittoHeaders)
@@ -231,6 +232,7 @@ private CompletionStage<Policy> getCopiedPolicy(final String policyIdOrPlacehold
 
     private CompletionStage<Policy> retrievePolicyWithEnforcement(final DittoHeaders dittoHeaders,
             final PolicyId policyId) {
+
         final var adjustedHeaders = dittoHeaders.toBuilder()
                 .removePreconditionHeaders()
                 .responseRequired(true)