Merge "Consider equivalent time zone IDs as equal in metrics." into sc-dev

7 files changed, 140 insertions, 10 deletions

diff --git a/services/core/java/com/android/server/timezonedetector/MetricsTimeZoneDetectorState.java b/services/core/java/com/android/server/timezonedetector/MetricsTimeZoneDetectorState.java
index c8c828f10ad3..27b50d892144 100644
--- a/services/core/java/com/android/server/timezonedetector/MetricsTimeZoneDetectorState.java
+++ b/services/core/java/com/android/server/timezonedetector/MetricsTimeZoneDetectorState.java
@@ -89,8 +89,6 @@ public final class MetricsTimeZoneDetectorState {
             @Nullable TelephonyTimeZoneSuggestion latestTelephonySuggestion,
             @Nullable GeolocationTimeZoneSuggestion latestGeolocationSuggestion) {
 
-        // TODO(b/172934905) Add logic to canonicalize the time zone IDs to Android's preferred IDs
-        //  so that the ordinals will match even when the ID is not identical, just equivalent.
         int deviceTimeZoneIdOrdinal =
                 tzIdOrdinalGenerator.ordinal(Objects.requireNonNull(deviceTimeZoneId));
         MetricsTimeZoneSuggestion latestObfuscatedManualSuggestion =
diff --git a/services/core/java/com/android/server/timezonedetector/OrdinalGenerator.java b/services/core/java/com/android/server/timezonedetector/OrdinalGenerator.java
index a448773c40d5..50875308db7d 100644
--- a/services/core/java/com/android/server/timezonedetector/OrdinalGenerator.java
+++ b/services/core/java/com/android/server/timezonedetector/OrdinalGenerator.java
@@ -15,9 +15,12 @@
  */
 package com.android.server.timezonedetector;
 
+import android.annotation.NonNull;
 import android.util.ArraySet;
 
 import java.util.List;
+import java.util.Objects;
+import java.util.function.Function;
 
 /**
  * A helper class that turns a set of objects into ordinal values, i.e. each object is offered
@@ -30,11 +33,19 @@ import java.util.List;
 class OrdinalGenerator<T> {
     private final ArraySet<T> mKnownIds = new ArraySet<>();
 
+    private final @NonNull Function<T, T> mCanonicalizationFunction;
+
+    OrdinalGenerator(@NonNull Function<T, T> canonicalizationFunction) {
+        mCanonicalizationFunction = Objects.requireNonNull(canonicalizationFunction);
+    }
+
     int ordinal(T object) {
-        int ordinal = mKnownIds.indexOf(object);
+        T canonical = mCanonicalizationFunction.apply(object);
+
+        int ordinal = mKnownIds.indexOf(canonical);
         if (ordinal < 0) {
             ordinal = mKnownIds.size();
-            mKnownIds.add(object);
+            mKnownIds.add(canonical);
         }
         return ordinal;
     }
diff --git a/services/core/java/com/android/server/timezonedetector/TimeZoneCanonicalizer.java b/services/core/java/com/android/server/timezonedetector/TimeZoneCanonicalizer.java
new file mode 100644
index 000000000000..bdbf60712008
--- /dev/null
+++ b/services/core/java/com/android/server/timezonedetector/TimeZoneCanonicalizer.java
@@ -0,0 +1,35 @@
+/*
+ * Copyright (C) 2021 The Android Open Source Project
+ *
+ * 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 com.android.server.timezonedetector;
+
+import com.android.i18n.timezone.TimeZoneFinder;
+
+import java.util.function.Function;
+
+/**
+ * Returns preferred time zone ID if {@code timeZoneId} was deprecated. For example, returns
+ * America/Nuuk for America/Godthab.
+ */
+final class TimeZoneCanonicalizer implements Function<String, String> {
+    @Override
+    public String apply(String timeZoneId) {
+        String canonicialZoneId = TimeZoneFinder.getInstance().getCountryZonesFinder()
+                .findCanonicalTimeZoneId(timeZoneId);
+
+        return canonicialZoneId == null ? timeZoneId : canonicialZoneId;
+    }
+}
diff --git a/services/core/java/com/android/server/timezonedetector/TimeZoneDetectorStrategyImpl.java b/services/core/java/com/android/server/timezonedetector/TimeZoneDetectorStrategyImpl.java
index c34a7d37ba24..ab2a88b44a87 100644
--- a/services/core/java/com/android/server/timezonedetector/TimeZoneDetectorStrategyImpl.java
+++ b/services/core/java/com/android/server/timezonedetector/TimeZoneDetectorStrategyImpl.java
@@ -383,7 +383,8 @@ public final class TimeZoneDetectorStrategyImpl implements TimeZoneDetectorStrat
                 bestQualifiedTelephonySuggestion == null
                         ? null : bestQualifiedTelephonySuggestion.suggestion;
         // A new generator is created each time: we don't want / require consistency.
-        OrdinalGenerator<String> tzIdOrdinalGenerator = new OrdinalGenerator<>();
+        OrdinalGenerator<String> tzIdOrdinalGenerator =
+                new OrdinalGenerator<>(new TimeZoneCanonicalizer());
         return MetricsTimeZoneDetectorState.create(
                 tzIdOrdinalGenerator,
                 getConfigurationInternal(currentUserId),
diff --git a/services/tests/servicestests/src/com/android/server/timezonedetector/OrdinalGeneratorTest.java b/services/tests/servicestests/src/com/android/server/timezonedetector/OrdinalGeneratorTest.java
index af954d599334..3fdac66225a8 100644
--- a/services/tests/servicestests/src/com/android/server/timezonedetector/OrdinalGeneratorTest.java
+++ b/services/tests/servicestests/src/com/android/server/timezonedetector/OrdinalGeneratorTest.java
@@ -25,13 +25,14 @@ import org.junit.Test;
 import org.junit.runner.RunWith;
 
 import java.util.Arrays;
+import java.util.function.Function;
 
 @RunWith(AndroidJUnit4.class)
 public class OrdinalGeneratorTest {
 
     @Test
-    public void testOrdinal() {
-        OrdinalGenerator<String> ordinalGenerator = new OrdinalGenerator<>();
+    public void testOrdinal_withIdentityFunction() {
+        OrdinalGenerator<String> ordinalGenerator = new OrdinalGenerator<>(Function.identity());
         int oneOrd = ordinalGenerator.ordinal("One");
         int twoOrd = ordinalGenerator.ordinal("Two");
         assertNotEquals(oneOrd, twoOrd);
@@ -45,8 +46,8 @@ public class OrdinalGeneratorTest {
     }
 
     @Test
-    public void testOrdinals() {
-        OrdinalGenerator<String> ordinalGenerator = new OrdinalGenerator<>();
+    public void testOrdinals_withIdentityFunction() {
+        OrdinalGenerator<String> ordinalGenerator = new OrdinalGenerator<>(Function.identity());
         int[] oneTwoOrds = ordinalGenerator.ordinals(Arrays.asList("One", "Two"));
         int[] twoThreeOrds = ordinalGenerator.ordinals(Arrays.asList("Two", "Three"));
         assertEquals(oneTwoOrds[0], ordinalGenerator.ordinal("One"));
@@ -54,4 +55,33 @@ public class OrdinalGeneratorTest {
         assertEquals(twoThreeOrds[0], ordinalGenerator.ordinal("Two"));
         assertEquals(twoThreeOrds[1], ordinalGenerator.ordinal("Three"));
     }
+
+    @Test
+    public void testOrdinal_withCanonicalizationFunction() {
+        OrdinalGenerator<String> ordinalGenerator = new OrdinalGenerator<>(String::toLowerCase);
+
+        int oneOrd = ordinalGenerator.ordinal("One");
+        int twoOrd = ordinalGenerator.ordinal("Two");
+        assertNotEquals(oneOrd, twoOrd);
+
+        assertEquals(oneOrd, ordinalGenerator.ordinal("ONE"));
+        assertEquals(twoOrd, ordinalGenerator.ordinal("two"));
+
+        int threeOrd = ordinalGenerator.ordinal("Three");
+        assertNotEquals(oneOrd, threeOrd);
+        assertNotEquals(twoOrd, threeOrd);
+    }
+
+    @Test
+    public void testOrdinals_withCanonicalizationFunction() {
+        OrdinalGenerator<String> ordinalGenerator = new OrdinalGenerator<>(String::toLowerCase);
+
+        int[] oneTwoOrds = ordinalGenerator.ordinals(Arrays.asList("One", "Two"));
+        int[] twoThreeOrds = ordinalGenerator.ordinals(Arrays.asList("Two", "Three"));
+
+        assertEquals(oneTwoOrds[0], ordinalGenerator.ordinal("ONE"));
+        assertEquals(oneTwoOrds[1], ordinalGenerator.ordinal("two"));
+        assertEquals(twoThreeOrds[0], ordinalGenerator.ordinal("TWO"));
+        assertEquals(twoThreeOrds[1], ordinalGenerator.ordinal("threE"));
+    }
 }
diff --git a/services/tests/servicestests/src/com/android/server/timezonedetector/TimeZoneCanonicalizerTest.java b/services/tests/servicestests/src/com/android/server/timezonedetector/TimeZoneCanonicalizerTest.java
new file mode 100644
index 000000000000..0c78f5b85fac
--- /dev/null
+++ b/services/tests/servicestests/src/com/android/server/timezonedetector/TimeZoneCanonicalizerTest.java
@@ -0,0 +1,54 @@
+/*
+ * Copyright (C) 2021 The Android Open Source Project
+ *
+ * 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 com.android.server.timezonedetector;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import androidx.test.runner.AndroidJUnit4;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+@RunWith(AndroidJUnit4.class)
+public class TimeZoneCanonicalizerTest {
+
+    TimeZoneCanonicalizer mFunction = new TimeZoneCanonicalizer();
+
+    @Test
+    public void deprecatedTimeZonesAreEqualToCanonical() {
+        assertThat(mFunction.apply("America/Godthab")).isEqualTo("America/Nuuk");
+        assertThat(mFunction.apply("Australia/Currie")).isEqualTo("Australia/Hobart");
+    }
+
+    @Test
+    public void wellKnownCanonicalIDs() {
+        assertThat(mFunction.apply("America/Detroit")).isEqualTo("America/Detroit");
+        assertThat(mFunction.apply("Europe/London")).isEqualTo("Europe/London");
+        assertThat(mFunction.apply("America/New_York")).isEqualTo("America/New_York");
+        assertThat(mFunction.apply("Europe/Volgograd")).isEqualTo("Europe/Volgograd");
+    }
+
+    @Test
+    public void timeZonesAsGmtOffsetsTreatedAsCanonical() {
+        assertThat(mFunction.apply("Etc/GMT-11")).isEqualTo("Etc/GMT-11");
+    }
+
+    @Test
+    public void nonExistingOneMappedToThemselves() {
+        assertThat(mFunction.apply("Mars/Base")).isEqualTo("Mars/Base");
+    }
+}
diff --git a/services/tests/servicestests/src/com/android/server/timezonedetector/TimeZoneDetectorStrategyImplTest.java b/services/tests/servicestests/src/com/android/server/timezonedetector/TimeZoneDetectorStrategyImplTest.java
index 47475a66c0d5..331f76cf7dc2 100644
--- a/services/tests/servicestests/src/com/android/server/timezonedetector/TimeZoneDetectorStrategyImplTest.java
+++ b/services/tests/servicestests/src/com/android/server/timezonedetector/TimeZoneDetectorStrategyImplTest.java
@@ -56,6 +56,7 @@ import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.function.Function;
 
 /**
  * White-box unit tests for {@link TimeZoneDetectorStrategyImpl}.
@@ -1008,7 +1009,7 @@ public class TimeZoneDetectorStrategyImplTest {
         // Check the various feature state values are what we expect.
         assertFeatureStateMatchesConfig(expectedInternalConfig, actualState, expectedDetectionMode);
 
-        OrdinalGenerator<String> tzIdOrdinalGenerator = new OrdinalGenerator<>();
+        OrdinalGenerator<String> tzIdOrdinalGenerator = new OrdinalGenerator<>(Function.identity());
         MetricsTimeZoneDetectorState expectedState =
                 MetricsTimeZoneDetectorState.create(
                         tzIdOrdinalGenerator, expectedInternalConfig, expectedDeviceTimeZoneId,