-
Notifications
You must be signed in to change notification settings - Fork 19
Referees 2018/scores sync #257
base: master
Are you sure you want to change the base?
Changes from all commits
2b6275b
39d9422
698c517
d13338c
bff8e14
42e1d5e
bdaae48
1ef02c4
3865c07
d5f16d2
54f76df
b1d0419
7c2cff7
b4cf42e
eb38315
9e4adf6
5b99c53
c54e452
ef1d2dc
cf44e67
f95e5be
ca9a558
4f99d87
59c3b4f
74e85cc
e85c1da
4d36900
288f373
d9fdc9c
7d81136
2677a21
951f56e
c167b25
f84b4c1
807e3da
73bbe41
f4cbc7d
9f93fb7
d61db34
420028b
922d10f
2a3166a
1b572c7
6fa085d
706d549
1b760ab
ae1d1bc
76f21e8
fae6d59
2290e4f
ad85d83
6494876
f4b8fe2
8259226
f839d8f
388b740
2279f1e
882e55b
6ed40e9
54a260c
104a43c
01da877
a2dc1af
0e58cb1
645d36e
1d11dbb
ad95803
6321439
7dbd14b
ef8f73a
3eeb0cb
6982314
384a931
aea9595
1f03d5b
65ae723
f174bf0
cabc6f3
b1bf445
8f08c0a
0270c27
6b303f5
6dd369a
6741200
494151d
068ec57
6549406
3520b83
23eac34
8b3b684
924ecf2
067a926
d06641f
ceef2ef
1a9db9c
f46345e
b9e8b29
c4d8d26
0bb4427
e70f572
d053b27
8307c42
2cc771a
3b5dc2b
45c5b7b
e487f95
cd2503c
cd56007
e72cde7
ff4592b
ba18987
5ac279f
ac25467
3d128dc
47cc7cd
777d7ee
b5d5c8a
d6199bb
10752c3
19aa792
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,17 @@ | ||
| language: node_js | ||
| dist: trusty | ||
| sudo: true | ||
| node_js: | ||
| - 0.10 | ||
| - "stable" | ||
| before_install: | ||
| - "npm i -g bower karma-cli" | ||
| before_script: | ||
| - export CHROME_BIN=/usr/bin/google-chrome | ||
| - export DISPLAY=:99.0 | ||
| - sh -e /etc/init.d/xvfb start | ||
| - sudo apt-get install -y libappindicator1 fonts-liberation | ||
| - wget https://dl.google.com/linux/direct/google-chrome-stable_current_amd64.deb | ||
| - sudo dpkg -i google-chrome*.deb | ||
| after_script: | ||
| - ls ./coverage | ||
| - 'npm install coveralls@2.10.0 && cat "./coverage/Firefox 31.0.0 (Linux)/lcov.info" | coveralls' |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| //This module wraps lockfile with promises. | ||
| var lockfile = require('lockfile'); | ||
|
|
||
| module.exports = function(filename, options) { | ||
| this.filename = filename; | ||
| this.options = options || {}; | ||
|
|
||
| this.lock = function() { | ||
| var self = this; | ||
|
|
||
| return new Promise(function(resolve, reject) { | ||
| lockfile.lock('scores.json.lock', self.options, function(err) { | ||
| if(err !err.startsWith('EEXIST')) { | ||
| reject(err); | ||
| } | ||
|
|
||
| resolve(); | ||
| }); | ||
| }); | ||
| }; | ||
|
|
||
| this.unlock = function() { | ||
| return new Promise(function(resolve, reject) { | ||
| lockfile.unlock('scores.json.lock', function(err) { | ||
| if(err && !err.startsWith('EEXIST')){ | ||
| reject(err); | ||
| } | ||
|
|
||
| resolve(); | ||
| }); | ||
| }); | ||
| }; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,9 @@ | ||
| var Lock = require('./lock'); | ||
| var utils = require('./utils'); | ||
| var fileSystem = require('./file_system'); | ||
| var log = require('./log').log; | ||
| var Q = require('q'); | ||
| var id = require('uuid/v4'); | ||
|
|
||
| function filterPublished(score) { | ||
| return score.published; | ||
|
|
@@ -15,6 +18,45 @@ function reduceToMap(key) { | |
| } | ||
| } | ||
|
|
||
| /** | ||
| * Atomically change scores file. | ||
| * Action callback is called with the current contents of the scores file, and is expected | ||
| * to return the new contents (or a Promise for it). | ||
| * A lock is acquired and held during the entire operation. | ||
| * @param action Callback that receives current scores.json object, must return new contents (or Promise for it) | ||
| * @return Promise for updated scores object | ||
| */ | ||
| function changeScores(action) { | ||
| var path = fileSystem.getDataFilePath('scores.json'); | ||
| var lock = new Lock('scores.json.lock', { retries: 5, retryWait: 100 }); | ||
|
|
||
| console.log(lock.options); | ||
|
|
||
| return lock.lock() | ||
| .then(() => fileSystem.readJsonFile(path)) | ||
| .catch((err) => { //Ignoring all file not found errors, and just returning empty scores.json | ||
| console.log("catching"); | ||
| if(err.message === 'file not found') { | ||
| console.log("hells yeah!"); | ||
| return { version:3, scores: [] }; | ||
| } else { | ||
| console.log("ho no! " + err.message); | ||
| throw err; | ||
| } | ||
| }) | ||
| .then(action) | ||
| .then(scores => { | ||
| return fileSystem.writeFile(path, JSON.stringify(scores)) | ||
| .then(() => { | ||
| return lock.unlock(); | ||
| }).catch((err) => { | ||
| return lock.unlock(); | ||
| }).then(function() { | ||
| return scores; | ||
| }); | ||
| }); | ||
| } | ||
|
|
||
| exports.route = function(app) { | ||
|
|
||
| //get all, grouped by round | ||
|
|
@@ -32,7 +74,7 @@ exports.route = function(app) { | |
| return rounds; | ||
| },{}); | ||
| res.json(published); | ||
| }).catch(utils.sendError(res)).done(); | ||
| }).catch(err => utils.sendError(res, err)).done(); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Much better, but still tricky in that you return raw server errors. (minor thing for now)
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Very well, since I'm swamp, I think we can leave that for later on |
||
| }); | ||
|
|
||
| //get scores by round | ||
|
|
@@ -44,7 +86,84 @@ exports.route = function(app) { | |
| return score.published && score.round === round; | ||
| }); | ||
| res.json(scoresForRound); | ||
| }).catch(utils.sendError(res)).done(); | ||
| }).catch(err => utils.sendError(res, err)).done(); | ||
| }); | ||
|
|
||
| //save a new score | ||
| app.post('/scores/create',function(req,res) { | ||
| var body = JSON.parse(req.body); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What happens when body is invalid JSON?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. An error is thrown, and sent to the client |
||
| var scoresheet = body.scoresheet; | ||
| var score = body.score; | ||
|
|
||
| fileSystem.writeFile(fileSystem.getDataFilePath("scoresheets/" + score.file), req.body) | ||
| .then(changeScores(function(result) { | ||
| if(typeof(score.id) === 'undefined') { | ||
| score.id = id(); | ||
| } | ||
| result.scores.push(score); | ||
| return result; | ||
| })) | ||
| .then(function(scores) { | ||
| res.json(scores).end(); | ||
| }).catch(err => utils.sendError(res, err)).done(); | ||
|
|
||
| }); | ||
|
|
||
| //delete a score at an id | ||
| app.post('/scores/delete/:id',function(req,res) { | ||
| changeScores(function(result) { | ||
| var index = result.scores.findIndex((score) => score.id === req.params.id); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What happens if the |
||
| if(index === -1) { | ||
| throw new Error(`Could not find score with id ${req.params.id}`); | ||
| } | ||
| result.scores.splice(index, 1); | ||
| return result; | ||
| }).then(function(scores) { | ||
| res.json(scores).end(); | ||
| }).catch(err => utils.sendError(res, err)).done(); | ||
| }); | ||
|
|
||
| //edit a score at an id | ||
| app.post('/scores/update/:id',function(req,res) { | ||
| var score = JSON.parse(req.body); | ||
| changeScores(function(result) { | ||
| var index = result.scores.findIndex((score) => score.id === req.params.id); | ||
| if(index === -1) { | ||
| throw new Error(`Could not find score with id ${req.params.id}`); | ||
| } | ||
| result.scores[index] = score; | ||
| return result; | ||
| }).then(function(scores) { | ||
| res.json(scores).end(); | ||
| }).catch(err => utils.sendError(res, err)).done(); | ||
| }); | ||
|
|
||
|
|
||
| }; | ||
|
|
||
|
|
||
| // For backward compatibility | ||
|
|
||
| changeScores(function(scores) { | ||
| if(typeof(scores.version) === 'undefined') { | ||
| log.warn('Deprecated scores version. Updating to version 3.') | ||
| scores.forEach(score => score.id = id()) | ||
| return { | ||
| version: 3, | ||
| scores: scores | ||
| } | ||
|
|
||
| } else if(scores.version === 3) { | ||
| return scores; | ||
|
|
||
| } else if(scores.version === 2) { | ||
| log.warn('Deprecated scores version. Updating to version 3.'); | ||
| scores.scores.forEach(score => score.id = id()) | ||
| scores.version = 3; | ||
| return scores; | ||
|
|
||
| } else { | ||
| throw new Error('Unkown scores version'); | ||
| } | ||
|
|
||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,16 +1,11 @@ | ||
| exports.root = __dirname + '/../'; | ||
|
|
||
| exports.sendError = function(res) { | ||
| return function(err) { | ||
| res.status(err.status).send(err.message); | ||
| } | ||
| } | ||
|
|
||
| if (!String.prototype.format) { | ||
| String.prototype.format = function() { | ||
| var args = arguments; | ||
| return this.replace(/{(\d+)}/g, function(match, number) { | ||
| return typeof args[number] !== 'undefined' ? args[number] : match; | ||
| }); | ||
| }; | ||
| } | ||
| var log = require('./log').log; | ||
|
|
||
| exports.root = __dirname + '/../'; | ||
|
|
||
| exports.sendError = function(res, err) { | ||
| var status = err.status || 500; | ||
| var message = err.message || "Internal server error" | ||
|
|
||
| log.error(message); | ||
| res.status(status).send(message); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Slightly unrelated to this PR, but instead of (or in addition to) logging the start of a request, it would be helpful to log the completion of a request because that can include the reponse code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How can I do that? Is there a way to call a function after the routing functions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I looked it up, and it is possible. You just need to add next() calls to all the routing functions. It will be a separate PR though