CLDR-18683

macchiati · macchiati · commit f4a868231b07 · 2026-03-12T18:15:42.000-07:00
Fixed streaming to allow values to be maps. Using simpler implementation, added more tests
diff --git a/tools/cldr-code/src/main/java/org/unicode/cldr/util/FlatArraySpliterator.java b/tools/cldr-code/src/main/java/org/unicode/cldr/util/FlatArraySpliterator.java
@@ -0,0 +1,74 @@
+package org.unicode.cldr.util;
+
+import java.util.Iterator;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.Spliterator;
+import java.util.function.Consumer;
+
+/**
+ * Optimized Spliterator for fixed-depth nested maps. Returns a Stream of Object[] where: [0...N-1]
+ * are Keys [N] is the Value. Doesn't parallelize.
+ */
+public class FlatArraySpliterator implements Spliterator<Object[]> {
+    private final int keyCount;
+    private final Iterator<Entry<?, ?>>[] stack;
+    private final Object[] buffer; // Keys + 1 slot for Value
+    private int remainingKeys = 0;
+
+    @SuppressWarnings("unchecked")
+    public FlatArraySpliterator(Map<?, ?> root, int keyCount) {
+        this.keyCount = keyCount;
+        this.stack = (Iterator<Entry<?, ?>>[]) new Iterator[keyCount];
+        this.buffer = new Object[keyCount + 1];
+        this.stack[0] = (Iterator<Entry<?, ?>>) (Iterator<?>) root.entrySet().iterator();
+        // very ugly cast but works
+    }
+
+    @Override
+    @SuppressWarnings("unchecked")
+    public boolean tryAdvance(Consumer<? super Object[]> action) {
+        while (remainingKeys >= 0) {
+            Iterator<Entry<?, ?>> it = stack[remainingKeys];
+
+            if (!it.hasNext()) {
+                remainingKeys--;
+                continue;
+            }
+
+            Entry<?, ?> entry = it.next();
+            buffer[remainingKeys] = entry.getKey(); // Place key in its specific slot
+
+            if (remainingKeys < keyCount - 1) {
+                // Navigate deeper: Cast is safe per user guarantee
+                stack[++remainingKeys] =
+                        (Iterator<Entry<?, ?>>)
+                                (Iterator<?>) ((Map<?, ?>) entry.getValue()).entrySet().iterator();
+                // very ugly cast but works
+            } else {
+                // Leaf reached: Place value in the final slot
+                buffer[keyCount] = entry.getValue();
+
+                // Clone creates a snapshot [K1, K2, ... KN, V]
+                action.accept(buffer.clone());
+                return true;
+            }
+        }
+        return false;
+    }
+
+    @Override
+    public Spliterator<Object[]> trySplit() {
+        return null;
+    }
+
+    @Override
+    public long estimateSize() {
+        return Long.MAX_VALUE;
+    }
+
+    @Override
+    public int characteristics() {
+        return IMMUTABLE | NONNULL | DISTINCT;
+    }
+}
diff --git a/tools/cldr-code/src/main/java/org/unicode/cldr/util/NestedMap.java b/tools/cldr-code/src/main/java/org/unicode/cldr/util/NestedMap.java
@@ -2,18 +2,13 @@
 
 import com.google.common.collect.MapDifference;
 import com.google.common.collect.Maps;
-import java.util.ArrayDeque;
 import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.Deque;
-import java.util.Iterator;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
-import java.util.Spliterator;
-import java.util.Spliterators;
-import java.util.function.Consumer;
 import java.util.function.Supplier;
 import java.util.stream.Stream;
 import java.util.stream.StreamSupport;
@@ -57,7 +52,9 @@
  *   <li>ConcurrentHashMap::new — for thread-safety across all levels
  * </ul>
  *
- * Future possible extensions:
+ * NOTE: neither keys nor values can be null.
+ *
+ * <p>Future possible extensions:
  *
  * <ul>
  *   <li>The current implementation is limited to what CLDR needs. Addition methods like contains()
@@ -95,20 +92,23 @@ public class NestedMap {
      */
     @SafeVarargs
     private NestedMap(int keyCount, Supplier<Map<Object, Object>>... mapFactories) {
-        if (mapFactories.length == 0) {
-            throw new IllegalArgumentException("Too few mapFactories");
-        } else if (mapFactories.length > keyCount) {
+        if (mapFactories.length > keyCount) {
             throw new IllegalArgumentException("Too many mapFactories");
         }
         this.keyCount = keyCount;
         if (mapFactories.length == keyCount) {
             this.mapFactories = List.of(mapFactories);
         } else { // if the list is too short, fill out with copies of the last
             List<Supplier<Map<Object, Object>>> temp = new ArrayList<>();
-            temp.addAll(Arrays.asList(mapFactories));
-            Supplier<Map<Object, Object>> last = temp.get(temp.size() - 1);
+            Supplier<Map<Object, Object>> toDuplicate;
+            if (mapFactories.length == 0) {
+                toDuplicate = HashMap::new;
+            } else {
+                temp.addAll(Arrays.asList(mapFactories));
+                toDuplicate = temp.get(temp.size() - 1);
+            }
             while (temp.size() < keyCount) {
-                temp.add(last);
+                temp.add(toDuplicate);
             }
             this.mapFactories = List.copyOf(temp);
         }
@@ -147,7 +147,7 @@ private Object getInternal(Object... keys) {
             current = ((Map<Object, Object>) current).get(key);
             if (current == null) return null;
         }
-        return current;
+        return keys.length == keyCount ? current : ((Map<Object, Object>) current).keySet();
     }
 
     @SuppressWarnings("unchecked")
@@ -161,7 +161,7 @@ private void putInternal(Object... keysAndValue) {
         }
         for (Object keyOrValue : keysAndValue) {
             if (keyOrValue == null) {
-                throw new IllegalArgumentException("Cannot store null value");
+                throw new IllegalArgumentException("Cannot store null key or value");
             }
         }
 
@@ -181,7 +181,7 @@ private void putInternal(Object... keysAndValue) {
      * Deeply removes a value at the end of a key chain and prunes empty parent maps. If the
      * keysAndPossibleValue is not a long as the keyCount, then prunes the end.
      */
-    public void removeInternal(Object... keysAndPossibleValue) {
+    private void removeInternal(Object... keysAndPossibleValue) {
         if (this.mapFactories == null) { // attempt to modify immutable
             throw new UnsupportedOperationException("Cannot modify immutable object");
         }
@@ -277,67 +277,14 @@ private NestedMap toImmutable() {
     }
 
     /**
-     * Returns a LAZY stream of all terminal entries in the nested map. The stream is built
-     * on-the-fly and does not store all entries in memory. Each entry is a list of objects, where
-     * the last element is the value.
+     * Returns a lazy stream of all logical entries in the nested map. Each entry is a list of
+     * objects, where the last element is the value.
      */
     private Stream<List<Object>> stream() {
-        return StreamSupport.stream(new NestedMapSpliterator(this.root), false);
-    }
-
-    /**
-     * A custom Spliterator that traverses the nested map structure lazily. It uses a stack of
-     * iterators to perform a depth-first traversal.
-     */
-    private static class NestedMapSpliterator
-            extends Spliterators.AbstractSpliterator<List<Object>> {
-        private final Deque<Iterator<Map.Entry<Object, Object>>> iteratorStack = new ArrayDeque<>();
-        private final Deque<Object> keyPath = new ArrayDeque<>();
-
-        NestedMapSpliterator(Map<Object, Object> root) {
-            super(
-                    Long.MAX_VALUE,
-                    Spliterator.ORDERED); // Assumes ORDERED for TreeMap, harmless for others.
-            if (root != null && !root.isEmpty()) {
-                iteratorStack.push(root.entrySet().iterator());
-            }
-        }
-
-        @SuppressWarnings("unchecked")
-        @Override
-        public boolean tryAdvance(Consumer<? super List<Object>> action) {
-            while (!iteratorStack.isEmpty()) {
-                Iterator<Map.Entry<Object, Object>> currentIterator = iteratorStack.peek();
-
-                if (currentIterator.hasNext()) {
-                    Map.Entry<Object, Object> entry = currentIterator.next();
-                    Object key = entry.getKey();
-                    Object value = entry.getValue();
-
-                    keyPath.addLast(key);
-
-                    if (value instanceof Map) {
-                        // Go deeper: push the new map's iterator onto the stack.
-                        iteratorStack.push(((Map<Object, Object>) value).entrySet().iterator());
-                    } else {
-                        // Terminal value found: build the result and consume it.
-                        List<Object> result = new ArrayList<>(keyPath);
-                        result.add(value);
-                        action.accept(result);
-                        keyPath.removeLast(); // Backtrack path after consuming.
-                        return true;
-                    }
-                } else {
-                    // This level is exhausted: backtrack.
-                    iteratorStack.pop();
-                    if (!keyPath.isEmpty()) {
-                        keyPath.removeLast();
-                    }
-                }
-            }
-            // The entire map has been traversed.
-            return false;
-        }
+        // Note: the FlatArraySpliterator is much simpler to understand, but has some limitations.
+        // Notably, it doesn't parallelize.
+        return StreamSupport.stream(new FlatArraySpliterator(root, keyCount), false)
+                .map(array -> Arrays.asList(array));
     }
 
     @SuppressWarnings("unchecked")
@@ -446,7 +393,17 @@ private Map2(NestedMap engine) {
             this.engine = engine;
         }
 
-        /** Takes Treemap::new, HashMap::new, ConcurrentHashMap::new, and other suppliers */
+        /**
+         * Create a nested map with 2 keys, and 0..2 suppliers
+         *
+         * @param suppliers Common ones are Treemap::new, HashMap::new, ConcurrentHashMap::new, etc.
+         *     The default is a HashMap::new. If the number of suppliers is not insufficient, the
+         *     last supplier is used to fill it out.
+         *     <p>Examples So Map2.create(TreeMap::new) is equivalent to Map2.create(TreeMap::new,
+         *     TreeMap::new)<br>
+         *     So Map2.create() is equivalent to Map2.create(HashMap::new, HashMap::new)
+         * @return
+         */
         @SafeVarargs
         public static <K1, K2, V> Map2<K1, K2, V> create(
                 Supplier<Map<Object, Object>>... suppliers) {
@@ -476,10 +433,22 @@ public V get(K1 key1, K2 key2) {
             return (V) engine.getInternal(key1, key2);
         }
 
+        public Set<K2> keySet2(K1 key1) {
+            return (Set<K2>) engine.getInternal(key1);
+        }
+
+        public Set<K2> keySet() {
+            return (Set<K2>) engine.root.keySet();
+        }
+
         public void put(K1 key1, K2 key2, V value) {
             engine.putInternal(key1, key2, value);
         }
 
+        public void put(Entry3<K1, K2, V> entry3) {
+            engine.putInternal(entry3.getKey1(), entry3.getKey2(), entry3.getValue());
+        }
+
         public void putAll(Map2<K1, K2, V> other) {
             // Might optimize later
             other.stream().forEach(x -> put(x.getKey1(), x.getKey2(), x.getValue()));
@@ -527,7 +496,17 @@ private Map3(NestedMap engine) {
             this.engine = engine;
         }
 
-        /** Takes Treemap::new, HashMap::new, ConcurrentHashMap::new, and other suppliers */
+        /**
+         * Create a nested map with 3 keys, and 0..3 Suppliers
+         *
+         * @param suppliers Common ones are Treemap::new, HashMap::new, ConcurrentHashMap::new, etc.
+         *     The default is a HashMap::new. If the number of suppliers is not insufficient, the
+         *     last supplier is used to fill it out.
+         *     <p>Examples So Map3.create(TreeMap::new) is equivalent to Map2.create(TreeMap::new,
+         *     TreeMap::new, TreeMap::new)<br>
+         *     So Map3.create() is equivalent to Map2.create(HashMap::new, HashMap::new,
+         *     HashMap::new)
+         */
         @SafeVarargs
         public static <K1, K2, K3, V> Map3<K1, K2, K3, V> create(
                 Supplier<Map<Object, Object>>... suppliers) {
@@ -538,10 +517,27 @@ public V get(K1 key1, K2 key2, K3 key3) {
             return (V) engine.getInternal(key1, key2, key3);
         }
 
+        public Set<K3> keySet3(K1 key1, K2 key2) {
+            return (Set<K3>) engine.getInternal(key1, key2);
+        }
+
+        public Set<K2> keySet2(K1 key1) {
+            return (Set<K2>) engine.getInternal(key1);
+        }
+
+        public Set<K2> keySet() {
+            return (Set<K2>) engine.root.keySet();
+        }
+
         public void put(K1 key1, K2 key2, K3 key3, V value) {
             engine.putInternal(key1, key2, key3, value);
         }
 
+        public void put(Entry4<K1, K2, K3, V> entry3) {
+            engine.putInternal(
+                    entry3.getKey1(), entry3.getKey2(), entry3.getKey3(), entry3.getValue());
+        }
+
         public void remove(K1 key1, K2 key2, K3 key3, V value) {
             engine.removeInternal(key1, key2, key3, value);
         }
@@ -615,6 +611,18 @@ private Multimap2(NestedMap engine) {
          * Takes Treemap::new, HashMap::new, ConcurrentHashMap::new, and other suppliers. Note: the
          * final supplier should be one suitable for producing Map<V, Boolean>.
          */
+        /**
+         * Create a multimap map with 2 keys, and 0..3 suppliers
+         *
+         * @param suppliers Common ones are Treemap::new, HashMap::new, ConcurrentHashMap::new, etc.
+         *     The default is a HashMap::new. If the number of suppliers is not insufficient, the
+         *     last supplier is used to fill it out.
+         *     <p>Examples So Multimap2.create(TreeMap::new) is equivalent to
+         *     Multimap2.create(TreeMap::new, TreeMap::new, TreeMap::new)<br>
+         *     So Multimap2.create() is equivalent to Map2.create(HashMap::new, HashMap::new,
+         *     HashMap::new)<br>
+         *     Note: the final supplier should be one suitable for producing Map<V, Boolean>
+         */
         @SafeVarargs
         public static <K1, K2, V> Multimap2<K1, K2, V> create(
                 Supplier<Map<Object, Object>>... suppliers) {
@@ -625,14 +633,27 @@ public void put(K1 key1, K2 key2, V value) {
             engine.putInternal(key1, key2, value, Boolean.TRUE);
         }
 
+        public void put(Entry3<K1, K2, V> entry3) {
+            engine.putInternal(entry3.getKey1(), entry3.getKey2(), entry3.getValue(), Boolean.TRUE);
+        }
+
         @SuppressWarnings("unchecked")
         public Set<V> get(K1 key1, K2 key2) {
-            Map<V, Boolean> map = (Map<V, Boolean>) engine.getInternal(key1, key2);
-            return map == null ? null : map.keySet();
+            return (Set<V>) engine.getInternal(key1, key2);
+        }
+
+        @SuppressWarnings("unchecked")
+        public Set<K2> keySet2(K1 key1) {
+            return (Set<K2>) engine.getInternal(key1);
+        }
+
+        @SuppressWarnings("unchecked")
+        public Set<K1> keySet() {
+            return (Set<K1>) engine.root.keySet();
         }
 
         public boolean contains(K1 key1, K2 key2, V value) {
-            return engine.getInternal(key1, key2) != null;
+            return engine.getInternal(key1, key2, value) != null;
         }
 
         public void remove(K1 key1, K2 key2, V value) {
diff --git a/tools/cldr-code/src/test/java/org/unicode/cldr/util/NestedMapTest.java b/tools/cldr-code/src/test/java/org/unicode/cldr/util/NestedMapTest.java
