diff options
| author | 2019-11-19 12:17:45 -0800 | |
|---|---|---|
| committer | 2019-11-21 14:51:17 -0800 | |
| commit | 4bd0a6a25996930669afc965f2ffb8a2a01673ac (patch) | |
| tree | 85d96735c25930667bcdfc64af2ff9a8a9769a9b | |
| parent | 35fbfcbd24f53ba695e3175603071f79129fc20f (diff) | |
Add DeviceConfig.setProperties method for atomic writing.
This method accepts multiple flags and overrides the values of all flags
in the provided namespace atomically. If any listeners are registered
for that namespace it will also trigger a callback with all of the flags
that were changed (added, updated, or removed).
Test: atest FrameworksCoreTests:DeviceConfigTest
atest FrameworksCoreTests:SettingsProviderTest
atest SettingsProviderTest:DeviceConfigServiceTest
Bug: 136135417
Change-Id: I2e496a3807493750ba8891dd6390055c919b5f45
6 files changed, 321 insertions, 17 deletions
diff --git a/api/system-current.txt b/api/system-current.txt index 412e0959570c..321e3f56be14 100644 --- a/api/system-current.txt +++ b/api/system-current.txt @@ -6158,6 +6158,7 @@ package android.provider { method @RequiresPermission(android.Manifest.permission.READ_DEVICE_CONFIG) public static String getString(@NonNull String, @NonNull String, @Nullable String); method public static void removeOnPropertiesChangedListener(@NonNull android.provider.DeviceConfig.OnPropertiesChangedListener); method @RequiresPermission(android.Manifest.permission.WRITE_DEVICE_CONFIG) public static void resetToDefaults(int, @Nullable String); + method @RequiresPermission(android.Manifest.permission.WRITE_DEVICE_CONFIG) public static boolean setProperties(@NonNull android.provider.DeviceConfig.Properties); method @RequiresPermission(android.Manifest.permission.WRITE_DEVICE_CONFIG) public static boolean setProperty(@NonNull String, @NonNull String, @Nullable String, boolean); field public static final String NAMESPACE_ACTIVITY_MANAGER = "activity_manager"; field public static final String NAMESPACE_ACTIVITY_MANAGER_NATIVE_BOOT = "activity_manager_native_boot"; diff --git a/core/java/android/provider/DeviceConfig.java b/core/java/android/provider/DeviceConfig.java index 0401d7fb8d5d..0c4a6d5f0232 100644 --- a/core/java/android/provider/DeviceConfig.java +++ b/core/java/android/provider/DeviceConfig.java @@ -424,8 +424,9 @@ public final class DeviceConfig { * Look up the values of multiple properties for a particular namespace. The lookup is atomic, * such that the values of these properties cannot change between the time when the first is * fetched and the time when the last is fetched. - * - * TODO: reference setProperties when it is added. + * <p> + * Each call to {@link #setProperties(Properties)} is also atomic and ensures that either none + * or all of the change is picked up here, but never only part of it. * * @param namespace The namespace containing the properties to look up. * @param names The names of properties to look up, or empty to fetch all properties for the @@ -593,6 +594,27 @@ public final class DeviceConfig { } /** + * Set all of the properties for a specific namespace. Pre-existing properties will be updated + * and new properties will be added if necessary. Any pre-existing properties for the specific + * namespace which are not part of the provided {@link Properties} object will be deleted from + * the namespace. These changes are all applied atomically, such that no calls to read or reset + * these properties can happen in the middle of this update. + * <p> + * Each call to {@link #getProperties(String, String...)} is also atomic and ensures that either + * none or all of this update is picked up, but never only part of it. + * + * @param properties the complete set of properties to set for a specific namespace. + * @hide + */ + @SystemApi + @RequiresPermission(WRITE_DEVICE_CONFIG) + public static boolean setProperties(@NonNull Properties properties) { + ContentResolver contentResolver = ActivityThread.currentApplication().getContentResolver(); + return Settings.Config.setStrings(contentResolver, properties.getNamespace(), + properties.mMap); + } + + /** * Reset properties to their default values. * <p> * The method accepts an optional namespace parameter. If provided, only properties set within @@ -736,23 +758,26 @@ public final class DeviceConfig { List<String> pathSegments = uri.getPathSegments(); // pathSegments(0) is "config" final String namespace = pathSegments.get(1); - final String name = pathSegments.get(2); - final String value; + Map<String, String> propertyMap = new ArrayMap<>(); try { - value = getProperty(namespace, name); + Properties allProperties = getProperties(namespace); + for (int i = 2; i < pathSegments.size(); ++i) { + String key = pathSegments.get(i); + propertyMap.put(key, allProperties.getString(key, null)); + } } catch (SecurityException e) { // Silently failing to not crash binder or listener threads. Log.e(TAG, "OnPropertyChangedListener update failed: permission violation."); return; } + Properties properties = new Properties(namespace, propertyMap); + synchronized (sLock) { for (int i = 0; i < sListeners.size(); i++) { if (namespace.equals(sListeners.valueAt(i).first)) { final OnPropertiesChangedListener listener = sListeners.keyAt(i); sListeners.valueAt(i).second.execute(() -> { - Map<String, String> propertyMap = new ArrayMap<>(1); - propertyMap.put(name, value); - listener.onPropertiesChanged(new Properties(namespace, propertyMap)); + listener.onPropertiesChanged(properties); }); } } @@ -774,7 +799,11 @@ public final class DeviceConfig { } /** - * Interface for monitoring changes to properties. + * Interface for monitoring changes to properties. Implementations will receive callbacks when + * properties change, including a {@link Properties} object which contains a single namespace + * and all of the properties which changed for that namespace. This includes properties which + * were added, updated, or deleted. This is not necessarily a complete list of all properties + * belonging to the namespace, as properties which don't change are omitted. * <p> * Override {@link #onPropertiesChanged(Properties)} to handle callbacks for changes. * @@ -784,10 +813,13 @@ public final class DeviceConfig { @TestApi public interface OnPropertiesChangedListener { /** - * Called when one or more properties have changed. + * Called when one or more properties have changed, providing a Properties object with all + * of the changed properties. This object will contain only properties which have changed, + * not the complete set of all properties belonging to the namespace. * * @param properties Contains the complete collection of properties which have changed for a - * single namespace. + * single namespace. This includes only those which were added, updated, + * or deleted. */ void onPropertiesChanged(@NonNull Properties properties); } diff --git a/core/java/android/provider/Settings.java b/core/java/android/provider/Settings.java index 3ac7deb0db08..cce0d4fb0ca6 100644 --- a/core/java/android/provider/Settings.java +++ b/core/java/android/provider/Settings.java @@ -1960,6 +1960,11 @@ public final class Settings { */ public static final String CALL_METHOD_PREFIX_KEY = "_prefix"; + /** + * @hide - String argument extra to the fast-path call()-based requests + */ + public static final String CALL_METHOD_FLAGS_KEY = "_flags"; + /** @hide - Private call() method to write to 'system' table */ public static final String CALL_METHOD_PUT_SYSTEM = "PUT_system"; @@ -1972,6 +1977,9 @@ public final class Settings { /** @hide - Private call() method to write to 'configuration' table */ public static final String CALL_METHOD_PUT_CONFIG = "PUT_config"; + /** @hide - Private call() method to write to and delete from the 'configuration' table */ + public static final String CALL_METHOD_SET_ALL_CONFIG = "SET_ALL_config"; + /** @hide - Private call() method to delete from the 'system' table */ public static final String CALL_METHOD_DELETE_SYSTEM = "DELETE_system"; @@ -2304,21 +2312,23 @@ public final class Settings { private final String mCallGetCommand; private final String mCallSetCommand; private final String mCallListCommand; + private final String mCallSetAllCommand; @GuardedBy("this") private GenerationTracker mGenerationTracker; public NameValueCache(Uri uri, String getCommand, String setCommand, ContentProviderHolder providerHolder) { - this(uri, getCommand, setCommand, null, providerHolder); + this(uri, getCommand, setCommand, null, null, providerHolder); } NameValueCache(Uri uri, String getCommand, String setCommand, String listCommand, - ContentProviderHolder providerHolder) { + String setAllCommand, ContentProviderHolder providerHolder) { mUri = uri; mCallGetCommand = getCommand; mCallSetCommand = setCommand; mCallListCommand = listCommand; + mCallSetAllCommand = setAllCommand; mProviderHolder = providerHolder; } @@ -2344,6 +2354,26 @@ public final class Settings { return true; } + public boolean setStringsForPrefix(ContentResolver cr, String prefix, + HashMap<String, String> keyValues) { + if (mCallSetAllCommand == null) { + // This NameValueCache does not support atomically setting multiple flags + return false; + } + try { + Bundle args = new Bundle(); + args.putString(CALL_METHOD_PREFIX_KEY, prefix); + args.putSerializable(CALL_METHOD_FLAGS_KEY, keyValues); + IContentProvider cp = mProviderHolder.getProvider(cr); + cp.call(cr.getPackageName(), cr.getFeatureId(), mProviderHolder.mUri.getAuthority(), + mCallSetAllCommand, null, args); + } catch (RemoteException e) { + // Not supported by the remote side + return false; + } + return true; + } + @UnsupportedAppUsage public String getStringForUser(ContentResolver cr, String name, final int userHandle) { final boolean isSelf = (userHandle == UserHandle.myUserId()); @@ -13718,6 +13748,7 @@ public final class Settings { CALL_METHOD_GET_CONFIG, CALL_METHOD_PUT_CONFIG, CALL_METHOD_LIST_CONFIG, + CALL_METHOD_SET_ALL_CONFIG, sProviderHolder); /** @@ -13792,6 +13823,29 @@ public final class Settings { } /** + * Clear all name/value pairs for the provided namespace and save new name/value pairs in + * their place. + * + * @param resolver to access the database with. + * @param namespace to which the names should be set. + * @param keyValues map of key names (without the prefix) to values. + * @return + * + * @hide + */ + @RequiresPermission(Manifest.permission.WRITE_DEVICE_CONFIG) + static boolean setStrings(@NonNull ContentResolver resolver, @NonNull String namespace, + @NonNull Map<String, String> keyValues) { + HashMap<String, String> compositeKeyValueMap = new HashMap<>(keyValues.keySet().size()); + for (Map.Entry<String, String> entry : keyValues.entrySet()) { + compositeKeyValueMap.put( + createCompositeName(namespace, entry.getKey()), entry.getValue()); + } + return sNameValueCache.setStringsForPrefix(resolver, createPrefix(namespace), + compositeKeyValueMap); + } + + /** * Reset the values to their defaults. * <p> * The method accepts an optional prefix parameter. If provided, only pairs with a name that diff --git a/core/tests/coretests/src/android/provider/DeviceConfigTest.java b/core/tests/coretests/src/android/provider/DeviceConfigTest.java index 0c83390f9f1e..8c1c3b569303 100644 --- a/core/tests/coretests/src/android/provider/DeviceConfigTest.java +++ b/core/tests/coretests/src/android/provider/DeviceConfigTest.java @@ -33,6 +33,9 @@ import org.junit.Assert; import org.junit.Test; import org.junit.runner.RunWith; +import java.util.HashMap; +import java.util.Map; + /** Tests that ensure appropriate settings are backed up. */ @Presubmit @RunWith(AndroidJUnit4.class) @@ -484,12 +487,74 @@ public class DeviceConfigTest { assertThat(properties.getString(KEY3, DEFAULT_VALUE)).isEqualTo(DEFAULT_VALUE); } - // TODO(mpape): resolve b/142727848 and re-enable this test + @Test + public void setProperties() { + Map<String, String> keyValues = new HashMap<>(); + keyValues.put(KEY, VALUE); + keyValues.put(KEY2, VALUE2); + + DeviceConfig.setProperties(new Properties(NAMESPACE, keyValues)); + Properties properties = DeviceConfig.getProperties(NAMESPACE); + assertThat(properties.getKeyset()).containsExactly(KEY, KEY2); + assertThat(properties.getString(KEY, DEFAULT_VALUE)).isEqualTo(VALUE); + assertThat(properties.getString(KEY2, DEFAULT_VALUE)).isEqualTo(VALUE2); + + Map<String, String> newKeyValues = new HashMap<>(); + newKeyValues.put(KEY, VALUE2); + newKeyValues.put(KEY3, VALUE3); + + DeviceConfig.setProperties(new Properties(NAMESPACE, newKeyValues)); + properties = DeviceConfig.getProperties(NAMESPACE); + assertThat(properties.getKeyset()).containsExactly(KEY, KEY3); + assertThat(properties.getString(KEY, DEFAULT_VALUE)).isEqualTo(VALUE2); + assertThat(properties.getString(KEY3, DEFAULT_VALUE)).isEqualTo(VALUE3); + + assertThat(properties.getKeyset()).doesNotContain(KEY2); + assertThat(properties.getString(KEY2, DEFAULT_VALUE)).isEqualTo(DEFAULT_VALUE); + } + + @Test + public void setProperties_multipleNamespaces() { + Map<String, String> keyValues = new HashMap<>(); + keyValues.put(KEY, VALUE); + keyValues.put(KEY2, VALUE2); + + Map<String, String> keyValues2 = new HashMap<>(); + keyValues2.put(KEY2, VALUE); + keyValues2.put(KEY3, VALUE2); + + final String namespace2 = "namespace2"; + DeviceConfig.setProperties(new Properties(NAMESPACE, keyValues)); + DeviceConfig.setProperties(new Properties(namespace2, keyValues2)); + + Properties properties = DeviceConfig.getProperties(NAMESPACE); + assertThat(properties.getKeyset()).containsExactly(KEY, KEY2); + assertThat(properties.getString(KEY, DEFAULT_VALUE)).isEqualTo(VALUE); + assertThat(properties.getString(KEY2, DEFAULT_VALUE)).isEqualTo(VALUE2); + + assertThat(properties.getKeyset()).doesNotContain(KEY3); + assertThat(properties.getString(KEY3, DEFAULT_VALUE)).isEqualTo(DEFAULT_VALUE); + + properties = DeviceConfig.getProperties(namespace2); + assertThat(properties.getKeyset()).containsExactly(KEY2, KEY3); + assertThat(properties.getString(KEY2, DEFAULT_VALUE)).isEqualTo(VALUE); + assertThat(properties.getString(KEY3, DEFAULT_VALUE)).isEqualTo(VALUE2); + + assertThat(properties.getKeyset()).doesNotContain(KEY); + assertThat(properties.getString(KEY, DEFAULT_VALUE)).isEqualTo(DEFAULT_VALUE); + + // clean up + deleteViaContentProvider(namespace2, KEY); + deleteViaContentProvider(namespace2, KEY2); + deleteViaContentProvider(namespace2, KEY3); + } + + // TODO(mpape): resolve b/142727848 and re-enable listener tests // @Test -// public void testOnPropertiesChangedListener() throws InterruptedException { +// public void onPropertiesChangedListener_setPropertyCallback() throws InterruptedException { // final CountDownLatch countDownLatch = new CountDownLatch(1); // -// OnPropertiesChangedListener changeListener = (properties) -> { +// DeviceConfig.OnPropertiesChangedListener changeListener = (properties) -> { // assertThat(properties.getNamespace()).isEqualTo(NAMESPACE); // assertThat(properties.getKeyset()).contains(KEY); // assertThat(properties.getString(KEY, "default_value")).isEqualTo(VALUE); @@ -508,6 +573,42 @@ public class DeviceConfigTest { // DeviceConfig.removeOnPropertiesChangedListener(changeListener); // } // } +// +// @Test +// public void onPropertiesChangedListener_setPropertiesCallback() throws InterruptedException { +// final CountDownLatch countDownLatch = new CountDownLatch(1); +// DeviceConfig.setProperty(NAMESPACE, KEY, VALUE, false); +// DeviceConfig.setProperty(NAMESPACE, KEY2, VALUE2, false); +// +// Map<String, String> keyValues = new HashMap<>(2); +// keyValues.put(KEY, VALUE2); +// keyValues.put(KEY3, VALUE3); +// Properties setProperties = new Properties(NAMESPACE, keyValues); +// +// DeviceConfig.OnPropertiesChangedListener changeListener = (properties) -> { +// assertThat(properties.getNamespace()).isEqualTo(NAMESPACE); +// assertThat(properties.getKeyset()).containsExactly(KEY, KEY2, KEY3); +// // KEY updated from VALUE to VALUE2 +// assertThat(properties.getString(KEY, "default_value")).isEqualTo(VALUE2); +// // KEY2 deleted (returns default_value) +// assertThat(properties.getString(KEY2, "default_value")).isEqualTo("default_value"); +// //KEY3 added with VALUE3 +// assertThat(properties.getString(KEY3, "default_value")).isEqualTo(VALUE3); +// countDownLatch.countDown(); +// }; +// +// try { +// DeviceConfig.addOnPropertiesChangedListener(NAMESPACE, +// ActivityThread.currentApplication().getMainExecutor(), changeListener); +// DeviceConfig.setProperties(setProperties); +// assertThat(countDownLatch.await( +// WAIT_FOR_PROPERTY_CHANGE_TIMEOUT_MILLIS, TimeUnit.MILLISECONDS)).isTrue(); +// } catch (InterruptedException e) { +// Assert.fail(e.getMessage()); +// } finally { +// DeviceConfig.removeOnPropertiesChangedListener(changeListener); +// } +// } private static boolean deleteViaContentProvider(String namespace, String key) { ContentResolver resolver = InstrumentationRegistry.getContext().getContentResolver(); diff --git a/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java b/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java index f5d1ccfe378b..0959de9238b5 100644 --- a/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java +++ b/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java @@ -101,6 +101,7 @@ import java.security.SecureRandom; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -383,6 +384,13 @@ public class SettingsProvider extends ContentProvider { break; } + case Settings.CALL_METHOD_SET_ALL_CONFIG: { + String prefix = getSettingPrefix(args); + Map<String, String> flags = getSettingFlags(args); + setAllConfigSettings(prefix, flags); + break; + } + case Settings.CALL_METHOD_RESET_CONFIG: { final int mode = getResetModeEnforcingPermission(args); String prefix = getSettingPrefix(args); @@ -1030,6 +1038,19 @@ public class SettingsProvider extends ContentProvider { MUTATION_OPERATION_INSERT, 0); } + private boolean setAllConfigSettings(String prefix, Map<String, String> keyValues) { + if (DEBUG) { + Slog.v(LOG_TAG, "setAllConfigSettings for prefix: " + prefix); + } + + enforceWritePermission(Manifest.permission.WRITE_DEVICE_CONFIG); + + synchronized (mLock) { + return mSettingsRegistry.setSettingsLocked(SETTINGS_TYPE_CONFIG, UserHandle.USER_SYSTEM, + prefix, keyValues, resolveCallingPackage()); + } + } + private boolean deleteConfigSetting(String name) { if (DEBUG) { Slog.v(LOG_TAG, "deleteConfigSetting(" + name + ")"); @@ -2117,6 +2138,11 @@ public class SettingsProvider extends ContentProvider { return (args != null) ? args.getString(Settings.CALL_METHOD_PREFIX_KEY) : null; } + private static Map<String, String> getSettingFlags(Bundle args) { + return (args != null) ? (HashMap) args.getSerializable(Settings.CALL_METHOD_FLAGS_KEY) + : Collections.emptyMap(); + } + private static boolean getSettingMakeDefault(Bundle args) { return (args != null) && args.getBoolean(Settings.CALL_METHOD_MAKE_DEFAULT_KEY); } @@ -2485,7 +2511,7 @@ public class SettingsProvider extends ContentProvider { final int key = makeKey(type, userId); SettingsState settingsState = peekSettingsStateLocked(key); if (settingsState == null) { - return new ArrayList<String>(); + return new ArrayList<>(); } return settingsState.getSettingNamesLocked(); } @@ -2645,6 +2671,22 @@ public class SettingsProvider extends ContentProvider { return success; } + public boolean setSettingsLocked(int type, int userId, String prefix, + Map<String, String> keyValues, String packageName) { + final int key = makeKey(type, userId); + + SettingsState settingsState = peekSettingsStateLocked(key); + if (settingsState != null) { + List<String> changedSettings = + settingsState.setSettingsLocked(prefix, keyValues, packageName); + if (!changedSettings.isEmpty()) { + notifyForConfigSettingsChangeLocked(key, prefix, changedSettings); + } + } + + return settingsState != null; + } + public boolean deleteSettingLocked(int type, int userId, String name, boolean forceNotify, Set<String> criticalSettings) { final int key = makeKey(type, userId); @@ -3043,6 +3085,28 @@ public class SettingsProvider extends ContentProvider { mHandler.obtainMessage(MyHandler.MSG_NOTIFY_DATA_CHANGED).sendToTarget(); } + private void notifyForConfigSettingsChangeLocked(int key, String prefix, + List<String> changedSettings) { + + // Increment the generation first, so observers always see the new value + mGenerationRegistry.incrementGeneration(key); + + StringBuilder stringBuilder = new StringBuilder(prefix); + for (int i = 0; i < changedSettings.size(); ++i) { + stringBuilder.append(changedSettings.get(i).split("/")[1]).append("/"); + } + + final long token = Binder.clearCallingIdentity(); + try { + notifySettingChangeForRunningUsers(key, stringBuilder.toString()); + } finally { + Binder.restoreCallingIdentity(token); + } + + // Always notify that our data changed + mHandler.obtainMessage(MyHandler.MSG_NOTIFY_DATA_CHANGED).sendToTarget(); + } + private void maybeNotifyProfiles(int type, int userId, Uri uri, String name, Collection<String> keysCloned) { if (keysCloned.contains(name)) { diff --git a/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java b/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java index de6a3a8840a4..4731e6894baf 100644 --- a/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java +++ b/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java @@ -64,6 +64,7 @@ import java.io.PrintWriter; import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.List; +import java.util.Map; import java.util.Objects; /** @@ -416,6 +417,57 @@ final class SettingsState { } // The settings provider must hold its lock when calling here. + // Returns the list of keys which changed (added, updated, or deleted). + @GuardedBy("mLock") + public List<String> setSettingsLocked(String prefix, Map<String, String> keyValues, + String packageName) { + List<String> changedKeys = new ArrayList<>(); + // Delete old keys with the prefix that are not part of the new set. + for (int i = 0; i < mSettings.keySet().size(); ++i) { + String key = mSettings.keyAt(i); + if (key.startsWith(prefix) && !keyValues.containsKey(key)) { + Setting oldState = mSettings.remove(key); + + StatsLog.write(StatsLog.SETTING_CHANGED, key, /* value= */ "", /* newValue= */ "", + oldState.value, /* tag */ "", false, getUserIdFromKey(mKey), + StatsLog.SETTING_CHANGED__REASON__DELETED); + addHistoricalOperationLocked(HISTORICAL_OPERATION_DELETE, oldState); + changedKeys.add(key); // key was removed + } + } + + // Update/add new keys + for (String key : keyValues.keySet()) { + String value = keyValues.get(key); + String oldValue = null; + Setting state = mSettings.get(key); + if (state == null) { + state = new Setting(key, value, false, packageName, null); + mSettings.put(key, state); + changedKeys.add(key); // key was added + } else if (state.value != value) { + oldValue = state.value; + state.update(value, false, packageName, null, true); + changedKeys.add(key); // key was updated + } else { + // this key/value already exists, no change and no logging necessary + continue; + } + + StatsLog.write(StatsLog.SETTING_CHANGED, key, value, state.value, oldValue, + /* tag */ null, /* make default */ false, + getUserIdFromKey(mKey), StatsLog.SETTING_CHANGED__REASON__UPDATED); + addHistoricalOperationLocked(HISTORICAL_OPERATION_UPDATE, state); + } + + if (!changedKeys.isEmpty()) { + scheduleWriteIfNeededLocked(); + } + + return changedKeys; + } + + // The settings provider must hold its lock when calling here. public void persistSyncLocked() { mHandler.removeMessages(MyHandler.MSG_PERSIST_SETTINGS); doWriteState(); |