From 1ee65dc313fdde8dcabb65e16022fd45aed0419a Mon Sep 17 00:00:00 2001
From: Edouard Oger <eoger@fastmail.com>
Date: Tue, 14 Mar 2017 14:42:24 -0400
Subject: [PATCH] Bug 692700 part 1 - Set X-If-Unmodified-Since when uploading
 meta/global. r=markh

MozReview-Commit-ID: IoiThiDlvCj

--HG--
extra : rebase_source : f78434e661810f9b42da6d58422713fd20687e28
---
 services/sync/modules/service.js | 53 +++++++++++++++++---------------
 1 file changed, 28 insertions(+), 25 deletions(-)

diff --git a/services/sync/modules/service.js b/services/sync/modules/service.js
index d1e5d9f83740..c308ff0cc749 100644
--- a/services/sync/modules/service.js
+++ b/services/sync/modules/service.js
@@ -902,16 +902,9 @@ Sync11Service.prototype = {
 
       if (this.recordManager.response.status == 404) {
         this._log.debug("No meta/global record on the server. Creating one.");
-        newMeta = new WBORecord("meta", "global");
-        newMeta.payload.syncID = this.syncID;
-        newMeta.payload.storageVersion = STORAGE_VERSION;
-        newMeta.payload.declined = this.engineManager.getDeclined();
-
-        newMeta.isNew = true;
-
-        this.recordManager.set(this.metaURL, newMeta);
-        let uploadRes = newMeta.upload(this.resource(this.metaURL));
-        if (!uploadRes.success) {
+        try {
+          this._uploadNewMetaGlobal();
+        } catch (uploadRes) {
           this._log.warn("Unable to upload new meta/global. Failing remote setup.");
           this.errorHandler.checkServerError(uploadRes);
           return false;
@@ -1116,20 +1109,36 @@ Sync11Service.prototype = {
   },
 
   /**
-   * Upload meta/global, throwing the response on failure.
+   * Upload a fresh meta/global record
+   * @throws the response object if the upload request was not a success
+   */
+  _uploadNewMetaGlobal() {
+    let meta = new WBORecord("meta", "global");
+    meta.payload.syncID = this.syncID;
+    meta.payload.storageVersion = STORAGE_VERSION;
+    meta.payload.declined = this.engineManager.getDeclined();
+    meta.modified = 0;
+    meta.isNew = true;
+
+    this.uploadMetaGlobal(meta);
+  },
+
+  /**
+   * Upload meta/global, throwing the response on failure
+   * @param {WBORecord} meta meta/global record
+   * @throws the response object if the request was not a success
    */
   uploadMetaGlobal(meta) {
-    this._log.debug("Uploading meta/global: " + JSON.stringify(meta));
-
-    // It would be good to set the X-If-Unmodified-Since header to `timestamp`
-    // for this PUT to ensure at least some level of transactionality.
-    // Unfortunately, the servers don't support it after a wipe right now
-    // (bug 693893), so we're going to defer this until bug 692700.
+    this._log.debug("Uploading meta/global", meta);
     let res = this.resource(this.metaURL);
+    res.setHeader("X-If-Unmodified-Since", meta.modified);
     let response = res.put(meta);
     if (!response.success) {
       throw response;
     }
+    // From https://docs.services.mozilla.com/storage/apis-1.5.html:
+    // "Successful responses will return the new last-modified time for the collection."
+    meta.modified = response.obj;
     this.recordManager.set(this.metaURL, meta);
   },
 
@@ -1142,17 +1151,11 @@ Sync11Service.prototype = {
     this.wipeServer();
 
     // Upload a new meta/global record.
-    let meta = new WBORecord("meta", "global");
-    meta.payload.syncID = this.syncID;
-    meta.payload.storageVersion = STORAGE_VERSION;
-    meta.payload.declined = this.engineManager.getDeclined();
-    meta.isNew = true;
-
-    // uploadMetaGlobal throws on failure -- including race conditions.
+    // _uploadNewMetaGlobal throws on failure -- including race conditions.
     // If we got into a race condition, we'll abort the sync this way, too.
     // That's fine. We'll just wait till the next sync. The client that we're
     // racing is probably busy uploading stuff right now anyway.
-    this.uploadMetaGlobal(meta);
+    this._uploadNewMetaGlobal();
 
     // Wipe everything we know about except meta because we just uploaded it
     // TODO: there's a bug here. We should be calling resetClient, no?