Issue 537 - Mark first few downloads

lib/downloader.js

Index: lib/downloader.js
===================================================================
--- a/lib/downloader.js
+++ b/lib/downloader.js
@@ -197,7 +197,8 @@
         "&applicationVersion=" + encodeURIComponent(applicationVersion) +
         "&platform=" + encodeURIComponent(platform) +
         "&platformVersion=" + encodeURIComponent(platformVersion) +
-        "&lastVersion=" + encodeURIComponent(downloadable.lastVersion);
+        "&lastVersion=" + encodeURIComponent(downloadable.lastVersion) +
+        "&downloadCount=" + encodeURIComponent(downloadable.downloadCount);
     return url;
   },
 
@@ -291,6 +292,10 @@
         return;
       }
 
+      // We limit the download count to 5+ to keep the request anonymized
+      if (!isNaN(downloadable.downloadCount) && ++downloadable.downloadCount > 5)
+        downloadable.downloadCount = "5+";

[Wladimir Palant, 2014/11/10 19:18:15]

Storing numbers and strings in the same variable seems wrong. In particular,
this will turn into a mess once you start incrementing strings. Why not use
(downloadCount > 5 ? "5+" : downloadCount) when setting request parameters?

[Thomas Greiner, 2014/11/11 11:54:46]

What I wanted to avoid is (a) stopping the counter at 6 which could cause
problems in case we need to increase it at some point and (b) have the counter
go beyond 5 even on the client-side.
If you don't consider (b) to be a problem then I don't mind changing it
accordingly.

[Wladimir Palant, 2014/11/11 16:07:00]

Yes, I was also considering whether (b) is a problem - but I don't think it is,
as long as we never send the raw number over the wire. It also makes sense in
case we decide to change the threshold later.

[Thomas Greiner, 2014/11/11 17:17:18]

Done.

+
       this.onDownloadSuccess(downloadable, request.responseText, errorCallback, function redirectCallback(url)
       {
         if (redirects >= this.maxRedirects)
@@ -378,4 +383,10 @@
    * @type Integer
    */
   hardExpiration: 0,
+  
+  /**
+   * Number indicating how often the object was downloaded.
+   * @type Integer
+   */
+  downloadCount: 1,

[Thomas Greiner, 2014/11/10 18:14:55]

This number is always one higher than the number of successful downloads which
is why I described it as an "indicator" (also see subscriptionClasses.js). The
reason for that is that I wanted to avoid having to increment the counter when
making the request due to duplicated logic regarding the "5+" case.

Is this too confusing?

[Wladimir Palant, 2014/11/10 19:18:15]

Incrementing after success seems ok, but why not start with 0 here? Then this
number will actually reflect the number of successful downloads.

[Thomas Greiner, 2014/11/11 11:54:46]

If this is set to 0 then the number we send is 0-based (0-4+) instead of 1-based
(1-5+) as agreed on. I don't have any strong preferences towards either.

[Wladimir Palant, 2014/11/11 16:07:00]

I don't think we actually agreed on the exact meaning of this parameter but
0-based (meaning not counting the current download) makes much more sense IMHO.

[Thomas Greiner, 2014/11/11 17:17:18]

Done.

 };