Merge "Add TimeZoneProviderEventPreProcessor." into sc-dev

services/core/java/com/android/server/timezonedetector/location/BinderLocationTimeZoneProvider.java

@@ -47,7 +47,7 @@ class BinderLocationTimeZoneProvider extends LocationTimeZoneProvider {
             @NonNull String providerName,
             @NonNull LocationTimeZoneProviderProxy proxy) {
         super(providerMetricsLogger, threadingDomain, providerName,
-                new ZoneInfoDbTimeZoneIdValidator());
+                new ZoneInfoDbTimeZoneProviderEventPreProcessor());
         mProxy = Objects.requireNonNull(proxy);
     }
 

services/core/java/com/android/server/timezonedetector/location/LocationTimeZoneProvider.java

@@ -20,7 +20,6 @@ import static android.service.timezone.TimeZoneProviderService.TEST_COMMAND_RESU
 import static android.service.timezone.TimeZoneProviderService.TEST_COMMAND_RESULT_SUCCESS_KEY;
 
 import static com.android.server.timezonedetector.location.LocationTimeZoneManagerService.debugLog;
-import static com.android.server.timezonedetector.location.LocationTimeZoneManagerService.infoLog;
 import static com.android.server.timezonedetector.location.LocationTimeZoneManagerService.warnLog;
 import static com.android.server.timezonedetector.location.LocationTimeZoneProvider.ProviderState.PROVIDER_STATE_DESTROYED;
 import static com.android.server.timezonedetector.location.LocationTimeZoneProvider.ProviderState.PROVIDER_STATE_PERM_FAILED;
@@ -85,18 +84,6 @@ abstract class LocationTimeZoneProvider implements Dumpable {
         void onProviderStateChange(@NonNull ProviderState providerState);
     }
 
-    /**
-     * Used by {@link LocationTimeZoneProvider} to check if time zone IDs are understood
-     * by the platform.
-     */
-    interface TimeZoneIdValidator {
-
-        /**
-         * Returns whether {@code timeZoneId} is supported by the platform or not.
-         */
-        boolean isValid(@NonNull String timeZoneId);
-    }
-
     /**
      * Listener interface used to log provider events for metrics.
      */
@@ -386,19 +373,20 @@ abstract class LocationTimeZoneProvider implements Dumpable {
     // Non-null and effectively final after initialize() is called.
     ProviderListener mProviderListener;
 
-    @NonNull private TimeZoneIdValidator mTimeZoneIdValidator;
+    @NonNull private final TimeZoneProviderEventPreProcessor mTimeZoneProviderEventPreProcessor;
 
     /** Creates the instance. */
     LocationTimeZoneProvider(@NonNull ProviderMetricsLogger providerMetricsLogger,
             @NonNull ThreadingDomain threadingDomain,
             @NonNull String providerName,
-            @NonNull TimeZoneIdValidator timeZoneIdValidator) {
+            @NonNull TimeZoneProviderEventPreProcessor timeZoneProviderEventPreProcessor) {
         mThreadingDomain = Objects.requireNonNull(threadingDomain);
         mProviderMetricsLogger = Objects.requireNonNull(providerMetricsLogger);
         mInitializationTimeoutQueue = threadingDomain.createSingleRunnableQueue();
         mSharedLock = threadingDomain.getLockObject();
         mProviderName = Objects.requireNonNull(providerName);
-        mTimeZoneIdValidator = Objects.requireNonNull(timeZoneIdValidator);
+        mTimeZoneProviderEventPreProcessor =
+                Objects.requireNonNull(timeZoneProviderEventPreProcessor);
     }
 
     /**
@@ -639,24 +627,8 @@ abstract class LocationTimeZoneProvider implements Dumpable {
         mThreadingDomain.assertCurrentThread();
         Objects.requireNonNull(timeZoneProviderEvent);
 
-        // If the provider has made a suggestion with unknown time zone IDs it cannot be used to set
+        timeZoneProviderEvent =
-        // the device's time zone. This logic prevents bad time zone IDs entering the time zone
+                mTimeZoneProviderEventPreProcessor.preProcess(timeZoneProviderEvent);
-        // detection logic from third party code.
-        //
-        // An event containing an unknown time zone ID could occur if the provider is using a
-        // different TZDB version than the device. Provider developers are expected to take steps to
-        // avoid version skew problem, e.g. by ensuring atomic updates with the platform time zone
-        // rules, or providing IDs based on the device's TZDB version, so this is not considered a
-        // common case.
-        //
-        // Treating a suggestion containing unknown time zone IDs as "uncertain" in the primary
-        // enables immediate failover to a secondary provider, one that might provide valid IDs for
-        // the same location, which should provide better behavior than just ignoring the event.
-        if (hasInvalidTimeZones(timeZoneProviderEvent)) {
-            infoLog("event=" + timeZoneProviderEvent + " has unsupported time zones. "
-                    + "Replacing it with uncertain event.");
-            timeZoneProviderEvent = TimeZoneProviderEvent.createUncertainEvent();
-        }
 
         synchronized (mSharedLock) {
             debugLog("handleTimeZoneProviderEvent: mProviderName=" + mProviderName
@@ -755,20 +727,6 @@ abstract class LocationTimeZoneProvider implements Dumpable {
         }
     }
 
-    private boolean hasInvalidTimeZones(@NonNull TimeZoneProviderEvent event) {
-        if (event.getSuggestion() == null) {
-            return false;
-        }
-
-        for (String timeZone : event.getSuggestion().getTimeZoneIds()) {
-            if (!mTimeZoneIdValidator.isValid(timeZone)) {
-                return true;
-            }
-        }
-
-        return false;
-    }
-
     @GuardedBy("mSharedLock")
     private void assertIsStarted() {
         ProviderState currentState = mCurrentState.get();

services/core/java/com/android/server/timezonedetector/location/TimeZoneProviderEventPreProcessor.java

@@ -18,13 +18,16 @@ package com.android.server.timezonedetector.location;
 
 import android.annotation.NonNull;
 
-import com.android.i18n.timezone.ZoneInfoDb;
+/**
+ * Used by {@link LocationTimeZoneProvider} to ensure that all time zone IDs are understood by the
+ * platform.
+ */
+public interface TimeZoneProviderEventPreProcessor {
 
-class ZoneInfoDbTimeZoneIdValidator implements
+    /**
-        LocationTimeZoneProvider.TimeZoneIdValidator {
+     * May return uncertain event if {@code timeZoneProviderEvent} is ill-formed or drop/rewrite
+     * time zone IDs.
+     */
+    TimeZoneProviderEvent preProcess(@NonNull TimeZoneProviderEvent timeZoneProviderEvent);
 
-    @Override
-    public boolean isValid(@NonNull String timeZoneId) {
-        return ZoneInfoDb.getInstance().hasTimeZone(timeZoneId);
-    }
 }

services/core/java/com/android/server/timezonedetector/location/ZoneInfoDbTimeZoneProviderEventPreProcessor.java

@@ -0,0 +1,72 @@
+/*
+ * 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.location;
+
+import static com.android.server.timezonedetector.location.LocationTimeZoneManagerService.infoLog;
+
+import android.annotation.NonNull;
+
+import com.android.i18n.timezone.ZoneInfoDb;
+
+/**
+ * {@link TimeZoneProviderEventPreProcessor} implementation which makes validations against
+ * {@link ZoneInfoDb}.
+ */
+public class ZoneInfoDbTimeZoneProviderEventPreProcessor
+        implements TimeZoneProviderEventPreProcessor {
+
+    /**
+     * Returns uncertain event if {@code event} has at least one unsupported time zone ID.
+     */
+    @Override
+    public TimeZoneProviderEvent preProcess(@NonNull TimeZoneProviderEvent event) {
+        if (event.getSuggestion() == null || event.getSuggestion().getTimeZoneIds().isEmpty()) {
+            return event;
+        }
+
+        // If the provider has made a suggestion with unknown time zone IDs it cannot be used to set
+        // the device's time zone. This logic prevents bad time zone IDs entering the time zone
+        // detection logic from third party code.
+        //
+        // An event containing an unknown time zone ID could occur if the provider is using a
+        // different TZDB version than the device. Provider developers are expected to take steps to
+        // avoid version skew problem, e.g. by ensuring atomic updates with the platform time zone
+        // rules, or providing IDs based on the device's TZDB version, so this is not considered a
+        // common case.
+        //
+        // Treating a suggestion containing unknown time zone IDs as "uncertain" in the primary
+        // enables immediate failover to a secondary provider, one that might provide valid IDs for
+        // the same location, which should provide better behavior than just ignoring the event.
+        if (hasInvalidZones(event)) {
+            return TimeZoneProviderEvent.createUncertainEvent();
+        }
+
+        return event;
+    }
+
+    private static boolean hasInvalidZones(TimeZoneProviderEvent event) {
+        for (String timeZone : event.getSuggestion().getTimeZoneIds()) {
+            if (!ZoneInfoDb.getInstance().hasTimeZone(timeZone)) {
+                infoLog("event=" + event + " has unsupported zone(" + timeZone + ")");
+                return true;
+            }
+        }
+
+        return false;
+    }
+
+}

services/tests/servicestests/src/com/android/server/timezonedetector/location/ControllerImplTest.java

@@ -72,7 +72,6 @@ public class ControllerImplTest {
     private TestCallback mTestCallback;
     private TestLocationTimeZoneProvider mTestPrimaryLocationTimeZoneProvider;
     private TestLocationTimeZoneProvider mTestSecondaryLocationTimeZoneProvider;
-    private FakeTimeZoneIdValidator mTimeZoneAvailabilityChecker;
 
     @Before
     public void setUp() {
@@ -84,13 +83,10 @@ public class ControllerImplTest {
         };
         mTestThreadingDomain = new TestThreadingDomain();
         mTestCallback = new TestCallback(mTestThreadingDomain);
-        mTimeZoneAvailabilityChecker = new FakeTimeZoneIdValidator();
         mTestPrimaryLocationTimeZoneProvider = new TestLocationTimeZoneProvider(
-                stubbedProviderMetricsLogger, mTestThreadingDomain, "primary",
+                stubbedProviderMetricsLogger, mTestThreadingDomain, "primary");
-                mTimeZoneAvailabilityChecker);
         mTestSecondaryLocationTimeZoneProvider = new TestLocationTimeZoneProvider(
-                stubbedProviderMetricsLogger, mTestThreadingDomain, "secondary",
+                stubbedProviderMetricsLogger, mTestThreadingDomain, "secondary");
-                mTimeZoneAvailabilityChecker);
     }
 
     @Test
@@ -1185,10 +1181,9 @@ public class ControllerImplTest {
          * Creates the instance.
          */
         TestLocationTimeZoneProvider(ProviderMetricsLogger providerMetricsLogger,
-                ThreadingDomain threadingDomain, String providerName,
+                ThreadingDomain threadingDomain, String providerName) {
-                TimeZoneIdValidator timeZoneIdValidator) {
             super(providerMetricsLogger, threadingDomain, providerName,
-                    timeZoneIdValidator);
+                    new FakeTimeZoneProviderEventPreProcessor());
         }
 
         public void setFailDuringInitialization(boolean failInitialization) {
@@ -1321,14 +1316,4 @@ public class ControllerImplTest {
             mTestProviderState.commitLatest();
         }
     }
-
-    private static final class FakeTimeZoneIdValidator
-            implements LocationTimeZoneProvider.TimeZoneIdValidator {
-
-        @Override
-        public boolean isValid(@NonNull String timeZoneId) {
-            return true;
-        }
-
-    }
 }

services/tests/servicestests/src/com/android/server/timezonedetector/location/FakeTimeZoneProviderEventPreProcessor.java

@@ -0,0 +1,39 @@
+/*
+ * 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.location;
+
+/**
+ * Fake implementation of {@link TimeZoneProviderEventPreProcessor} which assumes that all events
+ * are valid or always uncertain if {@link #enterUncertainMode()} was called.
+ */
+public final class FakeTimeZoneProviderEventPreProcessor
+        implements TimeZoneProviderEventPreProcessor {
+
+    private boolean mIsUncertain = false;
+
+    @Override
+    public TimeZoneProviderEvent preProcess(TimeZoneProviderEvent timeZoneProviderEvent) {
+        if (mIsUncertain) {
+            return TimeZoneProviderEvent.createUncertainEvent();
+        }
+        return timeZoneProviderEvent;
+    }
+
+    public void enterUncertainMode() {
+        mIsUncertain = true;
+    }
+}

services/tests/servicestests/src/com/android/server/timezonedetector/location/LocationTimeZoneProviderTest.java

@@ -52,10 +52,8 @@ import org.junit.Test;
 
 import java.time.Duration;
 import java.util.Arrays;
-import java.util.HashSet;
 import java.util.LinkedList;
 import java.util.List;
-import java.util.Set;
 import java.util.concurrent.atomic.AtomicReference;
 
 /**
@@ -68,13 +66,13 @@ public class LocationTimeZoneProviderTest {
 
     private TestThreadingDomain mTestThreadingDomain;
     private TestProviderListener mProviderListener;
-    private FakeTimeZoneIdValidator mTimeZoneAvailabilityChecker;
+    private FakeTimeZoneProviderEventPreProcessor mTimeZoneProviderEventPreProcessor;
 
     @Before
     public void setUp() {
         mTestThreadingDomain = new TestThreadingDomain();
         mProviderListener = new TestProviderListener();
-        mTimeZoneAvailabilityChecker = new FakeTimeZoneIdValidator();
+        mTimeZoneProviderEventPreProcessor = new FakeTimeZoneProviderEventPreProcessor();
     }
 
     @Test
@@ -82,9 +80,10 @@ public class LocationTimeZoneProviderTest {
         String providerName = "arbitrary";
         RecordingProviderMetricsLogger providerMetricsLogger = new RecordingProviderMetricsLogger();
         TestLocationTimeZoneProvider provider = new TestLocationTimeZoneProvider(
-                providerMetricsLogger, mTestThreadingDomain, providerName,
+                providerMetricsLogger,
-                mTimeZoneAvailabilityChecker);
+                mTestThreadingDomain,
-        mTimeZoneAvailabilityChecker.validIds("Europe/London");
+                providerName,
+                mTimeZoneProviderEventPreProcessor);
 
         // initialize()
         provider.initialize(mProviderListener);
@@ -174,8 +173,10 @@ public class LocationTimeZoneProviderTest {
         String providerName = "primary";
         StubbedProviderMetricsLogger providerMetricsLogger = new StubbedProviderMetricsLogger();
         TestLocationTimeZoneProvider provider = new TestLocationTimeZoneProvider(
-                providerMetricsLogger, mTestThreadingDomain, providerName,
+                providerMetricsLogger,
-                mTimeZoneAvailabilityChecker);
+                mTestThreadingDomain,
+                providerName,
+                mTimeZoneProviderEventPreProcessor);
 
         TestCommand testCommand = TestCommand.createForTests("test", new Bundle());
         AtomicReference<Bundle> resultReference = new AtomicReference<>();
@@ -193,10 +194,11 @@ public class LocationTimeZoneProviderTest {
         String providerName = "primary";
         StubbedProviderMetricsLogger providerMetricsLogger = new StubbedProviderMetricsLogger();
         TestLocationTimeZoneProvider provider = new TestLocationTimeZoneProvider(
-                providerMetricsLogger, mTestThreadingDomain, providerName,
+                providerMetricsLogger,
-                mTimeZoneAvailabilityChecker);
+                mTestThreadingDomain,
+                providerName,
+                mTimeZoneProviderEventPreProcessor);
         provider.setStateChangeRecordingEnabled(true);
-        mTimeZoneAvailabilityChecker.validIds("Europe/London");
 
         // initialize()
         provider.initialize(mProviderListener);
@@ -234,14 +236,17 @@ public class LocationTimeZoneProviderTest {
     }
 
     @Test
-    public void considerSuggestionWithInvalidTimeZoneIdsAsUncertain() {
+    public void entersUncertainState_whenEventHasUnsupportedZones() {
         String providerName = "primary";
         StubbedProviderMetricsLogger providerMetricsLogger = new StubbedProviderMetricsLogger();
         TestLocationTimeZoneProvider provider = new TestLocationTimeZoneProvider(
-                providerMetricsLogger, mTestThreadingDomain, providerName,
+                providerMetricsLogger,
-                mTimeZoneAvailabilityChecker);
+                mTestThreadingDomain,
+                providerName,
+                mTimeZoneProviderEventPreProcessor);
         provider.setStateChangeRecordingEnabled(true);
         provider.initialize(mProviderListener);
+        mTimeZoneProviderEventPreProcessor.enterUncertainMode();
 
         ConfigurationInternal config = USER1_CONFIG_GEO_DETECTION_ENABLED;
         Duration arbitraryInitializationTimeout = Duration.ofMinutes(5);
@@ -309,8 +314,9 @@ public class LocationTimeZoneProviderTest {
         TestLocationTimeZoneProvider(@NonNull ProviderMetricsLogger providerMetricsLogger,
                 @NonNull ThreadingDomain threadingDomain,
                 @NonNull String providerName,
-                @NonNull TimeZoneIdValidator timeZoneIdValidator) {
+                @NonNull TimeZoneProviderEventPreProcessor timeZoneProviderEventPreProcessor) {
-            super(providerMetricsLogger, threadingDomain, providerName, timeZoneIdValidator);
+            super(providerMetricsLogger,
+                    threadingDomain, providerName, timeZoneProviderEventPreProcessor);
         }
 
         @Override
@@ -367,20 +373,6 @@ public class LocationTimeZoneProviderTest {
         }
     }
 
-    private static final class FakeTimeZoneIdValidator
-            implements LocationTimeZoneProvider.TimeZoneIdValidator {
-        private final Set<String> mValidTimeZoneIds = new HashSet<>();
-
-        @Override
-        public boolean isValid(@NonNull String timeZoneId) {
-            return mValidTimeZoneIds.contains(timeZoneId);
-        }
-
-        public void validIds(String... timeZoneIdss) {
-            mValidTimeZoneIds.addAll(asList(timeZoneIdss));
-        }
-    }
-
     private static class StubbedProviderMetricsLogger implements
             LocationTimeZoneProvider.ProviderMetricsLogger {
 

services/tests/servicestests/src/com/android/server/timezonedetector/location/ZoneInfoDbTimeZoneProviderEventPreProcessorTest.java

@@ -19,6 +19,7 @@ package com.android.server.timezonedetector.location;
 import static com.google.common.truth.Truth.assertWithMessage;
 
 import android.platform.test.annotations.Presubmit;
+import android.service.timezone.TimeZoneProviderSuggestion;
 
 import org.junit.Test;
 
@@ -26,29 +27,44 @@ import java.util.Arrays;
 import java.util.List;
 import java.util.TimeZone;
 
+/** Tests for {@link ZoneInfoDbTimeZoneProviderEventPreProcessor}. */
 @Presubmit
-public class ZoneInfoDbTimeZoneIdValidatorTest {
+public class ZoneInfoDbTimeZoneProviderEventPreProcessorTest {
-    private final LocationTimeZoneProvider.TimeZoneIdValidator mTzChecker =
+
-            new ZoneInfoDbTimeZoneIdValidator();
+    private static final long ARBITRARY_TIME_MILLIS = 11223344;
+
+    private final ZoneInfoDbTimeZoneProviderEventPreProcessor mPreProcessor =
+            new ZoneInfoDbTimeZoneProviderEventPreProcessor();
 
     @Test
     public void timeZoneIdsFromZoneInfoDbAreValid() {
         for (String timeZone : TimeZone.getAvailableIDs()) {
+            TimeZoneProviderEvent event = timeZoneProviderEvent(timeZone);
             assertWithMessage("Time zone %s should be supported", timeZone)
-                    .that(mTzChecker.isValid(timeZone)).isTrue();
+                    .that(mPreProcessor.preProcess(event)).isEqualTo(event);
         }
     }
 
     @Test
-    public void nonExistingZones_areNotSupported() {
+    public void eventWithNonExistingZones_areMappedToUncertainEvent() {
         List<String> nonExistingTimeZones = Arrays.asList(
-                "SystemV/HST10", "Atlantic/Atlantis", "EUROPE/LONDON", "Etc/GMT-5:30"
+                "SystemV/HST10", "Atlantic/Atlantis", "EUROPE/LONDON", "Etc/GMT-5:30");
-        );
 
         for (String timeZone : nonExistingTimeZones) {
+            TimeZoneProviderEvent event = timeZoneProviderEvent(timeZone);
+
             assertWithMessage(timeZone + " is not a valid time zone")
-                    .that(mTzChecker.isValid(timeZone))
+                    .that(mPreProcessor.preProcess(event))
-                    .isFalse();
+                    .isEqualTo(TimeZoneProviderEvent.createUncertainEvent());
         }
     }
+
+    private static TimeZoneProviderEvent timeZoneProviderEvent(String... timeZoneIds) {
+        return TimeZoneProviderEvent.createSuggestionEvent(
+                new TimeZoneProviderSuggestion.Builder()
+                        .setTimeZoneIds(Arrays.asList(timeZoneIds))
+                        .setElapsedRealtimeMillis(ARBITRARY_TIME_MILLIS)
+                .build());
+    }
+
 }