From b8e316c4904b18d8d704249338bd336c5a9da522 Mon Sep 17 00:00:00 2001 From: Neil Fuller Date: Mon, 28 Jun 2021 14:13:15 +0100 Subject: [PATCH] Tidy up naming around "sync disabled" behavior Tidy up naming around the DeviceConfig "sync disabled" behavior. Adds "mode" to method names make the behavior more obvious / consistent, as suggested in the review for http://ag/15083070. Various strings and constants have been updated to track this. Some javadoc improvements are included for improved clarity. This is an internal refactoring only. The command line verbs have been left as "get_sync_disabled_for_tests" and "set_sync_disabled_for_tests". Bug: 185786624 Test: treehugger only Change-Id: I00a472966b0a8e2658a1d3365b9beb9a47c58191 --- core/java/android/provider/DeviceConfig.java | 14 +++---- core/java/android/provider/Settings.java | 39 +++++++++++-------- .../android/provider/DeviceConfigTest.java | 22 +++++------ .../settings/DeviceConfigService.java | 36 ++++++++--------- .../providers/settings/SettingsProvider.java | 28 ++++++------- 5 files changed, 72 insertions(+), 67 deletions(-) diff --git a/core/java/android/provider/DeviceConfig.java b/core/java/android/provider/DeviceConfig.java index 712fd77927644..5d99dbab4dc34 100644 --- a/core/java/android/provider/DeviceConfig.java +++ b/core/java/android/provider/DeviceConfig.java @@ -860,31 +860,31 @@ public final class DeviceConfig { /** * Disables or re-enables bulk modifications ({@link #setProperties(Properties)}) to device * config values. This is intended for use during tests to prevent a sync operation clearing - * config values, which could influence the outcome of the tests, i.e. by changing behavior. + * config values which could influence the outcome of the tests, i.e. by changing behavior. * * @param syncDisabledMode the mode to use, see {@link Settings.Config#SYNC_DISABLED_MODE_NONE}, * {@link Settings.Config#SYNC_DISABLED_MODE_PERSISTENT} and {@link * Settings.Config#SYNC_DISABLED_MODE_UNTIL_REBOOT} * - * @see #getSyncDisabled() + * @see #getSyncDisabledMode() * @hide */ @RequiresPermission(WRITE_DEVICE_CONFIG) - public static void setSyncDisabled(@SyncDisabledMode int syncDisabledMode) { + public static void setSyncDisabledMode(@SyncDisabledMode int syncDisabledMode) { ContentResolver contentResolver = ActivityThread.currentApplication().getContentResolver(); - Settings.Config.setSyncDisabled(contentResolver, syncDisabledMode); + Settings.Config.setSyncDisabledMode(contentResolver, syncDisabledMode); } /** * Returns the current mode of sync disabling. * - * @see #setSyncDisabled(int) + * @see #setSyncDisabledMode(int) * @hide */ @RequiresPermission(WRITE_DEVICE_CONFIG) - public static @SyncDisabledMode int getSyncDisabled() { + public static @SyncDisabledMode int getSyncDisabledMode() { ContentResolver contentResolver = ActivityThread.currentApplication().getContentResolver(); - return Settings.Config.getSyncDisabled(contentResolver); + return Settings.Config.getSyncDisabledMode(contentResolver); } /** diff --git a/core/java/android/provider/Settings.java b/core/java/android/provider/Settings.java index 4efe0fd8d5e5c..414d103f24f23 100644 --- a/core/java/android/provider/Settings.java +++ b/core/java/android/provider/Settings.java @@ -299,8 +299,8 @@ public final class Settings { public static final String KEY_CONFIG_SET_ALL_RETURN = "config_set_all_return"; /** @hide */ - public static final String KEY_CONFIG_GET_SYNC_DISABLED_RETURN = - "config_get_sync_disabled_return"; + public static final String KEY_CONFIG_GET_SYNC_DISABLED_MODE_RETURN = + "config_get_sync_disabled_mode_return"; /** * An int extra specifying a subscription ID. @@ -2439,13 +2439,15 @@ public final class Settings { public static final String CALL_METHOD_LIST_CONFIG = "LIST_config"; /** @hide - Private call() method to disable / re-enable syncs to the 'configuration' table */ - public static final String CALL_METHOD_SET_SYNC_DISABLED_CONFIG = "SET_SYNC_DISABLED_config"; + public static final String CALL_METHOD_SET_SYNC_DISABLED_MODE_CONFIG = + "SET_SYNC_DISABLED_MODE_config"; /** * @hide - Private call() method to return the current mode of sync disabling for the * 'configuration' table */ - public static final String CALL_METHOD_GET_SYNC_DISABLED_CONFIG = "GET_SYNC_DISABLED_config"; + public static final String CALL_METHOD_GET_SYNC_DISABLED_MODE_CONFIG = + "GET_SYNC_DISABLED_MODE_config"; /** @hide - Private call() method to register monitor callback for 'configuration' table */ public static final String CALL_METHOD_REGISTER_MONITOR_CALLBACK_CONFIG = @@ -14335,8 +14337,11 @@ public final class Settings { "are_user_disabled_hdr_formats_allowed"; /** - * Whether or not syncs (bulk set operations) for {@link DeviceConfig} are disabled - * currently. The value is boolean (1 or 0). The value '1' means that {@link + * Whether or not syncs (bulk set operations) for {@link DeviceConfig} are currently + * persistently disabled. This is only used for the {@link + * Config#SYNC_DISABLED_MODE_PERSISTENT persistent} mode, {@link + * Config#SYNC_DISABLED_MODE_UNTIL_REBOOT until_reboot} mode is not stored in settings. + * The value is boolean (1 or 0). The value '1' means that {@link * DeviceConfig#setProperties(DeviceConfig.Properties)} will return {@code false}. * * @hide @@ -16663,45 +16668,45 @@ public final class Settings { } /** - * Bridge method between {@link DeviceConfig#setSyncDisabled(int)} and the + * Bridge method between {@link DeviceConfig#setSyncDisabledMode(int)} and the * {@link com.android.providers.settings.SettingsProvider} implementation. * * @hide */ @SuppressLint("AndroidFrameworkRequiresPermission") @RequiresPermission(Manifest.permission.WRITE_DEVICE_CONFIG) - static void setSyncDisabled( + static void setSyncDisabledMode( @NonNull ContentResolver resolver, @SyncDisabledMode int disableSyncMode) { try { Bundle args = new Bundle(); args.putInt(CALL_METHOD_SYNC_DISABLED_MODE_KEY, disableSyncMode); IContentProvider cp = sProviderHolder.getProvider(resolver); - cp.call(resolver.getAttributionSource(), - sProviderHolder.mUri.getAuthority(), CALL_METHOD_SET_SYNC_DISABLED_CONFIG, - null, args); + cp.call(resolver.getAttributionSource(), sProviderHolder.mUri.getAuthority(), + CALL_METHOD_SET_SYNC_DISABLED_MODE_CONFIG, null, args); } catch (RemoteException e) { - Log.w(TAG, "Can't set sync disabled " + DeviceConfig.CONTENT_URI, e); + Log.w(TAG, "Can't set sync disabled mode " + DeviceConfig.CONTENT_URI, e); } } /** - * Bridge method between {@link DeviceConfig#getSyncDisabled()} and the + * Bridge method between {@link DeviceConfig#getSyncDisabledMode()} and the * {@link com.android.providers.settings.SettingsProvider} implementation. * * @hide */ @SuppressLint("AndroidFrameworkRequiresPermission") @RequiresPermission(Manifest.permission.WRITE_DEVICE_CONFIG) - static int getSyncDisabled(@NonNull ContentResolver resolver) { + static int getSyncDisabledMode(@NonNull ContentResolver resolver) { try { Bundle args = Bundle.EMPTY; IContentProvider cp = sProviderHolder.getProvider(resolver); Bundle bundle = cp.call(resolver.getAttributionSource(), - sProviderHolder.mUri.getAuthority(), CALL_METHOD_GET_SYNC_DISABLED_CONFIG, + sProviderHolder.mUri.getAuthority(), + CALL_METHOD_GET_SYNC_DISABLED_MODE_CONFIG, null, args); - return bundle.getInt(KEY_CONFIG_GET_SYNC_DISABLED_RETURN); + return bundle.getInt(KEY_CONFIG_GET_SYNC_DISABLED_MODE_RETURN); } catch (RemoteException e) { - Log.w(TAG, "Can't query sync disabled " + DeviceConfig.CONTENT_URI, e); + Log.w(TAG, "Can't query sync disabled mode " + DeviceConfig.CONTENT_URI, e); } return -1; } diff --git a/core/tests/coretests/src/android/provider/DeviceConfigTest.java b/core/tests/coretests/src/android/provider/DeviceConfigTest.java index 17f7fbf41ba5d..e53fc07bb32c0 100644 --- a/core/tests/coretests/src/android/provider/DeviceConfigTest.java +++ b/core/tests/coretests/src/android/provider/DeviceConfigTest.java @@ -773,10 +773,10 @@ public class DeviceConfigTest { try { // Ensure the device starts in a known state. - DeviceConfig.setSyncDisabled(Settings.Config.SYNC_DISABLED_MODE_NONE); + DeviceConfig.setSyncDisabledMode(Settings.Config.SYNC_DISABLED_MODE_NONE); // Assert starting state. - assertThat(DeviceConfig.getSyncDisabled()) + assertThat(DeviceConfig.getSyncDisabledMode()) .isEqualTo(Settings.Config.SYNC_DISABLED_MODE_NONE); assertThat(DeviceConfig.setProperties(properties1)).isTrue(); assertThat(DeviceConfig.getProperties(NAMESPACE, KEY).getString(KEY, DEFAULT_VALUE)) @@ -784,16 +784,16 @@ public class DeviceConfigTest { // Test disabled (persistent). Persistence is not actually tested, that would require // a host test. - DeviceConfig.setSyncDisabled(Settings.Config.SYNC_DISABLED_MODE_PERSISTENT); - assertThat(DeviceConfig.getSyncDisabled()) + DeviceConfig.setSyncDisabledMode(Settings.Config.SYNC_DISABLED_MODE_PERSISTENT); + assertThat(DeviceConfig.getSyncDisabledMode()) .isEqualTo(Settings.Config.SYNC_DISABLED_MODE_PERSISTENT); assertThat(DeviceConfig.setProperties(properties2)).isFalse(); assertThat(DeviceConfig.getProperties(NAMESPACE, KEY).getString(KEY, DEFAULT_VALUE)) .isEqualTo(VALUE); // Return to not disabled. - DeviceConfig.setSyncDisabled(Settings.Config.SYNC_DISABLED_MODE_NONE); - assertThat(DeviceConfig.getSyncDisabled()) + DeviceConfig.setSyncDisabledMode(Settings.Config.SYNC_DISABLED_MODE_NONE); + assertThat(DeviceConfig.getSyncDisabledMode()) .isEqualTo(Settings.Config.SYNC_DISABLED_MODE_NONE); assertThat(DeviceConfig.setProperties(properties2)).isTrue(); assertThat(DeviceConfig.getProperties(NAMESPACE, KEY).getString(KEY, DEFAULT_VALUE)) @@ -801,23 +801,23 @@ public class DeviceConfigTest { // Test disabled (persistent). Absence of persistence is not actually tested, that would // require a host test. - DeviceConfig.setSyncDisabled(Settings.Config.SYNC_DISABLED_MODE_UNTIL_REBOOT); - assertThat(DeviceConfig.getSyncDisabled()) + DeviceConfig.setSyncDisabledMode(Settings.Config.SYNC_DISABLED_MODE_UNTIL_REBOOT); + assertThat(DeviceConfig.getSyncDisabledMode()) .isEqualTo(Settings.Config.SYNC_DISABLED_MODE_UNTIL_REBOOT); assertThat(DeviceConfig.setProperties(properties1)).isFalse(); assertThat(DeviceConfig.getProperties(NAMESPACE, KEY).getString(KEY, DEFAULT_VALUE)) .isEqualTo(VALUE2); // Return to not disabled. - DeviceConfig.setSyncDisabled(Settings.Config.SYNC_DISABLED_MODE_NONE); - assertThat(DeviceConfig.getSyncDisabled()) + DeviceConfig.setSyncDisabledMode(Settings.Config.SYNC_DISABLED_MODE_NONE); + assertThat(DeviceConfig.getSyncDisabledMode()) .isEqualTo(Settings.Config.SYNC_DISABLED_MODE_NONE); assertThat(DeviceConfig.setProperties(properties1)).isTrue(); assertThat(DeviceConfig.getProperties(NAMESPACE, KEY).getString(KEY, DEFAULT_VALUE)) .isEqualTo(VALUE); } finally { // Try to return to the default sync disabled state in case of failure. - DeviceConfig.setSyncDisabled(Settings.Config.SYNC_DISABLED_MODE_NONE); + DeviceConfig.setSyncDisabledMode(Settings.Config.SYNC_DISABLED_MODE_NONE); // NAMESPACE will be cleared by cleanUp() } diff --git a/packages/SettingsProvider/src/com/android/providers/settings/DeviceConfigService.java b/packages/SettingsProvider/src/com/android/providers/settings/DeviceConfigService.java index 059291ebf49ce..f83ebaacb8e63 100644 --- a/packages/SettingsProvider/src/com/android/providers/settings/DeviceConfigService.java +++ b/packages/SettingsProvider/src/com/android/providers/settings/DeviceConfigService.java @@ -118,7 +118,7 @@ public final class DeviceConfigService extends Binder { } // Parse args for those commands that have them. - int disableSyncMode = -1; + int syncDisabledModeArg = -1; int resetMode = -1; boolean makeDefault = false; String namespace = null; @@ -155,10 +155,10 @@ public final class DeviceConfigService extends Binder { } } } else if (verb == CommandVerb.SET_SYNC_DISABLED_FOR_TESTS) { - if (disableSyncMode == -1) { - // DISABLE_SYNC_FOR_TESTS 1st arg (required) - disableSyncMode = parseDisableSyncMode(arg); - if (disableSyncMode == -1) { + if (syncDisabledModeArg == -1) { + // SET_SYNC_DISABLED_FOR_TESTS 1st arg (required) + syncDisabledModeArg = parseSyncDisabledMode(arg); + if (syncDisabledModeArg == -1) { // invalid perr.println("Invalid sync disabled mode: " + arg); return -1; @@ -248,13 +248,13 @@ public final class DeviceConfigService extends Binder { DeviceConfig.resetToDefaults(resetMode, namespace); break; case SET_SYNC_DISABLED_FOR_TESTS: - DeviceConfig.setSyncDisabled(disableSyncMode); + DeviceConfig.setSyncDisabledMode(syncDisabledModeArg); break; case GET_SYNC_DISABLED_FOR_TESTS: - int syncDisabledMode = DeviceConfig.getSyncDisabled(); - String syncDisabledModeString = formatDisableSyncMode(syncDisabledMode); + int syncDisabledModeInt = DeviceConfig.getSyncDisabledMode(); + String syncDisabledModeString = formatSyncDisabledMode(syncDisabledModeInt); if (syncDisabledModeString == null) { - perr.println("Unknown mode:" + syncDisabledMode); + perr.println("Unknown mode: " + syncDisabledModeInt); return -1; } pout.println(syncDisabledModeString); @@ -362,22 +362,22 @@ public final class DeviceConfigService extends Binder { } } - private static @SyncDisabledMode int parseDisableSyncMode(String arg) { - int disableSyncMode; + private static @SyncDisabledMode int parseSyncDisabledMode(String arg) { + int syncDisabledMode; if ("none".equalsIgnoreCase(arg)) { - disableSyncMode = SYNC_DISABLED_MODE_NONE; + syncDisabledMode = SYNC_DISABLED_MODE_NONE; } else if ("persistent".equalsIgnoreCase(arg)) { - disableSyncMode = SYNC_DISABLED_MODE_PERSISTENT; + syncDisabledMode = SYNC_DISABLED_MODE_PERSISTENT; } else if ("until_reboot".equalsIgnoreCase(arg)) { - disableSyncMode = SYNC_DISABLED_MODE_UNTIL_REBOOT; + syncDisabledMode = SYNC_DISABLED_MODE_UNTIL_REBOOT; } else { - disableSyncMode = -1; + syncDisabledMode = -1; } - return disableSyncMode; + return syncDisabledMode; } - private static String formatDisableSyncMode(@SyncDisabledMode int disableSyncMode) { - switch (disableSyncMode) { + private static String formatSyncDisabledMode(@SyncDisabledMode int syncDisabledMode) { + switch (syncDisabledMode) { case SYNC_DISABLED_MODE_NONE: return "none"; case SYNC_DISABLED_MODE_PERSISTENT: diff --git a/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java b/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java index 67ddbc2d994ff..ef9fd96696b35 100644 --- a/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java +++ b/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java @@ -471,16 +471,16 @@ public class SettingsProvider extends ContentProvider { return result; } - case Settings.CALL_METHOD_SET_SYNC_DISABLED_CONFIG: { + case Settings.CALL_METHOD_SET_SYNC_DISABLED_MODE_CONFIG: { final int mode = getSyncDisabledMode(args); - setSyncDisabledConfig(mode); + setSyncDisabledModeConfig(mode); break; } - case Settings.CALL_METHOD_GET_SYNC_DISABLED_CONFIG: { + case Settings.CALL_METHOD_GET_SYNC_DISABLED_MODE_CONFIG: { Bundle result = new Bundle(); - result.putInt(Settings.KEY_CONFIG_GET_SYNC_DISABLED_RETURN, - getSyncDisabledConfig()); + result.putInt(Settings.KEY_CONFIG_GET_SYNC_DISABLED_MODE_RETURN, + getSyncDisabledModeConfig()); return result; } @@ -1147,7 +1147,7 @@ public class SettingsProvider extends ContentProvider { enforceWritePermission(Manifest.permission.WRITE_DEVICE_CONFIG); synchronized (mLock) { - if (getSyncDisabledConfigLocked() != SYNC_DISABLED_MODE_NONE) { + if (getSyncDisabledModeConfigLocked() != SYNC_DISABLED_MODE_NONE) { return SET_ALL_RESULT_DISABLED; } final int key = makeKey(SETTINGS_TYPE_CONFIG, UserHandle.USER_SYSTEM); @@ -1157,32 +1157,32 @@ public class SettingsProvider extends ContentProvider { } } - private void setSyncDisabledConfig(@SyncDisabledMode int syncDisabledMode) { + private void setSyncDisabledModeConfig(@SyncDisabledMode int syncDisabledMode) { if (DEBUG) { - Slog.v(LOG_TAG, "setSyncDisabledConfig(" + syncDisabledMode + ")"); + Slog.v(LOG_TAG, "setSyncDisabledModeConfig(" + syncDisabledMode + ")"); } enforceWritePermission(Manifest.permission.WRITE_DEVICE_CONFIG); synchronized (mLock) { - setSyncDisabledConfigLocked(syncDisabledMode); + setSyncDisabledModeConfigLocked(syncDisabledMode); } } - private int getSyncDisabledConfig() { + private int getSyncDisabledModeConfig() { if (DEBUG) { - Slog.v(LOG_TAG, "getSyncDisabledConfig"); + Slog.v(LOG_TAG, "getSyncDisabledModeConfig"); } enforceWritePermission(Manifest.permission.WRITE_DEVICE_CONFIG); synchronized (mLock) { - return getSyncDisabledConfigLocked(); + return getSyncDisabledModeConfigLocked(); } } @GuardedBy("mLock") - private void setSyncDisabledConfigLocked(@SyncDisabledMode int syncDisabledMode) { + private void setSyncDisabledModeConfigLocked(@SyncDisabledMode int syncDisabledMode) { boolean persistentValue; boolean inMemoryValue; if (syncDisabledMode == SYNC_DISABLED_MODE_NONE) { @@ -1214,7 +1214,7 @@ public class SettingsProvider extends ContentProvider { } @GuardedBy("mLock") - private int getSyncDisabledConfigLocked() { + private int getSyncDisabledModeConfigLocked() { // Check the values used for both SYNC_DISABLED_MODE_PERSISTENT and // SYNC_DISABLED_MODE_UNTIL_REBOOT.