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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]