You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by "dlmarion (via GitHub)" <gi...@apache.org> on 2026/03/10 18:38:45 UTC
[PR] Reduce method calls [accumulo-access]
dlmarion opened a new pull request, #106:
URL: https://github.com/apache/accumulo-access/pull/106
Reduce calls to term.length() and short-circuit when term is known to be length of 2 and contain '""'.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reduce method calls [accumulo-access]
Posted by "ctubbsii (via GitHub)" <gi...@apache.org>.
ctubbsii commented on code in PR #106:
URL: https://github.com/apache/accumulo-access/pull/106#discussion_r2913785814
##########
modules/core/src/main/java/org/apache/accumulo/access/impl/AccessExpressionImpl.java:
##########
@@ -77,7 +77,11 @@ public static CharSequence quote(CharSequence term) {
}
public static CharSequence unquote(CharSequence term) {
- if (term.length() >= 2 && term.charAt(0) == '"' && term.charAt(term.length() - 1) == '"') {
+ int len = term.length();
+ if (len >= 2 && term.charAt(0) == '"' && term.charAt(len - 1) == '"') {
+ if (len == 2) {
+ throw new IllegalArgumentException("Empty strings are not legal authorizations.");
+ }
term = AccessEvaluatorImpl.unescape(term.subSequence(1, term.length() - 1));
Review Comment:
@keith-turner and I discussed trying to avoid multiple throws statements that effectively handled the same case, and we didn't think it was worth optimizing the exceptional case. AccessEvaluatorImpl.unescape for this case trivially creates a `new String(new char[0])`.
I think this probably gets you what you want with less code, and without the redundant throws:
```suggestion
term = len == 2 ? "" : AccessEvaluatorImpl.unescape(term.subSequence(1, term.length() - 1));
```
A better option would be to handle this in `AccessEvaluatorImpl.unescape`, so it returns an empty string literal instead of `new String(new char[0])`.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reduce method calls [accumulo-access]
Posted by "ctubbsii (via GitHub)" <gi...@apache.org>.
ctubbsii commented on code in PR #106:
URL: https://github.com/apache/accumulo-access/pull/106#discussion_r2913785814
##########
modules/core/src/main/java/org/apache/accumulo/access/impl/AccessExpressionImpl.java:
##########
@@ -77,7 +77,11 @@ public static CharSequence quote(CharSequence term) {
}
public static CharSequence unquote(CharSequence term) {
- if (term.length() >= 2 && term.charAt(0) == '"' && term.charAt(term.length() - 1) == '"') {
+ int len = term.length();
+ if (len >= 2 && term.charAt(0) == '"' && term.charAt(len - 1) == '"') {
+ if (len == 2) {
+ throw new IllegalArgumentException("Empty strings are not legal authorizations.");
+ }
term = AccessEvaluatorImpl.unescape(term.subSequence(1, term.length() - 1));
Review Comment:
@keith-turner and I discussed trying to avoid multiple throws statements that effectively handled the same case, and we didn't think it was worth optimizing the exceptional case. AccessEvaluatorImpl.unescape for this case trivially creates a `new String(new char[0])`.
I think this probably gets you what you want with less code, and without the redundant throws:
```suggestion
term = len == 2 ? "" : AccessEvaluatorImpl.unescape(term.subSequence(1, term.length() - 1));
```
A (possibly) better option would be to handle this in `AccessEvaluatorImpl.unescape`, so it returns an empty string literal instead of `new String(new char[0])`.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reduce method calls [accumulo-access]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion merged PR #106:
URL: https://github.com/apache/accumulo-access/pull/106
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org