From 51af941cf39c195757d3709ac002bc6329e794b6 Mon Sep 17 00:00:00 2001 From: Richard Newman Date: Tue, 28 Jan 2014 16:20:50 -0800 Subject: [PATCH] Bug 963424 - Use token server-provided cluster URL. r=nalexander --- mobile/android/base/android-services.mozbuild | 1 + .../base/fxa/sync/FxAccountGlobalSession.java | 6 +- mobile/android/base/sync/GlobalSession.java | 1 - .../base/sync/Sync11Configuration.java | 84 +++++++++++++++++++ .../android/base/sync/SyncConfiguration.java | 55 ++++-------- .../sync/config/ClientRecordTerminator.java | 18 ++-- .../receivers/SyncAccountDeletedService.java | 43 +++++----- .../sync/repositories/Server11Repository.java | 1 - .../base/sync/syncadapter/SyncAdapter.java | 4 +- 9 files changed, 137 insertions(+), 76 deletions(-) create mode 100644 mobile/android/base/sync/Sync11Configuration.java diff --git a/mobile/android/base/android-services.mozbuild b/mobile/android/base/android-services.mozbuild index f1a024a16c59..8e6ed82c76e2 100644 --- a/mobile/android/base/android-services.mozbuild +++ b/mobile/android/base/android-services.mozbuild @@ -801,6 +801,7 @@ sync_java_files = [ 'sync/stage/ServerSyncStage.java', 'sync/stage/SyncClientsEngineStage.java', 'sync/stage/UploadMetaGlobalStage.java', + 'sync/Sync11Configuration.java', 'sync/syncadapter/SyncAdapter.java', 'sync/syncadapter/SyncService.java', 'sync/SyncConfiguration.java', diff --git a/mobile/android/base/fxa/sync/FxAccountGlobalSession.java b/mobile/android/base/fxa/sync/FxAccountGlobalSession.java index 94e725a9ae8a..692045c46d19 100644 --- a/mobile/android/base/fxa/sync/FxAccountGlobalSession.java +++ b/mobile/android/base/fxa/sync/FxAccountGlobalSession.java @@ -33,9 +33,9 @@ public class FxAccountGlobalSession extends GlobalSession { throws SyncConfigurationException, IllegalArgumentException, IOException, ParseException, NonObjectJSONException, URISyntaxException { super(config, callback, context, extras, clientsDelegate, null); - URI uri = new URI(storageEndpoint); - this.config.clusterURL = new URI(uri.getScheme(), uri.getUserInfo(), uri.getHost(), uri.getPort(), "/", null, null); - FxAccountConstants.pii(LOG_TAG, "storageEndpoint is " + uri + " and clusterURL is " + config.clusterURL); + URI storageURI = new URI(storageEndpoint); + this.config.setClusterURL(storageURI); + FxAccountConstants.pii(LOG_TAG, "clusterURL is " + config.getClusterURLString()); } @Override diff --git a/mobile/android/base/sync/GlobalSession.java b/mobile/android/base/sync/GlobalSession.java index d1a9bade1116..19f5e56d7379 100644 --- a/mobile/android/base/sync/GlobalSession.java +++ b/mobile/android/base/sync/GlobalSession.java @@ -63,7 +63,6 @@ import ch.boye.httpclientandroidlib.HttpResponse; public class GlobalSession implements PrefsSource, HttpResponseObserver { private static final String LOG_TAG = "GlobalSession"; - public static final String API_VERSION = "1.1"; public static final long STORAGE_VERSION = 5; public SyncConfiguration config = null; diff --git a/mobile/android/base/sync/Sync11Configuration.java b/mobile/android/base/sync/Sync11Configuration.java new file mode 100644 index 000000000000..4b2280895589 --- /dev/null +++ b/mobile/android/base/sync/Sync11Configuration.java @@ -0,0 +1,84 @@ +/* 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 org.mozilla.gecko.background.common.log.Logger; +import org.mozilla.gecko.sync.crypto.KeyBundle; +import org.mozilla.gecko.sync.net.AuthHeaderProvider; + +import android.content.SharedPreferences; +import android.content.SharedPreferences.Editor; + +/** + * Override SyncConfiguration to restore the old behavior of clusterURL -- + * that is, a URL without the protocol version etc. + * + */ +public class Sync11Configuration extends SyncConfiguration { + private static final String LOG_TAG = "Sync11Configuration"; + private static final String API_VERSION = "1.1"; + + public Sync11Configuration(String username, + AuthHeaderProvider authHeaderProvider, + SharedPreferences prefs) { + super(username, authHeaderProvider, prefs); + } + + public Sync11Configuration(String username, + AuthHeaderProvider authHeaderProvider, + SharedPreferences prefs, + KeyBundle keyBundle) { + super(username, authHeaderProvider, prefs, keyBundle); + } + + @Override + public String getAPIVersion() { + return API_VERSION; + } + + @Override + public String storageURL() { + return clusterURL + API_VERSION + "/" + username + "/storage"; + } + + @Override + protected String infoBaseURL() { + return clusterURL + API_VERSION + "/" + username + "/info/"; + } + + protected void setAndPersistClusterURL(URI u, SharedPreferences prefs) { + boolean shouldPersist = (prefs != null) && (clusterURL == null); + + Logger.trace(LOG_TAG, "Setting cluster URL to " + u.toASCIIString() + + (shouldPersist ? ". Persisting." : ". Not persisting.")); + clusterURL = u; + if (shouldPersist) { + Editor edit = prefs.edit(); + edit.putString(PREF_CLUSTER_URL, clusterURL.toASCIIString()); + edit.commit(); + } + } + + protected void setClusterURL(URI u, SharedPreferences prefs) { + if (u == null) { + Logger.warn(LOG_TAG, "Refusing to set cluster URL to null."); + return; + } + URI uri = u.normalize(); + if (uri.toASCIIString().endsWith("/")) { + setAndPersistClusterURL(u, prefs); + return; + } + setAndPersistClusterURL(uri.resolve("/"), prefs); + Logger.trace(LOG_TAG, "Set cluster URL to " + clusterURL.toASCIIString() + ", given input " + u.toASCIIString()); + } + + @Override + public void setClusterURL(URI u) { + setClusterURL(u, this.getPrefs()); + } +} \ No newline at end of file diff --git a/mobile/android/base/sync/SyncConfiguration.java b/mobile/android/base/sync/SyncConfiguration.java index 316543e675d9..a801818a5b42 100644 --- a/mobile/android/base/sync/SyncConfiguration.java +++ b/mobile/android/base/sync/SyncConfiguration.java @@ -258,6 +258,8 @@ public class SyncConfiguration { public static final String PREF_CLIENT_NAME = "account.clientName"; public static final String PREF_NUM_CLIENTS = "account.numClients"; + private static final String API_VERSION = "1.5"; + /** * Create a new SyncConfiguration instance. Pass in a PrefsSource to * provide access to preferences. @@ -278,6 +280,10 @@ public class SyncConfiguration { this.syncKeyBundle = syncKeyBundle; } + public String getAPIVersion() { + return API_VERSION; + } + public SharedPreferences getPrefs() { return this.prefs; } @@ -453,8 +459,17 @@ public class SyncConfiguration { collectionKeys = k; } + /** + * Return path to storage endpoint without trailing slash. + * + * @return storage endpoint without trailing slash. + */ + public String storageURL() { + return clusterURL + "/storage"; + } + protected String infoBaseURL() { - return clusterURL + GlobalSession.API_VERSION + "/" + username + "/info/"; + return clusterURL + "/info/"; } public String infoCollectionsURL() { @@ -469,15 +484,6 @@ public class SyncConfiguration { return storageURL() + "/meta/global"; } - /** - * Return path to storage endpoint without trailing slash. - * - * @return storage endpoint without trailing slash. - */ - public String storageURL() { - return clusterURL + GlobalSession.API_VERSION + "/" + username + "/storage"; - } - public URI collectionURI(String collection) throws URISyntaxException { return new URI(storageURL() + "/" + collection); } @@ -517,35 +523,8 @@ public class SyncConfiguration { return clusterURL.toASCIIString(); } - protected void setAndPersistClusterURL(URI u, SharedPreferences prefs) { - boolean shouldPersist = (prefs != null) && (clusterURL == null); - - Logger.trace(LOG_TAG, "Setting cluster URL to " + u.toASCIIString() + - (shouldPersist ? ". Persisting." : ". Not persisting.")); - clusterURL = u; - if (shouldPersist) { - Editor edit = prefs.edit(); - edit.putString(PREF_CLUSTER_URL, clusterURL.toASCIIString()); - edit.commit(); - } - } - - protected void setClusterURL(URI u, SharedPreferences prefs) { - if (u == null) { - Logger.warn(LOG_TAG, "Refusing to set cluster URL to null."); - return; - } - URI uri = u.normalize(); - if (uri.toASCIIString().endsWith("/")) { - setAndPersistClusterURL(u, prefs); - return; - } - setAndPersistClusterURL(uri.resolve("/"), prefs); - Logger.trace(LOG_TAG, "Set cluster URL to " + clusterURL.toASCIIString() + ", given input " + u.toASCIIString()); - } - public void setClusterURL(URI u) { - setClusterURL(u, this.getPrefs()); + this.clusterURL = u; } /** diff --git a/mobile/android/base/sync/config/ClientRecordTerminator.java b/mobile/android/base/sync/config/ClientRecordTerminator.java index b7f7efaa5cf4..0ec1d3955d03 100644 --- a/mobile/android/base/sync/config/ClientRecordTerminator.java +++ b/mobile/android/base/sync/config/ClientRecordTerminator.java @@ -7,7 +7,7 @@ package org.mozilla.gecko.sync.config; import java.net.URI; import org.mozilla.gecko.background.common.log.Logger; -import org.mozilla.gecko.sync.GlobalSession; +import org.mozilla.gecko.sync.SyncConfiguration; import org.mozilla.gecko.sync.net.AuthHeaderProvider; import org.mozilla.gecko.sync.net.BaseResource; import org.mozilla.gecko.sync.net.SyncStorageRecordRequest; @@ -27,22 +27,18 @@ public class ClientRecordTerminator { super(); // Stop this class from being instantiated. } - public static void deleteClientRecord(final String username, - final String clusterURL, - final String clientGuid, - final AuthHeaderProvider authHeaderProvider) + public static void deleteClientRecord(final SyncConfiguration config, final String clientGUID) throws Exception { - // Would prefer to delegate to SyncConfiguration, but that would proliferate static methods. final String collection = "clients"; - final URI wboURI = new URI(clusterURL + GlobalSession.API_VERSION + "/" + username + "/storage/" + collection + "/" + clientGuid); + final URI wboURI = config.wboURI(collection, clientGUID); // Would prefer to break this out into a self-contained client library. final SyncStorageRecordRequest r = new SyncStorageRecordRequest(wboURI); r.delegate = new SyncStorageRequestDelegate() { @Override public AuthHeaderProvider getAuthHeaderProvider() { - return authHeaderProvider; + return config.getAuthHeaderProvider(); } @Override @@ -52,13 +48,13 @@ public class ClientRecordTerminator { @Override public void handleRequestSuccess(SyncStorageResponse response) { - Logger.info(LOG_TAG, "Deleted client record with GUID " + clientGuid + " from server."); + Logger.info(LOG_TAG, "Deleted client record with GUID " + clientGUID + " from server."); BaseResource.consumeEntity(response); } @Override public void handleRequestFailure(SyncStorageResponse response) { - Logger.warn(LOG_TAG, "Failed to delete client record with GUID " + clientGuid + " from server."); + Logger.warn(LOG_TAG, "Failed to delete client record with GUID " + clientGUID + " from server."); try { Logger.warn(LOG_TAG, "Server error message was: " + response.getErrorMessage()); } catch (Exception e) { @@ -71,7 +67,7 @@ public class ClientRecordTerminator { public void handleRequestError(Exception ex) { // It could be that we don't have network access when trying // to remove an Account; not much to be done in this situation. - Logger.error(LOG_TAG, "Got exception trying to delete client record with GUID " + clientGuid + " from server; ignoring.", ex); + Logger.error(LOG_TAG, "Got exception trying to delete client record with GUID " + clientGUID + " from server; ignoring.", ex); } }; diff --git a/mobile/android/base/sync/receivers/SyncAccountDeletedService.java b/mobile/android/base/sync/receivers/SyncAccountDeletedService.java index 8b5b55b80d22..ac95549e6a72 100644 --- a/mobile/android/base/sync/receivers/SyncAccountDeletedService.java +++ b/mobile/android/base/sync/receivers/SyncAccountDeletedService.java @@ -7,6 +7,7 @@ package org.mozilla.gecko.sync.receivers; import org.mozilla.gecko.sync.ExtendedJSONObject; import org.mozilla.gecko.background.common.GlobalConstants; import org.mozilla.gecko.background.common.log.Logger; +import org.mozilla.gecko.sync.Sync11Configuration; import org.mozilla.gecko.sync.SyncConstants; import org.mozilla.gecko.sync.SyncConfiguration; import org.mozilla.gecko.sync.Utils; @@ -113,27 +114,29 @@ public class SyncAccountDeletedService extends IntentService { return; } - final String clientGuid = prefs.getString(SyncConfiguration.PREF_ACCOUNT_GUID, null); - final String clusterURL = prefs.getString(SyncConfiguration.PREF_CLUSTER_URL, null); - - // Finally, a good place to do this. - prefs.edit().clear().commit(); - - if (clientGuid == null) { - Logger.warn(LOG_TAG, "Client GUID was null; not deleting client record from server."); - return; - } - - if (clusterURL == null) { - Logger.warn(LOG_TAG, "Cluster URL was null; not deleting client record from server."); - return; - } - try { - ClientRecordTerminator.deleteClientRecord(encodedUsername, clusterURL, clientGuid, new BasicAuthHeaderProvider(encodedUsername, password)); - } catch (Exception e) { - // This should never happen, but we really don't want to die in a background thread. - Logger.warn(LOG_TAG, "Got exception deleting client record from server; ignoring.", e); + final String clientGUID = prefs.getString(SyncConfiguration.PREF_ACCOUNT_GUID, null); + if (clientGUID == null) { + Logger.warn(LOG_TAG, "Client GUID was null; not deleting client record from server."); + return; + } + + BasicAuthHeaderProvider authHeaderProvider = new BasicAuthHeaderProvider(encodedUsername, password); + SyncConfiguration configuration = new Sync11Configuration(encodedUsername, authHeaderProvider, prefs); + if (configuration.getClusterURL() == null) { + Logger.warn(LOG_TAG, "Cluster URL was null; not deleting client record from server."); + return; + } + + try { + ClientRecordTerminator.deleteClientRecord(configuration, clientGUID); + } catch (Exception e) { + // This should never happen, but we really don't want to die in a background thread. + Logger.warn(LOG_TAG, "Got exception deleting client record from server; ignoring.", e); + } + } finally { + // Finally, a good place to do this. + prefs.edit().clear().commit(); } } } diff --git a/mobile/android/base/sync/repositories/Server11Repository.java b/mobile/android/base/sync/repositories/Server11Repository.java index 7f0db6847f4a..0421e353f752 100644 --- a/mobile/android/base/sync/repositories/Server11Repository.java +++ b/mobile/android/base/sync/repositories/Server11Repository.java @@ -24,7 +24,6 @@ public class Server11Repository extends Repository { protected String collection; protected URI collectionURI; protected final AuthHeaderProvider authHeaderProvider; - public static final String VERSION_PATH_FRAGMENT = "1.1/"; /** * Construct a new repository that fetches and stores against the Sync 1.1. API. diff --git a/mobile/android/base/sync/syncadapter/SyncAdapter.java b/mobile/android/base/sync/syncadapter/SyncAdapter.java index 7f2d8ca11e01..77565cf58ab8 100644 --- a/mobile/android/base/sync/syncadapter/SyncAdapter.java +++ b/mobile/android/base/sync/syncadapter/SyncAdapter.java @@ -19,6 +19,7 @@ import org.mozilla.gecko.sync.GlobalSession; import org.mozilla.gecko.sync.NonObjectJSONException; import org.mozilla.gecko.sync.SharedPreferencesClientsDataDelegate; import org.mozilla.gecko.sync.SharedPreferencesNodeAssignmentCallback; +import org.mozilla.gecko.sync.Sync11Configuration; import org.mozilla.gecko.sync.SyncConfiguration; import org.mozilla.gecko.sync.SyncConfigurationException; import org.mozilla.gecko.sync.SyncConstants; @@ -527,8 +528,7 @@ public class SyncAdapter extends AbstractThreadedSyncAdapter implements BaseGlob final AuthHeaderProvider authHeaderProvider = new BasicAuthHeaderProvider(username, password); final SharedPreferences prefs = getContext().getSharedPreferences(prefsPath, Utils.SHARED_PREFERENCES_MODE); - final SyncConfiguration config = new SyncConfiguration(username, authHeaderProvider, prefs); - config.syncKeyBundle = keyBundle; + final SyncConfiguration config = new Sync11Configuration(username, authHeaderProvider, prefs, keyBundle); GlobalSession globalSession = new GlobalSession(config, this, this.mContext, extras, clientsDataDelegate, nodeAssignmentDelegate); globalSession.start();