Bug 960887 - Handle storage endpoints returned by 1.5 token server. r=rnewman

services/sync/modules/browserid_identity.js

@@ -478,12 +478,14 @@ BrowserIDClusterManager.prototype = {
     let promiseClusterURL = function() {
       return fxAccounts.getSignedInUser().then(userData => {
         return this.identity._fetchTokenForUser(userData).then(token => {
-          // Set the clusterURI for this user based on the endpoint in the
-          // token. This is a bit of a hack, and we should figure out a better
-          // way of distributing it to components that need it.
-          let clusterURI = Services.io.newURI(token.endpoint, null, null);
-          clusterURI.path = "/";
-          return clusterURI.spec;
+          let endpoint = token.endpoint;
+          // For Sync 1.5 storage endpoints, we use the base endpoint verbatim.
+          // However, it should end in "/" because we will extend it with
+          // well known path components. So we add a "/" if it's missing.
+          if (!endpoint.endsWith("/")) {
+            endpoint += "/";
+          }
+          return endpoint;
         });
       });
     }.bind(this);
@@ -497,4 +499,13 @@ BrowserIDClusterManager.prototype = {
     });
     return cb.wait();
   },
+
+  getUserBaseURL: function() {
+    // Legacy Sync and FxA Sync construct the userBaseURL differently. Legacy
+    // Sync appends path components onto an empty path, and in FxA Sync the
+    // token server constructs this for us in an opaque manner. Since the
+    // cluster manager already sets the clusterURL on Service and also has
+    // access to the current identity, we added this functionality here.
+    return this.service.clusterURL;
+  }
 }

services/sync/modules/engines.js

@@ -651,8 +651,7 @@ SyncEngine.prototype = {
   // How many records to process in a single batch.
   applyIncomingBatchSize: DEFAULT_STORE_BATCH_SIZE,
 
-  get storageURL() Svc.Prefs.get("clusterURL") + SYNC_API_VERSION +
-    "/" + this.service.identity.username + "/storage/",
+  get storageURL() this.service.storageURL,
 
   get engineURL() this.storageURL + this.name,
 

services/sync/modules/service.js

@@ -60,7 +60,6 @@ Sync11Service.prototype = {
   _locked: false,
   _loggedIn: false,
 
-  userBaseURL: null,
   infoURL: null,
   storageURL: null,
   metaURL: null,
@@ -156,13 +155,18 @@ Sync11Service.prototype = {
     return Utils.catch.call(this, func, lockExceptions);
   },
 
+  get userBaseURL() {
+    if (!this._clusterManager) {
+      return null;
+    }
+    return this._clusterManager.getUserBaseURL();
+  },
+
   _updateCachedURLs: function _updateCachedURLs() {
     // Nothing to cache yet if we don't have the building blocks
     if (this.clusterURL == "" || this.identity.username == "")
       return;
 
-    let storageAPI = this.clusterURL + SYNC_API_VERSION + "/";
-    this.userBaseURL = storageAPI + this.identity.username + "/";
     this._log.debug("Caching URLs under storage user base: " + this.userBaseURL);
 
     // Generate and cache various URLs under the storage API for this user

services/sync/modules/stages/cluster.js

@@ -91,5 +91,21 @@ ClusterManager.prototype = {
 
     return true;
   },
+
+  getUserBaseURL: function getUserBaseURL() {
+    // Legacy Sync and FxA Sync construct the userBaseURL differently. Legacy
+    // Sync appends path components onto an empty path, and in FxA Sync, the
+    // token server constructs this for us in an opaque manner. Since the
+    // cluster manager already sets the clusterURL on Service and also has
+    // access to the current identity, we added this functionality here.
+
+    // If the clusterURL hasn't been set, the userBaseURL shouldn't be set
+    // either. Some tests expect "undefined" to be returned here.
+    if (!this.service.clusterURL) {
+      return undefined;
+    }
+    let storageAPI = this.service.clusterURL + SYNC_API_VERSION + "/";
+    return storageAPI + this.identity.username + "/";
+  }
 };
 Object.freeze(ClusterManager.prototype);

services/sync/tests/unit/test_service_attributes.js

@@ -7,7 +7,7 @@ Cu.import("resource://services-sync/util.js");
 Cu.import("resource://testing-common/services/sync/fakeservices.js");
 
 function test_urls() {
-  _("URL related Service properties corresopnd to preference settings.");
+  _("URL related Service properties correspond to preference settings.");
   try {
     do_check_true(!!Service.serverURL); // actual value may change
     do_check_eq(Service.clusterURL, "");

services/sync/tests/unit/test_syncengine_sync.js

@@ -48,6 +48,7 @@ function createServerAndConfigureClient() {
   Service.serverURL = server.baseURI;
   Service.clusterURL = server.baseURI;
   Service.identity.username = USER;
+  Service._updateCachedURLs();
 
   return [engine, server, USER];
 }