Skip to content

Commit

Permalink
Don't cache policyEnforcer
Browse files Browse the repository at this point in the history
* This is temporary. We should add enforcer caching again.
  It's just done to simplify the logic and make it easier to find problems
  right now.

Signed-off-by: Yannic Klem <Yannic.Klem@bosch.io>
  • Loading branch information
Yannic92 committed Jun 24, 2022
1 parent 90b8539 commit 08871a9
Show file tree
Hide file tree
Showing 12 changed files with 487 additions and 941 deletions.
Expand Up @@ -12,10 +12,6 @@
*/
package org.eclipse.ditto.policies.enforcement;

import java.util.concurrent.CompletionStage;
import java.util.function.Consumer;
import java.util.function.Function;

import javax.annotation.Nullable;

import org.eclipse.ditto.base.model.exceptions.DittoInternalErrorException;
Expand All @@ -25,10 +21,6 @@
import org.eclipse.ditto.base.model.signals.commands.CommandResponse;
import org.eclipse.ditto.internal.utils.akka.logging.DittoLoggerFactory;
import org.eclipse.ditto.internal.utils.akka.logging.ThreadSafeDittoLogger;
import org.eclipse.ditto.policies.model.Policy;
import org.eclipse.ditto.policies.model.PolicyId;

import akka.pattern.AskTimeoutException;

/**
* Abstract implementation of {@link EnforcementReloaded} providing common functionality of all entity specific
Expand All @@ -43,20 +35,6 @@ public abstract class AbstractEnforcementReloaded<S extends Signal<?>, R extends
protected static final ThreadSafeDittoLogger LOGGER =
DittoLoggerFactory.getThreadSafeLogger(AbstractEnforcementReloaded.class);

@Nullable protected Function<PolicyId, CompletionStage<PolicyEnforcer>> policyEnforcerLoader;
@Nullable protected Consumer<Policy> policyInjectionConsumer;

@Override
public void registerPolicyEnforcerLoader(
final Function<PolicyId, CompletionStage<PolicyEnforcer>> policyEnforcerLoader) {
this.policyEnforcerLoader = policyEnforcerLoader;
}

@Override
public void registerPolicyInjectionConsumer(final Consumer<Policy> policyInjectionConsumer) {
this.policyInjectionConsumer = policyInjectionConsumer;
}

/**
* Reports an error differently based on type of the error. If the error is of type
* {@link org.eclipse.ditto.base.model.exceptions.DittoRuntimeException}, it is returned as is
Expand All @@ -67,7 +45,7 @@ public void registerPolicyInjectionConsumer(final Consumer<Policy> policyInjecti
* @param dittoHeaders the DittoHeaders to use for the DittoRuntimeException.
* @return DittoRuntimeException suitable for transmission of the error.
*/
protected static DittoRuntimeException reportError(final String hint, @Nullable final Throwable throwable,
public static DittoRuntimeException reportError(final String hint, @Nullable final Throwable throwable,
final DittoHeaders dittoHeaders) {
final Throwable error = throwable == null
? new NullPointerException("Result and error are both null")
Expand Down Expand Up @@ -120,17 +98,6 @@ protected static DittoRuntimeException reportUnknownResponse(final String hint,
return DittoInternalErrorException.newBuilder().dittoHeaders(dittoHeaders).build();
}

/**
* Check whether response or error from a future is {@code AskTimeoutException}.
*
* @param response response from a future.
* @param error error thrown in a future.
* @return whether either is {@code AskTimeoutException}.
*/
protected static boolean isAskTimeoutException(final Object response, @Nullable final Throwable error) {
return error instanceof AskTimeoutException || response instanceof AskTimeoutException;
}

private static DittoRuntimeException reportUnexpectedError(final String hint, final Throwable error,
final DittoHeaders dittoHeaders) {
LOGGER.withCorrelationId(dittoHeaders)
Expand Down

Large diffs are not rendered by default.

Expand Up @@ -13,13 +13,9 @@
package org.eclipse.ditto.policies.enforcement;

import java.util.concurrent.CompletionStage;
import java.util.function.Consumer;
import java.util.function.Function;

import org.eclipse.ditto.base.model.signals.Signal;
import org.eclipse.ditto.base.model.signals.commands.CommandResponse;
import org.eclipse.ditto.policies.model.Policy;
import org.eclipse.ditto.policies.model.PolicyId;

/**
* Interface providing enforcement/authorization of {@code Signal}s and filtering of {@code CommandResponse}s with the
Expand Down Expand Up @@ -72,23 +68,4 @@ public interface EnforcementReloaded<S extends Signal<?>, R extends CommandRespo
*/
CompletionStage<R> filterResponse(R commandResponse, PolicyEnforcer policyEnforcer);

/**
* 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<PolicyId, CompletionStage<PolicyEnforcer>> policyEnforcerLoader);

/**
* Allows to register consumers which should be notified if this enforcement implementation received a Policy, e.g.
* as response of a {@code CreatePolicy} command issued by this implementation.
* This can optimize the registered consumer as it does not have to load the policy of the policy shard region as
* a consequence.
*
* @param policyInjectionConsumer the consumer to register which shall be notified about an injected Policy.
*/
void registerPolicyInjectionConsumer(Consumer<Policy> policyInjectionConsumer);

}
Expand Up @@ -20,6 +20,7 @@
import org.eclipse.ditto.internal.utils.cache.entry.Entry;
import org.eclipse.ditto.policies.model.Policy;
import org.eclipse.ditto.policies.model.enforcers.Enforcer;
import org.eclipse.ditto.policies.model.enforcers.PolicyEnforcers;

/**
* Policy together with its enforcer.
Expand All @@ -36,24 +37,25 @@ private PolicyEnforcer(@Nullable final Policy policy, final Enforcer enforcer) {
}

/**
* Create a policy together with its enforcer.
* Create a policy enforcer from policy.
*
* @param policy the policy
* @param enforcer the enforcer
* @return the pair
*/
public static PolicyEnforcer of(@Nullable final Policy policy, final Enforcer enforcer) {
return new PolicyEnforcer(policy, enforcer);
public static PolicyEnforcer of(final Policy policy) {
final var enforcer = PolicyEnforcers.defaultEvaluator(policy);
return of(policy, enforcer);
}

/**
* Create a policy enforcer without policy.
* Create a policy together with its enforcer.
*
* @param policy the policy
* @param enforcer the enforcer
* @return the pair
*/
public static PolicyEnforcer of(final Enforcer enforcer) {
return new PolicyEnforcer(null, enforcer);
public static PolicyEnforcer of(final Policy policy, final Enforcer enforcer) {
return new PolicyEnforcer(policy, enforcer);
}

/**
Expand Down
Expand Up @@ -21,7 +21,6 @@
import org.eclipse.ditto.internal.utils.cacheloaders.EnforcementCacheKey;
import org.eclipse.ditto.internal.utils.cacheloaders.config.AskWithRetryConfig;
import org.eclipse.ditto.policies.model.Policy;
import org.eclipse.ditto.policies.model.enforcers.PolicyEnforcers;

import com.github.benmanes.caffeine.cache.AsyncCacheLoader;

Expand Down Expand Up @@ -62,8 +61,7 @@ private static Entry<PolicyEnforcer> evaluatePolicy(final Entry<Policy> entry) {
if (entry.exists()) {
final var revision = entry.getRevision();
final var policy = entry.getValueOrThrow();
final var enforcer = PolicyEnforcers.defaultEvaluator(policy);
return Entry.of(revision, PolicyEnforcer.of(policy, enforcer));
return Entry.of(revision, PolicyEnforcer.of(policy));
} else {
return Entry.nonexistent();
}
Expand Down
Expand Up @@ -36,7 +36,6 @@
import org.eclipse.ditto.policies.model.PolicyEntry;
import org.eclipse.ditto.policies.model.ResourceKey;
import org.eclipse.ditto.policies.model.enforcers.Enforcer;
import org.eclipse.ditto.policies.model.enforcers.PolicyEnforcers;
import org.eclipse.ditto.policies.model.signals.commands.PolicyCommand;
import org.eclipse.ditto.policies.model.signals.commands.PolicyCommandResponse;
import org.eclipse.ditto.policies.model.signals.commands.actions.PolicyActionCommand;
Expand All @@ -46,7 +45,6 @@
import org.eclipse.ditto.policies.model.signals.commands.exceptions.PolicyCommandToModifyExceptionRegistry;
import org.eclipse.ditto.policies.model.signals.commands.exceptions.PolicyNotAccessibleException;
import org.eclipse.ditto.policies.model.signals.commands.modify.CreatePolicy;
import org.eclipse.ditto.policies.model.signals.commands.modify.ModifyPolicy;
import org.eclipse.ditto.policies.model.signals.commands.modify.PolicyModifyCommand;
import org.eclipse.ditto.policies.model.signals.commands.query.PolicyQueryCommandResponse;

Expand Down Expand Up @@ -115,15 +113,9 @@ private PolicyCommand<?> authorizeCreatePolicy(final Enforcer enforcer,

@Override
public CompletionStage<PolicyCommand<?>> authorizeSignalWithMissingEnforcer(final PolicyCommand<?> command) {

if (command instanceof CreatePolicy createPolicy) {
final var enforcer = PolicyEnforcers.defaultEvaluator(createPolicy.getPolicy());
return authorizeSignal(createPolicy, PolicyEnforcer.of(enforcer));
} else {
throw PolicyNotAccessibleException.newBuilder(command.getEntityId())
.dittoHeaders(command.getDittoHeaders())
.build();
}
throw PolicyNotAccessibleException.newBuilder(command.getEntityId())
.dittoHeaders(command.getDittoHeaders())
.build();
}

@Override
Expand Down
Expand Up @@ -22,16 +22,15 @@
import org.eclipse.ditto.base.model.headers.DittoHeaders;
import org.eclipse.ditto.base.model.signals.Signal;
import org.eclipse.ditto.internal.utils.namespaces.BlockedNamespaces;
import org.eclipse.ditto.policies.enforcement.AbstractEnforcerActor;
import org.eclipse.ditto.policies.api.commands.sudo.SudoRetrievePolicy;
import org.eclipse.ditto.policies.api.commands.sudo.SudoRetrievePolicyResponse;
import org.eclipse.ditto.policies.enforcement.AbstractEnforcerActor;
import org.eclipse.ditto.policies.enforcement.PolicyEnforcer;
import org.eclipse.ditto.policies.model.PolicyId;
import org.eclipse.ditto.policies.model.enforcers.PolicyEnforcers;
import org.eclipse.ditto.policies.model.signals.commands.PolicyCommand;
import org.eclipse.ditto.policies.model.signals.commands.PolicyCommandResponse;
import org.eclipse.ditto.policies.model.signals.commands.exceptions.PolicyNotAccessibleException;
import org.eclipse.ditto.policies.model.signals.commands.modify.PolicyModifyCommand;
import org.eclipse.ditto.policies.model.signals.commands.modify.CreatePolicy;
import org.eclipse.ditto.policies.service.enforcement.PolicyCommandEnforcement;

import akka.actor.ActorRef;
Expand Down Expand Up @@ -74,7 +73,7 @@ public static Props props(final PolicyId policyId,
}

@Override
protected CompletionStage<PolicyId> providePolicyIdForEnforcement() {
protected CompletionStage<PolicyId> providePolicyIdForEnforcement(final Signal<?> signal) {
return CompletableFuture.completedStage(entityId);
}

Expand All @@ -93,16 +92,17 @@ protected CompletionStage<PolicyEnforcer> providePolicyEnforcer(@Nullable final
}

@Override
protected boolean shouldInvalidatePolicyEnforcerAfterEnforcement(final Signal<?> signal) {
// this should always be done for modifying commands:
return signal instanceof PolicyModifyCommand<?>;
// TODO CR-11297 optimization: only if the resources/subjects of the policy were changed
protected CompletionStage<Optional<PolicyEnforcer>> loadPolicyEnforcer(final Signal<?> signal) {
if (signal instanceof CreatePolicy createPolicy) {
return CompletableFuture.completedStage(Optional.of(PolicyEnforcer.of(createPolicy.getPolicy())));
}
return super.loadPolicyEnforcer(signal);
}

private static Optional<PolicyEnforcer> handleSudoRetrievePolicyResponse(final Object response) {
if (response instanceof SudoRetrievePolicyResponse sudoRetrievePolicyResponse) {
final var policy = sudoRetrievePolicyResponse.getPolicy();
return Optional.of(PolicyEnforcer.of(policy, PolicyEnforcers.defaultEvaluator(policy)));
return Optional.of(PolicyEnforcer.of(policy));
} else if (response instanceof PolicyNotAccessibleException) {
return Optional.empty();
} else {
Expand Down

0 comments on commit 08871a9

Please sign in to comment.