dlmarion commented on code in PR #27:
URL: https://github.com/apache/accumulo-access/pull/27#discussion_r1372227374
##########
src/main/java/org/apache/accumulo/access/AccessExpressionImpl.java:
##########
@@ -60,7 +55,7 @@ public Authorizations getAuthorizations() {
}
/**
- * Creates a column visibility for a Mutation.
+ * Creates an AccessExpression for a Mutation.
Review Comment:
```suggestion
* Creates an AccessExpression.
```
I don't know that we need to reference Mutation anywhere.
##########
src/main/java/org/apache/accumulo/access/AccessEvaluatorImpl.java:
##########
@@ -121,38 +124,32 @@ static byte[] escape(byte[] auth, boolean quote) {
}
@Override
- public boolean canAccess(String expression) throws IllegalArgumentException {
-
+ public boolean canAccess(String expression) throws
IllegalAccessExpressionException {
return evaluate(new AccessExpressionImpl(expression));
-
}
@Override
- public boolean canAccess(byte[] expression) throws IllegalArgumentException {
-
+ public boolean canAccess(byte[] expression) throws
IllegalAccessExpressionException {
return evaluate(new AccessExpressionImpl(expression));
-
}
@Override
- public boolean canAccess(AccessExpression expression) throws
IllegalArgumentException {
+ public boolean canAccess(AccessExpression expression) throws
IllegalAccessExpressionException {
if (expression instanceof AccessExpressionImpl) {
-
return evaluate((AccessExpressionImpl) expression);
-
} else {
return canAccess(expression.getExpression());
}
}
public boolean evaluate(AccessExpressionImpl accessExpression)
throws IllegalAccessExpressionException {
- // The VisibilityEvaluator computes a trie from the given Authorizations,
that ColumnVisibility
- // expressions can be evaluated against.
+ // The AccessEvaluator computes a tree from the given Authorizations, that
AccessExpressions can
Review Comment:
`trie` was not a mis-spelling.
##########
src/main/java/org/apache/accumulo/access/AccessExpressionImpl.java:
##########
@@ -34,11 +34,6 @@ class AccessExpressionImpl implements AccessExpression {
@Override
public String getExpression() {
- var expStr = expressionString.get();
- if (expStr != null) {
- return expStr;
- }
-
return expressionString.updateAndGet(es -> es == null ? new
String(expression, UTF_8) : es);
Review Comment:
Won't this re-set `expressionString` every time `getExpression()` is called?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
