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));
   }
 

Reply via email to