Add support for syncing specified keys only
We don't want to sync every single key from client's default
SharedPreference to the sandbox. We will be sycing specific keys only.
Support for calling setKeysToSync() multiple times will be added in
follow up cl.
Bug: 239403323
Test: SdkSdkSandboxFrameworkUnitTests
Change-Id: I6d4ca3b37dde037756c09f83dfd7417480e4767d
diff --git a/sdksandbox/SdkSandbox/src/com/android/sdksandbox/SdkSandboxServiceImpl.java b/sdksandbox/SdkSandbox/src/com/android/sdksandbox/SdkSandboxServiceImpl.java
index 7b22a22..ce69d84 100644
--- a/sdksandbox/SdkSandbox/src/com/android/sdksandbox/SdkSandboxServiceImpl.java
+++ b/sdksandbox/SdkSandbox/src/com/android/sdksandbox/SdkSandboxServiceImpl.java
@@ -143,6 +143,7 @@
// TODO(b/239403323): Add support for non-string keys
editor.putString(key, data.getString(key));
}
+ // TODO(b/239403323): What if writing to persistent storage fails?
editor.apply();
}
diff --git a/sdksandbox/framework/java/android/app/sdksandbox/SharedPreferencesSyncManager.java b/sdksandbox/framework/java/android/app/sdksandbox/SharedPreferencesSyncManager.java
index e20d64d..e356b77 100644
--- a/sdksandbox/framework/java/android/app/sdksandbox/SharedPreferencesSyncManager.java
+++ b/sdksandbox/framework/java/android/app/sdksandbox/SharedPreferencesSyncManager.java
@@ -16,6 +16,8 @@
package android.app.sdksandbox;
+import android.annotation.NonNull;
+import android.annotation.Nullable;
import android.content.Context;
import android.content.SharedPreferences;
import android.os.Bundle;
@@ -25,6 +27,8 @@
import com.android.internal.annotations.GuardedBy;
import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
/**
* Syncs all keys in default {@link SharedPreferences} containing string values to Sdk Sandbox.
@@ -38,18 +42,46 @@
private final ISdkSandboxManager mService;
private final Context mContext;
private final ChangeListener mListener = new ChangeListener();
-
private final Object mLock = new Object();
// TODO(b/239403323): Maintain a dynamic sync status based on lifecycle events
@GuardedBy("mLock")
private boolean mInitialSyncComplete = false;
- public SharedPreferencesSyncManager(Context context, ISdkSandboxManager service) {
+ // List of keys that this manager needs to keep in sync.
+ @Nullable
+ @GuardedBy("mLock")
+ private Set<String> mKeysToSync = null;
+
+ public SharedPreferencesSyncManager(
+ @NonNull Context context, @NonNull ISdkSandboxManager service) {
mContext = context.getApplicationContext();
mService = service;
}
+ /**
+ * Set of keys which the sync manager should be syncing to Sandbox.
+ *
+ * <p>Keys outside of this list will be ignored. This method should be called only once.
+ * Subsequent calls won't update the list of keys being synced.
+ *
+ * @param keysToSync set of keys that will be synced to Sandbox. Must not be null.
+ * @return true if set of keys have been successfully updated, otherwise returns false.
+ */
+ public boolean setKeysToSync(@NonNull Set<String> keysToSync) {
+ // TODO(b/239403323): Validate keysToSync does not contain null.
+ Objects.requireNonNull(keysToSync, "keysToSync must not be null");
+ synchronized (mLock) {
+ // TODO(b/239403323): Allow updating mKeysToSync
+ if (mKeysToSync == null) {
+ mKeysToSync = keysToSync;
+ return true;
+ } else {
+ return false;
+ }
+ }
+ }
+
// TODO(b/239403323): On sandbox restart, we need to sync again.
// TODO(b/239403323): Also sync non-string values.
/**
@@ -64,6 +96,11 @@
*/
public void syncData() {
synchronized (mLock) {
+ // Do not sync if keys have not been specified by the client.
+ if (mKeysToSync == null || mKeysToSync.isEmpty()) {
+ return;
+ }
+
if (!mInitialSyncComplete) {
bulkSyncData();
@@ -77,17 +114,27 @@
}
}
+ @GuardedBy("mLock")
private void bulkSyncData() {
final Bundle data = new Bundle();
final SharedPreferences pref = getDefaultSharedPreferences();
final Map<String, ?> allData = pref.getAll();
for (Map.Entry<String, ?> entry : allData.entrySet()) {
final String key = entry.getKey();
+ // Sync only specified keys
+ if (!mKeysToSync.contains(key)) {
+ continue;
+ }
if (entry.getValue() instanceof String) {
data.putString(key, pref.getString(key, ""));
}
}
+ // No need to sync if there data is empty
+ if (data.isEmpty()) {
+ return;
+ }
+
try {
mService.syncDataFromClient(mContext.getPackageName(), data);
} catch (RemoteException ignore) {
@@ -102,14 +149,23 @@
private class ChangeListener implements SharedPreferences.OnSharedPreferenceChangeListener {
@Override
- public void onSharedPreferenceChanged(SharedPreferences pref, String key) {
+ public void onSharedPreferenceChanged(SharedPreferences pref, @Nullable String key) {
+ // Sync specified keys only
+ synchronized (mLock) {
+ if (key == null || mKeysToSync == null || !mKeysToSync.contains(key)) {
+ return;
+ }
+ }
final Bundle data = new Bundle();
final Object value = pref.getAll().get(key);
if (!(value instanceof String)) {
// TODO(b/239403323): Add support for non-string values
return;
}
+
+ // TODO(b/239403323): Support removal of keys
data.putString(key, pref.getString(key, ""));
+
try {
mService.syncDataFromClient(mContext.getPackageName(), data);
} catch (RemoteException e) {
diff --git a/sdksandbox/tests/unittest/src/android/app/sdksandbox/SharedPreferencesSyncManagerUnitTest.java b/sdksandbox/tests/unittest/src/android/app/sdksandbox/SharedPreferencesSyncManagerUnitTest.java
index 7918e0a..50af6c1 100644
--- a/sdksandbox/tests/unittest/src/android/app/sdksandbox/SharedPreferencesSyncManagerUnitTest.java
+++ b/sdksandbox/tests/unittest/src/android/app/sdksandbox/SharedPreferencesSyncManagerUnitTest.java
@@ -24,6 +24,7 @@
import android.content.SharedPreferences;
import android.os.Bundle;
import android.preference.PreferenceManager;
+import android.util.ArraySet;
import androidx.test.InstrumentationRegistry;
@@ -36,6 +37,8 @@
import org.junit.runners.JUnit4;
import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.CountDownLatch;
@@ -66,17 +69,42 @@
getDefaultSharedPreferences().edit().clear().commit();
}
- // TODO(b/239403323): Bulk sync should be syncing a subset of keys as defined by app
@Test
- public void test_syncData_syncsAllKeys() throws Exception {
+ public void test_syncData_doesNotSyncIfKeysNotSpecified() throws Exception {
// Populate default shared preference with test data
populateDefaultSharedPreference(TEST_DATA);
+
+ // Sync data without specifying list of keys to sync
+ mSyncManager.syncData();
+
+ // Verify that sync manager does not try to sync at all
+ assertThat(mSdkSandboxManagerService.getNumberOfUpdatesReceived()).isEqualTo(0);
+ }
+
+ @Test
+ public void test_syncData_doesNotSyncEmptyUpdates() throws Exception {
+ // Unpopulated shared preference. There is nothing to sync.
+ mSyncManager.setKeysToSync(TEST_DATA.keySet());
+
+ mSyncManager.syncData();
+
+ // Verify that sync manager does not try to sync with empty data
+ assertThat(mSdkSandboxManagerService.getNumberOfUpdatesReceived()).isEqualTo(0);
+ }
+
+ @Test
+ public void test_syncData_syncSpecifiedKeys() throws Exception {
+ // Populate default shared preference with test data
+ populateDefaultSharedPreference(TEST_DATA);
+ // Set specific shared keys that we want to sync
+ mSyncManager.setKeysToSync(TEST_DATA.keySet());
+
mSyncManager.syncData();
// Verify that sync manager passes the correct data to SdkSandboxManager
+ final Bundle capturedData = mSdkSandboxManagerService.getLastUpdate();
assertThat(mSdkSandboxManagerService.getCallingPackageName())
.isEqualTo(mContext.getPackageName());
- final Bundle capturedData = mSdkSandboxManagerService.getLastUpdate();
assertThat(capturedData.keySet()).containsExactlyElementsIn(TEST_DATA.keySet());
for (String key : TEST_DATA.keySet()) {
assertThat(capturedData.getString(key)).isEqualTo(TEST_DATA.get(key));
@@ -84,9 +112,26 @@
}
@Test
- public void test_syncData_multipleCalls() throws Exception {
- // Populate default shared preference with test data
+ public void test_syncData_ignoreUnspecifiedKeys() throws Exception {
+ // Populate default shared preference and set specific keys for sycing
populateDefaultSharedPreference(TEST_DATA);
+ mSyncManager.setKeysToSync(TEST_DATA.keySet());
+
+ // Populate extra data outside of shared key list
+ populateDefaultSharedPreference(Map.of("extraKey", "notSpecifiedByApi"));
+
+ mSyncManager.syncData();
+
+ // Verify that sync manager passes the correct data to SdkSandboxManager
+ final Bundle capturedData = mSdkSandboxManagerService.getLastUpdate();
+ assertThat(capturedData.keySet()).containsExactlyElementsIn(TEST_DATA.keySet());
+ }
+
+ @Test
+ public void test_syncData_multipleCalls() throws Exception {
+ // Populate default shared preference and set specific keys for sycing
+ populateDefaultSharedPreference(TEST_DATA);
+ mSyncManager.setKeysToSync(TEST_DATA.keySet());
// Sync data multiple times
mSyncManager.syncData();
@@ -97,7 +142,7 @@
}
@Test
- public void test_syncData_syncsAllKeys_ignoresUnsupportedValues() throws Exception {
+ public void test_syncData_ignoresUnsupportedValues() throws Exception {
// Populate default shared preference with test data
populateDefaultSharedPreference(TEST_DATA);
@@ -107,9 +152,14 @@
editor.putFloat("float", 1.2f);
editor.putInt("int", 1);
editor.putLong("long", 1L);
- editor.putStringSet("long", Set.of("value"));
+ editor.putStringSet("set", Set.of("value"));
editor.commit();
+ // Set keys to sync and then sync data
+ final Set<String> keysToSync =
+ new ArraySet<>(Arrays.asList("boolean", "float", "int", "long", "set"));
+ keysToSync.addAll(TEST_DATA.keySet());
+ mSyncManager.setKeysToSync(keysToSync);
mSyncManager.syncData();
// Verify that sync manager passes the correct data to SdkSandboxManager
@@ -120,34 +170,69 @@
}
}
+ // TODO(b/239403323): We probably want to allow client update this the list dynamically.
@Test
- public void test_syncData_registerListener_syncsFurtherUpdates() throws Exception {
- // Populate default shared preference with test data
+ public void test_setKeysToSync_canBeSetOnlyOnce() throws Exception {
+ // Populate default shared preference and set specific keys for sycing
populateDefaultSharedPreference(TEST_DATA);
+ // Setting keys to sync for the first time should return true
+ assertThat(mSyncManager.setKeysToSync(TEST_DATA.keySet())).isTrue();
+
+ // Try to update keys to sync again
+ assertThat(mSyncManager.setKeysToSync(Collections.emptySet())).isFalse();
mSyncManager.syncData();
+ // Verify that sync manager is still using first set of keys
+ final Bundle capturedData = mSdkSandboxManagerService.getLastUpdate();
+ assertThat(capturedData.keySet()).containsExactlyElementsIn(TEST_DATA.keySet());
+ }
+
+ @Test
+ public void test_updateListener_syncsFurtherUpdates() throws Exception {
+ // Set specified keys for sycing and register listener
+ mSyncManager.setKeysToSync(TEST_DATA.keySet());
+ mSyncManager.syncData();
+
// Update the SharedPreference to trigger listeners
- getDefaultSharedPreferences().edit().putString("update", "value").commit();
+ final String keyToUpdate = TEST_DATA.keySet().toArray()[0].toString();
+ getDefaultSharedPreferences().edit().putString(keyToUpdate, "update").commit();
// Verify we registered a listener that called SdkSandboxManagerService
- mSdkSandboxManagerService.blockForReceivingUpdates(2);
+ mSdkSandboxManagerService.blockForReceivingUpdates(1);
final Bundle capturedData = mSdkSandboxManagerService.getLastUpdate();
- assertThat(capturedData.keySet()).containsExactly("update");
+ assertThat(capturedData.keySet()).containsExactly(keyToUpdate);
+ assertThat(capturedData.getString(keyToUpdate)).isEqualTo("update");
+ }
+
+ @Test
+ public void test_updateListener_ignoresUnspecifiedKeys() throws Exception {
+ // Set specified keys for sycing and register listener
+ mSyncManager.setKeysToSync(TEST_DATA.keySet());
+ mSyncManager.syncData();
+
+ // Update the SharedPreference to trigger listeners
+ getDefaultSharedPreferences().edit().putString("unspecified_key", "update").commit();
+
+ // Verify SdkSandboxManagerService does not receive the update for unspecified key
+ Thread.sleep(5000);
+ assertThat(mSdkSandboxManagerService.getNumberOfUpdatesReceived()).isEqualTo(0);
}
/** Test that listener for live update is registered only once */
@Test
- public void test_syncData_registerListener_onlyOnce() throws Exception {
- // Populate default shared preference with test data
+ public void test_updateListener_registersOnlyOnce() throws Exception {
+ // Populate default shared preference and set specific keys for sycing
populateDefaultSharedPreference(TEST_DATA);
+ mSyncManager.setKeysToSync(TEST_DATA.keySet());
// Sync data multiple times
mSyncManager.syncData();
mSyncManager.syncData();
// Update the SharedPreference to trigger listeners
- getDefaultSharedPreferences().edit().putString("update", "value").commit();
+ final String keyToUpdate = TEST_DATA.keySet().toArray()[0].toString();
+ getDefaultSharedPreferences().edit().putString(keyToUpdate, "update").commit();
// Verify that SyncManager tried to sync only twice: once for bulk and once for live update.
mSdkSandboxManagerService.blockForReceivingUpdates(2);
@@ -197,7 +282,8 @@
@Nullable
public synchronized Bundle getLastUpdate() {
if (mDataCache.isEmpty()) {
- return null;
+ throw new AssertionError(
+ "Fake SdkSandboxManagerService did not receive any update");
}
return mDataCache.get(mDataCache.size() - 1);
}
