Commit bfded9bd authored by Henrique Nakashima's avatar Henrique Nakashima Committed by Commit Bot

Create KeyPrefix allowing dynamic SharedPreferences keys to be used.

SharedPreferencesManager now accepts dynamic keys as long as their
prefix was registered in ChromePreferenceKeys.

Bug: 1013781, 1022108
Change-Id: Ia5ed931d64fc5098dd4927c1794eede85b28e094
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1929636Reviewed-by: default avatarNatalie Chouinard <chouinard@chromium.org>
Commit-Queue: Henrique Nakashima <hnakashima@chromium.org>
Cr-Commit-Position: refs/heads/master@{#719299}
parent 466d0db4
...@@ -7,6 +7,7 @@ import("//build/config/android/rules.gni") ...@@ -7,6 +7,7 @@ import("//build/config/android/rules.gni")
android_library("java") { android_library("java") {
java_files = [ java_files = [
"android/java/src/org/chromium/chrome/browser/preferences/ChromePreferenceKeys.java", "android/java/src/org/chromium/chrome/browser/preferences/ChromePreferenceKeys.java",
"android/java/src/org/chromium/chrome/browser/preferences/KeyPrefix.java",
"android/java/src/org/chromium/chrome/browser/preferences/PrefChangeRegistrar.java", "android/java/src/org/chromium/chrome/browser/preferences/PrefChangeRegistrar.java",
"android/java/src/org/chromium/chrome/browser/preferences/PrefServiceBridge.java", "android/java/src/org/chromium/chrome/browser/preferences/PrefServiceBridge.java",
"android/java/src/org/chromium/chrome/browser/preferences/SharedPreferencesManager.java", "android/java/src/org/chromium/chrome/browser/preferences/SharedPreferencesManager.java",
...@@ -34,7 +35,10 @@ java_cpp_enum("java_enums_srcjar") { ...@@ -34,7 +35,10 @@ java_cpp_enum("java_enums_srcjar") {
android_library("javatests") { android_library("javatests") {
testonly = true testonly = true
java_files = [ "android/java/src/org/chromium/chrome/browser/preferences/ChromePreferenceKeysTest.java" ] java_files = [
"android/java/src/org/chromium/chrome/browser/preferences/ChromePreferenceKeysTest.java",
"android/java/src/org/chromium/chrome/browser/preferences/KeyPrefixTest.java",
]
deps = [ deps = [
":java", ":java",
"//base:base_java", "//base:base_java",
......
...@@ -10,12 +10,12 @@ import java.util.Arrays; ...@@ -10,12 +10,12 @@ import java.util.Arrays;
import java.util.List; import java.util.List;
/** /**
* Contains String constants with the SharedPreferences keys used by Chrome. * Contains String and {@link KeyPrefix} constants with the SharedPreferences keys used by Chrome.
* *
* All Chrome layer SharedPreferences keys should be declared in this class. * All Chrome layer SharedPreferences keys should be declared in this class.
* *
* To add a new key: * To add a new key:
* 1. Declare it as a constant in this class. Its value should follow the format * 1. Declare it as a String constant in this class. Its value should follow the format
* "Chrome.[Feature].[Key]" * "Chrome.[Feature].[Key]"
* 2. Add it to createKeysInUse(). * 2. Add it to createKeysInUse().
* *
...@@ -25,6 +25,17 @@ import java.util.List; ...@@ -25,6 +25,17 @@ import java.util.List;
* 3. If the key is in createGrandfatheredFormatKeysForTesting(), remove it from there. * 3. If the key is in createGrandfatheredFormatKeysForTesting(), remove it from there.
* 4. Delete the constant. * 4. Delete the constant.
* *
* To add a new KeyPrefix:
* 1. Declare it as a KeyPrefix constant in this class. Its value should follow the format
* "Chrome.[Feature].[KeyPrefix].*"
* 2. Add PREFIX_CONSTANT.pattern() to the list of used keys in
* {@link ChromePreferenceKeys#createKeysInUse()} ()}.
*
* To deprecate a KeyPrefix that is not used anymore:
* 1. Add its String value to createDeprecatedKeysForTesting(), including the ".*"
* 2. Remove it from createKeysInUse().
* 3. Delete the KeyPrefix constant.
*
* Tests in ChromePreferenceKeysTest ensure the sanity of this file. * Tests in ChromePreferenceKeysTest ensure the sanity of this file.
*/ */
public final class ChromePreferenceKeys { public final class ChromePreferenceKeys {
......
...@@ -113,7 +113,8 @@ public class ChromePreferenceKeysTest { ...@@ -113,7 +113,8 @@ public class ChromePreferenceKeysTest {
private void doTestKeysConformToFormat(List<String> usedList, List<String> grandfathered) { private void doTestKeysConformToFormat(List<String> usedList, List<String> grandfathered) {
Set<String> grandfatheredSet = new HashSet<>(grandfathered); Set<String> grandfatheredSet = new HashSet<>(grandfathered);
Pattern regex = Pattern.compile("Chrome.([A-Z][a-z0-9]+)+.([A-Z][a-z0-9]+)+"); String term = "([A-Z][a-z0-9]*)+";
Pattern regex = Pattern.compile("Chrome\\." + term + "\\." + term + "(\\.\\*)?");
for (String keyInUse : usedList) { for (String keyInUse : usedList) {
if (grandfatheredSet.contains(keyInUse)) { if (grandfatheredSet.contains(keyInUse)) {
...@@ -128,12 +129,17 @@ public class ChromePreferenceKeysTest { ...@@ -128,12 +129,17 @@ public class ChromePreferenceKeysTest {
// Below are tests to ensure that doTestKeysConformToFormat() works. // Below are tests to ensure that doTestKeysConformToFormat() works.
private static class TestFormatConstantsClass { private static class TestFormatConstantsClass {
public static final String GRANDFATHERED_IN = "grandfathered_in"; static final String GRANDFATHERED_IN = "grandfathered_in";
public static final String NEW1 = "Chrome.FeatureOne.Key1"; static final String NEW1 = "Chrome.FeatureOne.Key1";
public static final String NEW2 = "Chrome.Foo.Key"; static final String NEW2 = "Chrome.Foo.Key";
public static final String BROKEN_PREFIX = "Chrom.Foo.Key"; static final String BROKEN_PREFIX = "Chrom.Foo.Key";
public static final String MISSING_FEATURE = "Chrome..Key"; static final String MISSING_FEATURE = "Chrome..Key";
public static final String LOWERCASE_KEY = "Chrome.Foo.key"; static final String LOWERCASE_KEY = "Chrome.Foo.key";
static final KeyPrefix PREFIX = new KeyPrefix("Chrome.FeatureOne.KeyPrefix1.*");
static final KeyPrefix PREFIX_EXTRA_LEVEL =
new KeyPrefix("Chrome.FeatureOne.KeyPrefix1.ExtraLevel.*");
static final KeyPrefix PREFIX_MISSING_LEVEL = new KeyPrefix("Chrome.FeatureOne.*");
} }
@Test @Test
...@@ -180,4 +186,27 @@ public class ChromePreferenceKeysTest { ...@@ -180,4 +186,27 @@ public class ChromePreferenceKeysTest {
TestFormatConstantsClass.NEW1, TestFormatConstantsClass.LOWERCASE_KEY), TestFormatConstantsClass.NEW1, TestFormatConstantsClass.LOWERCASE_KEY),
Arrays.asList(TestFormatConstantsClass.GRANDFATHERED_IN)); Arrays.asList(TestFormatConstantsClass.GRANDFATHERED_IN));
} }
@Test
@SmallTest
public void testFormatCheck_prefixCorrect() {
doTestKeysConformToFormat(
Arrays.asList(TestFormatConstantsClass.PREFIX.pattern()), Collections.emptyList());
}
@Test(expected = AssertionError.class)
@SmallTest
public void testFormatCheck_prefixExtralevel() {
doTestKeysConformToFormat(
Arrays.asList(TestFormatConstantsClass.PREFIX_EXTRA_LEVEL.pattern()),
Collections.emptyList());
}
@Test(expected = AssertionError.class)
@SmallTest
public void testFormatCheck_prefixMissingLevel() {
doTestKeysConformToFormat(
Arrays.asList(TestFormatConstantsClass.PREFIX_MISSING_LEVEL.pattern()),
Collections.emptyList());
}
} }
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
package org.chromium.chrome.browser.preferences;
/**
* A prefix for a range of SharedPreferences keys generated dynamically.
*
* Instances should be declared as keys in {@link ChromePreferenceKeys}.
*/
public class KeyPrefix {
private final String mPrefix;
KeyPrefix(String pattern) {
// More thorough checking is performed in ChromePreferenceKeysTest.
assert pattern.endsWith(".*");
mPrefix = pattern.substring(0, pattern.length() - 1);
}
/**
* @param suffix A non-empty string. The '*' character is reserved.
* @return The complete SharedPreferences key to be passed to {@link SharedPreferencesManager}.
*/
public String createKey(String suffix) {
return mPrefix + suffix;
}
String pattern() {
return mPrefix + "*";
}
}
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
package org.chromium.chrome.browser.preferences;
import static org.junit.Assert.assertEquals;
import android.support.test.filters.SmallTest;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
/**
* Unit tests for {@link KeyPrefix}.
*/
@RunWith(ChromeJUnit4ClassRunner.class)
public class KeyPrefixTest {
@Test
@SmallTest
public void testSuccess_validPattern() {
KeyPrefix prefix = new KeyPrefix("Chrome.Feature.KP.*");
assertEquals(prefix.pattern(), "Chrome.Feature.KP.*");
assertEquals(prefix.createKey("DynamicKey"), "Chrome.Feature.KP.DynamicKey");
assertEquals(prefix.createKey("Level.DynamicKey"), "Chrome.Feature.KP.Level.DynamicKey");
}
@Test(expected = AssertionError.class)
@SmallTest
public void testError_missingPeriod() {
new KeyPrefix("Chrome.Feature.KP");
}
@Test(expected = AssertionError.class)
@SmallTest
public void testError_missingStar() {
new KeyPrefix("Chrome.Feature.KP.");
}
@Test(expected = AssertionError.class)
@SmallTest
public void testError_extraStar() {
new KeyPrefix("Chrome.Feature.KP.**");
}
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment