dlmarion commented on code in PR #102:
URL: https://github.com/apache/accumulo-access/pull/102#discussion_r2911212085
##########
modules/core/src/main/java/org/apache/accumulo/access/AccessExpression.java:
##########
@@ -50,11 +53,7 @@ protected AccessExpression() {}
@Override
public boolean equals(Object o) {
- if (o instanceof AccessExpression) {
- return ((AccessExpression) o).getExpression().equals(getExpression());
- }
-
- return false;
+ return o instanceof AccessExpression a && Objects.equals(getExpression(),
a.getExpression());
Review Comment:
```suggestion
return this == o || o instanceof AccessExpression a &&
Objects.equals(getExpression(), a.getExpression());
```
##########
modules/core/src/main/java/org/apache/accumulo/access/impl/CharsWrapper.java:
##########
@@ -69,22 +69,8 @@ public int hashCode() {
@Override
public boolean equals(Object o) {
- if (o instanceof CharsWrapper) {
- CharsWrapper obs = (CharsWrapper) o;
-
- if (this == o) {
- return true;
- }
-
- if (length() != obs.length()) {
- return false;
- }
-
- return Arrays.equals(wrapped, offset, offset + len, obs.wrapped,
obs.offset,
- obs.offset + obs.len);
- }
-
- return false;
+ return this == o || (o instanceof CharsWrapper obs && length() ==
obs.length() && Arrays
Review Comment:
Do we need to add a null check?
##########
modules/core/src/main/java/org/apache/accumulo/access/AccessExpression.java:
##########
@@ -50,11 +53,7 @@ protected AccessExpression() {}
@Override
public boolean equals(Object o) {
- if (o instanceof AccessExpression) {
- return ((AccessExpression) o).getExpression().equals(getExpression());
- }
-
- return false;
+ return o instanceof AccessExpression a && Objects.equals(getExpression(),
a.getExpression());
Review Comment:
Do we also need to add a null check?
##########
modules/core/src/main/java/org/apache/accumulo/access/impl/AccessExpressionImpl.java:
##########
@@ -77,15 +77,13 @@ public static CharSequence quote(CharSequence term) {
}
public static CharSequence unquote(CharSequence term) {
- if (term.equals("\"\"") || term.isEmpty()) {
- throw new IllegalArgumentException("Empty strings are not legal
authorizations.");
+ if (term.length() >= 2 && term.charAt(0) == '"' &&
term.charAt(term.length() - 1) == '"') {
Review Comment:
I think this allows "" to be processed further, but the original did not.
--
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]
