Skip to content
This repository has been archived by the owner on Sep 2, 2021. It is now read-only.

Commit

Permalink
Browse files Browse the repository at this point in the history
- added a route to acquire a csrf token to post the extension downloa…
…d data to the local web app

- make fewer calls to addDownloadDataToPackage and prevent the registry from being saved for every single download data update (this seemed to have cause the out of memory condition)
  • Loading branch information
ingorichter committed May 12, 2014
1 parent b50926d commit e3a9e47
Show file tree
Hide file tree
Showing 12 changed files with 194 additions and 138 deletions.
1 change: 1 addition & 0 deletions .gitignore
@@ -1,3 +1,4 @@
config/
node_modules/
npm-debug.log
/coverage/
1 change: 1 addition & 0 deletions app.js
Expand Up @@ -128,6 +128,7 @@ app.configure(function () {
// Must come before router (so locals are exposed properly) but after the CSRF middleware
// (so _csrf is set).
res.locals.csrfToken = req.csrfToken();

next();
});
app.use(app.router);
Expand Down
46 changes: 34 additions & 12 deletions downloadStats/downloadStatsUpdater.js
Expand Up @@ -41,6 +41,7 @@ programArgs
.option('-e, --extract', 'Extract Extension download data from downloaded logfiles')
.option('-t, --tempFolder <path>', 'Path to temp folder (makes it easier to inspect logfiles)')
.option('-p, --progress [true|false]', 'Print progress information')
.option('-u, --update <path>', 'Update the extension registry with download data from <path>')
.option('-v, --verbose [true|false]', 'Increase the level of output')
.parse(process.argv);

Expand Down Expand Up @@ -129,25 +130,44 @@ function extractExtensionDownloadData(progress) {
return deferred.promise;
}

function updateExtensionDownloadData(datafile, progress) {
var deferred = Promise.defer();

// posting works only from localhost
var url = protocol + "://localhost:" + httpPort;

var client = request.newClient(url);
client.get('/csrfTokenForUpload', function (err, res, body) {
if (!err) {
client.sendFile("/stats?_csrf=" + body.csrf, path.resolve(datafile), null, function (err, res, body) {
if (err) {
console.error(err);
deferred.reject(err);
} else {
log("File uploaded");
deferred.resolve();
}
});
} else {
console.error(err);
deferred.reject(err);
}
});

return deferred.promise;
}

function doItAll(progress) {
downloadLogFiles(progress).then(function () {
extractExtensionDownloadData(progress).then(function (downloadStats) {
writeFile(DOWNLOAD_STATS_FILENAME, JSON.stringify(downloadStats)).then(function () {
// posting works only from localhost
var url = protocol + "://localhost:" + httpPort;

var client = request.newClient(url);
client.sendFile("/stats", path.resolve(__dirname, DOWNLOAD_STATS_FILENAME), null, function (err, res, body) {
if (err) {
console.error(err);
} else {
log("File uploaded");
var datafile = path.resolve(__dirname, DOWNLOAD_STATS_FILENAME);
updateExtensionDownloadData(datafile, progress).then(function () {
if (!config["debug.keepTempFolder"]) {
fs.rmdirSync(tempFolder);
}
});

if (!config["debug.keepTempFolder"]) {
fs.rmdirSync(tempFolder);
}
});
});
});
Expand All @@ -158,6 +178,8 @@ if (programArgs.download) {
downloadLogFiles(programArgs.progress);
} else if (programArgs.extract) {
extractExtensionDownloadData(programArgs.progress);
} else if (programArgs.update) {
updateExtensionDownloadData(programArgs.update, programArgs.progress);
} else {
doItAll(programArgs.progress);
}
7 changes: 4 additions & 3 deletions downloadStats/logfileProcessor.js
Expand Up @@ -45,10 +45,10 @@ function LogfileProcessor(config) {
var accessKeyId = config["aws.accesskey"],
secretAccessKey = config["aws.secretkey"];

this.bucketName = config["s3.bucket"];
this.bucketName = config["s3.logBucket"];

if (!accessKeyId || !secretAccessKey || !this.bucketName) {
throw new Error("Configuration error: aws.accesskey, aws.secretkey, or s3.bucket missing");
throw new Error("Configuration error: aws.accesskey, aws.secretkey, or s3.logBucket missing");
}

AWS.config.update({
Expand Down Expand Up @@ -269,11 +269,12 @@ LogfileProcessor.prototype = {
if (matchResult) {
var uri = matchResult[8],
date = matchResult[3],
httpStatusCode = matchResult[12],
downloadDate = formatDownloadDate(date);

// // we are only interested in the Extension zip files
var m = uri.match(/(\S+)\/(\S+)\-(.*)\.zip/);
if (m) {
if (m && httpStatusCode === "200") {
var extensionName = m[1],
version = m[3];

Expand Down
25 changes: 25 additions & 0 deletions lib/filestorage.js
Expand Up @@ -43,6 +43,11 @@ function FileStorage(config) {
this.directory = config.directory;
this.extensionsDirectory = this.directory;
this.registryFile = path.join(config.directory, "registry.json");

// We use these flags to make sure that the latest registry is stored and that
// we only have one call to save the registry in progress at a time.
this._registrySaveInProgress = false;
this._pendingRegistry = null;
}

FileStorage.prototype = {
Expand All @@ -52,10 +57,30 @@ FileStorage.prototype = {
* @param <Object> updated registry information to save
*/
saveRegistry: function (updatedRegistry) {
// If we're already saving, then we hang on to the registry to save
// and we'll take care of it later.
if (this._registrySaveInProgress) {
this._pendingRegistry = updatedRegistry;
return;
}

this._registrySaveInProgress = true;

var self = this;

fs.writeFile(this.registryFile, JSON.stringify(updatedRegistry), "utf8", function (err) {
if (err) {
console.error(err);
}

self._registrySaveInProgress = false;

// If there was a pending registry save, now is the time to save it again.
var pendingRegistry = self._pendingRegistry;
self._pendingRegistry = null;
if (pendingRegistry !== null) {
self.saveRegistry(pendingRegistry);
}
});
},

Expand Down
18 changes: 13 additions & 5 deletions lib/repository.js
Expand Up @@ -143,16 +143,24 @@ function updateRecentDownloadsForPackage(name, newRecentDownloadDatapoints) {
return updated;
}

function addDownloadDataToPackage(name, version, newDownloads, recentDownloads) {
//function addDownloadDataToPackage(name, version, newDownloads, recentDownloads) {
/*
* newVersionDownloads = { ""0.3.0": 6,
"0.3.1": 276,
"0.4.3": 218 };
*/
function addDownloadDataToPackage(name, newVersionDownloads, recentDownloads) {
if (registry[name]) {
var updated = false;

logger.debug("Extension package with name " + name + " found");
var packageVersions = registry[name].versions;

packageVersions.forEach(function (versionInfo) {
// we found the version
if (versionInfo.version === version) {
Object.keys(newVersionDownloads).forEach(function (version) {
var versionInfo = _.find(packageVersions, {"version": version});
if (versionInfo) {
var newDownloads = newVersionDownloads[version];

if (!versionInfo.downloads) {
versionInfo.downloads = newDownloads;
} else {
Expand All @@ -169,7 +177,7 @@ function addDownloadDataToPackage(name, version, newDownloads, recentDownloads)
updated = true;
}
});

var recentDownloadsUpdated = updateRecentDownloadsForPackage(name, recentDownloads);

// save changes to registry if there were any updates
Expand Down
27 changes: 17 additions & 10 deletions lib/routes.js
Expand Up @@ -477,16 +477,10 @@ function _stats(req, res) {
// read the uploaded JSON data
var obj = JSON.parse(fs.readFileSync(req.files.file.path));

_.each(obj, function (n, extensionName) {
_.each(obj[extensionName].downloads, function (versions) {
// iterate over all updatable versions
_.each(Object.keys(versions), function (version) {
repository.addDownloadDataToPackage(extensionName,
version,
versions[version],
obj[extensionName].downloads.recent);
});
});
Object.keys(obj).forEach(function (extensionName, num) {
repository.addDownloadDataToPackage(extensionName,
obj[extensionName].downloads.versions,
obj[extensionName].downloads.recent);
});

res.send(202); // indicate that everything is alright
Expand All @@ -496,6 +490,18 @@ function _stats(req, res) {
}
}

function _csrfTokenForUpload(req, res) {
if (req.ip === "127.0.0.1" && req.host === "localhost") {
res.json({
"csrf": res.locals.csrfToken
});
} else {
res.json({
"csrf": undefined
});
}
}

//////////////
// Route setup
//////////////
Expand Down Expand Up @@ -543,6 +549,7 @@ function setup(app, configObj) {
app.post("/package/:name/changeRequirements", _changeRequirements);

app.post("/stats", _stats);
app.get("/csrfTokenForUpload", _csrfTokenForUpload);
}

exports.setup = setup;
13 changes: 11 additions & 2 deletions spec/logfileProcessor.spec.js
Expand Up @@ -37,15 +37,15 @@ describe("LogfileProcessor", function () {
beforeEach(function () {
config["aws.accesskey"] = "fake";
config["aws.secretkey"] = "secretKey";
config["s3.bucket"] = "no_bucket_name";
config["s3.logBucket"] = "no_bucket_name";
});

describe("Parse Logfiles", function () {
it("should throw an exception when configure without AWS credentials", function (done) {
try {
var lfp = new logfileProcessor.LogfileProcessor({});
} catch (e) {
expect(e.toString()).toBe("Error: Configuration error: aws.accesskey, aws.secretkey, or s3.bucket missing");
expect(e.toString()).toBe("Error: Configuration error: aws.accesskey, aws.secretkey, or s3.logBucket missing");
done();
}
});
Expand Down Expand Up @@ -79,6 +79,15 @@ describe("LogfileProcessor", function () {
done();
});
});

it("should return no information for unsuccessful get request http status != 200", function (done) {
var lfp = new logfileProcessor.LogfileProcessor(config);
lfp.extractDownloadStats(path.join(testLogfileDirectory, "failed-get-request")).then(function (downloadStats) {
expect(downloadStats).toEqual({});

done();
});
});
});

describe("Create recent download stats", function () {
Expand Down
9 changes: 4 additions & 5 deletions spec/repository.spec.js
Expand Up @@ -426,17 +426,16 @@ describe("Add download data", function () {
});

it("should add the download numbers to the 0.3.0 extension version and update the download total", function () {
repository.addDownloadDataToPackage("snippets-extension", "0.3.0", 5);
repository.addDownloadDataToPackage("snippets-extension", {"0.3.0" : 5}, {"20130805": 5});

var expectedRegistry = JSON.parse('{"snippets-extension":{"metadata":{"name":"snippets-extension","title":"Brackets Snippets","homepage":"https://github.com/testuser/brackets-snippets","author":{"name":"Testuser"},"version":"1.0.0","engines":{"brackets":">=0.24"},"description":"A simple brackets snippets extension."},"owner":"irichter","versions":[{"version":"0.2.0","published":"2014-01-10T17:27:25.996Z","brackets":">=0.24"},{"version":"0.3.0","published":"2014-01-10T17:27:25.996Z","brackets":">=0.24","downloads":5}],"totalDownloads":5}}');
var expectedRegistry = JSON.parse('{"snippets-extension":{"metadata":{"name":"snippets-extension","title":"Brackets Snippets","homepage":"https://github.com/testuser/brackets-snippets","author":{"name":"Testuser"},"version":"1.0.0","engines":{"brackets":">=0.24"},"description":"A simple brackets snippets extension."},"owner":"irichter","versions":[{"version":"0.2.0","published":"2014-01-10T17:27:25.996Z","brackets":">=0.24"},{"version":"0.3.0","published":"2014-01-10T17:27:25.996Z","brackets":">=0.24","downloads":5}],"totalDownloads":5,"recent":{"20130805":5}}}');
expect(JSON.stringify(repository.getRegistry())).toBe(JSON.stringify(expectedRegistry));
});

it("should add the download numbers to the 0.2.0 extension version and update the download total", function () {
repository.addDownloadDataToPackage("snippets-extension", "0.3.0", 5);
repository.addDownloadDataToPackage("snippets-extension", "0.2.0", 3);
repository.addDownloadDataToPackage("snippets-extension", {"0.3.0": 5, "0.2.0": 3}, {"20130805": 8});

var expectedRegistry = JSON.parse('{"snippets-extension":{"metadata":{"name":"snippets-extension","title":"Brackets Snippets","homepage":"https://github.com/testuser/brackets-snippets","author":{"name":"Testuser"},"version":"1.0.0","engines":{"brackets":">=0.24"},"description":"A simple brackets snippets extension."},"owner":"irichter","versions":[{"version":"0.2.0","published":"2014-01-10T17:27:25.996Z","brackets":">=0.24","downloads":3},{"version":"0.3.0","published":"2014-01-10T17:27:25.996Z","brackets":">=0.24","downloads":5}],"totalDownloads":8}}');
var expectedRegistry = JSON.parse('{"snippets-extension":{"metadata":{"name":"snippets-extension","title":"Brackets Snippets","homepage":"https://github.com/testuser/brackets-snippets","author":{"name":"Testuser"},"version":"1.0.0","engines":{"brackets":">=0.24"},"description":"A simple brackets snippets extension."},"owner":"irichter","versions":[{"version":"0.2.0","published":"2014-01-10T17:27:25.996Z","brackets":">=0.24","downloads":3},{"version":"0.3.0","published":"2014-01-10T17:27:25.996Z","brackets":">=0.24","downloads":5}],"totalDownloads":8, "recent":{"20130805":8}}}');
expect(JSON.stringify(repository.getRegistry())).toBe(JSON.stringify(expectedRegistry));
});

Expand Down
20 changes: 18 additions & 2 deletions spec/routes.spec.js
Expand Up @@ -22,7 +22,7 @@
*/

/*jslint vars: true, plusplus: true, nomen: true, node: true, indent: 4, maxerr: 50 */
/*global expect, describe, it, beforeEach, afterEach, createSpy, waitsFor */
/*global expect, describe, it, beforeEach, afterEach, createSpy, waitsFor, spyOn */

"use strict";

Expand Down Expand Up @@ -713,6 +713,7 @@ describe("routes", function () {
// configure repository with filestorage
var loaded = false;
repo.configure({"storage": "../lib/ramstorage.js"});
spyOn(repo, "addDownloadDataToPackage").andCallThrough();
var registry = JSON.parse(fs.readFileSync(path.join(path.dirname(module.filename), "testRegistry", "registry.json")));
repo.__set__("registry", registry);
setTimeout(function () {
Expand All @@ -728,6 +729,7 @@ describe("routes", function () {
it("should not accept post request to update the download stats other than localhost/127.0.0.1", function () {
req.ip = '10.32.1.2';
req.host = 'www.adobe.com';

_stats(req, res);
expect(res.send).toHaveBeenCalledWith(403);
});
Expand All @@ -742,7 +744,21 @@ describe("routes", function () {
var registry = repo.getRegistry();
expect(res.send).toHaveBeenCalledWith(202);
expect(registry["snippets-extension"].versions[0].downloads).toBe(6);
expect(registry["snippets-extension"].totalDownloads).toBe(6);
expect(registry["snippets-extension"].totalDownloads).toBe(29);
});

it("should call update addDownloadDataToPackage only once per extension", function () {
req.ip = '127.0.0.1';
req.host = 'localhost';
req.files = {file: {path: path.join(path.dirname(module.filename), "stats/downloadStatsOneExtension.json")}};

_stats(req, res);

var registry = repo.getRegistry();
expect(res.send).toHaveBeenCalledWith(202);
expect(repo.addDownloadDataToPackage.callCount).toEqual(1);
expect(registry["snippets-extension"].versions[0].downloads).toBe(6);
expect(registry["snippets-extension"].totalDownloads).toBe(15265);
});
});
});
Expand Down

0 comments on commit e3a9e47

Please sign in to comment.