corysanin/tubestation
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Bug 1662319 - remove the sync 'repairer' concept. r=lina

Browse Source 
Differential Revision: https://phabricator.services.mozilla.com/D88935
This commit is contained in:
Mark Hammond
2020-10-05 00:26:12 +00:00
parent e4dce0315b
commit 5b05eeb913
16 changed files with 2 additions and 3233 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
modules/libpref/init/all.js
View File

@@ -4561,12 +4561,6 @@ pref("services.common.log.logger.tokenserverclient", "Debug");
pref("services.sync.engine.passwords.validation.enabled", true);
#endif
#if defined(NIGHTLY_BUILD)
// Enable repair of bookmarks on Nightly only - requires validation also be
// enabled.
pref("services.sync.engine.bookmarks.repair.enabled", true);
#endif
// We consider validation this frequently. After considering validation, even
// if we don't end up validating, we won't try again unless this much time has passed.
pref("services.sync.engine.bookmarks.validation.interval", 86400); // 24 hours in seconds

873
services/sync/modules/bookmark_repair.js
View File

@@ -1,873 +0,0 @@
/* 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/. */
"use strict";
var EXPORTED_SYMBOLS = ["BookmarkRepairRequestor", "BookmarkRepairResponder"];
const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
const { Preferences } = ChromeUtils.import(
"resource://gre/modules/Preferences.jsm"
);
const { Log } = ChromeUtils.import("resource://gre/modules/Log.jsm");
const { Svc, Utils } = ChromeUtils.import("resource://services-sync/util.js");
const {
CollectionRepairRequestor,
CollectionRepairResponder,
} = ChromeUtils.import("resource://services-sync/collection_repair.js");
const { DEVICE_TYPE_DESKTOP } = ChromeUtils.import(
"resource://services-sync/constants.js"
);
const { Resource } = ChromeUtils.import("resource://services-sync/resource.js");
const { Doctor } = ChromeUtils.import("resource://services-sync/doctor.js");
const { SyncTelemetry } = ChromeUtils.import(
"resource://services-sync/telemetry.js"
);
const { Async } = ChromeUtils.import("resource://services-common/async.js");
const { CommonUtils } = ChromeUtils.import(
"resource://services-common/utils.js"
);
ChromeUtils.defineModuleGetter(
this,
"PlacesSyncUtils",
"resource://gre/modules/PlacesSyncUtils.jsm"
);
const log = Log.repository.getLogger("Sync.Engine.Bookmarks.Repair");
const PREF_BRANCH = "services.sync.repairs.bookmarks.";
// How long should we wait after sending a repair request before we give up?
const RESPONSE_INTERVAL_TIMEOUT = 60 * 60 * 24 * 3; // 3 days
// The maximum number of IDs we will request to be repaired. Beyond this
// number we assume that trying to repair may do more harm than good and may
// ask another client to wipe the server and reupload everything. Bug 1341972
// is tracking that work.
const MAX_REQUESTED_IDS = 1000;
class AbortRepairError extends Error {
constructor(reason) {
super();
this.reason = reason;
}
}
// The states we can be in.
const STATE = Object.freeze({
NOT_REPAIRING: "",
// We need to try to find another client to use.
NEED_NEW_CLIENT: "repair.need-new-client",
// We've sent the first request to a client.
SENT_REQUEST: "repair.sent",
// We've retried a request to a client.
SENT_SECOND_REQUEST: "repair.sent-again",
// There were no problems, but we've gone as far as we can.
FINISHED: "repair.finished",
// We've found an error that forces us to abort this entire repair cycle.
ABORTED: "repair.aborted",
});
// The preferences we use to hold our state.
const PREF = Object.freeze({
// If a repair is in progress, this is the generated GUID for the "flow ID".
REPAIR_ID: "flowID",
// The IDs we are currently trying to obtain via the repair process, space sep'd.
REPAIR_MISSING_IDS: "ids",
// The ID of the client we're currently trying to get the missing items from.
REPAIR_CURRENT_CLIENT: "currentClient",
// The IDs of the clients we've previously tried to get the missing items
// from, space sep'd.
REPAIR_PREVIOUS_CLIENTS: "previousClients",
// The time, in seconds, when we initiated the most recent client request.
REPAIR_WHEN: "when",
// Our current state.
CURRENT_STATE: "state",
});
class BookmarkRepairRequestor extends CollectionRepairRequestor {
constructor(service = null) {
super(service);
this.prefs = new Preferences(PREF_BRANCH);
}
/* Check if any other clients connected to our account are current performing
a repair. A thin wrapper which exists mainly for mocking during tests.
*/
anyClientsRepairing(flowID) {
return Doctor.anyClientsRepairing(this.service, "bookmarks", flowID);
}
/* Return a set of IDs we should request.
*/
getProblemIDs(validationInfo) {
// Set of ids of "known bad records". Many of the validation issues will
// report duplicates -- if the server is missing a record, it is unlikely
// to cause only a single problem.
let ids = new Set();
// Note that we allow any of the validation problem fields to be missing so
// that tests have a slightly easier time, hence the `|| []` in each loop.
// Missing children records when the parent exists but a child doesn't.
for (let { parent, child } of validationInfo.problems.missingChildren ||
[]) {
// We can't be sure if the child is missing or our copy of the parent is
// wrong, so request both
ids.add(parent);
ids.add(child);
}
if (ids.size > MAX_REQUESTED_IDS) {
return ids; // might as well give up here - we aren't going to repair.
}
// Orphans are when the child exists but the parent doesn't.
// This could either be a problem in the child (it's wrong about the node
// that should be its parent), or the parent could simply be missing.
for (let { parent, id } of validationInfo.problems.orphans || []) {
// Request both, to handle both cases
ids.add(id);
ids.add(parent);
}
if (ids.size > MAX_REQUESTED_IDS) {
return ids; // might as well give up here - we aren't going to repair.
}
// Entries where we have the parent but we have a record from the server that
// claims the child was deleted.
for (let { parent, child } of validationInfo.problems.deletedChildren ||
[]) {
// Request both, since we don't know if it's a botched deletion or revival
ids.add(parent);
ids.add(child);
}
if (ids.size > MAX_REQUESTED_IDS) {
return ids; // might as well give up here - we aren't going to repair.
}
// Entries where the child references a parent that we don't have, but we
// have a record from the server that claims the parent was deleted.
for (let { parent, child } of validationInfo.problems.deletedParents ||
[]) {
// Request both, since we don't know if it's a botched deletion or revival
ids.add(parent);
ids.add(child);
}
if (ids.size > MAX_REQUESTED_IDS) {
return ids; // might as well give up here - we aren't going to repair.
}
// Cases where the parent and child disagree about who the parent is.
for (let { parent, child } of validationInfo.problems
.parentChildMismatches || []) {
// Request both, since we don't know which is right.
ids.add(parent);
ids.add(child);
}
if (ids.size > MAX_REQUESTED_IDS) {
return ids; // might as well give up here - we aren't going to repair.
}
// Cases where multiple parents reference a child. We re-request both the
// child, and all the parents that think they own it. This may be more than
// we need, but I don't think we can safely make the assumption that the
// child is right.
for (let { parents, child } of validationInfo.problems.multipleParents ||
[]) {
for (let parent of parents) {
ids.add(parent);
}
ids.add(child);
}
return ids;
}
_countServerOnlyFixableProblems(validationInfo) {
const fixableProblems = ["clientMissing", "serverMissing", "serverDeleted"];
return fixableProblems.reduce((numProblems, problemLabel) => {
return numProblems + validationInfo.problems[problemLabel].length;
}, 0);
}
tryServerOnlyRepairs(validationInfo) {
if (this._countServerOnlyFixableProblems(validationInfo) == 0) {
return false;
}
let engine = this.service.engineManager.get("bookmarks");
for (let id of validationInfo.problems.serverMissing) {
engine.addForWeakUpload(id);
}
engine.toFetch = Utils.setAddAll(
Utils.setAddAll(engine.toFetch, validationInfo.problems.clientMissing),
validationInfo.problems.serverDeleted
);
return true;
}
/* See if the repairer is willing and able to begin a repair process given
the specified validation information.
Returns true if a repair was started and false otherwise.
*/
async startRepairs(validationInfo, flowID) {
if (this._currentState != STATE.NOT_REPAIRING) {
log.info(
`Can't start a repair - repair with ID ${this._flowID} is already in progress`
);
return false;
}
let ids = this.getProblemIDs(validationInfo);
if (ids.size > MAX_REQUESTED_IDS) {
log.info(
"Not starting a repair as there are over " +
MAX_REQUESTED_IDS +
" problems"
);
let extra = { flowID, reason: `too many problems: ${ids.size}` };
this.service.recordTelemetryEvent("repair", "aborted", undefined, extra);
return false;
}
if (ids.size == 0) {
log.info("Not starting a repair as there are no problems");
return false;
}
if (this.anyClientsRepairing()) {
log.info(
"Can't start repair, since other clients are already repairing bookmarks"
);
let extra = { flowID, reason: "other clients repairing" };
this.service.recordTelemetryEvent("repair", "aborted", undefined, extra);
return false;
}
log.info(`Starting a repair, looking for ${ids.size} missing item(s)`);
// setup our prefs to indicate we are on our way.
this._flowID = flowID;
this._currentIDs = Array.from(ids);
this._currentState = STATE.NEED_NEW_CLIENT;
this.service.recordTelemetryEvent("repair", "started", undefined, {
flowID,
numIDs: ids.size.toString(),
});
return this.continueRepairs();
}
/* Work out what state our current repair request is in, and whether it can
proceed to a new state.
Returns true if we could continue the repair - even if the state didn't
actually move. Returns false if we aren't actually repairing.
*/
async continueRepairs(response = null) {
// Note that "ABORTED" and "FINISHED" should never be current when this
// function returns - this function resets to NOT_REPAIRING in those cases.
if (this._currentState == STATE.NOT_REPAIRING) {
return false;
}
let state, newState;
let abortReason;
// we loop until the state doesn't change - but enforce a max of 10 times
// to prevent errors causing infinite loops.
for (let i = 0; i < 10; i++) {
state = this._currentState;
log.info("continueRepairs starting with state", state);
try {
newState = await this._continueRepairs(state, response);
log.info("continueRepairs has next state", newState);
} catch (ex) {
if (!(ex instanceof AbortRepairError)) {
throw ex;
}
log.info(`Repair has been aborted: ${ex.reason}`);
newState = STATE.ABORTED;
abortReason = ex.reason;
}
if (newState == STATE.ABORTED) {
break;
}
this._currentState = newState;
Services.prefs.savePrefFile(null); // flush prefs.
if (state == newState) {
break;
}
}
if (state != newState) {
log.error("continueRepairs spun without getting a new state");
}
if (newState == STATE.FINISHED || newState == STATE.ABORTED) {
let object = newState == STATE.FINISHED ? "finished" : "aborted";
let extra = {
flowID: this._flowID,
numIDs: this._currentIDs.length.toString(),
};
if (abortReason) {
extra.reason = abortReason;
}
this.service.recordTelemetryEvent("repair", object, undefined, extra);
// reset our state and flush our prefs.
this.prefs.resetBranch();
Services.prefs.savePrefFile(null); // flush prefs.
}
return true;
}
async _continueRepairs(state, response = null) {
if (this.anyClientsRepairing(this._flowID)) {
throw new AbortRepairError("other clients repairing");
}
switch (state) {
case STATE.SENT_REQUEST:
case STATE.SENT_SECOND_REQUEST:
let flowID = this._flowID;
let clientID = this._currentRemoteClient;
if (!clientID) {
throw new AbortRepairError(
`In state ${state} but have no client IDs listed`
);
}
if (response) {
// We got an explicit response - let's see how we went.
state = this._handleResponse(state, response);
break;
}
// So we've sent a request - and don't yet have a response. See if the
// client we sent it to has removed it from its list (ie, whether it
// has synced since we wrote the request.)
let client = this.service.clientsEngine.remoteClient(clientID);
if (!client) {
// hrmph - the client has disappeared.
log.info(
`previously requested client "${clientID}" has vanished - moving to next step`
);
state = STATE.NEED_NEW_CLIENT;
let extra = {
deviceID: this.service.identity.hashedDeviceID(clientID),
flowID,
};
this.service.recordTelemetryEvent(
"repair",
"abandon",
"missing",
extra
);
break;
}
if (await this._isCommandPending(clientID, flowID)) {
// So the command we previously sent is still queued for the client
// (ie, that client is yet to have synced). Let's see if we should
// give up on that client.
let lastRequestSent = this.prefs.get(PREF.REPAIR_WHEN);
let timeLeft =
lastRequestSent + RESPONSE_INTERVAL_TIMEOUT - this._now();
if (timeLeft <= 0) {
log.info(
`previous request to client ${clientID} is pending, but has taken too long`
);
state = STATE.NEED_NEW_CLIENT;
// XXX - should we remove the command?
let extra = {
deviceID: this.service.identity.hashedDeviceID(clientID),
flowID,
};
this.service.recordTelemetryEvent(
"repair",
"abandon",
"silent",
extra
);
break;
}
// Let's continue to wait for that client to respond.
log.trace(
`previous request to client ${clientID} has ${timeLeft} seconds before we give up on it`
);
break;
}
// The command isn't pending - if this was the first request, we give
// it another go (as that client may have cleared the command but is yet
// to complete the sync)
// XXX - note that this is no longer true - the responders don't remove
// their command until they have written a response. This might mean
// we could drop the entire STATE.SENT_SECOND_REQUEST concept???
if (state == STATE.SENT_REQUEST) {
log.info(
`previous request to client ${clientID} was removed - trying a second time`
);
state = STATE.SENT_SECOND_REQUEST;
await this._writeRequest(clientID);
} else {
// this was the second time around, so give up on this client
log.info(
`previous 2 requests to client ${clientID} were removed - need a new client`
);
state = STATE.NEED_NEW_CLIENT;
}
break;
case STATE.NEED_NEW_CLIENT:
// We need to find a new client to request.
let newClientID = this._findNextClient();
if (!newClientID) {
state = STATE.FINISHED;
break;
}
this._addToPreviousRemoteClients(this._currentRemoteClient);
this._currentRemoteClient = newClientID;
await this._writeRequest(newClientID);
state = STATE.SENT_REQUEST;
break;
case STATE.ABORTED:
break; // our caller will take the abort action.
case STATE.FINISHED:
break;
case STATE.NOT_REPAIRING:
// No repair is in progress. This is a common case, so only log trace.
log.trace("continue repairs called but no repair in progress.");
break;
default:
log.error(`continue repairs finds itself in an unknown state ${state}`);
state = STATE.ABORTED;
break;
}
return state;
}
/* Handle being in the SENT_REQUEST or SENT_SECOND_REQUEST state with an
explicit response.
*/
_handleResponse(state, response) {
let clientID = this._currentRemoteClient;
let flowID = this._flowID;
if (
response.flowID != flowID ||
response.clientID != clientID ||
response.request != "upload"
) {
log.info("got a response to a different repair request", response);
// hopefully just a stale request that finally came in (either from
// an entirely different repair flow, or from a client we've since
// given up on.) It doesn't mean we need to abort though...
return state;
}
// Pull apart the response and see if it provided everything we asked for.
let remainingIDs = Array.from(
CommonUtils.difference(this._currentIDs, response.ids)
);
log.info(
`repair response from ${clientID} provided "${response.ids}", remaining now "${remainingIDs}"`
);
this._currentIDs = remainingIDs;
if (remainingIDs.length) {
// try a new client for the remaining ones.
state = STATE.NEED_NEW_CLIENT;
} else {
state = STATE.FINISHED;
}
// record telemetry about this
let extra = {
deviceID: this.service.identity.hashedDeviceID(clientID),
flowID,
numIDs: response.ids.length.toString(),
};
this.service.recordTelemetryEvent("repair", "response", "upload", extra);
return state;
}
/* Issue a repair request to a specific client.
*/
async _writeRequest(clientID) {
log.trace("writing repair request to client", clientID);
let ids = this._currentIDs;
if (!ids) {
throw new AbortRepairError(
"Attempting to write a request, but there are no IDs"
);
}
let flowID = this._flowID;
// Post a command to that client.
let request = {
collection: "bookmarks",
request: "upload",
requestor: this.service.clientsEngine.localID,
ids,
flowID,
};
await this.service.clientsEngine.sendCommand(
"repairRequest",
[request],
clientID,
{ flowID }
);
this.prefs.set(PREF.REPAIR_WHEN, Math.floor(this._now()));
// record telemetry about this
let extra = {
deviceID: this.service.identity.hashedDeviceID(clientID),
flowID,
numIDs: ids.length.toString(),
};
this.service.recordTelemetryEvent("repair", "request", "upload", extra);
}
_findNextClient() {
let alreadyDone = this._getPreviousRemoteClients();
alreadyDone.push(this._currentRemoteClient);
let remoteClients = this.service.clientsEngine.remoteClients;
// we want to consider the most-recently synced clients first.
remoteClients.sort((a, b) => b.serverLastModified - a.serverLastModified);
for (let client of remoteClients) {
log.trace("findNextClient considering", client);
if (!alreadyDone.includes(client.id) && this._isSuitableClient(client)) {
return client.id;
}
}
log.trace("findNextClient found no client");
return null;
}
/* Is the passed client record suitable as a repair responder?
*/
_isSuitableClient(client) {
// filter only desktop firefox running > 53 (ie, any 54)
return (
client.type == DEVICE_TYPE_DESKTOP &&
Services.vc.compare(client.version, 53) > 0
);
}
/* Is our command still in the "commands" queue for the specific client?
*/
async _isCommandPending(clientID, flowID) {
// getClientCommands() is poorly named - it's only outgoing commands
// from us we have yet to write. For our purposes, we want to check
// them and commands previously written (which is in .commands)
let clientCommands = await this.service.clientsEngine.getClientCommands(
clientID
);
let commands = [
...clientCommands,
...(this.service.clientsEngine.remoteClient(clientID).commands || []),
];
for (let command of commands) {
if (command.command != "repairRequest" || command.args.length != 1) {
continue;
}
let arg = command.args[0];
if (
arg.collection == "bookmarks" &&
arg.request == "upload" &&
arg.flowID == flowID
) {
return true;
}
}
return false;
}
// accessors for our prefs.
get _currentState() {
return this.prefs.get(PREF.CURRENT_STATE, STATE.NOT_REPAIRING);
}
set _currentState(newState) {
this.prefs.set(PREF.CURRENT_STATE, newState);
}
get _currentIDs() {
let ids = this.prefs.get(PREF.REPAIR_MISSING_IDS, "");
return ids.length ? ids.split(" ") : [];
}
set _currentIDs(arrayOfIDs) {
this.prefs.set(PREF.REPAIR_MISSING_IDS, arrayOfIDs.join(" "));
}
get _currentRemoteClient() {
return this.prefs.get(PREF.REPAIR_CURRENT_CLIENT);
}
set _currentRemoteClient(clientID) {
this.prefs.set(PREF.REPAIR_CURRENT_CLIENT, clientID);
}
get _flowID() {
return this.prefs.get(PREF.REPAIR_ID);
}
set _flowID(val) {
this.prefs.set(PREF.REPAIR_ID, val);
}
// use a function for this pref to offer somewhat sane semantics.
_getPreviousRemoteClients() {
let alreadyDone = this.prefs.get(PREF.REPAIR_PREVIOUS_CLIENTS, "");
return alreadyDone.length ? alreadyDone.split(" ") : [];
}
_addToPreviousRemoteClients(clientID) {
let arrayOfClientIDs = this._getPreviousRemoteClients();
arrayOfClientIDs.push(clientID);
this.prefs.set(PREF.REPAIR_PREVIOUS_CLIENTS, arrayOfClientIDs.join(" "));
}
/* Used for test mocks.
*/
_now() {
// We use the server time, which is SECONDS
return Resource.serverTime;
}
}
/* An object that responds to repair requests initiated by some other device.
*/
class BookmarkRepairResponder extends CollectionRepairResponder {
async repair(request, rawCommand) {
if (request.request != "upload") {
await this._abortRepair(
request,
rawCommand,
`Don't understand request type '${request.request}'`
);
return;
}
// Note that we don't try and guard against multiple repairs being in
// progress as we don't do anything too smart that could cause problems,
// but just upload items. If we get any smarter we should re-think this
// (but when we do, note that checking this._currentState isn't enough as
// this responder is not a singleton)
this._currentState = {
request,
rawCommand,
processedCommand: false,
ids: [],
};
try {
let engine = this.service.engineManager.get("bookmarks");
let { toUpload, toDelete } = await this._fetchItemsToUpload(request);
if (toUpload.size || toDelete.size) {
log.debug(
`repair request will upload ${toUpload.size} items and delete ${toDelete.size} items`
);
// whew - now add these items to the tracker "weakly" (ie, they will not
// persist in the case of a restart, but that's OK - we'll then end up here
// again) and also record them in the response we send back.
for (let id of toUpload) {
engine.addForWeakUpload(id);
this._currentState.ids.push(id);
}
for (let id of toDelete) {
engine.addForWeakUpload(id, { forceTombstone: true });
this._currentState.ids.push(id);
}
// We have arranged for stuff to be uploaded, so wait until that's done.
Svc.Obs.add("weave:engine:sync:uploaded", this.onUploaded, this);
// and record in telemetry that we got this far - just incase we never
// end up doing the upload for some obscure reason.
let eventExtra = {
flowID: request.flowID,
numIDs: this._currentState.ids.length.toString(),
};
this.service.recordTelemetryEvent(
"repairResponse",
"uploading",
undefined,
eventExtra
);
} else {
// We were unable to help with the repair, so report that we are done.
await this._finishRepair();
}
} catch (ex) {
if (Async.isShutdownException(ex)) {
// this repair request will be tried next time.
throw ex;
}
// On failure, we still write a response so the requestor knows to move
// on, but we record the failure reason in telemetry.
log.error("Failed to respond to the repair request", ex);
this._currentState.failureReason = SyncTelemetry.transformError(ex);
await this._finishRepair();
}
}
async _fetchItemsToUpload(request) {
let toUpload = new Set(); // items we will upload.
let toDelete = new Set(); // items we will delete.
let requested = new Set(request.ids);
let engine = this.service.engineManager.get("bookmarks");
// Determine every item that may be impacted by the requested IDs - eg,
// this may include children if a requested ID is a folder.
// Turn an array of { recordId, syncable } into a map of recordId -> syncable.
let repairable = await PlacesSyncUtils.bookmarks.fetchRecordIdsForRepair(
request.ids
);
if (repairable.length == 0) {
// server will get upset if we request an empty set, and we can't do
// anything in that case, so bail now.
return { toUpload, toDelete };
}
// which of these items exist on the server?
let itemSource = engine.itemSource();
itemSource.ids = repairable.map(item => item.recordId);
log.trace(`checking the server for items`, itemSource.ids);
let itemsResponse = await itemSource.get();
// If the response failed, don't bother trying to parse the output.
// 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(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) {
if (requested.has(id)) {
if (syncable) {
log.debug(
`repair request to upload item '${id}' which exists locally; uploading`
);
toUpload.add(id);
} else {
// explicitly requested and not syncable, so tombstone.
log.debug(
`repair request to upload item '${id}' but it isn't under a syncable root; writing a tombstone`
);
toDelete.add(id);
}
// The item wasn't explicitly requested - only upload if it is syncable
// and doesn't exist on the server.
} else if (syncable && !existRemotely.has(id)) {
log.debug(
`repair request found related item '${id}' which isn't on the server; uploading`
);
toUpload.add(id);
} else if (!syncable && existRemotely.has(id)) {
log.debug(
`repair request found non-syncable related item '${id}' on the server; writing a tombstone`
);
toDelete.add(id);
} else {
log.debug(
`repair request found related item '${id}' which we will not upload; ignoring`
);
}
}
return { toUpload, toDelete };
}
onUploaded(subject, data) {
if (data != "bookmarks") {
return;
}
Svc.Obs.remove("weave:engine:sync:uploaded", this.onUploaded, this);
if (subject.failed) {
return;
}
log.debug(
`bookmarks engine has uploaded stuff - creating a repair response`,
subject
);
this.service.clientsEngine._tracker.asyncObserver.enqueueCall(() =>
this._finishRepair()
);
}
async _finishRepair() {
let clientsEngine = this.service.clientsEngine;
let flowID = this._currentState.request.flowID;
let response = {
request: this._currentState.request.request,
collection: "bookmarks",
clientID: clientsEngine.localID,
flowID,
ids: this._currentState.ids,
};
let clientID = this._currentState.request.requestor;
await clientsEngine.sendCommand("repairResponse", [response], clientID, {
flowID,
});
// and nuke the request from our client.
await clientsEngine.removeLocalCommand(this._currentState.rawCommand);
let eventExtra = {
flowID,
numIDs: response.ids.length.toString(),
};
if (this._currentState.failureReason) {
// *sob* - recording this in "extra" means the value must be a string of
// max 85 chars.
eventExtra.failureReason = JSON.stringify(
this._currentState.failureReason
).substring(0, 85);
this.service.recordTelemetryEvent(
"repairResponse",
"failed",
undefined,
eventExtra
);
} else {
this.service.recordTelemetryEvent(
"repairResponse",
"finished",
undefined,
eventExtra
);
}
this._currentState = null;
}
async _abortRepair(request, rawCommand, why) {
log.warn(`aborting repair request: ${why}`);
await this.service.clientsEngine.removeLocalCommand(rawCommand);
// record telemetry for this.
let eventExtra = {
flowID: request.flowID,
reason: why,
};
this.service.recordTelemetryEvent(
"repairResponse",
"aborted",
undefined,
eventExtra
);
// We could also consider writing a response here so the requestor can take
// some immediate action rather than timing out, but we abort only in cases
// that should be rare, so let's wait and see what telemetry tells us.
}
}
/* Exposed in case another module needs to understand our state.
*/
BookmarkRepairRequestor.STATE = STATE;
BookmarkRepairRequestor.PREF = PREF;

2
services/sync/modules/bookmark_validator.js
View File

@@ -1029,8 +1029,6 @@ class BookmarkValidator {
}
async _getServerState(engine) {
// XXXXX - todo - we need to capture last-modified of the server here and
// ensure the repairer only applys with if-unmodified-since that date.
let collection = engine.itemSource();
let collectionKey = engine.service.collectionKeys.keyForCollection(
engine.name

142
services/sync/modules/collection_repair.js
View File

@@ -1,142 +0,0 @@
/* 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/. */
"use strict";
const { Weave } = ChromeUtils.import("resource://services-sync/main.js");
ChromeUtils.defineModuleGetter(
this,
"BookmarkRepairRequestor",
"resource://services-sync/bookmark_repair.js"
);
var EXPORTED_SYMBOLS = [
"getRepairRequestor",
"getAllRepairRequestors",
"CollectionRepairRequestor",
"getRepairResponder",
"CollectionRepairResponder",
];
// The individual requestors/responders, lazily loaded.
const REQUESTORS = {
bookmarks: ["bookmark_repair.js", "BookmarkRepairRequestor"],
};
const RESPONDERS = {
bookmarks: ["bookmark_repair.js", "BookmarkRepairResponder"],
};
// Should we maybe enforce the requestors being a singleton?
function _getRepairConstructor(which, collection) {
if (!(collection in which)) {
return null;
}
let [modname, symbolname] = which[collection];
let ns = {};
ChromeUtils.import("resource://services-sync/" + modname, ns);
return ns[symbolname];
}
function getRepairRequestor(collection) {
let ctor = _getRepairConstructor(REQUESTORS, collection);
if (!ctor) {
return null;
}
return new ctor();
}
function getAllRepairRequestors() {
let result = {};
for (let collection of Object.keys(REQUESTORS)) {
let ctor = _getRepairConstructor(REQUESTORS, collection);
result[collection] = new ctor();
}
return result;
}
function getRepairResponder(collection) {
let ctor = _getRepairConstructor(RESPONDERS, collection);
if (!ctor) {
return null;
}
return new ctor();
}
// The abstract classes.
class CollectionRepairRequestor {
constructor(service = null) {
// allow service to be mocked in tests.
this.service = service || Weave.Service;
}
/* Try to resolve some issues with the server without involving other clients.
Returns true if we repaired some items.
@param validationInfo {Object}
The validation info as returned by the collection's validator.
*/
tryServerOnlyRepairs(validationInfo) {
return false;
}
/* See if the repairer is willing and able to begin a repair process given
the specified validation information.
Returns true if a repair was started and false otherwise.
@param validationInfo {Object}
The validation info as returned by the collection's validator.
@param flowID {String}
A guid that uniquely identifies this repair process for this
collection, and which should be sent to any requestors and
reported in telemetry.
*/
async startRepairs(validationInfo, flowID) {
throw new Error("not implemented");
}
/* Work out what state our current repair request is in, and whether it can
proceed to a new state.
Returns true if we could continue the repair - even if the state didn't
actually move. Returns false if we aren't actually repairing.
@param responseInfo {Object}
An optional response to a previous repair request, as returned
by a remote repair responder.
*/
async continueRepairs(responseInfo = null) {
throw new Error("not implemented");
}
}
class CollectionRepairResponder {
constructor(service = null) {
// allow service to be mocked in tests.
this.service = service || Weave.Service;
}
/* Take some action in response to a repair request. Returns a promise that
resolves once the repair process has started, or rejects if there
was an error starting the repair.
Note that when the promise resolves the repair is not yet complete - at
some point in the future the repair will auto-complete, at which time
|rawCommand| will be removed from the list of client commands for this
client.
@param request {Object}
The repair request as sent by another client.
@param rawCommand {Object}
The command object as stored in the clients engine, and which
will be automatically removed once a repair completes.
*/
async repair(request, rawCommand) {
throw new Error("not implemented");
}
}

116
services/sync/modules/doctor.js
View File

@@ -21,51 +21,10 @@ const { Service } = ChromeUtils.import("resource://services-sync/service.js");
const { Resource } = ChromeUtils.import("resource://services-sync/resource.js");
const { Svc, Utils } = ChromeUtils.import("resource://services-sync/util.js");
ChromeUtils.defineModuleGetter(
this,
"getRepairRequestor",
"resource://services-sync/collection_repair.js"
);
ChromeUtils.defineModuleGetter(
this,
"getAllRepairRequestors",
"resource://services-sync/collection_repair.js"
);
const log = Log.repository.getLogger("Sync.Doctor");
var REPAIR_ADVANCE_PERIOD = 86400; // 1 day
var Doctor = {
anyClientsRepairing(service, collection, ignoreFlowID = null) {
if (!service || !service.clientsEngine) {
log.info("Missing clients engine, assuming we're in test code");
return false;
}
return service.clientsEngine.remoteClients.some(
client =>
client.commands &&
client.commands.some(command => {
if (
command.command != "repairResponse" &&
command.command != "repairRequest"
) {
return false;
}
if (!command.args || command.args.length != 1) {
return false;
}
if (command.args[0].collection != collection) {
return false;
}
if (ignoreFlowID != null && command.args[0].flowID == ignoreFlowID) {
return false;
}
return true;
})
);
},
async consult(recentlySyncedEngines) {
if (!Services.telemetry.canRecordBase) {
log.info("Skipping consultation: telemetry reporting is disabled");
@@ -75,32 +34,6 @@ var Doctor = {
let engineInfos = this._getEnginesToValidate(recentlySyncedEngines);
await this._runValidators(engineInfos);
// We are called at the end of a sync, which is a good time to periodically
// check each repairer to see if it can advance.
if (this._now() - this.lastRepairAdvance > REPAIR_ADVANCE_PERIOD) {
try {
for (let [collection, requestor] of Object.entries(
this._getAllRepairRequestors()
)) {
try {
let advanced = await requestor.continueRepairs();
log.info(
`${collection} reparier ${
advanced ? "advanced" : "did not advance"
}.`
);
} catch (ex) {
if (Async.isShutdownException(ex)) {
throw ex;
}
log.error(`${collection} repairer failed`, ex);
}
}
} finally {
this.lastRepairAdvance = Math.floor(this._now());
}
}
},
_getEnginesToValidate(recentlySyncedEngines) {
@@ -207,12 +140,7 @@ var Doctor = {
`If you encounter any problems because of this, please file a bug.`
);
// Make a new flowID just incase we end up starting a repair.
let flowID = Utils.makeGUID();
try {
// XXX - must get the flow ID to either the validator, or directly to
// telemetry. I guess it's probably OK to always record a flowID even
// if we don't end up repairing?
log.info(`Running validator for ${engine.name}`);
let result = await validator.validate(engine);
let { problems, version, duration, recordCount } = result;
@@ -226,8 +154,6 @@ var Doctor = {
},
engine.name
);
// And see if we should repair.
await this._maybeCure(engine, result, flowID);
} catch (ex) {
if (Async.isShutdownException(ex)) {
throw ex;
@@ -240,32 +166,6 @@ var Doctor = {
}
},
async _maybeCure(engine, validationResults, flowID) {
if (!this._shouldRepair(engine)) {
log.info(`Skipping repair of ${engine.name} - disabled via preferences`);
return;
}
let requestor = this._getRepairRequestor(engine.name);
let didStart = false;
if (requestor) {
if (requestor.tryServerOnlyRepairs(validationResults)) {
return; // TODO: It would be nice if we could request a validation to be
// done on next sync.
}
didStart = await requestor.startRepairs(validationResults, flowID);
}
log.info(
`${didStart ? "did" : "didn't"} start a repair of ${
engine.name
} with flowID ${flowID}`
);
},
_shouldRepair(engine) {
return Svc.Prefs.get(`engine.${engine.name}.repair.enabled`, false);
},
// mainly for mocking.
async _fetchCollectionCounts() {
let collectionCountsURL = Service.userBaseURL + "info/collection_counts";
@@ -289,25 +189,9 @@ var Doctor = {
}
},
get lastRepairAdvance() {
return Svc.Prefs.get("doctor.lastRepairAdvance", 0);
},
set lastRepairAdvance(value) {
Svc.Prefs.set("doctor.lastRepairAdvance", value);
},
// functions used so tests can mock them
_now() {
// We use the server time, which is SECONDS
return Resource.serverTime;
},
_getRepairRequestor(name) {
return getRepairRequestor(name);
},
_getAllRepairRequestors() {
return getAllRepairRequestors();
},
};

3
services/sync/modules/engines.js
View File

@@ -823,9 +823,6 @@ function SyncEngine(name, service) {
// in the case of a conflict from the server, there's a window where our
// record would be marked as modified more recently than a change that occurs
// on another device change, and we lose data from the user.
//
// Additionally, we use this as the set of items to upload for bookmark
// repair reponse, which has similar constraints.
this._needWeakUpload = new Map();
this.asyncObserver = Async.asyncObserver(this, this._log);

71
services/sync/modules/engines/clients.js
View File

@@ -50,18 +50,6 @@ const { PREF_ACCOUNT_ROOT } = ChromeUtils.import(
"resource://gre/modules/FxAccountsCommon.js"
);
ChromeUtils.defineModuleGetter(
this,
"getRepairRequestor",
"resource://services-sync/collection_repair.js"
);
ChromeUtils.defineModuleGetter(
this,
"getRepairResponder",
"resource://services-sync/collection_repair.js"
);
const CLIENTS_TTL = 1814400; // 21 days
const CLIENTS_TTL_REFRESH = 604800; // 7 days
const STALE_CLIENT_REMOTE_AGE = 604800; // 7 days
@@ -770,16 +758,6 @@ ClientEngine.prototype = {
importance: 1,
desc: "Instruct a client to display a URI",
},
repairRequest: {
args: 1,
importance: 2,
desc: "Instruct a client to initiate a repair",
},
repairResponse: {
args: 1,
importance: 2,
desc: "Instruct a client a repair request is complete",
},
},
/**
@@ -887,55 +865,6 @@ ClientEngine.prototype = {
let [uri, clientId, title] = args;
URIsToDisplay.push({ uri, clientId, title });
break;
case "repairResponse": {
// When we send a repair request to another device that understands
// it, that device will send a response indicating what it did.
let response = args[0];
let requestor = getRepairRequestor(response.collection);
if (!requestor) {
this._log.warn("repairResponse for unknown collection", response);
break;
}
if (!(await requestor.continueRepairs(response))) {
this._log.warn(
"repairResponse couldn't continue the repair",
response
);
}
break;
}
case "repairRequest": {
// Another device has sent us a request to make some repair.
let request = args[0];
let responder = getRepairResponder(request.collection);
if (!responder) {
this._log.warn("repairRequest for unknown collection", request);
break;
}
try {
if (await responder.repair(request, rawCommand)) {
// We've started a repair - once that collection has synced it
// will write a "response" command and arrange for this repair
// request to be removed from the local command list - if we
// removed it now we might fail to write a response in cases of
// premature shutdown etc.
shouldRemoveCommand = false;
}
} catch (ex) {
if (Async.isShutdownException(ex)) {
// Let's assume this error was caused by the shutdown, so let
// it try again next time.
throw ex;
}
// otherwise there are no second chances - the command is removed
// and will not be tried again.
// (Note that this shouldn't be hit in the normal case - it's
// expected the responder will handle all reasonable failures and
// write a response indicating that it couldn't do what was asked.)
this._log.error("Failed to handle a repair request", ex);
}
break;
}
default:
this._log.warn("Received an unknown command: " + command);
break;

2
services/sync/modules/engines/extension-storage.js
View File

@@ -85,8 +85,6 @@ function ExtensionStorageEngineBridge(service) {
ExtensionStorageEngineBridge.prototype = {
__proto__: BridgedEngine.prototype,
syncPriority: 10,
// we don't support repair at all!
_skipPercentageChance: 100,
// Used to override the engine name in telemetry, so that we can distinguish .
overrideTelemetryName: "rust-webext-storage",

2
services/sync/moz.build
View File

@@ -18,11 +18,9 @@ EXTRA_COMPONENTS += [
EXTRA_JS_MODULES['services-sync'] += [
'modules/addonsreconciler.js',
'modules/addonutils.js',
'modules/bookmark_repair.js',
'modules/bookmark_validator.js',
'modules/bridged_engine.js',
'modules/browserid_identity.js',
'modules/collection_repair.js',
'modules/collection_validator.js',
'modules/constants.js',
'modules/doctor.js',

4
services/sync/tests/unit/test_bookmark_engine.js
View File

@@ -29,8 +29,8 @@ const LegacyEngine = BookmarksEngine;
function checkRecordedEvents(object, expected, message) {
// Ignore event telemetry from the merger.
let repairEvents = recordedEvents.filter(event => event.object == object);
deepEqual(repairEvents, expected, message);
let checkEvents = recordedEvents.filter(event => event.object == object);
deepEqual(checkEvents, expected, message);
// and clear the list so future checks are easier to write.
recordedEvents = [];
}

617
services/sync/tests/unit/test_bookmark_repair.js
View File

@@ -1,617 +0,0 @@
/* Any copyright is dedicated to the Public Domain.
* http://creativecommons.org/publicdomain/zero/1.0/ */
// Tests the bookmark repair requestor and responder end-to-end (ie, without
// many mocks)
const { BookmarkRepairRequestor } = ChromeUtils.import(
"resource://services-sync/bookmark_repair.js"
);
const { Service } = ChromeUtils.import("resource://services-sync/service.js");
const { ClientEngine } = ChromeUtils.import(
"resource://services-sync/engines/clients.js"
);
const { BookmarksEngine } = ChromeUtils.import(
"resource://services-sync/engines/bookmarks.js"
);
const BOOKMARK_REPAIR_STATE_PREFS = [
"client.GUID",
"doctor.lastRepairAdvance",
...Object.values(BookmarkRepairRequestor.PREF).map(
name => `repairs.bookmarks.${name}`
),
];
let clientsEngine;
let bookmarksEngine;
var recordedEvents = [];
add_task(async function setup() {
await Service.engineManager.unregister("bookmarks");
await Service.engineManager.register(BookmarksEngine);
clientsEngine = Service.clientsEngine;
clientsEngine.ignoreLastModifiedOnProcessCommands = true;
bookmarksEngine = Service.engineManager.get("bookmarks");
await generateNewKeys(Service.collectionKeys);
Service.recordTelemetryEvent = (object, method, value, extra = undefined) => {
recordedEvents.push({ object, method, value, extra });
};
});
function checkRecordedEvents(expected, message) {
deepEqual(recordedEvents, expected, message);
// and clear the list so future checks are easier to write.
recordedEvents = [];
}
// Backs up and resets all preferences to their default values. Returns a
// function that restores the preferences when called.
function backupPrefs(names) {
let state = new Map();
for (let name of names) {
state.set(name, Svc.Prefs.get(name));
Svc.Prefs.reset(name);
}
return () => {
for (let [name, value] of state) {
Svc.Prefs.set(name, value);
}
};
}
async function promiseValidationDone(expected) {
// wait for a validation to complete.
let obs = promiseOneObserver("weave:engine:validate:finish");
let { subject: validationResult } = await obs;
// check the results - anything non-zero is checked against |expected|
let summary = validationResult.problems;
let actual = summary.filter(({ name, count }) => count);
actual.sort((a, b) => String(a.name).localeCompare(b.name));
expected.sort((a, b) => String(a.name).localeCompare(b.name));
deepEqual(actual, expected);
}
async function cleanup(server) {
await bookmarksEngine._store.wipe();
await bookmarksEngine.resetClient();
await clientsEngine._store.wipe();
Svc.Prefs.resetBranch("");
Service.recordManager.clearCache();
await promiseStopServer(server);
}
add_task(async function test_bookmark_repair_integration() {
enableValidationPrefs();
_("Ensure that a validation error triggers a repair request.");
let server = await serverForFoo(bookmarksEngine);
await SyncTestingInfrastructure(server);
let user = server.user("foo");
let initialID = Service.clientsEngine.localID;
let remoteID = Utils.makeGUID();
try {
_("Syncing to initialize crypto etc.");
await Service.sync();
_("Create remote client record");
server.insertWBO(
"foo",
"clients",
new ServerWBO(
remoteID,
encryptPayload({
id: remoteID,
name: "Remote client",
type: "desktop",
commands: [],
version: "54",
protocols: ["1.5"],
}),
Date.now() / 1000
)
);
_("Create bookmark and folder");
let folderInfo = await PlacesUtils.bookmarks.insert({
parentGuid: PlacesUtils.bookmarks.toolbarGuid,
type: PlacesUtils.bookmarks.TYPE_FOLDER,
title: "Folder 1",
});
let bookmarkInfo = await PlacesUtils.bookmarks.insert({
parentGuid: folderInfo.guid,
url: "http://getfirefox.com/",
title: "Get Firefox!",
});
_(`Upload ${folderInfo.guid} and ${bookmarkInfo.guid} to server`);
let validationPromise = promiseValidationDone([]);
await Service.sync();
equal(
clientsEngine.stats.numClients,
2,
"Clients collection should have 2 records"
);
await validationPromise;
checkRecordedEvents([], "Should not start repair after first sync");
_("Back up last sync timestamps for remote client");
let lastSync = await PlacesSyncUtils.bookmarks.getLastSync();
_(`Delete ${bookmarkInfo.guid} locally and on server`);
// Now we will reach into the server and hard-delete the bookmark
user.collection("bookmarks").remove(bookmarkInfo.guid);
// And delete the bookmark, but cheat by telling places that Sync did
// it, so we don't end up with a tombstone.
await PlacesUtils.bookmarks.remove(bookmarkInfo.guid, {
source: PlacesUtils.bookmarks.SOURCE_SYNC,
});
deepEqual(
await PlacesSyncUtils.bookmarks.pullChanges(),
{},
`Should not upload tombstone for ${bookmarkInfo.guid}`
);
// sync again - we should have a few problems...
_("Sync again to trigger repair");
validationPromise = promiseValidationDone([
{ name: "missingChildren", count: 1 },
{ name: "sdiff:childGUIDs", count: 1 },
{ name: "structuralDifferences", count: 1 },
]);
await Service.sync();
await validationPromise;
let flowID = Svc.Prefs.get("repairs.bookmarks.flowID");
checkRecordedEvents(
[
{
object: "repair",
method: "started",
value: undefined,
extra: {
flowID,
numIDs: "2",
},
},
{
object: "sendcommand",
method: "repairRequest",
value: undefined,
extra: {
flowID,
deviceID: Service.identity.hashedDeviceID(remoteID),
},
},
{
object: "repair",
method: "request",
value: "upload",
extra: {
deviceID: Service.identity.hashedDeviceID(remoteID),
flowID,
numIDs: "2",
},
},
],
"Should record telemetry events for repair request"
);
// We should have started a repair with our second client.
equal(
(await clientsEngine.getClientCommands(remoteID)).length,
1,
"Should queue repair request for remote client after repair"
);
_("Sync to send outgoing repair request");
await Service.sync();
equal(
(await clientsEngine.getClientCommands(remoteID)).length,
0,
"Should send repair request to remote client after next sync"
);
checkRecordedEvents(
[],
"Should not record repair telemetry after sending repair request"
);
_("Back up repair state to restore later");
let restoreInitialRepairState = backupPrefs(BOOKMARK_REPAIR_STATE_PREFS);
let initialLastSync = await PlacesSyncUtils.bookmarks.getLastSync();
// so now let's take over the role of that other client!
_("Create new clients engine pretending to be remote client");
let remoteClientsEngine = (Service.clientsEngine = new ClientEngine(
Service
));
remoteClientsEngine.ignoreLastModifiedOnProcessCommands = true;
await remoteClientsEngine.initialize();
remoteClientsEngine.localID = remoteID;
_("Restore missing bookmark");
// Pretend Sync wrote the bookmark, so that we upload it as part of the
// repair instead of the sync.
bookmarkInfo.source = PlacesUtils.bookmarks.SOURCE_SYNC;
await PlacesUtils.bookmarks.insert(bookmarkInfo);
await PlacesSyncUtils.bookmarks.setLastSync(lastSync);
_("Sync as remote client");
await Service.sync();
checkRecordedEvents(
[
{
object: "processcommand",
method: "repairRequest",
value: undefined,
extra: {
flowID,
},
},
{
object: "repairResponse",
method: "uploading",
value: undefined,
extra: {
flowID,
numIDs: "2",
},
},
{
object: "sendcommand",
method: "repairResponse",
value: undefined,
extra: {
flowID,
deviceID: Service.identity.hashedDeviceID(initialID),
},
},
{
object: "repairResponse",
method: "finished",
value: undefined,
extra: {
flowID,
numIDs: "2",
},
},
],
"Should record telemetry events for repair response"
);
// We should queue the repair response for the initial client.
equal(
(await remoteClientsEngine.getClientCommands(initialID)).length,
1,
"Should queue repair response for initial client after repair"
);
ok(
user.collection("bookmarks").wbo(bookmarkInfo.guid),
"Should upload missing bookmark"
);
_("Sync to upload bookmark and send outgoing repair response");
await Service.sync();
equal(
(await remoteClientsEngine.getClientCommands(initialID)).length,
0,
"Should send repair response to initial client after next sync"
);
checkRecordedEvents(
[],
"Should not record repair telemetry after sending repair response"
);
ok(
!Services.prefs.prefHasUserValue("services.sync.repairs.bookmarks.state"),
"Remote client should not be repairing"
);
_("Pretend to be initial client again");
Service.clientsEngine = clientsEngine;
_("Restore incomplete Places database and prefs");
await PlacesUtils.bookmarks.remove(bookmarkInfo.guid, {
source: PlacesUtils.bookmarks.SOURCE_SYNC,
});
restoreInitialRepairState();
await PlacesSyncUtils.bookmarks.setLastSync(initialLastSync);
ok(
Services.prefs.prefHasUserValue("services.sync.repairs.bookmarks.state"),
"Initial client should still be repairing"
);
_("Sync as initial client");
let revalidationPromise = promiseValidationDone([]);
await Service.sync();
let restoredBookmarkInfo = await PlacesUtils.bookmarks.fetch(
bookmarkInfo.guid
);
ok(
restoredBookmarkInfo,
`Missing bookmark ${bookmarkInfo.guid} should be downloaded to initial client`
);
checkRecordedEvents([
{
object: "processcommand",
method: "repairResponse",
value: undefined,
extra: {
flowID,
},
},
{
object: "repair",
method: "response",
value: "upload",
extra: {
flowID,
deviceID: Service.identity.hashedDeviceID(remoteID),
numIDs: "2",
},
},
{
object: "repair",
method: "finished",
value: undefined,
extra: {
flowID,
numIDs: "0",
},
},
]);
await revalidationPromise;
ok(
!Services.prefs.prefHasUserValue("services.sync.repairs.bookmarks.state"),
"Should clear repair pref after successfully completing repair"
);
} finally {
await cleanup(server);
clientsEngine = Service.clientsEngine = new ClientEngine(Service);
clientsEngine.ignoreLastModifiedOnProcessCommands = true;
await clientsEngine.initialize();
}
});
add_task(async function test_repair_client_missing() {
enableValidationPrefs();
_(
"Ensure that a record missing from the client only will get re-downloaded from the server"
);
let server = await serverForFoo(bookmarksEngine);
await SyncTestingInfrastructure(server);
let remoteID = Utils.makeGUID();
try {
_("Syncing to initialize crypto etc.");
await Service.sync();
_("Create remote client record");
server.insertWBO(
"foo",
"clients",
new ServerWBO(
remoteID,
encryptPayload({
id: remoteID,
name: "Remote client",
type: "desktop",
commands: [],
version: "54",
protocols: ["1.5"],
}),
Date.now() / 1000
)
);
let bookmarkInfo = await PlacesUtils.bookmarks.insert({
parentGuid: PlacesUtils.bookmarks.toolbarGuid,
url: "http://getfirefox.com/",
title: "Get Firefox!",
});
let validationPromise = promiseValidationDone([]);
_("Syncing.");
await Service.sync();
// should have 2 clients
equal(clientsEngine.stats.numClients, 2);
await validationPromise;
// Delete the bookmark localy, but cheat by telling places that Sync did
// it, so Sync still thinks we have it.
await PlacesUtils.bookmarks.remove(bookmarkInfo.guid, {
source: PlacesUtils.bookmarks.SOURCE_SYNC,
});
// Ensure we won't upload a tombstone for the removed bookmark.
Assert.deepEqual(await PlacesSyncUtils.bookmarks.pullChanges(), {});
// Ensure the bookmark no longer exists in Places.
Assert.equal(null, await PlacesUtils.bookmarks.fetch(bookmarkInfo.guid));
// sync again - we should have a few problems...
_("Syncing again.");
validationPromise = promiseValidationDone([
{ name: "clientMissing", count: 1 },
{ name: "sdiff:childGUIDs", count: 1 },
{ name: "structuralDifferences", count: 1 },
]);
await Service.sync();
await validationPromise;
// We shouldn't have started a repair with our second client.
equal((await clientsEngine.getClientCommands(remoteID)).length, 0);
// Trigger a sync (will request the missing item)
await Service.sync();
// And we got our bookmark back
Assert.ok(await PlacesUtils.bookmarks.fetch(bookmarkInfo.guid));
} finally {
await cleanup(server);
}
});
add_task(async function test_repair_server_missing() {
enableValidationPrefs();
_(
"Ensure that a record missing from the server only will get re-upload from the client"
);
let server = await serverForFoo(bookmarksEngine);
await SyncTestingInfrastructure(server);
let user = server.user("foo");
let remoteID = Utils.makeGUID();
try {
_("Syncing to initialize crypto etc.");
await Service.sync();
_("Create remote client record");
server.insertWBO(
"foo",
"clients",
new ServerWBO(
remoteID,
encryptPayload({
id: remoteID,
name: "Remote client",
type: "desktop",
commands: [],
version: "54",
protocols: ["1.5"],
}),
Date.now() / 1000
)
);
let bookmarkInfo = await PlacesUtils.bookmarks.insert({
parentGuid: PlacesUtils.bookmarks.toolbarGuid,
url: "http://getfirefox.com/",
title: "Get Firefox!",
});
let validationPromise = promiseValidationDone([]);
_("Syncing.");
await Service.sync();
// should have 2 clients
equal(clientsEngine.stats.numClients, 2);
await validationPromise;
// Now we will reach into the server and hard-delete the bookmark
user
.collection("bookmarks")
.wbo(bookmarkInfo.guid)
.delete();
// sync again - we should have a few problems...
_("Syncing again.");
validationPromise = promiseValidationDone([
{ name: "serverMissing", count: 1 },
{ name: "missingChildren", count: 1 },
]);
await Service.sync();
await validationPromise;
// We shouldn't have started a repair with our second client.
equal((await clientsEngine.getClientCommands(remoteID)).length, 0);
// Trigger a sync (will upload the missing item)
await Service.sync();
// And the server got our bookmark back
Assert.ok(user.collection("bookmarks").wbo(bookmarkInfo.guid));
} finally {
await cleanup(server);
}
});
add_task(async function test_repair_server_deleted() {
enableValidationPrefs();
_(
"Ensure that a record marked as deleted on the server but present on the client will get deleted on the client"
);
let server = await serverForFoo(bookmarksEngine);
await SyncTestingInfrastructure(server);
let remoteID = Utils.makeGUID();
try {
_("Syncing to initialize crypto etc.");
await Service.sync();
_("Create remote client record");
server.insertWBO(
"foo",
"clients",
new ServerWBO(
remoteID,
encryptPayload({
id: remoteID,
name: "Remote client",
type: "desktop",
commands: [],
version: "54",
protocols: ["1.5"],
}),
Date.now() / 1000
)
);
let bookmarkInfo = await PlacesUtils.bookmarks.insert({
parentGuid: PlacesUtils.bookmarks.toolbarGuid,
url: "http://getfirefox.com/",
title: "Get Firefox!",
});
let validationPromise = promiseValidationDone([]);
_("Syncing.");
await Service.sync();
// should have 2 clients
equal(clientsEngine.stats.numClients, 2);
await validationPromise;
// Now we will reach into the server and create a tombstone for that bookmark
// but with a last-modified in the past - this way our sync isn't going to
// pick up the record.
_(`Adding server tombstone for ${bookmarkInfo.guid}`);
server.insertWBO(
"foo",
"bookmarks",
new ServerWBO(
bookmarkInfo.guid,
encryptPayload({
id: bookmarkInfo.guid,
deleted: true,
}),
(Date.now() - 60000) / 1000
)
);
// sync again - we should have a few problems...
_("Syncing again.");
validationPromise = promiseValidationDone([
{ name: "serverDeleted", count: 1 },
{ name: "deletedChildren", count: 1 },
]);
await Service.sync();
await validationPromise;
// We shouldn't have started a repair with our second client.
equal((await clientsEngine.getClientCommands(remoteID)).length, 0);
// Trigger a sync (will upload the missing item)
await Service.sync();
// And the client deleted our bookmark
Assert.ok(!(await PlacesUtils.bookmarks.fetch(bookmarkInfo.guid)));
} finally {
await cleanup(server);
}
});

569
services/sync/tests/unit/test_bookmark_repair_requestor.js
View File

@@ -1,569 +0,0 @@
/* Any copyright is dedicated to the Public Domain.
http://creativecommons.org/publicdomain/zero/1.0/ */
const { BookmarkRepairRequestor } = ChromeUtils.import(
"resource://services-sync/bookmark_repair.js"
);
function makeClientRecord(id, fields = {}) {
return {
id,
version: fields.version || "54.0a1",
type: fields.type || "desktop",
stale: fields.stale || false,
serverLastModified: fields.serverLastModified || 0,
};
}
class MockClientsEngine {
constructor(clientList) {
this._clientList = clientList;
this._sentCommands = {};
}
get remoteClients() {
return Object.values(this._clientList);
}
remoteClient(id) {
return this._clientList[id];
}
async sendCommand(command, args, clientID) {
let cc = this._sentCommands[clientID] || [];
cc.push({ command, args });
this._sentCommands[clientID] = cc;
}
async getClientCommands(clientID) {
return this._sentCommands[clientID] || [];
}
}
class MockIdentity {
hashedDeviceID(did) {
return did; // don't hash it to make testing easier.
}
}
class MockService {
constructor(clientList) {
this.clientsEngine = new MockClientsEngine(clientList);
this.identity = new MockIdentity();
this._recordedEvents = [];
}
recordTelemetryEvent(object, method, value, extra = undefined) {
this._recordedEvents.push({ method, object, value, extra });
}
}
function checkState(expected) {
equal(
Services.prefs.getCharPref("services.sync.repairs.bookmarks.state"),
expected
);
}
function checkRepairFinished() {
try {
let state = Services.prefs.getCharPref(
"services.sync.repairs.bookmarks.state"
);
ok(false, state);
} catch (ex) {
ok(true, "no repair preference exists");
}
}
function checkOutgoingCommand(service, clientID) {
let sent = service.clientsEngine._sentCommands;
deepEqual(Object.keys(sent), [clientID]);
equal(sent[clientID].length, 1);
equal(sent[clientID][0].command, "repairRequest");
}
function NewBookmarkRepairRequestor(mockService) {
let req = new BookmarkRepairRequestor(mockService);
req._now = () => Date.now() / 1000; // _now() is seconds.
return req;
}
add_task(async function test_requestor_no_clients() {
let mockService = new MockService({});
let requestor = NewBookmarkRepairRequestor(mockService);
let validationInfo = {
problems: {
missingChildren: [
{ parent: "x", child: "a" },
{ parent: "x", child: "b" },
{ parent: "x", child: "c" },
],
orphans: [],
},
};
let flowID = Utils.makeGUID();
await requestor.startRepairs(validationInfo, flowID);
// there are no clients, so we should end up in "finished" (which we need to
// check via telemetry)
deepEqual(mockService._recordedEvents, [
{
object: "repair",
method: "started",
value: undefined,
extra: { flowID, numIDs: 4 },
},
{
object: "repair",
method: "finished",
value: undefined,
extra: { flowID, numIDs: 4 },
},
]);
});
add_task(async function test_requestor_one_client_no_response() {
let mockService = new MockService({
"client-a": makeClientRecord("client-a"),
});
let requestor = NewBookmarkRepairRequestor(mockService);
let validationInfo = {
problems: {
missingChildren: [
{ parent: "x", child: "a" },
{ parent: "x", child: "b" },
{ parent: "x", child: "c" },
],
orphans: [],
},
};
let flowID = Utils.makeGUID();
await requestor.startRepairs(validationInfo, flowID);
// the command should now be outgoing.
checkOutgoingCommand(mockService, "client-a");
checkState(BookmarkRepairRequestor.STATE.SENT_REQUEST);
// asking it to continue stays in that state until we timeout or the command
// is removed.
await requestor.continueRepairs();
checkState(BookmarkRepairRequestor.STATE.SENT_REQUEST);
// now pretend that client synced.
mockService.clientsEngine._sentCommands = {};
await requestor.continueRepairs();
checkState(BookmarkRepairRequestor.STATE.SENT_SECOND_REQUEST);
// the command should be outgoing again.
checkOutgoingCommand(mockService, "client-a");
// pretend that client synced again without writing a command.
mockService.clientsEngine._sentCommands = {};
await requestor.continueRepairs();
// There are no more clients, so we've given up.
checkRepairFinished();
deepEqual(mockService._recordedEvents, [
{
object: "repair",
method: "started",
value: undefined,
extra: { flowID, numIDs: 4 },
},
{
object: "repair",
method: "request",
value: "upload",
extra: { flowID, numIDs: 4, deviceID: "client-a" },
},
{
object: "repair",
method: "request",
value: "upload",
extra: { flowID, numIDs: 4, deviceID: "client-a" },
},
{
object: "repair",
method: "finished",
value: undefined,
extra: { flowID, numIDs: 4 },
},
]);
});
add_task(async function test_requestor_one_client_no_sync() {
let mockService = new MockService({
"client-a": makeClientRecord("client-a"),
});
let requestor = NewBookmarkRepairRequestor(mockService);
let validationInfo = {
problems: {
missingChildren: [
{ parent: "x", child: "a" },
{ parent: "x", child: "b" },
{ parent: "x", child: "c" },
],
orphans: [],
},
};
let flowID = Utils.makeGUID();
await requestor.startRepairs(validationInfo, flowID);
// the command should now be outgoing.
checkOutgoingCommand(mockService, "client-a");
checkState(BookmarkRepairRequestor.STATE.SENT_REQUEST);
// pretend we are now in the future.
let theFuture = Date.now() + 300000000;
requestor._now = () => theFuture;
await requestor.continueRepairs();
// We should be finished as we gave up in disgust.
checkRepairFinished();
deepEqual(mockService._recordedEvents, [
{
object: "repair",
method: "started",
value: undefined,
extra: { flowID, numIDs: 4 },
},
{
object: "repair",
method: "request",
value: "upload",
extra: { flowID, numIDs: 4, deviceID: "client-a" },
},
{
object: "repair",
method: "abandon",
value: "silent",
extra: { flowID, deviceID: "client-a" },
},
{
object: "repair",
method: "finished",
value: undefined,
extra: { flowID, numIDs: 4 },
},
]);
});
add_task(async function test_requestor_latest_client_used() {
let mockService = new MockService({
"client-early": makeClientRecord("client-early", {
serverLastModified: Date.now() - 10,
}),
"client-late": makeClientRecord("client-late", {
serverLastModified: Date.now(),
}),
});
let requestor = NewBookmarkRepairRequestor(mockService);
let validationInfo = {
problems: {
missingChildren: [{ parent: "x", child: "a" }],
orphans: [],
},
};
await requestor.startRepairs(validationInfo, Utils.makeGUID());
// the repair command should be outgoing to the most-recent client.
checkOutgoingCommand(mockService, "client-late");
checkState(BookmarkRepairRequestor.STATE.SENT_REQUEST);
// and this test is done - reset the repair.
requestor.prefs.resetBranch();
});
add_task(async function test_requestor_client_vanishes() {
let mockService = new MockService({
"client-a": makeClientRecord("client-a"),
"client-b": makeClientRecord("client-b"),
});
let requestor = NewBookmarkRepairRequestor(mockService);
let validationInfo = {
problems: {
missingChildren: [
{ parent: "x", child: "a" },
{ parent: "x", child: "b" },
{ parent: "x", child: "c" },
],
orphans: [],
},
};
let flowID = Utils.makeGUID();
await requestor.startRepairs(validationInfo, flowID);
// the command should now be outgoing.
checkOutgoingCommand(mockService, "client-a");
checkState(BookmarkRepairRequestor.STATE.SENT_REQUEST);
mockService.clientsEngine._sentCommands = {};
// Now let's pretend the client vanished.
delete mockService.clientsEngine._clientList["client-a"];
await requestor.continueRepairs();
// We should have moved on to client-b.
checkState(BookmarkRepairRequestor.STATE.SENT_REQUEST);
checkOutgoingCommand(mockService, "client-b");
// Now let's pretend client B wrote all missing IDs.
let response = {
collection: "bookmarks",
request: "upload",
flowID: requestor._flowID,
clientID: "client-b",
ids: ["a", "b", "c", "x"],
};
await requestor.continueRepairs(response);
// We should be finished as we got all our IDs.
checkRepairFinished();
deepEqual(mockService._recordedEvents, [
{
object: "repair",
method: "started",
value: undefined,
extra: { flowID, numIDs: 4 },
},
{
object: "repair",
method: "request",
value: "upload",
extra: { flowID, numIDs: 4, deviceID: "client-a" },
},
{
object: "repair",
method: "abandon",
value: "missing",
extra: { flowID, deviceID: "client-a" },
},
{
object: "repair",
method: "request",
value: "upload",
extra: { flowID, numIDs: 4, deviceID: "client-b" },
},
{
object: "repair",
method: "response",
value: "upload",
extra: { flowID, deviceID: "client-b", numIDs: 4 },
},
{
object: "repair",
method: "finished",
value: undefined,
extra: { flowID, numIDs: 0 },
},
]);
});
add_task(async function test_requestor_success_responses() {
let mockService = new MockService({
"client-a": makeClientRecord("client-a"),
"client-b": makeClientRecord("client-b"),
});
let requestor = NewBookmarkRepairRequestor(mockService);
let validationInfo = {
problems: {
missingChildren: [
{ parent: "x", child: "a" },
{ parent: "x", child: "b" },
{ parent: "x", child: "c" },
],
orphans: [],
},
};
let flowID = Utils.makeGUID();
await requestor.startRepairs(validationInfo, flowID);
// the command should now be outgoing.
checkOutgoingCommand(mockService, "client-a");
checkState(BookmarkRepairRequestor.STATE.SENT_REQUEST);
mockService.clientsEngine._sentCommands = {};
// Now let's pretend the client wrote a response.
let response = {
collection: "bookmarks",
request: "upload",
clientID: "client-a",
flowID: requestor._flowID,
ids: ["a", "b"],
};
await requestor.continueRepairs(response);
// We should have moved on to client 2.
checkState(BookmarkRepairRequestor.STATE.SENT_REQUEST);
checkOutgoingCommand(mockService, "client-b");
// Now let's pretend client B write the missing ID.
response = {
collection: "bookmarks",
request: "upload",
clientID: "client-b",
flowID: requestor._flowID,
ids: ["c", "x"],
};
await requestor.continueRepairs(response);
// We should be finished as we got all our IDs.
checkRepairFinished();
deepEqual(mockService._recordedEvents, [
{
object: "repair",
method: "started",
value: undefined,
extra: { flowID, numIDs: 4 },
},
{
object: "repair",
method: "request",
value: "upload",
extra: { flowID, numIDs: 4, deviceID: "client-a" },
},
{
object: "repair",
method: "response",
value: "upload",
extra: { flowID, deviceID: "client-a", numIDs: 2 },
},
{
object: "repair",
method: "request",
value: "upload",
extra: { flowID, numIDs: 2, deviceID: "client-b" },
},
{
object: "repair",
method: "response",
value: "upload",
extra: { flowID, deviceID: "client-b", numIDs: 2 },
},
{
object: "repair",
method: "finished",
value: undefined,
extra: { flowID, numIDs: 0 },
},
]);
});
add_task(async function test_client_suitability() {
let mockService = new MockService({
"client-a": makeClientRecord("client-a"),
"client-b": makeClientRecord("client-b", { type: "mobile" }),
"client-c": makeClientRecord("client-c", { version: "52.0a1" }),
"client-d": makeClientRecord("client-c", { version: "54.0a1" }),
});
let requestor = NewBookmarkRepairRequestor(mockService);
ok(
requestor._isSuitableClient(
mockService.clientsEngine.remoteClient("client-a")
)
);
ok(
!requestor._isSuitableClient(
mockService.clientsEngine.remoteClient("client-b")
)
);
ok(
!requestor._isSuitableClient(
mockService.clientsEngine.remoteClient("client-c")
)
);
ok(
requestor._isSuitableClient(
mockService.clientsEngine.remoteClient("client-d")
)
);
});
add_task(async function test_requestor_already_repairing_at_start() {
let mockService = new MockService({});
let requestor = NewBookmarkRepairRequestor(mockService);
requestor.anyClientsRepairing = () => true;
let validationInfo = {
problems: {
missingChildren: [
{ parent: "x", child: "a" },
{ parent: "x", child: "b" },
{ parent: "x", child: "c" },
],
orphans: [],
},
};
let flowID = Utils.makeGUID();
ok(
!(await requestor.startRepairs(validationInfo, flowID)),
"Shouldn't start repairs"
);
equal(mockService._recordedEvents.length, 1);
equal(mockService._recordedEvents[0].method, "aborted");
});
add_task(async function test_requestor_already_repairing_continue() {
let clientB = makeClientRecord("client-b");
let mockService = new MockService({
"client-a": makeClientRecord("client-a"),
"client-b": clientB,
});
let requestor = NewBookmarkRepairRequestor(mockService);
let validationInfo = {
problems: {
missingChildren: [
{ parent: "x", child: "a" },
{ parent: "x", child: "b" },
{ parent: "x", child: "c" },
],
orphans: [],
},
};
let flowID = Utils.makeGUID();
await requestor.startRepairs(validationInfo, flowID);
// the command should now be outgoing.
checkOutgoingCommand(mockService, "client-a");
checkState(BookmarkRepairRequestor.STATE.SENT_REQUEST);
mockService.clientsEngine._sentCommands = {};
// Now let's pretend the client wrote a response (it doesn't matter what's in here)
let response = {
collection: "bookmarks",
request: "upload",
clientID: "client-a",
flowID: requestor._flowID,
ids: ["a", "b"],
};
// and another client also started a request
clientB.commands = [
{
args: [{ collection: "bookmarks", flowID: "asdf" }],
command: "repairRequest",
},
];
await requestor.continueRepairs(response);
// We should have aborted now
checkRepairFinished();
const expected = [
{
method: "started",
object: "repair",
value: undefined,
extra: { flowID, numIDs: "4" },
},
{
method: "request",
object: "repair",
value: "upload",
extra: { flowID, numIDs: "4", deviceID: "client-a" },
},
{
method: "aborted",
object: "repair",
value: undefined,
extra: { flowID, numIDs: "4", reason: "other clients repairing" },
},
];
deepEqual(mockService._recordedEvents, expected);
});

563
services/sync/tests/unit/test_bookmark_repair_responder.js
View File

@@ -1,563 +0,0 @@
/* Any copyright is dedicated to the Public Domain.
http://creativecommons.org/publicdomain/zero/1.0/ */
ChromeUtils.import("resource://services-sync/engines/bookmarks.js");
const { Service } = ChromeUtils.import("resource://services-sync/service.js");
const { BookmarkRepairResponder } = ChromeUtils.import(
"resource://services-sync/bookmark_repair.js"
);
// Disable validation so that we don't try to automatically repair the server
// when we sync.
Svc.Prefs.set("engine.bookmarks.validation.enabled", false);
// stub telemetry so we can easily check the right things are recorded.
var recordedEvents = [];
function checkRecordedEvents(expected) {
// Ignore event telemetry from the merger and Places maintenance.
let repairEvents = recordedEvents.filter(
event => !["mirror", "maintenance"].includes(event.object)
);
deepEqual(repairEvents, expected);
// and clear the list so future checks are easier to write.
recordedEvents = [];
}
function getServerBookmarks(server) {
return server.user("foo").collection("bookmarks");
}
async function makeServer() {
let server = await serverForFoo(bookmarksEngine);
await SyncTestingInfrastructure(server);
return server;
}
async function cleanup(server) {
await promiseStopServer(server);
await PlacesSyncUtils.bookmarks.wipe();
// clear keys so when each test finds a different server it accepts its keys.
Service.collectionKeys.clear();
}
let bookmarksEngine;
add_task(async function setup() {
bookmarksEngine = Service.engineManager.get("bookmarks");
Service.recordTelemetryEvent = (object, method, value, extra = undefined) => {
recordedEvents.push({ object, method, value, extra });
};
});
add_task(async function test_responder_error() {
let server = await makeServer();
// sync so the collection is created.
await Service.sync();
let request = {
request: "upload",
ids: [Utils.makeGUID()],
flowID: Utils.makeGUID(),
};
let responder = new BookmarkRepairResponder();
// mock the responder to simulate an error.
responder._fetchItemsToUpload = async function() {
throw new Error("oh no!");
};
await responder.repair(request, null);
checkRecordedEvents([
{
object: "repairResponse",
method: "failed",
value: undefined,
extra: {
flowID: request.flowID,
numIDs: "0",
failureReason: '{"name":"unexpectederror","error":"Error: oh no!"}',
},
},
]);
await cleanup(server);
});
add_task(async function test_responder_no_items() {
let server = await makeServer();
// sync so the collection is created.
await Service.sync();
let request = {
request: "upload",
ids: [Utils.makeGUID()],
flowID: Utils.makeGUID(),
};
let responder = new BookmarkRepairResponder();
await responder.repair(request, null);
checkRecordedEvents([
{
object: "repairResponse",
method: "finished",
value: undefined,
extra: { flowID: request.flowID, numIDs: "0" },
},
]);
await cleanup(server);
});
// One item requested and we have it locally, but it's not yet on the server.
add_task(async function test_responder_upload() {
let server = await makeServer();
await Service.sync();
deepEqual(
getServerBookmarks(server)
.keys()
.sort(),
["menu", "mobile", "toolbar", "unfiled"],
"Should only upload roots on first sync"
);
// Pretend we've already synced this bookmark, so that we can ensure it's
// uploaded in response to our repair request.
let bm = await PlacesUtils.bookmarks.insert({
parentGuid: PlacesUtils.bookmarks.unfiledGuid,
title: "Get Firefox",
url: "http://getfirefox.com/",
source: PlacesUtils.bookmarks.SOURCES.SYNC,
});
let request = {
request: "upload",
ids: [bm.guid],
flowID: Utils.makeGUID(),
};
let responder = new BookmarkRepairResponder();
await responder.repair(request, null);
checkRecordedEvents([
{
object: "repairResponse",
method: "uploading",
value: undefined,
extra: { flowID: request.flowID, numIDs: "1" },
},
]);
await Service.sync();
deepEqual(
getServerBookmarks(server)
.keys()
.sort(),
["menu", "mobile", "toolbar", "unfiled", bm.guid].sort(),
"Should upload requested bookmark on second sync"
);
checkRecordedEvents([
{
object: "repairResponse",
method: "finished",
value: undefined,
extra: { flowID: request.flowID, numIDs: "1" },
},
]);
await cleanup(server);
});
// One item requested and we have it locally and it's already on the server.
// As it was explicitly requested, we should upload it.
add_task(async function test_responder_item_exists_locally() {
let server = await makeServer();
let bm = await PlacesUtils.bookmarks.insert({
parentGuid: PlacesUtils.bookmarks.unfiledGuid,
title: "Get Firefox",
url: "http://getfirefox.com/",
});
// first sync to get the item on the server.
_("Syncing to get item on the server");
await Service.sync();
// issue a repair request for it.
let request = {
request: "upload",
ids: [bm.guid],
flowID: Utils.makeGUID(),
};
let responder = new BookmarkRepairResponder();
await responder.repair(request, null);
// We still re-upload the item.
checkRecordedEvents([
{
object: "repairResponse",
method: "uploading",
value: undefined,
extra: { flowID: request.flowID, numIDs: "1" },
},
]);
_("Syncing to do the upload.");
await Service.sync();
checkRecordedEvents([
{
object: "repairResponse",
method: "finished",
value: undefined,
extra: { flowID: request.flowID, numIDs: "1" },
},
]);
await cleanup(server);
});
add_task(async function test_responder_tombstone() {
let server = await makeServer();
// TODO: Request an item for which we have a tombstone locally. Decide if
// we want to store tombstones permanently for this. In the integration
// test, we can also try requesting a deleted child or ancestor.
// For now, we'll handle this identically to `test_responder_missing_items`.
// Bug 1343103 is a follow-up to better handle this.
await cleanup(server);
});
add_task(async function test_responder_missing_items() {
let server = await makeServer();
let fxBmk = await PlacesUtils.bookmarks.insert({
parentGuid: PlacesUtils.bookmarks.unfiledGuid,
title: "Get Firefox",
url: "http://getfirefox.com/",
});
await Service.sync();
deepEqual(
getServerBookmarks(server)
.keys()
.sort(),
["menu", "mobile", "toolbar", "unfiled", fxBmk.guid].sort(),
"Should upload roots and Firefox on first sync"
);
let tbBmk = await PlacesUtils.bookmarks.insert({
parentGuid: PlacesUtils.bookmarks.unfiledGuid,
title: "Get Thunderbird",
url: "http://getthunderbird.com/",
// Pretend we've already synced Thunderbird.
source: PlacesUtils.bookmarks.SOURCES.SYNC,
});
_("Request Firefox, Thunderbird, and nonexistent GUID");
let request = {
request: "upload",
ids: [fxBmk.guid, tbBmk.guid, Utils.makeGUID()],
flowID: Utils.makeGUID(),
};
let responder = new BookmarkRepairResponder();
await responder.repair(request, null);
checkRecordedEvents([
{
object: "repairResponse",
method: "uploading",
value: undefined,
extra: { flowID: request.flowID, numIDs: "2" },
},
]);
_("Sync after requesting IDs");
await Service.sync();
deepEqual(
getServerBookmarks(server)
.keys()
.sort(),
["menu", "mobile", "toolbar", "unfiled", fxBmk.guid, tbBmk.guid].sort(),
"Second sync should upload Thunderbird; skip nonexistent"
);
checkRecordedEvents([
{
object: "repairResponse",
method: "finished",
value: undefined,
extra: { flowID: request.flowID, numIDs: "2" },
},
]);
await cleanup(server);
});
add_task(async function test_folder_descendants() {
let server = await makeServer();
let parentFolder = await PlacesUtils.bookmarks.insert({
type: PlacesUtils.bookmarks.TYPE_FOLDER,
parentGuid: PlacesUtils.bookmarks.menuGuid,
title: "Parent folder",
});
let childFolder = await PlacesUtils.bookmarks.insert({
type: PlacesUtils.bookmarks.TYPE_FOLDER,
parentGuid: parentFolder.guid,
title: "Child folder",
});
// This item is in parentFolder and *should not* be uploaded as part of
// the repair even though we explicitly request its parent.
let existingChildBmk = await PlacesUtils.bookmarks.insert({
parentGuid: parentFolder.guid,
title: "Get Firefox",
url: "http://firefox.com",
});
// This item is in parentFolder and *should* be uploaded as part of
// the repair because we explicitly request its ID.
let childSiblingBmk = await PlacesUtils.bookmarks.insert({
parentGuid: parentFolder.guid,
title: "Get Thunderbird",
url: "http://getthunderbird.com",
});
_("Initial sync to upload roots and parent folder");
await Service.sync();
let initialRecordIds = [
"menu",
"mobile",
"toolbar",
"unfiled",
parentFolder.guid,
existingChildBmk.guid,
childFolder.guid,
childSiblingBmk.guid,
].sort();
deepEqual(
getServerBookmarks(server)
.keys()
.sort(),
initialRecordIds,
"Should upload roots and partial folder contents on first sync"
);
_("Insert missing bookmarks locally to request later");
let childBmk = await PlacesSyncUtils.bookmarks.insert({
kind: "bookmark",
recordId: Utils.makeGUID(),
parentRecordId: parentFolder.guid,
title: "Get Firefox",
url: "http://getfirefox.com",
});
let grandChildBmk = await PlacesSyncUtils.bookmarks.insert({
kind: "bookmark",
recordId: Utils.makeGUID(),
parentRecordId: childFolder.guid,
title: "Bugzilla",
url: "https://bugzilla.mozilla.org",
});
let grandChildSiblingBmk = await PlacesSyncUtils.bookmarks.insert({
kind: "bookmark",
recordId: Utils.makeGUID(),
parentRecordId: childFolder.guid,
title: "Mozilla",
url: "https://mozilla.org",
});
_("Sync again");
await Service.sync();
{
// The buffered engine will upload the missing records, since it does a full
// tree merge. The old engine won't, since it relies on the Sync status
// flag, and we used `PSU.b.i` to pretend that Sync added the bookmarks.
let collection = getServerBookmarks(server);
collection.remove(childBmk.recordId);
collection.remove(grandChildBmk.recordId);
collection.remove(grandChildSiblingBmk.recordId);
deepEqual(
collection.keys().sort(),
initialRecordIds,
"Second sync should not upload missing bookmarks"
);
}
// This assumes the parent record on the server is correct, and the server
// is just missing the children. This isn't a correct assumption if the
// parent's `children` array is wrong, or if the parent and children disagree.
_("Request missing bookmarks");
let request = {
request: "upload",
ids: [
// Already on server (but still uploaded as they are explicitly requested)
parentFolder.guid,
childSiblingBmk.guid,
// Explicitly upload these. We should also upload `grandChildBmk`,
// since it's a descendant of `parentFolder` and we requested its
// ancestor.
childBmk.recordId,
grandChildSiblingBmk.recordId,
],
flowID: Utils.makeGUID(),
};
let responder = new BookmarkRepairResponder();
await responder.repair(request, null);
checkRecordedEvents([
{
object: "repairResponse",
method: "uploading",
value: undefined,
extra: { flowID: request.flowID, numIDs: "5" },
},
]);
_("Sync after requesting repair; should upload missing records");
await Service.sync();
deepEqual(
getServerBookmarks(server)
.keys()
.sort(),
[
...initialRecordIds,
childBmk.recordId,
grandChildBmk.recordId,
grandChildSiblingBmk.recordId,
].sort(),
"Third sync should upload requested items"
);
checkRecordedEvents([
{
object: "repairResponse",
method: "finished",
value: undefined,
extra: { flowID: request.flowID, numIDs: "5" },
},
]);
await cleanup(server);
});
// Error handling.
add_task(async function test_aborts_unknown_request() {
let server = await makeServer();
let request = {
request: "not-upload",
ids: [],
flowID: Utils.makeGUID(),
};
let responder = new BookmarkRepairResponder();
await responder.repair(request, null);
checkRecordedEvents([
{
object: "repairResponse",
method: "aborted",
value: undefined,
extra: {
flowID: request.flowID,
reason: "Don't understand request type 'not-upload'",
},
},
]);
await cleanup(server);
});
add_task(async function test_upload_fail() {
let server = await makeServer();
// Pretend we've already synced this bookmark, so that we can ensure it's
// uploaded in response to our repair request.
let bm = await PlacesUtils.bookmarks.insert({
parentGuid: PlacesUtils.bookmarks.unfiledGuid,
title: "Get Firefox",
url: "http://getfirefox.com/",
source: PlacesUtils.bookmarks.SOURCES.SYNC,
});
await Service.sync();
let request = {
request: "upload",
ids: [bm.guid],
flowID: Utils.makeGUID(),
};
let responder = new BookmarkRepairResponder();
await responder.repair(request, null);
checkRecordedEvents([
{
object: "repairResponse",
method: "uploading",
value: undefined,
extra: { flowID: request.flowID, numIDs: "1" },
},
]);
// This sync would normally upload the item - arrange for it to fail.
let engine = Service.engineManager.get("bookmarks");
let oldCreateRecord = engine._createRecord;
engine._createRecord = async function(id) {
return "anything"; // doesn't have an "encrypt"
};
let numFailures = 0;
let numSuccesses = 0;
function onUploaded(subject, data) {
if (data != "bookmarks") {
return;
}
if (subject.failed) {
numFailures += 1;
} else {
numSuccesses += 1;
}
}
Svc.Obs.add("weave:engine:sync:uploaded", onUploaded, this);
await Service.sync();
equal(numFailures, 1);
equal(numSuccesses, 0);
// should be no recorded events
checkRecordedEvents([]);
// restore the error injection so next sync succeeds - the repair should
// restart
engine._createRecord = oldCreateRecord;
await responder.repair(request, null);
checkRecordedEvents([
{
object: "repairResponse",
method: "uploading",
value: undefined,
extra: { flowID: request.flowID, numIDs: "1" },
},
]);
await Service.sync();
checkRecordedEvents([
{
object: "repairResponse",
method: "finished",
value: undefined,
extra: { flowID: request.flowID, numIDs: "1" },
},
]);
equal(numFailures, 1);
equal(numSuccesses, 1);
Svc.Obs.remove("weave:engine:sync:uploaded", onUploaded, this);
await cleanup(server);
});
add_task(async function teardown() {
Svc.Prefs.reset("engine.bookmarks.validation.enabled");
});

202
services/sync/tests/unit/test_doctor.js
View File

@@ -1,202 +0,0 @@
/* Any copyright is dedicated to the Public Domain.
http://creativecommons.org/publicdomain/zero/1.0/ */
const { Doctor, REPAIR_ADVANCE_PERIOD } = ChromeUtils.import(
"resource://services-sync/doctor.js",
null
);
function mockDoctor(mocks) {
// Clone the object and put mocks in that.
return Object.assign({}, Doctor, mocks);
}
add_task(async function test_validation_interval() {
let now = 1000;
let doctor = mockDoctor({
_now() {
// note that the function being mocked actually returns seconds.
return now;
},
});
let engine = {
name: "test-engine",
getValidator() {
return {
validate(e) {
return {};
},
};
},
};
// setup prefs which enable test-engine validation.
Services.prefs.setBoolPref(
"services.sync.engine.test-engine.validation.enabled",
true
);
Services.prefs.setIntPref(
"services.sync.engine.test-engine.validation.percentageChance",
100
);
Services.prefs.setIntPref(
"services.sync.engine.test-engine.validation.maxRecords",
1
);
// And say we should validate every 10 seconds.
Services.prefs.setIntPref(
"services.sync.engine.test-engine.validation.interval",
10
);
deepEqual(doctor._getEnginesToValidate([engine]), {
"test-engine": {
engine,
maxRecords: 1,
},
});
// We haven't advanced the timestamp, so we should not validate again.
deepEqual(doctor._getEnginesToValidate([engine]), {});
// Advance our clock by 11 seconds.
now += 11;
// We should validate again.
deepEqual(doctor._getEnginesToValidate([engine]), {
"test-engine": {
engine,
maxRecords: 1,
},
});
});
add_task(async function test_repairs_start() {
let repairStarted = false;
let problems = {
missingChildren: ["a", "b", "c"],
};
let validator = {
validate(engine) {
return problems;
},
canValidate() {
return Promise.resolve(true);
},
};
let engine = {
name: "test-engine",
getValidator() {
return validator;
},
};
let requestor = {
async startRepairs(validationInfo, flowID) {
ok(flowID, "got a flow ID");
equal(validationInfo, problems);
repairStarted = true;
return true;
},
tryServerOnlyRepairs() {
return false;
},
};
let doctor = mockDoctor({
_getEnginesToValidate(recentlySyncedEngines) {
deepEqual(recentlySyncedEngines, [engine]);
return {
"test-engine": { engine, maxRecords: -1 },
};
},
_getRepairRequestor(engineName) {
equal(engineName, engine.name);
return requestor;
},
_shouldRepair(e) {
return true;
},
});
let promiseValidationDone = promiseOneObserver(
"weave:engine:validate:finish"
);
await doctor.consult([engine]);
await promiseValidationDone;
ok(repairStarted);
});
add_task(async function test_repairs_advanced_daily() {
let repairCalls = 0;
let requestor = {
async continueRepairs() {
repairCalls++;
},
tryServerOnlyRepairs() {
return false;
},
};
// start now at just after REPAIR_ADVANCE_PERIOD so we do a a first one.
let now = REPAIR_ADVANCE_PERIOD + 1;
let doctor = mockDoctor({
_getEnginesToValidate() {
return {}; // no validations.
},
_runValidators() {
// do nothing.
},
_getAllRepairRequestors() {
return {
foo: requestor,
};
},
_now() {
return now;
},
});
await doctor.consult();
equal(repairCalls, 1);
now += 10; // 10 seconds later...
await doctor.consult();
// should not have done another repair yet - it's too soon.
equal(repairCalls, 1);
// advance our pretend clock by the advance period (eg, day)
now += REPAIR_ADVANCE_PERIOD;
await doctor.consult();
// should have done another repair
equal(repairCalls, 2);
});
add_task(async function test_repairs_skip_if_cant_vaidate() {
let validator = {
canValidate() {
return Promise.resolve(false);
},
validate() {
ok(false, "Shouldn't validate");
},
};
let engine = {
name: "test-engine",
getValidator() {
return validator;
},
};
let requestor = {
async startRepairs(validationInfo, flowID) {
ok(false, "Never should start repairs");
},
tryServerOnlyRepairs() {
return false;
},
};
let doctor = mockDoctor({
_getEnginesToValidate(recentlySyncedEngines) {
deepEqual(recentlySyncedEngines, [engine]);
return {
"test-engine": { engine, maxRecords: -1 },
};
},
_getRepairRequestor(engineName) {
equal(engineName, engine.name);
return requestor;
},
});
await doctor.consult([engine]);
});

11
services/sync/tests/unit/xpcshell.ini
View File

@@ -127,15 +127,6 @@ skip-if = tsan # Runs unreasonably slow on TSan, bug 1612707
[test_bookmark_order.js]
[test_bookmark_places_query_rewriting.js]
[test_bookmark_record.js]
[test_bookmark_repair.js]
skip-if = release_or_beta
run-sequentially = Frequent timeouts, bug 1395148
[test_bookmark_repair_requestor.js]
# Repair is enabled only on Aurora and Nightly
skip-if = release_or_beta
[test_bookmark_repair_responder.js]
skip-if = release_or_beta
run-sequentially = Frequent timeouts, bug 1395148
[test_bookmark_store.js]
[test_bookmark_decline_undecline.js]
[test_bookmark_tracker.js]
@@ -146,8 +137,6 @@ requesttimeoutfactor = 4
[test_clients_engine.js]
run-sequentially = Frequent timeouts, bug 1395148
[test_clients_escape.js]
[test_doctor.js]
run-sequentially = extension-storage migration happens only once, and must be tested first.
[test_extension_storage_migration_telem.js]
run-sequentially = extension-storage migration happens only once, and must be tested first.
[test_extension_storage_engine.js]

52
toolkit/components/places/PlacesSyncUtils.jsm
View File

@@ -644,58 +644,6 @@ const BookmarkSyncUtils = (PlacesSyncUtils.bookmarks = Object.freeze({
);
},
/**
* Returns an array of `{ recordId, syncable }` tuples for all items in
* `requestedRecordIds`. If any requested ID is a folder, all its descendants
* will be included. Ancestors of non-syncable items are not included; if
* any are missing on the server, the requesting client will need to make
* another repair request.
*
* Sync calls this method to respond to incoming bookmark repair requests
* and upload items that are missing on the server.
*/
fetchRecordIdsForRepair(requestedRecordIds) {
let requestedGuids = requestedRecordIds.map(
BookmarkSyncUtils.recordIdToGuid
);
return PlacesUtils.withConnectionWrapper(
"BookmarkSyncUtils: fetchRecordIdsForRepair",
async function(db) {
let rows = await db.executeCached(`
WITH RECURSIVE
syncedItems(id) AS (
SELECT b.id FROM moz_bookmarks b
WHERE b.guid IN ('menu________', 'toolbar_____', 'unfiled_____',
'mobile______')
UNION ALL
SELECT b.id FROM moz_bookmarks b
JOIN syncedItems s ON b.parent = s.id
),
descendants(id) AS (
SELECT b.id FROM moz_bookmarks b
WHERE b.guid IN (${requestedGuids
.map(guid => JSON.stringify(guid))
.join(",")})
UNION ALL
SELECT b.id FROM moz_bookmarks b
JOIN descendants d ON d.id = b.parent
)
SELECT b.guid, s.id NOT NULL AS syncable
FROM descendants d
JOIN moz_bookmarks b ON b.id = d.id
LEFT JOIN syncedItems s ON s.id = d.id
`);
return rows.map(row => {
let recordId = BookmarkSyncUtils.guidToRecordId(
row.getResultByName("guid")
);
let syncable = !!row.getResultByName("syncable");
return { recordId, syncable };
});
}
);
},
/**
* Migrates an array of `{ recordId, modified }` tuples from the old JSON-based
* tracker to the new sync change counter. `modified` is when the change was