fix: minor refinements to collection utils (#29536)

closes: #29535

Signed-off-by: Steve Hawkins <shawkins@redhat.com>

common/src/main/java/org/keycloak/common/util/CollectionUtil.java

@@ -18,10 +18,8 @@
 package org.keycloak.common.util;
 
 import java.util.Collection;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
-import java.util.Iterator;
 import java.util.Map;
 import java.util.Set;
 import java.util.stream.Collectors;
@@ -36,15 +34,7 @@ public class CollectionUtil {
     }
 
     public static String join(Collection<String> strings, String separator) {
-        Iterator<String> iter = strings.iterator();
+        return strings.stream().collect(Collectors.joining(String.valueOf(separator)));
-        StringBuilder sb = new StringBuilder();
-        if(iter.hasNext()){
-            sb.append(iter.next());
-            while(iter.hasNext()){
-                sb.append(separator).append(iter.next());
-            }
-        }
-        return sb.toString();
     }
 
     // Return true if all items from col1 are in col2 and viceversa. Order is not taken into account
@@ -54,22 +44,20 @@ public class CollectionUtil {
         }
         Map<T, Integer> countMap = new HashMap<>();
         for(T o : col1) {
-            Integer v = countMap.get(o);
+            countMap.merge(o, 1, (v1, v2) -> v1 + v2);
-            countMap.put(o, v==null ? 1 : v+1);
         }
         for(T o : col2) {
             Integer v = countMap.get(o);
             if (v==null) {
                 return false;
             }
+            if (v == 1) {
+                countMap.remove(o);
+            } else {
                 countMap.put(o, v-1);
             }
-        for(Integer count : countMap.values()) {
-            if (count!=0) {
-                return false;
         }
-        }
+        return countMap.isEmpty();
-        return true;
     }
 
     public static boolean isEmpty(Collection<?> collection) {
@@ -80,17 +68,6 @@ public class CollectionUtil {
         return !isEmpty(collection);
     }
 
-    public static <T> Set<T> intersection(Collection<T> col1, Collection<T> col2) {
-        if (isEmpty(col1) || isEmpty(col2)) return Collections.emptySet();
-
-        final Collection<T> iteratorCollection = col1.size() <= col2.size() ? col1 : col2;
-        final Collection<T> searchCollection = iteratorCollection.equals(col1) ? col2 : col1;
-
-        return iteratorCollection.stream()
-                .filter(searchCollection::contains)
-                .collect(Collectors.toSet());
-    }
-
     public static <T> Set<T> collectionToSet(Collection<T> collection) {
         return collection == null ? null : new HashSet<>(collection);
     }

common/src/main/java/org/keycloak/common/util/ConcurrentMultivaluedHashMap.java

@@ -17,7 +17,6 @@
 
 package org.keycloak.common.util;
 
-import java.util.Collection;
 import java.util.List;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.CopyOnWriteArrayList;
@@ -27,84 +26,11 @@ import java.util.concurrent.CopyOnWriteArrayList;
  * @version $Revision: 1 $
  */
 @SuppressWarnings("serial")
-public class ConcurrentMultivaluedHashMap<K, V> extends ConcurrentHashMap<K, List<V>>
+public class ConcurrentMultivaluedHashMap<K, V> extends ConcurrentHashMap<K, List<V>> implements MultivaluedMap<K, V>
 {
-   public void putSingle(K key, V value)
-   {
-      List<V> list = createListInstance();
-      list.add(value);
-      put(key, list); // Just override with new List instance
-   }
 
-   public void addAll(K key, V... newValues)
+   @Override
-   {
+   public List<V> createListInstance() {
-      for (V value : newValues)
-      {
-         add(key, value);
-      }
-   }
-
-   public void addAll(K key, List<V> valueList)
-   {
-      for (V value : valueList)
-      {
-         add(key, value);
-      }
-   }
-
-   public void addFirst(K key, V value)
-   {
-      List<V> list = get(key);
-      if (list == null)
-      {
-         add(key, value);
-      }
-      else
-      {
-         list.add(0, value);
-      }
-   }
-   public final void add(K key, V value)
-   {
-      getList(key).add(value);
-   }
-
-
-   public final void addMultiple(K key, Collection<V> values)
-   {
-      getList(key).addAll(values);
-   }
-
-   public V getFirst(K key)
-   {
-      List<V> list = get(key);
-      return list == null ? null : list.get(0);
-   }
-
-   public final List<V> getList(K key)
-   {
-      List<V> list = get(key);
-
-      if (list == null) {
-         list = createListInstance();
-         List<V> existing = putIfAbsent(key, list);
-         if (existing != null) {
-            list = existing;
-         }
-      }
-
-      return list;
-   }
-
-   public void addAll(ConcurrentMultivaluedHashMap<K, V> other)
-   {
-      for (Entry<K, List<V>> entry : other.entrySet())
-      {
-         getList(entry.getKey()).addAll(entry.getValue());
-      }
-   }
-
-   protected List<V> createListInstance() {
       return new CopyOnWriteArrayList<>();
    }
 

common/src/main/java/org/keycloak/common/util/MultivaluedHashMap.java

@@ -17,8 +17,6 @@
 
 package org.keycloak.common.util;
 
-import java.util.ArrayList;
-import java.util.Collection;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -28,7 +26,7 @@ import java.util.Map;
  * @version $Revision: 1 $
  */
 @SuppressWarnings("serial")
-public class MultivaluedHashMap<K, V> extends HashMap<K, List<V>>
+public class MultivaluedHashMap<K, V> extends HashMap<K, List<V>> implements MultivaluedMap<K, V>
 {
    public MultivaluedHashMap() {
    }
@@ -44,89 +42,4 @@ public class MultivaluedHashMap<K, V> extends HashMap<K, List<V>>
    public MultivaluedHashMap(MultivaluedHashMap<K, V> config) {
       addAll(config);
    }
-
-   public void putSingle(K key, V value)
-   {
-      List<V> list = new ArrayList<>();
-      list.add(value);
-      put(key, list);
-   }
-
-   public void addAll(K key, V... newValues)
-   {
-      for (V value : newValues)
-      {
-         add(key, value);
-      }
-   }
-
-   public void addAll(K key, List<V> valueList)
-   {
-      for (V value : valueList)
-      {
-         add(key, value);
-      }
-   }
-
-   public void addFirst(K key, V value)
-   {
-      List<V> list = get(key);
-      if (list == null)
-      {
-         add(key, value);
-      }
-      else
-      {
-         list.add(0, value);
-      }
-   }
-   public final void add(K key, V value)
-   {
-      getList(key).add(value);
-   }
-
-
-   public final void addMultiple(K key, Collection<V> values)
-   {
-      getList(key).addAll(values);
-   }
-
-   public V getFirst(K key)
-   {
-      List<V> list = get(key);
-      return list == null ? null : list.get(0);
-   }
-
-   public final List<V> getList(K key)
-   {
-      List<V> list = get(key);
-      if (list == null)
-         put(key, list = new ArrayList<>());
-      return list;
-   }
-
-   public final void addAll(MultivaluedHashMap<K, V> other)
-   {
-      for (Map.Entry<K, List<V>> entry : other.entrySet())
-      {
-         getList(entry.getKey()).addAll(entry.getValue());
-      }
-   }
-
-   public boolean equalsIgnoreValueOrder(MultivaluedHashMap<K, V> omap) {
-      if (this == omap) {
-         return true;
-      }
-      if (!keySet().equals(omap.keySet())) {
-         return false;
-      }
-      for (Map.Entry<K, List<V>> e : entrySet()) {
-          List<V> list = e.getValue();
-          List<V> olist = omap.get(e.getKey());
-          if (!CollectionUtil.collectionEquals(list, olist)) {
-              return false;
-          }
-      }
-      return true;
-   }
 }

common/src/main/java/org/keycloak/common/util/MultivaluedMap.java

@@ -0,0 +1,93 @@
+/*
+ * Copyright 2024 Red Hat, Inc. and/or its affiliates
+ * and other contributors as indicated by the @author tags.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.keycloak.common.util;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+
+public interface MultivaluedMap<K, V> extends Map<K, List<V>> {
+
+    public default void putSingle(K key, V value) {
+        List<V> list = createListInstance();
+        list.add(value);
+        put(key, list); // Just override with new List instance
+    }
+
+    public default void addAll(K key, V... newValues) {
+        for (V value : newValues) {
+            add(key, value);
+        }
+    }
+
+    public default void addAll(K key, List<V> valueList) {
+        for (V value : valueList) {
+            add(key, value);
+        }
+    }
+
+    public default void addFirst(K key, V value) {
+        getList(key).add(0, value);
+    }
+
+    public default void add(K key, V value) {
+        getList(key).add(value);
+    }
+
+    public default void addMultiple(K key, Collection<V> values) {
+        getList(key).addAll(values);
+    }
+
+    public default V getFirst(K key) {
+        return Optional.ofNullable(get(key)).filter(l -> !l.isEmpty()).map(l -> l.get(0)).orElse(null);
+    }
+
+    public default List<V> getList(K key) {
+        return compute(key, (k, v) -> v != null ? v : createListInstance());
+    }
+
+    public default void addAll(MultivaluedMap<K, V> other) {
+        for (Entry<K, List<V>> entry : other.entrySet()) {
+            getList(entry.getKey()).addAll(entry.getValue());
+        }
+    }
+
+    public default boolean equalsIgnoreValueOrder(MultivaluedMap<K, V> omap) {
+       if (this == omap) {
+          return true;
+       }
+       if (!keySet().equals(omap.keySet())) {
+          return false;
+       }
+       for (Map.Entry<K, List<V>> e : entrySet()) {
+           List<V> list = e.getValue();
+           List<V> olist = omap.get(e.getKey());
+           if (!CollectionUtil.collectionEquals(list, olist)) {
+               return false;
+           }
+       }
+       return true;
+    }
+
+    public default List<V> createListInstance() {
+        return new ArrayList<>();
+    }
+
+}

docs/documentation/upgrading/topics/changes/changes-25_0_0.adoc

@@ -417,6 +417,10 @@ to avoid overflow after 2038.
 Method `String encode(String rawPassword, int iterations)` on the interface `org.keycloak.credential.hash.PasswordHashProvider` is deprecated. The method will be removed in
 one of the future {project_name} releases. It might be {project_name} 27 release.
 
+= CollectionUtil intesection method removed
+
+The method `org.keycloak.common.util.CollectionUtil.intersection` has been removed. You should use the 'java.util.Collection.retainAll' instead on an existing collection.
+
 = Resteasy util class is deprecated
 
 `org.keycloak.common.util.Resteasy` has been deprecated. You should use the `org.keycloak.util.KeycloakSessionUtil` to obtain the `KeycloakSession` instead.

operator/src/main/java/org/keycloak/operator/controllers/KeycloakDistConfigurator.java

@@ -31,13 +31,14 @@ import org.keycloak.operator.crds.v2alpha1.deployment.ValueOrSecret;
 import org.keycloak.operator.crds.v2alpha1.deployment.spec.DatabaseSpec;
 import org.keycloak.operator.crds.v2alpha1.deployment.spec.FeatureSpec;
 import org.keycloak.operator.crds.v2alpha1.deployment.spec.HostnameSpec;
-import org.keycloak.operator.crds.v2alpha1.deployment.spec.HttpSpec;
 import org.keycloak.operator.crds.v2alpha1.deployment.spec.HttpManagementSpec;
+import org.keycloak.operator.crds.v2alpha1.deployment.spec.HttpSpec;
 import org.keycloak.operator.crds.v2alpha1.deployment.spec.ProxySpec;
 import org.keycloak.operator.crds.v2alpha1.deployment.spec.TransactionsSpec;
 
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
@@ -158,12 +159,12 @@ public class KeycloakDistConfigurator {
                 .getAdditionalOptions()
                 .stream()
                 .map(ValueOrSecret::getName)
-                .collect(Collectors.toSet());
+                .collect(Collectors.toCollection(HashSet::new));
 
-        final var sameItems = CollectionUtil.intersection(serverConfigNames, firstClassConfigOptions.keySet());
+        serverConfigNames.retainAll(firstClassConfigOptions.keySet());
-        if (CollectionUtil.isNotEmpty(sameItems)) {
+        if (CollectionUtil.isNotEmpty(serverConfigNames)) {
             status.addWarningMessage("You need to specify these fields as the first-class citizen of the CR: "
-                    + CollectionUtil.join(sameItems, ","));
+                    + CollectionUtil.join(serverConfigNames, ","));
         }
     }