From 0af17c688f3476d98123b985db9a010321715de7 Mon Sep 17 00:00:00 2001 From: Jori Lallo Date: Sun, 21 Aug 2016 11:12:24 -0700 Subject: [PATCH 1/2] lock --- package.json | 3 ++ server/api/documents.js | 73 ++++++++++++++++++++++++----------------- server/models/Atlas.js | 2 +- server/redis.js | 10 ++++++ 4 files changed, 56 insertions(+), 32 deletions(-) create mode 100644 server/redis.js diff --git a/package.json b/package.json index c52f9fb8..68b6e98f 100644 --- a/package.json +++ b/package.json @@ -102,6 +102,8 @@ "react-keydown": "^1.6.1", "react-router": "2.5.1", "rebass": "0.2.6", + "redis": "^2.6.2", + "redis-lock": "^0.1.0", "reflexbox": "^2.0.0", "safestart": "0.8.0", "sass-loader": "4.0.0", @@ -109,6 +111,7 @@ "sequelize-cli": "2.4.0", "sequelize-encrypted": "0.1.0", "slug": "0.9.1", + "string-hash": "^1.1.0", "style-loader": "0.13.0", "truncate-html": "0.0.6", "url-loader": "0.5.7", diff --git a/server/api/documents.js b/server/api/documents.js index b3ed3c0e..ab4271ac 100644 --- a/server/api/documents.js +++ b/server/api/documents.js @@ -3,6 +3,7 @@ import httpErrors from 'http-errors'; import { sequelize, } from '../sequelize'; +import { lock } from '../redis'; const URL_REGEX = /^[a-zA-Z0-9-]*-([a-zA-Z0-9]{10,15})$/; @@ -109,7 +110,7 @@ router.post('documents.search', auth(), async (ctx) => { }); -router.post('documents.create', auth(), async (ctx) => { +router.post('documents.create', auth(), async (ctx, done) => { const { collection, title, @@ -130,39 +131,49 @@ router.post('documents.create', auth(), async (ctx) => { if (!ownerCollection) throw httpErrors.BadRequest(); - let parentDocumentObj = {}; - if (parentDocument && ownerCollection.type === 'atlas') { - parentDocumentObj = await Document.findOne({ - where: { - id: parentDocument, + await lock(ownerCollection.id, 10000, async (done) => { + return new Promise(async resolve => { + console.info(`Execute: ${title}`) + let parentDocumentObj = {}; + if (parentDocument && ownerCollection.type === 'atlas') { + parentDocumentObj = await Document.findOne({ + where: { + id: parentDocument, + atlasId: ownerCollection.id, + }, + }); + } + + const document = await Document.create({ + parentDocumentId: parentDocumentObj.id, atlasId: ownerCollection.id, - }, + teamId: user.teamId, + userId: user.id, + lastModifiedById: user.id, + createdById: user.id, + title, + text, + }); + + // TODO: Move to afterSave hook if possible with imports + if (parentDocument && ownerCollection.type === 'atlas') { + await ownerCollection.reload(); + ownerCollection.addNodeToNavigationTree(document); + await ownerCollection.save(); + } + + console.info(`Finish: ${title}`) + + ctx.body = { + data: await presentDocument(ctx, document, { + includeCollection: true, + includeCollaborators: true, + }), + }; + + done(resolve); }); - } - - const document = await Document.create({ - parentDocumentId: parentDocumentObj.id, - atlasId: ownerCollection.id, - teamId: user.teamId, - userId: user.id, - lastModifiedById: user.id, - createdById: user.id, - title, - text, }); - - // TODO: Move to afterSave hook if possible with imports - if (parentDocument && ownerCollection.type === 'atlas') { - ownerCollection.addNodeToNavigationTree(document); - await ownerCollection.save(); - } - - ctx.body = { - data: await presentDocument(ctx, document, { - includeCollection: true, - includeCollaborators: true, - }), - }; }); router.post('documents.update', auth(), async (ctx) => { diff --git a/server/models/Atlas.js b/server/models/Atlas.js index b8ca7029..6d13330d 100644 --- a/server/models/Atlas.js +++ b/server/models/Atlas.js @@ -142,7 +142,7 @@ const Atlas = sequelize.define('atlas', { return newTree; }, - addNodeToNavigationTree(document) { + async addNodeToNavigationTree(document) { const newNode = { id: document.id, title: document.title, diff --git a/server/redis.js b/server/redis.js new file mode 100644 index 00000000..fcbabbcd --- /dev/null +++ b/server/redis.js @@ -0,0 +1,10 @@ +import redis from 'redis'; +import redisLock from 'redis-lock'; + +const client = redis.createClient(process.env.REDIS_URL); +const lock = redisLock(client); + +export { + client, + lock, +}; From e527f3c4022325babd43013e17bd45b9c51ec246 Mon Sep 17 00:00:00 2001 From: Jori Lallo Date: Sun, 21 Aug 2016 15:45:48 -0700 Subject: [PATCH 2/2] Fixed locking with imports --- package.json | 2 +- server/api/documents.js | 108 +++++++++++++++++++++------------------- 2 files changed, 57 insertions(+), 53 deletions(-) diff --git a/package.json b/package.json index 68b6e98f..5f49436e 100644 --- a/package.json +++ b/package.json @@ -8,7 +8,7 @@ "build:webpack": "cross-env NODE_ENV=production webpack --config webpack.config.prod.js --progress", "build:analyze": "cross-env NODE_ENV=production webpack --config webpack.config.prod.js --json | webpack-bundle-size-analyzer", "build": "npm run clean && npm run build:webpack", - "start": "cross-env NODE_ENV=development DEBUG=cache,presenters ./node_modules/.bin/nodemon --watch server index.js", + "start": "cross-env NODE_ENV=development DEBUG=sql,cache,presenters ./node_modules/.bin/nodemon --watch server index.js", "lint": "eslint frontend", "deploy": "git push heroku master", "heroku-postbuild": "npm run build && npm run sequelize db:migrate", diff --git a/server/api/documents.js b/server/api/documents.js index ab4271ac..6c9f3f03 100644 --- a/server/api/documents.js +++ b/server/api/documents.js @@ -4,6 +4,7 @@ import { sequelize, } from '../sequelize'; import { lock } from '../redis'; +import isUUID from 'validator/lib/isUUID'; const URL_REGEX = /^[a-zA-Z0-9-]*-([a-zA-Z0-9]{10,15})$/; @@ -15,26 +16,28 @@ import { Document, Atlas } from '../models'; const router = new Router(); const getDocumentForId = async (id) => { - let document; - if (id.match(URL_REGEX)) { - document = await Document.findOne({ - where: { - urlId: id.match(URL_REGEX)[1], - }, - }); - } else { - try { + try { + let document; + if (isUUID(id)) { document = await Document.findOne({ where: { id, }, }); - } catch (e) { - // Invalid UUID + } else if (id.match(URL_REGEX)) { + document = await Document.findOne({ + where: { + urlId: id.match(URL_REGEX)[1], + }, + }); + } else { throw httpErrors.NotFound(); } + return document; + } catch (e) { + // Invalid UUID + throw httpErrors.NotFound(); } - return document; }; // FIXME: This really needs specs :/ @@ -110,7 +113,7 @@ router.post('documents.search', auth(), async (ctx) => { }); -router.post('documents.create', auth(), async (ctx, done) => { +router.post('documents.create', auth(), async (ctx) => { const { collection, title, @@ -131,49 +134,48 @@ router.post('documents.create', auth(), async (ctx, done) => { if (!ownerCollection) throw httpErrors.BadRequest(); - await lock(ownerCollection.id, 10000, async (done) => { - return new Promise(async resolve => { - console.info(`Execute: ${title}`) - let parentDocumentObj = {}; - if (parentDocument && ownerCollection.type === 'atlas') { - parentDocumentObj = await Document.findOne({ - where: { - id: parentDocument, - atlasId: ownerCollection.id, - }, + const document = await (() => { + return new Promise(resolve => { + lock(ownerCollection.id, 10000, async (done) => { + let parentDocumentObj = {}; + if (parentDocument && ownerCollection.type === 'atlas') { + parentDocumentObj = await Document.findOne({ + where: { + id: parentDocument, + atlasId: ownerCollection.id, + }, + }); + } + + const document = await Document.create({ + parentDocumentId: parentDocumentObj.id, + atlasId: ownerCollection.id, + teamId: user.teamId, + userId: user.id, + lastModifiedById: user.id, + createdById: user.id, + title, + text, }); - } - const document = await Document.create({ - parentDocumentId: parentDocumentObj.id, - atlasId: ownerCollection.id, - teamId: user.teamId, - userId: user.id, - lastModifiedById: user.id, - createdById: user.id, - title, - text, + // TODO: Move to afterSave hook if possible with imports + if (parentDocument && ownerCollection.type === 'atlas') { + await ownerCollection.reload(); + ownerCollection.addNodeToNavigationTree(document); + await ownerCollection.save(); + } + + done(resolve(document)); }); - - // TODO: Move to afterSave hook if possible with imports - if (parentDocument && ownerCollection.type === 'atlas') { - await ownerCollection.reload(); - ownerCollection.addNodeToNavigationTree(document); - await ownerCollection.save(); - } - - console.info(`Finish: ${title}`) - - ctx.body = { - data: await presentDocument(ctx, document, { - includeCollection: true, - includeCollaborators: true, - }), - }; - - done(resolve); }); - }); + })(); + + ctx.body = { + data: await presentDocument(ctx, document, { + includeCollection: true, + includeCollaborators: true, + }), + }; }); router.post('documents.update', auth(), async (ctx) => { @@ -198,6 +200,7 @@ router.post('documents.update', auth(), async (ctx) => { await document.save(); // Update + // TODO: Add locking const collection = await Atlas.findById(document.atlasId); if (collection.type === 'atlas') { await collection.updateNavigationTree(); @@ -223,6 +226,7 @@ router.post('documents.delete', auth(), async (ctx) => { if (!document || document.teamId !== user.teamId) throw httpErrors.BadRequest(); + // TODO: Add locking if (collection.type === 'atlas') { // Don't allow deletion of root docs if (!document.parentDocumentId) {