From 9509bcab3f5db2fe586bf33564151d39563e2f1f Mon Sep 17 00:00:00 2001 From: Songchun Fan Date: Fri, 20 Nov 2020 18:41:33 -0800 Subject: [PATCH] [SettingsProvider] allow null values in validators As discussed in b/165943447, null is an important indicator that a sepecific Settings element is unset, and should be treated as valid values that can be backed up. We should remove the non-null restriction for most of the validators, especially because SettingsBackupAgent already can handle null values. BUG: 170131663 Test: atest SettingsValidatorsTest Test: atest SettingsBackupAgentTest Change-Id: Ie44a6de503d83716c63be5c8e7937f8c9eb928bc --- .../validators/DiscreteValueValidator.java | 2 +- .../validators/GlobalSettingsValidators.java | 4 --- .../InclusiveFloatRangeValidator.java | 3 ++ .../InclusiveIntegerRangeValidator.java | 3 ++ .../validators/SecureSettingsValidators.java | 5 --- .../validators/SettingsValidators.java | 32 ++++++++++++----- .../validators/SystemSettingsValidators.java | 6 ---- .../validators/SettingsValidatorsTest.java | 36 +++++++++---------- 8 files changed, 49 insertions(+), 42 deletions(-) diff --git a/packages/SettingsProvider/src/android/provider/settings/validators/DiscreteValueValidator.java b/packages/SettingsProvider/src/android/provider/settings/validators/DiscreteValueValidator.java index 183651f77f5a9..e7d870ee75dac 100644 --- a/packages/SettingsProvider/src/android/provider/settings/validators/DiscreteValueValidator.java +++ b/packages/SettingsProvider/src/android/provider/settings/validators/DiscreteValueValidator.java @@ -34,6 +34,6 @@ public final class DiscreteValueValidator implements Validator { @Override public boolean validate(@Nullable String value) { - return ArrayUtils.contains(mValues, value); + return value == null || ArrayUtils.contains(mValues, value); } } diff --git a/packages/SettingsProvider/src/android/provider/settings/validators/GlobalSettingsValidators.java b/packages/SettingsProvider/src/android/provider/settings/validators/GlobalSettingsValidators.java index a02d67fd7bca0..668e26751b0c6 100644 --- a/packages/SettingsProvider/src/android/provider/settings/validators/GlobalSettingsValidators.java +++ b/packages/SettingsProvider/src/android/provider/settings/validators/GlobalSettingsValidators.java @@ -34,10 +34,6 @@ import java.util.Map; * Validators for Global settings */ public class GlobalSettingsValidators { - /** - * All settings in {@link Global.SETTINGS_TO_BACKUP} array *must* have a non-null validator, - * otherwise they won't be restored. - */ public static final Map VALIDATORS = new ArrayMap<>(); static { diff --git a/packages/SettingsProvider/src/android/provider/settings/validators/InclusiveFloatRangeValidator.java b/packages/SettingsProvider/src/android/provider/settings/validators/InclusiveFloatRangeValidator.java index 1a0b88c1c1501..58620b5f4c8f1 100644 --- a/packages/SettingsProvider/src/android/provider/settings/validators/InclusiveFloatRangeValidator.java +++ b/packages/SettingsProvider/src/android/provider/settings/validators/InclusiveFloatRangeValidator.java @@ -34,6 +34,9 @@ final class InclusiveFloatRangeValidator implements Validator { @Override public boolean validate(@Nullable String value) { + if (value == null) { + return true; + } try { final float floatValue = Float.parseFloat(value); return floatValue >= mMin && floatValue <= mMax; diff --git a/packages/SettingsProvider/src/android/provider/settings/validators/InclusiveIntegerRangeValidator.java b/packages/SettingsProvider/src/android/provider/settings/validators/InclusiveIntegerRangeValidator.java index f9f8ce8517193..aa27c877a7860 100644 --- a/packages/SettingsProvider/src/android/provider/settings/validators/InclusiveIntegerRangeValidator.java +++ b/packages/SettingsProvider/src/android/provider/settings/validators/InclusiveIntegerRangeValidator.java @@ -34,6 +34,9 @@ final class InclusiveIntegerRangeValidator implements Validator { @Override public boolean validate(@Nullable String value) { + if (value == null) { + return true; + } try { final int intValue = Integer.parseInt(value); return intValue >= mMin && intValue <= mMax; diff --git a/packages/SettingsProvider/src/android/provider/settings/validators/SecureSettingsValidators.java b/packages/SettingsProvider/src/android/provider/settings/validators/SecureSettingsValidators.java index d16aebb4d4b4a..df8abdd595530 100644 --- a/packages/SettingsProvider/src/android/provider/settings/validators/SecureSettingsValidators.java +++ b/packages/SettingsProvider/src/android/provider/settings/validators/SecureSettingsValidators.java @@ -42,11 +42,6 @@ import java.util.Map; * Validators for the Secure Settings. */ public class SecureSettingsValidators { - /** - * All settings in {@link Secure.SETTINGS_TO_BACKUP} and {@link - * DeviceSpecificSettings.DEVICE_SPECIFIC_SETTINGS_TO_BACKUP} array *must* have a non-null - * validator, otherwise they won't be restored. - */ public static final Map VALIDATORS = new ArrayMap<>(); static { diff --git a/packages/SettingsProvider/src/android/provider/settings/validators/SettingsValidators.java b/packages/SettingsProvider/src/android/provider/settings/validators/SettingsValidators.java index 8d5c6e69b850c..97e1d6848af66 100644 --- a/packages/SettingsProvider/src/android/provider/settings/validators/SettingsValidators.java +++ b/packages/SettingsProvider/src/android/provider/settings/validators/SettingsValidators.java @@ -16,6 +16,7 @@ package android.provider.settings.validators; +import android.annotation.NonNull; import android.annotation.Nullable; import android.content.ComponentName; import android.net.Uri; @@ -48,6 +49,9 @@ public class SettingsValidators { public static final Validator NON_NEGATIVE_INTEGER_VALIDATOR = new Validator() { @Override public boolean validate(@Nullable String value) { + if (value == null) { + return true; + } try { return Integer.parseInt(value) >= 0; } catch (NumberFormatException e) { @@ -59,6 +63,9 @@ public class SettingsValidators { public static final Validator ANY_INTEGER_VALIDATOR = new Validator() { @Override public boolean validate(@Nullable String value) { + if (value == null) { + return true; + } try { Integer.parseInt(value); return true; @@ -71,6 +78,9 @@ public class SettingsValidators { public static final Validator URI_VALIDATOR = new Validator() { @Override public boolean validate(@Nullable String value) { + if (value == null) { + return true; + } try { Uri.decode(value); return true; @@ -87,7 +97,7 @@ public class SettingsValidators { */ public static final Validator COMPONENT_NAME_VALIDATOR = new Validator() { @Override - public boolean validate(@Nullable String value) { + public boolean validate(@NonNull String value) { return value != null && ComponentName.unflattenFromString(value) != null; } }; @@ -104,18 +114,15 @@ public class SettingsValidators { public static final Validator PACKAGE_NAME_VALIDATOR = new Validator() { @Override - public boolean validate(@Nullable String value) { + public boolean validate(@NonNull String value) { return value != null && isStringPackageName(value); } - private boolean isStringPackageName(String value) { + private boolean isStringPackageName(@NonNull String value) { // The name may contain uppercase or lowercase letters ('A' through 'Z'), numbers, // and underscores ('_'). However, individual package name parts may only // start with letters. // (https://developer.android.com/guide/topics/manifest/manifest-element.html#package) - if (value == null) { - return false; - } String[] subparts = value.split("\\."); boolean isValidPackageName = true; for (String subpart : subparts) { @@ -143,7 +150,7 @@ public class SettingsValidators { @Override public boolean validate(@Nullable String value) { if (value == null) { - return false; + return true; } return value.length() <= MAX_IPV6_LENGTH; } @@ -153,7 +160,7 @@ public class SettingsValidators { @Override public boolean validate(@Nullable String value) { if (value == null) { - return false; + return true; } Locale[] validLocales = Locale.getAvailableLocales(); for (Locale locale : validLocales) { @@ -167,6 +174,9 @@ public class SettingsValidators { /** {@link Validator} that checks whether a value is a valid {@link JSONObject}. */ public static final Validator JSON_OBJECT_VALIDATOR = (value) -> { + if (value == null) { + return true; + } if (TextUtils.isEmpty(value)) { return false; } @@ -184,6 +194,9 @@ public class SettingsValidators { static final Validator DATE_FORMAT_VALIDATOR = value -> { try { + if (value == null) { + return true; + } new SimpleDateFormat(value); return true; } catch (IllegalArgumentException | NullPointerException e) { @@ -211,6 +224,9 @@ public class SettingsValidators { static final Validator NONE_NEGATIVE_LONG_VALIDATOR = new Validator() { @Override public boolean validate(String value) { + if (value == null) { + return true; + } try { return Long.parseLong(value) >= 0; } catch (NumberFormatException e) { diff --git a/packages/SettingsProvider/src/android/provider/settings/validators/SystemSettingsValidators.java b/packages/SettingsProvider/src/android/provider/settings/validators/SystemSettingsValidators.java index c5d4fa9f1b409..539e687decdfa 100644 --- a/packages/SettingsProvider/src/android/provider/settings/validators/SystemSettingsValidators.java +++ b/packages/SettingsProvider/src/android/provider/settings/validators/SystemSettingsValidators.java @@ -38,12 +38,6 @@ import java.util.Map; * Validators for System settings */ public class SystemSettingsValidators { - /** - * These are all public system settings - * - *

All settings in {@link System.SETTINGS_TO_BACKUP} array *must* have a non-null validator, - * otherwise they won't be restored. - */ @UnsupportedAppUsage public static final Map VALIDATORS = new ArrayMap<>(); diff --git a/packages/SettingsProvider/test/src/android/provider/settings/validators/SettingsValidatorsTest.java b/packages/SettingsProvider/test/src/android/provider/settings/validators/SettingsValidatorsTest.java index 9134d875097e7..e5d148cb213a4 100644 --- a/packages/SettingsProvider/test/src/android/provider/settings/validators/SettingsValidatorsTest.java +++ b/packages/SettingsProvider/test/src/android/provider/settings/validators/SettingsValidatorsTest.java @@ -56,8 +56,8 @@ public class SettingsValidatorsTest { } @Test - public void testNonNegativeIntegerValidator_onNullValue_returnsFalse() { - assertFalse(SettingsValidators.NON_NEGATIVE_INTEGER_VALIDATOR.validate(null)); + public void testNonNegativeIntegerValidator_onNullValue_returnsTrue() { + assertTrue(SettingsValidators.NON_NEGATIVE_INTEGER_VALIDATOR.validate(null)); } @Test @@ -69,8 +69,8 @@ public class SettingsValidatorsTest { } @Test - public void testAnyIntegerValidator_onNullValue_returnsFalse() { - assertFalse(SettingsValidators.ANY_INTEGER_VALIDATOR.validate(null)); + public void testAnyIntegerValidator_onNullValue_returnsTrue() { + assertTrue(SettingsValidators.ANY_INTEGER_VALIDATOR.validate(null)); } @Test @@ -91,8 +91,8 @@ public class SettingsValidatorsTest { } @Test - public void testLenientIpAddressValidator_onNullValue_returnsFalse() { - assertFalse(SettingsValidators.LENIENT_IP_ADDRESS_VALIDATOR.validate(null)); + public void testLenientIpAddressValidator_onNullValue_returnsTrue() { + assertTrue(SettingsValidators.LENIENT_IP_ADDRESS_VALIDATOR.validate(null)); } @Test @@ -120,8 +120,8 @@ public class SettingsValidatorsTest { } @Test - public void testLocaleValidator_onNullValue_returnsFalse() { - assertFalse(SettingsValidators.LOCALE_VALIDATOR.validate(null)); + public void testLocaleValidator_onNullValue_returnsTrue() { + assertTrue(SettingsValidators.LOCALE_VALIDATOR.validate(null)); } @Test @@ -149,11 +149,11 @@ public class SettingsValidatorsTest { } @Test - public void testDiscreteValueValidator_onNullValue_returnsFalse() { + public void testDiscreteValueValidator_onNullValue_returnsTrue() { String[] discreteTypes = new String[]{"Type1", "Type2"}; Validator v = new DiscreteValueValidator(discreteTypes); - assertFalse(v.validate(null)); + assertTrue(v.validate(null)); } @Test @@ -167,10 +167,10 @@ public class SettingsValidatorsTest { } @Test - public void testInclusiveIntegerRangeValidator_onNullValue_returnsFalse() { + public void testInclusiveIntegerRangeValidator_onNullValue_returnsTrue() { Validator v = new InclusiveIntegerRangeValidator(0, 5); - assertFalse(v.validate(null)); + assertTrue(v.validate(null)); } @Test @@ -184,10 +184,10 @@ public class SettingsValidatorsTest { } @Test - public void testInclusiveFloatRangeValidator_onNullValue_returnsFalse() { + public void testInclusiveFloatRangeValidator_onNullValue_returnsTrue() { Validator v = new InclusiveFloatRangeValidator(0.0f, 5.0f); - assertFalse(v.validate(null)); + assertTrue(v.validate(null)); } @Test @@ -220,8 +220,8 @@ public class SettingsValidatorsTest { } @Test - public void dateFormatValidator_onNullValue_returnsFalse() { - assertFalse(SettingsValidators.DATE_FORMAT_VALIDATOR.validate(null)); + public void dateFormatValidator_onNullValue_returnsTrue() { + assertTrue(SettingsValidators.DATE_FORMAT_VALIDATOR.validate(null)); } @Test @@ -240,8 +240,8 @@ public class SettingsValidatorsTest { } @Test - public void testJSONObjectValidator_onNullValue_returnsFalse() { - assertFalse(SettingsValidators.JSON_OBJECT_VALIDATOR.validate(null)); + public void testJSONObjectValidator_onNullValue_returnsTrue() { + assertTrue(SettingsValidators.JSON_OBJECT_VALIDATOR.validate(null)); } @Test