Merge "Treat time zone suggestions with unrecognized zones as unopinionated." into sc-dev

This commit is contained in:
Neil Fuller
2021-03-11 11:44:20 +00:00
committed by Android (Google) Code Review
7 changed files with 228 additions and 12 deletions

View File

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

View File

@@ -520,6 +520,12 @@ public class LocationTimeZoneManagerService extends Binder {
}
}
static void infoLog(String msg) {
if (Log.isLoggable(TAG, Log.INFO)) {
Slog.i(TAG, msg);
}
}
static void warnLog(String msg) {
warnLog(msg, null);
}

View File

@@ -20,6 +20,7 @@ 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;
@@ -84,6 +85,18 @@ 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);
}
/**
* Information about the provider's current state.
*/
@@ -364,13 +377,17 @@ abstract class LocationTimeZoneProvider implements Dumpable {
// Non-null and effectively final after initialize() is called.
ProviderListener mProviderListener;
@NonNull private TimeZoneIdValidator mTimeZoneIdValidator;
/** Creates the instance. */
LocationTimeZoneProvider(@NonNull ThreadingDomain threadingDomain,
@NonNull String providerName) {
@NonNull String providerName,
@NonNull TimeZoneIdValidator timeZoneIdValidator) {
mThreadingDomain = Objects.requireNonNull(threadingDomain);
mInitializationTimeoutQueue = threadingDomain.createSingleRunnableQueue();
mSharedLock = threadingDomain.getLockObject();
mProviderName = Objects.requireNonNull(providerName);
mTimeZoneIdValidator = Objects.requireNonNull(timeZoneIdValidator);
}
/**
@@ -610,6 +627,25 @@ 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();
}
synchronized (mSharedLock) {
debugLog("handleTimeZoneProviderEvent: mProviderName=" + mProviderName
+ ", timeZoneProviderEvent=" + timeZoneProviderEvent);
@@ -707,6 +743,20 @@ 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

@@ -0,0 +1,30 @@
/*
* 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 android.annotation.NonNull;
import com.android.i18n.timezone.ZoneInfoDb;
class ZoneInfoDbTimeZoneIdValidator implements
LocationTimeZoneProvider.TimeZoneIdValidator {
@Override
public boolean isValid(@NonNull String timeZoneId) {
return ZoneInfoDb.getInstance().hasTimeZone(timeZoneId);
}
}

View File

@@ -72,6 +72,7 @@ public class ControllerImplTest {
private TestCallback mTestCallback;
private TestLocationTimeZoneProvider mTestPrimaryLocationTimeZoneProvider;
private TestLocationTimeZoneProvider mTestSecondaryLocationTimeZoneProvider;
private FakeTimeZoneIdValidator mTimeZoneAvailabilityChecker;
@Before
public void setUp() {
@@ -80,10 +81,13 @@ public class ControllerImplTest {
// will never get a chance to execute.
mTestThreadingDomain = new TestThreadingDomain();
mTestCallback = new TestCallback(mTestThreadingDomain);
mTimeZoneAvailabilityChecker = new FakeTimeZoneIdValidator();
mTestPrimaryLocationTimeZoneProvider =
new TestLocationTimeZoneProvider(mTestThreadingDomain, "primary");
new TestLocationTimeZoneProvider(
mTestThreadingDomain, "primary", mTimeZoneAvailabilityChecker);
mTestSecondaryLocationTimeZoneProvider =
new TestLocationTimeZoneProvider(mTestThreadingDomain, "secondary");
new TestLocationTimeZoneProvider(
mTestThreadingDomain, "secondary", mTimeZoneAvailabilityChecker);
}
@Test
@@ -1177,8 +1181,10 @@ public class ControllerImplTest {
/**
* Creates the instance.
*/
TestLocationTimeZoneProvider(ThreadingDomain threadingDomain, String providerName) {
super(threadingDomain, providerName);
TestLocationTimeZoneProvider(ThreadingDomain threadingDomain,
String providerName,
TimeZoneIdValidator timeZoneIdValidator) {
super(threadingDomain, providerName, timeZoneIdValidator);
}
public void setFailDuringInitialization(boolean failInitialization) {
@@ -1311,4 +1317,14 @@ public class ControllerImplTest {
mTestProviderState.commitLatest();
}
}
private static final class FakeTimeZoneIdValidator
implements LocationTimeZoneProvider.TimeZoneIdValidator {
@Override
public boolean isValid(@NonNull String timeZoneId) {
return true;
}
}
}

View File

@@ -32,6 +32,8 @@ import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertTrue;
import static java.util.Arrays.asList;
import android.annotation.NonNull;
import android.annotation.Nullable;
import android.os.Bundle;
@@ -50,7 +52,9 @@ import org.junit.Test;
import java.time.Duration;
import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.concurrent.atomic.AtomicReference;
/**
@@ -62,20 +66,25 @@ public class LocationTimeZoneProviderTest {
private static final long ARBITRARY_ELAPSED_REALTIME_MILLIS = 123456789L;
private TestThreadingDomain mTestThreadingDomain;
private TestProviderListener mProviderListener;
private FakeTimeZoneIdValidator mTimeZoneAvailabilityChecker;
@Before
public void setUp() {
mTestThreadingDomain = new TestThreadingDomain();
mProviderListener = new TestProviderListener();
mTimeZoneAvailabilityChecker = new FakeTimeZoneIdValidator();
}
@Test
public void lifecycle() {
String providerName = "arbitrary";
TestLocationTimeZoneProvider provider =
new TestLocationTimeZoneProvider(mTestThreadingDomain, providerName);
new TestLocationTimeZoneProvider(
mTestThreadingDomain,
providerName,
mTimeZoneAvailabilityChecker);
mTimeZoneAvailabilityChecker.validIds("Europe/London");
// initialize()
provider.initialize(mProviderListener);
@@ -163,7 +172,10 @@ public class LocationTimeZoneProviderTest {
public void defaultHandleTestCommandImpl() {
String providerName = "primary";
TestLocationTimeZoneProvider provider =
new TestLocationTimeZoneProvider(mTestThreadingDomain, providerName);
new TestLocationTimeZoneProvider(
mTestThreadingDomain,
providerName,
mTimeZoneAvailabilityChecker);
TestCommand testCommand = TestCommand.createForTests("test", new Bundle());
AtomicReference<Bundle> resultReference = new AtomicReference<>();
@@ -180,8 +192,12 @@ public class LocationTimeZoneProviderTest {
public void stateRecording() {
String providerName = "primary";
TestLocationTimeZoneProvider provider =
new TestLocationTimeZoneProvider(mTestThreadingDomain, providerName);
new TestLocationTimeZoneProvider(
mTestThreadingDomain,
providerName,
mTimeZoneAvailabilityChecker);
provider.setStateChangeRecordingEnabled(true);
mTimeZoneAvailabilityChecker.validIds("Europe/London");
// initialize()
provider.initialize(mProviderListener);
@@ -218,6 +234,34 @@ public class LocationTimeZoneProviderTest {
provider.assertLatestRecordedState(PROVIDER_STATE_DESTROYED);
}
@Test
public void considerSuggestionWithInvalidTimeZoneIdsAsUncertain() {
String providerName = "primary";
TestLocationTimeZoneProvider provider =
new TestLocationTimeZoneProvider(
mTestThreadingDomain,
providerName,
mTimeZoneAvailabilityChecker);
provider.setStateChangeRecordingEnabled(true);
provider.initialize(mProviderListener);
ConfigurationInternal config = USER1_CONFIG_GEO_DETECTION_ENABLED;
Duration arbitraryInitializationTimeout = Duration.ofMinutes(5);
Duration arbitraryInitializationTimeoutFuzz = Duration.ofMinutes(2);
provider.startUpdates(config, arbitraryInitializationTimeout,
arbitraryInitializationTimeoutFuzz);
List<String> invalidTimeZoneIds = asList("Atlantic/Atlantis");
TimeZoneProviderSuggestion invalidIdSuggestion = new TimeZoneProviderSuggestion.Builder()
.setElapsedRealtimeMillis(ARBITRARY_ELAPSED_REALTIME_MILLIS)
.setTimeZoneIds(invalidTimeZoneIds)
.build();
TimeZoneProviderEvent event =
TimeZoneProviderEvent.createSuggestionEvent(invalidIdSuggestion);
provider.simulateProviderEventReceived(event);
provider.assertLatestRecordedState(PROVIDER_STATE_STARTED_UNCERTAIN);
}
/** A test stand-in for the real {@link LocationTimeZoneProviderController}'s listener. */
private static class TestProviderListener implements ProviderListener {
@@ -251,8 +295,9 @@ public class LocationTimeZoneProviderTest {
/** Creates the instance. */
TestLocationTimeZoneProvider(@NonNull ThreadingDomain threadingDomain,
@NonNull String providerName) {
super(threadingDomain, providerName);
@NonNull String providerName,
@NonNull TimeZoneIdValidator timeZoneIdValidator) {
super(threadingDomain, providerName, timeZoneIdValidator);
}
@Override
@@ -308,4 +353,19 @@ public class LocationTimeZoneProviderTest {
recordedStates.get(recordedStates.size() - 1).stateEnum);
}
}
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));
}
}
}

View File

@@ -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.location;
import static com.google.common.truth.Truth.assertWithMessage;
import android.platform.test.annotations.Presubmit;
import org.junit.Test;
import java.util.Arrays;
import java.util.List;
import java.util.TimeZone;
@Presubmit
public class ZoneInfoDbTimeZoneIdValidatorTest {
private final LocationTimeZoneProvider.TimeZoneIdValidator mTzChecker =
new ZoneInfoDbTimeZoneIdValidator();
@Test
public void timeZoneIdsFromZoneInfoDbAreValid() {
for (String timeZone : TimeZone.getAvailableIDs()) {
assertWithMessage("Time zone %s should be supported", timeZone)
.that(mTzChecker.isValid(timeZone)).isTrue();
}
}
@Test
public void nonExistingZones_areNotSupported() {
List<String> nonExistingTimeZones = Arrays.asList(
"SystemV/HST10", "Atlantic/Atlantis", "EUROPE/LONDON", "Etc/GMT-5:30"
);
for (String timeZone : nonExistingTimeZones) {
assertWithMessage(timeZone + " is not a valid time zone")
.that(mTzChecker.isValid(timeZone))
.isFalse();
}
}
}