Bug 1257533 - Optimize and add safety checks in Kinto updater. r=mgoodwin,MattN
- Skip changes from other bucket
- Leverage ETag to limit bandwidth
- Use setting for collection name
- Add safety check when server is failing. This also fixes
Bug 1259145.
MozReview-Commit-ID: 5YSVCrZirzQ
--- a/services/common/kinto-updater.js
+++ b/services/common/kinto-updater.js
@@ -7,74 +7,110 @@ this.EXPORTED_SYMBOLS = ["checkVersions"
const { classes: Cc, Constructor: CC, interfaces: Ci, utils: Cu } = Components;
Cu.import("resource://gre/modules/Services.jsm");
Cu.import("resource://gre/modules/Task.jsm");
Cu.importGlobalProperties(['fetch']);
const PREF_KINTO_CHANGES_PATH = "services.kinto.changes.path";
const PREF_KINTO_BASE = "services.kinto.base";
+const PREF_KINTO_BUCKET = "services.kinto.bucket";
const PREF_KINTO_LAST_UPDATE = "services.kinto.last_update_seconds";
+const PREF_KINTO_LAST_ETAG = "services.kinto.last_etag";
const PREF_KINTO_CLOCK_SKEW_SECONDS = "services.kinto.clock_skew_seconds";
+const PREF_KINTO_ONECRL_COLLECTION = "services.kinto.onecrl.collection";
const kintoClients = {
};
// This is called by the ping mechanism.
// returns a promise that rejects if something goes wrong
this.checkVersions = function() {
+
return Task.spawn(function *() {
// Fetch a versionInfo object that looks like:
// {"data":[
// {
// "host":"kinto-ota.dev.mozaws.net",
// "last_modified":1450717104423,
// "bucket":"blocklists",
// "collection":"certificates"
// }]}
// Right now, we only use the collection name and the last modified info
let kintoBase = Services.prefs.getCharPref(PREF_KINTO_BASE);
let changesEndpoint = kintoBase + Services.prefs.getCharPref(PREF_KINTO_CHANGES_PATH);
+ let blocklistsBucket = Services.prefs.getCharPref(PREF_KINTO_BUCKET);
- let response = yield fetch(changesEndpoint);
+ // Use ETag to obtain a `304 Not modified` when no change occurred.
+ const headers = {};
+ if (Services.prefs.prefHasUserValue(PREF_KINTO_LAST_ETAG)) {
+ const lastEtag = Services.prefs.getCharPref(PREF_KINTO_LAST_ETAG);
+ if (lastEtag) {
+ headers["If-None-Match"] = lastEtag;
+ }
+ }
+
+ let response = yield fetch(changesEndpoint, {headers});
+
+ let versionInfo;
+ // No changes since last time. Go on with empty list of changes.
+ if (response.status == 304) {
+ versionInfo = {data: []};
+ } else {
+ versionInfo = yield response.json();
+ }
+
+ // If the server is failing, the JSON response might not contain the
+ // expected data (e.g. error response - Bug 1259145)
+ if (!versionInfo.hasOwnProperty("data")) {
+ throw new Error("Polling for changes failed.");
+ }
// Record new update time and the difference between local and server time
let serverTimeMillis = Date.parse(response.headers.get("Date"));
let clockDifference = Math.abs(Date.now() - serverTimeMillis) / 1000;
+ Services.prefs.setIntPref(PREF_KINTO_CLOCK_SKEW_SECONDS, clockDifference);
Services.prefs.setIntPref(PREF_KINTO_LAST_UPDATE, serverTimeMillis / 1000);
- Services.prefs.setIntPref(PREF_KINTO_CLOCK_SKEW_SECONDS, clockDifference);
-
- let versionInfo = yield response.json();
let firstError;
for (let collectionInfo of versionInfo.data) {
+ // Skip changes that don't concern configured blocklist bucket.
+ if (collectionInfo.bucket != blocklistsBucket) {
+ continue;
+ }
+
let collection = collectionInfo.collection;
let kintoClient = kintoClients[collection];
if (kintoClient && kintoClient.maybeSync) {
let lastModified = 0;
if (collectionInfo.last_modified) {
- lastModified = collectionInfo.last_modified
+ lastModified = collectionInfo.last_modified;
}
try {
yield kintoClient.maybeSync(lastModified, serverTimeMillis);
} catch (e) {
if (!firstError) {
firstError = e;
}
}
}
}
if (firstError) {
// cause the promise to reject by throwing the first observed error
throw firstError;
}
+
+ // Save current Etag for next poll.
+ if (response.headers.has("ETag")) {
+ const currentEtag = response.headers.get("ETag");
+ Services.prefs.setCharPref(PREF_KINTO_LAST_ETAG, currentEtag);
+ }
});
};
// Add a kintoClient for testing purposes. Do not use for any other purpose
this.addTestKintoClient = function(name, kintoClient) {
kintoClients[name] = kintoClient;
};
// Add the various things that we know want updates
-kintoClients.certificates =
- Cu.import("resource://services-common/KintoCertificateBlocklist.js", {})
- .OneCRLClient;
+const KintoBlocklist = Cu.import("resource://services-common/KintoCertificateBlocklist.js", {});
+kintoClients[Services.prefs.getCharPref(PREF_KINTO_ONECRL_COLLECTION)] = KintoBlocklist.OneCRLClient;
--- a/services/common/tests/unit/test_kinto_updater.js
+++ b/services/common/tests/unit/test_kinto_updater.js
@@ -3,16 +3,17 @@
Cu.import("resource://services-common/kinto-updater.js")
Cu.import("resource://testing-common/httpd.js");
var server;
const PREF_KINTO_BASE = "services.kinto.base";
const PREF_LAST_UPDATE = "services.kinto.last_update_seconds";
+const PREF_LAST_ETAG = "services.kinto.last_etag";
const PREF_CLOCK_SKEW_SECONDS = "services.kinto.clock_skew_seconds";
// Check to ensure maybeSync is called with correct values when a changes
// document contains information on when a collection was last modified
add_task(function* test_check_maybeSync(){
const changesPath = "/v1/buckets/monitor/collections/changes/records";
// register a handler
@@ -26,45 +27,46 @@ add_task(function* test_check_maybeSync(
response.setStatusLine(null, sampled.status.status,
sampled.status.statusText);
// send the headers
for (let headerLine of sampled.sampleHeaders) {
let headerElements = headerLine.split(':');
response.setHeader(headerElements[0], headerElements[1].trimLeft());
}
- // set the
+ // set the server date
response.setHeader("Date", (new Date(2000)).toUTCString());
response.write(sampled.responseBody);
} catch (e) {
dump(`${e}\n`);
}
}
server.registerPathHandler(changesPath, handleResponse);
// set up prefs so the kinto updater talks to the test server
Services.prefs.setCharPref(PREF_KINTO_BASE,
`http://localhost:${server.identity.primaryPort}/v1`);
// set some initial values so we can check these are updated appropriately
- Services.prefs.setIntPref("services.kinto.last_update", 0);
- Services.prefs.setIntPref("services.kinto.clock_difference", 0);
+ Services.prefs.setIntPref(PREF_LAST_UPDATE, 0);
+ Services.prefs.setIntPref(PREF_CLOCK_SKEW_SECONDS, 0);
+ Services.prefs.clearUserPref(PREF_LAST_ETAG);
let startTime = Date.now();
+ let updater = Cu.import("resource://services-common/kinto-updater.js");
+
let syncPromise = new Promise(function(resolve, reject) {
- let updater = Cu.import("resource://services-common/kinto-updater.js");
// add a test kinto client that will respond to lastModified information
// for a collection called 'test-collection'
updater.addTestKintoClient("test-collection", {
- "maybeSync": function(lastModified, serverTime){
- // ensire the lastModified and serverTime values are as expected
+ maybeSync(lastModified, serverTime) {
do_check_eq(lastModified, 1000);
do_check_eq(serverTime, 2000);
resolve();
}
});
updater.checkVersions();
});
@@ -75,16 +77,54 @@ add_task(function* test_check_maybeSync(
do_check_eq(Services.prefs.getIntPref(PREF_LAST_UPDATE), 2);
// How does the clock difference look?
let endTime = Date.now();
let clockDifference = Services.prefs.getIntPref(PREF_CLOCK_SKEW_SECONDS);
// we previously set the serverTime to 2 (seconds past epoch)
do_check_eq(clockDifference <= endTime / 1000
&& clockDifference >= Math.floor(startTime / 1000) - 2, true);
+ // Last timestamp was saved. An ETag header value is a quoted string.
+ let lastEtag = Services.prefs.getCharPref(PREF_LAST_ETAG);
+ do_check_eq(lastEtag, "\"1100\"");
+
+
+ // Simulate a poll with up-to-date collection.
+ Services.prefs.setIntPref(PREF_LAST_UPDATE, 0);
+ // If server has no change, a 304 is received, maybeSync() is not called.
+ updater.addTestKintoClient("test-collection", {
+ maybeSync: () => {throw new Error("Should not be called");}
+ });
+ yield updater.checkVersions();
+ // Last update is overwritten
+ do_check_eq(Services.prefs.getIntPref(PREF_LAST_UPDATE), 2);
+
+
+ // Simulate a server error.
+ function simulateErrorResponse (request, response) {
+ response.setHeader("Date", (new Date(3000)).toUTCString());
+ response.setHeader("Content-Type", "application/json; charset=UTF-8");
+ response.write(JSON.stringify({
+ code: 503,
+ errno: 999,
+ error: "Service Unavailable",
+ }));
+ response.setStatusLine(null, 503, "Service Unavailable");
+ }
+ server.registerPathHandler(changesPath, simulateErrorResponse);
+ // checkVersions() fails with adequate error.
+ let error;
+ try {
+ yield updater.checkVersions();
+ } catch (e) {
+ error = e;
+ }
+ do_check_eq(error.message, "Polling for changes failed.");
+ // When an error occurs, last update was not overwritten (see Date header above).
+ do_check_eq(Services.prefs.getIntPref(PREF_LAST_UPDATE), 2);
});
function run_test() {
// Set up an HTTP Server
server = new HttpServer();
server.start(-1);
run_next_test();
@@ -94,17 +134,34 @@ function run_test() {
});
}
// get a response for a given request from sample data
function getSampleResponse(req, port) {
const responses = {
"GET:/v1/buckets/monitor/collections/changes/records?": {
"sampleHeaders": [
- "Content-Type: application/json; charset=UTF-8"
+ "Content-Type: application/json; charset=UTF-8",
+ "ETag: \"1100\""
],
"status": {status: 200, statusText: "OK"},
- "responseBody": JSON.stringify({"data":[{"host":"localhost","last_modified":1000,"bucket":"blocklists","id":"330a0c5f-fadf-ff0b-40c8-4eb0d924ff6a","collection":"test-collection"}]})
+ "responseBody": JSON.stringify({"data": [{
+ "host": "localhost",
+ "last_modified": 1100,
+ "bucket": "blocklists:aurora",
+ "id": "330a0c5f-fadf-ff0b-40c8-4eb0d924ff6a",
+ "collection": "test-collection"
+ }, {
+ "host": "localhost",
+ "last_modified": 1000,
+ "bucket": "blocklists",
+ "id": "254cbb9e-6888-4d9f-8e60-58b74faa8778",
+ "collection": "test-collection"
+ }]})
}
};
+
+ if (req.hasHeader("if-none-match") && req.getHeader("if-none-match", "") == "\"1100\"")
+ return {sampleHeaders: [], status: {status: 304, statusText: "Not Modified"}, responseBody: ""};
+
return responses[`${req.method}:${req.path}?${req.queryString}`] ||
responses[req.method];
}