Fix random timezone id selection to be consistent with DateUtilsTests.testTimezoneIds (#105989)

Author: mosche

server/src/test/java/org/elasticsearch/common/time/DateUtilsTests.java

@@ -21,9 +21,6 @@
 import java.time.ZoneOffset;
 import java.time.ZonedDateTime;
 import java.time.temporal.ChronoField;
-import java.util.Arrays;
-import java.util.HashSet;
-import java.util.Set;
 
 import static org.elasticsearch.common.time.DateUtils.clampToNanosRange;
 import static org.elasticsearch.common.time.DateUtils.toInstant;
@@ -35,42 +32,15 @@
 import static org.hamcrest.Matchers.is;
 
 public class DateUtilsTests extends ESTestCase {
-    // list of ignored timezones.
-    // These should be cleaned up when all tested jdks (oracle, adoptopenjdk, openjdk etc) have the timezone db included
-    // see when a timezone was included in jdk version here https://www.oracle.com/java/technologies/tzdata-versions.html
-    private static final Set<String> IGNORE = new HashSet<>(
-        Arrays.asList(
-            "Eire",
-            "Europe/Dublin", // dublin timezone in joda does not account for DST
-            "Asia/Qostanay", // part of tzdata2018h
-            "America/Godthab", // part of tzdata2020a (maps to America/Nuuk)
-            "America/Nuuk", // part of tzdata2020a
-            "America/Ciudad_Juarez", // part of tzdata2022g
-            "America/Pangnirtung", // part of tzdata2022g
-            "Europe/Kyiv", // part of tzdata2022c,
-            "Pacific/Kanton" // part of tzdata2021b
-        )
-    );
-
-    // A temporary list of zones and JDKs, where Joda and the JDK timezone data are out of sync, until either Joda or the JDK
-    // are updated, see https://github.com/elastic/elasticsearch/issues/82356
-    private static final Set<String> IGNORE_IDS = new HashSet<>(Arrays.asList("Pacific/Niue", "America/Pangnirtung", "Antarctica/Vostok"));
-
-    private static boolean maybeIgnore(String jodaId) {
-        if (IGNORE_IDS.contains(jodaId)) {
-            return true;
-        }
-        return false;
-    }
 
     public void testTimezoneIds() {
         assertNull(DateUtils.dateTimeZoneToZoneId(null));
         assertNull(DateUtils.zoneIdToDateTimeZone(null));
         for (String jodaId : DateTimeZone.getAvailableIDs()) {
-            if (IGNORE.contains(jodaId) || maybeIgnore(jodaId)) continue;
+            if (IGNORED_TIMEZONE_IDS.contains(jodaId)) continue;
             DateTimeZone jodaTz = DateTimeZone.forID(jodaId);
             // some timezones get mapped back to problematic timezones
-            if (IGNORE.contains(jodaTz.toString())) continue;
+            if (IGNORED_TIMEZONE_IDS.contains(jodaTz.toString())) continue;
             ZoneId zoneId = DateUtils.dateTimeZoneToZoneId(jodaTz); // does not throw
             long now = 0;
             assertThat(

test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java

@@ -192,6 +192,30 @@
 @LuceneTestCase.SuppressReproduceLine
 public abstract class ESTestCase extends LuceneTestCase {
 
+    // A temporary list of ignored timezones.
+    //
+    // These should be cleaned up when all tested jdks (oracle, adoptopenjdk, openjdk etc) have the timezone db included
+    // see when a timezone was included in jdk version here https://www.oracle.com/java/technologies/tzdata-versions.html
+    // In some cases Joda and the JDK timezone data might also be out of sync,
+    // see https://github.com/elastic/elasticsearch/issues/82356
+    protected static final Set<String> IGNORED_TIMEZONE_IDS = Collections.unmodifiableSet(
+        new HashSet<>(
+            Arrays.asList(
+                "Eire",
+                "Europe/Dublin", // dublin timezone in joda does not account for DST
+                "Asia/Qostanay", // part of tzdata2018h
+                "America/Godthab", // part of tzdata2020a (maps to America/Nuuk)
+                "America/Nuuk", // part of tzdata2020a
+                "America/Ciudad_Juarez", // part of tzdata2022g
+                "America/Pangnirtung", // part of tzdata2022g
+                "Europe/Kyiv", // part of tzdata2022c,
+                "Pacific/Kanton", // part of tzdata2021b
+                "Pacific/Niue",
+                "Antarctica/Vostok"
+            )
+        )
+    );
+
     protected static final List<String> JODA_TIMEZONE_IDS;
     protected static final List<String> JAVA_TIMEZONE_IDS;
     protected static final List<String> JAVA_ZONE_IDS;
@@ -1058,8 +1082,17 @@ public static ZoneId randomZone() {
      * still need to do internally e.g. in bwc serialization and in the extract() method
      * //TODO remove once joda is not supported
      */
-    private static String randomJodaAndJavaSupportedTimezone(List<String> zoneIds) {
-        return randomValueOtherThanMany(id -> JODA_TIMEZONE_IDS.contains(id) == false, () -> randomFrom(zoneIds));
+    private static boolean rejectTimezone(String zoneId) {
+        // reject if unknown to joda
+        return JODA_TIMEZONE_IDS.contains(zoneId) == false
+            // reject ignored ids
+            || IGNORED_TIMEZONE_IDS.contains(zoneId)
+            // some timezones get mapped back to problematic timezones
+            || IGNORED_TIMEZONE_IDS.contains(DateTimeZone.forID(zoneId).toString());
+    }
+
+    protected static String randomJodaAndJavaSupportedTimezone(List<String> javaZoneIds) {
+        return randomValueOtherThanMany(ESTestCase::rejectTimezone, () -> randomFrom(javaZoneIds));
     }
 
     /**
@@ -1099,13 +1132,13 @@ public static <T> T randomValueOtherThan(T input, Supplier<T> randomSupplier) {
     }
 
     /**
-     * helper to get a random value in a certain range that's different from the input
+     * Helper to repeatedly get a random value until it doesn't match the reject predicate.
      */
-    public static <T> T randomValueOtherThanMany(Predicate<T> input, Supplier<T> randomSupplier) {
+    public static <T> T randomValueOtherThanMany(Predicate<T> reject, Supplier<T> randomSupplier) {
         T randomValue = null;
         do {
             randomValue = randomSupplier.get();
-        } while (input.test(randomValue));
+        } while (reject.test(randomValue));
         return randomValue;
     }
 

x-pack/plugin/sql/qa/jdbc/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/JdbcIntegrationTestCase.java

@@ -22,13 +22,8 @@
 import java.sql.Connection;
 import java.sql.DriverManager;
 import java.sql.SQLException;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.HashSet;
-import java.util.List;
 import java.util.Map;
 import java.util.Properties;
-import java.util.Set;
 
 public abstract class JdbcIntegrationTestCase extends ESRestTestCase {
 
@@ -142,11 +137,7 @@ public static String randomKnownTimeZone() {
         // from all available JDK timezones. While Joda and JDK are generally in sync, some timezones might not be known
         // to the current version of Joda and in this case the test might fail. To avoid that, we specify a timezone
         // known for both Joda and JDK
-        Set<String> timeZones = new HashSet<>(JODA_TIMEZONE_IDS);
-        timeZones.retainAll(JAVA_TIMEZONE_IDS);
-        List<String> ids = new ArrayList<>(timeZones);
-        Collections.sort(ids);
-        return randomFrom(ids);
+        return randomJodaAndJavaSupportedTimezone(JAVA_TIMEZONE_IDS);
     }
 
     private static Map<String, Object> searchStats() throws IOException {

x-pack/plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/JdbcIntegrationTestCase.java

@@ -21,12 +21,7 @@
 import java.sql.Connection;
 import java.sql.DriverManager;
 import java.sql.SQLException;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.HashSet;
-import java.util.List;
 import java.util.Properties;
-import java.util.Set;
 
 import static org.elasticsearch.common.Strings.hasText;
 import static org.elasticsearch.xpack.ql.TestUtils.assertNoSearchContexts;
@@ -158,10 +153,6 @@ public static String randomKnownTimeZone() {
         // from all available JDK timezones. While Joda and JDK are generally in sync, some timezones might not be known
         // to the current version of Joda and in this case the test might fail. To avoid that, we specify a timezone
         // known for both Joda and JDK
-        Set<String> timeZones = new HashSet<>(JODA_TIMEZONE_IDS);
-        timeZones.retainAll(JAVA_TIMEZONE_IDS);
-        List<String> ids = new ArrayList<>(timeZones);
-        Collections.sort(ids);
-        return randomFrom(ids);
+        return randomJodaAndJavaSupportedTimezone(JAVA_TIMEZONE_IDS);
     }
 }