keith-turner commented on code in PR #90:
URL: https://github.com/apache/accumulo-access/pull/90#discussion_r2627538004
##########
src/main/java/org/apache/accumulo/access/Authorizations.java:
##########
@@ -33,70 +33,24 @@
*
* @since 1.0.0
*/
-public final class Authorizations implements Iterable<String>, Serializable {
+public final record Authorizations(Set<String> authorizations)
+ implements Iterable<String>, Serializable {
private static final long serialVersionUID = 1L;
private static final Authorizations EMPTY = new Authorizations(Set.of());
- private final Set<String> authorizations;
-
- private Authorizations(Set<String> authorizations) {
+ public Authorizations(Set<String> authorizations) {
this.authorizations = Set.copyOf(authorizations);
}
- /**
- * Returns the set of authorization strings in this Authorization object
- *
- * @return immutable set of authorization strings
- */
- public Set<String> asSet() {
- return authorizations;
- }
-
- @Override
- public boolean equals(Object o) {
- if (o instanceof Authorizations) {
- var oa = (Authorizations) o;
- return authorizations.equals(oa.authorizations);
- }
-
- return false;
- }
-
- @Override
- public int hashCode() {
- return authorizations.hashCode();
- }
-
@Override
- public String toString() {
- return authorizations.toString();
+ public Iterator<String> iterator() {
+ return authorizations.iterator();
}
- /**
- * @return a pre-allocated empty Authorizations object
- */
- public static Authorizations of() {
+ public static Authorizations empty() {
return EMPTY;
}
- /**
- * Creates an Authorizations object from the set of input authorization
strings.
- *
- * @param authorizations set of authorization strings
- * @return Authorizations object
- */
- public static Authorizations of(Set<String> authorizations) {
- if (authorizations.isEmpty()) {
- return EMPTY;
Review Comment:
Removing this we lose retruning EMPTY when trying to create w/ an empty set.
To get this same functionality the user would always need to do the following.
```java
Set<String> as.;
Authorizations auths;
if(as.isEmpty()){
auths = Authorizations.empty();
}else{
auths = new Authorizations(as);
}
```
##########
src/build/ci/find-unapproved-public.sh:
##########
@@ -18,15 +18,24 @@
# under the License.
#
+files=$(grep -E "public.*(class|interface|enum|record)"
src/main/java/org/apache/accumulo/access/*.java |
+ grep -v " interface AccessEvaluator " |
+ grep -v " class AccessExpression " |
+ grep -v " class ParsedAccessExpression " |
+ grep -v " enum ExpressionType " |
+ grep -v " record Authorizations" |
+ grep -v " class InvalidAccessExpressionException ")
+
count=$(grep -E "public.*(class|interface|enum|record)"
src/main/java/org/apache/accumulo/access/*.java |
grep -v " interface AccessEvaluator " |
grep -v " class AccessExpression " |
grep -v " class ParsedAccessExpression " |
grep -v " enum ExpressionType " |
- grep -v " class Authorizations " |
+ grep -v " record Authorizations" |
Review Comment:
We probably do not need this shell script anymore now that modules are
hiding the things moved to the impl package. This script existed because the
impl stuff used to be package private and it was really easy to change that w/o
realizing it.
##########
src/main/java/org/apache/accumulo/access/ParsedAccessExpression.java:
##########
@@ -28,16 +30,12 @@
*
* @since 1.0.0
*/
-public abstract class ParsedAccessExpression extends AccessExpression {
+public abstract sealed class ParsedAccessExpression extends AccessExpression
+ permits ParsedAccessExpressionImpl {
private static final long serialVersionUID = 1L;
- /*
- * This is package private so that it can not be extended by classes outside
of this package and
- * create a mutable implementation. In this package all implementations that
extends are
- * immutable.
- */
- ParsedAccessExpression() {}
+ protected ParsedAccessExpression() {}
Review Comment:
Could probably remove this. It only existed to prevent extension, but now
that is done w/ sealed.
```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]
