We need a client UI for the quota system (bug 577590). This will consist of * a warning when the server sends the X-Weave-Quota header (which means we're approaching quota), * an error message when the server refuses a write because we're over quota, * an overview dialog that lets the user review how much space each collection occupies and subsequently disable individual engines (which will trigger immediate DELETEs). Both the warning and the error message should point the user to that quota overview dialog.

Not forget that user can want to disable engine just for one client, keppeng it work for other.

(In reply to comment #1) > Not forget that user can want to disable engine just for one client, keppeng it > work for other. That's an explicit non-goal. See bug 578671.

Recognize quota warnings from server, implement API calls to retrieve quota information.

UI: Implement quota notifications and View Quota dialog

To test this, I used Tarek's Python server. It required a small patch because the info/collection_usage API call was mispelt. Then enable quotas in development.ini and set quota level to a ridiculously low amount (<=1 MB will always trigger the warning for instance): storage.use_quota = True storage.quota_size = 1024

Comment on attachment 473066 [details] [diff] [review] Part 1 (v1): Implement quota parts of Sync API >diff --git a/services/sync/modules/resource.js b/services/sync/modules/resource.js >+ if (success && headers["x-weave-quota-remaining"]) >+ Observers.notify("weave:service:quota:remaining", >+ parseInt(headers["x-weave-quota"], 10)); x-weave-quota-remaining here, I assume? >diff --git a/services/sync/modules/service.js b/services/sync/modules/service.js >--- a/services/sync/modules/service.js >+++ b/services/sync/modules/service.js >@@ -207,7 +207,7 @@ > return; > > let storageAPI = this.clusterURL + Svc.Prefs.get("storageAPI") + "/"; >- let userBase = storageAPI + this.username + "/"; >+ let userBase = this.userBaseURL = storageAPI + this.username + "/"; > this._log.debug("Caching URLs under storage user base: " + userBase); why still have userBase if we're adding this.userBaseURL? >@@ -1571,8 +1571,11 @@ > if (Utils.checkStatus(resp.status, null, [500, [502, 504]])) { > Status.enforceBackoff = true; > if (resp.status == 503 && resp.headers["retry-after"]) >- Svc.Obs.notify("weave:service:backoff:interval", parseInt(resp.headers["retry-after"], 10)); >+ Svc.Obs.notify("weave:service:backoff:interval", >+ parseInt(resp.headers["retry-after"], 10)); > } >+ if (resp.status == 400 && resp == "14") >+ Status.sync = OVER_QUOTA; we should get relevant consts into constants.js, no magic numbers! > // Load Weave on the first time this file is loaded >diff --git a/services/sync/tests/unit/test_resource.js b/services/sync/tests/unit/test_resource.js >--- a/services/sync/tests/unit/test_resource.js >+++ b/services/sync/tests/unit/test_resource.js >@@ -99,6 +99,19 @@ > response.bodyOutputStream.write(body, body.length); > } > >+function server_quota_notice(request, response) { >+ let body = "You're approaching quota."; >+ response.setHeader("X-Weave-Quota-Remaining", '1048576', false); >+ response.setStatusLine(request.httpVersion, 200, "OK"); >+ response.bodyOutputStream.write(body, body.length); >+} >+ >+function server_quota_error(request, response) { >+ let body = "14"; >+ response.setHeader("X-Weave-Quota-Remaining", '-1024', false); >+ response.setStatusLine(request.httpVersion, 400, "OK"); >+ response.bodyOutputStream.write(body, body.length); >+} > > function server_headers(metadata, response) { > let ignore_headers = ["host", "user-agent", "accept", "accept-language", >@@ -131,17 +144,19 @@ > logger = Log4Moz.repository.getLogger('Test'); > Log4Moz.repository.rootLogger.addAppender(new Log4Moz.DumpAppender()); > >- let server = new nsHttpServer(); >- server.registerPathHandler("/open", server_open); >- server.registerPathHandler("/protected", server_protected); >- server.registerPathHandler("/404", server_404); >- server.registerPathHandler("/upload", server_upload); >- server.registerPathHandler("/delete", server_delete); >- server.registerPathHandler("/json", server_json); >- server.registerPathHandler("/timestamp", server_timestamp); >- server.registerPathHandler("/headers", server_headers); >- server.registerPathHandler("/backoff", server_backoff); >- server.start(8080); >+ let server = httpd_setup({ >+ "/open": server_open, >+ "/protected": server_protected, >+ "/404": server_404, >+ "/upload": server_upload, >+ "/delete": server_delete, >+ "/json": server_json, >+ "/timestamp": server_timestamp, >+ "/headers": server_headers, >+ "/backoff": server_backoff, >+ "/quota-notice": server_quota_notice, >+ "/quota-error": server_quota_error >+ }); > > Utils.prefs.setIntPref("network.numRetries", 1); // speed up test > >@@ -181,7 +196,7 @@ > let res2 = new Resource("http://localhost:8080/protected"); > content = res2.get(); > do_check_true(did401); >- do_check_eq(content, "This path exists and is protected - failed") >+ do_check_eq(content, "This path exists and is protected - failed"); > do_check_eq(content.status, 401); > do_check_false(content.success); > >@@ -335,6 +350,25 @@ > content = res10.get(); > do_check_eq(backoffInterval, 600); > >+ >+ _("X-Weave-Quota header notifies observer on successful requests."); now I'm really confused about which header is which. >+ let quotaValue; >+ function onQuota(subject, data) { >+ quotaValue = subject; >+ } >+ Observers.add("weave:service:quota:remaining", onQuota); >diff --git a/services/sync/tests/unit/test_service_quota.js b/services/sync/tests/unit/test_service_quota.js >new file mode 100644 >--- /dev/null >+++ b/services/sync/tests/unit/test_service_quota.js >@@ -0,0 +1,50 @@ >+Cu.import("resource://services-sync/service.js"); >+Cu.import("resource://services-sync/util.js"); >+ >+function send(body) { >+ return function(request, response) { >+ response.setStatusLine(request.httpVersion, 200, "OK"); >+ response.bodyOutputStream.write(body, body.length); >+ }; >+} >+ >+function run_test() { >+ let collection_usage = {steam: 65.11328, >+ petrol: 82.488281, >+ diesel: 2.25488281}; >+ let quota = [2169.65136, 8192]; >+ >+ do_test_pending(); >+ let server = httpd_setup({ >+ "/1.0/johndoe/info/collection_usage": send(JSON.stringify(collection_usage)), >+ "/1.0/johndoe/info/quota": send(JSON.stringify(quota)), >+ "/1.0/janedoe/info/collection_usage": send("gargabe"), >+ "/1.0/janedoe/info/quota": send("more gargabe") >+ }); garbage? :D looks okay, with issues addressed.

(In reply to comment #6) Ugh, sorry for the half bit rotten patch. It should be X-Weave-Quota-Remaining everywhere. > why still have userBase if we're adding this.userBaseURL? No need, just shorter ;)

Addressed review comments: * Made "14" response code a constant * Fix resource.js (X-Weave-Quota -> X-Weave-Quota-Remaining) * Avoid definition of userBase * Fix typo

Also catch errors from the Clients engine. It's the first to sync and thus quite likely to run into quota issues.

Reorder error checks to actually catch OVER_QUOTA errors.

Comment on attachment 473608 [details] [diff] [review] Part 2 (v2): Quota notifications and dialog +quota.usage.label = You are using %S%% (%S) of your allowed %S. and +quota.freeup.label = This will free up %S. should get a localization note explaining the parameters that get inserted. Also, it'd be good for usage.label to use ordered variable references right away, i.e., %1$S and use those in the comment, too.

Right, I should have called those out, thanks Axel!

Address Axel's comments: Explain parameters in strings and use explicit ordering for multiple parameters.

Comment on attachment 473835 [details] [diff] [review] Part 2 (v3): Quota notifications and dialog There are a few candidates here that I'll need to look at with my head on, more tomorrow.

Correct typo in a comment.

Test cosmetics

Be more tolerant towards servers that implement the quota API but don't support quotas (as the prod servers do right now apparently...)

Comment on attachment 473915 [details] [diff] [review] Part 2 (v4): Quota notifications and dialog r- on the usage of DownloadUtils.convertByteUnits, that should be used as in other places in downloads, aka, both number and metric need to be independently placed. The localization note should reference the download manager sizes, too. Same for quota.usagePercentage.label and quota.freeup.label. Also, the ", " for quota.removal.label needs to be localizable.

Make the position of numeric values and units localizable and add localizer note that their definitions come from the download manager. Also make gap fillers localizable.

Comment on attachment 474166 [details] [diff] [review] Part 2 (v5): Quota notifications and dialog That's looking great, thanks.

Part 1: http://hg.mozilla.org/services/fx-sync/rev/886d90e528ff Part 2: http://hg.mozilla.org/services/fx-sync/rev/d5b925f006ef

verified with recent nightly minefield builds though currently doesn't seem to working quite right

Yes, because quota support isn't deployed on the server yet.