From e0a6d027c8f7f9f0a21645c35aaeeb1bd0eaa3b7 Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Tue, 3 Nov 2015 11:38:29 +0000 Subject: [PATCH] Move cross-table api counts into plugin refs #6009, #5615 - minimal refactor to remove the addition of count from pagination and other various points - create a include count plugin that overrides fetch and fetchAll - this ensures that counts get added at the right points --- core/server/models/base/include-count.js | 53 ++++++++++++++++++++++++ core/server/models/base/index.js | 6 ++- core/server/models/base/pagination.js | 4 -- core/server/models/base/utils.js | 21 ---------- core/server/models/tag.js | 3 -- 5 files changed, 58 insertions(+), 29 deletions(-) create mode 100644 core/server/models/base/include-count.js diff --git a/core/server/models/base/include-count.js b/core/server/models/base/include-count.js new file mode 100644 index 0000000000..b9ef5af6d8 --- /dev/null +++ b/core/server/models/base/include-count.js @@ -0,0 +1,53 @@ +var _ = require('lodash'); + +module.exports = function (Bookshelf) { + var modelProto = Bookshelf.Model.prototype, + Model, + countQueryBuilder; + + countQueryBuilder = { + tags: { + posts: function addPostCountToTags(model) { + model.query('columns', 'tags.*', function (qb) { + qb.count('posts_tags.post_id') + .from('posts_tags') + .whereRaw('tag_id = tags.id') + .as('post_count'); + }); + } + } + }; + + Model = Bookshelf.Model.extend({ + addCounts: function (options) { + if (!options) { + return; + } + + var tableName = _.result(this, 'tableName'); + + if (options.include && options.include.indexOf('post_count') > -1) { + // remove post_count from withRelated and include + options.withRelated = _.pull([].concat(options.withRelated), 'post_count'); + options.include = _.pull([].concat(options.include), 'post_count'); + + // Call the query builder + countQueryBuilder[tableName].posts(this); + } + }, + fetch: function () { + this.addCounts.apply(this, arguments); + + // Call parent fetch + return modelProto.fetch.apply(this, arguments); + }, + fetchAll: function () { + this.addCounts.apply(this, arguments); + + // Call parent fetchAll + return modelProto.fetchAll.apply(this, arguments); + } + }); + + Bookshelf.Model = Model; +}; diff --git a/core/server/models/base/index.js b/core/server/models/base/index.js index df08f6f977..1800961a2f 100644 --- a/core/server/models/base/index.js +++ b/core/server/models/base/index.js @@ -18,6 +18,7 @@ var _ = require('lodash'), uuid = require('node-uuid'), validation = require('../../data/validation'), baseUtils = require('./utils'), + includeCount = require('./include-count'), pagination = require('./pagination'), gql = require('ghost-gql'), @@ -30,6 +31,9 @@ ghostBookshelf = bookshelf(config.database.knex); // Load the Bookshelf registry plugin, which helps us avoid circular dependencies ghostBookshelf.plugin('registry'); +// Load the Ghost include count plugin, which allows for the inclusion of cross-table counts +ghostBookshelf.plugin(includeCount); + // Load the Ghost pagination plugin, which gives us the `fetchPage` method on Models ghostBookshelf.plugin(pagination); @@ -313,7 +317,7 @@ ghostBookshelf.Model = ghostBookshelf.Model.extend({ data.meta = {pagination: response.pagination}; return data; - }).catch(errors.logAndThrowError); + }); }, /** diff --git a/core/server/models/base/pagination.js b/core/server/models/base/pagination.js index 7433087595..66623a4cc5 100644 --- a/core/server/models/base/pagination.js +++ b/core/server/models/base/pagination.js @@ -3,7 +3,6 @@ // Extends Bookshelf.Model with a `fetchPage` method. Handles everything to do with paginated requests. var _ = require('lodash'), Promise = require('bluebird'), - baseUtils = require('./utils'), defaults, paginationUtils, @@ -173,9 +172,6 @@ pagination = function pagination(bookshelf) { }); } - // Apply count options if they are present - baseUtils.collectionQuery.count(self, options); - // Setup the promise to do a fetch on our collection, running the specified query // @TODO: ensure option handling is done using an explicit pick elsewhere collectionPromise = self.fetchAll(_.omit(options, ['page', 'limit'])); diff --git a/core/server/models/base/utils.js b/core/server/models/base/utils.js index 504e491862..f60cfd810b 100644 --- a/core/server/models/base/utils.js +++ b/core/server/models/base/utils.js @@ -3,28 +3,9 @@ * Parts of the model code which can be split out and unit tested */ var _ = require('lodash'), - collectionQuery, processGQLResult, - addPostCount, tagUpdate; -addPostCount = function addPostCount(options, model) { - if (options.include && options.include.indexOf('post_count') > -1) { - model.query('columns', 'tags.*', function (qb) { - qb.count('posts_tags.post_id').from('posts_tags').whereRaw('tag_id = tags.id').as('post_count'); - }); - - options.withRelated = _.pull([].concat(options.withRelated), 'post_count'); - options.include = _.pull([].concat(options.include), 'post_count'); - } -}; - -collectionQuery = { - count: function count(model, options) { - addPostCount(options, model); - } -}; - processGQLResult = function processGQLResult(itemCollection, options) { var joinTables = options.filter.joins, tagsHasIn = false; @@ -126,6 +107,4 @@ tagUpdate = { }; module.exports.processGQLResult = processGQLResult; -module.exports.collectionQuery = collectionQuery; -module.exports.addPostCount = addPostCount; module.exports.tagUpdate = tagUpdate; diff --git a/core/server/models/tag.js b/core/server/models/tag.js index ee38f4db98..173eba3501 100644 --- a/core/server/models/tag.js +++ b/core/server/models/tag.js @@ -1,7 +1,6 @@ var _ = require('lodash'), ghostBookshelf = require('./base'), events = require('../events'), - baseUtils = require('./base/utils'), Tag, Tags; @@ -101,8 +100,6 @@ Tag = ghostBookshelf.Model.extend({ var tag = this.forge(data); - baseUtils.addPostCount(options, tag); - // Add related objects options.withRelated = _.union(options.withRelated, options.include);