Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Re-remove dependency on Guava #88

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion CedarJava/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ dependencies {
implementation 'com.fasterxml.jackson.core:jackson-databind:2.16.1'
implementation 'com.fasterxml.jackson.datatype:jackson-datatype-jdk8:2.16.1'
implementation 'org.slf4j:slf4j-api:2.0.12'
implementation 'com.google.guava:guava:33.0.0-jre'
compileOnly 'com.github.spotbugs:spotbugs-annotations:4.8.3'
testImplementation 'org.slf4j:slf4j-simple:2.0.12'
testImplementation 'org.junit.jupiter:junit-jupiter-api:5.10.2'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
* The result of processing an AuthorizationRequest. The answer to the request is contained in the
Expand Down Expand Up @@ -54,10 +52,10 @@ public static class Diagnostics {
* Set of policyID's that caused the decision. For example, when a policy evaluates to Deny,
* all deny policies that evaluated to True will appear in Reasons.
*/
private ImmutableSet<String> reason;
private Set<String> reason;

/** Set of errors and warnings returned by Cedar. */
private ImmutableList<String> errors;
private List<String> errors;

/**
* Read the reasons and errors from a JSON object.
Expand All @@ -69,8 +67,8 @@ public static class Diagnostics {
public Diagnostics(
@JsonProperty("reason") Set<String> reason,
@JsonProperty("errors") List<String> errors) {
this.errors = ImmutableList.copyOf(errors);
this.reason = ImmutableSet.copyOf(reason);
this.errors = List.copyOf(errors);
this.reason = Set.copyOf(reason);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,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.Optional;
Expand Down Expand Up @@ -158,7 +157,7 @@ public Builder resource(Entity resource) {
* @return The builder.
*/
public Builder context(Map<String, Value> context) {
this.context = ImmutableMap.copyOf(context);
this.context = Map.copyOf(context);
return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@
import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.databind.JsonNode;
import com.google.common.collect.ImmutableSet;

import java.util.stream.Collectors;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Set;
Expand Down Expand Up @@ -109,17 +110,17 @@ public boolean reachedDecision() {
}

public static final class ResidualPartialAuthorizationResponse extends PartialAuthorizationResponse {
private final ImmutableSet<Policy> residuals;
private final Set<Policy> residuals;

public ResidualPartialAuthorizationResponse(Map<String, JsonNode> residuals, Diagnostics diagnostics) {
super(diagnostics);
this.residuals = residuals.entrySet().stream()
.map(e -> new Policy(e.getValue().toString(), e.getKey()))
.collect(ImmutableSet.toImmutableSet());
.collect(Collectors.toUnmodifiableSet());
}

public Set<Policy> getResiduals() {
return this.residuals;
return Collections.unmodifiableSet(this.residuals);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;
import java.util.List;
import com.google.common.collect.ImmutableList;

/** Template instantiation. */
public class TemplateInstantiation {
Expand All @@ -46,7 +45,7 @@ public TemplateInstantiation(
@JsonProperty("instantiations") List<Instantiation> instantiations) {
this.templateId = templateId;
this.resultPolicyId = resultPolicyId;
this.instantiations = ImmutableList.copyOf(instantiations);
this.instantiations = List.copyOf(instantiations);
}

/** Get the template ID. */
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@

package com.cedarpolicy.value;

import com.google.common.base.Suppliers;

import java.util.List;
import java.util.Optional;
import java.util.Objects;
Expand Down Expand Up @@ -34,7 +32,15 @@ public final class EntityTypeName {
protected EntityTypeName(List<String> namespace, String basename) {
this.namespace = namespace;
this.basename= basename;
this.entityTypeNameRepr = Suppliers.memoize(() -> getEntityTypeNameRepr(this));
this.entityTypeNameRepr = new Supplier<String>() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If anyone is wondering why this is stalled: The Guava memoizing supplier is thread safe, but this implementation would not be. We don't make an explicit guarantees about thread safety of the Java bindings, but out Rust implementation is thread safe, so I don't want to break that here, and I haven't had the time to do this properly.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for noting the current status @john-h-kastner-aws.

Reviewing the content of this class, is the purpose of using a Supplier to defer to the native JNI call until it is actually needed? Since instances of this class are effectively immutable, it seems like calling getEntityTypeNameRepr() multiple times is not necessarily a problem for the Java toString() representation, unless there is a state concern on the Rust side.

String entityTypeNameRepr = null;
public String get() {
if (entityTypeNameRepr == null) {
entityTypeNameRepr = getEntityTypeNameRepr(EntityTypeName.this);
}
return entityTypeNameRepr;
}
};
}

/**
Expand Down
11 changes: 9 additions & 2 deletions CedarJava/src/main/java/com/cedarpolicy/value/EntityUID.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import java.util.function.Supplier;

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
Expand All @@ -44,7 +43,15 @@ 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<String>() {
String uidRepr = null;
public String get() {
if (uidRepr == null) {
uidRepr = getEUIDRepr(type, id);
}
return uidRepr;
}
};
}

/**
Expand Down
Loading