This is an automated email from the ASF dual-hosted git repository.
dlmarion 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 42f6f98 Some touch-ups before release. (#34)
42f6f98 is described below
commit 42f6f986d6144bd9e60acfcebd51943362797a34
Author: Dave Marion <dlmar...@apache.org>
AuthorDate: Thu Jan 25 14:08:33 2024 -0500
Some touch-ups before release. (#34)
1. Fixed some javadoc
2. Removed unused code
3. Removed use of streams
4. Removed Builder from AccessEvaluator for consistency
5. Added note in javadoc for API objects about UTF-8 expectations
---
.../src/main/java/gse/AccessExample.java | 2 +-
.../apache/accumulo/access/AccessEvaluator.java | 162 +++++++++++----------
.../accumulo/access/AccessEvaluatorImpl.java | 151 ++++++++-----------
.../apache/accumulo/access/AccessExpression.java | 34 +++--
.../java/org/apache/accumulo/access/AeNode.java | 23 ---
.../org/apache/accumulo/access/Authorizations.java | 2 +
.../accumulo/access/AccessEvaluatorTest.java | 18 +--
.../accumulo/access/AccessExpressionBenchmark.java | 4 +-
8 files changed, 178 insertions(+), 218 deletions(-)
diff --git a/contrib/getting-started/src/main/java/gse/AccessExample.java
b/contrib/getting-started/src/main/java/gse/AccessExample.java
index 8ab8209..9cefd16 100644
--- a/contrib/getting-started/src/main/java/gse/AccessExample.java
+++ b/contrib/getting-started/src/main/java/gse/AccessExample.java
@@ -27,7 +27,7 @@ public class AccessExample {
public static void main(String[] args) {
// Create an access evaluator using the all the arguments passed in on
the command line as authorizations.
- AccessEvaluator evaluator =
AccessEvaluator.builder().authorizations(args).build();
+ AccessEvaluator evaluator = AccessEvaluator.of(args);
// For each record use the access evaluator to determine if it can be
accessed using the authorizations from
// the command line and the access expression associated with each
record.
diff --git a/src/main/java/org/apache/accumulo/access/AccessEvaluator.java
b/src/main/java/org/apache/accumulo/access/AccessEvaluator.java
index 5fe7619..14d498b 100644
--- a/src/main/java/org/apache/accumulo/access/AccessEvaluator.java
+++ b/src/main/java/org/apache/accumulo/access/AccessEvaluator.java
@@ -19,11 +19,12 @@
package org.apache.accumulo.access;
import java.util.Collection;
+import java.util.List;
/**
* <p>
* Used to decide if an entity with one more sets of authorizations can access
zero or more access
- * expression.
+ * expressions.
* </p>
* <p>
* Note: For performance improvements, especially in cases where expressions
are expected to repeat,
@@ -38,13 +39,16 @@ import java.util.Collection;
*
* <pre>
* {@code
- * var evaluator = AccessEvaluator.builder().authorizations("ALPHA",
"OMEGA").build();
+ * var evaluator = AccessEvaluator.of("ALPHA", "OMEGA");
*
* System.out.println(evaluator.canAccess("ALPHA&BETA"));
* System.out.println(evaluator.canAccess("(ALPHA|BETA)&(OMEGA|EPSILON)"));
* }
* </pre>
*
+ * <p>
+ * Note: The underlying implementation uses UTF-8 when converting between
bytes and Strings.
+ *
* @see <a href="https://github.com/apache/accumulo-access";>Accumulo Access
Documentation</a>
* @since 1.0.0
*/
@@ -74,88 +78,94 @@ public interface AccessEvaluator {
boolean canAccess(AccessExpression accessExpression);
/**
- * An interface that is used to check if an authorization seen in an access
expression is
- * authorized.
+ * Creates an AccessEvaluator from an Authorizations object
*
- * @since 1.0.0
+ * @param authorizations auths to use in the AccessEvaluator
+ * @return AccessEvaluator object
*/
- interface Authorizer {
- boolean isAuthorized(String auth);
+ static AccessEvaluator of(Authorizations authorizations) {
+ return new
AccessEvaluatorImpl(AccessEvaluatorImpl.convert(authorizations));
}
- interface AuthorizationsBuilder {
-
- EvaluatorBuilder authorizations(Authorizations authorizations);
-
- /**
- * 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.
- *
- * <pre>
- * {@code
- * Collection<Authorizations> authSets =
- * List.of(Authorizations.of("A", "B"), Authorizations.of("C", "D"));
- * var evaluator =
AccessEvaluator.builder().authorizations(authSets).build();
- *
- * System.out.println(evaluator.canAccess("A"));
- * System.out.println(evaluator.canAccess("A|D"));
- * System.out.println(evaluator.canAccess("A&D"));
- *
- * }
- * </pre>
- *
- * <p>
- * 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.
- *
- * <table>
- * <caption>Evaluations</caption>
- * <tr>
- * <td></td>
- * <td>[A,B]</td>
- * <td>[C,D]</td>
- * </tr>
- * <tr>
- * <td>A</td>
- * <td>True</td>
- * <td>False</td>
- * </tr>
- * <tr>
- * <td>A|D</td>
- * <td>True</td>
- * <td>True</td>
- * </tr>
- * <tr>
- * <td>A&D</td>
- * <td>False</td>
- * <td>False</td>
- * </tr>
- *
- * </table>
- *
- *
- *
- */
- EvaluatorBuilder authorizations(Collection<Authorizations> authorizations);
-
- /**
- * Allows specifying a single set of authorizations.
- */
- EvaluatorBuilder authorizations(String... 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 specifying an authorizer that is analogous to a single set of
authorization.
- */
- EvaluatorBuilder authorizations(Authorizer 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.
+ *
+ * <pre>
+ * {@code
+ * Collection<Authorizations> 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"));
+ *
+ * }
+ * </pre>
+ *
+ * <p>
+ * 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.
+ *
+ * <table>
+ * <caption>Evaluations</caption>
+ * <tr>
+ * <td></td>
+ * <td>[A,B]</td>
+ * <td>[C,D]</td>
+ * </tr>
+ * <tr>
+ * <td>A</td>
+ * <td>True</td>
+ * <td>False</td>
+ * </tr>
+ * <tr>
+ * <td>A|D</td>
+ * <td>True</td>
+ * <td>True</td>
+ * </tr>
+ * <tr>
+ * <td>A&D</td>
+ * <td>False</td>
+ * <td>False</td>
+ * </tr>
+ *
+ * </table>
+ *
+ *
+ *
+ */
+ static AccessEvaluator of(Collection<Authorizations> authorizationSets) {
+ return new
AccessEvaluatorImpl(AccessEvaluatorImpl.convert(authorizationSets));
}
- interface EvaluatorBuilder {
- AccessEvaluator build();
+ /**
+ * Allows specifying a single set of authorizations.
+ */
+ static AccessEvaluator of(String... authorizations) {
+ return new
AccessEvaluatorImpl(AccessEvaluatorImpl.convert(authorizations));
}
- static AuthorizationsBuilder builder() {
- return AccessEvaluatorImpl.builder();
+ /**
+ * 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/src/main/java/org/apache/accumulo/access/AccessEvaluatorImpl.java
b/src/main/java/org/apache/accumulo/access/AccessEvaluatorImpl.java
index 5619701..ef6b0ba 100644
--- a/src/main/java/org/apache/accumulo/access/AccessEvaluatorImpl.java
+++ b/src/main/java/org/apache/accumulo/access/AccessEvaluatorImpl.java
@@ -19,19 +19,18 @@
package org.apache.accumulo.access;
import static java.nio.charset.StandardCharsets.UTF_8;
-import static java.util.stream.Collectors.toSet;
-import static java.util.stream.Collectors.toUnmodifiableList;
import static org.apache.accumulo.access.ByteUtils.BACKSLASH;
import static org.apache.accumulo.access.ByteUtils.QUOTE;
import static org.apache.accumulo.access.ByteUtils.isQuoteOrSlash;
import static org.apache.accumulo.access.ByteUtils.isQuoteSymbol;
+import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
+import java.util.HashSet;
import java.util.List;
+import java.util.Set;
import java.util.function.Predicate;
-import java.util.stream.Collectors;
-import java.util.stream.Stream;
//this class is intentionally package private and should never be made public
class AccessEvaluatorImpl implements AccessEvaluator {
@@ -44,17 +43,65 @@ class AccessEvaluatorImpl implements AccessEvaluator {
private final ThreadLocal<Tokenizer> tokenizers =
ThreadLocal.withInitial(() -> new Tokenizer(EMPTY));
- private AccessEvaluatorImpl(Authorizer authorizationChecker) {
+ static Collection<List<byte[]>> convert(Collection<Authorizations>
authorizationSets) {
+ final List<List<byte[]>> authorizationLists = new
ArrayList<>(authorizationSets.size());
+ for (final Authorizations authorizations : authorizationSets) {
+ final Set<String> authSet = authorizations.asSet();
+ final List<byte[]> authList = new ArrayList<>(authSet.size());
+ for (final String auth : authSet) {
+ authList.add(auth.getBytes(UTF_8));
+ }
+ authorizationLists.add(authList);
+ }
+ return authorizationLists;
+ }
+
+ static Collection<List<byte[]>> convert(String... authorizations) {
+ final List<byte[]> authList = new ArrayList<>(authorizations.length);
+ for (final String auth : authorizations) {
+ authList.add(auth.getBytes(UTF_8));
+ }
+ return Collections.singletonList(authList);
+ }
+
+ static Collection<List<byte[]>> convert(Authorizations authorizations) {
+ final Set<String> authSet = authorizations.asSet();
+ final List<byte[]> authList = new ArrayList<>(authSet.size());
+ for (final String auth : authSet) {
+ authList.add(auth.getBytes(UTF_8));
+ }
+ return Collections.singletonList(authList);
+ }
+
+ /**
+ * Create an AccessEvaluatorImpl using an Authorizer object
+ */
+ AccessEvaluatorImpl(Authorizer authorizationChecker) {
this.authorizedPredicates = List.of(auth ->
authorizationChecker.isAuthorized(unescape(auth)));
}
- public AccessEvaluatorImpl(Collection<List<byte[]>> authorizationSets) {
- authorizedPredicates = authorizationSets.stream()
- .map(authorizations -> authorizations.stream()
- .map(auth -> AccessEvaluatorImpl.escape(auth,
false)).map(BytesWrapper::new)
- .collect(toSet()))
- .map(escapedAuths -> (Predicate<BytesWrapper>) escapedAuths::contains)
- .collect(Collectors.toUnmodifiableList());
+ /**
+ * Create an AccessEvaluatorImpl using a collection of authorizations
+ */
+ AccessEvaluatorImpl(final Collection<List<byte[]>> authorizationSets) {
+
+ for (final List<byte[]> auths : authorizationSets) {
+ for (final byte[] auth : auths) {
+ if (auth.length == 0) {
+ throw new IllegalArgumentException("Empty authorization");
+ }
+ }
+ }
+
+ final List<Predicate<BytesWrapper>> predicates = new
ArrayList<>(authorizationSets.size());
+ for (final List<byte[]> authorizationList : authorizationSets) {
+ final Set<BytesWrapper> authBytes = new
HashSet<>(authorizationList.size());
+ for (final byte[] authorization : authorizationList) {
+ authBytes.add(new
BytesWrapper(AccessEvaluatorImpl.escape(authorization, false)));
+ }
+ predicates.add((auth) -> authBytes.contains(auth));
+ }
+ authorizedPredicates = List.copyOf(predicates);
}
static String unescape(BytesWrapper auth) {
@@ -164,84 +211,4 @@ class AccessEvaluatorImpl implements AccessEvaluator {
return true;
}
- private static class BuilderImpl implements AuthorizationsBuilder,
EvaluatorBuilder {
-
- private Authorizer authorizationsChecker;
-
- private Collection<List<byte[]>> authorizationSets;
-
- private void setAuthorizations(List<byte[]> auths) {
- setAuthorizations(Collections.singletonList(auths));
- }
-
- private void setAuthorizations(Collection<List<byte[]>> authSets) {
- if (authorizationsChecker != null) {
- throw new IllegalStateException("Cannot set checker and
authorizations");
- }
-
- for (List<byte[]> auths : authSets) {
- for (byte[] auth : auths) {
- if (auth.length == 0) {
- throw new IllegalArgumentException("Empty authorization");
- }
- }
- }
- this.authorizationSets = authSets;
- }
-
- @Override
- public EvaluatorBuilder authorizations(Authorizations authorizations) {
- setAuthorizations(authorizations.asSet().stream().map(auth ->
auth.getBytes(UTF_8))
- .collect(toUnmodifiableList()));
- return this;
- }
-
- @Override
- public EvaluatorBuilder authorizations(Collection<Authorizations>
authorizationSets) {
- setAuthorizations(authorizationSets
- .stream().map(authorizations -> authorizations.asSet().stream()
- .map(auth -> auth.getBytes(UTF_8)).collect(toUnmodifiableList()))
- .collect(Collectors.toList()));
- return this;
- }
-
- @Override
- public EvaluatorBuilder authorizations(String... authorizations) {
- setAuthorizations(Stream.of(authorizations).map(auth ->
auth.getBytes(UTF_8))
- .collect(toUnmodifiableList()));
- return this;
- }
-
- @Override
- public EvaluatorBuilder authorizations(Authorizer authorizationChecker) {
- if (authorizationSets != null) {
- throw new IllegalStateException("Cannot set checker and
authorizations");
- }
- this.authorizationsChecker = authorizationChecker;
- return this;
- }
-
- @Override
- public AccessEvaluator build() {
- if (authorizationSets != null ^ authorizationsChecker == null) {
- throw new IllegalStateException(
- "Exactly one of authorizationSets or authorizationsChecker must be
set, not both or none.");
- }
-
- AccessEvaluator accessEvaluator;
- if (authorizationsChecker != null) {
- accessEvaluator = new AccessEvaluatorImpl(authorizationsChecker);
- } else {
- accessEvaluator = new AccessEvaluatorImpl(authorizationSets);
- }
-
- return accessEvaluator;
- }
-
- }
-
- public static AuthorizationsBuilder builder() {
- return new BuilderImpl();
- }
-
}
diff --git a/src/main/java/org/apache/accumulo/access/AccessExpression.java
b/src/main/java/org/apache/accumulo/access/AccessExpression.java
index c2fc000..f2dd07e 100644
--- a/src/main/java/org/apache/accumulo/access/AccessExpression.java
+++ b/src/main/java/org/apache/accumulo/access/AccessExpression.java
@@ -25,22 +25,27 @@ package org.apache.accumulo.access;
* passing this type over a String is that its known to be a valid expression.
*
* <p>
- * >Below is an example of using this API.
+ * Below is an example of how to use this API.
*
* <pre>
- * {@code
- * // The following authorization does not need quoting, so the return value
is the same as the
- * // input.
+ * {@code
+ * // The following authorization does not need quoting
+ * // so the return value is the same as the input.
* var auth1 = AccessExpression.quote("CAT");
+ *
* // The following two authorizations need quoting and the return values will
be quoted.
* var auth2 = AccessExpression.quote("🦕");
* var auth3 = AccessExpression.quote("🦖");
+ *
+ * // Create an AccessExpression using auth1, auth2, and auth3
* var exp = "(" + auth1 + "&" + auth3 + ")|(" + auth1 + "&" + auth2 + "&" +
auth1 + ")";
+ *
* // Validate the expression, but do not normalize it
- * var visExp = AccessExpression.of(exp);
- * System.out.println(visExp.getExpression());
+ * System.out.println(AccessExpression.of(exp).getExpression());
+ *
* // Validate and normalize the expression.
* System.out.println(AccessExpression.of(exp, true).getExpression());
+ *
* // Print the unique authorization in the expression
* System.out.println(visExp.getAuthorizations());
* }
@@ -58,11 +63,14 @@ package org.apache.accumulo.access;
* is not valid.
*
* <pre>
- * {@code
+ * {@code
* AccessExpression.validate("A&B|C");
* }
* </pre>
*
+ * <p>
+ * Note: The underlying implementation uses UTF-8 when converting between
bytes and Strings.
+ *
* @see <a href="https://github.com/apache/accumulo-access";>Accumulo Access
Documentation</a>
* @since 1.0.0
*/
@@ -92,17 +100,17 @@ public interface AccessExpression {
*
* <p>
* When the {@code normalize} parameter is true, then will deduplicate,
sort, flatten, and remove
- * unneeded parens or quotes in the expressions. Normalization is done in
addition to validation.
- * The following list gives examples of what each normalization step does.
+ * unneeded parentheses or quotes in the expressions. Normalization is done
in addition to
+ * validation. The following list gives examples of what each normalization
step does.
*
* <ul>
* <li>As an example of flattening, the expression {@code A&(B&C)} flattens
to {@code A&B&C}.</li>
* <li>As an example of sorting, the expression {@code (Z&Y)|(C&B)} sorts to
* {@code (B&C)|(Y&Z)}</li>
* <li>As an example of deduplication, the expression {@code X&Y&X}
normalizes to {@code X&Y}</li>
- * <li>As an example of unneed quotes, the expression {@code "ABC"&"XYZ"}
normalizes to
+ * <li>As an example of unneeded quotes, the expression {@code "ABC"&"XYZ"}
normalizes to
* {@code ABC&XYZ}</li>
- * <li>As an example of unneed parens, the expression {@code
(((ABC)|(XYZ)))} normalizes to *
+ * <li>As an example of unneeded parentheses, the expression {@code
(((ABC)|(XYZ)))} normalizes to
* {@code ABC|XYZ}</li>
* </ul>
*
@@ -133,8 +141,8 @@ public interface AccessExpression {
*
* <p>
* If only validation is needed, then call {@link #validate(byte[])} because
it will avoid copying
- * the expression like this method does. This method must copy the byte
array into a String
- * inorder to create an immutable AccessExpression.
+ * the expression like this method does. This method must copy the byte
array into a String in
+ * order to create an immutable AccessExpression.
*
* @see #of(String, boolean) for information about normlization.
* @param expression an access expression that is expected to be encoded
using UTF-8
diff --git a/src/main/java/org/apache/accumulo/access/AeNode.java
b/src/main/java/org/apache/accumulo/access/AeNode.java
index b9eb446..2a8d05a 100644
--- a/src/main/java/org/apache/accumulo/access/AeNode.java
+++ b/src/main/java/org/apache/accumulo/access/AeNode.java
@@ -51,29 +51,6 @@ abstract class AeNode implements Comparable<AeNode> {
return ordinal() - o.ordinal();
}
- private static class EmptyNode extends AeNode {
-
- @Override
- void stringify(StringBuilder builder, boolean addParens) {
-
- }
-
- @Override
- AeNode normalize() {
- return this;
- }
-
- @Override
- public int compareTo(AeNode o) {
- return super.compareTo(o);
- }
-
- @Override
- int ordinal() {
- return 0;
- }
- }
-
private static class AuthNode extends AeNode {
private final BytesWrapper authInExpression;
diff --git a/src/main/java/org/apache/accumulo/access/Authorizations.java
b/src/main/java/org/apache/accumulo/access/Authorizations.java
index 3d4df1b..a0dc1ef 100644
--- a/src/main/java/org/apache/accumulo/access/Authorizations.java
+++ b/src/main/java/org/apache/accumulo/access/Authorizations.java
@@ -24,6 +24,8 @@ import java.util.Set;
/**
* An immutable collection of authorization strings.
*
+ * Note: The underlying implementation uses UTF-8 when converting between
bytes and Strings.
+ *
* @since 1.0.0
*/
public class Authorizations {
diff --git a/src/test/java/org/apache/accumulo/access/AccessEvaluatorTest.java
b/src/test/java/org/apache/accumulo/access/AccessEvaluatorTest.java
index a041b8a..e7d025f 100644
--- a/src/test/java/org/apache/accumulo/access/AccessEvaluatorTest.java
+++ b/src/test/java/org/apache/accumulo/access/AccessEvaluatorTest.java
@@ -87,16 +87,16 @@ public class AccessEvaluatorTest {
AccessEvaluator evaluator;
assertTrue(testSet.auths.length >= 1);
if (testSet.auths.length == 1) {
- evaluator =
AccessEvaluator.builder().authorizations(testSet.auths[0]).build();
+ evaluator = AccessEvaluator.of(testSet.auths[0]);
runTestCases(testSet, evaluator);
Set<String> auths =
Stream.of(testSet.auths[0]).collect(Collectors.toSet());
- evaluator =
AccessEvaluator.builder().authorizations(auths::contains).build();
+ evaluator = AccessEvaluator.of(auths::contains);
runTestCases(testSet, evaluator);
} else {
var authSets =
Stream.of(testSet.auths).map(Authorizations::of).collect(Collectors.toList());
- evaluator = AccessEvaluator.builder().authorizations(authSets).build();
+ evaluator = AccessEvaluator.of(authSets);
runTestCases(testSet, evaluator);
}
}
@@ -187,14 +187,10 @@ public class AccessEvaluatorTest {
@Test
public void testEmptyAuthorizations() {
- assertThrows(IllegalArgumentException.class,
- () -> AccessEvaluator.builder().authorizations("").build());
- assertThrows(IllegalArgumentException.class,
- () -> AccessEvaluator.builder().authorizations("", "A").build());
- assertThrows(IllegalArgumentException.class,
- () -> AccessEvaluator.builder().authorizations("A", "").build());
- assertThrows(IllegalArgumentException.class,
- () ->
AccessEvaluator.builder().authorizations(Authorizations.of("")).build());
+ assertThrows(IllegalArgumentException.class, () -> AccessEvaluator.of(""));
+ assertThrows(IllegalArgumentException.class, () -> AccessEvaluator.of("",
"A"));
+ assertThrows(IllegalArgumentException.class, () -> AccessEvaluator.of("A",
""));
+ assertThrows(IllegalArgumentException.class, () ->
AccessEvaluator.of(Authorizations.of("")));
}
@Test
diff --git
a/src/test/java/org/apache/accumulo/access/AccessExpressionBenchmark.java
b/src/test/java/org/apache/accumulo/access/AccessExpressionBenchmark.java
index af9a14b..3d7ffa6 100644
--- a/src/test/java/org/apache/accumulo/access/AccessExpressionBenchmark.java
+++ b/src/test/java/org/apache/accumulo/access/AccessExpressionBenchmark.java
@@ -85,11 +85,11 @@ public class AccessExpressionBenchmark {
et.expressions = new ArrayList<>();
if (testDataSet.auths.length == 1) {
- et.evaluator =
AccessEvaluator.builder().authorizations(testDataSet.auths[0]).build();
+ et.evaluator = AccessEvaluator.of(testDataSet.auths[0]);
} else {
var authSets =
Stream.of(testDataSet.auths).map(Authorizations::of).collect(Collectors.toList());
- et.evaluator =
AccessEvaluator.builder().authorizations(authSets).build();
+ et.evaluator = AccessEvaluator.of(authSets);
}
for (var tests : testDataSet.tests) {
