Removed Caching from ExistenceChecker

Signed-off-by: David Schwilk <david.schwilk@bosch.io>
eclipse-ditto · Jun 2, 2022 · 3fa7813 · 3fa7813
1 parent 3b85931
commit 3fa7813
Show file tree

Hide file tree

Showing 11 changed files with 69 additions and 107 deletions.
diff --git a/internal/utils/cache-loaders/pom.xml b/internal/utils/cache-loaders/pom.xml
@@ -33,16 +33,6 @@
             <groupId>org.eclipse.ditto</groupId>
             <artifactId>ditto-internal-utils-cache</artifactId>
         </dependency>
-        <dependency>
-            <groupId>org.eclipse.ditto</groupId>
-            <artifactId>ditto-things-model</artifactId>
-        </dependency>
-
-        <!-- for Sudo commands: -->
-        <dependency>
-            <groupId>org.eclipse.ditto</groupId>
-            <artifactId>ditto-things-api</artifactId>
-        </dependency>
         <dependency>
             <groupId>com.github.ben-manes.caffeine</groupId>
             <artifactId>caffeine</artifactId>

diff --git a/...ders/src/main/java/org/eclipse/ditto/internal/utils/cacheloaders/ActorAskCacheLoader.java b/...ders/src/main/java/org/eclipse/ditto/internal/utils/cacheloaders/ActorAskCacheLoader.java
@@ -90,6 +90,7 @@ public static <V, C extends CacheLookupContext> ActorAskCacheLoader<V, Command<?
             final ActorRef entityRegion,
             final BiFunction<EntityId, C, Command<?>> commandCreator,
             final BiFunction<Object, C, Entry<V>> responseTransformer) {
+
         requireNonNull(askWithRetryConfig);
         requireNonNull(entityType);
         requireNonNull(entityRegion);

diff --git a/...ders/src/main/java/org/eclipse/ditto/internal/utils/cacheloaders/EnforcementCacheKey.java b/...ders/src/main/java/org/eclipse/ditto/internal/utils/cacheloaders/EnforcementCacheKey.java
@@ -67,7 +67,7 @@ public static EnforcementCacheKey of(final EntityId entityId) {
         return new EnforcementCacheKey(entityId, null);
     }
 
-    static EnforcementCacheKey of(final EntityId entityId, final EnforcementContext context) {
+    public static EnforcementCacheKey of(final EntityId entityId, final EnforcementContext context) {
         return new EnforcementCacheKey(entityId, context);
     }
 

diff --git a/...aders/src/main/java/org/eclipse/ditto/internal/utils/cacheloaders/EnforcementContext.java b/...aders/src/main/java/org/eclipse/ditto/internal/utils/cacheloaders/EnforcementContext.java
@@ -40,7 +40,7 @@ private EnforcementContext(@Nullable final PersistenceLifecycle persistenceLifec
      * @param persistenceLifecycle the persistence lifecycle of the looked up entity.
      * @return the created context.
      */
-    static EnforcementContext of(@Nullable final PersistenceLifecycle persistenceLifecycle) {
+    public static EnforcementContext of(@Nullable final PersistenceLifecycle persistenceLifecycle) {
         return new EnforcementContext(persistenceLifecycle);
     }
 

diff --git a/...n/java/org/eclipse/ditto/policies/enforcement/pre_enforcement/PolicyExistenceChecker.java b/...n/java/org/eclipse/ditto/policies/enforcement/pre_enforcement/PolicyExistenceChecker.java
@@ -12,53 +12,48 @@
  */
 package org.eclipse.ditto.policies.enforcement.pre_enforcement;
 
-import java.util.HashMap;
-import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.concurrent.CompletionStage;
 
 import org.eclipse.ditto.base.model.entity.id.EntityId;
 import org.eclipse.ditto.base.model.entity.id.WithEntityId;
-import org.eclipse.ditto.base.model.entity.type.EntityType;
 import org.eclipse.ditto.base.model.signals.Signal;
-import org.eclipse.ditto.internal.utils.cache.Cache;
-import org.eclipse.ditto.internal.utils.cache.CacheFactory;
 import org.eclipse.ditto.internal.utils.cache.entry.Entry;
 import org.eclipse.ditto.internal.utils.cacheloaders.EnforcementCacheKey;
-import org.eclipse.ditto.internal.utils.cacheloaders.PreEnforcementPolicyIdCacheLoader;
 import org.eclipse.ditto.internal.utils.cluster.ShardRegionProxyActorFactory;
 import org.eclipse.ditto.internal.utils.cluster.config.DefaultClusterConfig;
 import org.eclipse.ditto.internal.utils.config.DefaultScopedConfig;
 import org.eclipse.ditto.policies.api.PoliciesMessagingConstants;
 import org.eclipse.ditto.policies.enforcement.config.DefaultEnforcementConfig;
 import org.eclipse.ditto.policies.enforcement.config.EnforcementConfig;
-import org.eclipse.ditto.policies.model.PolicyConstants;
-import org.eclipse.ditto.policies.model.signals.commands.PolicyCommand;
 
 import com.github.benmanes.caffeine.cache.AsyncCacheLoader;
 
 import akka.actor.ActorRef;
 import akka.actor.ActorSystem;
-import akka.dispatch.MessageDispatcher;
 
 /**
  * Checks the existence of the entity from a Policy command.
+ *
  * @since 3.0.0
  */
 public final class PolicyExistenceChecker implements ExistenceChecker {
 
-    private static final String ID_CACHE_METRIC_NAME_PREFIX = "ditto_pre_enforcement_id_cache_";
+    public static final String ENFORCEMENT_CACHE_DISPATCHER = "enforcement-cache-dispatcher";
 
-    private final Map<EntityType, Cache<EnforcementCacheKey, ? extends Entry>> resourceToCacheMap;
+    private final AsyncCacheLoader<EnforcementCacheKey, Entry<EnforcementCacheKey>> policyIdCache;
+    private final ActorSystem actorSystem;
 
     public PolicyExistenceChecker(final ActorSystem actorSystem) {
+        this.actorSystem = actorSystem;
         final var enforcementConfig = DefaultEnforcementConfig.of(
                 DefaultScopedConfig.dittoScoped(actorSystem.settings().config()));
-        resourceToCacheMap = buildResourceToCacheMap(getPolicyIdCache(actorSystem, enforcementConfig));
+        policyIdCache = getPolicyIdCache(actorSystem, enforcementConfig);
     }
 
-    private Cache<EnforcementCacheKey, Entry<EnforcementCacheKey>> getPolicyIdCache(final ActorSystem actorSystem,
+    private AsyncCacheLoader<EnforcementCacheKey, Entry<EnforcementCacheKey>> getPolicyIdCache(
+            final ActorSystem actorSystem,
             final EnforcementConfig enforcementConfig) {
 
         final var clusterConfig = DefaultClusterConfig.of(actorSystem.settings().config().getConfig("ditto.cluster"));
@@ -68,42 +63,33 @@ private Cache<EnforcementCacheKey, Entry<EnforcementCacheKey>> getPolicyIdCache(
         final ActorRef policiesShardRegion = shardRegionProxyActorFactory.getShardRegionProxyActor(
                 PoliciesMessagingConstants.CLUSTER_ROLE, PoliciesMessagingConstants.SHARD_REGION);
 
-        final AsyncCacheLoader<EnforcementCacheKey, Entry<EnforcementCacheKey>> policyIdCacheLoader =
-                new PreEnforcementPolicyIdCacheLoader(enforcementConfig.getAskWithRetryConfig(),
-                        actorSystem.getScheduler(),
-                        policiesShardRegion);
-        final MessageDispatcher enforcementCacheDispatcher =
-                actorSystem.dispatchers().lookup("enforcement-cache-dispatcher");
-
-        return CacheFactory.createCache(policyIdCacheLoader, enforcementConfig.getIdCacheConfig(),
-                ID_CACHE_METRIC_NAME_PREFIX + PolicyCommand.RESOURCE_TYPE, enforcementCacheDispatcher);
-    }
-
-    private static Map<EntityType, Cache<EnforcementCacheKey, ? extends Entry>> buildResourceToCacheMap(
-            final Cache<EnforcementCacheKey, ? extends Entry> policyEnforcerCache) {
-
-        final Map<EntityType, Cache<EnforcementCacheKey, ? extends Entry>> map = new HashMap<>();
-        map.put(PolicyConstants.ENTITY_TYPE, policyEnforcerCache);
-        return map;
+        return new PreEnforcementPolicyIdCacheLoader(enforcementConfig.getAskWithRetryConfig(),
+                actorSystem.getScheduler(),
+                policiesShardRegion);
     }
 
     @Override
     public CompletionStage<Boolean> checkExistence(final Signal<?> signal) {
         final Optional<EntityId> entityIdOptional = WithEntityId.getEntityIdOfType(EntityId.class, signal);
-        final Optional<Cache<EnforcementCacheKey, ? extends Entry>> cacheOptional = entityIdOptional
-                .map(EntityId::getEntityType)
-                .map(resourceToCacheMap::get);
-
-        if (cacheOptional.isEmpty() || entityIdOptional.isEmpty()) {
-            final String message =
-                    String.format("ExistenceChecker: unknown entity type or empty ID <%s:%s> for signal <%s>",
-                            entityIdOptional.map(EntityId::getEntityType).map(Objects::toString).orElse(""),
-                            entityIdOptional.map(Objects::toString).orElse(""), signal.toString());
-            throw new IllegalArgumentException(message);
-        } else {
-            return cacheOptional.get().get(EnforcementCacheKey.of(entityIdOptional.get()))
-                    .thenApply(entryOptional -> entryOptional.map(Entry::exists).orElse(false));
+
+        try {
+            return policyIdCache.asyncLoad(EnforcementCacheKey.of(
+                                    entityIdOptional.orElseThrow(() -> getWrongEntityException(entityIdOptional, signal))),
+                            actorSystem.dispatchers().lookup(ENFORCEMENT_CACHE_DISPATCHER))
+                    .thenApply(Entry::exists);
+        } catch (final Exception e) {
+            throw new IllegalStateException("Could not load policyId via policyIdCacheLoader", e);
         }
     }
 
+    private static IllegalArgumentException getWrongEntityException(final Optional<EntityId> entityIdOptional,
+            final Signal<?> signal) {
+
+        final String message =
+                String.format("ExistenceChecker: unknown entity type or empty ID <%s:%s> for signal <%s>",
+                        entityIdOptional.map(EntityId::getEntityType).map(Objects::toString).orElse(""),
+                        entityIdOptional.map(Objects::toString).orElse(""), signal);
+        return new IllegalArgumentException(message);
+    }
+
 }
diff --git a/...rs/PreEnforcementPolicyIdCacheLoader.java → ...nt/PreEnforcementPolicyIdCacheLoader.java b/...rs/PreEnforcementPolicyIdCacheLoader.java → ...nt/PreEnforcementPolicyIdCacheLoader.java
@@ -10,7 +10,7 @@
  *
  * SPDX-License-Identifier: EPL-2.0
  */
-package org.eclipse.ditto.internal.utils.cacheloaders;
+package org.eclipse.ditto.policies.enforcement.pre_enforcement;
 
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.Executor;
@@ -22,6 +22,9 @@
 import org.eclipse.ditto.base.model.headers.DittoHeaders;
 import org.eclipse.ditto.base.model.signals.commands.Command;
 import org.eclipse.ditto.internal.utils.cache.entry.Entry;
+import org.eclipse.ditto.internal.utils.cacheloaders.ActorAskCacheLoader;
+import org.eclipse.ditto.internal.utils.cacheloaders.EnforcementCacheKey;
+import org.eclipse.ditto.internal.utils.cacheloaders.EnforcementContext;
 import org.eclipse.ditto.internal.utils.cacheloaders.config.AskWithRetryConfig;
 import org.eclipse.ditto.policies.api.commands.sudo.SudoRetrievePolicy;
 import org.eclipse.ditto.policies.api.commands.sudo.SudoRetrievePolicyResponse;

diff --git a/.../org/eclipse/ditto/policies/service/starter/PoliciesServiceGlobalCommandRegistryTest.java b/.../org/eclipse/ditto/policies/service/starter/PoliciesServiceGlobalCommandRegistryTest.java
@@ -27,7 +27,6 @@
 import org.eclipse.ditto.policies.model.signals.commands.actions.ActivateTokenIntegration;
 import org.eclipse.ditto.policies.model.signals.commands.modify.DeleteSubject;
 import org.eclipse.ditto.policies.model.signals.commands.query.RetrieveResource;
-import org.eclipse.ditto.things.api.commands.sudo.SudoRetrieveThing;
 import org.eclipse.ditto.things.model.signals.commands.modify.ModifyFeatureProperty;
 import org.eclipse.ditto.things.model.signals.commands.query.RetrieveFeature;
 
@@ -39,7 +38,6 @@ public PoliciesServiceGlobalCommandRegistryTest() {
                 SudoRetrievePolicy.class,
                 RetrieveFeature.class,          // TODO CR-11383 strictly speaking, the policies service should not must to "know" things-model
                 ModifyFeatureProperty.class,    // TODO CR-11383 strictly speaking, the policies service should not must to "know" things-model
-                SudoRetrieveThing.class,    // TODO CR-11383 strictly speaking, the policies service should not must to "know" things-api
                 ExecutePiggybackCommand.class,
                 SendClaimMessage.class,
                 Shutdown.class,

diff --git a/...ipse/ditto/policies/service/starter/PoliciesServiceGlobalCommandResponseRegistryTest.java b/...ipse/ditto/policies/service/starter/PoliciesServiceGlobalCommandResponseRegistryTest.java
@@ -27,7 +27,6 @@
 import org.eclipse.ditto.policies.model.signals.commands.actions.ActivateTokenIntegrationResponse;
 import org.eclipse.ditto.policies.model.signals.commands.modify.DeleteSubjectResponse;
 import org.eclipse.ditto.policies.model.signals.commands.query.RetrieveResourceResponse;
-import org.eclipse.ditto.things.api.commands.sudo.SudoRetrieveThingResponse;
 import org.eclipse.ditto.things.model.signals.commands.ThingErrorResponse;
 import org.eclipse.ditto.things.model.signals.commands.modify.ModifyFeaturePropertyResponse;
 import org.eclipse.ditto.things.model.signals.commands.query.RetrieveFeatureResponse;
@@ -40,7 +39,6 @@ public PoliciesServiceGlobalCommandResponseRegistryTest() {
                 RetrieveFeatureResponse.class,          // TODO CR-11383 strictly speaking, the policies service should not need to "know" things-model
                 ModifyFeaturePropertyResponse.class,    // TODO CR-11383 strictly speaking, the policies service should not need to "know" things-model
                 ThingErrorResponse.class,               // TODO CR-11383 strictly speaking, the policies service should not need to "know" things-model
-                SudoRetrieveThingResponse.class,
                 SendClaimMessageResponse.class,
                 PurgeNamespaceResponse.class,
                 RetrieveResourceResponse.class,

diff --git a/...va/org/eclipse/ditto/policies/service/starter/PoliciesServiceGlobalEventRegistryTest.java b/...va/org/eclipse/ditto/policies/service/starter/PoliciesServiceGlobalEventRegistryTest.java
@@ -15,15 +15,13 @@
 import org.eclipse.ditto.internal.utils.persistentactors.EmptyEvent;
 import org.eclipse.ditto.internal.utils.test.GlobalEventRegistryTestCases;
 import org.eclipse.ditto.policies.model.signals.events.ResourceDeleted;
-import org.eclipse.ditto.things.api.ThingSnapshotTaken;
 import org.eclipse.ditto.things.model.signals.events.ThingDeleted;
 
 public final class PoliciesServiceGlobalEventRegistryTest extends GlobalEventRegistryTestCases {
 
     public PoliciesServiceGlobalEventRegistryTest() {
         super(
                 ResourceDeleted.class,
-                ThingSnapshotTaken.class,
                 // added due to ditto-model-placeholders
                 ThingDeleted.class,         // TODO CR-11383 strictly speaking, the policies service should not must to "know" things-model
                 EmptyEvent.class

diff --git a/...ers/PreEnforcementThingIdCacheLoader.java → ...ent/PreEnforcementThingIdCacheLoader.java b/...ers/PreEnforcementThingIdCacheLoader.java → ...ent/PreEnforcementThingIdCacheLoader.java
@@ -10,7 +10,7 @@
  *
  * SPDX-License-Identifier: EPL-2.0
  */
-package org.eclipse.ditto.internal.utils.cacheloaders;
+package org.eclipse.ditto.things.service.enforcement;
 
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.Executor;
@@ -22,6 +22,9 @@
 import org.eclipse.ditto.base.model.headers.DittoHeaders;
 import org.eclipse.ditto.base.model.signals.commands.Command;
 import org.eclipse.ditto.internal.utils.cache.entry.Entry;
+import org.eclipse.ditto.internal.utils.cacheloaders.ActorAskCacheLoader;
+import org.eclipse.ditto.internal.utils.cacheloaders.EnforcementCacheKey;
+import org.eclipse.ditto.internal.utils.cacheloaders.EnforcementContext;
 import org.eclipse.ditto.internal.utils.cacheloaders.config.AskWithRetryConfig;
 import org.eclipse.ditto.things.api.commands.sudo.SudoRetrieveThing;
 import org.eclipse.ditto.things.api.commands.sudo.SudoRetrieveThingResponse;