Amoratinos commented on code in PR #2212:
URL: https://github.com/apache/jackrabbit-oak/pull/2212#discussion_r2032675189


##########
oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/basic/DefaultSyncConfig.java:
##########
@@ -18,10 +18,14 @@
 
 import java.util.Collections;
 import java.util.HashSet;
+import java.util.LinkedHashSet;
 import java.util.Map;
 import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import org.apache.jackrabbit.guava.common.collect.ImmutableSet;
+import org.apache.jackrabbit.oak.commons.collections.SetUtils;

Review Comment:
   These two are unused in the class could you please remove them?



##########
oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/AbstractLoginModule.java:
##########
@@ -212,9 +213,10 @@ public void initialize(Subject subject, CallbackHandler 
callbackHandler, Map<Str
     @Override
     public boolean logout() throws LoginException {
         boolean success = false;
-        Set<Object> creds = ImmutableSet.builder()
-                .addAll(subject.getPublicCredentials(Credentials.class))
-                .addAll(subject.getPublicCredentials(AuthInfo.class)).build();
+        Set<Object> builder = new 
LinkedHashSet<>(subject.getPublicCredentials(Credentials.class));
+        builder.addAll(subject.getPublicCredentials(AuthInfo.class));
+        Set<Object> creds = Collections.unmodifiableSet(builder);

Review Comment:
   Could you review the imports as `import 
org.apache.jackrabbit.guava.common.collect.ImmutableSet;` is not longer used 
after the changes



##########
oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/composite/CompositeAccessControlManager.java:
##########
@@ -18,7 +18,10 @@
 
 import java.security.Principal;
 import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;

Review Comment:
   Unused import, could you please remove it from here?



##########
oak-exercise/src/main/java/org/apache/jackrabbit/oak/exercise/security/authorization/models/simplifiedroles/Role.java:
##########
@@ -44,7 +49,9 @@ private Role(long permissions, String... privilegeNames) {
 
     private Role(@NotNull Role base, long permissions, String... 
privilegeNames) {
         this.permissions = base.permissions|permissions;
-        this.privilegeNames = 
ImmutableSet.<String>builder().addAll(base.privilegeNames).add(privilegeNames).build();
+        this.privilegeNames = Collections.unmodifiableSet(
+                (Set<String>) Stream.concat(base.privilegeNames.stream(), 
Arrays.stream(privilegeNames))
+                        .collect(Collectors.toCollection(LinkedHashSet::new)));

Review Comment:
   There's a shortcut to create unmodifiable sets from Collectors class:
   
   ```suggestion
           this.privilegeNames = Stream.concat(base.privilegeNames.stream(), 
Arrays.stream(privilegeNames))
                   .collect(Collectors.toUnmodifiableSet());
   ```



##########
oak-exercise/src/main/java/org/apache/jackrabbit/oak/exercise/security/authorization/models/simplifiedroles/Role.java:
##########
@@ -16,8 +16,13 @@
  */
 package 
org.apache.jackrabbit.oak.exercise.security.authorization.models.simplifiedroles;
 
+import java.util.Arrays;
 import java.util.Collections;
+import java.util.LinkedHashSet;
 import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
 import org.apache.jackrabbit.guava.common.collect.ImmutableSet;

Review Comment:
   With the changes this import is no longer used



-- 
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: oak-dev-unsubscr...@jackrabbit.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to