ctubbsii commented on code in PR #16:
URL: https://github.com/apache/accumulo-access/pull/16#discussion_r1340683904
##########
src/main/java/org/apache/accumulo/access/AccessEvaluator.java:
##########
@@ -117,15 +131,31 @@ interface AuthorizationsBuilder {
*
*
*/
- ExecutionBuilder authorizations(Collection<Authorizations> authorizations);
+ OptionalBuilder authorizations(Collection<Authorizations> authorizations);
- ExecutionBuilder authorizations(String... authorizations);
+ /**
+ * Allows specifying a single set of authorizations.
+ */
+ OptionalBuilder authorizations(String... authorizations);
- ExecutionBuilder authorizations(Authorizer authorizer);
+ /**
+ * Allows specifying an authorizer that is analogous to a single set of
authorization.
+ */
+ OptionalBuilder authorizations(Authorizer authorizer);
}
- interface ExecutionBuilder extends FinalBuilder {
- ExecutionBuilder cacheSize(int cacheSize);
+ interface OptionalBuilder extends FinalBuilder {
+
+ /**
+ * When set to a value greater than zero, the result of evaluating
expressions will be
+ * remembered and if the same expression is seen it again the remembered
result will be used
+ * instead of reevaluating it. If this method is not called on the builder
then no caching is
+ * done. When the same expressions can occur repeatedly caching can
greatly increase
+ * performance.
+ *
+ * @param cacheSize the number of expressions evaluations to remember in
an LRU cache.
+ */
+ OptionalBuilder cacheSize(int cacheSize);
Review Comment:
Instead of having a built-in internal cache with only an option for a fixed
size, I wonder if it'd be better to just have a pluggable cache, so users can
provide whatever cache implementation they want, rather than only control the
size.
##########
src/main/java/org/apache/accumulo/access/AccessExpression.java:
##########
@@ -88,7 +88,9 @@ static AccessExpression of(String expression) throws
IllegalAccessExpressionExce
* @param expression is expected to be encoded using UTF-8
*/
static AccessExpression of(byte[] expression) throws
IllegalAccessExpressionException {
- return new AccessExpressionImpl(expression);
+ byte[] copy = new byte[expression.length];
+ System.arraycopy(expression, 0, copy, 0, expression.length);
+ return new AccessExpressionImpl(copy);
Review Comment:
```suggestion
return new AccessExpressionImpl(Arrays.copyOf(expression,
expression.length));
```
--
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]
