Permalink
Browse files

Implemented review comments.

Also fixed various bugs similar to the following where the second `Project.compile` call, c.f. #483, caused the EPUB file to be written again and the EPUB checker at the same time trying to access the file causing it to fail:

    Error: 1: Error: Command failed: ERROR: /tmp/scripler/app/users/user548814ca484b378427b5e237/548814e0484b378427b5e47a.epub: I/O error: error in opening zip file Check finished with warnings or errors at ChildProcess.exithandler (child_process.js:648:15) at ChildProcess.emit (events.js:98:17) at maybeClose (child_process.js:756:16) at Socket. (child_process.js:969:11) at Socket.emit (events.js:95:17) at Pipe.close (net.js:465:12)
  • Loading branch information...
mortengf committed Dec 15, 2014
1 parent 6d35690 commit 1741ffd722982fa8403b41c341a29b1ffcf72a16
Showing with 72 additions and 56 deletions.
  1. +8 −1 app.js
  2. +4 −1 config/default.json
  3. +5 −10 lib/epub/index.js
  4. +55 −44 routes/project.js
View
9 app.js
@@ -6,7 +6,8 @@ var express = require('express')
, auth = require('./lib/auth')
, logger = require('./lib/logger')
, routes = require('./routes')
- , expressConf = require('./config/express');
+ , expressConf = require('./config/express')
+ , mkdirp = require('mkdirp');
var app = express();
@@ -30,3 +31,9 @@ mongoose.connect(conf.db.uri, function(err) {
});
+mkdirp(conf.epub.tmpDir, function (err) {
+ if (err) {
+ logger.error("Could not create " + conf.epub.tmpDir + ": " + JSON.stringify(err));
+ process.exit(1);
+ }
+});
View
@@ -74,7 +74,10 @@
"fontPrefix": "font_",
"imagePrefix": "img_",
"stylePrefix": "style_",
- "anchorIdPrefix": "id_" // TODO: move somewhere the frontend can access it when generating anchors
+ "anchorIdPrefix": "id_", // TODO: move somewhere the frontend can access it when generating anchors
+ "validationResultEmail": "ebookresults@scripler.com",
+ "validatorPath": "test/epubcheck-3.0.1.jar",
+ "tmpDir": "/tmp/scripler/app/"
},
"epub2": {
"oepbsDir": "OEBPS"
View
@@ -2,7 +2,6 @@ var conf = require('config');
var path = require('path');
var fs = require('fs');
var moment = require('moment');
-var archiver = require('archiver');
var uuid_lib = require('node-uuid');
var async = require('async');
var Project = require('../../models/project.js').Project;
@@ -17,7 +16,6 @@ var styleset_utils_shared = require('../../public/create/scripts/utils-shared');
var project_utils = require('../../lib/project-utils');
var html4Entities = require(path.join(__dirname, '../html4-entities.json'));
var package = require('../../package.json');
-var sanitize = require('sanitize-filename');
var getCloseNavPointsString = exports.getCloseNavPointsString = function (currentLevel, previousLevel, indentationSize) {
var closeNavPointsString = '';
@@ -453,8 +451,6 @@ var create = exports.create = function create(epubType, userId, project, archive
cover = cover.replace(/^images\//i, conf.epub.imagePrefix);
}
- var userDir = path.join(conf.resources.usersDir, conf.epub.userDirPrefix + userId);
-
// Filenames are not allowed to start with e.g. numbers (c.f.: http://code.google.com/p/epubcheck/issues/detail?id=193) which is typically the case for Mongo object ids
var docPrefix = conf.epub.documentPrefix;
var imgPrefix = conf.epub.imagePrefix;
@@ -677,14 +673,13 @@ var create = exports.create = function create(epubType, userId, project, archive
archive.finalize();
archive.on('finish', function() {
- // Save EPUB in user's folder
- var filename = (metadata.title || project.name) + ".epub";
- var saneTitle = sanitize(filename);
- var output = fs.createWriteStream(path.join(userDir, saneTitle));
- archive.pipe(output);
+ // Save EPUB in user's folder: save it with a unique name such that the user can have multiple projects with the same name
+ // TODO: de-comment this when the "real" EPUB in the user's folder should actually be used for something
+ //var userDir = path.join(conf.resources.usersDir, conf.epub.userDirPrefix + userId);
+ //var output = fs.createWriteStream(path.join(userDir, project._id + '.epub'));
+ //archive.pipe(output);
return next(null, archive);
});
-
});
}
View
@@ -20,6 +20,8 @@ var TOCEntry = require('../models/project.js').TOCEntry;
var env = process.env.NODE_ENV;
var emailer = require('../lib/email/email.js');
var exec = require('child_process').exec;
+var logger = require('../lib/logger');
+var uuid_lib = require('node-uuid');
//Load project by id
exports.load = function (id) {
@@ -415,57 +417,66 @@ exports.compile = function (req, res, next) {
return next(err);
}
+ // Create a temporary file to avoid the second compile() call from the client, c.f. #483, trying to write to the same file while the EPUB checker has it open.
+ // TODO: when fixing #483 it should no longer be necessary to create a temporary file because only one file will be created.
+ var tempFilename = path.join(conf.epub.tmpDir, uuid_lib.v4() + ".epub");
+ var tempFile = fs.createWriteStream(tempFilename);
+ epub.pipe(tempFile);
+
var filename = (req.project.metadata.title || req.project.name) + ".epub";
var saneTitle = sanitize(filename);
- res.setHeader('Content-disposition', 'attachment; filename="' + saneTitle + '"');
- res.setHeader('Content-type', 'application/epub+zip');
// TODO: When a GUI design has been made, also return the EPUB validation result to client
+ res.setHeader('Content-disposition', 'attachment; filename="' + saneTitle + '"');
+ res.setHeader('Content-type', 'application/epub+zip');
epub.pipe(res);
- // The sending of the validation result email can happen after the response has been returned to the user
- if ('test' != env) {
- var userDir = path.join(conf.resources.usersDir, conf.epub.userDirPrefix + userId);
- var fullPath = path.join(userDir, saneTitle);
- exec('java -jar test/epubcheck-3.0.1.jar "' + fullPath + '"',
- function (error, stdout, stderr) {
- var epubValidationResult = "OK";
- var errorMessage = null;
- if (error !== null) {
- epubValidationResult = "ERROR";
- errorMessage = error.code + ':\n' + error.stack;
- }
+ tempFile.once('close', function() {
+ // The sending of the validation result email can happen after the response has been returned to the user but must happen on the temp file, c.f. comment above.
+ if ('test' != env) {
+ var fullPath = tempFilename;
+ exec('java -jar ' + conf.epub.validatorPath + ' "' + fullPath + '"',
+ function (error, stdout, stderr) {
+ var epubValidationResult = "OK";
+ var errorMessage = null;
+ if (error !== null) {
+ epubValidationResult = "ERROR";
+ errorMessage = error.code + ':\n' + error.stack;
+ }
- var authorName = req.user.firstname + " " + req.user.lastname;
-
- console.log('EPUB Validation Result');
- console.log('Author name: ' + authorName);
- console.log('EPUB filename: ' + fullPath);
- console.log('Validation Result: ' + epubValidationResult);
- if (errorMessage !== null) console.log('Error: ' + errorMessage);
-
- // Dummy user who is the recipient of the validation result
- var validationResultRecipient = {
- _id: req.user._id, // TODO: just use a dummy id?
- email: conf.epub.validationResultEmail,
- firstname: "Frank",
- lastname: "EPUB"
- };
-
- emailer.sendUserEmail(
- validationResultRecipient,
- [
- {name: "USEREMAIL", content: req.user.email},
- {name: "NAME", content: authorName},
- {name: "EPUBTITLE", content: saneTitle},
- {name: "EPUBVALIDATIONRESULT", content: epubValidationResult},
- {name: "ERROR", content: errorMessage}
- ],
- 'epub-validation-result'
- );
- }
- );
- }
+ var authorName = req.user.firstname + " " + req.user.lastname;
+
+ logger.info('EPUB Validation Result');
+ logger.info('Author name: ' + authorName);
+ logger.info('EPUB filename: ' + saneTitle + ' (' + fullPath + ')');
+ logger.info('Validation Result: ' + epubValidationResult);
+ if (errorMessage !== null) logger.error('Error: ' + errorMessage);
+
+ // Dummy user who is the recipient of the validation result
+ var validationResultRecipient = {
+ _id: req.user._id, // TODO: just use a dummy id?
+ email: conf.epub.validationResultEmail,
+ firstname: "Frank",
+ lastname: "EPUB"
+ };
+
+ emailer.sendUserEmail(
+ validationResultRecipient,
+ [
+ {name: "USEREMAIL", content: req.user.email},
+ {name: "NAME", content: authorName},
+ {name: "EPUBTITLE", content: saneTitle},
+ {name: "EPUBVALIDATIONRESULT", content: epubValidationResult},
+ {name: "ERROR", content: errorMessage}
+ ],
+ 'epub-validation-result'
+ );
+
+ fs.unlink(tempFilename, function () {});
+ }
+ );
+ }
+ });
});
}

0 comments on commit 1741ffd

Please sign in to comment.