From 9ff5e82f6cbb869520dc90b377493e269c2aa528 Mon Sep 17 00:00:00 2001 From: Dave Marion Date: Mon, 22 Dec 2025 17:44:30 +0000 Subject: [PATCH] Modified entry point for AccessEvaluator Removed the 'of' methods from the AccessEvaluator interface. Created an interface for Authorizations and renamed the current class to AuthorizationsImpl. Added the following methods to get an AccessEvaluator: 1. The evaluator() method on an Authorizations instance 2. The Authorizations.evalutor() static method to get an instance for multiple Authorizations objects. 3. The Authorizations.using(Authorizor) static method. Closes #56 --- .../AccessExpressionAntlrEvaluator.java | 2 +- .../antlr/AccessExpressionAntlrBenchmark.java | 2 +- .../access/grammar/antlr/Antlr4Tests.java | 4 +- .../accumulo/access/AccessEvaluator.java | 88 +---------- .../accumulo/access/Authorizations.java | 144 +++++++++++------- .../access/impl/AccessEvaluatorImpl.java | 4 +- .../access/impl/AuthorizationsImpl.java | 84 ++++++++++ .../access/impl/MultiAccessEvaluatorImpl.java | 8 +- .../access/tests/AccessEvaluatorTest.java | 16 +- .../tests/AccessExpressionBenchmark.java | 4 +- .../access/examples/AccessExample.java | 2 +- 11 files changed, 195 insertions(+), 163 deletions(-) create mode 100644 core/src/main/java/org/apache/accumulo/access/impl/AuthorizationsImpl.java diff --git a/antlr4-example/src/main/java/org/apache/accumulo/access/antlr4/AccessExpressionAntlrEvaluator.java b/antlr4-example/src/main/java/org/apache/accumulo/access/antlr4/AccessExpressionAntlrEvaluator.java index 0d1ba19..e4b9d7d 100644 --- a/antlr4-example/src/main/java/org/apache/accumulo/access/antlr4/AccessExpressionAntlrEvaluator.java +++ b/antlr4-example/src/main/java/org/apache/accumulo/access/antlr4/AccessExpressionAntlrEvaluator.java @@ -26,7 +26,6 @@ import org.antlr.v4.runtime.tree.ParseTree; import org.antlr.v4.runtime.tree.TerminalNode; import org.apache.accumulo.access.AccessExpression; -import org.apache.accumulo.access.Authorizations; import org.apache.accumulo.access.InvalidAccessExpressionException; import org.apache.accumulo.access.grammars.AccessExpressionParser.Access_expressionContext; import org.apache.accumulo.access.grammars.AccessExpressionParser.Access_tokenContext; @@ -34,6 +33,7 @@ import org.apache.accumulo.access.grammars.AccessExpressionParser.And_operatorContext; import org.apache.accumulo.access.grammars.AccessExpressionParser.Or_expressionContext; import org.apache.accumulo.access.grammars.AccessExpressionParser.Or_operatorContext; +import org.apache.accumulo.access.Authorizations; public class AccessExpressionAntlrEvaluator { diff --git a/antlr4-example/src/test/java/org/apache/accumulo/access/grammar/antlr/AccessExpressionAntlrBenchmark.java b/antlr4-example/src/test/java/org/apache/accumulo/access/grammar/antlr/AccessExpressionAntlrBenchmark.java index e32593a..05ebf8c 100644 --- a/antlr4-example/src/test/java/org/apache/accumulo/access/grammar/antlr/AccessExpressionAntlrBenchmark.java +++ b/antlr4-example/src/test/java/org/apache/accumulo/access/grammar/antlr/AccessExpressionAntlrBenchmark.java @@ -29,11 +29,11 @@ import java.util.stream.Collectors; import java.util.stream.Stream; -import org.apache.accumulo.access.Authorizations; import org.apache.accumulo.access.antlr.TestDataLoader; import org.apache.accumulo.access.antlr4.AccessExpressionAntlrEvaluator; import org.apache.accumulo.access.antlr4.AccessExpressionAntlrParser; import org.apache.accumulo.access.grammars.AccessExpressionParser.Access_expressionContext; +import org.apache.accumulo.access.Authorizations; import org.openjdk.jmh.annotations.Benchmark; import org.openjdk.jmh.annotations.Mode; import org.openjdk.jmh.annotations.Scope; diff --git a/antlr4-example/src/test/java/org/apache/accumulo/access/grammar/antlr/Antlr4Tests.java b/antlr4-example/src/test/java/org/apache/accumulo/access/grammar/antlr/Antlr4Tests.java index 3ed6a9e..29a0fa5 100644 --- a/antlr4-example/src/test/java/org/apache/accumulo/access/grammar/antlr/Antlr4Tests.java +++ b/antlr4-example/src/test/java/org/apache/accumulo/access/grammar/antlr/Antlr4Tests.java @@ -41,7 +41,6 @@ import org.antlr.v4.runtime.Recognizer; import org.apache.accumulo.access.AccessEvaluator; import org.apache.accumulo.access.AccessExpression; -import org.apache.accumulo.access.Authorizations; import org.apache.accumulo.access.InvalidAccessExpressionException; import org.apache.accumulo.access.antlr.TestDataLoader; import org.apache.accumulo.access.antlr.TestDataLoader.ExpectedResult; @@ -51,6 +50,7 @@ import org.apache.accumulo.access.grammars.AccessExpressionLexer; import org.apache.accumulo.access.grammars.AccessExpressionParser; import org.apache.accumulo.access.grammars.AccessExpressionParser.Access_expressionContext; +import org.apache.accumulo.access.Authorizations; import org.junit.jupiter.api.Test; public class Antlr4Tests { @@ -144,7 +144,7 @@ public void testCompareAntlrEvaluationAgainstAccessEvaluatorImpl() throws Except List authSets = Stream.of(testSet.auths) .map(a -> Authorizations.of(Set.of(a))).collect(Collectors.toList()); - AccessEvaluator evaluator = AccessEvaluator.of(authSets); + AccessEvaluator evaluator = Authorizations.evaluator(authSets); AccessExpressionAntlrEvaluator antlr = new AccessExpressionAntlrEvaluator(authSets); for (TestExpressions test : testSet.tests) { diff --git a/core/src/main/java/org/apache/accumulo/access/AccessEvaluator.java b/core/src/main/java/org/apache/accumulo/access/AccessEvaluator.java index ea0fb2c..308eb81 100644 --- a/core/src/main/java/org/apache/accumulo/access/AccessEvaluator.java +++ b/core/src/main/java/org/apache/accumulo/access/AccessEvaluator.java @@ -18,8 +18,6 @@ */ package org.apache.accumulo.access; -import java.util.Collection; - import org.apache.accumulo.access.impl.AccessEvaluatorImpl; import org.apache.accumulo.access.impl.MultiAccessEvaluatorImpl; @@ -32,7 +30,7 @@ * * 
  * {@code
- * var evaluator = AccessEvaluator.of("ALPHA", "OMEGA");
+ * var evaluator = Authorizations.of(Set.of("ALPHA", "OMEGA")).evaluator();
  *
  * System.out.println(evaluator.canAccess("ALPHA&BETA")); // should print 'false'
  * System.out.println(evaluator.canAccess("(ALPHA|BETA)&(OMEGA|EPSILON)")); // should print 'true'
@@ -80,88 +78,4 @@ public sealed interface AccessEvaluator permits AccessEvaluatorImpl, MultiAccess
    */
   boolean canAccess(AccessExpression accessExpression);
 
-  /**
-   * Creates an AccessEvaluator from an Authorizations object
-   *
-   * @param authorizations auths to use in the AccessEvaluator
-   * @return AccessEvaluator object
-   */
-  static AccessEvaluator of(Authorizations authorizations) {
-    return new AccessEvaluatorImpl(authorizations);
-  }
-
-  /**
-   * Creates an AccessEvaluator from an Authorizer object
-   *
-   * @param authorizer authorizer to use in the AccessEvaluator
-   * @return AccessEvaluator object
-   */
-  static AccessEvaluator of(Authorizer authorizer) {
-    return new AccessEvaluatorImpl(authorizer);
-  }
-
-  /**
-   * Allows providing multiple sets of authorizations. Each expression will be evaluated
-   * independently against each set of authorizations and will only be deemed accessible if
-   * accessible for all. For example the following code would print false, true, and then false.
-   *
-   * 
-   *     {@code
-   * Collection authSets =
-   *     List.of(Authorizations.of("A", "B"), Authorizations.of("C", "D"));
-   * var evaluator = AccessEvaluator.of(authSets);
-   *
-   * System.out.println(evaluator.canAccess("A"));
-   * System.out.println(evaluator.canAccess("A|D"));
-   * System.out.println(evaluator.canAccess("A&D"));
-   *
-   * }
-   * 

-   *
-   * 

-   * The following table shows how each expression in the example above will evaluate for each
-   * authorization set. In order to return true for {@code canAccess()} the expression must evaluate
-   * to true for each authorization set.
-   *
-   * 
-   * 
-   * 
-   * 
-   * 
-   * 
-   * 
-   * 
-   * 
-   * 
-   * 
-   * 
-   * 
-   * 
-   * 
-   * 
-   * 
-   * 
-   * 
-   * 
-   * 
-   * 
-   *
-   * 
Evaluations
[A,B][C,D]
ATrueFalse
A|DTrueTrue
A&DFalseFalse

-   *
-   *
-   *
-   */
-  static AccessEvaluator of(Collection authorizationSets) {
-    return MultiAccessEvaluatorImpl.of(authorizationSets);
-  }
-
-  /**
-   * An interface that is used to check if an authorization seen in an access expression is
-   * authorized.
-   *
-   * @since 1.0.0
-   */
-  interface Authorizer {
-    boolean isAuthorized(String auth);
-  }
 }
diff --git a/core/src/main/java/org/apache/accumulo/access/Authorizations.java b/core/src/main/java/org/apache/accumulo/access/Authorizations.java
index 407b2eb..bd7e984 100644
--- a/core/src/main/java/org/apache/accumulo/access/Authorizations.java
+++ b/core/src/main/java/org/apache/accumulo/access/Authorizations.java
@@ -19,66 +19,31 @@
 package org.apache.accumulo.access;
 
 import java.io.Serializable;
-import java.util.Iterator;
+import java.util.Collection;
 import java.util.Set;
 
-/**
- * An immutable collection of authorization strings.
- *
- * 

- * Instances of this class are thread-safe.
- *
- * 

- * Note: The underlying implementation uses UTF-8 when converting between bytes and Strings.
- *
- * @since 1.0.0
- */
-public final class Authorizations implements Iterable, Serializable {
-
-  private static final long serialVersionUID = 1L;
-
-  private static final Authorizations EMPTY = new Authorizations(Set.of());
+import org.apache.accumulo.access.impl.AccessEvaluatorImpl;
+import org.apache.accumulo.access.impl.AuthorizationsImpl;
+import org.apache.accumulo.access.impl.MultiAccessEvaluatorImpl;
 
-  private final Set authorizations;
-
-  private Authorizations(Set authorizations) {
-    this.authorizations = Set.copyOf(authorizations);
-  }
+public sealed interface Authorizations extends Iterable, Serializable
+    permits AuthorizationsImpl {
 
   /**
-   * Returns the set of authorization strings in this Authorization object
+   * An interface that is used to check if an authorization seen in an access expression is
+   * authorized.
    *
-   * @return immutable set of authorization strings
+   * @since 1.0.0
    */
-  public Set asSet() {
-    return authorizations;
-  }
-
-  @Override
-  public boolean equals(Object o) {
-    if (o instanceof Authorizations) {
-      var oa = (Authorizations) o;
-      return authorizations.equals(oa.authorizations);
-    }
-
-    return false;
-  }
-
-  @Override
-  public int hashCode() {
-    return authorizations.hashCode();
-  }
-
-  @Override
-  public String toString() {
-    return authorizations.toString();
+  interface Authorizer {
+    boolean isAuthorized(String auth);
   }
 
   /**
    * @return a pre-allocated empty Authorizations object
    */
   public static Authorizations of() {
-    return EMPTY;
+    return AuthorizationsImpl.EMPTY;
   }
 
   /**
@@ -89,14 +54,89 @@ public static Authorizations of() {
    */
   public static Authorizations of(Set authorizations) {
     if (authorizations.isEmpty()) {
-      return EMPTY;
+      return AuthorizationsImpl.EMPTY;
     } else {
-      return new Authorizations(authorizations);
+      return new AuthorizationsImpl(authorizations);
     }
   }
 
-  @Override
-  public Iterator iterator() {
-    return authorizations.iterator();
+  /**
+   * Returns the set of authorization strings in this Authorization object
+   *
+   * @return immutable set of authorization strings
+   */
+  public Set asSet();
+
+  /**
+   * Creates an AccessEvaluator from an Authorizations object
+   *
+   * @return AccessEvaluator object
+   */
+  AccessEvaluator evaluator();
+
+  /**
+   * Creates an AccessEvaluator from an Authorizer object
+   *
+   * @param authorizer authorizer to use in the AccessEvaluator
+   * @return AccessEvaluator object
+   */
+  static AccessEvaluator using(Authorizer authorizer) {
+    return new AccessEvaluatorImpl(authorizer);
+  }
+
+  /**
+   * Allows providing multiple sets of authorizations. Each expression will be evaluated
+   * independently against each set of authorizations and will only be deemed accessible if
+   * accessible for all. For example the following code would print false, true, and then false.
+   *
+   * 
+   *     {@code
+   * Collection authSets =
+   *     List.of(Authorizations.of("A", "B"), Authorizations.of("C", "D"));
+   * var evaluator = AccessEvaluator.of(authSets);
+   *
+   * System.out.println(evaluator.canAccess("A"));
+   * System.out.println(evaluator.canAccess("A|D"));
+   * System.out.println(evaluator.canAccess("A&D"));
+   *
+   * }
+   * 

+   *
+   * 

+   * The following table shows how each expression in the example above will evaluate for each
+   * authorization set. In order to return true for {@code canAccess()} the expression must evaluate
+   * to true for each authorization set.
+   *
+   * 
+   * 
+   * 
+   * 
+   * 
+   * 
+   * 
+   * 
+   * 
+   * 
+   * 
+   * 
+   * 
+   * 
+   * 
+   * 
+   * 
+   * 
+   * 
+   * 
+   * 
+   * 
+   *
+   * 
Evaluations
[A,B][C,D]
ATrueFalse
A|DTrueTrue
A&DFalseFalse

+   *
+   *
+   *
+   */
+  static AccessEvaluator evaluator(Collection authorizationSets) {
+    return new MultiAccessEvaluatorImpl(authorizationSets);
   }
+
 }
diff --git a/core/src/main/java/org/apache/accumulo/access/impl/AccessEvaluatorImpl.java b/core/src/main/java/org/apache/accumulo/access/impl/AccessEvaluatorImpl.java
index d5189c2..c81c8e3 100644
--- a/core/src/main/java/org/apache/accumulo/access/impl/AccessEvaluatorImpl.java
+++ b/core/src/main/java/org/apache/accumulo/access/impl/AccessEvaluatorImpl.java
@@ -30,7 +30,7 @@
 
 import org.apache.accumulo.access.AccessEvaluator;
 import org.apache.accumulo.access.AccessExpression;
-import org.apache.accumulo.access.Authorizations;
+import org.apache.accumulo.access.Authorizations.Authorizer;
 import org.apache.accumulo.access.InvalidAccessExpressionException;
 
 public final class AccessEvaluatorImpl implements AccessEvaluator {
@@ -47,7 +47,7 @@ public AccessEvaluatorImpl(Authorizer authorizationChecker) {
   /**
    * Create an AccessEvaluatorImpl using a collection of authorizations
    */
-  public AccessEvaluatorImpl(Authorizations authorizations) {
+  public AccessEvaluatorImpl(AuthorizationsImpl authorizations) {
     var authsSet = authorizations.asSet();
     final Set authBytes = new HashSet<>(authsSet.size());
     for (String authorization : authsSet) {
diff --git a/core/src/main/java/org/apache/accumulo/access/impl/AuthorizationsImpl.java b/core/src/main/java/org/apache/accumulo/access/impl/AuthorizationsImpl.java
new file mode 100644
index 0000000..f6eb32f
--- /dev/null
+++ b/core/src/main/java/org/apache/accumulo/access/impl/AuthorizationsImpl.java
@@ -0,0 +1,84 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.access.impl;
+
+import java.util.Iterator;
+import java.util.Set;
+
+import org.apache.accumulo.access.AccessEvaluator;
+import org.apache.accumulo.access.Authorizations;
+
+/**
+ * An immutable collection of authorization strings.
+ *
+ * 

+ * Instances of this class are thread-safe.
+ *
+ * 

+ * Note: The underlying implementation uses UTF-8 when converting between bytes and Strings.
+ *
+ * @since 1.0.0
+ */
+public final class AuthorizationsImpl implements Authorizations {
+
+  private static final long serialVersionUID = 1L;
+
+  public static final AuthorizationsImpl EMPTY = new AuthorizationsImpl(Set.of());
+
+  private final Set authorizations;
+
+  public AuthorizationsImpl(Set authorizations) {
+    this.authorizations = Set.copyOf(authorizations);
+  }
+
+  public Set asSet() {
+    return authorizations;
+  }
+
+  @Override
+  public boolean equals(Object o) {
+    if (o instanceof AuthorizationsImpl) {
+      var oa = (AuthorizationsImpl) o;
+      return authorizations.equals(oa.authorizations);
+    }
+
+    return false;
+  }
+
+  @Override
+  public int hashCode() {
+    return authorizations.hashCode();
+  }
+
+  @Override
+  public String toString() {
+    return authorizations.toString();
+  }
+
+  @Override
+  public Iterator iterator() {
+    return authorizations.iterator();
+  }
+
+  @Override
+  public AccessEvaluator evaluator() {
+    return new AccessEvaluatorImpl(this);
+  }
+
+}
diff --git a/core/src/main/java/org/apache/accumulo/access/impl/MultiAccessEvaluatorImpl.java b/core/src/main/java/org/apache/accumulo/access/impl/MultiAccessEvaluatorImpl.java
index 8c268f8..666d978 100644
--- a/core/src/main/java/org/apache/accumulo/access/impl/MultiAccessEvaluatorImpl.java
+++ b/core/src/main/java/org/apache/accumulo/access/impl/MultiAccessEvaluatorImpl.java
@@ -31,16 +31,12 @@
 
 public final class MultiAccessEvaluatorImpl implements AccessEvaluator {
 
-  public static AccessEvaluator of(Collection authorizationSets) {
-    return new MultiAccessEvaluatorImpl(authorizationSets);
-  }
-
   private final List evaluators;
 
-  private MultiAccessEvaluatorImpl(Collection authorizationSets) {
+  public MultiAccessEvaluatorImpl(Collection authorizationSets) {
     evaluators = new ArrayList<>(authorizationSets.size());
     for (Authorizations authorizations : authorizationSets) {
-      evaluators.add(new AccessEvaluatorImpl(authorizations));
+      evaluators.add(authorizations.evaluator());
     }
   }
 
diff --git a/core/src/test/java/org/apache/accumulo/access/tests/AccessEvaluatorTest.java b/core/src/test/java/org/apache/accumulo/access/tests/AccessEvaluatorTest.java
index 35a0d5c..4884f88 100644
--- a/core/src/test/java/org/apache/accumulo/access/tests/AccessEvaluatorTest.java
+++ b/core/src/test/java/org/apache/accumulo/access/tests/AccessEvaluatorTest.java
@@ -90,16 +90,16 @@ public void runTestCases() throws IOException {
       AccessEvaluator evaluator;
       assertTrue(testSet.auths.length >= 1);
       if (testSet.auths.length == 1) {
-        evaluator = AccessEvaluator.of(Authorizations.of(Set.of(testSet.auths[0])));
+        evaluator = Authorizations.of(Set.of(testSet.auths[0])).evaluator();
         runTestCases(testSet, evaluator);
 
         Set auths = Stream.of(testSet.auths[0]).collect(Collectors.toSet());
-        evaluator = AccessEvaluator.of(auths::contains);
+        evaluator = Authorizations.using(auths::contains);
         runTestCases(testSet, evaluator);
       } else {
         var authSets = Stream.of(testSet.auths).map(a -> Authorizations.of(Set.of(a)))
             .collect(Collectors.toList());
-        evaluator = AccessEvaluator.of(authSets);
+        evaluator = Authorizations.evaluator(authSets);
         runTestCases(testSet, evaluator);
       }
     }
@@ -176,14 +176,12 @@ private static void runTestCases(TestDataSet testSet, AccessEvaluator evaluator)
 
   @Test
   public void testEmptyAuthorizations() {
+    assertThrows(IllegalArgumentException.class, () -> Authorizations.of(Set.of("")).evaluator());
     assertThrows(IllegalArgumentException.class,
-        () -> AccessEvaluator.of(Authorizations.of(Set.of(""))));
+        () -> Authorizations.of(Set.of("", "A")).evaluator());
     assertThrows(IllegalArgumentException.class,
-        () -> AccessEvaluator.of(Authorizations.of(Set.of("", "A"))));
-    assertThrows(IllegalArgumentException.class,
-        () -> AccessEvaluator.of(Authorizations.of(Set.of("A", ""))));
-    assertThrows(IllegalArgumentException.class,
-        () -> AccessEvaluator.of(Authorizations.of(Set.of(""))));
+        () -> Authorizations.of(Set.of("A", "")).evaluator());
+    assertThrows(IllegalArgumentException.class, () -> Authorizations.of(Set.of("")).evaluator());
   }
 
   @Test
diff --git a/core/src/test/java/org/apache/accumulo/access/tests/AccessExpressionBenchmark.java b/core/src/test/java/org/apache/accumulo/access/tests/AccessExpressionBenchmark.java
index 64de076..11325d9 100644
--- a/core/src/test/java/org/apache/accumulo/access/tests/AccessExpressionBenchmark.java
+++ b/core/src/test/java/org/apache/accumulo/access/tests/AccessExpressionBenchmark.java
@@ -119,11 +119,11 @@ public void loadData() throws IOException {
         et.expressions = new ArrayList<>();
 
         if (testDataSet.auths.length == 1) {
-          et.evaluator = AccessEvaluator.of(Authorizations.of(Set.of(testDataSet.auths[0])));
+          et.evaluator = Authorizations.of(Set.of(testDataSet.auths[0])).evaluator();
         } else {
           var authSets = Stream.of(testDataSet.auths).map(a -> Authorizations.of(Set.of(a)))
               .collect(Collectors.toList());
-          et.evaluator = AccessEvaluator.of(authSets);
+          et.evaluator = Authorizations.evaluator(authSets);
         }
 
         for (var tests : testDataSet.tests) {
diff --git a/examples/src/main/java/org/apache/accumulo/access/examples/AccessExample.java b/examples/src/main/java/org/apache/accumulo/access/examples/AccessExample.java
index d60bcfb..15df590 100644
--- a/examples/src/main/java/org/apache/accumulo/access/examples/AccessExample.java
+++ b/examples/src/main/java/org/apache/accumulo/access/examples/AccessExample.java
@@ -53,7 +53,7 @@ public void run(PrintStream out, String... authorizations) {
         Arrays.toString(authorizations));
 
     // Create an access evaluator using the provided authorizations
-    AccessEvaluator evaluator = AccessEvaluator.of(Authorizations.of(Set.of(authorizations)));
+    AccessEvaluator evaluator = Authorizations.of(Set.of(authorizations)).evaluator();
 
     // Print each record whose access expression permits viewing using the provided authorizations
     getData().forEach((record, accessExpression) -> {