use sqlite instead of racy json file for banners

tools/catalog-remote.js

@@ -426,11 +426,13 @@ _.extend(Db.prototype, {
 });
 
 
-var Table = function (name, jsonFields) {
+var Table = function (name, jsonFields, options) {
   var self = this;
+  options = options || {};
 
   self.name = name;
   self.jsonFields = jsonFields;
+  self.noContentColumn = options.noContentColumn;
 
   self._buildStatements();
 };
@@ -439,13 +441,15 @@ _.extend(Table.prototype, {
   _buildStatements: function () {
     var self = this;
 
-    var queryParams = self._generateQuestionMarks(self.jsonFields.length + 1);
+    var queryParams = self._generateQuestionMarks(
+      self.jsonFields.length + (self.noContentColumn ? 0 : 1));
     self._selectQuery = "SELECT * FROM " + self.name + " WHERE _id=?";
     self._insertQuery = "INSERT INTO " + self.name + " VALUES " + queryParams;
     self._deleteQuery = "DELETE FROM " + self.name + " WHERE _id=?";
   },
 
-  //Generate a string of the form (?, ?) where the n is the number of question mark
+  // Generate a string of the form (?, ?) where the n is the number of question
+  // mark.
   _generateQuestionMarks: function (n) {
     return "(" + _.times(n, function () { return "?" }).join(",") + ")";
   },
@@ -481,7 +485,9 @@ _.extend(Table.prototype, {
       _.each(self.jsonFields, function (jsonField) {
         row.push(o[jsonField]);
       });
-      row.push(JSON.stringify(o));
+      if (! self.noContentColumn) {
+        row.push(JSON.stringify(o));
+      }
       txn.execute(self._insertQuery, row);
     });
   },
@@ -493,13 +499,15 @@ _.extend(Table.prototype, {
     for (var i = 0; i < self.jsonFields.length; i++) {
       var jsonField = self.jsonFields[i];
       var sqlColumn = jsonField;
-      if (i != 0) sql += ",";
+      if (i != 0) sql += ", ";
       sql += sqlColumn + " STRING";
       if (sqlColumn === '_id') {
         sql += " PRIMARY KEY";
       }
     }
-    sql += ", content STRING";
+    if (! self.noContentColumn) {
+      sql += ", content STRING";
+    }
     sql += ")";
     txn.execute(sql);
 
@@ -692,14 +700,19 @@ _.extend(RemoteCatalog.prototype, {
     self.tablePackages = new Table('packages', ['name', '_id']);
     self.tableSyncToken = new Table('syncToken', ['_id']);
     self.tableMetadata = new Table('metadata', ['_id']);
+    self.tableBannersShown = new Table(
+      'bannersShown', ['_id', 'lastShown'], { noContentColumn: true });
 
-    self.allTables = [ self.tableVersions,
+    self.allTables = [
+      self.tableVersions,
       self.tableBuilds,
       self.tableReleaseTracks,
       self.tableReleaseVersions,
       self.tablePackages,
       self.tableSyncToken,
-      self.tableMetadata ]
+      self.tableMetadata,
+      self.tableBannersShown
+    ];
     return self.db.runInTransaction(function(txn) {
       _.each(self.allTables, function (table) {
         table.createTable(txn);
@@ -887,6 +900,30 @@ _.extend(RemoteCatalog.prototype, {
     var self = this;
     value._id = key;
     self.tableMetadata.upsert(txn, [value]);
+  },
+
+  shouldShowBanner: function (releaseName, bannerDate) {
+    var self = this;
+    var row = self.db.runInTransaction(function (txn) {
+      return self.tableBannersShown.find(txn, releaseName);
+    });
+    // We've never printed a banner for this release.
+    if (! row)
+      return true;
+    var lastShown = new Date(row.lastShown);
+    return lastShown < bannerDate;
+  },
+
+  setBannerShownDate: function (releaseName, bannerShownDate) {
+    var self = this;
+    self.db.runInTransaction(function (txn) {
+      self.tableBannersShown.upsert(txn, [{
+        _id: releaseName,
+        // XXX For now every column is a string, but this should probably change
+        // to a timestamp with time zone or whatever.
+        lastShown: JSON.stringify(bannerShownDate)
+      }]);
+    });
   }
 });
 

tools/config.js

@@ -245,12 +245,6 @@ _.extend(exports, {
     }
   },
 
-  // XXX this should just be moved into an extra table in sqlite
-  getBannersShownFilename: function() {
-    return path.join(tropohouse.default.root,
-                     "package-metadata", "v2.0.1", "banners-shown.json");
-  },
-
   // Return the domain name of the current Meteor Accounts server in
   // use. This is used as a key for storing your Meteor Accounts
   // login token.

tools/updater.js

@@ -87,38 +87,14 @@ var maybeShowBanners = function () {
 
   var banner = releaseData.banner;
   if (banner) {
-    var bannersShown = {};
-
-    var bannerFilename = config.getBannersShownFilename();
-    try {
-      bannersShown = JSON.parse(fs.readFileSync(bannerFilename));
-    } catch (e) {
-      // ... ignore
-    }
-
-    var shouldShowBanner = false;
-    if (_.has(bannersShown, release.current.name)) {
-      // XXX use EJSON so that we can just have Dates
-      var lastShown = new Date(bannersShown[release.current.name]);
-      var bannerUpdated = banner.lastUpdated ?
-            new Date(banner.lastUpdated) : new Date;
-      // XXX should the default really be "once ever" and not eg "once a week"?
-      if (lastShown < bannerUpdated) {
-        shouldShowBanner = true;
-      }
-    } else {
-      shouldShowBanner = true;
-    }
-
-    if (shouldShowBanner) {
+    var bannerDate =
+          banner.lastUpdated ? new Date(banner.lastUpdated) : new Date;
+    if (catalog.official.shouldShowBanner(release.current.name, bannerDate)) {
       // This banner is new; print it!
       runLog.log("");
       runLog.log(banner.text);
       runLog.log("");
-      bannersShown[release.current.name] = new Date;
-      // XXX ick slightly racy. we should just add this to sqlite!
-      files.mkdir_p(path.dirname(bannerFilename));
-      fs.writeFileSync(bannerFilename, JSON.stringify(bannersShown, null, 2));
+      catalog.official.setBannerShownDate(release.current.name, bannerDate);
       return;
     }
   }