Skip to content

Commit 7aada40

Browse files
rishabhdaimRishabh Kumar
andauthored
OAK-11602 : removed usage of Guava's ImmutableSet.copyOf with LinkedSet (#2178)
* OAK-11602 : removed usage of Guava's ImmutableSet.copyOf with LinkedSet * OAK-11602 : wrapped Set in Collections.unmodifiableSet wherever we are sending/receiving it to outer world * OAK-11602 : revert changes from 7 modules --------- Co-authored-by: Rishabh Kumar <diam@adobe.com>
1 parent 53c1964 commit 7aada40

File tree

19 files changed

+63
-37
lines changed

19 files changed

+63
-37
lines changed

oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/AutoMembershipPrincipals.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import org.apache.jackrabbit.api.security.user.UserManager;
2222
import org.apache.jackrabbit.guava.common.collect.ImmutableSet;
2323
import org.apache.jackrabbit.guava.common.collect.Iterators;
24+
import org.apache.jackrabbit.oak.commons.collections.SetUtils;
2425
import org.apache.jackrabbit.oak.spi.security.authentication.external.basic.AutoMembershipConfig;
2526
import org.apache.jackrabbit.oak.spi.security.principal.GroupPrincipals;
2627
import org.jetbrains.annotations.NotNull;
@@ -32,6 +33,7 @@
3233
import java.security.Principal;
3334
import java.util.ArrayList;
3435
import java.util.Arrays;
36+
import java.util.Collections;
3537
import java.util.HashMap;
3638
import java.util.HashSet;
3739
import java.util.Iterator;
@@ -220,7 +222,7 @@ private Map<Principal, Group> collectGlobalAutoMembershipPrincipals(@NotNull Str
220222
}
221223
}
222224
// only cache the principal instance but not the group (tree might become disconnected)
223-
principalMap.put(idpName, ImmutableSet.copyOf(map.keySet()));
225+
principalMap.put(idpName, Collections.unmodifiableSet(SetUtils.toLinkedSet(map.keySet())));
224226
} else {
225227
// resolve Group objects from cached principals
226228
principalMap.get(idpName).forEach(groupPrincipal -> {

oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/ExternalAuthorizableActionProvider.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import org.apache.jackrabbit.api.security.user.Group;
2222
import org.apache.jackrabbit.oak.api.Root;
2323
import org.apache.jackrabbit.oak.commons.PropertiesUtil;
24+
import org.apache.jackrabbit.oak.commons.collections.SetUtils;
2425
import org.apache.jackrabbit.oak.namepath.NamePathMapper;
2526
import org.apache.jackrabbit.oak.spi.security.SecurityProvider;
2627
import org.apache.jackrabbit.oak.spi.security.authentication.external.ExternalIdentityRef;
@@ -199,7 +200,7 @@ private Map<String, Set<String>> getAutomembership(boolean memberIsGroup) {
199200

200201
private static void updateAutoMembershipMap(@NotNull Map<String, Set<String>> map, @NotNull String syncHandlerName,
201202
@NotNull String idpName, @NotNull String[] membership) {
202-
Set<String> userMembership = ImmutableSet.copyOf(membership);
203+
Set<String> userMembership = Collections.unmodifiableSet(SetUtils.toLinkedSet(membership));
203204
Set<String> previous = map.put(idpName, userMembership);
204205
if (previous != null) {
205206
String msg = previous.equals(userMembership) ? "Duplicate" : "Colliding";

oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/ExternalGroupPrincipalProvider.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@
5757
import org.apache.jackrabbit.oak.api.Tree;
5858
import org.apache.jackrabbit.oak.api.Type;
5959
import org.apache.jackrabbit.oak.commons.collections.IterableUtils;
60+
import org.apache.jackrabbit.oak.commons.collections.SetUtils;
6061
import org.apache.jackrabbit.oak.namepath.NamePathMapper;
6162
import org.apache.jackrabbit.oak.plugins.memory.PropertyValues;
6263
import org.apache.jackrabbit.oak.spi.security.authentication.external.ExternalIdentityRef;
@@ -327,7 +328,7 @@ public boolean isMember(@NotNull Group group, @NotNull Authorizable authorizable
327328
return Collections.emptyIterator();
328329
}
329330

330-
Set<Value> valueSet = ImmutableSet.copyOf(vs);
331+
Set<Value> valueSet = Collections.unmodifiableSet(SetUtils.toLinkedSet(vs));
331332
Iterator<Group> declared = Iterators.filter(Iterators.transform(valueSet.iterator(), value -> {
332333
try {
333334
String groupPrincipalName = value.getString();

oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/ProtectionConfigImpl.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import org.apache.jackrabbit.oak.api.PropertyState;
2121
import org.apache.jackrabbit.oak.api.Tree;
2222
import org.apache.jackrabbit.oak.commons.PropertiesUtil;
23+
import org.apache.jackrabbit.oak.commons.collections.SetUtils;
2324
import org.apache.jackrabbit.oak.spi.security.authentication.external.ProtectionConfig;
2425
import org.jetbrains.annotations.NotNull;
2526
import org.osgi.service.component.annotations.Activate;
@@ -58,8 +59,8 @@ public class ProtectionConfigImpl implements ProtectionConfig {
5859

5960
@Activate
6061
protected void activate(Map<String, Object> properties) {
61-
propertyNames = ImmutableSet.copyOf(PropertiesUtil.toStringArray(properties.get("propertyNames"), new String[0]));
62-
nodeNames = ImmutableSet.copyOf(PropertiesUtil.toStringArray(properties.get("nodeNames"), new String[0]));
62+
propertyNames = Collections.unmodifiableSet(SetUtils.toLinkedSet(PropertiesUtil.toStringArray(properties.get("propertyNames"), new String[0])));
63+
nodeNames = Collections.unmodifiableSet(SetUtils.toLinkedSet(PropertiesUtil.toStringArray(properties.get("nodeNames"), new String[0])));
6364
}
6465

6566
@Override

oak-benchmarks/src/main/java/org/apache/jackrabbit/oak/benchmark/authorization/AbstractHasItemGetItemTest.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import org.apache.jackrabbit.commons.jackrabbit.authorization.AccessControlUtils;
2626
import org.apache.jackrabbit.oak.benchmark.ReadDeepTreeTest;
2727
import org.apache.jackrabbit.oak.commons.PathUtils;
28+
import org.apache.jackrabbit.oak.commons.collections.SetUtils;
2829
import org.apache.jackrabbit.oak.spi.security.authorization.accesscontrol.AccessControlConstants;
2930
import org.jetbrains.annotations.NotNull;
3031

@@ -86,7 +87,7 @@ protected void beforeSuite() throws Exception {
8687
createForEachPrincipal(principal, acMgr, allPrivileges);
8788

8889
adminSession.save();
89-
nodeSet = ImmutableSet.copyOf(nodePaths);
90+
nodeSet = SetUtils.toLinkedSet(nodePaths);
9091
}
9192

9293
private void createForEachPrincipal(@NotNull Principal pGrantedRead, @NotNull JackrabbitAccessControlManager acMgr, @NotNull List<Privilege> allPrivileges) throws RepositoryException {

oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/collections/SetUtils.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ public static <E> Set<E> newIdentityHashSet() {
114114
* @param <T> the type of the elements
115115
*/
116116
@NotNull
117-
public static <T> Set<T> toLinkedSet(@NotNull final Iterable<T> iterable) {
117+
public static <T> Set<T> toLinkedSet(@NotNull final Iterable<? extends T> iterable) {
118118
Objects.requireNonNull(iterable);
119119
final Set<T> result = new LinkedHashSet<>();
120120
iterable.forEach(result::add);

oak-core-spi/src/main/java/org/apache/jackrabbit/oak/spi/mount/MountInfo.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ public MountInfo(String name, boolean readOnly, List<String> pathsSupportingFrag
6565
this.readOnly = readOnly;
6666
this.pathFragmentName = "oak:mount-" + name;
6767
this.includedPaths = cleanCopy(includedPaths);
68-
this.pathsSupportingFragments = ImmutableSet.copyOf(pathsSupportingFragments);
68+
this.pathsSupportingFragments = Collections.unmodifiableSet(SetUtils.toLinkedSet(pathsSupportingFragments));
6969
}
7070

7171
@Override

oak-core-spi/src/main/java/org/apache/jackrabbit/oak/spi/observation/ChangeSet.java

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,14 @@
1818
*/
1919
package org.apache.jackrabbit.oak.spi.observation;
2020

21+
import java.util.Collections;
2122
import java.util.HashSet;
2223
import java.util.Set;
2324

2425
import org.apache.jackrabbit.guava.common.collect.ImmutableSet;
2526

2627
import org.apache.jackrabbit.oak.commons.PathUtils;
28+
import org.apache.jackrabbit.oak.commons.collections.SetUtils;
2729
import org.apache.jackrabbit.oak.commons.json.JsopBuilder;
2830
import org.apache.jackrabbit.oak.commons.json.JsopReader;
2931
import org.apache.jackrabbit.oak.commons.json.JsopTokenizer;
@@ -74,11 +76,11 @@ public final class ChangeSet {
7476
ChangeSet(int maxPathDepth, Set<String> parentPaths, Set<String> parentNodeNames, Set<String> parentNodeTypes,
7577
Set<String> propertyNames, Set<String> allNodeTypes) {
7678
this.maxPathDepth = maxPathDepth;
77-
this.parentPaths = parentPaths == null ? null : ImmutableSet.copyOf(parentPaths);
78-
this.parentNodeNames = parentNodeNames == null ? null : ImmutableSet.copyOf(parentNodeNames);
79-
this.parentNodeTypes = parentNodeTypes == null ? null : ImmutableSet.copyOf(parentNodeTypes);
80-
this.propertyNames = propertyNames == null ? null : ImmutableSet.copyOf(propertyNames);
81-
this.allNodeTypes = allNodeTypes == null ? null : ImmutableSet.copyOf(allNodeTypes);
79+
this.parentPaths = parentPaths == null ? null : Collections.unmodifiableSet(SetUtils.toLinkedSet(parentPaths));
80+
this.parentNodeNames = parentNodeNames == null ? null : Collections.unmodifiableSet(SetUtils.toLinkedSet(parentNodeNames));
81+
this.parentNodeTypes = parentNodeTypes == null ? null : Collections.unmodifiableSet(SetUtils.toLinkedSet(parentNodeTypes));
82+
this.propertyNames = propertyNames == null ? null : Collections.unmodifiableSet(SetUtils.toLinkedSet(propertyNames));
83+
this.allNodeTypes = allNodeTypes == null ? null : Collections.unmodifiableSet(SetUtils.toLinkedSet(allNodeTypes));
8284

8385
boolean hitsMaxPathDepth = false;
8486
if (parentPaths != null) {

oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/migration/NodeStateCopier.java

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import org.apache.jackrabbit.oak.api.PropertyState;
2222
import org.apache.jackrabbit.oak.api.Type;
2323
import org.apache.jackrabbit.oak.commons.PathUtils;
24+
import org.apache.jackrabbit.oak.commons.collections.SetUtils;
2425
import org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState;
2526
import org.apache.jackrabbit.oak.spi.commit.CommitHook;
2627
import org.apache.jackrabbit.oak.spi.commit.CommitInfo;
@@ -41,7 +42,6 @@
4142
import java.util.stream.StreamSupport;
4243

4344
import static java.util.Objects.requireNonNull;
44-
import static org.apache.jackrabbit.guava.common.collect.ImmutableSet.copyOf;
4545
import static java.util.Collections.emptySet;
4646

4747
/**
@@ -402,7 +402,7 @@ private Builder() {}
402402
@NotNull
403403
public Builder include(@NotNull Set<String> paths) {
404404
if (!requireNonNull(paths).isEmpty()) {
405-
this.includePaths = copyOf(paths);
405+
this.includePaths = Collections.unmodifiableSet(SetUtils.toLinkedSet(paths));
406406
}
407407
return this;
408408
}
@@ -416,7 +416,7 @@ public Builder include(@NotNull Set<String> paths) {
416416
*/
417417
@NotNull
418418
public Builder include(@NotNull String... paths) {
419-
return include(copyOf(requireNonNull(paths)));
419+
return include(Collections.unmodifiableSet(SetUtils.toLinkedSet(requireNonNull(paths))));
420420
}
421421

422422
/**
@@ -442,7 +442,7 @@ public Builder preserve(@NotNull boolean preserveOnTarget) {
442442
@NotNull
443443
public Builder exclude(@NotNull Set<String> paths) {
444444
if (!requireNonNull(paths).isEmpty()) {
445-
this.excludePaths = copyOf(paths);
445+
this.excludePaths = Collections.unmodifiableSet(SetUtils.toLinkedSet(paths));
446446
}
447447
return this;
448448
}
@@ -456,7 +456,7 @@ public Builder exclude(@NotNull Set<String> paths) {
456456
*/
457457
@NotNull
458458
public Builder exclude(@NotNull String... paths) {
459-
return exclude(copyOf(requireNonNull(paths)));
459+
return exclude(Collections.unmodifiableSet(SetUtils.toLinkedSet(requireNonNull(paths))));
460460
}
461461

462462
/**
@@ -469,7 +469,7 @@ public Builder exclude(@NotNull String... paths) {
469469
@NotNull
470470
public Builder supportFragment(@NotNull Set<String> paths) {
471471
if (!requireNonNull(paths).isEmpty()) {
472-
this.fragmentPaths = copyOf(paths);
472+
this.fragmentPaths = Collections.unmodifiableSet(SetUtils.toLinkedSet(paths));
473473
}
474474
return this;
475475
}
@@ -483,7 +483,7 @@ public Builder supportFragment(@NotNull Set<String> paths) {
483483
*/
484484
@NotNull
485485
public Builder supportFragment(@NotNull String... paths) {
486-
return supportFragment(copyOf(requireNonNull(paths)));
486+
return supportFragment(Collections.unmodifiableSet(SetUtils.toLinkedSet(requireNonNull(paths))));
487487
}
488488

489489
/**
@@ -496,7 +496,7 @@ public Builder supportFragment(@NotNull String... paths) {
496496
@NotNull
497497
public Builder excludeFragments(@NotNull Set<String> fragments) {
498498
if (!requireNonNull(fragments).isEmpty()) {
499-
this.excludeFragments = copyOf(fragments);
499+
this.excludeFragments = Collections.unmodifiableSet(SetUtils.toLinkedSet(fragments));
500500
}
501501
return this;
502502
}
@@ -510,7 +510,7 @@ public Builder excludeFragments(@NotNull Set<String> fragments) {
510510
*/
511511
@NotNull
512512
public Builder excludeFragments(@NotNull String... fragments) {
513-
return exclude(copyOf(requireNonNull(fragments)));
513+
return exclude(Collections.unmodifiableSet(SetUtils.toLinkedSet(requireNonNull(fragments))));
514514
}
515515

516516
/**
@@ -523,7 +523,7 @@ public Builder excludeFragments(@NotNull String... fragments) {
523523
@NotNull
524524
public Builder merge(@NotNull Set<String> paths) {
525525
if (!requireNonNull(paths).isEmpty()) {
526-
this.mergePaths = copyOf(paths);
526+
this.mergePaths = Collections.unmodifiableSet(SetUtils.toLinkedSet(paths));
527527
}
528528
return this;
529529
}
@@ -537,7 +537,7 @@ public Builder merge(@NotNull Set<String> paths) {
537537
*/
538538
@NotNull
539539
public Builder merge(@NotNull String... paths) {
540-
return merge(copyOf(requireNonNull(paths)));
540+
return merge(Collections.unmodifiableSet(SetUtils.toLinkedSet(requireNonNull(paths))));
541541
}
542542

543543
@NotNull

oak-core/src/main/java/org/apache/jackrabbit/oak/security/authentication/token/TokenConfigurationImpl.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import org.apache.jackrabbit.guava.common.collect.ImmutableList;
2020
import org.apache.jackrabbit.guava.common.collect.ImmutableSet;
2121
import org.apache.jackrabbit.oak.api.Root;
22+
import org.apache.jackrabbit.oak.commons.collections.SetUtils;
2223
import org.apache.jackrabbit.oak.spi.commit.MoveTracker;
2324
import org.apache.jackrabbit.oak.spi.commit.ValidatorProvider;
2425
import org.apache.jackrabbit.oak.spi.security.ConfigurationBase;
@@ -44,6 +45,7 @@
4445
import org.osgi.service.metatype.annotations.ObjectClassDefinition;
4546

4647
import java.security.Principal;
48+
import java.util.Collections;
4749
import java.util.List;
4850
import java.util.Map;
4951
import java.util.Set;
@@ -176,7 +178,7 @@ private CredentialsSupport newCredentialsSupport() {
176178
} else if (size == 1) {
177179
return credentialsSupport.values().iterator().next();
178180
} else {
179-
return CompositeCredentialsSupport.newInstance(() -> ImmutableSet.copyOf(credentialsSupport.values()));
181+
return CompositeCredentialsSupport.newInstance(() -> Collections.unmodifiableSet(SetUtils.toLinkedSet(credentialsSupport.values())));
180182
}
181183
}
182184
}

0 commit comments

Comments
 (0)