Infrastructure/ghost
0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-02-10 23:36:14 -05:00
Code Issues Activity

Added transaction support to limit-service (#190)

Browse source 
refs https://github.com/TryGhost/Ghost/pull/14780
refs https://github.com/TryGhost/Team/issues/1583

- We need transaction support in the limit-service so that we can run the count queries in the same transaction
- This is required to avoid deadlocks when we check the limits when a transaction is in progress on the same tables
- This issue specifically is required for newsletters, where we start a transaction when creating a newsletter.
- Bumped `eslint-plugin-ghost` so we have newer ECMA features available
- Updated README
- Renamed `metadata` to `options` in `limit-service`
This commit is contained in:
Simon Backx 2022-05-12 13:40:41 +02:00 • committed by GitHub
parent 9f96d256bc
commit cb7e7d34da
6 changed files with 437 additions and 61 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

37
ghost/limit-service/README.md
View file

@ -17,6 +17,7 @@ or
Below is a sample code to wire up limit service and perform few common limit checks:
```js
const knex = require('knex');
const errors = require('@tryghost/errors');
const LimitService = require('@tryghost/limit-service');
@ -80,15 +81,17 @@ const subscription = {
const helpLink = 'https://ghost.org/help/';
// initialize knex db connection for the limit service to use when running query checks
const db = knex({
client: 'mysql',
connection: {
user: 'root',
password: 'toor',
host: 'localhost',
database: 'ghost',
}
});
const db = {
knex: knex({
client: 'mysql',
connection: {
user: 'root',
password: 'toor',
host: 'localhost',
database: 'ghost',
}
});
};
// finish initializing the limits service
limitService.loadLimits({limits, subscription, db, helpLink, errors});
@ -133,6 +136,22 @@ if (limitService.checkIfAnyOverLimit()) {
}
```
### Transactions
Some limit types (`max` or `maxPeriodic`) need to fetch the current count from the database. Sometimes you need those checks to also run in a transaction. To fix that, you can pass the `transacting` option to all the available checks.
```js
db.transaction((transacting) => {
const options = {transacting};
await limitService.errorIfWouldGoOverLimit('newsletters', options);
await limitService.errorIfIsOverLimit('newsletters', options);
const a = await limitService.checkIsOverLimit('newsletters', options);
const b = await limitService.checkWouldGoOverLimit('newsletters', options);
const c = await limitService.checkIfAnyOverLimit(options);
});
```
### Types of limits
At the moment there are four different types of limits that limit service allows to define. These types are:
1. `flag` - is an "on/off" switch for certain feature. Example usecase: "disable all emails". It's identified by a `disabled: true` property in the "limits" configuration.

22
ghost/limit-service/lib/config.js
View file

@ -4,14 +4,14 @@
// 2. MaxLimit should contain a `currentCountQuery` function which would count the resources under limit
module.exports = {
members: {
currentCountQuery: async (db) => {
let result = await db.knex('members').count('id', {as: 'count'}).first();
currentCountQuery: async (knex) => {
let result = await knex('members').count('id', {as: 'count'}).first();
return result.count;
}
},
newsletters: {
currentCountQuery: async (db) => {
let result = await db.knex('newsletters')
currentCountQuery: async (knex) => {
let result = await knex('newsletters')
.count('id', {as: 'count'})
.where('status', '=', 'active')
.first();
@ -20,8 +20,8 @@ module.exports = {
}
},
emails: {
currentCountQuery: async (db, startDate) => {
let result = await db.knex('emails')
currentCountQuery: async (knex, startDate) => {
let result = await knex('emails')
.sum('email_count', {as: 'count'})
.where('created_at', '>=', startDate)
.first();
@ -30,13 +30,13 @@ module.exports = {
}
},
staff: {
currentCountQuery: async (db) => {
let result = await db.knex('users')
currentCountQuery: async (knex) => {
let result = await knex('users')
.select('users.id')
.leftJoin('roles_users', 'users.id', 'roles_users.user_id')
.leftJoin('roles', 'roles_users.role_id', 'roles.id')
.whereNot('roles.name', 'Contributor').andWhereNot('users.status', 'inactive').union([
db.knex('invites')
knex('invites')
.select('invites.id')
.leftJoin('roles', 'invites.role_id', 'roles.id')
.whereNot('roles.name', 'Contributor')
@ -46,8 +46,8 @@ module.exports = {
}
},
customIntegrations: {
currentCountQuery: async (db) => {
let result = await db.knex('integrations')
currentCountQuery: async (knex) => {
let result = await knex('integrations')
.count('id', {as: 'count'})
.whereNotIn('type', ['internal', 'builtin'])
.first();

74
ghost/limit-service/lib/limit-service.js
View file

@ -67,30 +67,20 @@ class LimitService {
return !!this.limits[_.camelCase(limitName)];
}
async checkIsOverLimit(limitName) {
/**
*
* @param {String} limitName - name of the configured limit
* @param {Object} [options] - limit parameters
* @param {Object} [options.transacting] Transaction to run the count query on (if required for the chosen limit)
* @returns
*/
async checkIsOverLimit(limitName, options = {}) {
if (!this.isLimited(limitName)) {
return;
}
try {
await this.limits[limitName].errorIfIsOverLimit();
return false;
} catch (error) {
if (error instanceof this.errors.HostLimitError) {
return true;
}
throw error;
}
}
async checkWouldGoOverLimit(limitName, metadata = {}) {
if (!this.isLimited(limitName)) {
return;
}
try {
await this.limits[limitName].errorIfWouldGoOverLimit(metadata);
await this.limits[limitName].errorIfIsOverLimit(options);
return false;
} catch (error) {
if (error instanceof this.errors.HostLimitError) {
@ -104,39 +94,67 @@ class LimitService {
/**
*
* @param {String} limitName - name of the configured limit
* @param {Object} metadata - limit parameters
* @param {Object} [options] - limit parameters
* @param {Object} [options.transacting] Transaction to run the count query on (if required for the chosen limit)
* @returns
*/
async errorIfIsOverLimit(limitName, metadata = {}) {
async checkWouldGoOverLimit(limitName, options = {}) {
if (!this.isLimited(limitName)) {
return;
}
await this.limits[limitName].errorIfIsOverLimit(metadata);
try {
await this.limits[limitName].errorIfWouldGoOverLimit(options);
return false;
} catch (error) {
if (error instanceof this.errors.HostLimitError) {
return true;
}
throw error;
}
}
/**
*
* @param {String} limitName - name of the configured limit
* @param {Object} metadata - limit parameters
* @param {Object} [options] - limit parameters
* @param {Object} [options.transacting] Transaction to run the count query on (if required for the chosen limit)
* @returns
*/
async errorIfWouldGoOverLimit(limitName, metadata = {}) {
async errorIfIsOverLimit(limitName, options = {}) {
if (!this.isLimited(limitName)) {
return;
}
await this.limits[limitName].errorIfWouldGoOverLimit(metadata);
await this.limits[limitName].errorIfIsOverLimit(options);
}
/**
*
* @param {String} limitName - name of the configured limit
* @param {Object} [options] - limit parameters
* @param {Object} [options.transacting] Transaction to run the count query on (if required for the chosen limit)
* @returns
*/
async errorIfWouldGoOverLimit(limitName, options = {}) {
if (!this.isLimited(limitName)) {
return;
}
await this.limits[limitName].errorIfWouldGoOverLimit(options);
}
/**
* Checks if any of the configured limits acceded
*
*
* @param {Object} [options] - limit parameters
* @param {Object} [options.transacting] Transaction to run the count queries on (if required for the chosen limit)
* @returns {Promise<boolean>}
*/
async checkIfAnyOverLimit() {
async checkIfAnyOverLimit(options = {}) {
for (const limit in this.limits) {
if (await this.checkIsOverLimit(limit)) {
if (await this.checkIsOverLimit(limit, options)) {
return true;
}
}

43
ghost/limit-service/lib/limit.js
View file

@ -98,8 +98,13 @@ class MaxLimit extends Limit {
return new this.errors.HostLimitError(errorObj);
}
async currentCountQuery() {
return await this.currentCountQueryFn(this.db);
/**
* @param {Object} [options]
* @param {Object} [options.transacting] Transaction to run the count query on
* @returns
*/
async currentCountQuery(options = {}) {
return await this.currentCountQueryFn(options.transacting ?? this.db?.knex);
}
/**
@ -108,9 +113,11 @@ class MaxLimit extends Limit {
* @param {Object} options
* @param {Number} [options.max] - overrides configured default max value to perform checks against
* @param {Number} [options.addedCount] - number of items to add to the currentCount during the check
* @param {Object} [options.transacting] Transaction to run the count query on
*/
async errorIfWouldGoOverLimit({max, addedCount = 1} = {}) {
let currentCount = await this.currentCountQuery();
async errorIfWouldGoOverLimit(options = {}) {
const {max, addedCount = 1} = options;
let currentCount = await this.currentCountQuery(options);
if ((currentCount + addedCount) > (max || this.max)) {
throw this.generateError(currentCount);
@ -123,11 +130,12 @@ class MaxLimit extends Limit {
* @param {Object} options
* @param {Number} [options.max] - overrides configured default max value to perform checks against
* @param {Number} [options.currentCount] - overrides currentCountQuery to perform checks against
* @param {Object} [options.transacting] Transaction to run the count query on
*/
async errorIfIsOverLimit({max, currentCount} = {}) {
currentCount = currentCount || await this.currentCountQuery();
async errorIfIsOverLimit(options = {}) {
const currentCount = options.currentCount || await this.currentCountQuery(options);
if (currentCount > (max || this.max)) {
if (currentCount > (options.max || this.max)) {
throw this.generateError(currentCount);
}
}
@ -202,10 +210,15 @@ class MaxPeriodicLimit extends Limit {
return new this.errors.HostLimitError(errorObj);
}
async currentCountQuery() {
/**
* @param {Object} [options]
* @param {Object} [options.transacting] Transaction to run the count query on
* @returns
*/
async currentCountQuery(options = {}) {
const lastPeriodStartDate = lastPeriodStart(this.startDate, this.interval);
return await this.currentCountQueryFn(this.db, lastPeriodStartDate);
return await this.currentCountQueryFn(options.transacting ? options.transacting : (this.db ? this.db.knex : undefined), lastPeriodStartDate);
}
/**
@ -214,9 +227,11 @@ class MaxPeriodicLimit extends Limit {
* @param {Object} options
* @param {Number} [options.max] - overrides configured default maxPeriodic value to perform checks against
* @param {Number} [options.addedCount] - number of items to add to the currentCount during the check
* @param {Object} [options.transacting] Transaction to run the count query on
*/
async errorIfWouldGoOverLimit({max, addedCount = 1} = {}) {
let currentCount = await this.currentCountQuery();
async errorIfWouldGoOverLimit(options = {}) {
const {max, addedCount = 1} = options;
let currentCount = await this.currentCountQuery(options);
if ((currentCount + addedCount) > (max || this.maxPeriodic)) {
throw this.generateError(currentCount);
@ -228,9 +243,11 @@ class MaxPeriodicLimit extends Limit {
*
* @param {Object} options
* @param {Number} [options.max] - overrides configured default maxPeriodic value to perform checks against
* @param {Object} [options.transacting] Transaction to run the count query on
*/
async errorIfIsOverLimit({max} = {}) {
let currentCount = await this.currentCountQuery();
async errorIfIsOverLimit(options = {}) {
const {max} = options;
let currentCount = await this.currentCountQuery(options);
if (currentCount > (max || this.maxPeriodic)) {
throw this.generateError(currentCount);

171
ghost/limit-service/test/limit-service.test.js
View file

@ -4,6 +4,7 @@ require('./utils');
const should = require('should');
const LimitService = require('../lib/limit-service');
const {MaxLimit, MaxPeriodicLimit, FlagLimit} = require('../lib/limit');
const sinon = require('sinon');
const errors = require('./fixtures/errors');
@ -340,4 +341,174 @@ describe('Limit Service', function () {
}
});
});
describe('Metadata', function () {
afterEach(function () {
sinon.restore();
});
it('passes options for checkIsOverLimit', async function () {
const limitService = new LimitService();
let limits = {
staff: {
max: 2,
currentCountQuery: () => 1
}
};
const maxSpy = sinon.spy(MaxLimit.prototype, 'errorIfIsOverLimit');
const subscription = {
interval: 'month',
startDate: '2021-09-18T19:00:52Z'
};
limitService.loadLimits({limits, errors, subscription});
const options = {
testData: 'true'
};
await limitService.checkIsOverLimit('staff', options);
sinon.assert.callCount(maxSpy, 1);
sinon.assert.alwaysCalledWithExactly(maxSpy, options);
});
it('passes options for checkWouldGoOverLimit', async function () {
const limitService = new LimitService();
let limits = {
staff: {
max: 2,
currentCountQuery: () => 1
}
};
const maxSpy = sinon.spy(MaxLimit.prototype, 'errorIfWouldGoOverLimit');
const subscription = {
interval: 'month',
startDate: '2021-09-18T19:00:52Z'
};
limitService.loadLimits({limits, errors, subscription});
const options = {
testData: 'true'
};
await limitService.checkWouldGoOverLimit('staff', options);
sinon.assert.callCount(maxSpy, 1);
sinon.assert.alwaysCalledWithExactly(maxSpy, options);
});
it('passes options for errorIfIsOverLimit', async function () {
const limitService = new LimitService();
let limits = {
staff: {
max: 2,
currentCountQuery: () => 1
}
};
const maxSpy = sinon.spy(MaxLimit.prototype, 'errorIfIsOverLimit');
const subscription = {
interval: 'month',
startDate: '2021-09-18T19:00:52Z'
};
limitService.loadLimits({limits, errors, subscription});
const options = {
testData: 'true'
};
await limitService.errorIfIsOverLimit('staff', options);
sinon.assert.callCount(maxSpy, 1);
sinon.assert.alwaysCalledWithExactly(maxSpy, options);
});
it('passes options for errorIfWouldGoOverLimit', async function () {
const limitService = new LimitService();
let limits = {
staff: {
max: 2,
currentCountQuery: () => 1
}
};
const maxSpy = sinon.spy(MaxLimit.prototype, 'errorIfWouldGoOverLimit');
const subscription = {
interval: 'month',
startDate: '2021-09-18T19:00:52Z'
};
limitService.loadLimits({limits, errors, subscription});
const options = {
testData: 'true'
};
await limitService.errorIfWouldGoOverLimit('staff', options);
sinon.assert.callCount(maxSpy, 1);
sinon.assert.alwaysCalledWithExactly(maxSpy, options);
});
it('passes options for checkIfAnyOverLimit', async function () {
const limitService = new LimitService();
let limits = {
staff: {
max: 2,
currentCountQuery: () => 2
},
members: {
max: 100,
currentCountQuery: () => 100
},
emails: {
maxPeriodic: 3,
currentCountQuery: () => 3
},
customIntegrations: {
disabled: true
}
};
const flagSpy = sinon.spy(FlagLimit.prototype, 'errorIfIsOverLimit');
const maxSpy = sinon.spy(MaxLimit.prototype, 'errorIfIsOverLimit');
const maxPeriodSpy = sinon.spy(MaxPeriodicLimit.prototype, 'errorIfIsOverLimit');
const subscription = {
interval: 'month',
startDate: '2021-09-18T19:00:52Z'
};
limitService.loadLimits({limits, errors, subscription});
const options = {
testData: 'true'
};
(await limitService.checkIfAnyOverLimit(options)).should.be.false();
sinon.assert.callCount(flagSpy, 1);
sinon.assert.alwaysCalledWithExactly(flagSpy, options);
sinon.assert.callCount(maxSpy, 2);
sinon.assert.alwaysCalledWithExactly(maxSpy, options);
sinon.assert.callCount(maxPeriodSpy, 1);
sinon.assert.alwaysCalledWithExactly(maxPeriodSpy, options);
});
});
});

151
ghost/limit-service/test/limit.test.js
View file

@ -2,6 +2,7 @@
// const testUtils = require('./utils');
require('./utils');
const should = require('should');
const sinon = require('sinon');
const errors = require('./fixtures/errors');
const {MaxLimit, AllowlistLimit, FlagLimit, MaxPeriodicLimit} = require('../lib/limit');
@ -278,6 +279,78 @@ describe('Limit Service', function () {
}
});
});
describe('Transactions', function () {
it('passes undefined if no db or transacting option passed', async function () {
const config = {
max: 5,
error: 'You have gone over the limit',
currentCountQuery: sinon.stub()
};
config.currentCountQuery.resolves(0);
try {
const limit = new MaxLimit({name: '', config, errors});
await limit.errorIfIsOverLimit();
await limit.errorIfWouldGoOverLimit();
} catch (error) {
should.fail('Should have not errored', error);
}
sinon.assert.calledTwice(config.currentCountQuery);
sinon.assert.alwaysCalledWithExactly(config.currentCountQuery, undefined);
});
it('passes default db if no transacting option passed', async function () {
const config = {
max: 5,
error: 'You have gone over the limit',
currentCountQuery: sinon.stub()
};
const db = {
knex: 'This is our connection'
};
config.currentCountQuery.resolves(0);
try {
const limit = new MaxLimit({name: '', config, db, errors});
await limit.errorIfIsOverLimit();
await limit.errorIfWouldGoOverLimit();
} catch (error) {
should.fail('Should have not errored', error);
}
sinon.assert.calledTwice(config.currentCountQuery);
sinon.assert.alwaysCalledWithExactly(config.currentCountQuery, db.knex);
});
it('passes transacting option', async function () {
const config = {
max: 5,
error: 'You have gone over the limit',
currentCountQuery: sinon.stub()
};
const db = {
knex: 'This is our connection'
};
const transaction = 'Our transaction';
config.currentCountQuery.resolves(0);
try {
const limit = new MaxLimit({name: '', config, db, errors});
await limit.errorIfIsOverLimit({transacting: transaction});
await limit.errorIfWouldGoOverLimit({transacting: transaction});
} catch (error) {
should.fail('Should have not errored', error);
}
sinon.assert.calledTwice(config.currentCountQuery);
sinon.assert.alwaysCalledWithExactly(config.currentCountQuery, transaction);
});
});
});
describe('Periodic Max Limit', function () {
@ -490,6 +563,84 @@ describe('Limit Service', function () {
}
});
});
describe('Transactions', function () {
it('passes undefined if no db or transacting option passed', async function () {
const config = {
maxPeriodic: 5,
error: 'You have exceeded the number of emails you can send within your billing period.',
interval: 'month',
startDate: '2021-01-01T00:00:00Z',
currentCountQuery: sinon.stub()
};
config.currentCountQuery.resolves(0);
try {
const limit = new MaxPeriodicLimit({name: 'mailguard', config, errors});
await limit.errorIfIsOverLimit();
await limit.errorIfWouldGoOverLimit();
} catch (error) {
should.fail('Should have not errored', error);
}
sinon.assert.calledTwice(config.currentCountQuery);
sinon.assert.alwaysCalledWith(config.currentCountQuery, undefined);
});
it('passes default db if no transacting option passed', async function () {
const config = {
maxPeriodic: 5,
error: 'You have exceeded the number of emails you can send within your billing period.',
interval: 'month',
startDate: '2021-01-01T00:00:00Z',
currentCountQuery: sinon.stub()
};
const db = {
knex: 'This is our connection'
};
config.currentCountQuery.resolves(0);
try {
const limit = new MaxPeriodicLimit({name: 'mailguard', config, db, errors});
await limit.errorIfIsOverLimit();
await limit.errorIfWouldGoOverLimit();
} catch (error) {
should.fail('Should have not errored', error);
}
sinon.assert.calledTwice(config.currentCountQuery);
sinon.assert.alwaysCalledWith(config.currentCountQuery, db.knex);
});
it('passes transacting option', async function () {
const config = {
maxPeriodic: 5,
error: 'You have exceeded the number of emails you can send within your billing period.',
interval: 'month',
startDate: '2021-01-01T00:00:00Z',
currentCountQuery: sinon.stub()
};
const db = {
knex: 'This is our connection'
};
const transaction = 'Our transaction';
config.currentCountQuery.resolves(0);
try {
const limit = new MaxPeriodicLimit({name: 'mailguard', config, db, errors});
await limit.errorIfIsOverLimit({transacting: transaction});
await limit.errorIfWouldGoOverLimit({transacting: transaction});
} catch (error) {
should.fail('Should have not errored', error);
}
sinon.assert.calledTwice(config.currentCountQuery);
sinon.assert.alwaysCalledWith(config.currentCountQuery, transaction);
});
});
});
describe('Allowlist limit', function () {