Merge "Add TimeZoneProviderEventPreProcessor." into sc-dev

This commit is contained in:
TreeHugger Robot
2021-03-22 22:19:50 +00:00
committed by Android (Google) Code Review
8 changed files with 180 additions and 115 deletions

View File

@@ -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);
}

View File

@@ -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
// 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 (hasInvalidTimeZones(timeZoneProviderEvent)) {
infoLog("event=" + timeZoneProviderEvent + " has unsupported time zones. "
+ "Replacing it with uncertain event.");
timeZoneProviderEvent = TimeZoneProviderEvent.createUncertainEvent();
}
timeZoneProviderEvent =
mTimeZoneProviderEventPreProcessor.preProcess(timeZoneProviderEvent);
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();

View File

@@ -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);
}
}

View File

@@ -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;
}
}

View File

@@ -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",
mTimeZoneAvailabilityChecker);
stubbedProviderMetricsLogger, mTestThreadingDomain, "primary");
mTestSecondaryLocationTimeZoneProvider = new TestLocationTimeZoneProvider(
stubbedProviderMetricsLogger, mTestThreadingDomain, "secondary",
mTimeZoneAvailabilityChecker);
stubbedProviderMetricsLogger, mTestThreadingDomain, "secondary");
}
@Test
@@ -1185,10 +1181,9 @@ public class ControllerImplTest {
* Creates the instance.
*/
TestLocationTimeZoneProvider(ProviderMetricsLogger providerMetricsLogger,
ThreadingDomain threadingDomain, String providerName,
TimeZoneIdValidator timeZoneIdValidator) {
ThreadingDomain threadingDomain, String providerName) {
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;
}
}
}

View File

@@ -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;
}
}

View File

@@ -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,
mTimeZoneAvailabilityChecker);
mTimeZoneAvailabilityChecker.validIds("Europe/London");
providerMetricsLogger,
mTestThreadingDomain,
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,
mTimeZoneAvailabilityChecker);
providerMetricsLogger,
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,
mTimeZoneAvailabilityChecker);
providerMetricsLogger,
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,
mTimeZoneAvailabilityChecker);
providerMetricsLogger,
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) {
super(providerMetricsLogger, threadingDomain, providerName, timeZoneIdValidator);
@NonNull TimeZoneProviderEventPreProcessor timeZoneProviderEventPreProcessor) {
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 {

View File

@@ -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 {
private final LocationTimeZoneProvider.TimeZoneIdValidator mTzChecker =
new ZoneInfoDbTimeZoneIdValidator();
public class ZoneInfoDbTimeZoneProviderEventPreProcessorTest {
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))
.isFalse();
.that(mPreProcessor.preProcess(event))
.isEqualTo(TimeZoneProviderEvent.createUncertainEvent());
}
}
private static TimeZoneProviderEvent timeZoneProviderEvent(String... timeZoneIds) {
return TimeZoneProviderEvent.createSuggestionEvent(
new TimeZoneProviderSuggestion.Builder()
.setTimeZoneIds(Arrays.asList(timeZoneIds))
.setElapsedRealtimeMillis(ARBITRARY_TIME_MILLIS)
.build());
}
}