From f90d17049730061ae712c0e3a0541ad1aba37e4f Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Mon, 3 Jul 2017 21:35:17 -0700 Subject: [PATCH 1/5] Using the magic of joins --- server/api/documents.js | 11 ++++++--- server/models/Document.js | 26 ++++++++++++++++++++- server/presenters/document.js | 43 +++++++---------------------------- 3 files changed, 41 insertions(+), 39 deletions(-) diff --git a/server/api/documents.js b/server/api/documents.js index 5c8e3d7b7..b43ba923e 100644 --- a/server/api/documents.js +++ b/server/api/documents.js @@ -5,10 +5,9 @@ import httpErrors from 'http-errors'; import auth from './middlewares/authentication'; import pagination from './middlewares/pagination'; import { presentDocument } from '../presenters'; -import { Document, Collection, Star, View } from '../models'; +import { User, Document, Collection, Star, View } from '../models'; const router = new Router(); - router.post('documents.list', auth(), pagination(), async ctx => { let { sort = 'updatedAt', direction } = ctx.body; if (direction !== 'ASC') direction = 'DESC'; @@ -19,6 +18,7 @@ router.post('documents.list', auth(), pagination(), async ctx => { order: [[sort, direction]], offset: ctx.state.pagination.offset, limit: ctx.state.pagination.limit, + include: [{ model: Star, as: 'starred', where: { userId: user.id } }], }); let data = await Promise.all(documents.map(doc => presentDocument(ctx, doc))); @@ -60,7 +60,12 @@ router.post('documents.starred', auth(), pagination(), async ctx => { const views = await Star.findAll({ where: { userId: user.id }, order: [[sort, direction]], - include: [{ model: Document }], + include: [ + { + model: Document, + include: [{ model: Star, as: 'starred', where: { userId: user.id } }], + }, + ], offset: ctx.state.pagination.offset, limit: ctx.state.pagination.limit, }); diff --git a/server/models/Document.js b/server/models/Document.js index f0f5e9001..82b5c439c 100644 --- a/server/models/Document.js +++ b/server/models/Document.js @@ -115,7 +115,31 @@ const Document = sequelize.define( }, classMethods: { associate: models => { - Document.belongsTo(models.User); + Document.belongsTo(models.Collection, { + foreignKey: 'atlasId', + }); + Document.belongsTo(models.User, { + as: 'createdBy', + foreignKey: 'createdById', + }); + Document.belongsTo(models.User, { + as: 'updatedBy', + foreignKey: 'lastModifiedById', + }); + Document.hasMany(models.Star, { + as: 'starred', + }); + Document.addScope( + 'defaultScope', + { + include: [ + { model: models.Collection }, + { model: models.User, as: 'createdBy' }, + { model: models.User, as: 'updatedBy' }, + ], + }, + { override: true } + ); }, findById: async id => { if (isUUID(id)) { diff --git a/server/presenters/document.js b/server/presenters/document.js index 8757ed7e8..124f8650c 100644 --- a/server/presenters/document.js +++ b/server/presenters/document.js @@ -1,8 +1,9 @@ -import { Collection, Star, User, View } from '../models'; +// @flow +import { Star, User, Document, View } from '../models'; import presentUser from './user'; import presentCollection from './collection'; -async function present(ctx, document, options) { +async function present(ctx: Object, document: Document, options: Object = {}) { options = { includeCollection: true, includeCollaborators: true, @@ -10,8 +11,6 @@ async function present(ctx, document, options) { ...options, }; ctx.cache.set(document.id, document); - - const userId = ctx.state.user.id; const data = { id: document.id, url: document.getUrl(), @@ -20,37 +19,23 @@ async function present(ctx, document, options) { text: document.text, html: document.html, preview: document.preview, + collection: presentCollection(ctx, document.collection), createdAt: document.createdAt, - createdBy: undefined, + createdBy: presentUser(ctx, document.createdBy), updatedAt: document.updatedAt, - updatedBy: undefined, + updatedBy: presentUser(ctx, document.updatedBy), team: document.teamId, collaborators: [], + starred: !!document.starred, + views: undefined, }; - data.starred = !!await Star.findOne({ - where: { documentId: document.id, userId }, - }); - if (options.includeViews) { data.views = await View.sum('count', { where: { documentId: document.id }, }); } - if (options.includeCollection) { - data.collection = await ctx.cache.get(document.atlasId, async () => { - const collection = - options.collection || - (await Collection.findOne({ - where: { - id: document.atlasId, - }, - })); - return presentCollection(ctx, collection); - }); - } - if (options.includeCollaborators) { // This could be further optimized by using ctx.cache data.collaborators = await User.findAll({ @@ -62,18 +47,6 @@ async function present(ctx, document, options) { }).map(user => presentUser(ctx, user)); } - const createdBy = await ctx.cache.get( - document.createdById, - async () => await User.findById(document.createdById) - ); - data.createdBy = await presentUser(ctx, createdBy); - - const updatedBy = await ctx.cache.get( - document.lastModifiedById, - async () => await User.findById(document.lastModifiedById) - ); - data.updatedBy = await presentUser(ctx, updatedBy); - return data; } From f08ca8d46004f2bf00fb09b2b0d09015a2d7bcd1 Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Thu, 6 Jul 2017 20:59:45 -0700 Subject: [PATCH 2/5] Moar --- index.js | 14 +++++++------- server/api/documents.js | 8 +------- server/presenters/document.js | 8 ++++++-- server/presenters/team.js | 5 ++++- 4 files changed, 18 insertions(+), 17 deletions(-) diff --git a/index.js b/index.js index 0ac7c2d07..9f822e6c2 100644 --- a/index.js +++ b/index.js @@ -1,13 +1,13 @@ require('./init'); -var app = require('./server').default; -var http = require('http'); +const app = require('./server').default; +const http = require('http'); -var server = http.createServer(app.callback()); +const server = http.createServer(app.callback()); server.listen(process.env.PORT || '3000'); -server.on('error', (err) => { +server.on('error', err => { throw err; }); server.on('listening', () => { - var address = server.address(); - console.log('Listening on %s%s', address.address, address.port); -}); \ No newline at end of file + const address = server.address(); + console.log(`Listening on http://localhost:${address.port}`); +}); diff --git a/server/api/documents.js b/server/api/documents.js index b43ba923e..c362f0979 100644 --- a/server/api/documents.js +++ b/server/api/documents.js @@ -5,7 +5,7 @@ import httpErrors from 'http-errors'; import auth from './middlewares/authentication'; import pagination from './middlewares/pagination'; import { presentDocument } from '../presenters'; -import { User, Document, Collection, Star, View } from '../models'; +import { Document, Collection, Star, View } from '../models'; const router = new Router(); router.post('documents.list', auth(), pagination(), async ctx => { @@ -118,7 +118,6 @@ router.post('documents.search', auth(), async ctx => { documents.map(async document => { data.push( await presentDocument(ctx, document, { - includeCollection: true, includeCollaborators: true, }) ); @@ -206,9 +205,7 @@ router.post('documents.create', auth(), async ctx => { ctx.body = { data: await presentDocument(ctx, newDocument, { - includeCollection: true, includeCollaborators: true, - collection: ownerCollection, }), }; }); @@ -236,9 +233,7 @@ router.post('documents.update', auth(), async ctx => { ctx.body = { data: await presentDocument(ctx, document, { - includeCollection: true, includeCollaborators: true, - collection: collection, }), }; }); @@ -279,7 +274,6 @@ router.post('documents.move', auth(), async ctx => { ctx.body = { data: await presentDocument(ctx, document, { - includeCollection: true, includeCollaborators: true, collection: collection, }), diff --git a/server/presenters/document.js b/server/presenters/document.js index 124f8650c..7e7cb31a6 100644 --- a/server/presenters/document.js +++ b/server/presenters/document.js @@ -1,5 +1,5 @@ // @flow -import { Star, User, Document, View } from '../models'; +import { User, Document, View } from '../models'; import presentUser from './user'; import presentCollection from './collection'; @@ -19,7 +19,6 @@ async function present(ctx: Object, document: Document, options: Object = {}) { text: document.text, html: document.html, preview: document.preview, - collection: presentCollection(ctx, document.collection), createdAt: document.createdAt, createdBy: presentUser(ctx, document.createdBy), updatedAt: document.updatedAt, @@ -27,9 +26,14 @@ async function present(ctx: Object, document: Document, options: Object = {}) { team: document.teamId, collaborators: [], starred: !!document.starred, + collection: undefined, views: undefined, }; + if (options.includeCollection) { + data.collection = presentCollection(ctx, document.collection); + } + if (options.includeViews) { data.views = await View.sum('count', { where: { documentId: document.id }, diff --git a/server/presenters/team.js b/server/presenters/team.js index c418192b2..6587798ad 100644 --- a/server/presenters/team.js +++ b/server/presenters/team.js @@ -1,4 +1,7 @@ -function present(ctx, team) { +// @flow +import { Team } from '../models'; + +function present(ctx: Object, team: Team) { ctx.cache.set(team.id, team); return { From b854c2ca53f2dd2fedf766cf73ea66a86db1cfda Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Thu, 6 Jul 2017 22:02:55 -0700 Subject: [PATCH 3/5] Tidy, move recent documents to query scope --- .../DocumentViews/DocumentViewersStore.js | 2 +- server/api/collections.js | 18 ++++------ server/api/documents.js | 35 ++++++------------- server/models/Collection.js | 10 ++++++ server/models/Document.js | 3 +- server/presenters/collection.js | 35 +++++++------------ server/presenters/document.js | 7 ++-- 7 files changed, 46 insertions(+), 64 deletions(-) diff --git a/frontend/components/DocumentViews/DocumentViewersStore.js b/frontend/components/DocumentViews/DocumentViewersStore.js index 8fda92231..0f620ffb4 100644 --- a/frontend/components/DocumentViews/DocumentViewersStore.js +++ b/frontend/components/DocumentViews/DocumentViewersStore.js @@ -18,7 +18,7 @@ class DocumentViewersStore { this.isFetching = true; try { - const res = await client.get( + const res = await client.post( '/views.list', { id: this.documentId, diff --git a/server/api/collections.js b/server/api/collections.js index b129c0852..1872fd6c8 100644 --- a/server/api/collections.js +++ b/server/api/collections.js @@ -24,7 +24,7 @@ router.post('collections.create', auth(), async ctx => { }); ctx.body = { - data: await presentCollection(ctx, atlas, true), + data: await presentCollection(ctx, atlas), }; }); @@ -33,7 +33,7 @@ router.post('collections.info', auth(), async ctx => { ctx.assertPresent(id, 'id is required'); const user = ctx.state.user; - const atlas = await Collection.findOne({ + const atlas = await Collection.scope('withRecentDocuments').findOne({ where: { id, teamId: user.teamId, @@ -43,13 +43,13 @@ router.post('collections.info', auth(), async ctx => { if (!atlas) throw httpErrors.NotFound(); ctx.body = { - data: await presentCollection(ctx, atlas, true), + data: await presentCollection(ctx, atlas), }; }); router.post('collections.list', auth(), pagination(), async ctx => { const user = ctx.state.user; - const collections = await Collection.findAll({ + const collections = await Collection.scope('withRecentDocuments').findAll({ where: { teamId: user.teamId, }, @@ -58,16 +58,10 @@ router.post('collections.list', auth(), pagination(), async ctx => { limit: ctx.state.pagination.limit, }); - // Collectiones - let data = []; - await Promise.all( - collections.map(async atlas => { - return data.push(await presentCollection(ctx, atlas, true)); - }) + const data = await Promise.all( + collections.map(async atlas => await presentCollection(ctx, atlas)) ); - data = _.orderBy(data, ['updatedAt'], ['desc']); - ctx.body = { pagination: ctx.state.pagination, data, diff --git a/server/api/documents.js b/server/api/documents.js index c362f0979..cf5258611 100644 --- a/server/api/documents.js +++ b/server/api/documents.js @@ -21,7 +21,9 @@ router.post('documents.list', auth(), pagination(), async ctx => { include: [{ model: Star, as: 'starred', where: { userId: user.id } }], }); - let data = await Promise.all(documents.map(doc => presentDocument(ctx, doc))); + const data = await Promise.all( + documents.map(document => presentDocument(ctx, document)) + ); ctx.body = { pagination: ctx.state.pagination, @@ -42,7 +44,7 @@ router.post('documents.viewed', auth(), pagination(), async ctx => { limit: ctx.state.pagination.limit, }); - let data = await Promise.all( + const data = await Promise.all( views.map(view => presentDocument(ctx, view.document)) ); @@ -70,7 +72,7 @@ router.post('documents.starred', auth(), pagination(), async ctx => { limit: ctx.state.pagination.limit, }); - let data = await Promise.all( + const data = await Promise.all( views.map(view => presentDocument(ctx, view.document)) ); @@ -99,8 +101,7 @@ router.post('documents.info', auth(), async ctx => { ctx.body = { data: await presentDocument(ctx, document, { - includeCollection: document.private, - includeCollaborators: true, + includeViews: true, }), }; }); @@ -113,15 +114,8 @@ router.post('documents.search', auth(), async ctx => { const documents = await Document.searchForUser(user, query); - const data = []; - await Promise.all( - documents.map(async document => { - data.push( - await presentDocument(ctx, document, { - includeCollaborators: true, - }) - ); - }) + const data = await Promise.all( + documents.map(async document => await presentDocument(ctx, document)) ); ctx.body = { @@ -204,9 +198,7 @@ router.post('documents.create', auth(), async ctx => { } ctx.body = { - data: await presentDocument(ctx, newDocument, { - includeCollaborators: true, - }), + data: await presentDocument(ctx, newDocument), }; }); @@ -232,9 +224,7 @@ router.post('documents.update', auth(), async ctx => { } ctx.body = { - data: await presentDocument(ctx, document, { - includeCollaborators: true, - }), + data: await presentDocument(ctx, document), }; }); @@ -273,10 +263,7 @@ router.post('documents.move', auth(), async ctx => { } ctx.body = { - data: await presentDocument(ctx, document, { - includeCollaborators: true, - collection: collection, - }), + data: await presentDocument(ctx, document), }; }); diff --git a/server/models/Collection.js b/server/models/Collection.js index 953a57aeb..382b54768 100644 --- a/server/models/Collection.js +++ b/server/models/Collection.js @@ -60,6 +60,16 @@ const Collection = sequelize.define( as: 'documents', foreignKey: 'atlasId', }); + Collection.addScope('withRecentDocuments', { + include: [ + { + as: 'documents', + limit: 10, + model: models.Document, + order: [['updatedAt', 'DESC']], + }, + ], + }); }, }, instanceMethods: { diff --git a/server/models/Document.js b/server/models/Document.js index 82b5c439c..121b2523a 100644 --- a/server/models/Document.js +++ b/server/models/Document.js @@ -116,6 +116,7 @@ const Document = sequelize.define( classMethods: { associate: models => { Document.belongsTo(models.Collection, { + as: 'collection', foreignKey: 'atlasId', }); Document.belongsTo(models.User, { @@ -133,7 +134,7 @@ const Document = sequelize.define( 'defaultScope', { include: [ - { model: models.Collection }, + { model: models.Collection, as: 'collection' }, { model: models.User, as: 'createdBy' }, { model: models.User, as: 'updatedBy' }, ], diff --git a/server/presenters/collection.js b/server/presenters/collection.js index 8dc1777db..a6fdbc8ee 100644 --- a/server/presenters/collection.js +++ b/server/presenters/collection.js @@ -1,8 +1,9 @@ +// @flow import _ from 'lodash'; -import { Document } from '../models'; +import { Collection } from '../models'; import presentDocument from './document'; -async function present(ctx, collection, includeRecentDocuments = false) { +async function present(ctx: Object, collection: Collection) { ctx.cache.set(collection.id, collection); const data = { @@ -13,31 +14,21 @@ async function present(ctx, collection, includeRecentDocuments = false) { type: collection.type, createdAt: collection.createdAt, updatedAt: collection.updatedAt, + recentDocuments: undefined, + documents: undefined, }; - if (collection.type === 'atlas') + if (collection.type === 'atlas') { data.documents = await collection.getDocumentsStructure(); + } - if (includeRecentDocuments) { - const documents = await Document.findAll({ - where: { - atlasId: collection.id, - }, - limit: 10, - order: [['updatedAt', 'DESC']], - }); - - const recentDocuments = []; - await Promise.all( - documents.map(async document => { - recentDocuments.push( - await presentDocument(ctx, document, { - includeCollaborators: true, - }) - ); - }) + if (collection.documents) { + data.recentDocuments = await Promise.all( + collection.documents.map( + async document => + await presentDocument(ctx, document, { includeCollaborators: true }) + ) ); - data.recentDocuments = _.orderBy(recentDocuments, ['updatedAt'], ['desc']); } return data; diff --git a/server/presenters/document.js b/server/presenters/document.js index 7e7cb31a6..80db7f531 100644 --- a/server/presenters/document.js +++ b/server/presenters/document.js @@ -5,9 +5,8 @@ import presentCollection from './collection'; async function present(ctx: Object, document: Document, options: Object = {}) { options = { - includeCollection: true, includeCollaborators: true, - includeViews: true, + includeViews: false, ...options, }; ctx.cache.set(document.id, document); @@ -30,8 +29,8 @@ async function present(ctx: Object, document: Document, options: Object = {}) { views: undefined, }; - if (options.includeCollection) { - data.collection = presentCollection(ctx, document.collection); + if (document.private) { + data.collection = await presentCollection(ctx, document.collection); } if (options.includeViews) { From c618b956d21e1ac3950929767df031e7c544eff1 Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Thu, 6 Jul 2017 22:17:37 -0700 Subject: [PATCH 4/5] Incorporate limit --- server/middlewares/cache.js | 3 ++- server/presenters/document.js | 5 ++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/server/middlewares/cache.js b/server/middlewares/cache.js index 7a10ad642..22068f621 100644 --- a/server/middlewares/cache.js +++ b/server/middlewares/cache.js @@ -1,9 +1,10 @@ +// @flow import debug from 'debug'; const debugCache = debug('cache'); export default function cache() { - return async function cacheMiddleware(ctx, next) { + return async function cacheMiddleware(ctx: Object, next: Function) { ctx.cache = {}; ctx.cache.set = async (id, value) => { diff --git a/server/presenters/document.js b/server/presenters/document.js index 80db7f531..e89658a56 100644 --- a/server/presenters/document.js +++ b/server/presenters/document.js @@ -1,4 +1,5 @@ // @flow +import _ from 'lodash'; import { User, Document, View } from '../models'; import presentUser from './user'; import presentCollection from './collection'; @@ -43,9 +44,7 @@ async function present(ctx: Object, document: Document, options: Object = {}) { // This could be further optimized by using ctx.cache data.collaborators = await User.findAll({ where: { - id: { - $in: document.collaboratorIds || [], - }, + id: { $in: _.takeRight(document.collaboratorIds, 10) || [] }, }, }).map(user => presentUser(ctx, user)); } From ff133f373c65082167928c1e4041f4726da7243b Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Thu, 6 Jul 2017 22:20:24 -0700 Subject: [PATCH 5/5] Remove recentDocuments from default collections list response --- server/api/collections.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/api/collections.js b/server/api/collections.js index 1872fd6c8..2b48f40fb 100644 --- a/server/api/collections.js +++ b/server/api/collections.js @@ -49,7 +49,7 @@ router.post('collections.info', auth(), async ctx => { router.post('collections.list', auth(), pagination(), async ctx => { const user = ctx.state.user; - const collections = await Collection.scope('withRecentDocuments').findAll({ + const collections = await Collection.findAll({ where: { teamId: user.teamId, },