From 7096f59fd582f8ab0ac70ec500f16dcc17caae9d Mon Sep 17 00:00:00 2001 From: btangmu Date: Wed, 12 Feb 2025 13:35:49 -0500 Subject: [PATCH 1/3] CLDR-18217 Make CLDRFile Iterable interface include extra paths by default -Private boolean CLDRFile.DEFAULT_ITERATION_INCLUDES_EXTRAS determines whether to skip extras; either way they skip paths with null values -Rename old iterator iteratorWithoutExtras -Iterable and iterator are distinct, as in iterableWithoutExtras versus iteratorWithoutExtras -Call iteratorWithoutExtras/iterableWithoutExtras directly where tests would otherwise fail with DEFAULT_ITERATION_INCLUDES_EXTRAS true --- .../java/org/unicode/cldr/util/CLDRFile.java | 99 ++++++++++++++----- 1 file changed, 75 insertions(+), 24 deletions(-) diff --git a/tools/cldr-code/src/main/java/org/unicode/cldr/util/CLDRFile.java b/tools/cldr-code/src/main/java/org/unicode/cldr/util/CLDRFile.java index b0993b6e451..88ca69da747 100644 --- a/tools/cldr-code/src/main/java/org/unicode/cldr/util/CLDRFile.java +++ b/tools/cldr-code/src/main/java/org/unicode/cldr/util/CLDRFile.java @@ -9,6 +9,7 @@ package org.unicode.cldr.util; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterators; import com.google.common.util.concurrent.UncheckedExecutionException; import com.ibm.icu.impl.Relation; import com.ibm.icu.impl.Row; @@ -110,7 +111,7 @@ public class CLDRFile implements Freezable, Iterable, LocaleSt private static boolean LOG_PROGRESS = false; public static boolean HACK_ORDER = false; - private static boolean DEBUG_LOGGING = false; + private static final boolean DEBUG_LOGGING = false; public static final String SUPPLEMENTAL_NAME = "supplementalData"; @@ -1449,7 +1450,7 @@ public CLDRFile setInitialComment(String comment) { * Utility to restrict to files matching a given regular expression. The expression does not * contain ".xml". Note that supplementalData is always skipped, and root is always included. */ - public static Set getMatchingXMLFiles(File sourceDirs[], Matcher m) { + public static Set getMatchingXMLFiles(File[] sourceDirs, Matcher m) { Set s = new TreeSet<>(); for (File dir : sourceDirs) { @@ -1473,22 +1474,68 @@ public static Set getMatchingXMLFiles(File sourceDirs[], Matcher m) { return s; } - @Override + private final boolean DEFAULT_ITERATION_INCLUDES_EXTRAS = true; + public Iterator iterator() { + if (DEFAULT_ITERATION_INCLUDES_EXTRAS) { + return Iterators.filter(fullIterable().iterator(), p -> getStringValue(p) != null); + } else { + return iteratorWithoutExtras(); + } + } + + public Iterator iterator(String prefix) { + if (DEFAULT_ITERATION_INCLUDES_EXTRAS) { + if (prefix == null || prefix.isEmpty()) { + return iterator(); + } + return Iterators.filter(iterator(), p -> p.startsWith(prefix)); + } else { + return iteratorWithoutExtras(prefix); + } + } + + public Iterator iterator(Matcher pathFilter) { + if (DEFAULT_ITERATION_INCLUDES_EXTRAS) { + if (pathFilter == null) { + return iterator(); + } + return Iterators.filter(iterator(), p -> pathFilter.reset(p).matches()); + } else { + return iteratorWithoutExtras(pathFilter); + } + } + + public Iterator iterator(String prefix, Comparator comparator) { + if (comparator == null) { + throw new IllegalArgumentException("iterator requires non-null comparator"); + } + if (DEFAULT_ITERATION_INCLUDES_EXTRAS) { + Iterator it = + (prefix == null || prefix.isEmpty()) ? iterator() : iterator(prefix); + Set orderedSet = new TreeSet<>(comparator); + it.forEachRemaining(orderedSet::add); + return orderedSet.iterator(); + } else { + return iteratorWithoutExtras(prefix, comparator); + } + } + + private Iterator iteratorWithoutExtras() { return dataSource.iterator(); } - public synchronized Iterator iterator(String prefix) { + private synchronized Iterator iteratorWithoutExtras(String prefix) { return dataSource.iterator(prefix); } - public Iterator iterator(Matcher pathFilter) { + private Iterator iteratorWithoutExtras(Matcher pathFilter) { return dataSource.iterator(pathFilter); } - public Iterator iterator(String prefix, Comparator comparator) { + private Iterator iteratorWithoutExtras(String prefix, Comparator comparator) { Iterator it = - (prefix == null || prefix.length() == 0) + (prefix == null || prefix.isEmpty()) ? dataSource.iterator() : dataSource.iterator(prefix); if (comparator == null) return it; @@ -1497,22 +1544,38 @@ public Iterator iterator(String prefix, Comparator comparator) { return orderedSet.iterator(); } + public Iterable iterableDefault() { + if (DEFAULT_ITERATION_INCLUDES_EXTRAS) { + // same as fullIterable except skip paths with null values + Iterable it = new FullIterable(this); + return () -> Iterators.filter(it.iterator(), p -> getStringValue(p) != null); + } else { + return iterableWithoutExtras(); + } + } + + private Iterable iterableWithoutExtras() { + return this::iteratorWithoutExtras; + } + public Iterable fullIterable() { return new FullIterable(this); } - public static class FullIterable implements Iterable, SimpleIterator { + private static class FullIterable implements Iterable, SimpleIterator { private final CLDRFile file; - private final Iterator fileIterator; + private final Iterator fileIterator; // dataSource only private Iterator extraPaths; FullIterable(CLDRFile file) { this.file = file; - this.fileIterator = file.iterator(); + this.fileIterator = file.dataSource.iterator(); // file.iteratorWithoutExtras(); } @Override public Iterator iterator() { + // With.toIterator relies on the next() method to turn this FullIterable into an + // Iterator return With.toIterator(this); } @@ -3067,7 +3130,7 @@ public String getWinningPath(String path) { */ public Collection getExtraPaths() { Set toAddTo = new HashSet<>(getRawExtraPaths()); - for (String path : this) { + for (String path : this.iterableWithoutExtras()) { toAddTo.remove(path); } return toAddTo; @@ -3128,7 +3191,7 @@ public Set getRawExtraPaths() { private List getRawExtraPathsPrivate() { Set toAddTo = new HashSet<>(); ExtraPaths.addConstant(toAddTo); - ExtraPaths.addLocaleDependent(toAddTo, this, getLocaleID()); + ExtraPaths.addLocaleDependent(toAddTo, this.iterableWithoutExtras(), getLocaleID()); return toAddTo.stream().map(String::intern).collect(Collectors.toList()); } @@ -3372,18 +3435,6 @@ public String getConstructedValue(String xpath) { return null; } - /** - * Get the string value for the given path in this locale, without resolving to any other path - * or locale. - * - * @param xpath the given path - * @return the string value, unresolved - */ - private String getStringValueUnresolved(String xpath) { - CLDRFile sourceFileUnresolved = this.getUnresolved(); - return sourceFileUnresolved.getStringValue(xpath); - } - /** * Create an overriding LocaleStringProvider for testing and example generation * From e30d8374883d2de4f34c2e70e86da4ca3084e26b Mon Sep 17 00:00:00 2001 From: btangmu Date: Fri, 14 Feb 2025 12:51:01 -0500 Subject: [PATCH 2/3] CLDR-18217 For clarity in FullIterable, call iteratorWithoutExtras directly -Also for clarity, rename FullIterable.fileIterator to FullIterable.iteratorWithoutExtras --- .../src/main/java/org/unicode/cldr/util/CLDRFile.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tools/cldr-code/src/main/java/org/unicode/cldr/util/CLDRFile.java b/tools/cldr-code/src/main/java/org/unicode/cldr/util/CLDRFile.java index 88ca69da747..df0cc9f35b4 100644 --- a/tools/cldr-code/src/main/java/org/unicode/cldr/util/CLDRFile.java +++ b/tools/cldr-code/src/main/java/org/unicode/cldr/util/CLDRFile.java @@ -1564,12 +1564,12 @@ public Iterable fullIterable() { private static class FullIterable implements Iterable, SimpleIterator { private final CLDRFile file; - private final Iterator fileIterator; // dataSource only + private final Iterator iteratorWithoutExtras; private Iterator extraPaths; FullIterable(CLDRFile file) { this.file = file; - this.fileIterator = file.dataSource.iterator(); // file.iteratorWithoutExtras(); + this.iteratorWithoutExtras = file.iteratorWithoutExtras(); } @Override @@ -1581,8 +1581,8 @@ public Iterator iterator() { @Override public String next() { - if (fileIterator.hasNext()) { - return fileIterator.next(); + if (iteratorWithoutExtras.hasNext()) { + return iteratorWithoutExtras.next(); } if (extraPaths == null) { extraPaths = file.getExtraPaths().iterator(); From 1a44fda7685ea7ddb3dda195a8567056809b314f Mon Sep 17 00:00:00 2001 From: btangmu Date: Fri, 14 Feb 2025 13:01:25 -0500 Subject: [PATCH 3/3] CLDR-18217 Make DEFAULT_ITERATION_INCLUDES_EXTRAS false for now -Keeping DEFAULT_ITERATION_INCLUDES_EXTRAS false for now seems safest if we merge this for v47 --- .../cldr-code/src/main/java/org/unicode/cldr/util/CLDRFile.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/cldr-code/src/main/java/org/unicode/cldr/util/CLDRFile.java b/tools/cldr-code/src/main/java/org/unicode/cldr/util/CLDRFile.java index df0cc9f35b4..897fa3292dc 100644 --- a/tools/cldr-code/src/main/java/org/unicode/cldr/util/CLDRFile.java +++ b/tools/cldr-code/src/main/java/org/unicode/cldr/util/CLDRFile.java @@ -1474,7 +1474,7 @@ public static Set getMatchingXMLFiles(File[] sourceDirs, Matcher m) { return s; } - private final boolean DEFAULT_ITERATION_INCLUDES_EXTRAS = true; + private final boolean DEFAULT_ITERATION_INCLUDES_EXTRAS = false; public Iterator iterator() { if (DEFAULT_ITERATION_INCLUDES_EXTRAS) {