From 63b2c3aaab36bc2755900e7857ca28b6629fc1cf Mon Sep 17 00:00:00 2001 From: Remy D'Agostino Date: Fri, 29 Aug 2014 15:54:53 +1000 Subject: [PATCH 1/4] Added route for favicon (which does nothing) --- server.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/server.js b/server.js index d680057..9d10a03 100644 --- a/server.js +++ b/server.js @@ -8,6 +8,11 @@ app.use(bodyParser.json()) app.use(cors()); app.use("/themes.json", express.static('themes.json')); + +app.get('/favicon.ico', function(req, res) { + res.end(); +}); + app.get('/:theme', theme); app.post('/:theme', theme); From 9893a61135c53c0dd51a74a2408e47de14a9f13b Mon Sep 17 00:00:00 2001 From: Remy D'Agostino Date: Fri, 29 Aug 2014 19:15:25 +1000 Subject: [PATCH 2/4] Major refactoring splitting out functions into smaller parts --- lib/theme.js | 408 ++++++++++++++++++++++++++++++++------------------- 1 file changed, 257 insertions(+), 151 deletions(-) diff --git a/lib/theme.js b/lib/theme.js index 5a04a5d..f42ae8e 100755 --- a/lib/theme.js +++ b/lib/theme.js @@ -3,185 +3,291 @@ var fs = require('fs'); var mkdirp = require('mkdirp'); var path = require('path'); var root = path.dirname(require.main.filename); -var npm = require('npm'); -var npmi = require('npmi'); var tarball = require('tarball-extract') var exec = require('child_process').exec, child; var schema = require('resume-schema'); -var resume = require('resume-schema').resumeJson; var themeDir = 'themes'; -function runTheme(options, req, res) { - var themeDirectory = options.themeDirectory; - console.log('Generating HTML'); - console.log('Serve', themeDirectory) - +// Loads a resume into the system using node's require function +// +// This is incredibly unsafe. +// +// It should be replaced with something that renders the resume +// using more restrictive permissions +var performUnsafeRender = function(theme, data, cb) { try { - var theme = require( path.join(root, themeDirectory) ); - var themePackageJson = fs.readFileSync(path.join(root, themeDirectory, 'package.json'), 'utf8'); - var render = theme.render(options.resume); - } catch(e) { - var error = e; - console.log(error); - } + var theme = require(theme); + var render = theme.render(data); - if (typeof render !== 'undefined') { - res.send(render); - } else { - var output = ''; - output += 'Theme returned an error.'; - output += '
';
-    output += error.stack;
-    output += '
'; - output += '

Theme Info

'; - output += '
';
-    output += themePackageJson;
-    output += '
'; - output += '

We just launched the official 0.0.0 version recently, please make sure you update before continuing.

'; - output += '

Resume.json Validation Test

'; - delete options.resume._id; - delete options.resume.jsonresume; - delete options.resume.profiles; - schema.validate(options.resume, function(blah, err) { - output += '
';
-      output += JSON.stringify(err, undefined, 4);
-      output += '
'; - res.send(output) - }) + cb(null, render); + } + catch (err) { + cb(err); } }; -var getTheme = function(req, res) { - var resumeObject = resume; +// Renders a resume, swallowing common errors and returning html +var getRenderHtml = function(options, cb) { + var themeDirectory = options.themeDirectory; + + console.log('Rendering: ' + themeDirectory); + + performUnsafeRender( + path.join(root, themeDirectory), + options.resume, + function(renderError, render) { + if (!renderError) { + return cb(null, render); + } + else { + fs.readFile( + path.join(root, themeDirectory, 'package.json'), + 'utf8', + function(error, themePackageJson) { + if (error) { + return cb(error); + } + + var output = [ + 'Theme returned an error.', + '
',
+              renderError.stack,
+              '
', + '

Theme Info

', + '
',
+              themePackageJson,
+              '
', + '

We just launched the official 0.0.0 version recently, please make sure you update before continuing.

', + '

Resume.json Validation Test

' + ].join(''); + delete options.resume._id; + delete options.resume.jsonresume; + delete options.resume.profiles; + + schema.validate(options.resume, function(blah, err) { + output += '
' + JSON.stringify(err, undefined, 4) + '
'; + + cb(null, output); + }) + } + ); + } + } + ); +} + +// Derive the resume data from the request object +// If there is no resume in the body then we use the placeholder data +var getResumeObject = function(req) { if (req.body && req.body.resume) { - console.log('Use posted resume'); - resumeObject = req.body.resume; + return req.body.resume; } - - var theme = 'jsonresume-theme-' + req.params.theme; - var version = '0'; - var versionCheck = theme.split('@'); - if (versionCheck.length > 1) { - theme = versionCheck[0]; - version = versionCheck[1]; + else { + return require('resume-schema').resumeJson; } +}; - var directoryFolder = path.join(themeDir, theme, version); +// Render the theme with the given data +var getThemeHtml = function(themeData, resumeObject, cb) { + var directoryFolder = themeData.directory; + var version = themeData.version; - console.log(theme, version); fs.exists(directoryFolder, function(exists) { - console.log(directoryFolder, exists); if (exists && version !== '0') { - console.log('Theme cached'); - runTheme({ - themeDirectory: directoryFolder, - resume: resumeObject - }, req, res); - return; + getRenderHtml({ themeDirectory: directoryFolder, resume: resumeObject }, cb); } else { - console.log('Checking NPM'); - request.get('https://registry.npmjs.org/' + theme, function(response) { - var lib = response.body; - if (!lib || Object.keys(lib).length === 0) { - res.send('Theme could not be found in the npm registry.'); - console.log(theme, 'not found'); - return; - } + cb(new Error('Theme not found'), null); + } + }); +}; - if (version === '0') { - version = lib['dist-tags'].latest; - } - try { - var author = lib['versions'][version].author.name; - } catch(e) { - // .. - } +// Download, save and install theme, then update the theme config +var downloadTheme = function(themeData, cb) { + var directoryFolder, theme, version, author, lib; + + // Deconstruct theme data + directoryFolder = themeData.directory; + theme = themeData.name; + version = themeData.version; + author = themeData.author; + lib = themeData.lib; + + var themeVersion = theme + '@' + version; + var tarballURL = lib.versions[version].dist.tarball; + + mkdirp(directoryFolder, function(err) { + fs.exists( root + '/tmp/', function(exists) { + if ( ! exists) { + // create folder /tmp to download && extract + mkdirp(root + '/tmp/', function(err) { + // handle err + if (err) + console.error('cannot create folder ' + root + '/tmp/'+'. Try creating it manually, maybe?'); + }); + } + }); + + var tempExtractPath = root + '/tmp/' + themeVersion; + + tarball.extractTarballDownload(tarballURL, root + '/tmp/' + themeVersion + '.tar.gz', tempExtractPath, {}, function(err, result) { + fs.readdir(tempExtractPath, function(err, files) { + var containingFolder = files[0]; + + // Save in themes.json + fs.readFile(path.join(root, '/themes.json'), 'utf-8', function(err, data) { + var name = theme.replace('jsonresume-theme-', ''); - var directoryFolder = path.join(themeDir, theme, version); - fs.exists(directoryFolder, function(exists) { - if (exists) { - runTheme({ - themeDirectory: directoryFolder, - resume: resumeObject - }, req, res); - return; - } else { - if (!lib.versions[version]) { - var msg = theme + '@' + version + ' does not exist.'; - res.send(msg); - console.log(msg); - return; + var themes = { + themes: {} + }; + + if (!err) { + try { + themes = JSON.parse(data); + } + catch(e) { + // .. + } + } + + themes['themes'][name] = themes['themes'][name] || { + author: "", + versions: [] + }; + + themes['themes'][name].author = author || ""; + themes['themes'][name].versions.push(version); + themes['themes'][name].versions.sort(); + + var json = JSON.stringify(themes, null, ' '); + + fs.writeFile(path.join(root, '/themes.json'), json, function(err) { + if (!err) { + console.log('Updated themes.json'); } + }); + }); - var themeVersion = theme + '@' + version; - var tarballURL = lib.versions[version].dist.tarball; - - mkdirp(directoryFolder, function(err) { - fs.exists( root + '/tmp/', function(exists) { - if ( ! exists) { - // create folder /tmp to download && extract - mkdirp(root + '/tmp/', function(err) { - // handle err - if (err) - console.log('cannot create folder ' + root + '/tmp/'+'. Try creating it manually, maybe?'); - }); - } - }); - - var tempExtractPath = root + '/tmp/' + themeVersion; - console.log('Downloading NPM module'); - tarball.extractTarballDownload(tarballURL, root + '/tmp/' + themeVersion + '.tar.gz', tempExtractPath, {}, function(err, result) { - fs.readdir(tempExtractPath, function(err, files) { - var containingFolder = files[0]; - - // Save in themes.json - fs.readFile(path.join(root, '/themes.json'), 'utf-8', function(err, data) { - var name = theme.replace('jsonresume-theme-', ''); - var themes = { - themes: {} - }; - if (!err) { - try { - themes = JSON.parse(data); - } catch(e) {} - } - themes['themes'][name] = themes['themes'][name] || { - author: "", - versions: [] - }; - themes['themes'][name].author = author || ""; - themes['themes'][name].versions.push(version); - themes['themes'][name].versions.sort(); - var json = JSON.stringify(themes, null, ' '); - fs.writeFile(path.join(root, '/themes.json'), json, function(err) { - if (!err) { - console.log('Updated themes.json'); - } - }); - }); - - fs.rename(path.join(tempExtractPath, containingFolder), directoryFolder, function() { - console.log('Installing dependencies'); - child = exec('cd ' + directoryFolder + ' && npm install', - function(error, stdout, stderr) { - runTheme({ - themeDirectory: directoryFolder, - resume: resumeObject - }, req, res); - if (error !== null) { - console.log('exec error: ' + error); - } - }); - }); - }) - }) - }); + fs.rename(path.join(tempExtractPath, containingFolder), directoryFolder, function() { + console.log('Installing dependencies'); + child = exec( + 'cd ' + directoryFolder + ' && npm install', + function(error, stdout, stderr) { + cb(error); + } + ); + }); + }) + }) + }); +}; + +// Construct a basic theme object using only the theme name +var getThemeData = function(themeName) { + var name, version, versionCheck; + + if (/^jsonresume\-theme\-/.test(themeName)) { + name = themeName; + } + else { + name = 'jsonresume-theme-' + themeName; + } + + version = '0'; + versionCheck = name.split('@'); + + if (versionCheck.length > 1) { + name = versionCheck[0]; + version = versionCheck[1]; + } + + var directoryFolder = path.join(themeDir, name, version); + + return { + name: name, + version: version, + directory: directoryFolder, + lib: null, + author: null + }; +}; + +// Fetch the theme object from NPM +var augmentThemeData = function(themeData, cb) { + request.get('https://registry.npmjs.org/' + themeData.name, function(response) { + themeData.lib = response.body; + + if (!themeData.lib || Object.keys(themeData.lib).length === 0) { + return cb(new Error('Theme could not be found in the npm registry.')); + } + + if (themeData.version === '0') { + themeData.version = themeData.lib['dist-tags'].latest; + } + + if (!themeData.lib.versions[themeData.version]) { + return cb(new Error(themeData.name + '@' + themeData.version + ' does not exist.')); + } + + try { + themeData.author = themeData.lib['versions'][version].author.name; + } catch(e) { + // .. + } + + themeData.directory = path.join(themeDir, themeData.name, themeData.version); + + return cb(null, themeData); + }); +}; + +// Asyncronously process the request, returning the rendered HTML in the callback +var processRequest = function(req, cb) { + var resumeObject = getResumeObject(req); + var themeData = getThemeData(req.params.theme); + + // If we already have the theme and we know the version we can render it + // straight away + getThemeHtml(themeData, resumeObject, function(error, html) { + if (!error) { + return cb(null, html); + } + + // Otherwise we need to make a trip to NPM to get the package data + augmentThemeData(themeData, function(error, themeData) { + if (error) { + return cb(error); + } + + // Now that we know what the latest version is, we try rendering again + getThemeHtml(themeData, resumeObject, function(error, html) { + if (!error) { + return cb(null, html); + } + + // If we still don't have the theme then we download and install it + downloadTheme(themeData, function(error) { + if (error) { + return cb(error); } + + // Now we almost definitely have the theme, try rending again. + return getThemeHtml(themeData, resumeObject, cb); }); }); + }); + }); +}; + +var getTheme = function(req, res) { + processRequest(req, function(error, html) { + if (error) { + res.status(500).send(error.message); } + + res.send(html); }); }; From aa9eaf8807e740e767368b8ddb3b4b568ade7dd9 Mon Sep 17 00:00:00 2001 From: Remy D'Agostino Date: Fri, 29 Aug 2014 19:23:40 +1000 Subject: [PATCH 3/4] Modified download theme so that it waits for writing to themes.json --- lib/theme.js | 33 ++++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/lib/theme.js b/lib/theme.js index f42ae8e..f8a04f5 100755 --- a/lib/theme.js +++ b/lib/theme.js @@ -164,21 +164,32 @@ var downloadTheme = function(themeData, cb) { var json = JSON.stringify(themes, null, ' '); fs.writeFile(path.join(root, '/themes.json'), json, function(err) { - if (!err) { - console.log('Updated themes.json'); + if (err) { + return cb(err); } + + fs.rename( + path.join(tempExtractPath, containingFolder), + directoryFolder, + function(err) { + if (err) { + return cb(err); + } + + console.log('Installing dependencies'); + + child = exec( + 'cd ' + directoryFolder + ' && npm install', + function(error, stdout, stderr) { + console.log('Dependancies installed'); + cb(error); + } + ); + } + ); }); }); - fs.rename(path.join(tempExtractPath, containingFolder), directoryFolder, function() { - console.log('Installing dependencies'); - child = exec( - 'cd ' + directoryFolder + ' && npm install', - function(error, stdout, stderr) { - cb(error); - } - ); - }); }) }) }); From 26e9353726a2f4c6b8a8aee88bcb128592a5c3a0 Mon Sep 17 00:00:00 2001 From: Remy D'Agostino Date: Sat, 30 Aug 2014 17:33:34 +1000 Subject: [PATCH 4/4] Added basic request coalescing to theme installation --- lib/theme.js | 78 +++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 62 insertions(+), 16 deletions(-) diff --git a/lib/theme.js b/lib/theme.js index f8a04f5..bbebeb2 100755 --- a/lib/theme.js +++ b/lib/theme.js @@ -8,6 +8,7 @@ var tarball = require('tarball-extract') var exec = require('child_process').exec, child; var schema = require('resume-schema'); var themeDir = 'themes'; +var installCache = {}; // Loads a resume into the system using node's require function // @@ -15,12 +16,17 @@ var themeDir = 'themes'; // // It should be replaced with something that renders the resume // using more restrictive permissions -var performUnsafeRender = function(theme, data, cb) { +var performUnsafeRender = function(themePath, data, cb) { try { - var theme = require(theme); + var theme = require(themePath); var render = theme.render(data); - cb(null, render); + if (typeof render !== 'undefined') { + cb(null, render); + } + else { + cb(new Error('Theme did not return a string')); + } } catch (err) { cb(err); @@ -78,22 +84,15 @@ var getRenderHtml = function(options, cb) { ); } -// Derive the resume data from the request object -// If there is no resume in the body then we use the placeholder data -var getResumeObject = function(req) { - if (req.body && req.body.resume) { - return req.body.resume; - } - else { - return require('resume-schema').resumeJson; - } -}; - // Render the theme with the given data var getThemeHtml = function(themeData, resumeObject, cb) { var directoryFolder = themeData.directory; var version = themeData.version; + if (installCache[themeData.directory]) { + return cb(new Error('Theme is being installed'), null); + } + fs.exists(directoryFolder, function(exists) { if (exists && version !== '0') { getRenderHtml({ themeDirectory: directoryFolder, resume: resumeObject }, cb); @@ -103,6 +102,18 @@ var getThemeHtml = function(themeData, resumeObject, cb) { }); }; + +// Derive the resume data from the request object +// If there is no resume in the body then we use the placeholder data +var getResumeObject = function(req) { + if (req.body && req.body.resume) { + return req.body.resume; + } + else { + return require('resume-schema').resumeJson; + } +}; + // Download, save and install theme, then update the theme config var downloadTheme = function(themeData, cb) { var directoryFolder, theme, version, author, lib; @@ -178,6 +189,7 @@ var downloadTheme = function(themeData, cb) { console.log('Installing dependencies'); + // `child` is declared at the top of the file child = exec( 'cd ' + directoryFolder + ' && npm install', function(error, stdout, stderr) { @@ -195,6 +207,38 @@ var downloadTheme = function(themeData, cb) { }); }; +// Coalesces requests to download the same theme into a single request +// Download theme is not safe if it is called multiple times on the same theme +var installTheme = function(themeData, cb) { + var dir = themeData.directory; + + if (!installCache[dir]) { + console.log('"' + dir + '" being installed now'); + + installCache[dir] = (function() { + var listeners = []; + + downloadTheme(themeData, function downloadComplete(error) { + // Remove the theme from the list of currenly installing themes + delete installCache[dir]; + + // Fire each of the callbacks + listeners.forEach(function(listener, i) { + listener(error); + }); + + listeners = null; + }); + + return function(onFinish) { + listeners.push(onFinish); + }; + })(); + } + + installCache[dir](cb); +}; + // Construct a basic theme object using only the theme name var getThemeData = function(themeName) { var name, version, versionCheck; @@ -254,7 +298,7 @@ var augmentThemeData = function(themeData, cb) { }); }; -// Asyncronously process the request, returning the rendered HTML in the callback +// Asynchronously process the request, returning the rendered HTML in the callback var processRequest = function(req, cb) { var resumeObject = getResumeObject(req); var themeData = getThemeData(req.params.theme); @@ -279,7 +323,7 @@ var processRequest = function(req, cb) { } // If we still don't have the theme then we download and install it - downloadTheme(themeData, function(error) { + installTheme(themeData, function(error) { if (error) { return cb(error); } @@ -293,6 +337,8 @@ var processRequest = function(req, cb) { }; var getTheme = function(req, res) { + console.log('incoming request:', req.params.theme); + processRequest(req, function(error, html) { if (error) { res.status(500).send(error.message);