diff --git a/services/settings/RemoteSettingsClient.jsm b/services/settings/RemoteSettingsClient.jsm index 8550295ed15d..cec244531f63 100644 --- a/services/settings/RemoteSettingsClient.jsm +++ b/services/settings/RemoteSettingsClient.jsm @@ -325,32 +325,6 @@ class RemoteSettingsClient extends EventEmitter { } } - // If the data is up to date, there's no need to sync. We still need - // to record the fact that a check happened. - if (expectedTimestamp <= collectionLastModified) { - console.debug(`${this.identifier} local data is up-to-date`); - // If the data is up-to-date but don't have metadata (records loaded from dump), - // we fetch them and validate the signature immediately. - if (this.verifySignature && ObjectUtils.isEmpty(await kintoCollection.metadata())) { - console.debug(`${this.identifier} verify signature of local data`); - let allData = importedFromDump; - if (importedFromDump.length == 0) { - // If dump was imported at some other point (eg. `.get()`), list local DB. - ({ data: allData } = await kintoCollection.list({ order: "" })); - } - const localRecords = allData.map(r => kintoCollection.cleanLocalFields(r)); - const metadata = await kintoCollection.pullMetadata(this.httpClient()); - if (this.verifySignature) { - await this._validateCollectionSignature([], - collectionLastModified, - metadata, - { localRecords }); - } - } - reportStatus = UptakeTelemetry.STATUS.UP_TO_DATE; - return; - } - // If signature verification is enabled, then add a synchronization hook // for incoming changes that validates the signature. if (this.verifySignature) { @@ -373,13 +347,44 @@ class RemoteSettingsClient extends EventEmitter { let syncResult; try { - // Fetch changes from server, and make sure we overwrite local data. - const strategy = Kinto.syncStrategy.PULL_ONLY; - syncResult = await kintoCollection.sync({ remote: gServerURL, strategy, expectedTimestamp }); - if (!syncResult.ok) { - // With PULL_ONLY, there cannot be any conflicts, but don't silent it anyway. - throw new Error("Sync failed"); - } + // Is local timestamp up to date with the server? + if (expectedTimestamp <= collectionLastModified) { + console.debug(`${this.identifier} local data is up-to-date`); + reportStatus = UptakeTelemetry.STATUS.UP_TO_DATE; + + // If the data is up-to-date but don't have metadata (records loaded from dump), + // we fetch them and validate the signature immediately. + if (this.verifySignature && ObjectUtils.isEmpty(await kintoCollection.metadata())) { + console.debug(`${this.identifier} pull collection metadata`); + const metadata = await kintoCollection.pullMetadata(this.httpClient()); + // We don't bother validating the signature if the dump was just loaded. We do + // if the dump was loaded at some other point (eg. from .get()). + if (this.verifySignature && importedFromDump.length == 0) { + console.debug(`${this.identifier} verify signature of local data`); + const { data: allData } = await kintoCollection.list({ order: "" }); + const localRecords = allData.map(r => kintoCollection.cleanLocalFields(r)); + await this._validateCollectionSignature([], + collectionLastModified, + metadata, + { localRecords }); + } + } + + // Since the data is up-to-date, if we didn't load any dump then we're done here. + if (importedFromDump.length == 0) { + return; + } + // Otherwise we want to continue with sending the sync event to notify about the created records. + syncResult = { current: importedFromDump, created: importedFromDump, updated: [], deleted: [] }; + } else { + // Local data is outdated. + // Fetch changes from server, and make sure we overwrite local data. + const strategy = Kinto.syncStrategy.PULL_ONLY; + syncResult = await kintoCollection.sync({ remote: gServerURL, strategy, expectedTimestamp }); + if (!syncResult.ok) { + // With PULL_ONLY, there cannot be any conflicts, but don't silent it anyway. + throw new Error("Sync failed"); + } // The records imported from the dump should be considered as "created" for the // listeners. const importedById = importedFromDump.reduce((acc, r) => { @@ -395,6 +400,7 @@ class RemoteSettingsClient extends EventEmitter { } }); syncResult.add("created", Array.from(importedById.values())); + } } catch (e) { if (e instanceof RemoteSettingsClient.InvalidSignatureError) { // Signature verification failed during synchronization. diff --git a/services/settings/test/unit/test_remote_settings.js b/services/settings/test/unit/test_remote_settings.js index ae4be860e528..45ec315b61c2 100644 --- a/services/settings/test/unit/test_remote_settings.js +++ b/services/settings/test/unit/test_remote_settings.js @@ -112,6 +112,28 @@ add_task(async function test_records_from_dump_are_listed_as_created_in_event() }); add_task(clear_state); +add_task(async function test_sync_event_is_sent_even_if_up_to_date() { + if (IS_ANDROID) { + // Skip test: we don't ship remote settings dumps on Android (see package-manifest). + return; + } + const startHistogram = getUptakeTelemetrySnapshot(clientWithDump.identifier); + let received; + clientWithDump.on("sync", ({ data }) => received = data); + // Use a timestamp inferior to latest record in dump. + const timestamp = 1000000000000; // Sun Sep 09 2001 + + await clientWithDump.maybeSync(timestamp); + + ok(received.current.length > 0, "Dump records are listed as created"); + equal(received.current.length, received.created.length); + + const endHistogram = getUptakeTelemetrySnapshot(clientWithDump.identifier); + const expectedIncrements = { [UptakeTelemetry.STATUS.UP_TO_DATE]: 1 }; + checkUptakeTelemetry(startHistogram, endHistogram, expectedIncrements); +}); +add_task(clear_state); + add_task(async function test_records_can_have_local_fields() { const c = RemoteSettings("with-local-fields", { localFields: [ "accepted" ]}); c.verifySignature = false;