Bug 750346 - Persist engine enabled state. r=nalexander,rnewman a=blocking-fennec,mfinkle
authorRichard Newman <rnewman@mozilla.com>
Fri, 04 May 2012 14:30:15 -0700
changeset 95784 e789e4a48423917fcdbe77eba9c645bc286cf36a
parent 95783 fa61eedd233740273b113a898a864ff7038a29a3
child 95785 cb65d67468f88421f850e4c09f5db27d04b90c8c
push id886
push userlsblakk@mozilla.com
push dateMon, 04 Jun 2012 19:57:52 +0000
treeherdermozilla-beta@bbd8d5efd6d1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnalexander, rnewman, blocking-fennec, mfinkle
bugs750346
milestone14.0a2
Bug 750346 - Persist engine enabled state. r=nalexander,rnewman a=blocking-fennec,mfinkle
mobile/android/base/sync/ExtendedJSONObject.java
mobile/android/base/sync/GlobalSession.java
mobile/android/base/sync/MetaGlobal.java
mobile/android/base/sync/SyncConfiguration.java
--- a/mobile/android/base/sync/ExtendedJSONObject.java
+++ b/mobile/android/base/sync/ExtendedJSONObject.java
@@ -39,16 +39,17 @@ package org.mozilla.gecko.sync;
 
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.io.Reader;
 import java.io.StringReader;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.Set;
 
 import org.json.simple.JSONArray;
 import org.json.simple.JSONObject;
 import org.json.simple.parser.JSONParser;
 import org.json.simple.parser.ParseException;
 
 /**
  * Extend JSONObject to do little things, like, y'know, accessing members.
@@ -234,19 +235,28 @@ public class ExtendedJSONObject {
     return ExtendedJSONObject.parseJSONObject(val);
   }
 
   @SuppressWarnings("unchecked")
   public Iterable<Entry<String, Object>> entryIterable() {
     return this.object.entrySet();
   }
 
+  @SuppressWarnings("unchecked")
+  public Set<String> keySet() {
+    return this.object.keySet();
+  }
+
   public org.json.simple.JSONArray getArray(String key) throws NonArrayJSONException {
     Object o = this.object.get(key);
     if (o == null) {
       return null;
     }
     if (o instanceof JSONArray) {
       return (JSONArray) o;
     }
     throw new NonArrayJSONException(o);
   }
+
+  public int size() {
+    return this.object.size();
+  }
 }
--- a/mobile/android/base/sync/GlobalSession.java
+++ b/mobile/android/base/sync/GlobalSession.java
@@ -501,17 +501,16 @@ public class GlobalSession implements Cr
       // Should not occur.
       keyUploadDelegate.onKeyUploadFailed(e);
       return;
     }
 
     request.put(keysRecord);
   }
 
-
   /*
    * meta/global callbacks.
    */
   public void processMetaGlobal(MetaGlobal global) {
     config.metaGlobal = global;
 
     Long storageVersion = global.getStorageVersion();
     if (storageVersion < STORAGE_VERSION) {
@@ -532,16 +531,17 @@ public class GlobalSession implements Cr
     }
     String localSyncID = this.getSyncID();
     if (!remoteSyncID.equals(localSyncID)) {
       // Sync ID has changed. Reset timestamps and fetch new keys.
       resetAllStages();
       config.purgeCryptoKeys();
       config.syncID = remoteSyncID;
     }
+    config.enabledEngineNames = global.getEnabledEngineNames();
     config.persistToPrefs();
     advance();
   }
 
   public void processMissingMetaGlobal(MetaGlobal global) {
     freshStart();
   }
 
@@ -799,38 +799,42 @@ public class GlobalSession implements Cr
    *        with this, throwing the appropriate MetaGlobalException if not.
    * @return
    *        true if the engine with the provided name is present in the
    *        meta/global "engines" object, and verification passed.
    *
    * @throws MetaGlobalException
    */
   public boolean engineIsEnabled(String engineName, EngineSettings engineSettings) throws MetaGlobalException {
-    if (this.config.metaGlobal == null) {
-      throw new MetaGlobalNotSetException();
-    }
-    if (this.config.metaGlobal.engines == null) {
-      throw new MetaGlobalMissingEnginesException();
-    }
-    ExtendedJSONObject engineEntry;
-    try {
-      engineEntry = this.config.metaGlobal.engines.getObject(engineName);
-    } catch (NonObjectJSONException e) {
-      Logger.error(LOG_TAG, "Engine field for " + engineName + " in meta/global is not an object.");
+    // This should not occur.
+    if (this.config.enabledEngineNames == null) {
+      Logger.error(LOG_TAG, "No enabled engines in config. Giving up.");
+      if (this.config.metaGlobal == null) {
+        throw new MetaGlobalNotSetException();
+      }
       throw new MetaGlobalMissingEnginesException();
     }
 
-    if (engineEntry == null) {
+    if (!(this.config.enabledEngineNames.contains(engineName))) {
       Logger.debug(LOG_TAG, "Engine " + engineName + " not enabled: no meta/global entry.");
       return false;
     }
 
+    if (this.config.metaGlobal == null) {
+      Logger.warn(LOG_TAG, "No meta/global; using historical enabled engine names.");
+      return true;
+    }
+
+    // If we have a meta/global, check that it's safe for us to sync.
+    // (If we don't, we'll create one later, which is why we return `true` above.)
     if (engineSettings != null) {
-      MetaGlobal.verifyEngineSettings(engineEntry, engineSettings);
+      // Throws if there's a problem.
+      this.config.metaGlobal.verifyEngineSettings(engineName, engineSettings);
     }
+
     return true;
   }
 
   public ClientsDataDelegate getClientsDelegate() {
     return this.clientsDelegate;
   }
 
   /**
--- a/mobile/android/base/sync/MetaGlobal.java
+++ b/mobile/android/base/sync/MetaGlobal.java
@@ -1,39 +1,44 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 package org.mozilla.gecko.sync;
 
 import java.io.IOException;
 import java.net.URISyntaxException;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
 
 import org.json.simple.parser.ParseException;
 import org.mozilla.gecko.sync.MetaGlobalException.MetaGlobalMalformedSyncIDException;
 import org.mozilla.gecko.sync.MetaGlobalException.MetaGlobalMalformedVersionException;
-import org.mozilla.gecko.sync.MetaGlobalException.MetaGlobalStaleClientSyncIDException;
-import org.mozilla.gecko.sync.MetaGlobalException.MetaGlobalStaleClientVersionException;
 import org.mozilla.gecko.sync.delegates.MetaGlobalDelegate;
 import org.mozilla.gecko.sync.net.SyncStorageRecordRequest;
 import org.mozilla.gecko.sync.net.SyncStorageRequestDelegate;
 import org.mozilla.gecko.sync.net.SyncStorageResponse;
 
-import android.util.Log;
-
 public class MetaGlobal implements SyncStorageRequestDelegate {
   private static final String LOG_TAG = "MetaGlobal";
   protected String metaURL;
   protected String credentials;
 
   // Fields.
   protected ExtendedJSONObject  engines;
   protected Long                storageVersion;
   protected String              syncID;
 
+  // Lookup tables.
+  protected Map<String, String> syncIDs;
+  protected Map<String, Integer> versions;
+  protected Map<String, MetaGlobalException> exceptions;
+
   // Temporary location to store our callback.
   private MetaGlobalDelegate callback;
 
   // A little hack so we can use the same delegate implementation for upload and download.
   private boolean isUploading;
 
   public MetaGlobal(String metaURL, String credentials) {
     this.metaURL     = metaURL;
@@ -82,17 +87,17 @@ public class MetaGlobal implements SyncS
     CryptoRecord record = new CryptoRecord(payload);
     record.collection = "meta";
     record.guid       = "global";
     record.deleted    = false;
     return record;
   }
 
   public void setFromRecord(CryptoRecord record) throws IllegalStateException, IOException, ParseException, NonObjectJSONException {
-    Log.i(LOG_TAG, "meta/global is " + record.payload.toJSONString());
+    Logger.info(LOG_TAG, "meta/global is " + record.payload.toJSONString());
     this.storageVersion = (Long) record.payload.get("storageVersion");
     this.engines = record.payload.getObject("engines");
     this.syncID = (String) record.payload.get("syncID");
   }
 
   public Long getStorageVersion() {
     return this.storageVersion;
   }
@@ -102,65 +107,126 @@ public class MetaGlobal implements SyncS
   }
 
   public ExtendedJSONObject getEngines() {
     return engines;
   }
 
   public void setEngines(ExtendedJSONObject engines) {
     this.engines = engines;
+    final int count = engines.size();
+    versions   = new HashMap<String, Integer>(count);
+    syncIDs    = new HashMap<String, String>(count);
+    exceptions = new HashMap<String, MetaGlobalException>(count);
+    for (String engineName : engines.keySet()) {
+      try {
+        ExtendedJSONObject engineEntry = engines.getObject(engineName);
+        recordEngineState(engineName, engineEntry);
+      } catch (NonObjectJSONException e) {
+        Logger.error(LOG_TAG, "Engine field for " + engineName + " in meta/global is not an object.");
+      }
+    }
+  }
+
+  /**
+   * Take a JSON object corresponding to the 'engines' field for the provided engine name,
+   * updating {@link #syncIDs} and {@link #versions} accordingly.
+   *
+   * If the record is malformed, an entry is added to {@link #exceptions}, to be rethrown
+   * during validation.
+   */
+  protected void recordEngineState(String engineName, ExtendedJSONObject engineEntry) {
+    if (engineEntry == null) {
+      throw new IllegalArgumentException("engineEntry cannot be null.");
+    }
+    try {
+      Integer version = engineEntry.getIntegerSafely("version");
+      Logger.trace(LOG_TAG, "Engine " + engineName + " has server version " + version);
+      if (version == null ||
+          version.intValue() == 0) {
+        // Invalid version. Wipe the server.
+        Logger.warn(LOG_TAG, "Malformed version " + version +
+                             " for " + engineName + ". Recording exception.");
+        exceptions.put(engineName, new MetaGlobalMalformedVersionException());
+        return;
+      }
+      versions.put(engineName, version);
+    } catch (NumberFormatException e) {
+      // Invalid version. Wipe the server.
+      Logger.warn(LOG_TAG, "Malformed version " + engineEntry.get("version") +
+                           " for " + engineName + ". Recording exception.");
+      exceptions.put(engineName, new MetaGlobalMalformedVersionException());
+      return;
+    }
+
+    try {
+      String syncID = engineEntry.getString("syncID");
+      if (syncID == null) {
+        Logger.warn(LOG_TAG, "No syncID for " + engineName + ". Recording exception.");
+        exceptions.put(engineName, new MetaGlobalMalformedSyncIDException());
+      }
+      syncIDs.put(engineName, syncID);
+    } catch (ClassCastException e) {
+      // Malformed syncID on the server. Wipe the server.
+      Logger.warn(LOG_TAG, "Malformed syncID " + engineEntry.get("syncID") +
+                           " for " + engineName + ". Recording exception.");
+      exceptions.put(engineName, new MetaGlobalException.MetaGlobalMalformedSyncIDException());
+    }
+  }
+
+  /**
+   * Get enabled engine names.
+   *
+   * @return a collection of engine names or <code>null</code> if meta/global
+   *         was malformed.
+   */
+  public Set<String> getEnabledEngineNames() {
+    if (engines == null) {
+      return null;
+    }
+    return new HashSet<String>(engines.keySet());
   }
 
   /**
    * Returns if the server settings and local settings match.
-   * Throws a specific exception if that's not the case.
+   * Throws a specific MetaGlobalException if that's not the case.
    */
-  public static void verifyEngineSettings(ExtendedJSONObject engineEntry,
-                                          EngineSettings engineSettings)
-  throws MetaGlobalMalformedVersionException, MetaGlobalMalformedSyncIDException, MetaGlobalStaleClientVersionException, MetaGlobalStaleClientSyncIDException {
+  public void verifyEngineSettings(String engineName, EngineSettings engineSettings)
+  throws MetaGlobalException {
 
-    if (engineEntry == null) {
-      throw new IllegalArgumentException("engineEntry cannot be null.");
+    // We use syncIDs as our canary.
+    if (syncIDs == null) {
+      throw new IllegalStateException("No meta/global record yet processed.");
     }
+
     if (engineSettings == null) {
       throw new IllegalArgumentException("engineSettings cannot be null.");
     }
-    try {
-      Integer version = engineEntry.getIntegerSafely("version");
-      if (version == null ||
-          version.intValue() == 0) {
-        // Invalid version. Wipe the server.
-        throw new MetaGlobalException.MetaGlobalMalformedVersionException();
-      }
-      if (version > engineSettings.version) {
-        // We're out of date.
-        throw new MetaGlobalException.MetaGlobalStaleClientVersionException(version);
-      }
-      try {
-        String syncID = engineEntry.getString("syncID");
-        if (syncID == null) {
-          // No syncID. This should never happen. Wipe the server.
-          throw new MetaGlobalException.MetaGlobalMalformedSyncIDException();
-        }
-        if (!syncID.equals(engineSettings.syncID)) {
-          // Our syncID is wrong. Reset client and take the server syncID.
-          throw new MetaGlobalException.MetaGlobalStaleClientSyncIDException(syncID);
-        }
-        // Great!
-        return;
 
-      } catch (ClassCastException e) {
-        // Malformed syncID on the server. Wipe the server.
-        throw new MetaGlobalException.MetaGlobalMalformedSyncIDException();
-      }
-    } catch (NumberFormatException e) {
-      // Invalid version. Wipe the server.
-      throw new MetaGlobalException.MetaGlobalMalformedVersionException();
+    final String syncID = syncIDs.get(engineName);
+    if (syncID == null) {
+      throw new IllegalArgumentException("Unknown engine " + engineName);
+    }
+
+    final MetaGlobalException exception = exceptions.get(engineName);
+    if (exception != null) {
+      throw exception;
     }
 
+    final Integer version = versions.get(engineName);
+
+    if (version > engineSettings.version) {
+      // We're out of date.
+      throw new MetaGlobalException.MetaGlobalStaleClientVersionException(version);
+    }
+
+    if (!syncID.equals(engineSettings.syncID)) {
+      // Our syncID is wrong. Reset client and take the server syncID.
+      throw new MetaGlobalException.MetaGlobalStaleClientSyncIDException(syncID);
+    }
   }
 
   public String getSyncID() {
     return syncID;
   }
 
   public void setSyncID(String syncID) {
     this.syncID = syncID;
--- a/mobile/android/base/sync/SyncConfiguration.java
+++ b/mobile/android/base/sync/SyncConfiguration.java
@@ -1,16 +1,17 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 package org.mozilla.gecko.sync;
 
 import java.net.URI;
 import java.net.URISyntaxException;
+import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
 
 import org.mozilla.gecko.sync.crypto.KeyBundle;
 import org.mozilla.gecko.sync.crypto.PersistedCrypto5Keys;
 
 import android.content.SharedPreferences;
 import android.content.SharedPreferences.Editor;
@@ -180,27 +181,39 @@ public class SyncConfiguration implement
   public KeyBundle       syncKeyBundle;
 
   public CollectionKeys  collectionKeys;
   public InfoCollections infoCollections;
   public MetaGlobal      metaGlobal;
   public String          password;
   public String          syncID;
 
+  /**
+   * Persisted collection of enabledEngineNames.
+   * <p>
+   * Can contain engines Android Sync is not currently aware of, such as "prefs"
+   * or "addons".
+   * <p>
+   * Copied from latest downloaded meta/global record and used to generate a
+   * fresh meta/global record for upload.
+   */
+  public Set<String>     enabledEngineNames;
+
   // Fields that maintain a reference to a SharedPreferences instance, used for
   // persistence.
   // Behavior is undefined if the PrefsSource is switched out in flight.
   public String          prefsPath;
   public PrefsSource     prefsSource;
 
   public static final String CLIENTS_COLLECTION_TIMESTAMP = "serverClientsTimestamp";  // When the collection was touched.
   public static final String CLIENT_RECORD_TIMESTAMP = "serverClientRecordTimestamp";  // When our record was touched.
 
   public static final String PREF_CLUSTER_URL = "clusterURL";
   public static final String PREF_SYNC_ID = "syncID";
+  public static final String PREF_ENABLED_ENGINE_NAMES = "enabledEngineNames";
 
   /**
    * Create a new SyncConfiguration instance. Pass in a PrefsSource to
    * provide access to preferences.
    */
   public SyncConfiguration(String prefsPath, PrefsSource prefsSource) {
     this.prefsPath   = prefsPath;
     this.prefsSource = prefsSource;
@@ -232,16 +245,25 @@ public class SyncConfiguration implement
       } catch (URISyntaxException e) {
         Logger.warn(LOG_TAG, "Ignoring bundle clusterURL (" + u + "): invalid URI.", e);
       }
     }
     if (prefs.contains(PREF_SYNC_ID)) {
       syncID = prefs.getString(PREF_SYNC_ID, null);
       Logger.info(LOG_TAG, "Set syncID from bundle: " + syncID);
     }
+    if (prefs.contains(PREF_ENABLED_ENGINE_NAMES)) {
+      String json = prefs.getString(PREF_ENABLED_ENGINE_NAMES, null);
+      try {
+        ExtendedJSONObject o = ExtendedJSONObject.parseJSONObject(json);
+        enabledEngineNames = new HashSet<String>(o.keySet());
+      } catch (Exception e) {
+        // enabledEngineNames can be null.
+      }
+    }
     // We don't set crypto/keys here because we need the syncKeyBundle to decrypt the JSON
     // and we won't have it on construction.
     // TODO: MetaGlobal, password, infoCollections.
   }
 
   public void persistToPrefs() {
     this.persistToPrefs(this.getPrefs());
   }
@@ -251,16 +273,25 @@ public class SyncConfiguration implement
     if (clusterURL == null) {
       edit.remove(PREF_CLUSTER_URL);
     } else {
       edit.putString(PREF_CLUSTER_URL, clusterURL.toASCIIString());
     }
     if (syncID != null) {
       edit.putString(PREF_SYNC_ID, syncID);
     }
+    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.commit();
     // TODO: keys.
   }
 
   @Override
   public String credentials() {
     return username + ":" + password;
   }