Skip to content

Commit ceb1995

Browse files
authored
Added .isDisabled() method to the Limit Service (#550)
ref https://linear.app/ghost/issue/PROD-2172 - Limits such as `FlagLimit` are calculated solely based on their `disabled` boolean field - The Limit Service only exposes async methods such as `checkWouldGoOverLimit`, which forces downstream methods in e.g. Ghost to be async too - The new `.isDisabled()` sync method returns true/false based on the value of the `disabled` field, or throws an error if the limit does not support a `disabled` field
1 parent d0f1402 commit ceb1995

File tree

5 files changed

+131
-3
lines changed

5 files changed

+131
-3
lines changed

packages/limit-service/README.md

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,12 @@ const limits = {
6868
error: 'Your plan supports uploads of max size up to {{max}}. Please upgrade to reenable uploading.'
6969
},
7070
limitStripeConnect: {},
71-
limitAnalytics: {},
72-
limitSocialWeb: {}
71+
limitAnalytics: {
72+
disabled: false
73+
},
74+
limitSocialWeb: {
75+
disabled: true
76+
}
7377
};
7478

7579
// This information is needed for the limit service to work with "max periodic" limits
@@ -134,6 +138,16 @@ if (limitService.isLimited('uploads')) {
134138
await limitService.errorIfIsOverLimit('uploads', {currentCount: frame.file.size});
135139
}
136140

141+
// Limits expose an async `checkWouldGoOverLimit` method, which can be used to check whether a limit has been reached, but not throw an error:
142+
if (await limitService.checkWouldGoOverLimit('members')) {
143+
console.log('Members limit has been reached!');
144+
}
145+
146+
// Flag limits additionally expose a `isDisabled` sync check, which can be used instead of the async `checkWouldGoOverLimit`:
147+
if (limitService.isDisabled('limitSocialWeb')) {
148+
console.log('Social web is disabled by config!'));
149+
}
150+
137151
// check if any of the limits are acceding
138152
if (limitService.checkIfAnyOverLimit()) {
139153
console.log('One of the limits has acceded!');

packages/limit-service/lib/LimitService.js

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,27 @@ class LimitService {
6969
return !!this.limits[camelCase(limitName)];
7070
}
7171

72+
/**
73+
* Check if a limit is disabled, applicable only to limits that support the disabled flag (e.g. FlagLimit)
74+
* @returns {boolean|undefined} undefined if limit is not configured
75+
* @throws {IncorrectUsageError} if limit does not support disabled flag
76+
*/
77+
isDisabled(limitName) {
78+
if (!this.isLimited(limitName)) {
79+
return;
80+
}
81+
82+
const limit = this.limits[camelCase(limitName)];
83+
84+
if (typeof limit.isDisabled !== 'function') {
85+
throw new IncorrectUsageError({
86+
message: `Limit ${limitName} does not support .isDisabled()`
87+
});
88+
}
89+
90+
return limit.isDisabled();
91+
}
92+
7293
/**
7394
*
7495
* @param {String} limitName - name of the configured limit

packages/limit-service/lib/limit.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,14 @@ class FlagLimit extends Limit {
303303
async errorIfIsOverLimit() {
304304
return;
305305
}
306+
307+
/**
308+
* Checks whether the Flag limit is disabled or not
309+
* @returns boolean
310+
*/
311+
isDisabled() {
312+
return !!this.disabled;
313+
}
306314
}
307315

308316
class AllowlistLimit extends Limit {

packages/limit-service/test/LimitService.test.js

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// const testUtils = require('./utils');
33
require('./utils');
44
const should = require('should');
5+
const assert = require('node:assert').strict;
56
const LimitService = require('../lib/LimitService');
67
const {MaxLimit, MaxPeriodicLimit, FlagLimit} = require('../lib/limit');
78
const sinon = require('sinon');
@@ -544,4 +545,58 @@ describe('Limit Service', function () {
544545
sinon.assert.alwaysCalledWithExactly(maxPeriodSpy, options);
545546
});
546547
});
548+
549+
describe('isDisabled', function () {
550+
it('returns undefined if limit is not configured', function () {
551+
const limitService = new LimitService();
552+
553+
assert.equal(limitService.isDisabled('test'), undefined);
554+
});
555+
556+
it('throws if the limit does not implement .isDisabled()', function () {
557+
const limitService = new LimitService();
558+
559+
let limits = {
560+
staff: {
561+
max: 2,
562+
currentCountQuery: () => 1
563+
}
564+
};
565+
566+
limitService.loadLimits({limits, errors});
567+
568+
try {
569+
limitService.isDisabled('staff');
570+
assert.fail('Should have thrown an error');
571+
} catch (err) {
572+
assert.equal(err.message, `Limit staff does not support .isDisabled()`);
573+
}
574+
});
575+
576+
it('returns true if the limit is disabled', function () {
577+
const limitService = new LimitService();
578+
579+
let limits = {
580+
limitSocialWeb: {
581+
disabled: true
582+
}
583+
};
584+
585+
limitService.loadLimits({limits, errors});
586+
assert.equal(limitService.isDisabled('limitSocialWeb'), true);
587+
});
588+
589+
it('returns false if the limit is not disabled', function () {
590+
const limitService = new LimitService();
591+
592+
let limits = {
593+
limitSocialWeb: {
594+
disabled: false
595+
}
596+
};
597+
598+
limitService.loadLimits({limits, errors});
599+
assert.equal(limitService.isDisabled('limitSocialWeb'), false);
600+
});
601+
});
547602
});

packages/limit-service/test/limit.test.js

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,12 @@
33
require('./utils');
44
const should = require('should');
55
const sinon = require('sinon');
6+
const assert = require('node:assert').strict;
67

78
const errors = require('./fixtures/errors');
89
const {MaxLimit, AllowlistLimit, FlagLimit, MaxPeriodicLimit} = require('../lib/limit');
910

10-
describe('Limit Service', function () {
11+
describe('Limit', function () {
1112
describe('Flag Limit', function () {
1213
it('do nothing if is over limit', async function () {
1314
// NOTE: the behavior of flag limit in "is over limit" use case is flawed and should not be relied on
@@ -43,6 +44,35 @@ describe('Limit Service', function () {
4344
should.equal(err.message, 'Your plan does not support flaggy. Please upgrade to enable flaggy.');
4445
}
4546
});
47+
48+
describe('isDisabled', function () {
49+
it('returns true if limit is disabled', function () {
50+
const config = {
51+
disabled: true
52+
};
53+
const limit = new FlagLimit({name: 'flaggy', config, errors});
54+
55+
assert.equal(limit.isDisabled(), true);
56+
});
57+
58+
it('returns false if limit is disabled', function () {
59+
const config = {
60+
disabled: false
61+
};
62+
const limit = new FlagLimit({name: 'flaggy', config, errors});
63+
64+
assert.equal(limit.isDisabled(), false);
65+
});
66+
67+
it('returns false if limit is not defined', function () {
68+
const config = {
69+
disabled: undefined
70+
};
71+
const limit = new FlagLimit({name: 'flaggy', config, errors});
72+
73+
assert.equal(limit.isDisabled(), false);
74+
});
75+
});
4676
});
4777

4878
describe('Max Limit', function () {

0 commit comments

Comments
 (0)