dlmarion commented on code in PR #3746:
URL: https://github.com/apache/accumulo/pull/3746#discussion_r1473167829
##########
core/pom.xml:
##########
@@ -258,6 +262,9 @@
<allow>javax[.]security[.]auth[.]DestroyFailedException</allow>
<!-- allow questionable Hadoop exceptions for mapreduce -->
<allow>org[.]apache[.]hadoop[.]mapred[.](FileAlreadyExistsException|InvalidJobConfException)</allow>
+ <!-- allow the following types from the visibility API -->
+
<allow>org[.]apache[.]accumulo[.]access[.]AccessExpression</allow>
+
<allow>org[.]apache[.]accumulo[.]access[.]IllegalAccessExpressionException</allow>
Review Comment:
It appears that you might not be throwing this Exception and are throwing
other types instead.
##########
core/src/main/java/org/apache/accumulo/core/security/ColumnVisibility.java:
##########
@@ -598,6 +600,7 @@ public Node getParseTree() {
* @param term term to quote
* @return quoted term (unquoted if unnecessary)
*/
+ // TODO deprecate?
Review Comment:
I think if we were going to make AccessExpression the main way to create a
ColumnVisibility in the future, such that the user had to create one and pass
it to the ColumnVisibility, then I would say yes, deprecate this.
##########
core/src/main/java/org/apache/accumulo/core/security/ColumnVisibility.java:
##########
@@ -496,13 +511,32 @@ public ColumnVisibility(Text expression) {
}
/**
- * Creates a column visibility for a Mutation from a string already encoded
in UTF-8 bytes.
+ * Creates a column visibility for a Mutation from bytes already encoded in
UTF-8.
*
* @param expression visibility expression, encoded as UTF-8 bytes
* @see #ColumnVisibility(String)
*/
public ColumnVisibility(byte[] expression) {
- validate(expression);
+ this.expression = expression;
+ try {
+ AccessExpression.validate(this.expression);
+ } catch (IllegalAccessExpressionException e) {
+ // This is thrown for compatability with the exception this class used
to throw when it parsed
+ // exceptions itself.
+ throw new BadArgumentException(e);
+ }
+ nodeSupplier = Suppliers.memoize(() -> createNodeTree(this.expression));
+ }
+
+ /**
+ * Creates a column visibility for a Mutation from an AccessExpression.
+ *
+ * @param expression visibility expression, encoded as UTF-8 bytes
+ * @see #ColumnVisibility(String)
+ * @since 3.1.0
+ */
+ public ColumnVisibility(AccessExpression expression) {
+ this(expression.getExpression());
Review Comment:
calling `this` will re-validate an already validated AccessExpression.
##########
core/src/main/java/org/apache/accumulo/core/security/ColumnVisibility.java:
##########
@@ -282,13 +295,13 @@ public static void stringify(Node root, byte[]
expression, StringBuilder out) {
*
* @return normalized expression in byte[] form
*/
+ @Deprecated(since = "3.1.0")
public byte[] flatten() {
- Node normRoot = normalize(node, expression);
- StringBuilder builder = new StringBuilder(expression.length);
- stringify(normRoot, expression, builder);
- return builder.toString().getBytes(UTF_8);
+ // expression was not normalized when we created it
Review Comment:
```suggestion
```
--
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]
