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 715613d Avoids looping when evaluating single set of authorizations
(#71)
715613d is described below
commit 715613d9e7601ae38f7f7033131a7b7bb70cbb89
Author: Keith Turner <ktur...@apache.org>
AuthorDate: Mon Jul 8 14:33:27 2024 -0700
Avoids looping when evaluating single set of authorizations (#71)
Simplified the code to avoid looping when there is a single
set of authorizations. This change was made in an attempt to
improve performance and it did very slightly.
Before this change saw the following.
```
Benchmark Mode Cnt Score Error
Units
AccessExpressionBenchmark.measureEvaluation thrpt 12 13.609 ± 0.300
ops/us
```
And after this change saw the following.
```
Benchmark Mode Cnt Score Error
Units
AccessExpressionBenchmark.measureEvaluation thrpt 12 13.806 ± 0.254
ops/us
```
The performance improvement is slight so maybe not worthwhile. However
this change simplified the evaluator code a good bit, so that is
worthwhile.
---
.../apache/accumulo/access/AccessEvaluator.java | 5 +-
.../accumulo/access/AccessEvaluatorImpl.java | 73 +++++-----------------
.../accumulo/access/MultiAccessEvaluatorImpl.java | 56 +++++++++++++++++
3 files changed, 76 insertions(+), 58 deletions(-)
diff --git a/src/main/java/org/apache/accumulo/access/AccessEvaluator.java
b/src/main/java/org/apache/accumulo/access/AccessEvaluator.java
index b3199bc..d80f2e1 100644
--- a/src/main/java/org/apache/accumulo/access/AccessEvaluator.java
+++ b/src/main/java/org/apache/accumulo/access/AccessEvaluator.java
@@ -19,6 +19,7 @@
package org.apache.accumulo.access;
import java.util.Collection;
+import java.util.List;
/**
* This class is used to decide if an entity with a given set of
authorizations can access
@@ -84,7 +85,7 @@ public interface AccessEvaluator {
* @return AccessEvaluator object
*/
static AccessEvaluator of(Authorizations authorizations) {
- return new
AccessEvaluatorImpl(AccessEvaluatorImpl.convert(authorizations));
+ return new AccessEvaluatorImpl(authorizations);
}
/**
@@ -149,7 +150,7 @@ public interface AccessEvaluator {
*
*/
static AccessEvaluator of(Collection<Authorizations> authorizationSets) {
- return new
AccessEvaluatorImpl(AccessEvaluatorImpl.convert(authorizationSets));
+ return new MultiAccessEvaluatorImpl(authorizationSets);
}
/**
diff --git a/src/main/java/org/apache/accumulo/access/AccessEvaluatorImpl.java
b/src/main/java/org/apache/accumulo/access/AccessEvaluatorImpl.java
index 6c5daed..29feb1a 100644
--- a/src/main/java/org/apache/accumulo/access/AccessEvaluatorImpl.java
+++ b/src/main/java/org/apache/accumulo/access/AccessEvaluatorImpl.java
@@ -24,17 +24,13 @@ 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;
//this class is intentionally package private and should never be made public
final class AccessEvaluatorImpl implements AccessEvaluator {
- private final Collection<Predicate<BytesWrapper>> authorizedPredicates;
+ private final Predicate<BytesWrapper> authorizedPredicate;
private static final byte[] EMPTY = new byte[0];
@@ -43,57 +39,28 @@ final class AccessEvaluatorImpl implements AccessEvaluator {
private static final ThreadLocal<Tokenizer> tokenizers =
ThreadLocal.withInitial(() -> new Tokenizer(EMPTY));
- 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(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)));
+ this.authorizedPredicate = auth ->
authorizationChecker.isAuthorized(unescape(auth));
}
/**
* 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");
- }
+ AccessEvaluatorImpl(Authorizations authorizations) {
+ var authsSet = authorizations.asSet();
+ final Set<BytesWrapper> authBytes = new HashSet<>(authsSet.size());
+ for (String authorization : authsSet) {
+ var auth = authorization.getBytes(UTF_8);
+ if (auth.length == 0) {
+ throw new IllegalArgumentException("Empty authorization");
}
+ authBytes.add(new BytesWrapper(AccessEvaluatorImpl.escape(auth, false)));
}
- 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);
+ authorizedPredicate = authBytes::contains;
}
static String unescape(BytesWrapper auth) {
@@ -190,17 +157,11 @@ final class AccessEvaluatorImpl implements
AccessEvaluator {
var bytesWrapper = lookupWrappers.get();
var tokenizer = tokenizers.get();
- for (var auths : authorizedPredicates) {
- tokenizer.reset(accessExpression);
- Predicate<Tokenizer.AuthorizationToken> atp = authToken -> {
- bytesWrapper.set(authToken.data, authToken.start, authToken.len);
- return auths.test(bytesWrapper);
- };
- if (!ParserEvaluator.parseAccessExpression(tokenizer, atp, authToken ->
true)) {
- return false;
- }
- }
- return true;
+ tokenizer.reset(accessExpression);
+ Predicate<Tokenizer.AuthorizationToken> atp = authToken -> {
+ bytesWrapper.set(authToken.data, authToken.start, authToken.len);
+ return authorizedPredicate.test(bytesWrapper);
+ };
+ return ParserEvaluator.parseAccessExpression(tokenizer, atp, authToken ->
true);
}
-
}
diff --git
a/src/main/java/org/apache/accumulo/access/MultiAccessEvaluatorImpl.java
b/src/main/java/org/apache/accumulo/access/MultiAccessEvaluatorImpl.java
new file mode 100644
index 0000000..067310f
--- /dev/null
+++ b/src/main/java/org/apache/accumulo/access/MultiAccessEvaluatorImpl.java
@@ -0,0 +1,56 @@
+/*
+ * 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;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+
+class MultiAccessEvaluatorImpl implements AccessEvaluator {
+ final List<AccessEvaluatorImpl> evaluators;
+
+ MultiAccessEvaluatorImpl(Collection<Authorizations> authorizationSets) {
+ evaluators = new ArrayList<>(authorizationSets.size());
+ for (Authorizations authorizations : authorizationSets) {
+ evaluators.add(new AccessEvaluatorImpl(authorizations));
+ }
+ }
+
+ @Override
+ public boolean canAccess(String accessExpression) throws
InvalidAccessExpressionException {
+ return canAccess(accessExpression.getBytes(UTF_8));
+ }
+
+ @Override
+ public boolean canAccess(byte[] accessExpression) throws
InvalidAccessExpressionException {
+ for (AccessEvaluatorImpl evaluator : evaluators) {
+ if (!evaluator.canAccess(accessExpression)) {
+ return false;
+ }
+ }
+ return true;
+ }
+
+ @Override
+ public boolean canAccess(AccessExpression accessExpression) {
+ return canAccess(accessExpression.getExpression());
+ }
+}
