0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-02-03 23:00:14 -05:00

Added config for caching the member lookup for link clicks (#21855)

https://linear.app/ghost/issue/ENG-1850/
- added cache/memoized the member uuid lookup within the
LinkClickRepository (used by the LinkClickTrackingService)
- added repository tests for the save method which were absent

This one one of a series of options we're testing out in order to smooth
out the surge in requests following a newsletter send. Most of this
activity is due to link checkers, but Ghost still needs to spend time
processing the member lookup to know whether or not it is a valid id,
and memoizing this lookup could significantly improve throughput by
reducing DB contention.
This commit is contained in:
Steve Larson 2024-12-10 11:37:47 -06:00 committed by GitHub
parent 1fb417b6a3
commit 78c1d5bcf0
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 118 additions and 2 deletions

View file

@ -2,6 +2,7 @@ const {LinkClick} = require('@tryghost/link-tracking');
const ObjectID = require('bson-objectid').default;
const sentry = require('../../../shared/sentry');
const config = require('../../../shared/config');
const _ = require('lodash');
module.exports = class LinkClickRepository {
/** @type {Object} */
@ -28,6 +29,11 @@ module.exports = class LinkClickRepository {
this.#Member = deps.Member;
this.#MemberLinkClickEvent = deps.MemberLinkClickEvent;
this.#DomainEvents = deps.DomainEvents;
// Memoize the findOne function
this.memoizedFindOne = _.memoize(async (uuid) => {
return await this.#Member.findOne({uuid});
});
}
async getAll(options) {
@ -51,8 +57,14 @@ module.exports = class LinkClickRepository {
* @returns {Promise<void>}
*/
async save(linkClick) {
// Convert uuid to id
const member = await this.#Member.findOne({uuid: linkClick.member_uuid});
let member;
if (config && config.get('linkClickTrackingCacheMemberUuid')) {
member = await this.memoizedFindOne(linkClick.member_uuid);
} else {
member = await this.#Member.findOne({uuid: linkClick.member_uuid});
}
if (!member) {
if (config.get('bulkEmail:captureLinkClickBadMemberUuid')) {
sentry.captureMessage('LinkClickTrackingService > Member not found', {extra: {member_uuid: linkClick.member_uuid}});

View file

@ -193,6 +193,7 @@
"enableStripePromoCodes": false,
"emailAnalytics": true,
"linkClickTracking": true,
"linkClickTrackingCacheMemberUuid": false,
"backgroundJobs": {
"emailAnalytics": true,
"clickTrackingLastSeenAtUpdater": true

View file

@ -0,0 +1,103 @@
const should = require('should');
const sinon = require('sinon');
const ObjectID = require('bson-objectid').default;
const configUtils = require('../../../../utils/configUtils');
const LinkClickRepository = require('../../../../../core/server/services/link-tracking/LinkClickRepository');
const {LinkClick} = require('@tryghost/link-tracking');
const linkClicks = [
new LinkClick({
link_id: ObjectID(),
member_uuid: 'test-uuid'
}),
new LinkClick({
link_id: ObjectID(),
member_uuid: 'test-uuid'
})
];
describe('UNIT: LinkClickRepository class', function () {
let linkClickRepository;
let memberStub;
let memberLinkClickEventModelStub;
let memberLinkClickEventStub;
let domainEventsStub;
beforeEach(function () {
memberStub = {
findOne: sinon.stub()
};
memberLinkClickEventModelStub = {
add: sinon.stub()
};
memberLinkClickEventStub = {
create: sinon.stub()
};
domainEventsStub = {
dispatch: sinon.stub()
};
linkClickRepository = new LinkClickRepository({
MemberLinkClickEventModel: memberLinkClickEventModelStub,
Member: memberStub,
MemberLinkClickEvent: memberLinkClickEventStub,
DomainEvents: domainEventsStub
});
});
afterEach(function () {
sinon.restore();
});
describe('save', function () {
afterEach(function () {
configUtils.restore();
});
it('should save a link click event when member is found', async function () {
const member = {
id: 'member-id',
get: sinon.stub().returns('last-seen-at')
};
memberStub.findOne.resolves(member);
memberLinkClickEventModelStub.add.resolves({id: ObjectID().toHexString()});
// configStub.get.withArgs('linkClickTrackingCacheMemberUuid').returns(true);
await linkClickRepository.save(linkClicks[0]);
sinon.assert.calledOnce(memberStub.findOne);
sinon.assert.calledOnce(memberLinkClickEventModelStub.add);
sinon.assert.calledOnce(memberLinkClickEventStub.create);
sinon.assert.calledOnce(domainEventsStub.dispatch);
});
it('should not save a link click event when member is not found', async function () {
memberStub.findOne.resolves(null);
await linkClickRepository.save(linkClicks[0]);
sinon.assert.notCalled(memberLinkClickEventModelStub.add);
sinon.assert.notCalled(memberLinkClickEventStub.create);
sinon.assert.notCalled(domainEventsStub.dispatch);
});
it('should always call findOne when cacheMemberUuidLinkClick is false', async function () {
configUtils.set('linkClickTrackingCacheMemberUuid', false);
await linkClickRepository.save(linkClicks[0]);
sinon.assert.calledOnce(memberStub.findOne);
await linkClickRepository.save(linkClicks[1]);
sinon.assert.calledTwice(memberStub.findOne);
});
it('should use memoized findOne when cacheMemberUuidLinkClick is true', async function () {
configUtils.set('linkClickTrackingCacheMemberUuid', true);
await linkClickRepository.save(linkClicks[0]);
sinon.assert.calledOnce(memberStub.findOne);
await linkClickRepository.save(linkClicks[1]);
sinon.assert.calledOnce(memberStub.findOne);
});
});
});