[GitHub] [jackrabbit-oak] rishabhdaim commented on a diff in pull request #890: OAK-10180: switch oak-core-spi to shaded guava

["rishabhdaim (via GitHub)" <gi...@apache.org>]

rishabhdaim commented on code in PR #890:
URL: https://github.com/apache/jackrabbit-oak/pull/890#discussion_r1163710154


##########
oak-core-spi/src/main/java/org/apache/jackrabbit/oak/spi/gc/DelegatingGCMonitor.java:
##########
@@ -41,15 +40,15 @@ public class DelegatingGCMonitor implements GCMonitor {
      * @param gcMonitors
      */
     public DelegatingGCMonitor(@NotNull Collection<? extends GCMonitor> gcMonitors) {
-        this.gcMonitors = newConcurrentHashSet();
+        this.gcMonitors = Collections.synchronizedSet(new HashSet<>());

Review Comment:
   Shouldn't we use `ConcurrentHashMap.newKeySet()` ?



##########
oak-core-spi/src/main/java/org/apache/jackrabbit/oak/spi/lock/LockConstants.java:
##########
@@ -16,14 +16,15 @@
  */
 package org.apache.jackrabbit.oak.spi.lock;
 
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashSet;
 import java.util.Set;
 
-import com.google.common.collect.ImmutableSet;
 import org.apache.jackrabbit.JcrConstants;
 
 public interface LockConstants extends JcrConstants {
 
-    Set<String> LOCK_PROPERTY_NAMES = ImmutableSet.of(
-            JCR_LOCKISDEEP,
-            JCR_LOCKOWNER);
+    Set<String> LOCK_PROPERTY_NAMES = Collections

Review Comment:
   I believe the goal is to upgrade `guava` not remove it. So we can keep using it, instead of moving this to Java, wdyt ?
   cc @mreutegg 



##########
oak-core-spi/src/main/java/org/apache/jackrabbit/oak/spi/gc/DelegatingGCMonitor.java:
##########
@@ -41,15 +40,15 @@ public class DelegatingGCMonitor implements GCMonitor {
      * @param gcMonitors
      */
     public DelegatingGCMonitor(@NotNull Collection<? extends GCMonitor> gcMonitors) {
-        this.gcMonitors = newConcurrentHashSet();
+        this.gcMonitors = Collections.synchronizedSet(new HashSet<>());
         this.gcMonitors.addAll(gcMonitors);
     }
 
     /**
      * New instance without any delegate.
      */
     public DelegatingGCMonitor() {
-        this(Sets.<GCMonitor>newConcurrentHashSet());
+        this(Collections.synchronizedSet(new HashSet<>()));

Review Comment:
   Same as above



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

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