Bug 969672 - Handle declined engines on Android. r=nalexander

This commit is contained in:
Richard Newman 2014-03-14 19:14:34 -07:00
parent 0882a4b3bb
commit 14e83db018
5 changed files with 218 additions and 13 deletions

View File

@ -18,6 +18,7 @@ import java.util.Map.Entry;
import java.util.Set;
import java.util.concurrent.atomic.AtomicLong;
import org.json.simple.JSONArray;
import org.json.simple.parser.ParseException;
import org.mozilla.gecko.background.common.log.Logger;
import org.mozilla.gecko.sync.crypto.CryptoException;
@ -374,6 +375,7 @@ public class GlobalSession implements PrefsSource, HttpResponseObserver {
}
public void updateMetaGlobalInPlace() {
config.metaGlobal.declined = this.declinedEngineNames();
ExtendedJSONObject engines = config.metaGlobal.getEngines();
for (Entry<String, EngineSettings> pair : enginesToUpdate.entrySet()) {
if (pair.getValue() == null) {
@ -651,6 +653,38 @@ public class GlobalSession implements PrefsSource, HttpResponseObserver {
Utils.toCommaSeparatedString(config.enabledEngineNames) + "' from meta/global.");
}
}
// Persist declined.
// Our declined engines at any point are:
// Whatever they were remotely, plus whatever they were locally, less any
// engines that were just enabled locally or remotely.
// If remote just 'won', our recently enabled list just got cleared.
final HashSet<String> allDeclined = new HashSet<String>();
final Set<String> newRemoteDeclined = global.getDeclinedEngineNames();
final Set<String> oldLocalDeclined = config.declinedEngineNames;
allDeclined.addAll(newRemoteDeclined);
allDeclined.addAll(oldLocalDeclined);
if (config.userSelectedEngines != null) {
for (Entry<String, Boolean> selection : config.userSelectedEngines.entrySet()) {
if (selection.getValue()) {
allDeclined.remove(selection.getKey());
}
}
}
config.declinedEngineNames = allDeclined;
if (config.declinedEngineNames.isEmpty()) {
Logger.debug(LOG_TAG, "meta/global reported no declined engine names, and we have none declined locally.");
} else {
if (Logger.shouldLogVerbose(LOG_TAG)) {
Logger.trace(LOG_TAG, "Persisting declined engine names '" +
Utils.toCommaSeparatedString(config.declinedEngineNames) + "' from meta/global.");
}
}
config.persistToPrefs();
advance();
}
@ -901,6 +935,27 @@ public class GlobalSession implements PrefsSource, HttpResponseObserver {
resetStages(this.getSyncStagesByName(names));
}
/**
* Engines to explicitly mark as declined in a fresh meta/global record.
* <p>
* Returns an empty array if the user hasn't elected to customize data types,
* or an array of engines that the user un-checked during customization.
* <p>
* Engines that Android Sync doesn't recognize are <b>not</b> included in
* the returned array.
*
* @return a new JSONArray of engine names.
*/
@SuppressWarnings("unchecked")
protected JSONArray declinedEngineNames() {
final JSONArray declined = new JSONArray();
for (String engine : config.declinedEngineNames) {
declined.add(engine);
};
return declined;
}
/**
* Engines to include in a fresh meta/global record.
* <p>
@ -983,6 +1038,10 @@ public class GlobalSession implements PrefsSource, HttpResponseObserver {
metaGlobal.setStorageVersion(STORAGE_VERSION);
metaGlobal.setEngines(engines);
// We assume that the config's declined engines have been updated
// according to the user's selections.
metaGlobal.setDeclinedEngineNames(this.declinedEngineNames());
return metaGlobal;
}

View File

@ -6,11 +6,13 @@ package org.mozilla.gecko.sync;
import java.io.IOException;
import java.net.URISyntaxException;
import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import org.json.simple.JSONArray;
import org.json.simple.parser.ParseException;
import org.mozilla.gecko.background.common.log.Logger;
import org.mozilla.gecko.sync.MetaGlobalException.MetaGlobalMalformedSyncIDException;
@ -27,6 +29,7 @@ public class MetaGlobal implements SyncStorageRequestDelegate {
// Fields.
protected ExtendedJSONObject engines;
protected JSONArray declined;
protected Long storageVersion;
protected String syncID;
@ -77,6 +80,7 @@ public class MetaGlobal implements SyncStorageRequestDelegate {
json.put("storageVersion", storageVersion);
json.put("engines", engines);
json.put("syncID", syncID);
json.put("declined", declined);
return json;
}
@ -93,14 +97,18 @@ public class MetaGlobal implements SyncStorageRequestDelegate {
return record;
}
public void setFromRecord(CryptoRecord record) throws IllegalStateException, IOException, ParseException, NonObjectJSONException {
public void setFromRecord(CryptoRecord record) throws IllegalStateException, IOException, ParseException, NonObjectJSONException, NonArrayJSONException {
if (record == null) {
throw new IllegalArgumentException("Cannot set meta/global from null record");
}
Logger.debug(LOG_TAG, "meta/global is " + record.payload.toJSONString());
this.storageVersion = (Long) record.payload.get("storageVersion");
this.syncID = (String) record.payload.get("syncID");
setEngines(record.payload.getObject("engines"));
// Accepts null -- declined can be missing.
setDeclinedEngineNames(record.payload.getArray("declined"));
}
public Long getStorageVersion() {
@ -115,6 +123,59 @@ public class MetaGlobal implements SyncStorageRequestDelegate {
return engines;
}
@SuppressWarnings("unchecked")
public void declineEngine(String engine) {
if (this.declined == null) {
JSONArray replacement = new JSONArray();
replacement.add(engine);
setDeclinedEngineNames(replacement);
return;
}
this.declined.add(engine);
}
@SuppressWarnings("unchecked")
public void declineEngineNames(Collection<String> additional) {
if (this.declined == null) {
JSONArray replacement = new JSONArray();
replacement.addAll(additional);
setDeclinedEngineNames(replacement);
return;
}
for (String engine : additional) {
if (!this.declined.contains(engine)) {
this.declined.add(engine);
}
}
}
public void setDeclinedEngineNames(JSONArray declined) {
if (declined == null) {
this.declined = new JSONArray();
return;
}
this.declined = declined;
}
/**
* Return the set of engines that we support (given as an argument)
* but the user hasn't explicitly declined on another device.
*
* Can return the input if the user hasn't declined any engines.
*/
public Set<String> getNonDeclinedEngineNames(Set<String> supported) {
if (this.declined == null ||
this.declined.isEmpty()) {
return supported;
}
final Set<String> result = new HashSet<String>(supported);
result.removeAll(this.declined);
return result;
}
public void setEngines(ExtendedJSONObject engines) {
if (engines == null) {
engines = new ExtendedJSONObject();
@ -196,6 +257,14 @@ public class MetaGlobal implements SyncStorageRequestDelegate {
return new HashSet<String>(engines.keySet());
}
@SuppressWarnings("unchecked")
public Set<String> getDeclinedEngineNames() {
if (declined == null) {
return null;
}
return new HashSet<String>(declined);
}
/**
* Returns if the server settings and local settings match.
* Throws a specific MetaGlobalException if that's not the case.

View File

@ -198,6 +198,7 @@ public class SyncConfiguration {
* fresh meta/global record for upload.
*/
public Set<String> enabledEngineNames;
public Set<String> declinedEngineNames = new HashSet<String>();
/**
* Names of stages to sync <it>this sync</it>, or <code>null</code> to sync
@ -248,6 +249,7 @@ public class SyncConfiguration {
public static final String PREF_SYNC_ID = "syncID";
public static final String PREF_ENABLED_ENGINE_NAMES = "enabledEngineNames";
public static final String PREF_DECLINED_ENGINE_NAMES = "declinedEngineNames";
public static final String PREF_USER_SELECTED_ENGINES_TO_SYNC = "userSelectedEngines";
public static final String PREF_USER_SELECTED_ENGINES_TO_SYNC_TIMESTAMP = "userSelectedEnginesTimestamp";
@ -311,27 +313,47 @@ public class SyncConfiguration {
}
/**
* Gets the engine names that are enabled in meta/global.
* Gets the engine names that are enabled, declined, or other (depending on pref) in meta/global.
*
* @param prefs
* SharedPreferences that the engines are associated with.
* @param pref
* The preference name to use. E.g, PREF_ENABLED_ENGINE_NAMES.
* @return Set<String> of the enabled engine names if they have been stored,
* or null otherwise.
*/
public static Set<String> getEnabledEngineNames(SharedPreferences prefs) {
String json = prefs.getString(PREF_ENABLED_ENGINE_NAMES, null);
protected static Set<String> getEngineNamesFromPref(SharedPreferences prefs, String pref) {
final String json = prefs.getString(pref, null);
if (json == null) {
return null;
}
try {
ExtendedJSONObject o = ExtendedJSONObject.parseJSONObject(json);
final ExtendedJSONObject o = ExtendedJSONObject.parseJSONObject(json);
return new HashSet<String>(o.keySet());
} catch (Exception e) {
// enabledEngineNames can be null.
return null;
}
}
/**
* Returns the set of engine names that the user has enabled. If none
* have been stored in prefs, <code>null</code> is returned.
*/
public static Set<String> getEnabledEngineNames(SharedPreferences prefs) {
return getEngineNamesFromPref(prefs, PREF_ENABLED_ENGINE_NAMES);
}
/**
* Returns the set of engine names that the user has declined.
*/
public static Set<String> getDeclinedEngineNames(SharedPreferences prefs) {
final Set<String> names = getEngineNamesFromPref(prefs, PREF_DECLINED_ENGINE_NAMES);
if (names == null) {
return new HashSet<String>();
}
return names;
}
/**
* Gets the engines whose sync states have been changed by the user through the
* SelectEnginesActivity.
@ -371,6 +393,9 @@ public class SyncConfiguration {
/**
* Store a Map of engines and their sync states to prefs.
*
* Any engine that's disabled in the input is also recorded
* as a declined engine, overwriting the stored values.
*
* @param prefs
* SharedPreferences that the engines are associated with.
* @param selectedEngines
@ -378,20 +403,33 @@ public class SyncConfiguration {
*/
public static void storeSelectedEnginesToPrefs(SharedPreferences prefs, Map<String, Boolean> selectedEngines) {
ExtendedJSONObject jObj = new ExtendedJSONObject();
HashSet<String> declined = new HashSet<String>();
for (Entry<String, Boolean> e : selectedEngines.entrySet()) {
jObj.put(e.getKey(), e.getValue());
final Boolean enabled = e.getValue();
final String engine = e.getKey();
jObj.put(engine, enabled);
if (!enabled) {
declined.add(engine);
}
}
// Our history checkbox drives form history, too.
// We don't need to do this for enablement: that's done at retrieval time.
if (selectedEngines.containsKey("history") && !selectedEngines.get("history").booleanValue()) {
declined.add("forms");
}
String json = jObj.toJSONString();
long currentTime = System.currentTimeMillis();
Editor edit = prefs.edit();
edit.putString(PREF_USER_SELECTED_ENGINES_TO_SYNC, json);
edit.putString(PREF_DECLINED_ENGINE_NAMES, setToJSONObjectString(declined));
edit.putLong(PREF_USER_SELECTED_ENGINES_TO_SYNC_TIMESTAMP, currentTime);
Logger.error(LOG_TAG, "Storing user-selected engines at [" + currentTime + "].");
edit.commit();
}
public void loadFromPrefs(SharedPreferences prefs) {
if (prefs.contains(PREF_CLUSTER_URL)) {
String u = prefs.getString(PREF_CLUSTER_URL, null);
try {
@ -406,6 +444,7 @@ public class SyncConfiguration {
Logger.trace(LOG_TAG, "Set syncID from bundle: " + syncID);
}
enabledEngineNames = getEnabledEngineNames(prefs);
declinedEngineNames = getDeclinedEngineNames(prefs);
userSelectedEngines = getUserSelectedEngines(prefs);
userSelectedEnginesTimestamp = prefs.getLong(PREF_USER_SELECTED_ENGINES_TO_SYNC_TIMESTAMP, 0);
// We don't set crypto/keys here because we need the syncKeyBundle to decrypt the JSON
@ -417,6 +456,14 @@ public class SyncConfiguration {
this.persistToPrefs(this.getPrefs());
}
private static String setToJSONObjectString(Set<String> set) {
ExtendedJSONObject o = new ExtendedJSONObject();
for (String name : set) {
o.put(name, 0);
}
return o.toJSONString();
}
public void persistToPrefs(SharedPreferences prefs) {
Editor edit = prefs.edit();
if (clusterURL == null) {
@ -430,11 +477,12 @@ public class SyncConfiguration {
if (enabledEngineNames == null) {
edit.remove(PREF_ENABLED_ENGINE_NAMES);
} else {
ExtendedJSONObject o = new ExtendedJSONObject();
for (String engineName : enabledEngineNames) {
o.put(engineName, 0);
}
edit.putString(PREF_ENABLED_ENGINE_NAMES, o.toJSONString());
edit.putString(PREF_ENABLED_ENGINE_NAMES, setToJSONObjectString(enabledEngineNames));
}
if (declinedEngineNames.isEmpty()) {
edit.remove(PREF_DECLINED_ENGINE_NAMES);
} else {
edit.putString(PREF_DECLINED_ENGINE_NAMES, setToJSONObjectString(declinedEngineNames));
}
if (userSelectedEngines == null) {
edit.remove(PREF_USER_SELECTED_ENGINES_TO_SYNC);

View File

@ -515,7 +515,9 @@ public abstract class ServerSyncStage extends AbstractSessionManagingSyncStage i
if (!isEnabled) {
// Engine has been disabled; update meta/global with engine removal for upload.
session.removeEngineFromMetaGlobal(name);
session.config.declinedEngineNames.add(name);
} else {
session.config.declinedEngineNames.remove(name);
// Add engine with new syncID to meta/global for upload.
String newSyncID = Utils.generateGuid();
session.recordForMetaGlobalUpdate(name, new EngineSettings(newSyncID, this.getStorageVersion()));

View File

@ -25,6 +25,33 @@ public class TestSyncConfiguration extends AndroidSyncTestCase implements PrefsS
return this.getApplicationContext().getSharedPreferences(name, mode);
}
/**
* Ensure that declined engines persist through prefs.
*/
public void testDeclinedEngineNames() {
SyncConfiguration config = null;
SharedPreferences prefs = getPrefs(TEST_PREFS_NAME, 0);
config = newSyncConfiguration();
config.declinedEngineNames = new HashSet<String>();
config.declinedEngineNames.add("test1");
config.declinedEngineNames.add("test2");
config.persistToPrefs();
assertTrue(prefs.contains(SyncConfiguration.PREF_DECLINED_ENGINE_NAMES));
config = newSyncConfiguration();
Set<String> expected = new HashSet<String>();
for (String name : new String[] { "test1", "test2" }) {
expected.add(name);
}
assertEquals(expected, config.declinedEngineNames);
config.declinedEngineNames = null;
config.persistToPrefs();
assertFalse(prefs.contains(SyncConfiguration.PREF_DECLINED_ENGINE_NAMES));
config = newSyncConfiguration();
assertNull(config.declinedEngineNames);
}
public void testEnabledEngineNames() {
SyncConfiguration config = null;
SharedPreferences prefs = getPrefs(TEST_PREFS_NAME, 0);