OAK-11640 : removed usage of ImmutableSet.builder (#2212)

* OAK-11640 : removed usage of ImmutableSet.builder

* OAK-11640 : incorporated review comments)

* OAK-11640 : revert unintendent code change

---------

Co-authored-by: Rishabh Kumar <diam@adobe.com>

Author: rishabhdaim

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;
 import org.jetbrains.annotations.NotNull;
 
 /**
@@ -203,9 +207,9 @@ public Authorizable setAutoMembershipConfig(@NotNull AutoMembershipConfig autoMe
          */
         @NotNull
         public Set<String> getAutoMembership(@NotNull org.apache.jackrabbit.api.security.user.Authorizable authorizable) {
-            return ImmutableSet.<String>builder().
-                    addAll(autoMembershipConfig.getAutoMembership(authorizable)).
-                    addAll(getAutoMembership()).build();
+            return Collections.unmodifiableSet((Set<String>)
+                    Stream.concat(autoMembershipConfig.getAutoMembership(authorizable).stream(), getAutoMembership().stream())
+                            .collect(Collectors.toCollection(LinkedHashSet::new)));
         }
 
         /**

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;
 import java.util.Iterator;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Set;
 import javax.jcr.AccessDeniedException;
@@ -71,12 +74,11 @@ public CompositeAccessControlManager(@NotNull Root root,
     @NotNull
     @Override
     public Privilege[] getSupportedPrivileges(String absPath) throws RepositoryException {
-        ImmutableSet.Builder<Privilege> privs = ImmutableSet.builder();
+        Set<Privilege> privs = new LinkedHashSet<>();
         for (AccessControlManager acMgr : acMgrs) {
-            privs.add(acMgr.getSupportedPrivileges(absPath));
+            privs.addAll(Arrays.asList(acMgr.getSupportedPrivileges(absPath)));
         }
-        Set<Privilege> s = privs.build();
-        return s.toArray(new Privilege[0]);
+        return privs.toArray(new Privilege[0]);
     }
 
     @Override

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;
 import org.apache.jackrabbit.oak.commons.collections.SetUtils;
 import org.apache.jackrabbit.oak.spi.security.authorization.permission.Permissions;
@@ -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)));
     }
 
     boolean grants(long permissions) {

oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/AbstractLoginModule.java

@@ -22,6 +22,7 @@
 import java.security.PrivilegedExceptionAction;
 import java.util.Collections;
 import java.util.HashSet;
+import java.util.LinkedHashSet;
 import java.util.Map;
 import java.util.Set;
 import javax.jcr.Credentials;
@@ -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);
+
         if (!subject.getPrincipals().isEmpty() && !creds.isEmpty()) {
             // clear subject if not readonly
             if (!subject.isReadOnly()) {

oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/privilege/ImmutablePrivilegeDefinition.java

@@ -16,11 +16,13 @@
  */
 package org.apache.jackrabbit.oak.spi.security.privilege;
 
+import java.util.Collections;
 import java.util.Objects;
 import java.util.Set;
 
 import org.apache.jackrabbit.guava.common.collect.ImmutableSet;
 
+import org.apache.jackrabbit.oak.commons.collections.SetUtils;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
@@ -37,11 +39,7 @@ public final class ImmutablePrivilegeDefinition implements PrivilegeDefinition {
     public ImmutablePrivilegeDefinition(@NotNull String name, boolean isAbstract, @Nullable Iterable<String> declaredAggregateNames) {
         this.name = name;
         this.isAbstract = isAbstract;
-        ImmutableSet.Builder<String> builder = ImmutableSet.builder();
-        if (declaredAggregateNames != null) {
-            builder.addAll(declaredAggregateNames);
-        }
-        this.declaredAggregateNames = builder.build();
+        this.declaredAggregateNames = declaredAggregateNames != null ? Collections.unmodifiableSet(SetUtils.toLinkedSet(declaredAggregateNames)) : Set.of();
         hashcode = Objects.hash(this.name, this.isAbstract, this.declaredAggregateNames);
     }
 

oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/privilege/PrivilegeBitsProvider.java

@@ -20,6 +20,7 @@
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.LinkedHashSet;
 import java.util.Map;
 import java.util.Set;
 import java.util.function.Function;
@@ -268,15 +269,15 @@ private Iterable<String> extractAggregatedPrivileges(@NotNull Iterable<String> p
 
     @NotNull
     private Set<String> resolveBuiltInAggregation(@NotNull String privilegeName) {
-        ImmutableSet.Builder<String> builder = ImmutableSet.builder();
+        Set<String> builder = new LinkedHashSet<>();
         for (String name : AGGREGATE_PRIVILEGES.get(privilegeName)) {
             if (!AGGREGATE_PRIVILEGES.containsKey(name)) {
                 builder.add(name);
             } else {
                 builder.addAll(resolveBuiltInAggregation(name));
             }
         }
-        Set<String> set = builder.build();
+        Set<String> set = Collections.unmodifiableSet(builder);
         aggregation.put(privilegeName, set);
         return set;
     }
@@ -295,10 +296,10 @@ public Iterable<String> apply(@Nullable String privName) {
                 } else if (AGGREGATE_PRIVILEGES.containsKey(privName)) {
                     return resolveBuiltInAggregation(privName);
                 } else {
-                    ImmutableSet.Builder<String> builder = ImmutableSet.builder();
+                    Set<String> builder = new LinkedHashSet<>();
                     fillAggregation(getPrivilegesTree().getChild(privName), builder);
 
-                    Set<String> aggregates = builder.build();
+                    Set<String> aggregates = Collections.unmodifiableSet(builder);
                     if (!JCR_ALL.equals(privName) && !aggregates.isEmpty()) {
                         aggregation.put(privName, aggregates);
                     }
@@ -307,7 +308,7 @@ public Iterable<String> apply(@Nullable String privName) {
             }
         }
 
-        private void fillAggregation(@NotNull Tree privTree, @NotNull ImmutableSet.Builder<String> builder) {
+        private void fillAggregation(@NotNull Tree privTree, @NotNull Set<String> builder) {
             if (!privTree.exists()) {
                 return;
             }

oak-segment-azure/src/test/java/org/apache/jackrabbit/oak/segment/azure/v8/AzureSegmentStoreV8Test.java

@@ -23,13 +23,15 @@
 import java.net.URISyntaxException;
 import java.time.Duration;
 import java.time.Instant;
+import java.util.Collections;
 import java.util.Date;
 import java.util.EnumSet;
 import java.util.HashMap;
 import java.util.Set;
 import java.util.stream.StreamSupport;
 
 import org.apache.jackrabbit.oak.blob.cloud.azure.blobstorage.AzuriteDockerRule;
+import org.apache.jackrabbit.oak.commons.collections.SetUtils;
 import org.apache.jackrabbit.oak.segment.azure.AzureSegmentStoreService;
 import org.apache.jackrabbit.oak.segment.azure.Configuration;
 import org.apache.jackrabbit.oak.segment.azure.util.Environment;
@@ -244,8 +246,11 @@ private static Instant yesterday() {
         return Instant.now().minus(Duration.ofDays(1));
     }
 
-    private static ImmutableSet<String> concat(ImmutableSet<String> blobs, String element) {
-        return ImmutableSet.<String>builder().addAll(blobs).add(element).build();
+    private static Set<String> concat(Set<String> blobs, String element) {
+
+        final Set<String> set = SetUtils.toLinkedSet(blobs);
+        set.add(element);
+        return Collections.unmodifiableSet(set);
     }
 
     private static Configuration getConfigurationWithSharedAccessSignature(String sasToken) {

oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/CompositionContext.java

@@ -30,7 +30,9 @@
 import java.io.InputStream;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashMap;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -67,10 +69,10 @@ class CompositionContext {
         this.prefetchNodeStore = globalStore instanceof PrefetchNodeStore ? (PrefetchNodeStore) globalStore : PrefetchNodeStore.NOOP;
         this.nonDefaultStores = nonDefaultStores;
 
-        ImmutableSet.Builder<MountedNodeStore> b = ImmutableSet.builder();
+        Set<MountedNodeStore> b = new LinkedHashSet<>();
         b.add(this.globalStore);
         b.addAll(this.nonDefaultStores);
-        allStores = b.build();
+        allStores = Collections.unmodifiableSet(b);
 
         this.nodeStoresByMount = allStores.stream().collect(Collectors.toMap(MountedNodeStore::getMount, Function.identity()));
         this.nodeStateMonitor = nodeStateMonitor;

oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoStatus.java

@@ -38,17 +38,15 @@
 
 import java.util.HashMap;
 import java.util.Map;
+import java.util.Set;
 import java.util.concurrent.TimeUnit;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 public class MongoStatus implements ServerMonitorListener {
 
     private static final Logger LOG = LoggerFactory.getLogger(MongoStatus.class);
 
-    private static final ImmutableSet<String> SERVER_DETAIL_FIELD_NAMES
-            = ImmutableSet.<String>builder()
-            .add("host", "process", "connections", "repl", "storageEngine", "mem")
-            .build();
+    private static final Set<String> SERVER_DETAIL_FIELD_NAMES = Set.of("host", "process", "connections", "repl", "storageEngine", "mem");
 
     private final MongoClient client;
 

oak-upgrade/src/main/java/org/apache/jackrabbit/oak/upgrade/RepositorySidegrade.java

@@ -21,6 +21,7 @@
 import java.util.Calendar;
 import java.util.Collections;
 import java.util.LinkedHashMap;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -465,8 +466,7 @@ private boolean isCompleteMigration() {
 
     private void copyWorkspace(NodeState sourceRoot, NodeBuilder targetRoot) {
         final Set<String> includes = calculateEffectiveIncludePaths(includePaths, sourceRoot);
-        ImmutableSet.Builder<String> excludes = new ImmutableSet.Builder<>();
-        excludes.addAll(excludePaths);
+        Set<String> excludes = new LinkedHashSet<>(excludePaths);
         if (!versionCopyConfiguration.isCopyAll()) {
             excludes.add("/jcr:system/jcr:versionStorage");
         }
@@ -477,7 +477,7 @@ private void copyWorkspace(NodeState sourceRoot, NodeBuilder targetRoot) {
 
         NodeStateCopier.builder()
             .include(includes)
-            .exclude(excludes.build())
+            .exclude(Collections.unmodifiableSet(excludes))
             .merge(merges)
             .copy(sourceRoot, targetRoot);