From 715a7eb19426c02399aa732b6ffab83629039941 Mon Sep 17 00:00:00 2001 From: Ed Hillmann Date: Sat, 8 Feb 2025 22:35:28 +1000 Subject: [PATCH 1/2] Changes, but using ConcurrentHashMap for thread safety Signed-off-by: Ed Hillmann --- CedarJava/build.gradle | 1 - .../model/AuthorizationResponse.java | 12 +++---- .../model/AuthorizationSuccessResponse.java | 10 +++--- .../com/cedarpolicy/model/DetailedError.java | 13 ++++--- .../model/PartialAuthorizationRequest.java | 3 +- .../model/PartialAuthorizationResponse.java | 12 +++---- .../PartialAuthorizationSuccessResponse.java | 34 ++++++++----------- .../cedarpolicy/model/ValidationResponse.java | 25 ++++++-------- .../model/policy/TemplateLink.java | 3 +- .../com/cedarpolicy/value/EntityTypeName.java | 12 +++++-- .../java/com/cedarpolicy/value/EntityUID.java | 13 +++++-- 11 files changed, 71 insertions(+), 67 deletions(-) diff --git a/CedarJava/build.gradle b/CedarJava/build.gradle index 7d6e41a3..ac62cc0f 100644 --- a/CedarJava/build.gradle +++ b/CedarJava/build.gradle @@ -83,7 +83,6 @@ dependencies { implementation 'com.fasterxml.jackson.core:jackson-databind:2.20.0' implementation 'com.fasterxml.jackson.datatype:jackson-datatype-jdk8:2.20.0' implementation 'com.fizzed:jne:4.5.3' - implementation 'com.google.guava:guava:33.5.0-jre' compileOnly 'com.github.spotbugs:spotbugs-annotations:4.9.7' compileOnly 'org.projectlombok:lombok:1.18.42' annotationProcessor 'org.projectlombok:lombok:1.18.42' diff --git a/CedarJava/src/main/java/com/cedarpolicy/model/AuthorizationResponse.java b/CedarJava/src/main/java/com/cedarpolicy/model/AuthorizationResponse.java index 0a64ea0a..abaa35ea 100644 --- a/CedarJava/src/main/java/com/cedarpolicy/model/AuthorizationResponse.java +++ b/CedarJava/src/main/java/com/cedarpolicy/model/AuthorizationResponse.java @@ -18,8 +18,8 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; -import com.google.common.collect.ImmutableList; import java.util.ArrayList; +import java.util.List; import java.util.Optional; /** @@ -34,10 +34,10 @@ public final class AuthorizationResponse { public final Optional success; /** This will be present if and only if `type` is `Failure`. */ @JsonProperty("errors") - public final Optional> errors; + public final Optional> errors; /** Warnings can be produced regardless of whether we have a `Success` or `Failure`. */ @JsonProperty("warnings") - public final ImmutableList warnings; + public final List warnings; /** * If `type` is `Success`, `success` should be present and `errors` empty. @@ -52,11 +52,11 @@ public AuthorizationResponse( ) { this.type = type; this.success = success; - this.errors = errors.map((list) -> ImmutableList.copyOf(list)); + this.errors = errors.map((list) -> List.copyOf(list)); if (warnings == null) { - this.warnings = ImmutableList.of(); // empty + this.warnings = List.of(); // empty } else { - this.warnings = ImmutableList.copyOf(warnings); + this.warnings = List.copyOf(warnings); } } diff --git a/CedarJava/src/main/java/com/cedarpolicy/model/AuthorizationSuccessResponse.java b/CedarJava/src/main/java/com/cedarpolicy/model/AuthorizationSuccessResponse.java index ee1792a9..37cd82fe 100644 --- a/CedarJava/src/main/java/com/cedarpolicy/model/AuthorizationSuccessResponse.java +++ b/CedarJava/src/main/java/com/cedarpolicy/model/AuthorizationSuccessResponse.java @@ -21,8 +21,6 @@ import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.util.List; import java.util.Set; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableSet; /** * Successful authorization response @@ -44,10 +42,10 @@ public static class Diagnostics { * Set of policyID's that caused the decision. For example, when a policy evaluates to Deny, * all forbid policies that evaluated to True will appear in `reason`. */ - private ImmutableSet reason; + private Set reason; /** Set of errors and warnings returned by Cedar. */ - private ImmutableList errors; + private List errors; /** * Read the reasons and errors from a JSON object. @@ -59,8 +57,8 @@ public static class Diagnostics { public Diagnostics( @JsonProperty("reason") Set reason, @JsonProperty("errors") List errors) { - this.errors = ImmutableList.copyOf(errors); - this.reason = ImmutableSet.copyOf(reason); + this.errors = List.copyOf(errors); + this.reason = Set.copyOf(reason); } /** diff --git a/CedarJava/src/main/java/com/cedarpolicy/model/DetailedError.java b/CedarJava/src/main/java/com/cedarpolicy/model/DetailedError.java index 19a0d5fb..7c0191e7 100644 --- a/CedarJava/src/main/java/com/cedarpolicy/model/DetailedError.java +++ b/CedarJava/src/main/java/com/cedarpolicy/model/DetailedError.java @@ -18,7 +18,6 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; -import com.google.common.collect.ImmutableList; import java.util.List; import java.util.Optional; @@ -40,10 +39,10 @@ public class DetailedError { public final Optional severity; /** Source labels (ranges) */ @JsonProperty("sourceLocations") - public final ImmutableList sourceLocations; + public final List sourceLocations; /** Related errors */ @JsonProperty("related") - public final ImmutableList related; + public final List related; @JsonCreator public DetailedError( @@ -61,14 +60,14 @@ public DetailedError( this.url = url; this.severity = severity; if (sourceLocations.isPresent()) { - this.sourceLocations = ImmutableList.copyOf(sourceLocations.get()); + this.sourceLocations = List.copyOf(sourceLocations.get()); } else { - this.sourceLocations = ImmutableList.of(); // empty + this.sourceLocations = List.of(); // empty } if (related.isPresent()) { - this.related = ImmutableList.copyOf(related.get()); + this.related = List.copyOf(related.get()); } else { - this.related = ImmutableList.of(); // empty + this.related = List.of(); // empty } } diff --git a/CedarJava/src/main/java/com/cedarpolicy/model/PartialAuthorizationRequest.java b/CedarJava/src/main/java/com/cedarpolicy/model/PartialAuthorizationRequest.java index 9520592d..338f19f8 100644 --- a/CedarJava/src/main/java/com/cedarpolicy/model/PartialAuthorizationRequest.java +++ b/CedarJava/src/main/java/com/cedarpolicy/model/PartialAuthorizationRequest.java @@ -23,7 +23,6 @@ import com.cedarpolicy.value.EntityUID; import com.cedarpolicy.value.Value; import com.fasterxml.jackson.annotation.JsonInclude; -import com.google.common.collect.ImmutableMap; import java.util.Map; import java.util.HashMap; @@ -156,7 +155,7 @@ public Builder resource(EntityUID resourceEUID) { * @return The builder. */ public Builder context(Map context) { - this.context = Optional.of(ImmutableMap.copyOf(context)); + this.context = Optional.of(Map.copyOf(context)); return this; } diff --git a/CedarJava/src/main/java/com/cedarpolicy/model/PartialAuthorizationResponse.java b/CedarJava/src/main/java/com/cedarpolicy/model/PartialAuthorizationResponse.java index 29e14a94..ec36a373 100644 --- a/CedarJava/src/main/java/com/cedarpolicy/model/PartialAuthorizationResponse.java +++ b/CedarJava/src/main/java/com/cedarpolicy/model/PartialAuthorizationResponse.java @@ -4,9 +4,9 @@ import com.cedarpolicy.ExperimentalFeature; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; -import com.google.common.collect.ImmutableList; import java.util.ArrayList; +import java.util.List; import java.util.Optional; @Experimental(ExperimentalFeature.PARTIAL_EVALUATION) @@ -25,12 +25,12 @@ public class PartialAuthorizationResponse { * This will be present if and only if `type` is `Failure`. */ @JsonProperty("errors") - public final Optional> errors; + public final Optional> errors; /** * Warnings can be produced regardless of whether we have a `Success` or `Failure`. */ @JsonProperty("warnings") - public final ImmutableList warnings; + public final List warnings; /** * If `type` is `Success`, `success` should be present and `errors` empty. @@ -45,11 +45,11 @@ public PartialAuthorizationResponse( ) { this.type = type; this.success = success; - this.errors = errors.map((list) -> ImmutableList.copyOf(list)); + this.errors = errors.map((list) -> List.copyOf(list)); if (warnings == null) { - this.warnings = ImmutableList.of(); // empty + this.warnings = List.of(); // empty } else { - this.warnings = ImmutableList.copyOf(warnings); + this.warnings = List.copyOf(warnings); } } diff --git a/CedarJava/src/main/java/com/cedarpolicy/model/PartialAuthorizationSuccessResponse.java b/CedarJava/src/main/java/com/cedarpolicy/model/PartialAuthorizationSuccessResponse.java index a6592a51..38448835 100644 --- a/CedarJava/src/main/java/com/cedarpolicy/model/PartialAuthorizationSuccessResponse.java +++ b/CedarJava/src/main/java/com/cedarpolicy/model/PartialAuthorizationSuccessResponse.java @@ -24,8 +24,6 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.databind.JsonNode; -import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableSet; /** * Successful partial authorization response @@ -33,31 +31,29 @@ @Experimental(ExperimentalFeature.PARTIAL_EVALUATION) public final class PartialAuthorizationSuccessResponse { private final AuthorizationSuccessResponse.Decision decision; - private final ImmutableSet satisfied; - private final ImmutableSet errored; - private final ImmutableSet mayBeDetermining; - private final ImmutableSet mustBeDetermining; - private final ImmutableMap residuals; - private final ImmutableSet nontrivialResiduals; - private final ImmutableSet warnings; + private final Set satisfied; + private final Set errored; + private final Set mayBeDetermining; + private final Set mustBeDetermining; + private final Map residuals; + private final Set nontrivialResiduals; + private final Set warnings; public PartialAuthorizationSuccessResponse( AuthorizationSuccessResponse.Decision decision, Set satisfied, Set errored, Set mayBeDetermining, Set mustBeDetermining, Map residuals, Set nontrivialResiduals, Set warnings) { this.decision = decision; - // note that ImmutableSet.copyOf() attempts to avoid a full copy when possible - // see https://github.com/google/guava/wiki/ImmutableCollectionsExplained - this.satisfied = ImmutableSet.copyOf(satisfied); - this.errored = ImmutableSet.copyOf(errored); - this.mayBeDetermining = ImmutableSet.copyOf(mayBeDetermining); - this.mustBeDetermining = ImmutableSet.copyOf(mustBeDetermining); - this.residuals = ImmutableMap.copyOf(residuals); - this.nontrivialResiduals = ImmutableSet.copyOf(nontrivialResiduals); + this.satisfied = Set.copyOf(satisfied); + this.errored = Set.copyOf(errored); + this.mayBeDetermining = Set.copyOf(mayBeDetermining); + this.mustBeDetermining = Set.copyOf(mustBeDetermining); + this.residuals = Map.copyOf(residuals); + this.nontrivialResiduals = Set.copyOf(nontrivialResiduals); if (warnings == null) { - this.warnings = ImmutableSet.of(); // empty + this.warnings = Set.of(); // empty } else { - this.warnings = ImmutableSet.copyOf(warnings); + this.warnings = Set.copyOf(warnings); } } diff --git a/CedarJava/src/main/java/com/cedarpolicy/model/ValidationResponse.java b/CedarJava/src/main/java/com/cedarpolicy/model/ValidationResponse.java index 5a55d5f2..87b1023a 100644 --- a/CedarJava/src/main/java/com/cedarpolicy/model/ValidationResponse.java +++ b/CedarJava/src/main/java/com/cedarpolicy/model/ValidationResponse.java @@ -19,7 +19,6 @@ import com.fasterxml.jackson.annotation.JsonAlias; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; -import com.google.common.collect.ImmutableList; import java.util.List; import java.util.Objects; @@ -39,38 +38,36 @@ public final class ValidationResponse { * reported in `success`), but rather higher-level errors, like * a failure to parse or to call the validator. */ - public final Optional> errors; + public final Optional> errors; /** * Other warnings not associated with particular policies. * For instance, warnings about your schema itself. * These warnings can be produced regardless of whether `type` is * `Success` or `Failure`. */ - public final ImmutableList warnings; + public final List warnings; public static final class ValidationSuccessResponse { /** Validation errors associated with particular policies. */ @JsonProperty("validationErrors") - public final ImmutableList validationErrors; + public final List validationErrors; /** Validation warnings associated with particular policies. */ @JsonProperty("validationWarnings") - public final ImmutableList validationWarnings; + public final List validationWarnings; @JsonCreator public ValidationSuccessResponse( @JsonProperty("validationErrors") Optional> validationErrors, @JsonProperty("validationWarnings") Optional> validationWarnings) { - // note that ImmutableSet.copyOf() attempts to avoid a full copy when possible - // see https://github.com/google/guava/wiki/ImmutableCollectionsExplained if (validationErrors.isPresent()) { - this.validationErrors = ImmutableList.copyOf(validationErrors.get()); + this.validationErrors = List.copyOf(validationErrors.get()); } else { - this.validationErrors = ImmutableList.of(); // empty + this.validationErrors = List.of(); // empty } if (validationWarnings.isPresent()) { - this.validationWarnings = ImmutableList.copyOf(validationWarnings.get()); + this.validationWarnings = List.copyOf(validationWarnings.get()); } else { - this.validationWarnings = ImmutableList.of(); // empty + this.validationWarnings = List.of(); // empty } } } @@ -91,16 +88,16 @@ public ValidationResponse( @JsonProperty("errors") Optional> errors, @JsonProperty("warnings") @JsonAlias("otherWarnings") Optional> warnings) { this.type = type; - this.errors = errors.map((list) -> ImmutableList.copyOf(list)); + this.errors = errors.map((list) -> List.copyOf(list)); if (type == SuccessOrFailure.Success) { this.success = Optional.of(new ValidationSuccessResponse(validationErrors, validationWarnings)); } else { this.success = Optional.empty(); } if (warnings.isPresent()) { - this.warnings = ImmutableList.copyOf(warnings.get()); + this.warnings = List.copyOf(warnings.get()); } else { - this.warnings = ImmutableList.of(); // empty + this.warnings = List.of(); // empty } } diff --git a/CedarJava/src/main/java/com/cedarpolicy/model/policy/TemplateLink.java b/CedarJava/src/main/java/com/cedarpolicy/model/policy/TemplateLink.java index 90881391..3cd235bb 100644 --- a/CedarJava/src/main/java/com/cedarpolicy/model/policy/TemplateLink.java +++ b/CedarJava/src/main/java/com/cedarpolicy/model/policy/TemplateLink.java @@ -21,7 +21,6 @@ import java.util.Map; import java.util.stream.Collectors; -import com.google.common.collect.ImmutableList; /** Template-linked policy. */ public class TemplateLink { @@ -42,7 +41,7 @@ public class TemplateLink { public TemplateLink(String templateId, String resultPolicyId, List linkValues) { this.templateId = templateId; this.resultPolicyId = resultPolicyId; - this.linkValues = ImmutableList.copyOf(linkValues); + this.linkValues = List.copyOf(linkValues); } /** Get the template ID. */ diff --git a/CedarJava/src/main/java/com/cedarpolicy/value/EntityTypeName.java b/CedarJava/src/main/java/com/cedarpolicy/value/EntityTypeName.java index 70bb0113..65301be0 100644 --- a/CedarJava/src/main/java/com/cedarpolicy/value/EntityTypeName.java +++ b/CedarJava/src/main/java/com/cedarpolicy/value/EntityTypeName.java @@ -17,10 +17,10 @@ package com.cedarpolicy.value; import com.cedarpolicy.loader.LibraryLoader; -import com.google.common.base.Suppliers; import java.util.List; import java.util.Optional; +import java.util.concurrent.ConcurrentHashMap; import java.util.Objects; import java.util.function.Supplier; import java.util.stream.Collectors; @@ -50,7 +50,15 @@ public final class EntityTypeName { protected EntityTypeName(List namespace, String basename) { this.namespace = namespace; this.basename = basename; - this.entityTypeNameRepr = Suppliers.memoize(() -> getEntityTypeNameRepr(this)); + this.entityTypeNameRepr = new Supplier() { + + private ConcurrentHashMap localMap = new ConcurrentHashMap<>(); + + @Override + public String get() { + return localMap.computeIfAbsent("entityTypeNameRepr", k -> EntityTypeName.getEntityTypeNameRepr(EntityTypeName.this)); + } + }; } /** diff --git a/CedarJava/src/main/java/com/cedarpolicy/value/EntityUID.java b/CedarJava/src/main/java/com/cedarpolicy/value/EntityUID.java index 7ce53b02..16c4f0f0 100644 --- a/CedarJava/src/main/java/com/cedarpolicy/value/EntityUID.java +++ b/CedarJava/src/main/java/com/cedarpolicy/value/EntityUID.java @@ -17,12 +17,12 @@ package com.cedarpolicy.value; import java.util.Optional; +import java.util.concurrent.ConcurrentHashMap; import java.util.Objects; import java.util.function.Supplier; import com.cedarpolicy.loader.LibraryLoader; import com.cedarpolicy.serializer.JsonEUID; -import com.google.common.base.Suppliers; /** * Represents a Cedar Entity UID. An entity UID contains both the entity type and a unique @@ -45,7 +45,16 @@ public final class EntityUID extends Value { public EntityUID(EntityTypeName type, EntityIdentifier id) { this.type = type; this.id = id; - this.euidRepr = Suppliers.memoize(() -> getEUIDRepr(type, id)); + this.euidRepr = new Supplier() { + + private ConcurrentHashMap localMap = new ConcurrentHashMap<>(); + + @Override + public String get() { + return localMap.computeIfAbsent("euidRepr", k -> EntityUID.getEUIDRepr(type, id)); + } + + }; } /** From 776c064b215d2052f13d19060c38271158a1eb84 Mon Sep 17 00:00:00 2001 From: Ed Hillmann Date: Fri, 21 Feb 2025 22:10:51 +1000 Subject: [PATCH 2/2] Fix up after rebase Signed-off-by: Ed Hillmann --- .../java/com/cedarpolicy/model/PartialAuthorizationRequest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CedarJava/src/main/java/com/cedarpolicy/model/PartialAuthorizationRequest.java b/CedarJava/src/main/java/com/cedarpolicy/model/PartialAuthorizationRequest.java index 338f19f8..4a11d978 100644 --- a/CedarJava/src/main/java/com/cedarpolicy/model/PartialAuthorizationRequest.java +++ b/CedarJava/src/main/java/com/cedarpolicy/model/PartialAuthorizationRequest.java @@ -160,7 +160,7 @@ public Builder context(Map context) { } public Builder context(Context context) { - this.context = Optional.of(ImmutableMap.copyOf(context.getContext())); + this.context = Optional.of(Map.copyOf(context.getContext())); return this; }