This is an automated email from the ASF dual-hosted git repository. kturner pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/accumulo-access.git
The following commit(s) were added to refs/heads/main by this push: new 535cebb Updates documentation and removes TODOs (#16) 535cebb is described below commit 535cebb6782659df2a21f0cdc0eaffdadb2cef1d Author: Keith Turner <ktur...@apache.org> AuthorDate: Thu Sep 28 21:07:25 2023 -0400 Updates documentation and removes TODOs (#16) --- .../apache/accumulo/access/AccessEvaluator.java | 48 ++++++++++++++++++---- .../accumulo/access/AccessEvaluatorImpl.java | 13 +++--- .../apache/accumulo/access/AccessExpression.java | 2 +- .../accumulo/access/AccessExpressionImpl.java | 2 - .../accumulo/access/CachingAccessEvaluator.java | 1 - 5 files changed, 46 insertions(+), 20 deletions(-) diff --git a/src/main/java/org/apache/accumulo/access/AccessEvaluator.java b/src/main/java/org/apache/accumulo/access/AccessEvaluator.java index edd4b69..3343200 100644 --- a/src/main/java/org/apache/accumulo/access/AccessEvaluator.java +++ b/src/main/java/org/apache/accumulo/access/AccessEvaluator.java @@ -41,21 +41,35 @@ import java.util.Collection; * @since 1.0.0 */ public interface AccessEvaluator { + /** + * @param accessExpression for this parameter a valid access expression is expected. * @return true if the expression is visible using the authorizations supplied at creation, false * otherwise - * @throws IllegalArgumentException when the expression is not valid + * @throws IllegalAccessExpressionException when the expression is not valid */ boolean canAccess(String accessExpression) throws IllegalAccessExpressionException; + /** + * @param accessExpression for this parameter a valid access expression is expected. + * @return true if the expression is visible using the authorizations supplied at creation, false + * otherwise + * @throws IllegalAccessExpressionException when the expression is not valid + */ boolean canAccess(byte[] accessExpression) throws IllegalAccessExpressionException; /** - * TODO documnet that may be more efficient + * @param accessExpression a validated and parsed access expression. The implementation of this + * method may be able to reuse the internal parse tree and avoid re-parsing. + * @return true if the expression is visible using the authorizations supplied at creation, false + * otherwise */ - boolean canAccess(AccessExpression accessExpression) throws IllegalAccessExpressionException; + boolean canAccess(AccessExpression accessExpression); /** + * An interface that is used to check if an authorization seen in an access expression is + * authorized. + * * @since 1.0.0 */ interface Authorizer { @@ -64,7 +78,7 @@ public interface AccessEvaluator { interface AuthorizationsBuilder { - ExecutionBuilder authorizations(Authorizations authorizations); + OptionalBuilder authorizations(Authorizations authorizations); /** * Allows providing multiple sets of authorizations. Each expression will be evaluated @@ -117,15 +131,31 @@ public interface AccessEvaluator { * * */ - ExecutionBuilder authorizations(Collection<Authorizations> authorizations); + OptionalBuilder authorizations(Collection<Authorizations> authorizations); - ExecutionBuilder authorizations(String... authorizations); + /** + * Allows specifying a single set of authorizations. + */ + OptionalBuilder authorizations(String... authorizations); - ExecutionBuilder authorizations(Authorizer authorizer); + /** + * Allows specifying an authorizer that is analogous to a single set of authorization. + */ + OptionalBuilder authorizations(Authorizer authorizer); } - interface ExecutionBuilder extends FinalBuilder { - ExecutionBuilder cacheSize(int cacheSize); + interface OptionalBuilder extends FinalBuilder { + + /** + * When set to a value greater than zero, the result of evaluating expressions will be + * remembered and if the same expression is seen it again the remembered result will be used + * instead of reevaluating it. If this method is not called on the builder then no caching is + * done. When the same expressions can occur repeatedly caching can greatly increase + * performance. + * + * @param cacheSize the number of expressions evaluations to remember in an LRU cache. + */ + OptionalBuilder cacheSize(int cacheSize); } interface FinalBuilder { diff --git a/src/main/java/org/apache/accumulo/access/AccessEvaluatorImpl.java b/src/main/java/org/apache/accumulo/access/AccessEvaluatorImpl.java index f4851de..b815a61 100644 --- a/src/main/java/org/apache/accumulo/access/AccessEvaluatorImpl.java +++ b/src/main/java/org/apache/accumulo/access/AccessEvaluatorImpl.java @@ -152,8 +152,7 @@ class AccessEvaluatorImpl implements AccessEvaluator { return authorizedPredicates.stream().allMatch(accessExpression.aeNode::canAccess); } - private static class BuilderImpl - implements AuthorizationsBuilder, FinalBuilder, ExecutionBuilder { + private static class BuilderImpl implements AuthorizationsBuilder, FinalBuilder, OptionalBuilder { private Authorizer authorizationsChecker; @@ -180,14 +179,14 @@ class AccessEvaluatorImpl implements AccessEvaluator { } @Override - public ExecutionBuilder authorizations(Authorizations authorizations) { + public OptionalBuilder authorizations(Authorizations authorizations) { setAuthorizations(authorizations.asSet().stream().map(auth -> auth.getBytes(UTF_8)) .collect(toUnmodifiableList())); return this; } @Override - public ExecutionBuilder authorizations(Collection<Authorizations> authorizationSets) { + public OptionalBuilder authorizations(Collection<Authorizations> authorizationSets) { setAuthorizations(authorizationSets .stream().map(authorizations -> authorizations.asSet().stream() .map(auth -> auth.getBytes(UTF_8)).collect(toUnmodifiableList())) @@ -196,14 +195,14 @@ class AccessEvaluatorImpl implements AccessEvaluator { } @Override - public ExecutionBuilder authorizations(String... authorizations) { + public OptionalBuilder authorizations(String... authorizations) { setAuthorizations(Stream.of(authorizations).map(auth -> auth.getBytes(UTF_8)) .collect(toUnmodifiableList())); return this; } @Override - public ExecutionBuilder authorizations(Authorizer authorizationChecker) { + public OptionalBuilder authorizations(Authorizer authorizationChecker) { if (authorizationSets != null) { throw new IllegalStateException("Cannot set checker and authorizations"); } @@ -212,7 +211,7 @@ class AccessEvaluatorImpl implements AccessEvaluator { } @Override - public ExecutionBuilder cacheSize(int cacheSize) { + public OptionalBuilder cacheSize(int cacheSize) { if (cacheSize < 0) { throw new IllegalArgumentException(); } diff --git a/src/main/java/org/apache/accumulo/access/AccessExpression.java b/src/main/java/org/apache/accumulo/access/AccessExpression.java index 0c6b51b..6da3bdb 100644 --- a/src/main/java/org/apache/accumulo/access/AccessExpression.java +++ b/src/main/java/org/apache/accumulo/access/AccessExpression.java @@ -88,7 +88,7 @@ public interface AccessExpression { * @param expression is expected to be encoded using UTF-8 */ static AccessExpression of(byte[] expression) throws IllegalAccessExpressionException { - return new AccessExpressionImpl(expression); + return new AccessExpressionImpl(Arrays.copyOf(expression, expression.length)); } /** diff --git a/src/main/java/org/apache/accumulo/access/AccessExpressionImpl.java b/src/main/java/org/apache/accumulo/access/AccessExpressionImpl.java index 514ed30..3285f5a 100644 --- a/src/main/java/org/apache/accumulo/access/AccessExpressionImpl.java +++ b/src/main/java/org/apache/accumulo/access/AccessExpressionImpl.java @@ -47,7 +47,6 @@ class AccessExpressionImpl implements AccessExpression { @Override public String normalize() { - // TODO pass a string builder StringBuilder builder = new StringBuilder(); aeNode.normalize().stringify(builder, false); return builder.toString(); @@ -78,7 +77,6 @@ class AccessExpressionImpl implements AccessExpression { * @see #AccessExpressionImpl(String) */ AccessExpressionImpl(byte[] expression) { - // TODO copy? this.expression = expression; aeNode = Parser.parseAccessExpression(expression); } diff --git a/src/main/java/org/apache/accumulo/access/CachingAccessEvaluator.java b/src/main/java/org/apache/accumulo/access/CachingAccessEvaluator.java index c82fd2c..99f8764 100644 --- a/src/main/java/org/apache/accumulo/access/CachingAccessEvaluator.java +++ b/src/main/java/org/apache/accumulo/access/CachingAccessEvaluator.java @@ -48,7 +48,6 @@ class CachingAccessEvaluator implements AccessEvaluator { @Override public boolean canAccess(byte[] expression) throws IllegalArgumentException { - // TODO avoid converting to string, maybe create separate cache for byte arrays keys return canAccess(new String(expression, UTF_8)); }