Fix TzIdProtos TimeZoneReplacement traversal logic am: 66751582b7 am: bf3146315b

Original change: https://android-review.googlesource.com/c/platform/system/timezone/+/1478612

Change-Id: Ib533420affc7d1116ed68e2bc5c830721eb16192
diff --git a/input_tools/android/tzids/src/main/java/com/android/timezone/tzids/TimeZoneIds.java b/input_tools/android/tzids/src/main/java/com/android/timezone/tzids/TimeZoneIds.java
index cc35e17..2e68057 100644
--- a/input_tools/android/tzids/src/main/java/com/android/timezone/tzids/TimeZoneIds.java
+++ b/input_tools/android/tzids/src/main/java/com/android/timezone/tzids/TimeZoneIds.java
@@ -170,21 +170,53 @@
                 .stream()
                 .collect(toMap(x -> x.getAlternativeId(), x -> x.getPreferredId())));
 
-        // Handle replacements: either add the replacementId or replacedId depending on the
-        // replacementThreshold.
-        putAllSafely(tzIdMap, countryMapping.getTimeZoneReplacementsList()
+        // Handle replacements: replacements are zone IDs that are recognized but potentially no
+        // longer used. Each replacement has a time when it comes into effect, and it may link to
+        // another replacement or a "top level" time zone ID. Below, the replacementThreshold is
+        // used to work out which replacements are in use and map each replacedId to a zone ID. It
+        // is ok if a zone maps to itself, i.e. when a replacement is in the future.
+
+        // Create a lookup of replacements by replacedId to make it easier to traverse the linked
+        // list formed by replacements referring to other replacements.
+        Map<String, TzIdsProto.TimeZoneReplacement> replacementLookupMap =
+                countryMapping.getTimeZoneReplacementsList()
+                        .stream()
+                        .collect(toMap(x -> x.getReplacedId(), Function.identity()));
+
+        // For each replacement in the replacement list, work out what the replacedId should map to
+        // before the replacementThreshold.
+        Map<String, String> replacementsAtThreshold = countryMapping.getTimeZoneReplacementsList()
                 .stream()
-                .collect(toMap(x -> x.getReplacedId(), x -> {
-                    if (x.getFromMillis() <= replacementThreshold.toEpochMilli()) {
-                        return x.getReplacementId();
-                    } else {
-                        return x.getReplacedId();
-                    }
-                })));
+                .collect(toMap(x -> x.getReplacedId(),
+                        x -> traverseReplacementList(x, replacementLookupMap, replacementThreshold)
+                ));
+        putAllSafely(tzIdMap, replacementsAtThreshold);
         return tzIdMap;
     }
 
     /**
+     * Returns the replacement ID for {@code start} by following the replacement links until there
+     * are no more replacements or the replacement happens after {@code replacementThreshold}.
+     */
+    private static String traverseReplacementList(TzIdsProto.TimeZoneReplacement start,
+            Map<String, TzIdsProto.TimeZoneReplacement> replacementLookupMap,
+            Instant replacementThreshold) {
+        TzIdsProto.TimeZoneReplacement current = start;
+        while(current.getFromMillis() <= replacementThreshold.toEpochMilli()) {
+            TzIdsProto.TimeZoneReplacement next =
+                    replacementLookupMap.get(current.getReplacementId());
+            if (next == null) {
+                // No more links to follow - use the replacement ID.
+                return current.getReplacementId();
+            }
+            current = next;
+        }
+        // There were more links that could be followed, but we stopped following
+        // because of the replacementThreshold.
+        return current.getReplacedId();
+    }
+
+    /**
      * Like {@link Map#putAll(Map)} but throws an exception if {@code sourceMap} contains a key
      * already present in {@code targetMap}.
      */
diff --git a/input_tools/android/tzids/src/main/proto/tz_ids_proto.proto b/input_tools/android/tzids/src/main/proto/tz_ids_proto.proto
index e92aae9..7aa3b1a 100644
--- a/input_tools/android/tzids/src/main/proto/tz_ids_proto.proto
+++ b/input_tools/android/tzids/src/main/proto/tz_ids_proto.proto
@@ -45,7 +45,7 @@
 
     // Replacements for time zones where the replaced time zone is not identical to the replacement
     // before some point in time. After that point in time, the two zones have been judged as
-    // equivalent. e.g. "America/Boise" has the same rules as "America/Phoenix" after Sun, 03 Feb
+    // equivalent. e.g. "America/Boise" has the same rules as "America/Denver" after Sun, 03 Feb
     // 1974, so the two can be considered equivalent today, but not for dates before that.
     repeated TimeZoneReplacement timeZoneReplacements = 4;
 }
diff --git a/input_tools/android/tzids/src/test/java/com/android/timezone/tzids/TimeZoneIdsTest.java b/input_tools/android/tzids/src/test/java/com/android/timezone/tzids/TimeZoneIdsTest.java
index 7d8fb63..95c095c 100644
--- a/input_tools/android/tzids/src/test/java/com/android/timezone/tzids/TimeZoneIdsTest.java
+++ b/input_tools/android/tzids/src/test/java/com/android/timezone/tzids/TimeZoneIdsTest.java
@@ -21,6 +21,7 @@
 import static org.junit.Assert.assertNull;
 
 import com.android.timezone.tzids.proto.TzIdsProto;
+
 import org.junit.Test;
 
 import java.time.Instant;
@@ -62,54 +63,53 @@
     public void getCountryIdMap_replacements() throws Exception {
         TzIdsProto.TimeZoneIds.Builder tzIdsBuilder = TzIdsProto.TimeZoneIds.newBuilder();
 
-        // A much-simplified version of the US time zone IDs.
-        Instant boiseFrom = LocalDateTime.of(1974, Month.FEBRUARY, 3, 9, 0).toInstant(UTC);
-        Instant dakotaFrom = LocalDateTime.of(1992, Month.OCTOBER, 25, 8, 0).toInstant(UTC);
+        // A much-simplified version of the US time zone IDs with a chain of replacements.
+        Instant knoxFrom = LocalDateTime.of(1991, Month.OCTOBER, 27, 7, 0).toInstant(UTC);
+        Instant tellCityFrom = LocalDateTime.of(2006, Month.APRIL, 2, 8, 0).toInstant(UTC);
         TzIdsProto.CountryMapping us = TzIdsProto.CountryMapping.newBuilder()
                 .setIsoCode("us")
-                .addTimeZoneIds("America/Phoenix")
                 .addTimeZoneIds("America/Chicago")
                 .addTimeZoneReplacements(
-                        createReplacement("America/Boise", "America/Phoenix", boiseFrom))
+                        createReplacement(
+                                "America/Indiana/Tell_City", "America/Chicago", tellCityFrom))
                 .addTimeZoneReplacements(
                         createReplacement(
-                                "America/North_Dakota/Center", "America/Chicago", dakotaFrom))
+                                "America/Indiana/Knox", "America/Indiana/Tell_City", knoxFrom))
                 .build();
         tzIdsBuilder.addCountryMappings(us);
 
         TimeZoneIds tzIds = new TimeZoneIds(tzIdsBuilder.build());
 
         Map<String, String> baseExpectedMap = new HashMap<>();
-        baseExpectedMap.put("America/Phoenix", "America/Phoenix");
         baseExpectedMap.put("America/Chicago", "America/Chicago");
 
-        // Before all replacements in effect.
+        // Before all replacements are in effect.
         {
             Map<String, String> expectedMap = new HashMap<>(baseExpectedMap);
-            expectedMap.put("America/Boise", "America/Boise");
-            expectedMap.put("America/North_Dakota/Center", "America/North_Dakota/Center");
+            expectedMap.put("America/Indiana/Tell_City", "America/Indiana/Tell_City");
+            expectedMap.put("America/Indiana/Knox", "America/Indiana/Knox");
 
             assertEquals(expectedMap, tzIds.getCountryIdMap("us", Instant.EPOCH));
-            assertEquals(expectedMap, tzIds.getCountryIdMap("us", boiseFrom.minusMillis(1)));
+            assertEquals(expectedMap, tzIds.getCountryIdMap("us", knoxFrom.minusMillis(1)));
         }
 
-        // One replacement in effect.
+        // One replacement is in effect.
         {
             Map<String, String> expectedMap = new HashMap<>(baseExpectedMap);
-            expectedMap.put("America/Boise", "America/Phoenix");
-            expectedMap.put("America/North_Dakota/Center", "America/North_Dakota/Center");
+            expectedMap.put("America/Indiana/Knox", "America/Indiana/Tell_City");
+            expectedMap.put("America/Indiana/Tell_City", "America/Indiana/Tell_City");
 
-            assertEquals(expectedMap, tzIds.getCountryIdMap("us", boiseFrom));
-            assertEquals(expectedMap, tzIds.getCountryIdMap("us", dakotaFrom.minusMillis(1)));
+            assertEquals(expectedMap, tzIds.getCountryIdMap("us", knoxFrom));
+            assertEquals(expectedMap, tzIds.getCountryIdMap("us", tellCityFrom.minusMillis(1)));
         }
 
-        // All replacements in effect.
+        // All replacements are in effect.
         {
             Map<String, String> expectedMap = new HashMap<>(baseExpectedMap);
-            expectedMap.put("America/Boise", "America/Phoenix");
-            expectedMap.put("America/North_Dakota/Center", "America/Chicago");
+            expectedMap.put("America/Indiana/Knox", "America/Chicago");
+            expectedMap.put("America/Indiana/Tell_City", "America/Chicago");
 
-            assertEquals(expectedMap, tzIds.getCountryIdMap("us", dakotaFrom));
+            assertEquals(expectedMap, tzIds.getCountryIdMap("us", tellCityFrom));
             assertEquals(expectedMap,
                     tzIds.getCountryIdMap("us", Instant.ofEpochMilli(Long.MAX_VALUE)));
         }