From 35f7d0a378abaf4d3948efc3f636e47a7a923fc9 Mon Sep 17 00:00:00 2001 From: Neil Fuller Date: Tue, 25 Oct 2022 18:06:33 +0100 Subject: [PATCH] Use ServiceConfigAccessor directly Architectural change: Make ServiceConfigAccessor a direct dependency of TimeZoneDetectorStrategyImpl. Previously, TimeZoneDetectorInternalImpl and TimeZoneDetectorService used ServiceConfigAccessor directly for reading / writing config, and TimeZoneDetectorStrategyImpl read it via it's "Environment" facade and relied on async updates to follow along. Upcoming changes that are likely to add more information to the cached status information that downstream components will be interested in (i.e. will want to listen to). This extra information will be held by TimeZoneDetectorStrategyImpl so it will be easier if it is (only) TimeZoneDetectorStrategyImpl that manages the ServiceConfigAccessor of the three. The dependency on ServiceConfigAccessor is removed from the classes higher in the stack (TimeZoneDetectorService / TimeZoneDetectorInternalImpl), which now call through to the Strategy. The ServiceConfigAccessor functionality has been removed from the Environment, and TimeZoneDetectorStrategyImpl uses ServiceConfigAccessor directly. Now that the TimeZoneDetectorStrategyImpl is in the direct config read / update flow for binder calls, some care has been taken to ensure the cached ConfigurationInternal is kept up to date. Previously, the TimeZoneDetectorStrategyImpl updates happened asynchronously. There's some complexity around caching / eventual consistency here that might be detectable but should not have any impact on behavior. Notes for people following along: We cannot assume that the only thing affecting config / status happens via TimeZoneDetectorStrategyImpl: Android's settings and device config are mutatable via all sorts of routes (e.g. command line, backup&restore, random partner apps). This is not likely to change. Therefore, the TimeZoneDetectorStrategyImpl still listens for ServiceConfigAccessor updates, but will (now) only forward them on if something changes Vs the last one it notified about. This commit changes the time zone strategy code. A follow-up will make an equivalent change for the time code at a later time for consistency. Bug: 253015306 Test: atest services/tests/servicestests/src/com/android/server/timedetector/ services/tests/servicestests/src/com/android/server/timezonedetector/ Test: atest ./tests/tests/time/src/android/app/time/cts/ Change-Id: Ie3fe60a624be8a5bb3e161a3e88f0a257cd96c3d --- .../server/timedetector/EnvironmentImpl.java | 8 +- .../server/timedetector/ServerFlags.java | 9 +- .../timedetector/ServiceConfigAccessor.java | 10 +- .../ServiceConfigAccessorImpl.java | 12 +- .../TimeDetectorStrategyImpl.java | 8 +- .../ConfigurationInternal.java | 39 +-- .../timezonedetector/EnvironmentImpl.java | 27 +- .../ServiceConfigAccessor.java | 13 +- .../ServiceConfigAccessorImpl.java | 24 +- ...Listener.java => StateChangeListener.java} | 6 +- .../TimeZoneDetectorInternalImpl.java | 10 +- .../TimeZoneDetectorService.java | 35 +-- .../TimeZoneDetectorStrategy.java | 44 +++ .../TimeZoneDetectorStrategyImpl.java | 161 +++++++--- ...ZoneProviderControllerEnvironmentImpl.java | 4 +- .../FakeServiceConfigAccessor.java | 10 +- .../TimeDetectorStrategyImplTest.java | 6 +- .../ConfigurationInternalTest.java | 60 ++-- .../FakeServiceConfigAccessor.java | 96 ++++-- .../FakeTimeZoneDetectorStrategy.java | 48 +++ .../MetricsTimeZoneDetectorStateTest.java | 3 +- .../server/timezonedetector/TestHandler.java | 8 +- .../TimeZoneDetectorInternalImplTest.java | 22 +- .../TimeZoneDetectorServiceTest.java | 20 +- .../TimeZoneDetectorStrategyImplTest.java | 288 ++++++++++++++++-- .../location/TestSupport.java | 3 +- 26 files changed, 683 insertions(+), 291 deletions(-) rename services/core/java/com/android/server/timezonedetector/{ConfigurationChangeListener.java => StateChangeListener.java} (78%) diff --git a/services/core/java/com/android/server/timedetector/EnvironmentImpl.java b/services/core/java/com/android/server/timedetector/EnvironmentImpl.java index 4972412472f1e..8d106f7d7fa41 100644 --- a/services/core/java/com/android/server/timedetector/EnvironmentImpl.java +++ b/services/core/java/com/android/server/timedetector/EnvironmentImpl.java @@ -28,7 +28,7 @@ import com.android.server.AlarmManagerInternal; import com.android.server.LocalServices; import com.android.server.SystemClockTime; import com.android.server.SystemClockTime.TimeConfidence; -import com.android.server.timezonedetector.ConfigurationChangeListener; +import com.android.server.timezonedetector.StateChangeListener; import java.io.PrintWriter; import java.util.Objects; @@ -60,10 +60,10 @@ final class EnvironmentImpl implements TimeDetectorStrategyImpl.Environment { @Override public void setConfigurationInternalChangeListener( - @NonNull ConfigurationChangeListener listener) { - ConfigurationChangeListener configurationChangeListener = + @NonNull StateChangeListener listener) { + StateChangeListener stateChangeListener = () -> mHandler.post(listener::onChange); - mServiceConfigAccessor.addConfigurationInternalChangeListener(configurationChangeListener); + mServiceConfigAccessor.addConfigurationInternalChangeListener(stateChangeListener); } @Override diff --git a/services/core/java/com/android/server/timedetector/ServerFlags.java b/services/core/java/com/android/server/timedetector/ServerFlags.java index 773b5175c954b..e9827ce33739e 100644 --- a/services/core/java/com/android/server/timedetector/ServerFlags.java +++ b/services/core/java/com/android/server/timedetector/ServerFlags.java @@ -25,8 +25,8 @@ import android.provider.DeviceConfig; import android.util.ArrayMap; import com.android.internal.annotations.GuardedBy; -import com.android.server.timezonedetector.ConfigurationChangeListener; import com.android.server.timezonedetector.ServiceConfigAccessor; +import com.android.server.timezonedetector.StateChangeListener; import java.lang.annotation.ElementType; import java.lang.annotation.Retention; @@ -185,8 +185,7 @@ public final class ServerFlags { * ensure O(1) lookup performance when working out whether a listener should trigger. */ @GuardedBy("mListeners") - private final ArrayMap> mListeners = - new ArrayMap<>(); + private final ArrayMap> mListeners = new ArrayMap<>(); private static final Object SLOCK = new Object(); @@ -213,7 +212,7 @@ public final class ServerFlags { private void handlePropertiesChanged(@NonNull DeviceConfig.Properties properties) { synchronized (mListeners) { - for (Map.Entry> listenerEntry + for (Map.Entry> listenerEntry : mListeners.entrySet()) { // It's unclear which set of the following two Sets is going to be larger in the // average case: monitoredKeys will be a subset of the set of possible keys, but @@ -249,7 +248,7 @@ public final class ServerFlags { *

Note: Only for use by long-lived objects like other singletons. There is deliberately no * associated remove method. */ - public void addListener(@NonNull ConfigurationChangeListener listener, + public void addListener(@NonNull StateChangeListener listener, @NonNull Set keys) { Objects.requireNonNull(listener); Objects.requireNonNull(keys); diff --git a/services/core/java/com/android/server/timedetector/ServiceConfigAccessor.java b/services/core/java/com/android/server/timedetector/ServiceConfigAccessor.java index a39f64c1f82ac..ff180eb908822 100644 --- a/services/core/java/com/android/server/timedetector/ServiceConfigAccessor.java +++ b/services/core/java/com/android/server/timedetector/ServiceConfigAccessor.java @@ -19,7 +19,7 @@ import android.annotation.NonNull; import android.annotation.UserIdInt; import android.app.time.TimeConfiguration; -import com.android.server.timezonedetector.ConfigurationChangeListener; +import com.android.server.timezonedetector.StateChangeListener; /** * An interface that provides access to service configuration for time detection. This hides @@ -33,18 +33,18 @@ public interface ServiceConfigAccessor { * Adds a listener that will be invoked when {@link ConfigurationInternal} may have changed. * The listener is invoked on the main thread. */ - void addConfigurationInternalChangeListener(@NonNull ConfigurationChangeListener listener); + void addConfigurationInternalChangeListener(@NonNull StateChangeListener listener); /** * Removes a listener previously added via {@link - * #addConfigurationInternalChangeListener(ConfigurationChangeListener)}. + * #addConfigurationInternalChangeListener(StateChangeListener)}. */ - void removeConfigurationInternalChangeListener(@NonNull ConfigurationChangeListener listener); + void removeConfigurationInternalChangeListener(@NonNull StateChangeListener listener); /** * Returns a snapshot of the {@link ConfigurationInternal} for the current user. This is only a * snapshot so callers must use {@link - * #addConfigurationInternalChangeListener(ConfigurationChangeListener)} to be notified when it + * #addConfigurationInternalChangeListener(StateChangeListener)} to be notified when it * changes. */ @NonNull diff --git a/services/core/java/com/android/server/timedetector/ServiceConfigAccessorImpl.java b/services/core/java/com/android/server/timedetector/ServiceConfigAccessorImpl.java index 71acf35bd3a21..4ef713c9b6452 100644 --- a/services/core/java/com/android/server/timedetector/ServiceConfigAccessorImpl.java +++ b/services/core/java/com/android/server/timedetector/ServiceConfigAccessorImpl.java @@ -49,7 +49,7 @@ import com.android.internal.annotations.GuardedBy; import com.android.internal.util.Preconditions; import com.android.server.LocalServices; import com.android.server.timedetector.TimeDetectorStrategy.Origin; -import com.android.server.timezonedetector.ConfigurationChangeListener; +import com.android.server.timezonedetector.StateChangeListener; import java.time.Instant; import java.util.ArrayList; @@ -104,8 +104,8 @@ final class ServiceConfigAccessorImpl implements ServiceConfigAccessor { @NonNull private final ServerFlagsOriginPrioritiesSupplier mServerFlagsOriginPrioritiesSupplier; @GuardedBy("this") - @NonNull private final List mConfigurationInternalListeners = - new ArrayList<>(); + @NonNull + private final List mConfigurationInternalListeners = new ArrayList<>(); /** * If a newly calculated system clock time and the current system clock time differs by this or @@ -167,20 +167,20 @@ final class ServiceConfigAccessorImpl implements ServiceConfigAccessor { } private synchronized void handleConfigurationInternalChangeOnMainThread() { - for (ConfigurationChangeListener changeListener : mConfigurationInternalListeners) { + for (StateChangeListener changeListener : mConfigurationInternalListeners) { changeListener.onChange(); } } @Override public synchronized void addConfigurationInternalChangeListener( - @NonNull ConfigurationChangeListener listener) { + @NonNull StateChangeListener listener) { mConfigurationInternalListeners.add(Objects.requireNonNull(listener)); } @Override public synchronized void removeConfigurationInternalChangeListener( - @NonNull ConfigurationChangeListener listener) { + @NonNull StateChangeListener listener) { mConfigurationInternalListeners.remove(Objects.requireNonNull(listener)); } diff --git a/services/core/java/com/android/server/timedetector/TimeDetectorStrategyImpl.java b/services/core/java/com/android/server/timedetector/TimeDetectorStrategyImpl.java index 3cee19cbe385a..13ec75329e39a 100644 --- a/services/core/java/com/android/server/timedetector/TimeDetectorStrategyImpl.java +++ b/services/core/java/com/android/server/timedetector/TimeDetectorStrategyImpl.java @@ -44,8 +44,8 @@ import com.android.internal.annotations.VisibleForTesting; import com.android.server.SystemClockTime; import com.android.server.SystemClockTime.TimeConfidence; import com.android.server.timezonedetector.ArrayMapWithHistory; -import com.android.server.timezonedetector.ConfigurationChangeListener; import com.android.server.timezonedetector.ReferenceWithHistory; +import com.android.server.timezonedetector.StateChangeListener; import java.io.PrintWriter; import java.time.Duration; @@ -136,11 +136,11 @@ public final class TimeDetectorStrategyImpl implements TimeDetectorStrategy { public interface Environment { /** - * Sets a {@link ConfigurationChangeListener} that will be invoked when there are any - * changes that could affect the content of {@link ConfigurationInternal}. + * Sets a {@link StateChangeListener} that will be invoked when there are any changes that + * could affect the content of {@link ConfigurationInternal}. * This is invoked during system server setup. */ - void setConfigurationInternalChangeListener(@NonNull ConfigurationChangeListener listener); + void setConfigurationInternalChangeListener(@NonNull StateChangeListener listener); /** Returns the {@link ConfigurationInternal} for the current user. */ @NonNull ConfigurationInternal getCurrentUserConfigurationInternal(); diff --git a/services/core/java/com/android/server/timezonedetector/ConfigurationInternal.java b/services/core/java/com/android/server/timezonedetector/ConfigurationInternal.java index 8e2a5f4725586..0409a8416dc3b 100644 --- a/services/core/java/com/android/server/timezonedetector/ConfigurationInternal.java +++ b/services/core/java/com/android/server/timezonedetector/ConfigurationInternal.java @@ -26,7 +26,6 @@ import android.annotation.NonNull; import android.annotation.UserIdInt; import android.app.time.Capabilities.CapabilityState; import android.app.time.TimeZoneCapabilities; -import android.app.time.TimeZoneCapabilitiesAndConfig; import android.app.time.TimeZoneConfiguration; import android.os.UserHandle; @@ -75,7 +74,7 @@ public final class ConfigurationInternal { mEnhancedMetricsCollectionEnabled = builder.mEnhancedMetricsCollectionEnabled; mAutoDetectionEnabledSetting = builder.mAutoDetectionEnabledSetting; - mUserId = builder.mUserId; + mUserId = Objects.requireNonNull(builder.mUserId, "userId must be set"); mUserConfigAllowed = builder.mUserConfigAllowed; mLocationEnabledSetting = builder.mLocationEnabledSetting; mGeoDetectionEnabledSetting = builder.mGeoDetectionEnabledSetting; @@ -151,8 +150,7 @@ public final class ConfigurationInternal { * Returns true if the user is allowed to modify time zone configuration, e.g. can be false due * to device policy (enterprise). * - *

See also {@link #createCapabilitiesAndConfig(boolean)} for situations where this value - * are ignored. + *

See also {@link #asCapabilities(boolean)} for situations where this value is ignored. */ public boolean isUserConfigAllowed() { return mUserConfigAllowed; @@ -196,20 +194,8 @@ public final class ConfigurationInternal { || getGeoDetectionRunInBackgroundEnabled()); } - /** - * Creates a {@link TimeZoneCapabilitiesAndConfig} object using the configuration values. - * - * @param bypassUserPolicyChecks {@code true} for device policy manager use cases where device - * policy restrictions that should apply to actual users can be ignored - */ - public TimeZoneCapabilitiesAndConfig createCapabilitiesAndConfig( - boolean bypassUserPolicyChecks) { - return new TimeZoneCapabilitiesAndConfig( - asCapabilities(bypassUserPolicyChecks), asConfiguration()); - } - @NonNull - private TimeZoneCapabilities asCapabilities(boolean bypassUserPolicyChecks) { + public TimeZoneCapabilities asCapabilities(boolean bypassUserPolicyChecks) { UserHandle userHandle = UserHandle.of(mUserId); TimeZoneCapabilities.Builder builder = new TimeZoneCapabilities.Builder(userHandle); @@ -262,7 +248,7 @@ public final class ConfigurationInternal { } /** Returns a {@link TimeZoneConfiguration} from the configuration values. */ - private TimeZoneConfiguration asConfiguration() { + public TimeZoneConfiguration asConfiguration() { return new TimeZoneConfiguration.Builder() .setAutoDetectionEnabled(getAutoDetectionEnabledSetting()) .setGeoDetectionEnabled(getGeoDetectionEnabledSetting()) @@ -335,8 +321,7 @@ public final class ConfigurationInternal { */ public static class Builder { - private final @UserIdInt int mUserId; - + private @UserIdInt Integer mUserId; private boolean mUserConfigAllowed; private boolean mTelephonyDetectionSupported; private boolean mGeoDetectionSupported; @@ -348,11 +333,9 @@ public final class ConfigurationInternal { private boolean mGeoDetectionEnabledSetting; /** - * Creates a new Builder with only the userId set. + * Creates a new Builder. */ - public Builder(@UserIdInt int userId) { - mUserId = userId; - } + public Builder() {} /** * Creates a new Builder by copying values from an existing instance. @@ -370,6 +353,14 @@ public final class ConfigurationInternal { this.mGeoDetectionEnabledSetting = toCopy.mGeoDetectionEnabledSetting; } + /** + * Sets the user ID the configuration is for. + */ + public Builder setUserId(@UserIdInt int userId) { + mUserId = userId; + return this; + } + /** * Sets whether the user is allowed to configure time zone settings on this device. */ diff --git a/services/core/java/com/android/server/timezonedetector/EnvironmentImpl.java b/services/core/java/com/android/server/timezonedetector/EnvironmentImpl.java index 4749f73d89e16..5cb48c2756be7 100644 --- a/services/core/java/com/android/server/timezonedetector/EnvironmentImpl.java +++ b/services/core/java/com/android/server/timezonedetector/EnvironmentImpl.java @@ -18,8 +18,6 @@ package com.android.server.timezonedetector; import android.annotation.ElapsedRealtimeLong; import android.annotation.NonNull; -import android.content.Context; -import android.os.Handler; import android.os.SystemClock; import android.os.SystemProperties; @@ -29,7 +27,6 @@ import com.android.server.SystemTimeZone; import com.android.server.SystemTimeZone.TimeZoneConfidence; import java.io.PrintWriter; -import java.util.Objects; /** * The real implementation of {@link TimeZoneDetectorStrategyImpl.Environment}. @@ -38,29 +35,7 @@ final class EnvironmentImpl implements TimeZoneDetectorStrategyImpl.Environment private static final String TIMEZONE_PROPERTY = "persist.sys.timezone"; - @NonNull private final Context mContext; - @NonNull private final Handler mHandler; - @NonNull private final ServiceConfigAccessor mServiceConfigAccessor; - - EnvironmentImpl(@NonNull Context context, @NonNull Handler handler, - @NonNull ServiceConfigAccessor serviceConfigAccessor) { - mContext = Objects.requireNonNull(context); - mHandler = Objects.requireNonNull(handler); - mServiceConfigAccessor = Objects.requireNonNull(serviceConfigAccessor); - } - - @Override - public void setConfigurationInternalChangeListener( - @NonNull ConfigurationChangeListener listener) { - ConfigurationChangeListener configurationChangeListener = - () -> mHandler.post(listener::onChange); - mServiceConfigAccessor.addConfigurationInternalChangeListener(configurationChangeListener); - } - - @Override - @NonNull - public ConfigurationInternal getCurrentUserConfigurationInternal() { - return mServiceConfigAccessor.getCurrentUserConfigurationInternal(); + EnvironmentImpl() { } @Override diff --git a/services/core/java/com/android/server/timezonedetector/ServiceConfigAccessor.java b/services/core/java/com/android/server/timezonedetector/ServiceConfigAccessor.java index 8da5d6aefdd66..4ac2ba5a89bba 100644 --- a/services/core/java/com/android/server/timezonedetector/ServiceConfigAccessor.java +++ b/services/core/java/com/android/server/timezonedetector/ServiceConfigAccessor.java @@ -59,20 +59,18 @@ public interface ServiceConfigAccessor { * Adds a listener that will be invoked when {@link ConfigurationInternal} may have changed. * The listener is invoked on the main thread. */ - void addConfigurationInternalChangeListener( - @NonNull ConfigurationChangeListener listener); + void addConfigurationInternalChangeListener(@NonNull StateChangeListener listener); /** * Removes a listener previously added via {@link - * #addConfigurationInternalChangeListener(ConfigurationChangeListener)}. + * #addConfigurationInternalChangeListener(StateChangeListener)}. */ - void removeConfigurationInternalChangeListener( - @NonNull ConfigurationChangeListener listener); + void removeConfigurationInternalChangeListener(@NonNull StateChangeListener listener); /** * Returns a snapshot of the {@link ConfigurationInternal} for the current user. This is only a * snapshot so callers must use {@link - * #addConfigurationInternalChangeListener(ConfigurationChangeListener)} to be notified when it + * #addConfigurationInternalChangeListener(StateChangeListener)} to be notified when it * changes. */ @NonNull @@ -104,8 +102,7 @@ public interface ServiceConfigAccessor { * *

Note: Currently only for use by long-lived objects; there is no associated remove method. */ - void addLocationTimeZoneManagerConfigListener( - @NonNull ConfigurationChangeListener listener); + void addLocationTimeZoneManagerConfigListener(@NonNull StateChangeListener listener); /** * Returns {@code true} if the telephony-based time zone detection feature is supported on the diff --git a/services/core/java/com/android/server/timezonedetector/ServiceConfigAccessorImpl.java b/services/core/java/com/android/server/timezonedetector/ServiceConfigAccessorImpl.java index e2f42467c5501..dfb961938273c 100644 --- a/services/core/java/com/android/server/timezonedetector/ServiceConfigAccessorImpl.java +++ b/services/core/java/com/android/server/timezonedetector/ServiceConfigAccessorImpl.java @@ -22,7 +22,6 @@ import android.annotation.Nullable; import android.annotation.UserIdInt; import android.app.ActivityManagerInternal; import android.app.time.TimeZoneCapabilities; -import android.app.time.TimeZoneCapabilitiesAndConfig; import android.app.time.TimeZoneConfiguration; import android.content.BroadcastReceiver; import android.content.ContentResolver; @@ -104,8 +103,8 @@ public final class ServiceConfigAccessorImpl implements ServiceConfigAccessor { @NonNull private final LocationManager mLocationManager; @GuardedBy("this") - @NonNull private final List mConfigurationInternalListeners = - new ArrayList<>(); + @NonNull + private final List mConfigurationInternalListeners = new ArrayList<>(); /** * The mode to use for the primary location time zone provider in a test. Setting this @@ -207,20 +206,20 @@ public final class ServiceConfigAccessorImpl implements ServiceConfigAccessor { } private synchronized void handleConfigurationInternalChangeOnMainThread() { - for (ConfigurationChangeListener changeListener : mConfigurationInternalListeners) { + for (StateChangeListener changeListener : mConfigurationInternalListeners) { changeListener.onChange(); } } @Override public synchronized void addConfigurationInternalChangeListener( - @NonNull ConfigurationChangeListener listener) { + @NonNull StateChangeListener listener) { mConfigurationInternalListeners.add(Objects.requireNonNull(listener)); } @Override public synchronized void removeConfigurationInternalChangeListener( - @NonNull ConfigurationChangeListener listener) { + @NonNull StateChangeListener listener) { mConfigurationInternalListeners.remove(Objects.requireNonNull(listener)); } @@ -237,10 +236,10 @@ public final class ServiceConfigAccessorImpl implements ServiceConfigAccessor { @NonNull TimeZoneConfiguration requestedConfiguration, boolean bypassUserPolicyChecks) { Objects.requireNonNull(requestedConfiguration); - TimeZoneCapabilitiesAndConfig capabilitiesAndConfig = getConfigurationInternal(userId) - .createCapabilitiesAndConfig(bypassUserPolicyChecks); - TimeZoneCapabilities capabilities = capabilitiesAndConfig.getCapabilities(); - TimeZoneConfiguration oldConfiguration = capabilitiesAndConfig.getConfiguration(); + ConfigurationInternal configurationInternal = getConfigurationInternal(userId); + TimeZoneCapabilities capabilities = + configurationInternal.asCapabilities(bypassUserPolicyChecks); + TimeZoneConfiguration oldConfiguration = configurationInternal.asConfiguration(); final TimeZoneConfiguration newConfiguration = capabilities.tryApplyConfigChanges(oldConfiguration, requestedConfiguration); @@ -292,7 +291,8 @@ public final class ServiceConfigAccessorImpl implements ServiceConfigAccessor { @Override @NonNull public synchronized ConfigurationInternal getConfigurationInternal(@UserIdInt int userId) { - return new ConfigurationInternal.Builder(userId) + return new ConfigurationInternal.Builder() + .setUserId(userId) .setTelephonyDetectionFeatureSupported( isTelephonyTimeZoneDetectionFeatureSupported()) .setGeoDetectionFeatureSupported(isGeoTimeZoneDetectionFeatureSupported()) @@ -354,7 +354,7 @@ public final class ServiceConfigAccessorImpl implements ServiceConfigAccessor { @Override public void addLocationTimeZoneManagerConfigListener( - @NonNull ConfigurationChangeListener listener) { + @NonNull StateChangeListener listener) { mServerFlags.addListener(listener, LOCATION_TIME_ZONE_MANAGER_SERVER_FLAGS_KEYS_TO_WATCH); } diff --git a/services/core/java/com/android/server/timezonedetector/ConfigurationChangeListener.java b/services/core/java/com/android/server/timezonedetector/StateChangeListener.java similarity index 78% rename from services/core/java/com/android/server/timezonedetector/ConfigurationChangeListener.java rename to services/core/java/com/android/server/timezonedetector/StateChangeListener.java index aa8ad37815bfe..2b5639c1fd779 100644 --- a/services/core/java/com/android/server/timezonedetector/ConfigurationChangeListener.java +++ b/services/core/java/com/android/server/timezonedetector/StateChangeListener.java @@ -17,11 +17,11 @@ package com.android.server.timezonedetector; /** - * A listener used to receive notification that configuration has / may have changed (depending on + * A listener used to receive notification that state has / may have changed (depending on * the usecase). */ @FunctionalInterface -public interface ConfigurationChangeListener { - /** Called when the configuration may have changed. */ +public interface StateChangeListener { + /** Called when something (may have) changed. */ void onChange(); } diff --git a/services/core/java/com/android/server/timezonedetector/TimeZoneDetectorInternalImpl.java b/services/core/java/com/android/server/timezonedetector/TimeZoneDetectorInternalImpl.java index ce64eaccd98f7..dfb44df7b993f 100644 --- a/services/core/java/com/android/server/timezonedetector/TimeZoneDetectorInternalImpl.java +++ b/services/core/java/com/android/server/timezonedetector/TimeZoneDetectorInternalImpl.java @@ -35,17 +35,14 @@ public final class TimeZoneDetectorInternalImpl implements TimeZoneDetectorInter @NonNull private final Context mContext; @NonNull private final Handler mHandler; @NonNull private final CurrentUserIdentityInjector mCurrentUserIdentityInjector; - @NonNull private final ServiceConfigAccessor mServiceConfigAccessor; @NonNull private final TimeZoneDetectorStrategy mTimeZoneDetectorStrategy; public TimeZoneDetectorInternalImpl(@NonNull Context context, @NonNull Handler handler, @NonNull CurrentUserIdentityInjector currentUserIdentityInjector, - @NonNull ServiceConfigAccessor serviceConfigAccessor, @NonNull TimeZoneDetectorStrategy timeZoneDetectorStrategy) { mContext = Objects.requireNonNull(context); mHandler = Objects.requireNonNull(handler); mCurrentUserIdentityInjector = Objects.requireNonNull(currentUserIdentityInjector); - mServiceConfigAccessor = Objects.requireNonNull(serviceConfigAccessor); mTimeZoneDetectorStrategy = Objects.requireNonNull(timeZoneDetectorStrategy); } @@ -53,10 +50,9 @@ public final class TimeZoneDetectorInternalImpl implements TimeZoneDetectorInter @NonNull public TimeZoneCapabilitiesAndConfig getCapabilitiesAndConfigForDpm() { int currentUserId = mCurrentUserIdentityInjector.getCurrentUserId(); - ConfigurationInternal configurationInternal = - mServiceConfigAccessor.getConfigurationInternal(currentUserId); final boolean bypassUserPolicyChecks = true; - return configurationInternal.createCapabilitiesAndConfig(bypassUserPolicyChecks); + return mTimeZoneDetectorStrategy.getCapabilitiesAndConfig( + currentUserId, bypassUserPolicyChecks); } @Override @@ -65,7 +61,7 @@ public final class TimeZoneDetectorInternalImpl implements TimeZoneDetectorInter int currentUserId = mCurrentUserIdentityInjector.getCurrentUserId(); final boolean bypassUserPolicyChecks = true; - return mServiceConfigAccessor.updateConfiguration( + return mTimeZoneDetectorStrategy.updateConfiguration( currentUserId, configuration, bypassUserPolicyChecks); } diff --git a/services/core/java/com/android/server/timezonedetector/TimeZoneDetectorService.java b/services/core/java/com/android/server/timezonedetector/TimeZoneDetectorService.java index 13f169461511f..f415cf03fdecc 100644 --- a/services/core/java/com/android/server/timezonedetector/TimeZoneDetectorService.java +++ b/services/core/java/com/android/server/timezonedetector/TimeZoneDetectorService.java @@ -83,7 +83,7 @@ public final class TimeZoneDetectorService extends ITimeZoneDetectorService.Stub ServiceConfigAccessor serviceConfigAccessor = ServiceConfigAccessorImpl.getInstance(context); TimeZoneDetectorStrategy timeZoneDetectorStrategy = - TimeZoneDetectorStrategyImpl.create(context, handler, serviceConfigAccessor); + TimeZoneDetectorStrategyImpl.create(handler, serviceConfigAccessor); DeviceActivityMonitor deviceActivityMonitor = DeviceActivityMonitorImpl.create(context, handler); @@ -99,16 +99,14 @@ public final class TimeZoneDetectorService extends ITimeZoneDetectorService.Stub CurrentUserIdentityInjector currentUserIdentityInjector = CurrentUserIdentityInjector.REAL; TimeZoneDetectorInternal internal = new TimeZoneDetectorInternalImpl( - context, handler, currentUserIdentityInjector, serviceConfigAccessor, - timeZoneDetectorStrategy); + context, handler, currentUserIdentityInjector, timeZoneDetectorStrategy); publishLocalService(TimeZoneDetectorInternal.class, internal); // Publish the binder service so it can be accessed from other (appropriately // permissioned) processes. CallerIdentityInjector callerIdentityInjector = CallerIdentityInjector.REAL; TimeZoneDetectorService service = new TimeZoneDetectorService( - context, handler, callerIdentityInjector, serviceConfigAccessor, - timeZoneDetectorStrategy); + context, handler, callerIdentityInjector, timeZoneDetectorStrategy); // Dump the device activity monitor when the service is dumped. service.addDumpable(deviceActivityMonitor); @@ -126,9 +124,6 @@ public final class TimeZoneDetectorService extends ITimeZoneDetectorService.Stub @NonNull private final CallerIdentityInjector mCallerIdentityInjector; - @NonNull - private final ServiceConfigAccessor mServiceConfigAccessor; - @NonNull private final TimeZoneDetectorStrategy mTimeZoneDetectorStrategy; @@ -150,18 +145,16 @@ public final class TimeZoneDetectorService extends ITimeZoneDetectorService.Stub @VisibleForTesting public TimeZoneDetectorService(@NonNull Context context, @NonNull Handler handler, @NonNull CallerIdentityInjector callerIdentityInjector, - @NonNull ServiceConfigAccessor serviceConfigAccessor, @NonNull TimeZoneDetectorStrategy timeZoneDetectorStrategy) { mContext = Objects.requireNonNull(context); mHandler = Objects.requireNonNull(handler); mCallerIdentityInjector = Objects.requireNonNull(callerIdentityInjector); - mServiceConfigAccessor = Objects.requireNonNull(serviceConfigAccessor); mTimeZoneDetectorStrategy = Objects.requireNonNull(timeZoneDetectorStrategy); // Wire up a change listener so that ITimeZoneDetectorListeners can be notified when - // the configuration changes for any reason. - mServiceConfigAccessor.addConfigurationInternalChangeListener( - () -> mHandler.post(this::handleConfigurationInternalChangedOnHandlerThread)); + // the detector state changes for any reason. + mTimeZoneDetectorStrategy.addChangeListener( + () -> mHandler.post(this::handleChangeOnHandlerThread)); } @Override @@ -174,12 +167,15 @@ public final class TimeZoneDetectorService extends ITimeZoneDetectorService.Stub TimeZoneCapabilitiesAndConfig getCapabilitiesAndConfig(@UserIdInt int userId) { enforceManageTimeZoneDetectorPermission(); + // Resolve constants like USER_CURRENT to the true user ID as needed. + int resolvedUserId = ActivityManager.handleIncomingUser(Binder.getCallingPid(), + Binder.getCallingUid(), userId, false, false, "getCapabilitiesAndConfig", null); + final long token = mCallerIdentityInjector.clearCallingIdentity(); try { - ConfigurationInternal configurationInternal = - mServiceConfigAccessor.getConfigurationInternal(userId); final boolean bypassUserPolicyChecks = false; - return configurationInternal.createCapabilitiesAndConfig(bypassUserPolicyChecks); + return mTimeZoneDetectorStrategy.getCapabilitiesAndConfig( + resolvedUserId, bypassUserPolicyChecks); } finally { mCallerIdentityInjector.restoreCallingIdentity(token); } @@ -204,7 +200,7 @@ public final class TimeZoneDetectorService extends ITimeZoneDetectorService.Stub final long token = mCallerIdentityInjector.clearCallingIdentity(); try { final boolean bypassUserPolicyChecks = false; - return mServiceConfigAccessor.updateConfiguration( + return mTimeZoneDetectorStrategy.updateConfiguration( resolvedUserId, configuration, bypassUserPolicyChecks); } finally { mCallerIdentityInjector.restoreCallingIdentity(token); @@ -285,8 +281,9 @@ public final class TimeZoneDetectorService extends ITimeZoneDetectorService.Stub } } - void handleConfigurationInternalChangedOnHandlerThread() { - // Configuration has changed, but each user may have a different view of the configuration. + void handleChangeOnHandlerThread() { + // Detector state has changed. Each user may have a different view of the configuration so + // no information is passed; each client must query what they're interested in. // It's possible that this will cause unnecessary notifications but that shouldn't be a // problem. synchronized (mListeners) { diff --git a/services/core/java/com/android/server/timezonedetector/TimeZoneDetectorStrategy.java b/services/core/java/com/android/server/timezonedetector/TimeZoneDetectorStrategy.java index 69284e3226630..328cf72b262eb 100644 --- a/services/core/java/com/android/server/timezonedetector/TimeZoneDetectorStrategy.java +++ b/services/core/java/com/android/server/timezonedetector/TimeZoneDetectorStrategy.java @@ -17,6 +17,8 @@ package com.android.server.timezonedetector; import android.annotation.NonNull; import android.annotation.UserIdInt; +import android.app.time.TimeZoneCapabilitiesAndConfig; +import android.app.time.TimeZoneConfiguration; import android.app.time.TimeZoneState; import android.app.timezonedetector.ManualTimeZoneSuggestion; import android.app.timezonedetector.TelephonyTimeZoneSuggestion; @@ -94,6 +96,48 @@ import android.util.IndentingPrintWriter; */ public interface TimeZoneDetectorStrategy extends Dumpable { + /** + * Adds a listener that will be triggered when something changes that could affect the result + * of the {@link #getCapabilitiesAndConfig} call for the current user only. This + * includes the current user changing. This is exposed so that (indirect) users like SettingsUI + * can monitor for changes to data derived from {@link TimeZoneCapabilitiesAndConfig} and update + * the UI accordingly. + */ + void addChangeListener(StateChangeListener listener); + + /** + * Returns a {@link TimeZoneCapabilitiesAndConfig} object for the specified user. + * + *

The strategy is dependent on device state like current user, settings and device config. + * These updates are usually handled asynchronously, so callers should expect some delay between + * a change being made directly to services like settings and the strategy becoming aware of + * them. Changes made via {@link #updateConfiguration} will be visible immediately. + * + * @param userId the user ID to retrieve the information for + * @param bypassUserPolicyChecks {@code true} for device policy manager use cases where device + * policy restrictions that should apply to actual users can be ignored + */ + TimeZoneCapabilitiesAndConfig getCapabilitiesAndConfig( + @UserIdInt int userId, boolean bypassUserPolicyChecks); + + /** + * Updates the configuration properties that control a device's time zone behavior. + * + *

This method returns {@code true} if the configuration was changed, {@code false} + * otherwise. + * + *

See {@link #getCapabilitiesAndConfig} for guarantees about visibility of updates to + * subsequent calls. + * + * @param userId the current user ID, supplied to make sure that the asynchronous process + * that happens when users switch is completed when the call is made + * @param configuration the configuration changes + * @param bypassUserPolicyChecks {@code true} for device policy manager use cases where device + * policy restrictions that should apply to actual users can be ignored + */ + boolean updateConfiguration(@UserIdInt int userId, TimeZoneConfiguration configuration, + boolean bypassUserPolicyChecks); + /** Returns a snapshot of the system time zone state. See {@link TimeZoneState} for details. */ @NonNull TimeZoneState getTimeZoneState(); diff --git a/services/core/java/com/android/server/timezonedetector/TimeZoneDetectorStrategyImpl.java b/services/core/java/com/android/server/timezonedetector/TimeZoneDetectorStrategyImpl.java index 18c8885ee0593..ecf25e9c157c8 100644 --- a/services/core/java/com/android/server/timezonedetector/TimeZoneDetectorStrategyImpl.java +++ b/services/core/java/com/android/server/timezonedetector/TimeZoneDetectorStrategyImpl.java @@ -31,10 +31,10 @@ import android.annotation.Nullable; import android.annotation.UserIdInt; import android.app.time.TimeZoneCapabilities; import android.app.time.TimeZoneCapabilitiesAndConfig; +import android.app.time.TimeZoneConfiguration; import android.app.time.TimeZoneState; import android.app.timezonedetector.ManualTimeZoneSuggestion; import android.app.timezonedetector.TelephonyTimeZoneSuggestion; -import android.content.Context; import android.os.Handler; import android.os.TimestampedValue; import android.util.IndentingPrintWriter; @@ -46,6 +46,7 @@ import com.android.server.SystemTimeZone.TimeZoneConfidence; import java.io.PrintWriter; import java.time.Duration; +import java.util.ArrayList; import java.util.List; import java.util.Objects; @@ -67,16 +68,6 @@ public final class TimeZoneDetectorStrategyImpl implements TimeZoneDetectorStrat @VisibleForTesting public interface Environment { - /** - * Sets a {@link ConfigurationChangeListener} that will be invoked when there are any - * changes that could affect the content of {@link ConfigurationInternal}. - * This is invoked during system server setup. - */ - void setConfigurationInternalChangeListener(@NonNull ConfigurationChangeListener listener); - - /** Returns the {@link ConfigurationInternal} for the current user. */ - @NonNull ConfigurationInternal getCurrentUserConfigurationInternal(); - /** * Returns the device's currently configured time zone. May return an empty string. */ @@ -206,6 +197,22 @@ public final class TimeZoneDetectorStrategyImpl implements TimeZoneDetectorStrat private final ReferenceWithHistory mLatestManualSuggestion = new ReferenceWithHistory<>(KEEP_SUGGESTION_HISTORY_SIZE); + @NonNull + private final ServiceConfigAccessor mServiceConfigAccessor; + + /** The handler used for asynchronous operations triggered by this. */ + @NonNull + private final Handler mStateChangeHandler; + + @GuardedBy("this") + @NonNull private final List mStateChangeListeners = new ArrayList<>(); + + /** + * A snapshot of the current user's {@link ConfigurationInternal}. A local copy is cached + * because it is relatively heavyweight to obtain and is used more often than it is expected to + * change. Because many operations are asynchronous, this value may be out of date but should + * be "eventually consistent". + */ @GuardedBy("this") @NonNull private ConfigurationInternal mCurrentConfigurationInternal; @@ -229,28 +236,92 @@ public final class TimeZoneDetectorStrategyImpl implements TimeZoneDetectorStrat * Creates a new instance of {@link TimeZoneDetectorStrategyImpl}. */ public static TimeZoneDetectorStrategyImpl create( - @NonNull Context context, @NonNull Handler handler, - @NonNull ServiceConfigAccessor serviceConfigAccessor) { + @NonNull Handler handler, @NonNull ServiceConfigAccessor serviceConfigAccessor) { - Environment environment = new EnvironmentImpl(context, handler, serviceConfigAccessor); - return new TimeZoneDetectorStrategyImpl(environment); + Environment environment = new EnvironmentImpl(); + return new TimeZoneDetectorStrategyImpl(serviceConfigAccessor, handler, environment); } @VisibleForTesting - public TimeZoneDetectorStrategyImpl(@NonNull Environment environment) { + public TimeZoneDetectorStrategyImpl( + @NonNull ServiceConfigAccessor serviceConfigAccessor, + @NonNull Handler handler, @NonNull Environment environment) { mEnvironment = Objects.requireNonNull(environment); + mServiceConfigAccessor = Objects.requireNonNull(serviceConfigAccessor); + mStateChangeHandler = Objects.requireNonNull(handler); // Start with telephony fallback enabled. mTelephonyTimeZoneFallbackEnabled = new TimestampedValue<>(mEnvironment.elapsedRealtimeMillis(), true); synchronized (this) { - mEnvironment.setConfigurationInternalChangeListener( - this::handleConfigurationInternalChanged); - mCurrentConfigurationInternal = mEnvironment.getCurrentUserConfigurationInternal(); + // Listen for config and user changes and get an initial snapshot of configuration. + StateChangeListener stateChangeListener = this::handleConfigurationInternalMaybeChanged; + mServiceConfigAccessor.addConfigurationInternalChangeListener(stateChangeListener); + mCurrentConfigurationInternal = + mServiceConfigAccessor.getCurrentUserConfigurationInternal(); } } + @Override + public synchronized TimeZoneCapabilitiesAndConfig getCapabilitiesAndConfig( + @UserIdInt int userId, boolean bypassUserPolicyChecks) { + ConfigurationInternal configurationInternal; + if (mCurrentConfigurationInternal.getUserId() == userId) { + // Use the cached snapshot we have. + configurationInternal = mCurrentConfigurationInternal; + } else { + // This is not a common case: It would be unusual to want the configuration for a user + // other than the "current" user, but it is supported because it is trivial to do so. + // Unlike the current user config, there's no cached copy to worry about so read it + // directly from mServiceConfigAccessor. + configurationInternal = mServiceConfigAccessor.getConfigurationInternal(userId); + } + return new TimeZoneCapabilitiesAndConfig( + configurationInternal.asCapabilities(bypassUserPolicyChecks), + configurationInternal.asConfiguration()); + } + + @Override + public synchronized boolean updateConfiguration( + @UserIdInt int userId, @NonNull TimeZoneConfiguration configuration, + boolean bypassUserPolicyChecks) { + + // Write-through + boolean updateSuccessful = mServiceConfigAccessor.updateConfiguration( + userId, configuration, bypassUserPolicyChecks); + + // The update above will trigger config update listeners asynchronously if they are needed, + // but that could mean an immediate call to getCapabilitiesAndConfig() for the current user + // wouldn't see the update. So, handle the cache update and notifications here. When the + // async update listener triggers it will find everything already up to date and do nothing. + if (updateSuccessful && mCurrentConfigurationInternal.getUserId() == userId) { + ConfigurationInternal configurationInternal = + mServiceConfigAccessor.getConfigurationInternal(userId); + + // If the configuration actually changed, update the cached copy synchronously and do + // other necessary house-keeping / (async) listener notifications. + if (!configurationInternal.equals(mCurrentConfigurationInternal)) { + mCurrentConfigurationInternal = configurationInternal; + + String logMsg = "updateConfiguration:" + + " userId=" + userId + + ", configuration=" + configuration + + ", bypassUserPolicyChecks=" + bypassUserPolicyChecks + + ", mCurrentConfigurationInternal=" + mCurrentConfigurationInternal; + logTimeZoneDebugInfo(logMsg); + + handleConfigurationInternalChanged(logMsg); + } + } + return updateSuccessful; + } + + @Override + public synchronized void addChangeListener(StateChangeListener listener) { + mStateChangeListeners.add(listener); + } + @Override public synchronized boolean confirmTimeZone(@NonNull String timeZoneId) { Objects.requireNonNull(timeZoneId); @@ -334,9 +405,8 @@ public final class TimeZoneDetectorStrategyImpl implements TimeZoneDetectorStrat String timeZoneId = suggestion.getZoneId(); String cause = "Manual time suggestion received: suggestion=" + suggestion; - TimeZoneCapabilitiesAndConfig capabilitiesAndConfig = - currentUserConfig.createCapabilitiesAndConfig(bypassUserPolicyChecks); - TimeZoneCapabilities capabilities = capabilitiesAndConfig.getCapabilities(); + TimeZoneCapabilities capabilities = + currentUserConfig.asCapabilities(bypassUserPolicyChecks); if (capabilities.getSetManualTimeZoneCapability() != CAPABILITY_POSSESSED) { Slog.i(LOG_TAG, "User does not have the capability needed to set the time zone manually" + ": capabilities=" + capabilities @@ -735,18 +805,37 @@ public final class TimeZoneDetectorStrategyImpl implements TimeZoneDetectorStrat return findBestTelephonySuggestion(); } - private synchronized void handleConfigurationInternalChanged() { + /** + * Handles a configuration change notification. + */ + private synchronized void handleConfigurationInternalMaybeChanged() { ConfigurationInternal currentUserConfig = - mEnvironment.getCurrentUserConfigurationInternal(); - String logMsg = "handleConfigurationInternalChanged:" - + " oldConfiguration=" + mCurrentConfigurationInternal - + ", newConfiguration=" + currentUserConfig; - logTimeZoneDebugInfo(logMsg); - mCurrentConfigurationInternal = currentUserConfig; + mServiceConfigAccessor.getCurrentUserConfigurationInternal(); - // The configuration change may have changed available suggestions or the way suggestions - // are used, so re-run detection. - doAutoTimeZoneDetection(currentUserConfig, logMsg); + // The configuration may not actually have changed so check before doing anything. + if (!currentUserConfig.equals(mCurrentConfigurationInternal)) { + String logMsg = "handleConfigurationInternalMaybeChanged:" + + " oldConfiguration=" + mCurrentConfigurationInternal + + ", newConfiguration=" + currentUserConfig; + logTimeZoneDebugInfo(logMsg); + + mCurrentConfigurationInternal = currentUserConfig; + + handleConfigurationInternalChanged(logMsg); + } + } + + /** House-keeping that needs to be done when the mCurrentConfigurationInternal has changed. */ + @GuardedBy("this") + private void handleConfigurationInternalChanged(@NonNull String logMsg) { + // Notify change listeners asynchronously. + for (StateChangeListener listener : mStateChangeListeners) { + mStateChangeHandler.post(listener::onChange); + } + + // The configuration change may have changed available suggestions or the way + // suggestions are used, so re-run detection. + doAutoTimeZoneDetection(mCurrentConfigurationInternal, logMsg); } /** @@ -760,8 +849,7 @@ public final class TimeZoneDetectorStrategyImpl implements TimeZoneDetectorStrat ipw.println("mCurrentConfigurationInternal=" + mCurrentConfigurationInternal); final boolean bypassUserPolicyChecks = false; ipw.println("[Capabilities=" - + mCurrentConfigurationInternal.createCapabilitiesAndConfig(bypassUserPolicyChecks) - + "]"); + + mCurrentConfigurationInternal.asCapabilities(bypassUserPolicyChecks) + "]"); ipw.println("mEnvironment.getDeviceTimeZone()=" + mEnvironment.getDeviceTimeZone()); ipw.println("mEnvironment.getDeviceTimeZoneConfidence()=" + mEnvironment.getDeviceTimeZoneConfidence()); @@ -824,6 +912,11 @@ public final class TimeZoneDetectorStrategyImpl implements TimeZoneDetectorStrat return mTelephonyTimeZoneFallbackEnabled.getValue(); } + @VisibleForTesting + public synchronized ConfigurationInternal getCachedCapabilitiesAndConfigForTests() { + return mCurrentConfigurationInternal; + } + /** * A {@link TelephonyTimeZoneSuggestion} with additional qualifying metadata. */ diff --git a/services/core/java/com/android/server/timezonedetector/location/LocationTimeZoneProviderControllerEnvironmentImpl.java b/services/core/java/com/android/server/timezonedetector/location/LocationTimeZoneProviderControllerEnvironmentImpl.java index e7d16c85b1c6c..5eeafc1e70d0f 100644 --- a/services/core/java/com/android/server/timezonedetector/location/LocationTimeZoneProviderControllerEnvironmentImpl.java +++ b/services/core/java/com/android/server/timezonedetector/location/LocationTimeZoneProviderControllerEnvironmentImpl.java @@ -20,9 +20,9 @@ import android.annotation.ElapsedRealtimeLong; import android.annotation.NonNull; import android.os.SystemClock; -import com.android.server.timezonedetector.ConfigurationChangeListener; import com.android.server.timezonedetector.ConfigurationInternal; import com.android.server.timezonedetector.ServiceConfigAccessor; +import com.android.server.timezonedetector.StateChangeListener; import java.time.Duration; import java.util.Objects; @@ -35,7 +35,7 @@ class LocationTimeZoneProviderControllerEnvironmentImpl extends LocationTimeZoneProviderController.Environment { @NonNull private final ServiceConfigAccessor mServiceConfigAccessor; - @NonNull private final ConfigurationChangeListener mConfigurationInternalChangeListener; + @NonNull private final StateChangeListener mConfigurationInternalChangeListener; LocationTimeZoneProviderControllerEnvironmentImpl(@NonNull ThreadingDomain threadingDomain, @NonNull ServiceConfigAccessor serviceConfigAccessor, diff --git a/services/tests/servicestests/src/com/android/server/timedetector/FakeServiceConfigAccessor.java b/services/tests/servicestests/src/com/android/server/timedetector/FakeServiceConfigAccessor.java index a98a43b75d9bb..93464cdd13d0c 100644 --- a/services/tests/servicestests/src/com/android/server/timedetector/FakeServiceConfigAccessor.java +++ b/services/tests/servicestests/src/com/android/server/timedetector/FakeServiceConfigAccessor.java @@ -25,7 +25,7 @@ import android.app.time.TimeCapabilities; import android.app.time.TimeCapabilitiesAndConfig; import android.app.time.TimeConfiguration; -import com.android.server.timezonedetector.ConfigurationChangeListener; +import com.android.server.timezonedetector.StateChangeListener; import java.util.ArrayList; import java.util.List; @@ -33,17 +33,17 @@ import java.util.List; /** A partially implemented, fake implementation of ServiceConfigAccessor for tests. */ public class FakeServiceConfigAccessor implements ServiceConfigAccessor { - private final List mConfigurationInternalChangeListeners = + private final List mConfigurationInternalChangeListeners = new ArrayList<>(); private ConfigurationInternal mConfigurationInternal; @Override - public void addConfigurationInternalChangeListener(ConfigurationChangeListener listener) { + public void addConfigurationInternalChangeListener(StateChangeListener listener) { mConfigurationInternalChangeListeners.add(listener); } @Override - public void removeConfigurationInternalChangeListener(ConfigurationChangeListener listener) { + public void removeConfigurationInternalChangeListener(StateChangeListener listener) { mConfigurationInternalChangeListeners.remove(listener); } @@ -86,7 +86,7 @@ public class FakeServiceConfigAccessor implements ServiceConfigAccessor { } void simulateConfigurationChangeForTests() { - for (ConfigurationChangeListener listener : mConfigurationInternalChangeListeners) { + for (StateChangeListener listener : mConfigurationInternalChangeListeners) { listener.onChange(); } } diff --git a/services/tests/servicestests/src/com/android/server/timedetector/TimeDetectorStrategyImplTest.java b/services/tests/servicestests/src/com/android/server/timedetector/TimeDetectorStrategyImplTest.java index 62dae481ac0b7..caef4943118b2 100644 --- a/services/tests/servicestests/src/com/android/server/timedetector/TimeDetectorStrategyImplTest.java +++ b/services/tests/servicestests/src/com/android/server/timedetector/TimeDetectorStrategyImplTest.java @@ -40,7 +40,7 @@ import android.os.TimestampedValue; import com.android.server.SystemClockTime.TimeConfidence; import com.android.server.timedetector.TimeDetectorStrategy.Origin; -import com.android.server.timezonedetector.ConfigurationChangeListener; +import com.android.server.timezonedetector.StateChangeListener; import org.junit.Before; import org.junit.Test; @@ -1821,7 +1821,7 @@ public class TimeDetectorStrategyImplTest { private long mElapsedRealtimeMillis; private long mSystemClockMillis; private int mSystemClockConfidence = TIME_CONFIDENCE_LOW; - private ConfigurationChangeListener mConfigurationInternalChangeListener; + private StateChangeListener mConfigurationInternalChangeListener; // Tracking operations. private boolean mSystemClockWasSet; @@ -1837,7 +1837,7 @@ public class TimeDetectorStrategyImplTest { } @Override - public void setConfigurationInternalChangeListener(ConfigurationChangeListener listener) { + public void setConfigurationInternalChangeListener(StateChangeListener listener) { mConfigurationInternalChangeListener = Objects.requireNonNull(listener); } diff --git a/services/tests/servicestests/src/com/android/server/timezonedetector/ConfigurationInternalTest.java b/services/tests/servicestests/src/com/android/server/timezonedetector/ConfigurationInternalTest.java index 7140097bb6c01..153d746aa8ec8 100644 --- a/services/tests/servicestests/src/com/android/server/timezonedetector/ConfigurationInternalTest.java +++ b/services/tests/servicestests/src/com/android/server/timezonedetector/ConfigurationInternalTest.java @@ -57,7 +57,8 @@ public class ConfigurationInternalTest { @Parameters({ "true,true", "true,false", "false,true", "false,false" }) public void test_autoDetectionSupported_capabilitiesAndConfiguration( boolean userConfigAllowed, boolean bypassUserPolicyChecks) { - ConfigurationInternal baseConfig = new ConfigurationInternal.Builder(ARBITRARY_USER_ID) + ConfigurationInternal baseConfig = new ConfigurationInternal.Builder() + .setUserId(ARBITRARY_USER_ID) .setUserConfigAllowed(userConfigAllowed) .setTelephonyDetectionFeatureSupported(true) .setGeoDetectionFeatureSupported(true) @@ -82,10 +83,7 @@ public class ConfigurationInternalTest { assertTrue(autoOnConfig.isGeoDetectionExecutionEnabled()); assertEquals(DETECTION_MODE_GEO, autoOnConfig.getDetectionMode()); - TimeZoneCapabilitiesAndConfig capabilitiesAndConfig = - autoOnConfig.createCapabilitiesAndConfig(bypassUserPolicyChecks); - - TimeZoneCapabilities capabilities = capabilitiesAndConfig.getCapabilities(); + TimeZoneCapabilities capabilities = autoOnConfig.asCapabilities(bypassUserPolicyChecks); if (userRestrictionsExpected) { assertEquals(CAPABILITY_NOT_ALLOWED, capabilities.getConfigureAutoDetectionEnabledCapability()); @@ -101,7 +99,7 @@ public class ConfigurationInternalTest { assertEquals(CAPABILITY_POSSESSED, capabilities.getConfigureGeoDetectionEnabledCapability()); - TimeZoneConfiguration configuration = capabilitiesAndConfig.getConfiguration(); + TimeZoneConfiguration configuration = autoOnConfig.asConfiguration(); assertTrue(configuration.isAutoDetectionEnabled()); assertTrue(configuration.isGeoDetectionEnabled()); } @@ -117,10 +115,8 @@ public class ConfigurationInternalTest { assertFalse(autoOffConfig.isGeoDetectionExecutionEnabled()); assertEquals(DETECTION_MODE_MANUAL, autoOffConfig.getDetectionMode()); - TimeZoneCapabilitiesAndConfig capabilitiesAndConfig = - autoOffConfig.createCapabilitiesAndConfig(bypassUserPolicyChecks); - - TimeZoneCapabilities capabilities = capabilitiesAndConfig.getCapabilities(); + TimeZoneCapabilities capabilities = + autoOffConfig.asCapabilities(bypassUserPolicyChecks); if (userRestrictionsExpected) { assertEquals(CAPABILITY_NOT_ALLOWED, capabilities.getConfigureAutoDetectionEnabledCapability()); @@ -136,7 +132,7 @@ public class ConfigurationInternalTest { assertEquals(CAPABILITY_NOT_APPLICABLE, capabilities.getConfigureGeoDetectionEnabledCapability()); - TimeZoneConfiguration configuration = capabilitiesAndConfig.getConfiguration(); + TimeZoneConfiguration configuration = autoOffConfig.asConfiguration(); assertFalse(configuration.isAutoDetectionEnabled()); assertTrue(configuration.isGeoDetectionEnabled()); } @@ -150,7 +146,8 @@ public class ConfigurationInternalTest { @Parameters({ "true,true", "true,false", "false,true", "false,false" }) public void test_autoDetectNotSupported_capabilitiesAndConfiguration( boolean userConfigAllowed, boolean bypassUserPolicyChecks) { - ConfigurationInternal baseConfig = new ConfigurationInternal.Builder(ARBITRARY_USER_ID) + ConfigurationInternal baseConfig = new ConfigurationInternal.Builder() + .setUserId(ARBITRARY_USER_ID) .setUserConfigAllowed(userConfigAllowed) .setTelephonyDetectionFeatureSupported(false) .setGeoDetectionFeatureSupported(false) @@ -175,10 +172,7 @@ public class ConfigurationInternalTest { assertFalse(autoOnConfig.isGeoDetectionExecutionEnabled()); assertEquals(DETECTION_MODE_MANUAL, autoOnConfig.getDetectionMode()); - TimeZoneCapabilitiesAndConfig capabilitiesAndConfig = - autoOnConfig.createCapabilitiesAndConfig(bypassUserPolicyChecks); - - TimeZoneCapabilities capabilities = capabilitiesAndConfig.getCapabilities(); + TimeZoneCapabilities capabilities = autoOnConfig.asCapabilities(bypassUserPolicyChecks); assertEquals(CAPABILITY_NOT_SUPPORTED, capabilities.getConfigureAutoDetectionEnabledCapability()); if (userRestrictionsExpected) { @@ -189,7 +183,7 @@ public class ConfigurationInternalTest { assertEquals(CAPABILITY_NOT_SUPPORTED, capabilities.getConfigureGeoDetectionEnabledCapability()); - TimeZoneConfiguration configuration = capabilitiesAndConfig.getConfiguration(); + TimeZoneConfiguration configuration = autoOnConfig.asConfiguration(); assertTrue(configuration.isAutoDetectionEnabled()); assertTrue(configuration.isGeoDetectionEnabled()); } @@ -205,10 +199,8 @@ public class ConfigurationInternalTest { assertFalse(autoOffConfig.isGeoDetectionExecutionEnabled()); assertEquals(DETECTION_MODE_MANUAL, autoOffConfig.getDetectionMode()); - TimeZoneCapabilitiesAndConfig capabilitiesAndConfig = - autoOffConfig.createCapabilitiesAndConfig(bypassUserPolicyChecks); - - TimeZoneCapabilities capabilities = capabilitiesAndConfig.getCapabilities(); + TimeZoneCapabilities capabilities = + autoOffConfig.asCapabilities(bypassUserPolicyChecks); assertEquals(CAPABILITY_NOT_SUPPORTED, capabilities.getConfigureAutoDetectionEnabledCapability()); if (userRestrictionsExpected) { @@ -219,7 +211,7 @@ public class ConfigurationInternalTest { assertEquals(CAPABILITY_NOT_SUPPORTED, capabilities.getConfigureGeoDetectionEnabledCapability()); - TimeZoneConfiguration configuration = capabilitiesAndConfig.getConfiguration(); + TimeZoneConfiguration configuration = autoOffConfig.asConfiguration(); assertFalse(configuration.isAutoDetectionEnabled()); assertTrue(configuration.isGeoDetectionEnabled()); } @@ -233,7 +225,8 @@ public class ConfigurationInternalTest { @Parameters({ "true,true", "true,false", "false,true", "false,false" }) public void test_geoDetectNotSupported_capabilitiesAndConfiguration( boolean userConfigAllowed, boolean bypassUserPolicyChecks) { - ConfigurationInternal baseConfig = new ConfigurationInternal.Builder(ARBITRARY_USER_ID) + ConfigurationInternal baseConfig = new ConfigurationInternal.Builder() + .setUserId(ARBITRARY_USER_ID) .setUserConfigAllowed(userConfigAllowed) .setTelephonyDetectionFeatureSupported(true) .setGeoDetectionFeatureSupported(false) @@ -258,10 +251,7 @@ public class ConfigurationInternalTest { assertFalse(autoOnConfig.isGeoDetectionExecutionEnabled()); assertEquals(DETECTION_MODE_TELEPHONY, autoOnConfig.getDetectionMode()); - TimeZoneCapabilitiesAndConfig capabilitiesAndConfig = - autoOnConfig.createCapabilitiesAndConfig(bypassUserPolicyChecks); - - TimeZoneCapabilities capabilities = capabilitiesAndConfig.getCapabilities(); + TimeZoneCapabilities capabilities = autoOnConfig.asCapabilities(bypassUserPolicyChecks); if (userRestrictionsExpected) { assertEquals(CAPABILITY_NOT_ALLOWED, capabilities.getConfigureAutoDetectionEnabledCapability()); @@ -276,7 +266,7 @@ public class ConfigurationInternalTest { assertEquals(CAPABILITY_NOT_SUPPORTED, capabilities.getConfigureGeoDetectionEnabledCapability()); - TimeZoneConfiguration configuration = capabilitiesAndConfig.getConfiguration(); + TimeZoneConfiguration configuration = autoOnConfig.asConfiguration(); assertTrue(configuration.isAutoDetectionEnabled()); assertTrue(configuration.isGeoDetectionEnabled()); } @@ -292,10 +282,8 @@ public class ConfigurationInternalTest { assertFalse(autoOffConfig.isGeoDetectionExecutionEnabled()); assertEquals(DETECTION_MODE_MANUAL, autoOffConfig.getDetectionMode()); - TimeZoneCapabilitiesAndConfig capabilitiesAndConfig = - autoOffConfig.createCapabilitiesAndConfig(bypassUserPolicyChecks); - - TimeZoneCapabilities capabilities = capabilitiesAndConfig.getCapabilities(); + TimeZoneCapabilities capabilities = + autoOffConfig.asCapabilities(bypassUserPolicyChecks); if (userRestrictionsExpected) { assertEquals(CAPABILITY_NOT_ALLOWED, capabilities.getConfigureAutoDetectionEnabledCapability()); @@ -308,7 +296,7 @@ public class ConfigurationInternalTest { assertEquals(CAPABILITY_NOT_SUPPORTED, capabilities.getConfigureGeoDetectionEnabledCapability()); - TimeZoneConfiguration configuration = capabilitiesAndConfig.getConfiguration(); + TimeZoneConfiguration configuration = autoOffConfig.asConfiguration(); assertFalse(configuration.isAutoDetectionEnabled()); assertTrue(configuration.isGeoDetectionEnabled()); } @@ -316,7 +304,8 @@ public class ConfigurationInternalTest { @Test public void test_telephonyFallbackSupported() { - ConfigurationInternal config = new ConfigurationInternal.Builder(ARBITRARY_USER_ID) + ConfigurationInternal config = new ConfigurationInternal.Builder() + .setUserId(ARBITRARY_USER_ID) .setUserConfigAllowed(true) .setTelephonyDetectionFeatureSupported(true) .setGeoDetectionFeatureSupported(false) @@ -331,7 +320,8 @@ public class ConfigurationInternalTest { /** Tests when {@link ConfigurationInternal#getGeoDetectionRunInBackgroundEnabled()} is true. */ @Test public void test_geoDetectionRunInBackgroundEnabled() { - ConfigurationInternal baseConfig = new ConfigurationInternal.Builder(ARBITRARY_USER_ID) + ConfigurationInternal baseConfig = new ConfigurationInternal.Builder() + .setUserId(ARBITRARY_USER_ID) .setUserConfigAllowed(true) .setTelephonyDetectionFeatureSupported(true) .setGeoDetectionFeatureSupported(true) diff --git a/services/tests/servicestests/src/com/android/server/timezonedetector/FakeServiceConfigAccessor.java b/services/tests/servicestests/src/com/android/server/timezonedetector/FakeServiceConfigAccessor.java index fdee86ea12857..fc6afe4861878 100644 --- a/services/tests/servicestests/src/com/android/server/timezonedetector/FakeServiceConfigAccessor.java +++ b/services/tests/servicestests/src/com/android/server/timezonedetector/FakeServiceConfigAccessor.java @@ -16,14 +16,11 @@ package com.android.server.timezonedetector; -import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.fail; import android.annotation.NonNull; import android.annotation.UserIdInt; import android.app.time.TimeZoneCapabilities; -import android.app.time.TimeZoneCapabilitiesAndConfig; import android.app.time.TimeZoneConfiguration; import java.time.Duration; @@ -31,76 +28,104 @@ import java.util.ArrayList; import java.util.List; import java.util.Optional; -/** A partially implemented, fake implementation of ServiceConfigAccessor for tests. */ +/** + * A partially implemented, fake implementation of ServiceConfigAccessor for tests. + * + *

This class has rudamentary support for multiple users, but unlike the real thing, it doesn't + * simulate that some settings are global and shared between users. It also delivers config updates + * synchronously. + */ public class FakeServiceConfigAccessor implements ServiceConfigAccessor { - private final List mConfigurationInternalChangeListeners = + private final List mConfigurationInternalChangeListeners = new ArrayList<>(); - private ConfigurationInternal mConfigurationInternal; + private ConfigurationInternal mCurrentUserConfigurationInternal; + private ConfigurationInternal mOtherUserConfigurationInternal; @Override - public void addConfigurationInternalChangeListener(ConfigurationChangeListener listener) { + public void addConfigurationInternalChangeListener(StateChangeListener listener) { mConfigurationInternalChangeListeners.add(listener); } @Override - public void removeConfigurationInternalChangeListener(ConfigurationChangeListener listener) { + public void removeConfigurationInternalChangeListener(StateChangeListener listener) { mConfigurationInternalChangeListeners.remove(listener); } @Override public ConfigurationInternal getCurrentUserConfigurationInternal() { - return mConfigurationInternal; + return getConfigurationInternal(mCurrentUserConfigurationInternal.getUserId()); } @Override public boolean updateConfiguration( - @UserIdInt int userID, @NonNull TimeZoneConfiguration requestedChanges, + @UserIdInt int userId, @NonNull TimeZoneConfiguration requestedChanges, boolean bypassUserPolicyChecks) { - assertNotNull(mConfigurationInternal); + assertNotNull(mCurrentUserConfigurationInternal); assertNotNull(requestedChanges); + ConfigurationInternal toUpdate = getConfigurationInternal(userId); + // Simulate the real strategy's behavior: the new configuration will be updated to be the - // old configuration merged with the new if the user has the capability to up the settings. - // Then, if the configuration changed, the change listener is invoked. - TimeZoneCapabilitiesAndConfig capabilitiesAndConfig = - mConfigurationInternal.createCapabilitiesAndConfig(bypassUserPolicyChecks); - TimeZoneCapabilities capabilities = capabilitiesAndConfig.getCapabilities(); - TimeZoneConfiguration configuration = capabilitiesAndConfig.getConfiguration(); + // old configuration merged with the new if the user has the capability to update the + // settings. Then, if the configuration changed, the change listener is invoked. + TimeZoneCapabilities capabilities = toUpdate.asCapabilities(bypassUserPolicyChecks); + TimeZoneConfiguration configuration = toUpdate.asConfiguration(); TimeZoneConfiguration newConfiguration = capabilities.tryApplyConfigChanges(configuration, requestedChanges); if (newConfiguration == null) { return false; } - if (!newConfiguration.equals(capabilitiesAndConfig.getConfiguration())) { - mConfigurationInternal = mConfigurationInternal.merge(newConfiguration); - + if (!newConfiguration.equals(configuration)) { + ConfigurationInternal updatedConfiguration = toUpdate.merge(newConfiguration); + if (updatedConfiguration.getUserId() == mCurrentUserConfigurationInternal.getUserId()) { + mCurrentUserConfigurationInternal = updatedConfiguration; + } else if (mOtherUserConfigurationInternal != null + && updatedConfiguration.getUserId() + == mOtherUserConfigurationInternal.getUserId()) { + mOtherUserConfigurationInternal = updatedConfiguration; + } // Note: Unlike the real strategy, the listeners are invoked synchronously. - simulateConfigurationChangeForTests(); + notifyConfigurationChange(); } return true; } - void initializeConfiguration(ConfigurationInternal configurationInternal) { - mConfigurationInternal = configurationInternal; + void initializeCurrentUserConfiguration(ConfigurationInternal configurationInternal) { + mCurrentUserConfigurationInternal = configurationInternal; } - void simulateConfigurationChangeForTests() { - for (ConfigurationChangeListener listener : mConfigurationInternalChangeListeners) { - listener.onChange(); - } + void initializeOtherUserConfiguration(ConfigurationInternal configurationInternal) { + mOtherUserConfigurationInternal = configurationInternal; + } + + void simulateCurrentUserConfigurationInternalChange( + ConfigurationInternal configurationInternal) { + mCurrentUserConfigurationInternal = configurationInternal; + // Note: Unlike the real strategy, the listeners are invoked synchronously. + notifyConfigurationChange(); + } + + void simulateOtherUserConfigurationInternalChange(ConfigurationInternal configurationInternal) { + mOtherUserConfigurationInternal = configurationInternal; + // Note: Unlike the real strategy, the listeners are invoked synchronously. + notifyConfigurationChange(); } @Override public ConfigurationInternal getConfigurationInternal(int userId) { - assertEquals("Multi-user testing not supported currently", - userId, mConfigurationInternal.getUserId()); - return mConfigurationInternal; + if (userId == mCurrentUserConfigurationInternal.getUserId()) { + return mCurrentUserConfigurationInternal; + } else if (mOtherUserConfigurationInternal != null + && userId == mOtherUserConfigurationInternal.getUserId()) { + return mOtherUserConfigurationInternal; + } + throw new AssertionError("userId not known: " + userId); } @Override - public void addLocationTimeZoneManagerConfigListener(ConfigurationChangeListener listener) { + public void addLocationTimeZoneManagerConfigListener(StateChangeListener listener) { failUnimplemented(); } @@ -206,9 +231,14 @@ public class FakeServiceConfigAccessor implements ServiceConfigAccessor { failUnimplemented(); } + private void notifyConfigurationChange() { + for (StateChangeListener listener : mConfigurationInternalChangeListeners) { + listener.onChange(); + } + } + @SuppressWarnings("UnusedReturnValue") private static T failUnimplemented() { - fail("Unimplemented"); - return null; + throw new AssertionError("Unimplemented"); } } diff --git a/services/tests/servicestests/src/com/android/server/timezonedetector/FakeTimeZoneDetectorStrategy.java b/services/tests/servicestests/src/com/android/server/timezonedetector/FakeTimeZoneDetectorStrategy.java index 228dc952356a9..fed8b4040aba8 100644 --- a/services/tests/servicestests/src/com/android/server/timezonedetector/FakeTimeZoneDetectorStrategy.java +++ b/services/tests/servicestests/src/com/android/server/timezonedetector/FakeTimeZoneDetectorStrategy.java @@ -15,22 +15,70 @@ */ package com.android.server.timezonedetector; +import static org.junit.Assert.assertEquals; + import android.annotation.NonNull; import android.annotation.UserIdInt; +import android.app.time.TimeZoneCapabilitiesAndConfig; +import android.app.time.TimeZoneConfiguration; import android.app.time.TimeZoneState; import android.app.timezonedetector.ManualTimeZoneSuggestion; import android.app.timezonedetector.TelephonyTimeZoneSuggestion; import android.util.IndentingPrintWriter; +import java.util.ArrayList; + public class FakeTimeZoneDetectorStrategy implements TimeZoneDetectorStrategy { + private final FakeServiceConfigAccessor mFakeServiceConfigAccessor = + new FakeServiceConfigAccessor(); + private final ArrayList mListeners = new ArrayList<>(); private TimeZoneState mTimeZoneState; + public FakeTimeZoneDetectorStrategy() { + mFakeServiceConfigAccessor.addConfigurationInternalChangeListener( + this::notifyChangeListeners); + } + + public void initializeConfiguration(ConfigurationInternal configuration) { + mFakeServiceConfigAccessor.initializeCurrentUserConfiguration(configuration); + } + @Override public boolean confirmTimeZone(String timeZoneId) { return false; } + @Override + public TimeZoneCapabilitiesAndConfig getCapabilitiesAndConfig(int userId, + boolean bypassUserPolicyChecks) { + ConfigurationInternal configurationInternal = + mFakeServiceConfigAccessor.getCurrentUserConfigurationInternal(); + assertEquals("Multi-user testing not supported", + configurationInternal.getUserId(), userId); + return new TimeZoneCapabilitiesAndConfig( + configurationInternal.asCapabilities(bypassUserPolicyChecks), + configurationInternal.asConfiguration()); + } + + @Override + public boolean updateConfiguration(int userId, TimeZoneConfiguration requestedChanges, + boolean bypassUserPolicyChecks) { + return mFakeServiceConfigAccessor.updateConfiguration( + userId, requestedChanges, bypassUserPolicyChecks); + } + + @Override + public void addChangeListener(StateChangeListener listener) { + mListeners.add(listener); + } + + private void notifyChangeListeners() { + for (StateChangeListener listener : mListeners) { + listener.onChange(); + } + } + @Override public TimeZoneState getTimeZoneState() { return mTimeZoneState; diff --git a/services/tests/servicestests/src/com/android/server/timezonedetector/MetricsTimeZoneDetectorStateTest.java b/services/tests/servicestests/src/com/android/server/timezonedetector/MetricsTimeZoneDetectorStateTest.java index 782eebfb92701..223c532330650 100644 --- a/services/tests/servicestests/src/com/android/server/timezonedetector/MetricsTimeZoneDetectorStateTest.java +++ b/services/tests/servicestests/src/com/android/server/timezonedetector/MetricsTimeZoneDetectorStateTest.java @@ -161,7 +161,8 @@ public class MetricsTimeZoneDetectorStateTest { private static ConfigurationInternal createConfigurationInternal( boolean enhancedMetricsCollectionEnabled) { - return new ConfigurationInternal.Builder(ARBITRARY_USER_ID) + return new ConfigurationInternal.Builder() + .setUserId(ARBITRARY_USER_ID) .setUserConfigAllowed(true) .setTelephonyDetectionFeatureSupported(true) .setGeoDetectionFeatureSupported(true) diff --git a/services/tests/servicestests/src/com/android/server/timezonedetector/TestHandler.java b/services/tests/servicestests/src/com/android/server/timezonedetector/TestHandler.java index 21c9685b05d20..eb6f00c9250ba 100644 --- a/services/tests/servicestests/src/com/android/server/timezonedetector/TestHandler.java +++ b/services/tests/servicestests/src/com/android/server/timezonedetector/TestHandler.java @@ -66,10 +66,14 @@ public class TestHandler extends Handler { /** * Waits for all enqueued work to be completed before returning. */ - public void waitForMessagesToBeProcessed() throws InterruptedException { + public void waitForMessagesToBeProcessed() { synchronized (mMonitor) { if (mMessagesSent != mMessagesProcessed) { - mMonitor.wait(); + try { + mMonitor.wait(); + } catch (InterruptedException e) { + throw new AssertionError("Unexpected exception", e); + } } } } diff --git a/services/tests/servicestests/src/com/android/server/timezonedetector/TimeZoneDetectorInternalImplTest.java b/services/tests/servicestests/src/com/android/server/timezonedetector/TimeZoneDetectorInternalImplTest.java index 276fdb9711721..8909832391a48 100644 --- a/services/tests/servicestests/src/com/android/server/timezonedetector/TimeZoneDetectorInternalImplTest.java +++ b/services/tests/servicestests/src/com/android/server/timezonedetector/TimeZoneDetectorInternalImplTest.java @@ -50,7 +50,6 @@ public class TimeZoneDetectorInternalImplTest { private HandlerThread mHandlerThread; private TestHandler mTestHandler; private TestCurrentUserIdentityInjector mTestCurrentUserIdentityInjector; - private FakeServiceConfigAccessor mFakeServiceConfigAccessorSpy; private FakeTimeZoneDetectorStrategy mFakeTimeZoneDetectorStrategySpy; private TimeZoneDetectorInternalImpl mTimeZoneDetectorInternal; @@ -65,12 +64,11 @@ public class TimeZoneDetectorInternalImplTest { mTestHandler = new TestHandler(mHandlerThread.getLooper()); mTestCurrentUserIdentityInjector = new TestCurrentUserIdentityInjector(); mTestCurrentUserIdentityInjector.initializeCurrentUserId(ARBITRARY_USER_ID); - mFakeServiceConfigAccessorSpy = spy(new FakeServiceConfigAccessor()); mFakeTimeZoneDetectorStrategySpy = spy(new FakeTimeZoneDetectorStrategy()); mTimeZoneDetectorInternal = new TimeZoneDetectorInternalImpl( mMockContext, mTestHandler, mTestCurrentUserIdentityInjector, - mFakeServiceConfigAccessorSpy, mFakeTimeZoneDetectorStrategySpy); + mFakeTimeZoneDetectorStrategySpy); } @After @@ -83,17 +81,20 @@ public class TimeZoneDetectorInternalImplTest { public void testGetCapabilitiesAndConfigForDpm() throws Exception { final boolean autoDetectionEnabled = true; ConfigurationInternal testConfig = createConfigurationInternal(autoDetectionEnabled); - mFakeServiceConfigAccessorSpy.initializeConfiguration(testConfig); + mFakeTimeZoneDetectorStrategySpy.initializeConfiguration(testConfig); TimeZoneCapabilitiesAndConfig actualCapabilitiesAndConfig = mTimeZoneDetectorInternal.getCapabilitiesAndConfigForDpm(); int expectedUserId = mTestCurrentUserIdentityInjector.getCurrentUserId(); - verify(mFakeServiceConfigAccessorSpy).getConfigurationInternal(expectedUserId); + final boolean expectedBypassUserPolicyChecks = true; + verify(mFakeTimeZoneDetectorStrategySpy).getCapabilitiesAndConfig( + expectedUserId, expectedBypassUserPolicyChecks); - final boolean bypassUserPolicyChecks = true; TimeZoneCapabilitiesAndConfig expectedCapabilitiesAndConfig = - testConfig.createCapabilitiesAndConfig(bypassUserPolicyChecks); + new TimeZoneCapabilitiesAndConfig( + testConfig.asCapabilities(expectedBypassUserPolicyChecks), + testConfig.asConfiguration()); assertEquals(expectedCapabilitiesAndConfig, actualCapabilitiesAndConfig); } @@ -102,7 +103,7 @@ public class TimeZoneDetectorInternalImplTest { final boolean autoDetectionEnabled = false; ConfigurationInternal initialConfigurationInternal = createConfigurationInternal(autoDetectionEnabled); - mFakeServiceConfigAccessorSpy.initializeConfiguration(initialConfigurationInternal); + mFakeTimeZoneDetectorStrategySpy.initializeConfiguration(initialConfigurationInternal); TimeZoneConfiguration timeConfiguration = new TimeZoneConfiguration.Builder() .setAutoDetectionEnabled(true) @@ -110,7 +111,7 @@ public class TimeZoneDetectorInternalImplTest { assertTrue(mTimeZoneDetectorInternal.updateConfigurationForDpm(timeConfiguration)); final boolean expectedBypassUserPolicyChecks = true; - verify(mFakeServiceConfigAccessorSpy).updateConfiguration( + verify(mFakeTimeZoneDetectorStrategySpy).updateConfiguration( mTestCurrentUserIdentityInjector.getCurrentUserId(), timeConfiguration, expectedBypassUserPolicyChecks); @@ -148,7 +149,8 @@ public class TimeZoneDetectorInternalImplTest { } private static ConfigurationInternal createConfigurationInternal(boolean autoDetectionEnabled) { - return new ConfigurationInternal.Builder(ARBITRARY_USER_ID) + return new ConfigurationInternal.Builder() + .setUserId(ARBITRARY_USER_ID) .setTelephonyDetectionFeatureSupported(true) .setGeoDetectionFeatureSupported(true) .setTelephonyFallbackSupported(false) diff --git a/services/tests/servicestests/src/com/android/server/timezonedetector/TimeZoneDetectorServiceTest.java b/services/tests/servicestests/src/com/android/server/timezonedetector/TimeZoneDetectorServiceTest.java index bb9d564b5aad8..d8346ee4355b2 100644 --- a/services/tests/servicestests/src/com/android/server/timezonedetector/TimeZoneDetectorServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/timezonedetector/TimeZoneDetectorServiceTest.java @@ -69,7 +69,6 @@ public class TimeZoneDetectorServiceTest { private HandlerThread mHandlerThread; private TestHandler mTestHandler; private TestCallerIdentityInjector mTestCallerIdentityInjector; - private FakeServiceConfigAccessor mFakeServiceConfigAccessorSpy; private FakeTimeZoneDetectorStrategy mFakeTimeZoneDetectorStrategySpy; @@ -85,12 +84,11 @@ public class TimeZoneDetectorServiceTest { mTestCallerIdentityInjector = new TestCallerIdentityInjector(); mTestCallerIdentityInjector.initializeCallingUserId(ARBITRARY_USER_ID); - mFakeServiceConfigAccessorSpy = spy(new FakeServiceConfigAccessor()); mFakeTimeZoneDetectorStrategySpy = spy(new FakeTimeZoneDetectorStrategy()); mTimeZoneDetectorService = new TimeZoneDetectorService( mMockContext, mTestHandler, mTestCallerIdentityInjector, - mFakeServiceConfigAccessorSpy, mFakeTimeZoneDetectorStrategySpy); + mFakeTimeZoneDetectorStrategySpy); } @After @@ -115,7 +113,7 @@ public class TimeZoneDetectorServiceTest { ConfigurationInternal configuration = createConfigurationInternal(true /* autoDetectionEnabled*/); - mFakeServiceConfigAccessorSpy.initializeConfiguration(configuration); + mFakeTimeZoneDetectorStrategySpy.initializeConfiguration(configuration); TimeZoneCapabilitiesAndConfig actualCapabilitiesAndConfig = mTimeZoneDetectorService.getCapabilitiesAndConfig(); @@ -124,11 +122,14 @@ public class TimeZoneDetectorServiceTest { eq(android.Manifest.permission.MANAGE_TIME_AND_ZONE_DETECTION), anyString()); int expectedUserId = mTestCallerIdentityInjector.getCallingUserId(); - verify(mFakeServiceConfigAccessorSpy).getConfigurationInternal(expectedUserId); - boolean expectedBypassUserPolicyChecks = false; + verify(mFakeTimeZoneDetectorStrategySpy) + .getCapabilitiesAndConfig(expectedUserId, expectedBypassUserPolicyChecks); + TimeZoneCapabilitiesAndConfig expectedCapabilitiesAndConfig = - configuration.createCapabilitiesAndConfig(expectedBypassUserPolicyChecks); + new TimeZoneCapabilitiesAndConfig( + configuration.asCapabilities(expectedBypassUserPolicyChecks), + configuration.asConfiguration()); assertEquals(expectedCapabilitiesAndConfig, actualCapabilitiesAndConfig); } @@ -160,7 +161,7 @@ public class TimeZoneDetectorServiceTest { public void testListenerRegistrationAndCallbacks() throws Exception { ConfigurationInternal initialConfiguration = createConfigurationInternal(false /* autoDetectionEnabled */); - mFakeServiceConfigAccessorSpy.initializeConfiguration(initialConfiguration); + mFakeTimeZoneDetectorStrategySpy.initializeConfiguration(initialConfiguration); IBinder mockListenerBinder = mock(IBinder.class); ITimeZoneDetectorListener mockListener = mock(ITimeZoneDetectorListener.class); @@ -455,7 +456,8 @@ public class TimeZoneDetectorServiceTest { // Default geo detection settings from auto detection settings - they are not important to // the tests. final boolean geoDetectionEnabled = autoDetectionEnabled; - return new ConfigurationInternal.Builder(ARBITRARY_USER_ID) + return new ConfigurationInternal.Builder() + .setUserId(ARBITRARY_USER_ID) .setTelephonyDetectionFeatureSupported(true) .setGeoDetectionFeatureSupported(true) .setTelephonyFallbackSupported(false) 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 d0a7c92b04364..f50e7fbc76bb1 100644 --- a/services/tests/servicestests/src/com/android/server/timezonedetector/TimeZoneDetectorStrategyImplTest.java +++ b/services/tests/servicestests/src/com/android/server/timezonedetector/TimeZoneDetectorStrategyImplTest.java @@ -38,19 +38,28 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.reset; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; import android.annotation.ElapsedRealtimeLong; import android.annotation.NonNull; import android.annotation.UserIdInt; +import android.app.time.TimeZoneCapabilitiesAndConfig; +import android.app.time.TimeZoneConfiguration; import android.app.time.TimeZoneState; import android.app.timezonedetector.ManualTimeZoneSuggestion; import android.app.timezonedetector.TelephonyTimeZoneSuggestion; import android.app.timezonedetector.TelephonyTimeZoneSuggestion.MatchType; import android.app.timezonedetector.TelephonyTimeZoneSuggestion.Quality; +import android.os.HandlerThread; import com.android.server.SystemTimeZone.TimeZoneConfidence; import com.android.server.timezonedetector.TimeZoneDetectorStrategyImpl.QualifiedTelephonyTimeZoneSuggestion; +import org.junit.After; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -97,7 +106,8 @@ public class TimeZoneDetectorStrategyImplTest { }; private static final ConfigurationInternal CONFIG_USER_RESTRICTED_AUTO_DISABLED = - new ConfigurationInternal.Builder(USER_ID) + new ConfigurationInternal.Builder() + .setUserId(USER_ID) .setTelephonyDetectionFeatureSupported(true) .setGeoDetectionFeatureSupported(true) .setTelephonyFallbackSupported(false) @@ -110,7 +120,8 @@ public class TimeZoneDetectorStrategyImplTest { .build(); private static final ConfigurationInternal CONFIG_USER_RESTRICTED_AUTO_ENABLED = - new ConfigurationInternal.Builder(USER_ID) + new ConfigurationInternal.Builder() + .setUserId(USER_ID) .setTelephonyDetectionFeatureSupported(true) .setGeoDetectionFeatureSupported(true) .setTelephonyFallbackSupported(false) @@ -123,7 +134,8 @@ public class TimeZoneDetectorStrategyImplTest { .build(); private static final ConfigurationInternal CONFIG_AUTO_DETECT_NOT_SUPPORTED = - new ConfigurationInternal.Builder(USER_ID) + new ConfigurationInternal.Builder() + .setUserId(USER_ID) .setTelephonyDetectionFeatureSupported(false) .setGeoDetectionFeatureSupported(false) .setTelephonyFallbackSupported(false) @@ -136,7 +148,8 @@ public class TimeZoneDetectorStrategyImplTest { .build(); private static final ConfigurationInternal CONFIG_AUTO_DISABLED_GEO_DISABLED = - new ConfigurationInternal.Builder(USER_ID) + new ConfigurationInternal.Builder() + .setUserId(USER_ID) .setTelephonyDetectionFeatureSupported(true) .setGeoDetectionFeatureSupported(true) .setTelephonyFallbackSupported(false) @@ -149,7 +162,8 @@ public class TimeZoneDetectorStrategyImplTest { .build(); private static final ConfigurationInternal CONFIG_AUTO_ENABLED_GEO_DISABLED = - new ConfigurationInternal.Builder(USER_ID) + new ConfigurationInternal.Builder() + .setUserId(USER_ID) .setTelephonyDetectionFeatureSupported(true) .setGeoDetectionFeatureSupported(true) .setTelephonyFallbackSupported(false) @@ -162,7 +176,8 @@ public class TimeZoneDetectorStrategyImplTest { .build(); private static final ConfigurationInternal CONFIG_AUTO_ENABLED_GEO_ENABLED = - new ConfigurationInternal.Builder(USER_ID) + new ConfigurationInternal.Builder() + .setUserId(USER_ID) .setTelephonyDetectionFeatureSupported(true) .setGeoDetectionFeatureSupported(true) .setTelephonyFallbackSupported(false) @@ -174,14 +189,223 @@ public class TimeZoneDetectorStrategyImplTest { .setGeoDetectionEnabledSetting(true) .build(); - private TimeZoneDetectorStrategyImpl mTimeZoneDetectorStrategy; + private FakeServiceConfigAccessor mFakeServiceConfigAccessorSpy; private FakeEnvironment mFakeEnvironment; + private HandlerThread mHandlerThread; + private TestHandler mTestHandler; + + private TimeZoneDetectorStrategyImpl mTimeZoneDetectorStrategy; @Before public void setUp() { mFakeEnvironment = new FakeEnvironment(); - mFakeEnvironment.initializeConfig(CONFIG_AUTO_DISABLED_GEO_DISABLED); - mTimeZoneDetectorStrategy = new TimeZoneDetectorStrategyImpl(mFakeEnvironment); + mFakeServiceConfigAccessorSpy = spy(new FakeServiceConfigAccessor()); + mFakeServiceConfigAccessorSpy.initializeCurrentUserConfiguration( + CONFIG_AUTO_DISABLED_GEO_DISABLED); + + // Create a thread + handler for processing the work that the strategy posts. + mHandlerThread = new HandlerThread("TimeZoneDetectorStrategyImplTest"); + mHandlerThread.start(); + mTestHandler = new TestHandler(mHandlerThread.getLooper()); + mTimeZoneDetectorStrategy = new TimeZoneDetectorStrategyImpl( + mFakeServiceConfigAccessorSpy, mTestHandler, mFakeEnvironment); + } + + @After + public void tearDown() throws Exception { + mHandlerThread.quit(); + mHandlerThread.join(); + } + + @Test + public void testChangeListenerBehavior_currentUser() throws Exception { + ConfigurationInternal currentUserConfig = CONFIG_AUTO_DISABLED_GEO_DISABLED; + // The strategy initializes itself with the current user's config during construction. + assertEquals(currentUserConfig, + mTimeZoneDetectorStrategy.getCachedCapabilitiesAndConfigForTests()); + + TestStateChangeListener stateChangeListener = new TestStateChangeListener(); + mTimeZoneDetectorStrategy.addChangeListener(stateChangeListener); + + boolean bypassUserPolicyChecks = false; + + // Report a config change, but not one that actually changes anything. + { + mFakeServiceConfigAccessorSpy.simulateCurrentUserConfigurationInternalChange( + CONFIG_AUTO_DISABLED_GEO_DISABLED); + mTestHandler.waitForMessagesToBeProcessed(); + + stateChangeListener.assertNotificationsReceived(0); + assertEquals(CONFIG_AUTO_DISABLED_GEO_DISABLED, + mTimeZoneDetectorStrategy.getCachedCapabilitiesAndConfigForTests()); + } + + // Report a config change that actually changes something. + { + mFakeServiceConfigAccessorSpy.simulateCurrentUserConfigurationInternalChange( + CONFIG_AUTO_ENABLED_GEO_ENABLED); + mTestHandler.waitForMessagesToBeProcessed(); + + stateChangeListener.assertNotificationsReceived(1); + stateChangeListener.resetNotificationsReceivedCount(); + assertEquals(CONFIG_AUTO_ENABLED_GEO_ENABLED, + mTimeZoneDetectorStrategy.getCachedCapabilitiesAndConfigForTests()); + } + + // Perform a (current user) update via the strategy. + { + TimeZoneConfiguration requestedChanges = + new TimeZoneConfiguration.Builder().setGeoDetectionEnabled(false).build(); + mTimeZoneDetectorStrategy.updateConfiguration( + USER_ID, requestedChanges, bypassUserPolicyChecks); + mTestHandler.waitForMessagesToBeProcessed(); + + stateChangeListener.assertNotificationsReceived(1); + stateChangeListener.resetNotificationsReceivedCount(); + } + } + + // Perform a (not current user) update via the strategy. There's no listener behavior for + // updates to "other" users. + @Test + public void testChangeListenerBehavior_otherUser() throws Exception { + ConfigurationInternal currentUserConfig = CONFIG_AUTO_DISABLED_GEO_DISABLED; + // The strategy initializes itself with the current user's config during construction. + assertEquals(currentUserConfig, + mTimeZoneDetectorStrategy.getCachedCapabilitiesAndConfigForTests()); + + TestStateChangeListener stateChangeListener = new TestStateChangeListener(); + mTimeZoneDetectorStrategy.addChangeListener(stateChangeListener); + + boolean bypassUserPolicyChecks = false; + + int otherUserId = currentUserConfig.getUserId() + 1; + ConfigurationInternal otherUserConfig = new ConfigurationInternal.Builder(currentUserConfig) + .setUserId(otherUserId) + .setGeoDetectionEnabledSetting(true) + .build(); + mFakeServiceConfigAccessorSpy.initializeOtherUserConfiguration(otherUserConfig); + + TimeZoneConfiguration requestedChanges = + new TimeZoneConfiguration.Builder().setGeoDetectionEnabled(false).build(); + mTimeZoneDetectorStrategy.updateConfiguration( + otherUserId, requestedChanges, bypassUserPolicyChecks); + mTestHandler.waitForMessagesToBeProcessed(); + + // Only changes to the current user's config are notified. + stateChangeListener.assertNotificationsReceived(0); + stateChangeListener.resetNotificationsReceivedCount(); + } + + // Current user behavior: the strategy caches and returns the latest configuration. + @Test + public void testReadAndWriteConfiguration_currentUser() throws Exception { + ConfigurationInternal currentUserConfig = CONFIG_AUTO_ENABLED_GEO_DISABLED; + mFakeServiceConfigAccessorSpy.simulateCurrentUserConfigurationInternalChange( + currentUserConfig); + + int otherUserId = currentUserConfig.getUserId() + 1; + ConfigurationInternal otherUserConfig = new ConfigurationInternal.Builder(currentUserConfig) + .setUserId(otherUserId) + .setGeoDetectionEnabledSetting(true) + .build(); + mFakeServiceConfigAccessorSpy.simulateOtherUserConfigurationInternalChange(otherUserConfig); + reset(mFakeServiceConfigAccessorSpy); + + final boolean bypassUserPolicyChecks = false; + + ConfigurationInternal cachedConfigurationInternal = + mTimeZoneDetectorStrategy.getCachedCapabilitiesAndConfigForTests(); + assertEquals(currentUserConfig, cachedConfigurationInternal); + + // Confirm getCapabilitiesAndConfig() does not call through to the ServiceConfigAccessor. + { + reset(mFakeServiceConfigAccessorSpy); + TimeZoneCapabilitiesAndConfig actualCapabilitiesAndConfig = + mTimeZoneDetectorStrategy.getCapabilitiesAndConfig( + currentUserConfig.getUserId(), bypassUserPolicyChecks); + verify(mFakeServiceConfigAccessorSpy, never()).getConfigurationInternal( + currentUserConfig.getUserId()); + + assertEquals(currentUserConfig.asCapabilities(bypassUserPolicyChecks), + actualCapabilitiesAndConfig.getCapabilities()); + assertEquals(currentUserConfig.asConfiguration(), + actualCapabilitiesAndConfig.getConfiguration()); + } + + // Confirm updateConfiguration() calls through to the ServiceConfigAccessor and updates + // the cached copy. + { + boolean newGeoDetectionEnabled = + !cachedConfigurationInternal.asConfiguration().isGeoDetectionEnabled(); + TimeZoneConfiguration requestedChanges = new TimeZoneConfiguration.Builder() + .setGeoDetectionEnabled(newGeoDetectionEnabled) + .build(); + ConfigurationInternal expectedConfigAfterChange = + new ConfigurationInternal.Builder(cachedConfigurationInternal) + .setGeoDetectionEnabledSetting(newGeoDetectionEnabled) + .build(); + + reset(mFakeServiceConfigAccessorSpy); + mTimeZoneDetectorStrategy.updateConfiguration( + currentUserConfig.getUserId(), requestedChanges, bypassUserPolicyChecks); + verify(mFakeServiceConfigAccessorSpy, times(1)).updateConfiguration( + currentUserConfig.getUserId(), requestedChanges, bypassUserPolicyChecks); + assertEquals(expectedConfigAfterChange, + mTimeZoneDetectorStrategy.getCachedCapabilitiesAndConfigForTests()); + } + } + + // Not current user behavior: the strategy reads from the ServiceConfigAccessor. + @Test + public void testReadAndWriteConfiguration_otherUser() throws Exception { + ConfigurationInternal currentUserConfig = CONFIG_AUTO_ENABLED_GEO_DISABLED; + mFakeServiceConfigAccessorSpy.simulateCurrentUserConfigurationInternalChange( + currentUserConfig); + + int otherUserId = currentUserConfig.getUserId() + 1; + ConfigurationInternal otherUserConfig = new ConfigurationInternal.Builder(currentUserConfig) + .setUserId(otherUserId) + .setGeoDetectionEnabledSetting(true) + .build(); + mFakeServiceConfigAccessorSpy.simulateOtherUserConfigurationInternalChange(otherUserConfig); + reset(mFakeServiceConfigAccessorSpy); + + final boolean bypassUserPolicyChecks = false; + + // Confirm getCapabilitiesAndConfig() does not call through to the ServiceConfigAccessor. + { + reset(mFakeServiceConfigAccessorSpy); + TimeZoneCapabilitiesAndConfig actualCapabilitiesAndConfig = + mTimeZoneDetectorStrategy.getCapabilitiesAndConfig( + otherUserId, bypassUserPolicyChecks); + verify(mFakeServiceConfigAccessorSpy, times(1)).getConfigurationInternal(otherUserId); + + assertEquals(otherUserConfig.asCapabilities(bypassUserPolicyChecks), + actualCapabilitiesAndConfig.getCapabilities()); + assertEquals(otherUserConfig.asConfiguration(), + actualCapabilitiesAndConfig.getConfiguration()); + } + + // Confirm updateConfiguration() calls through to the ServiceConfigAccessor and doesn't + // touch the cached copy. + { + ConfigurationInternal cachedConfigBeforeChange = + mTimeZoneDetectorStrategy.getCachedCapabilitiesAndConfigForTests(); + boolean newGeoDetectionEnabled = + !otherUserConfig.asConfiguration().isGeoDetectionEnabled(); + TimeZoneConfiguration requestedChanges = new TimeZoneConfiguration.Builder() + .setGeoDetectionEnabled(newGeoDetectionEnabled) + .build(); + + reset(mFakeServiceConfigAccessorSpy); + mTimeZoneDetectorStrategy.updateConfiguration( + currentUserConfig.getUserId(), requestedChanges, bypassUserPolicyChecks); + verify(mFakeServiceConfigAccessorSpy, times(1)).updateConfiguration( + currentUserConfig.getUserId(), requestedChanges, bypassUserPolicyChecks); + assertEquals(cachedConfigBeforeChange, + mTimeZoneDetectorStrategy.getCachedCapabilitiesAndConfigForTests()); + } } @Test @@ -1201,19 +1425,13 @@ public class TimeZoneDetectorStrategyImplTest { private final TestState mTimeZoneId = new TestState<>(); private final TestState mTimeZoneConfidence = new TestState<>(); - private ConfigurationInternal mConfigurationInternal; private @ElapsedRealtimeLong long mElapsedRealtimeMillis; - private ConfigurationChangeListener mConfigurationInternalChangeListener; FakeEnvironment() { // Ensure the fake environment starts with the defaults a fresh device would. initializeTimeZoneSetting("", TIME_ZONE_CONFIDENCE_LOW); } - void initializeConfig(ConfigurationInternal configurationInternal) { - mConfigurationInternal = configurationInternal; - } - void initializeClock(@ElapsedRealtimeLong long elapsedRealtimeMillis) { mElapsedRealtimeMillis = elapsedRealtimeMillis; } @@ -1227,16 +1445,6 @@ public class TimeZoneDetectorStrategyImplTest { mElapsedRealtimeMillis++; } - @Override - public void setConfigurationInternalChangeListener(ConfigurationChangeListener listener) { - mConfigurationInternalChangeListener = listener; - } - - @Override - public ConfigurationInternal getCurrentUserConfigurationInternal() { - return mConfigurationInternal; - } - @Override public String getDeviceTimeZone() { return mTimeZoneId.getLatest(); @@ -1254,11 +1462,6 @@ public class TimeZoneDetectorStrategyImplTest { mTimeZoneConfidence.set(confidence); } - void simulateConfigurationInternalChange(ConfigurationInternal configurationInternal) { - mConfigurationInternal = configurationInternal; - mConfigurationInternalChangeListener.onChange(); - } - void assertTimeZoneNotChanged() { mTimeZoneId.assertHasNotBeenSet(); mTimeZoneConfidence.assertHasNotBeenSet(); @@ -1322,7 +1525,8 @@ public class TimeZoneDetectorStrategyImplTest { * Simulates the user / user's configuration changing. */ Script simulateConfigurationInternalChange(ConfigurationInternal configurationInternal) { - mFakeEnvironment.simulateConfigurationInternalChange(configurationInternal); + mFakeServiceConfigAccessorSpy.simulateCurrentUserConfigurationInternalChange( + configurationInternal); return this; } @@ -1331,7 +1535,7 @@ public class TimeZoneDetectorStrategyImplTest { */ Script simulateSetAutoMode(boolean autoDetectionEnabled) { ConfigurationInternal newConfig = new ConfigurationInternal.Builder( - mFakeEnvironment.getCurrentUserConfigurationInternal()) + mFakeServiceConfigAccessorSpy.getCurrentUserConfigurationInternal()) .setAutoDetectionEnabledSetting(autoDetectionEnabled) .build(); simulateConfigurationInternalChange(newConfig); @@ -1343,7 +1547,7 @@ public class TimeZoneDetectorStrategyImplTest { */ Script simulateSetGeoDetectionEnabled(boolean geoDetectionEnabled) { ConfigurationInternal newConfig = new ConfigurationInternal.Builder( - mFakeEnvironment.getCurrentUserConfigurationInternal()) + mFakeServiceConfigAccessorSpy.getCurrentUserConfigurationInternal()) .setGeoDetectionEnabledSetting(geoDetectionEnabled) .build(); simulateConfigurationInternalChange(newConfig); @@ -1457,4 +1661,22 @@ public class TimeZoneDetectorStrategyImplTest { @MatchType int matchType, @Quality int quality, int expectedScore) { return new TelephonyTestCase(matchType, quality, expectedScore); } + + private static class TestStateChangeListener implements StateChangeListener { + + private int mNotificationsReceived; + + @Override + public void onChange() { + mNotificationsReceived++; + } + + public void resetNotificationsReceivedCount() { + mNotificationsReceived = 0; + } + + public void assertNotificationsReceived(int expectedCount) { + assertEquals(expectedCount, mNotificationsReceived); + } + } } diff --git a/services/tests/servicestests/src/com/android/server/timezonedetector/location/TestSupport.java b/services/tests/servicestests/src/com/android/server/timezonedetector/location/TestSupport.java index 2c3a7c43bc99e..042e3ef892f91 100644 --- a/services/tests/servicestests/src/com/android/server/timezonedetector/location/TestSupport.java +++ b/services/tests/servicestests/src/com/android/server/timezonedetector/location/TestSupport.java @@ -42,7 +42,8 @@ final class TestSupport { private static ConfigurationInternal createUserConfig( @UserIdInt int userId, boolean geoDetectionEnabledSetting) { - return new ConfigurationInternal.Builder(userId) + return new ConfigurationInternal.Builder() + .setUserId(userId) .setUserConfigAllowed(true) .setTelephonyDetectionFeatureSupported(true) .setGeoDetectionFeatureSupported(true)