diff --git a/core/src/main/java/org/apache/accumulo/access/AccessExpression.java b/core/src/main/java/org/apache/accumulo/access/AccessExpression.java index cf32670..e120934 100644 --- a/core/src/main/java/org/apache/accumulo/access/AccessExpression.java +++ b/core/src/main/java/org/apache/accumulo/access/AccessExpression.java @@ -182,6 +182,11 @@ public static ParsedAccessExpression parse(byte[] expression) return ParsedAccessExpressionImpl.parseExpression(Arrays.copyOf(expression, expression.length)); } + /** + * @return true if {@link AccessExpression#parse} has been called on this object false if not. + */ + public abstract boolean isParsed(); + /** * Validates an access expression and returns an immutable object with a parse tree. Creating the * parse tree is expensive relative to calling {@link #of(String)} or {@link #validate(String)}, 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..a711d55 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 @@ -19,12 +19,14 @@ package org.apache.accumulo.access.impl; import static java.nio.charset.StandardCharsets.UTF_8; +import static java.util.Objects.requireNonNull; import static org.apache.accumulo.access.impl.ByteUtils.BACKSLASH; import static org.apache.accumulo.access.impl.ByteUtils.QUOTE; import static org.apache.accumulo.access.impl.ByteUtils.isQuoteOrSlash; import static org.apache.accumulo.access.impl.ByteUtils.isQuoteSymbol; import java.util.HashSet; +import java.util.List; import java.util.Set; import java.util.function.Predicate; @@ -32,6 +34,7 @@ import org.apache.accumulo.access.AccessExpression; import org.apache.accumulo.access.Authorizations; import org.apache.accumulo.access.InvalidAccessExpressionException; +import org.apache.accumulo.access.ParsedAccessExpression; public final class AccessEvaluatorImpl implements AccessEvaluator { @@ -138,7 +141,11 @@ public static byte[] escape(byte[] auth, boolean shouldQuote) { @Override public boolean canAccess(AccessExpression expression) { - return canAccess(expression.getExpression()); + if (expression.isParsed()) { + return canAccess(expression.parse()); + } else { + return canAccess(expression.getExpression()); + } } @Override @@ -159,4 +166,60 @@ boolean evaluate(byte[] accessExpression) throws InvalidAccessExpressionExceptio }; return ParserEvaluator.parseAccessExpression(accessExpression, atp, authToken -> true); } + + private boolean canAccess(ParsedAccessExpression pae) { + switch (pae.getType()) { + case AND: + return canAccessParsedAnd(pae.getChildren()); + case OR: + return canAccessParsedOr(pae.getChildren()); + case AUTHORIZATION: + return canAccessParsedAuthorization(pae.getExpression()); + case EMPTY: + return true; + default: + throw new IllegalArgumentException("Unhandled type: " + pae.getType()); + } + } + + private boolean canAccessParsedAnd(List children) { + requireNonNull(children, "null children list passed to method"); + if (children.isEmpty() || children.size() < 2) { + throw new IllegalArgumentException( + "Malformed AND expression, children: " + children.size() + " -> " + children); + } + boolean result = true; + for (ParsedAccessExpression child : children) { + result &= canAccess(child); + if (result == false) { + return result; + } + } + return result; + } + + private boolean canAccessParsedOr(List children) { + requireNonNull(children, "null children list passed to method"); + if (children.isEmpty()) { + throw new IllegalArgumentException("Malformed OR expression, no children"); + } + for (ParsedAccessExpression child : children) { + if (canAccess(child)) { + return true; + } + } + return false; + } + + private boolean canAccessParsedAuthorization(String token) { + var bytesWrapper = ParserEvaluator.lookupWrappers.get(); + var authToken = token.getBytes(UTF_8); + if (authToken[0] == '"' && authToken[authToken.length - 1] == '"') { + bytesWrapper.set(authToken, 1, authToken.length - 2); + } else { + bytesWrapper.set(authToken, 0, authToken.length); + } + return authorizedPredicate.test(bytesWrapper); + } + } diff --git a/core/src/main/java/org/apache/accumulo/access/impl/AccessExpressionImpl.java b/core/src/main/java/org/apache/accumulo/access/impl/AccessExpressionImpl.java index 9f60e33..fe3adac 100644 --- a/core/src/main/java/org/apache/accumulo/access/impl/AccessExpressionImpl.java +++ b/core/src/main/java/org/apache/accumulo/access/impl/AccessExpressionImpl.java @@ -48,6 +48,10 @@ public String getExpression() { return expression; } + public boolean isParsed() { + return parseTreeRef.get() != null; + } + @Override public ParsedAccessExpression parse() { ParsedAccessExpression parseTree = parseTreeRef.get(); 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..6faab46 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 @@ -61,6 +61,11 @@ public boolean canAccess(byte[] accessExpression) throws InvalidAccessExpression @Override public boolean canAccess(AccessExpression accessExpression) { - return canAccess(accessExpression.getExpression()); + for (AccessEvaluator evaluator : evaluators) { + if (!evaluator.canAccess(accessExpression)) { + return false; + } + } + return true; } } diff --git a/core/src/main/java/org/apache/accumulo/access/impl/ParsedAccessExpressionImpl.java b/core/src/main/java/org/apache/accumulo/access/impl/ParsedAccessExpressionImpl.java index 99f6464..833b444 100644 --- a/core/src/main/java/org/apache/accumulo/access/impl/ParsedAccessExpressionImpl.java +++ b/core/src/main/java/org/apache/accumulo/access/impl/ParsedAccessExpressionImpl.java @@ -174,4 +174,9 @@ private static ParsedAccessExpressionImpl parseExpression(Tokenizer tokenizer, return new ParsedAccessExpressionImpl(auth.data, auth.start, auth.len); } } + + @Override + public boolean isParsed() { + return true; + } } 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 b9706b2..920d521 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 @@ -74,7 +74,13 @@ public static class VisibilityEvaluatorTests { public static class EvaluatorTests { AccessEvaluator evaluator; - List expressions; + List byteExpressions; + + List strExpressions; + + List accessExpressions; + + List parsedExpressions; } @State(Scope.Benchmark) @@ -117,7 +123,10 @@ public void loadData() throws IOException { // Create new EvaluatorTests et = new EvaluatorTests(); - et.expressions = new ArrayList<>(); + et.byteExpressions = new ArrayList<>(); + et.strExpressions = new ArrayList<>(); + et.accessExpressions = new ArrayList<>(); + et.parsedExpressions = new ArrayList<>(); if (testDataSet.auths.length == 1) { et.evaluator = AccessEvaluator.of(Authorizations.of(Set.of(testDataSet.auths[0]))); @@ -133,7 +142,11 @@ public void loadData() throws IOException { allTestExpressionsStr.add(exp); byte[] byteExp = exp.getBytes(UTF_8); allTestExpressions.add(byteExp); - et.expressions.add(byteExp); + et.byteExpressions.add(byteExp); + et.strExpressions.add(exp); + AccessExpression ae = AccessExpression.of(byteExp); + et.accessExpressions.add(ae); + et.parsedExpressions.add(ae.parse()); vet.expressions.add(byteExp); vet.columnVisibilities.add(new ColumnVisibility(byteExp)); } @@ -185,7 +198,6 @@ public void measureStringValidation(BenchmarkState state, Blackhole blackhole) { /** * Measures the time it takes to parse an expression stored in a String and produce a parse tree. - * */ @Benchmark public void measureCreateParseTree(BenchmarkState state, Blackhole blackhole) { @@ -195,12 +207,52 @@ public void measureCreateParseTree(BenchmarkState state, Blackhole blackhole) { } /** - * Measures the time it takes to evaluate an expression. + * Measures the time it takes to parse and evaluate an expression byte[]. This has to create the + * parse tree an operate on it. */ @Benchmark - public void measureParseAndEvaluation(BenchmarkState state, Blackhole blackhole) { + public void measureBytesParseAndEvaluation(BenchmarkState state, Blackhole blackhole) { for (EvaluatorTests evaluatorTests : state.getEvaluatorTests()) { - for (byte[] expression : evaluatorTests.expressions) { + for (byte[] expression : evaluatorTests.byteExpressions) { + blackhole.consume(evaluatorTests.evaluator.canAccess(expression)); + } + } + } + + /** + * Measures the time it takes to parse and evaluate an expression String. This has to create the + * parse tree an operate on it. + */ + @Benchmark + public void measureStringParseAndEvaluation(BenchmarkState state, Blackhole blackhole) { + for (EvaluatorTests evaluatorTests : state.getEvaluatorTests()) { + for (String expression : evaluatorTests.strExpressions) { + blackhole.consume(evaluatorTests.evaluator.canAccess(expression)); + } + } + } + + /** + * Measures the time it takes to parse and evaluate an AccessExpression. This has to create the + * parse tree an operate on it. + */ + @Benchmark + public void measureExpressionEvaluation(BenchmarkState state, Blackhole blackhole) { + for (EvaluatorTests evaluatorTests : state.getEvaluatorTests()) { + for (AccessExpression expression : evaluatorTests.accessExpressions) { + blackhole.consume(evaluatorTests.evaluator.canAccess(expression)); + } + } + } + + /** + * Measures the time it takes to evaluate a ParsedAccessExpression. This evaluates the existing + * parse tree. + */ + @Benchmark + public void measureParsedEvaluation(BenchmarkState state, Blackhole blackhole) { + for (EvaluatorTests evaluatorTests : state.getEvaluatorTests()) { + for (ParsedAccessExpression expression : evaluatorTests.parsedExpressions) { blackhole.consume(evaluatorTests.evaluator.canAccess(expression)); } }