Changed member limit to be DRY & use raw query

- Member limit code was duplicated in 2 places unnecessarily
- Also used member api code that fetched members and subscriptions fully hyrated when we only need a count
- Using a raw query significantly improves performance here

core/server/api/canary/posts.js

@@ -6,8 +6,6 @@ const {mega} = require('../../services/mega');
 const membersService = require('../../services/members');
 const allowedIncludes = ['tags', 'authors', 'authors.roles', 'email'];
 const unsafeAttrs = ['status', 'authors', 'visibility'];
-const _ = require('lodash');
-const config = require('../../../shared/config');
 
 module.exports = {
     docName: 'posts',
@@ -150,23 +148,9 @@ module.exports = {
             unsafeAttrs: unsafeAttrs
         },
         async query(frame) {
-            /**Check host limits for members when send email is true*/
+            /**Check host limits for members when send email is true**/
-            const membersHostLimit = config.get('host_settings:limits:members');
+            if (frame.options.send_email_when_published) {
-            if (frame.options.send_email_when_published && membersHostLimit) {
+                await membersService.checkHostLimit();
-                const allowedMembersLimit = membersHostLimit.max;
-                const hostUpgradeLink = config.get('host_settings:limits').upgrade_url;
-                const knexOptions = _.pick(frame.options, ['transacting', 'forUpdate']);
-                const {members} = await membersService.api.members.list(Object.assign(knexOptions, {filter: 'subscribed:true'}, {limit: 'all'}));
-                if (members.length > allowedMembersLimit) {
-                    throw new errors.HostLimitError({
-                        message: `Your current plan allows you to send email to up to ${allowedMembersLimit} members, but you currently have ${members.length} members`,
-                        help: hostUpgradeLink,
-                        errorDetails: {
-                            limit: allowedMembersLimit,
-                            total: members.length
-                        }
-                    });
-                }
             }
 
             let model = await models.Post.edit(frame.data.posts[0], frame.options);

core/server/services/mega/mega.js

@@ -1,4 +1,5 @@
 const _ = require('lodash');
+const debug = require('ghost-ignition').debug('mega');
 const url = require('url');
 const moment = require('moment');
 const errors = require('@tryghost/errors');
@@ -8,7 +9,6 @@ const membersService = require('../members');
 const bulkEmailService = require('../bulk-email');
 const models = require('../../models');
 const postEmailSerializer = require('./post-email-serializer');
-const config = require('../../../shared/config');
 
 const getEmailData = async (postModel, members = []) => {
     const {emailTmpl, replacements} = await postEmailSerializer.serialize(postModel);
@@ -81,10 +81,13 @@ const sendTestEmail = async (postModel, toEmails) => {
 const addEmail = async (postModel, options) => {
     const knexOptions = _.pick(options, ['transacting', 'forUpdate']);
 
+    // @TODO: improve performance of this members.list call
+    debug('addEmail: retrieving members list');
     const {members} = await membersService.api.members.list(Object.assign(knexOptions, {filter: 'subscribed:true'}, {limit: 'all'}));
     const membersToSendTo = members.filter((member) => {
         return membersService.contentGating.checkPostAccess(postModel.toJSON(), member);
     });
+    debug('addEmail: retrieved members list');
 
     // NOTE: don't create email object when there's nobody to send the email to
     if (!membersToSendTo.length) {
@@ -180,24 +183,6 @@ async function handleUnsubscribeRequest(req) {
     }
 }
 
-function checkHostLimitForMembers(members = []) {
-    const membersHostLimit = config.get('host_settings:limits:members');
-    if (membersHostLimit) {
-        const allowedMembersLimit = membersHostLimit.max;
-        const hostUpgradeLink = config.get('host_settings:limits').upgrade_url;
-        if (members.length > allowedMembersLimit) {
-            throw new errors.HostLimitError({
-                message: `Your current plan allows you to send email to up to ${allowedMembersLimit} members, but you currently have ${members.length} members`,
-                help: hostUpgradeLink,
-                errorDetails: {
-                    limit: allowedMembersLimit,
-                    total: members.length
-                }
-            });
-        }
-    }
-}
-
 async function pendingEmailHandler(emailModel, options) {
     // CASE: do not send email if we import a database
     // TODO: refactor post.published events to never fire on importing
@@ -210,24 +195,29 @@ async function pendingEmailHandler(emailModel, options) {
         return;
     }
 
-    const {members} = await membersService.api.members.list(Object.assign({filter: 'subscribed:true'}, {limit: 'all'}));
-
-    if (!members.length) {
-        return;
-    }
-
-    await models.Email.edit({
-        status: 'submitting'
-    }, {
-        id: emailModel.id
-    });
-
     let meta = [];
     let error = null;
 
     try {
         // Check host limit for allowed member count and throw error if over limit
-        checkHostLimitForMembers(members);
+        await membersService.checkHostLimit();
+
+        // No need to fetch list until after we've passed the check
+        // @TODO: improve performance of this members.list call
+        debug('pendingEmailHandler: retrieving members list');
+        const {members} = await membersService.api.members.list(Object.assign({filter: 'subscribed:true'}, {limit: 'all'}));
+        debug('pendingEmailHandler: retrieved members list');
+
+        if (!members.length) {
+            return;
+        }
+
+        await models.Email.edit({
+            status: 'submitting'
+        }, {
+            id: emailModel.id
+        });
+
         // NOTE: meta can contains an array which can be a mix of successful and error responses
         //       needs filtering and saving objects of {error, batchData} form to separate property
         meta = await sendEmail(postModel, members);

core/server/services/members/index.js

@@ -58,6 +58,8 @@ events.on('settings.edited', function updateSettingFromModel(settingModel) {
 const membersService = {
     contentGating: require('./content-gating'),
 
+    checkHostLimit: require('./limit'),
+
     config: membersConfig,
 
     get api() {

core/server/services/members/limit.js

@@ -0,0 +1,32 @@
+const config = require('../../../shared/config');
+const db = require('../../data/db');
+const errors = require('@tryghost/errors');
+
+// Get total members direct from DB
+// @TODO: determine performance difference between this, normal knex, and using the model layer
+async function getTotalMembers() {
+    const isSQLite = config.get('database:client') === 'sqlite3';
+    const result = await db.knex.raw('SELECT COUNT(id) AS total FROM members');
+    return isSQLite ? result[0].total : result[0][0].total;
+}
+
+module.exports = async () => {
+    const membersHostLimit = config.get('host_settings:limits:members');
+    if (membersHostLimit) {
+        const allowedMembersLimit = membersHostLimit.max;
+        const hostUpgradeLink = config.get('host_settings:limits').upgrade_url;
+
+        const totalMembers = await getTotalMembers();
+
+        if (totalMembers > allowedMembersLimit) {
+            throw new errors.HostLimitError({
+                message: `Your current plan allows you to send email to up to ${allowedMembersLimit} members, but you currently have ${totalMembers} members`,
+                help: hostUpgradeLink,
+                errorDetails: {
+                    limit: allowedMembersLimit,
+                    total: totalMembers
+                }
+            });
+        }
+    }
+};