From bc3e94fdc1ffc21b1683110892f495c64efe793b Mon Sep 17 00:00:00 2001 From: Linus Tufvesson Date: Fri, 17 Jan 2020 17:06:56 +0000 Subject: [PATCH] Cache empty prefixs in NameValueCache and cache things on the first round trip. Test: atest frameworks/base/core/tests/coretests/src/android/provider/NameValueCacheTest.java Bug: 147739090 Change-Id: I9ed48f34be096f2b2d673ee1f5330388c14b695c --- core/java/android/provider/Settings.java | 32 ++- .../android/provider/NameValueCacheTest.java | 232 ++++++++++++++++++ 2 files changed, 251 insertions(+), 13 deletions(-) create mode 100644 core/tests/coretests/src/android/provider/NameValueCacheTest.java diff --git a/core/java/android/provider/Settings.java b/core/java/android/provider/Settings.java index a62412068fdac..3e0b46b6d8e8b 100644 --- a/core/java/android/provider/Settings.java +++ b/core/java/android/provider/Settings.java @@ -2721,6 +2721,7 @@ public final class Settings { } } }); + currentGeneration = generation; } } if (mGenerationTracker != null && currentGeneration == @@ -2801,14 +2802,7 @@ public final class Settings { } mValues.clear(); } else { - boolean prefixCached = false; - int size = mValues.size(); - for (int i = 0; i < size; ++i) { - if (mValues.keyAt(i).startsWith(prefix)) { - prefixCached = true; - break; - } - } + boolean prefixCached = mValues.containsKey(prefix); if (prefixCached) { if (!names.isEmpty()) { for (String name : names) { @@ -2817,9 +2811,11 @@ public final class Settings { } } } else { - for (int i = 0; i < size; ++i) { + for (int i = 0; i < mValues.size(); ++i) { String key = mValues.keyAt(i); - if (key.startsWith(prefix)) { + // Explicitly exclude the prefix as it is only there to + // signal that the prefix has been cached. + if (key.startsWith(prefix) && !key.equals(prefix)) { keyValues.put(key, mValues.get(key)); } } @@ -2907,12 +2903,15 @@ public final class Settings { } } }); + currentGeneration = generation; } } if (mGenerationTracker != null && currentGeneration == mGenerationTracker.getCurrentGeneration()) { // cache the complete list of flags for the namespace mValues.putAll(flagsToValues); + // Adding the prefix as a signal that the prefix is cached. + mValues.put(prefix, null); } } return keyValues; @@ -14104,7 +14103,7 @@ public final class Settings { * @hide */ @RequiresPermission(Manifest.permission.READ_DEVICE_CONFIG) - static Map getStrings(@NonNull ContentResolver resolver, + public static Map getStrings(@NonNull ContentResolver resolver, @NonNull String namespace, @NonNull List names) { List compositeNames = new ArrayList<>(names.size()); for (String name : names) { @@ -14163,8 +14162,9 @@ public final class Settings { * @hide */ @RequiresPermission(Manifest.permission.WRITE_DEVICE_CONFIG) - static boolean setStrings(@NonNull ContentResolver resolver, @NonNull String namespace, - @NonNull Map keyValues) throws DeviceConfig.BadConfigException { + public static boolean setStrings(@NonNull ContentResolver resolver, + @NonNull String namespace, @NonNull Map keyValues) + throws DeviceConfig.BadConfigException { HashMap compositeKeyValueMap = new HashMap<>(keyValues.keySet().size()); for (Map.Entry entry : keyValues.entrySet()) { compositeKeyValueMap.put( @@ -14241,6 +14241,12 @@ public final class Settings { } } + /** @hide */ + public static void clearProviderForTest() { + sProviderHolder.clearProviderForTest(); + sNameValueCache.clearGenerationTrackerForTest(); + } + private static String createCompositeName(@NonNull String namespace, @NonNull String name) { Preconditions.checkNotNull(namespace); Preconditions.checkNotNull(name); diff --git a/core/tests/coretests/src/android/provider/NameValueCacheTest.java b/core/tests/coretests/src/android/provider/NameValueCacheTest.java new file mode 100644 index 0000000000000..4212ef21e0af4 --- /dev/null +++ b/core/tests/coretests/src/android/provider/NameValueCacheTest.java @@ -0,0 +1,232 @@ +/* + * Copyright (C) 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.provider; + +import static com.google.common.truth.Truth.assertThat; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.when; + +import android.content.ContentProvider; +import android.content.IContentProvider; +import android.os.Bundle; +import android.platform.test.annotations.Presubmit; +import android.test.mock.MockContentResolver; +import android.util.MemoryIntArray; + +import androidx.test.filters.SmallTest; +import androidx.test.runner.AndroidJUnit4; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; + +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; + +/** + * These tests are verifying that Settings#NameValueCache is working as expected with DeviceConfig. + * Due to how the classes are structured, we have to test it in a somewhat roundabout way. We're + * mocking out the contentProvider and are handcrafting very specific Bundles to answer the queries. + */ +@Presubmit +@RunWith(AndroidJUnit4.class) +@SmallTest +public class NameValueCacheTest { + + private static final String NAMESPACE = "namespace"; + private static final String NAMESPACE2 = "namespace2"; + + @Mock + private IContentProvider mMockIContentProvider; + @Mock + private ContentProvider mMockContentProvider; + private MockContentResolver mMockContentResolver; + private MemoryIntArray mCacheGenerationStore; + private int mCurrentGeneration = 123; + + private HashMap> mStorage; + + @Before + public void setUp() throws Exception { + Settings.Config.clearProviderForTest(); + MockitoAnnotations.initMocks(this); + when(mMockContentProvider.getIContentProvider()).thenReturn(mMockIContentProvider); + mMockContentResolver = new MockContentResolver(); + mMockContentResolver.addProvider(DeviceConfig.CONTENT_URI.getAuthority(), + mMockContentProvider); + mCacheGenerationStore = new MemoryIntArray(1); + mStorage = new HashMap<>(); + + // Stores keyValues for a given prefix and increments the generation. (Note that this + // increments the generation no matter what, it doesn't pay attention to if anything + // actually changed). + when(mMockIContentProvider.call(any(), any(), eq(DeviceConfig.CONTENT_URI.getAuthority()), + eq(Settings.CALL_METHOD_SET_ALL_CONFIG), + any(), any(Bundle.class))).thenAnswer(invocationOnMock -> { + Bundle incomingBundle = invocationOnMock.getArgument(5); + HashMap keyValues = + (HashMap) incomingBundle.getSerializable( + Settings.CALL_METHOD_FLAGS_KEY); + String prefix = incomingBundle.getString(Settings.CALL_METHOD_PREFIX_KEY); + mStorage.put(prefix, keyValues); + mCacheGenerationStore.set(0, ++mCurrentGeneration); + + Bundle result = new Bundle(); + result.putBoolean(Settings.KEY_CONFIG_SET_RETURN, true); + return result; + }); + + // Returns the keyValues corresponding to a namespace, or an empty map if the namespace + // doesn't have anything stored for it. Returns the generation key if the caller asked + // for one. + when(mMockIContentProvider.call(any(), any(), eq(DeviceConfig.CONTENT_URI.getAuthority()), + eq(Settings.CALL_METHOD_LIST_CONFIG), + any(), any(Bundle.class))).thenAnswer(invocationOnMock -> { + Bundle incomingBundle = invocationOnMock.getArgument(5); + + String prefix = incomingBundle.getString(Settings.CALL_METHOD_PREFIX_KEY); + + if (!mStorage.containsKey(prefix)) { + mStorage.put(prefix, new HashMap<>()); + } + HashMap keyValues = mStorage.get(prefix); + + Bundle bundle = new Bundle(); + bundle.putSerializable(Settings.NameValueTable.VALUE, keyValues); + + if (incomingBundle.containsKey(Settings.CALL_METHOD_TRACK_GENERATION_KEY)) { + bundle.putParcelable(Settings.CALL_METHOD_TRACK_GENERATION_KEY, + mCacheGenerationStore); + bundle.putInt(Settings.CALL_METHOD_GENERATION_INDEX_KEY, 0); + bundle.putInt(Settings.CALL_METHOD_GENERATION_KEY, + mCacheGenerationStore.get(0)); + } + return bundle; + }); + } + + @Test + public void testCaching_singleNamespace() throws Exception { + HashMap keyValues = new HashMap<>(); + keyValues.put("a", "b"); + Settings.Config.setStrings(mMockContentResolver, NAMESPACE, keyValues); + verify(mMockIContentProvider).call(any(), any(), any(), + eq(Settings.CALL_METHOD_SET_ALL_CONFIG), + any(), any(Bundle.class)); + + Map returnedValues = Settings.Config.getStrings(mMockContentResolver, + NAMESPACE, + Collections.emptyList()); + verify(mMockIContentProvider).call(any(), any(), any(), + eq(Settings.CALL_METHOD_LIST_CONFIG), + any(), any(Bundle.class)); + assertThat(returnedValues).containsExactlyEntriesIn(keyValues); + + Map cachedKeyValues = Settings.Config.getStrings(mMockContentResolver, + NAMESPACE, Collections.emptyList()); + verifyNoMoreInteractions(mMockIContentProvider); + assertThat(cachedKeyValues).containsExactlyEntriesIn(keyValues); + + // Modify the value to invalidate the cache. + keyValues.put("a", "c"); + Settings.Config.setStrings(mMockContentResolver, NAMESPACE, keyValues); + verify(mMockIContentProvider, times(2)).call(any(), any(), any(), + eq(Settings.CALL_METHOD_SET_ALL_CONFIG), + any(), any(Bundle.class)); + + Map returnedValues2 = Settings.Config.getStrings(mMockContentResolver, + NAMESPACE, Collections.emptyList()); + verify(mMockIContentProvider, times(2)).call(any(), any(), any(), + eq(Settings.CALL_METHOD_LIST_CONFIG), + any(), any(Bundle.class)); + assertThat(returnedValues2).containsExactlyEntriesIn(keyValues); + + Map cachedKeyValues2 = Settings.Config.getStrings(mMockContentResolver, + NAMESPACE, Collections.emptyList()); + verifyNoMoreInteractions(mMockIContentProvider); + assertThat(cachedKeyValues2).containsExactlyEntriesIn(keyValues); + } + + @Test + public void testCaching_multipleNamespaces() throws Exception { + HashMap keyValues = new HashMap<>(); + keyValues.put("a", "b"); + Settings.Config.setStrings(mMockContentResolver, NAMESPACE, keyValues); + verify(mMockIContentProvider).call(any(), any(), any(), + eq(Settings.CALL_METHOD_SET_ALL_CONFIG), + any(), any(Bundle.class)); + + HashMap keyValues2 = new HashMap<>(); + keyValues2.put("c", "d"); + keyValues2.put("e", "f"); + Settings.Config.setStrings(mMockContentResolver, NAMESPACE2, keyValues2); + verify(mMockIContentProvider, times(2)).call(any(), any(), any(), + eq(Settings.CALL_METHOD_SET_ALL_CONFIG), + any(), any(Bundle.class)); + + Map returnedValues = Settings.Config.getStrings(mMockContentResolver, + NAMESPACE, + Collections.emptyList()); + verify(mMockIContentProvider).call(any(), any(), any(), + eq(Settings.CALL_METHOD_LIST_CONFIG), + any(), any(Bundle.class)); + assertThat(returnedValues).containsExactlyEntriesIn(keyValues); + + Map returnedValues2 = Settings.Config.getStrings(mMockContentResolver, + NAMESPACE2, + Collections.emptyList()); + verify(mMockIContentProvider, times(2)).call(any(), any(), any(), + eq(Settings.CALL_METHOD_LIST_CONFIG), + any(), any(Bundle.class)); + assertThat(returnedValues2).containsExactlyEntriesIn(keyValues2); + + Map cachedKeyValues = Settings.Config.getStrings(mMockContentResolver, + NAMESPACE, Collections.emptyList()); + verifyNoMoreInteractions(mMockIContentProvider); + assertThat(cachedKeyValues).containsExactlyEntriesIn(keyValues); + + Map cachedKeyValues2 = Settings.Config.getStrings(mMockContentResolver, + NAMESPACE2, Collections.emptyList()); + verifyNoMoreInteractions(mMockIContentProvider); + assertThat(cachedKeyValues2).containsExactlyEntriesIn(keyValues2); + } + + @Test + public void testCaching_emptyNamespace() throws Exception { + Map returnedValues = Settings.Config.getStrings(mMockContentResolver, + NAMESPACE, + Collections.emptyList()); + verify(mMockIContentProvider).call(any(), any(), any(), + eq(Settings.CALL_METHOD_LIST_CONFIG), + any(), any(Bundle.class)); + assertThat(returnedValues).isEmpty(); + + Map cachedKeyValues = Settings.Config.getStrings(mMockContentResolver, + NAMESPACE, Collections.emptyList()); + verifyNoMoreInteractions(mMockIContentProvider); + assertThat(cachedKeyValues).isEmpty(); + } + +}