--- a/services/sync/modules/bookmark_repair.js
+++ b/services/sync/modules/bookmark_repair.js
@@ -654,17 +654,17 @@ class BookmarkRepairResponder extends Co
// Throwing here means we abort the repair, which isn't ideal for transient
// errors (eg, no network, 500 service outage etc), but we don't currently
// have a sane/safe way to try again later (we'd need to implement a kind
// of timeout, otherwise we might end up retrying forever and never remove
// our request command.) Bug 1347805.
if (!itemsResponse.success) {
throw new Error(`request for server IDs failed: ${itemsResponse.status}`);
}
- let existRemotely = new Set(JSON.parse(itemsResponse));
+ let existRemotely = new Set(itemsResponse.obj);
// We need to be careful about handing the requested items:
// * If the item exists locally but isn't in the tree of items we sync
// (eg, it might be a left-pane item or similar, we write a tombstone.
// * If the item exists locally as a folder, we upload the folder and any
// children which don't exist on the server. (Note that we assume the
// parents *do* exist)
// Bug 1343101 covers additional issues we might repair in the future.
for (let { recordId: id, syncable } of repairable) {
--- a/services/sync/modules/engines.js
+++ b/services/sync/modules/engines.js
@@ -1003,19 +1003,17 @@ SyncEngine.prototype = {
// Put the new data back into meta/global and mark for upload
engines[this.name] = engineData;
metaGlobal.payload.engines = engines;
metaGlobal.changed = true;
} else if (engineData.version > this.version) {
// Don't sync this engine if the server has newer data
- // Changes below need to be processed in bug 1295510 that's why eslint is ignored
- // eslint-disable-next-line no-new-wrappers
- let error = new String("New data: " + [engineData.version, this.version]);
+ let error = new Error("New data: " + [engineData.version, this.version]);
error.failureCode = VERSION_OUT_OF_DATE;
throw error;
} else {
// Changes to syncID mean we'll need to upload everything
let assignedSyncID = await this.ensureCurrentSyncID(engineData.syncID);
if (assignedSyncID != engineData.syncID) {
engineData.syncID = assignedSyncID;
metaGlobal.changed = true;
--- a/services/sync/modules/engines/bookmarks.js
+++ b/services/sync/modules/engines/bookmarks.js
@@ -515,21 +515,20 @@ BookmarksEngine.prototype = {
continue;
}
let parentName = parent.title || "";
if (guidMap[parentName] == null)
guidMap[parentName] = {};
// If the entry already exists, remember that there are explicit dupes.
-
- // Changes below need to be processed in bug 1295510 that's why eslint is ignored
- // eslint-disable-next-line no-new-wrappers
- let entry = new String(guid);
- entry.hasDupe = guidMap[parentName][key] != null;
+ let entry = {
+ guid,
+ hasDupe: guidMap[parentName][key] != null,
+ };
// Remember this item's GUID for its parent-name/key pair.
guidMap[parentName][key] = entry;
this._log.trace("Mapped: " + [parentName, key, entry, entry.hasDupe]);
}
return guidMap;
},
@@ -576,24 +575,24 @@ BookmarksEngine.prototype = {
if (!parent) {
this._log.trace("No parent => no dupe.");
return undefined;
}
let dupe = parent[key];
if (dupe) {
- this._log.trace("Mapped dupe: " + dupe);
+ this._log.trace("Mapped dupe", dupe);
return dupe;
}
if (altKey) {
dupe = parent[altKey];
if (dupe) {
- this._log.trace("Mapped dupe using altKey " + altKey + ": " + dupe);
+ this._log.trace("Mapped dupe using altKey " + altKey, dupe);
return dupe;
}
}
this._log.trace("No dupe found for key " + key + "/" + altKey + ".");
return undefined;
},
@@ -712,20 +711,18 @@ BookmarksEngine.prototype = {
" (already duped: " + item.hasDupe + ").");
// Don't bother finding a dupe if the incoming item has duplicates.
if (item.hasDupe) {
this._log.trace(item.id + " already a dupe: not finding one.");
return null;
}
let mapped = await this._mapDupe(item);
- this._log.debug(item.id + " mapped to " + mapped);
- // We must return a string, not an object, and the entries in the GUIDMap
- // are created via "new String()" making them an object.
- return mapped ? mapped.toString() : mapped;
+ this._log.debug(item.id + " mapped to", mapped);
+ return mapped ? mapped.guid : null;
},
// Called when _findDupe returns a dupe item and the engine has decided to
// switch the existing item to the new incoming item.
async _switchItemToDupe(localDupeGUID, incomingItem) {
let newChanges = await PlacesSyncUtils.bookmarks.dedupe(localDupeGUID,
incomingItem.id,
incomingItem.parentid);
--- a/services/sync/modules/record.js
+++ b/services/sync/modules/record.js
@@ -41,17 +41,17 @@ WBORecord.prototype = {
// Set thine 'response' field.
async fetch(resource) {
if (!(resource instanceof Resource)) {
throw new Error("First argument must be a Resource instance.");
}
let r = await resource.get();
if (r.success) {
- this.deserialize(r); // Warning! Muffles exceptions!
+ this.deserialize(r.obj); // Warning! Muffles exceptions!
}
this.response = r;
return this;
},
upload(resource) {
if (!(resource instanceof Resource)) {
throw new Error("First argument must be a Resource instance.");
@@ -67,18 +67,20 @@ WBORecord.prototype = {
let url = CommonUtils.makeURI(base + this.collection + "/" + this.id);
url.QueryInterface(Ci.nsIURL);
return url;
}
return null;
},
deserialize: function deserialize(json) {
- this.data = json.constructor.toString() == String ? JSON.parse(json) : json;
-
+ if (!json || typeof json !== "object") {
+ throw new TypeError("Can't deserialize record from: " + json);
+ }
+ this.data = json;
try {
// The payload is likely to be JSON, but if not, keep it as a string
this.payload = JSON.parse(this.payload);
} catch (ex) {}
},
toJSON: function toJSON() {
// Copy fields from data to be stringified, making sure payload is a string
@@ -233,17 +235,17 @@ RecordManager.prototype = {
this.response = {};
this.response = await this.service.resource(url).get();
// Don't parse and save the record on failure
if (!this.response.success)
return null;
let record = new this._recordType(url);
- record.deserialize(this.response);
+ record.deserialize(this.response.obj);
return this.set(url, record);
} catch (ex) {
if (Async.isShutdownException(ex)) {
throw ex;
}
this._log.debug("Failed to import record", ex);
return null;
--- a/services/sync/modules/resource.js
+++ b/services/sync/modules/resource.js
@@ -186,39 +186,38 @@ Resource.prototype = {
return this._processResponse(response, method);
},
async _processResponse(response, method) {
const data = await response.text();
this._logResponse(response, method, data);
this._processResponseHeaders(response);
- // Changes below need to be processed in bug 1295510 that's why eslint is ignored
- // eslint-disable-next-line no-new-wrappers
- const ret = new String(data);
- ret.url = response.url;
- ret.status = response.status;
- ret.success = response.ok;
- ret.headers = {};
+ const ret = {
+ data,
+ url: response.url,
+ status: response.status,
+ success: response.ok,
+ headers: {},
+ };
for (const [k, v] of response.headers) {
ret.headers[k] = v;
}
// Make a lazy getter to convert the json response into an object.
// Note that this can cause a parse error to be thrown far away from the
// actual fetch, so be warned!
XPCOMUtils.defineLazyGetter(ret, "obj", () => {
try {
- return JSON.parse(ret);
+ return JSON.parse(ret.data);
} catch (ex) {
this._log.warn("Got exception parsing response body", ex);
// Stringify to avoid possibly printing non-printable characters.
- this._log.debug("Parse fail: Response body starts: \"" +
- JSON.stringify((ret + "").slice(0, 100)) +
- "\".");
+ this._log.debug("Parse fail: Response body starts",
+ (ret.data + "").slice(0, 100));
throw ex;
}
});
return ret;
},
_logResponse(response, method, data) {
--- a/services/sync/modules/service.js
+++ b/services/sync/modules/service.js
@@ -746,17 +746,17 @@ Sync11Service.prototype = {
}
this._log.info("Got status " + uploadRes.status + " uploading keys.");
let serverModified = uploadRes.obj; // Modified timestamp according to server.
this._log.debug("Server reports crypto modified: " + serverModified);
// Now verify that info/collections shows them!
this._log.debug("Verifying server collection records.");
let info = await this._fetchInfo();
- this._log.debug("info/collections is: " + info);
+ this._log.debug("info/collections is: " + info.data);
if (info.status != 200) {
this._log.warn("Non-200 info/collections response. Aborting.");
throw new Error("Unable to upload symmetric keys.");
}
info = info.obj;
if (!(CRYPTO_COLLECTION in info)) {
--- a/services/sync/tests/unit/head_http_server.js
+++ b/services/sync/tests/unit/head_http_server.js
@@ -85,17 +85,17 @@ function ServerWBO(id, initialPayload, m
}
ServerWBO.prototype = {
get data() {
return JSON.parse(this.payload);
},
get() {
- return JSON.stringify(this, ["id", "modified", "payload"]);
+ return { id: this.id, modified: this.modified, payload: this.payload };
},
put(input) {
input = JSON.parse(input);
this.payload = input.payload;
this.modified = new_timestamp();
this.sortindex = input.sortindex || 0;
},
@@ -115,17 +115,17 @@ ServerWBO.prototype = {
return function(request, response) {
var statusCode = 200;
var status = "OK";
var body;
switch (request.method) {
case "GET":
if (self.payload) {
- body = self.get();
+ body = JSON.stringify(self.get());
} else {
statusCode = 404;
status = "Not Found";
body = "Not Found";
}
break;
case "PUT":
--- a/services/sync/tests/unit/test_collection_getBatched.js
+++ b/services/sync/tests/unit/test_collection_getBatched.js
@@ -2,17 +2,17 @@
http://creativecommons.org/publicdomain/zero/1.0/ */
ChromeUtils.import("resource://services-sync/record.js");
ChromeUtils.import("resource://services-sync/service.js");
function recordRange(lim, offset, total) {
let res = [];
for (let i = offset; i < Math.min(lim + offset, total); ++i) {
- res.push(JSON.stringify({ id: String(i), payload: "test:" + i }));
+ res.push({ id: String(i), payload: "test:" + i });
}
return res;
}
function get_test_collection_info({ totalRecords, batchSize, lastModified,
throwAfter = Infinity,
interruptedAfter = Infinity }) {
let coll = new Collection("http://example.com/test/", WBORecord, Service);
--- a/services/sync/tests/unit/test_resource.js
+++ b/services/sync/tests/unit/test_resource.js
@@ -172,17 +172,17 @@ add_task(async function test_proxy_auth_
});
PACSystemSettings.PACURI = server.baseURI + "/pac2";
installFakePAC();
let res = new Resource(server.baseURI + "/open");
let result = await res.get();
Assert.ok(pacFetched);
Assert.ok(fetched);
- Assert.equal("This path exists", result);
+ Assert.equal("This path exists", result.data);
pacFetched = fetched = false;
uninstallFakePAC();
await promiseStopServer(server);
});
add_task(async function test_new_channel() {
_("Ensure a redirect to a new channel is handled properly.");
@@ -251,41 +251,41 @@ add_test(function test_members() {
run_next_test();
});
add_task(async function test_get() {
_("GET a non-password-protected resource");
let res = new Resource(server.baseURI + "/open");
let content = await res.get();
- Assert.equal(content, "This path exists");
+ Assert.equal(content.data, "This path exists");
Assert.equal(content.status, 200);
Assert.ok(content.success);
// Observe logging messages.
let resLogger = res._log;
let dbg = resLogger.debug;
let debugMessages = [];
- resLogger.debug = function(msg) {
- debugMessages.push(msg);
+ resLogger.debug = function(msg, extra) {
+ debugMessages.push(`${msg}: ${JSON.stringify(extra)}`);
dbg.call(this, msg);
};
// Since we didn't receive proper JSON data, accessing content.obj
// will result in a SyntaxError from JSON.parse
let didThrow = false;
try {
content.obj;
} catch (ex) {
didThrow = true;
}
Assert.ok(didThrow);
Assert.equal(debugMessages.length, 1);
Assert.equal(debugMessages[0],
- "Parse fail: Response body starts: \"\"This path exists\"\".");
+ "Parse fail: Response body starts: \"This path exists\"");
resLogger.debug = dbg;
});
add_test(function test_basicauth() {
_("Test that the BasicAuthenticator doesn't screw up header case.");
let res1 = new Resource(server.baseURI + "/foo");
res1.setHeader("Authorization", "Basic foobar");
Assert.equal(res1._headers.authorization, "Basic foobar");
@@ -293,95 +293,95 @@ add_test(function test_basicauth() {
run_next_test();
});
add_task(async function test_get_protected_fail() {
_("GET a password protected resource (test that it'll fail w/o pass, no throw)");
let res2 = new Resource(server.baseURI + "/protected");
let content = await res2.get();
- Assert.equal(content, "This path exists and is protected - failed");
+ Assert.equal(content.data, "This path exists and is protected - failed");
Assert.equal(content.status, 401);
Assert.ok(!content.success);
});
add_task(async function test_get_protected_success() {
_("GET a password protected resource");
let identityConfig = makeIdentityConfig();
let browseridManager = new BrowserIDManager();
configureFxAccountIdentity(browseridManager, identityConfig);
let auth = browseridManager.getResourceAuthenticator();
let res3 = new Resource(server.baseURI + "/protected");
res3.authenticator = auth;
Assert.equal(res3.authenticator, auth);
let content = await res3.get();
- Assert.equal(content, "This path exists and is protected");
+ Assert.equal(content.data, "This path exists and is protected");
Assert.equal(content.status, 200);
Assert.ok(content.success);
});
add_task(async function test_get_404() {
_("GET a non-existent resource (test that it'll fail, but not throw)");
let res4 = new Resource(server.baseURI + "/404");
let content = await res4.get();
- Assert.equal(content, "File not found");
+ Assert.equal(content.data, "File not found");
Assert.equal(content.status, 404);
Assert.ok(!content.success);
// Check some headers of the 404 response
Assert.equal(content.headers.connection, "close");
Assert.equal(content.headers.server, "httpd.js");
Assert.equal(content.headers["content-length"], 14);
});
add_task(async function test_put_string() {
_("PUT to a resource (string)");
let res_upload = new Resource(server.baseURI + "/upload");
let content = await res_upload.put(JSON.stringify(sample_data));
- Assert.equal(content, "Valid data upload via PUT");
+ Assert.equal(content.data, "Valid data upload via PUT");
Assert.equal(content.status, 200);
});
add_task(async function test_put_object() {
_("PUT to a resource (object)");
let res_upload = new Resource(server.baseURI + "/upload");
let content = await res_upload.put(sample_data);
- Assert.equal(content, "Valid data upload via PUT");
+ Assert.equal(content.data, "Valid data upload via PUT");
Assert.equal(content.status, 200);
});
add_task(async function test_post_string() {
_("POST to a resource (string)");
let res_upload = new Resource(server.baseURI + "/upload");
let content = await res_upload.post(JSON.stringify(sample_data));
- Assert.equal(content, "Valid data upload via POST");
+ Assert.equal(content.data, "Valid data upload via POST");
Assert.equal(content.status, 200);
});
add_task(async function test_post_object() {
_("POST to a resource (object)");
let res_upload = new Resource(server.baseURI + "/upload");
let content = await res_upload.post(sample_data);
- Assert.equal(content, "Valid data upload via POST");
+ Assert.equal(content.data, "Valid data upload via POST");
Assert.equal(content.status, 200);
});
add_task(async function test_delete() {
_("DELETE a resource");
let res6 = new Resource(server.baseURI + "/delete");
let content = await res6.delete();
- Assert.equal(content, "This resource has been deleted");
+ Assert.equal(content.data, "This resource has been deleted");
Assert.equal(content.status, 200);
});
add_task(async function test_json_body() {
_("JSON conversion of response body");
let res7 = new Resource(server.baseURI + "/json");
let content = await res7.get();
- Assert.equal(content, JSON.stringify(sample_data));
+ Assert.equal(content.data, JSON.stringify(sample_data));
Assert.equal(content.status, 200);
Assert.equal(JSON.stringify(content.obj), JSON.stringify(sample_data));
});
add_task(async function test_weave_timestamp() {
_("X-Weave-Timestamp header updates Resource.serverTime");
// Before having received any response containing the
// X-Weave-Timestamp header, Resource.serverTime is null.
@@ -390,69 +390,69 @@ add_task(async function test_weave_times
await res8.get();
Assert.equal(Resource.serverTime, TIMESTAMP);
});
add_task(async function test_get_no_headers() {
_("GET: no special request headers");
let res_headers = new Resource(server.baseURI + "/headers");
let content = await res_headers.get();
- Assert.equal(content, "{}");
+ Assert.equal(content.data, "{}");
});
add_task(async function test_put_default_content_type() {
_("PUT: Content-Type defaults to text/plain");
let res_headers = new Resource(server.baseURI + "/headers");
let content = await res_headers.put("data");
- Assert.equal(content, JSON.stringify({"content-type": "text/plain"}));
+ Assert.equal(content.data, JSON.stringify({"content-type": "text/plain"}));
});
add_task(async function test_post_default_content_type() {
_("POST: Content-Type defaults to text/plain");
let res_headers = new Resource(server.baseURI + "/headers");
let content = await res_headers.post("data");
- Assert.equal(content, JSON.stringify({"content-type": "text/plain"}));
+ Assert.equal(content.data, JSON.stringify({"content-type": "text/plain"}));
});
add_task(async function test_setHeader() {
_("setHeader(): setting simple header");
let res_headers = new Resource(server.baseURI + "/headers");
res_headers.setHeader("X-What-Is-Weave", "awesome");
Assert.equal(res_headers.headers["x-what-is-weave"], "awesome");
let content = await res_headers.get();
- Assert.equal(content, JSON.stringify({"x-what-is-weave": "awesome"}));
+ Assert.equal(content.data, JSON.stringify({"x-what-is-weave": "awesome"}));
});
add_task(async function test_setHeader_overwrite() {
_("setHeader(): setting multiple headers, overwriting existing header");
let res_headers = new Resource(server.baseURI + "/headers");
res_headers.setHeader("X-WHAT-is-Weave", "more awesomer");
res_headers.setHeader("X-Another-Header", "hello world");
Assert.equal(res_headers.headers["x-what-is-weave"], "more awesomer");
Assert.equal(res_headers.headers["x-another-header"], "hello world");
let content = await res_headers.get();
- Assert.equal(content, JSON.stringify({"x-another-header": "hello world",
- "x-what-is-weave": "more awesomer"}));
+ Assert.equal(content.data, JSON.stringify({"x-another-header": "hello world",
+ "x-what-is-weave": "more awesomer"}));
});
add_task(async function test_put_override_content_type() {
_("PUT: override default Content-Type");
let res_headers = new Resource(server.baseURI + "/headers");
res_headers.setHeader("Content-Type", "application/foobar");
Assert.equal(res_headers.headers["content-type"], "application/foobar");
let content = await res_headers.put("data");
- Assert.equal(content, JSON.stringify({"content-type": "application/foobar"}));
+ Assert.equal(content.data, JSON.stringify({"content-type": "application/foobar"}));
});
add_task(async function test_post_override_content_type() {
_("POST: override default Content-Type");
let res_headers = new Resource(server.baseURI + "/headers");
res_headers.setHeader("Content-Type", "application/foobar");
let content = await res_headers.post("data");
- Assert.equal(content, JSON.stringify({"content-type": "application/foobar"}));
+ Assert.equal(content.data, JSON.stringify({"content-type": "application/foobar"}));
});
add_task(async function test_weave_backoff() {
_("X-Weave-Backoff header notifies observer");
let backoffInterval;
function onBackoff(subject, data) {
backoffInterval = subject;
}
--- a/services/sync/tests/unit/test_resource_header.js
+++ b/services/sync/tests/unit/test_resource_header.js
@@ -44,16 +44,16 @@ add_task(async function test_headers_cop
triggerRedirect();
_("Issuing request.");
let resource = new Resource(TEST_URL);
resource.setHeader("Authorization", "Basic foobar");
resource.setHeader("X-Foo", "foofoo");
let result = await resource.get(TEST_URL);
- _("Result: " + result);
+ _("Result: " + result.data);
- Assert.equal(result, BODY);
+ Assert.equal(result.data, BODY);
Assert.equal(auth, "Basic foobar");
Assert.equal(foo, "foofoo");
await promiseStopServer(httpServer);
});