OpenSourceArk/Ghost
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Moved attribution event handler to events service (#15379)

Browse Source 
fixes https://github.com/TryGhost/Team/issues/1821

This change moves all the event storage logic to one new place: the event storage class in the MembersEventsService, which is initialised in a new members events service wrapper.

Apart from this, this includes some improvements:
- Removed DomainEvents from the constructor arguments to the subscribe method (to make it more clear where to subscribe to and decrease dependencies)
- LastSeenAtUpdater doesn't subscribe in the constructor any longer (removes unclear side effect)
- Moved LastSeenAtUpdater initialisation to new members events service wrapper
- Added missing tests to LastSeenAtUpdater to assure that the MembersEventsService package has 100% coverage.
This commit is contained in:
Simon Backx 2022-09-07 16:41:59 +02:00 committed by GitHub
parent 4438a72095
commit 74ecde73db
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 204 additions and 87 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
ghost/core/core/boot.js
View File

@ -281,6 +281,7 @@ async function initServices({config}) {
const comments = require('./server/services/comments');
const staffService = require('./server/services/staff');
const memberAttribution = require('./server/services/member-attribution');
const membersEvents = require('./server/services/members-events');
const urlUtils = require('./shared/url-utils');
@ -296,6 +297,7 @@ async function initServices({config}) {
memberAttribution.init(),
staffService.init(),
members.init(),
membersEvents.init(),
permissions.init(),
xmlrpc.listen(),
slack.listen(),

22
ghost/core/core/server/services/member-attribution/index.js
View File

@ -1,17 +1,15 @@
const urlService = require('../url');
const labsService = require('../../../shared/labs');
const DomainEvents = require('@tryghost/domain-events');
const urlUtils = require('../../../shared/url-utils');
class MemberAttributionServiceWrapper {
init() {
if (this.eventHandler) {
// Prevent creating duplicate DomainEvents subscribers
if (this.service) {
// Already done
return;
}
// Wire up all the dependencies
const {MemberAttributionService, UrlTranslator, AttributionBuilder, EventHandler} = require('@tryghost/member-attribution');
const {MemberAttributionService, UrlTranslator, AttributionBuilder} = require('@tryghost/member-attribution');
const models = require('../../models');
const urlTranslator = new UrlTranslator({
@ -32,20 +30,8 @@ class MemberAttributionServiceWrapper {
MemberCreatedEvent: models.MemberCreatedEvent,
SubscriptionCreatedEvent: models.SubscriptionCreatedEvent
},
attributionBuilder: this.attributionBuilder,
labsService
attributionBuilder: this.attributionBuilder
});
// Listen for events and store them in the database
this.eventHandler = new EventHandler({
models: {
MemberCreatedEvent: models.MemberCreatedEvent,
SubscriptionCreatedEvent: models.SubscriptionCreatedEvent
},
DomainEvents,
labsService
});
this.eventHandler.subscribe();
}
}

40
ghost/core/core/server/services/members-events/index.js Normal file
View File

@ -0,0 +1,40 @@
const labsService = require('../../../shared/labs');
const DomainEvents = require('@tryghost/domain-events');
const settingsCache = require('../../../shared/settings-cache');
const members = require('../members');
class MembersEventsServiceWrapper {
init() {
if (this.eventStorage) {
// Prevent creating duplicate DomainEvents subscribers
return;
}
// Wire up all the dependencies
const {EventStorage, LastSeenAtUpdater} = require('@tryghost/members-events-service');
const models = require('../../models');
// Listen for events and store them in the database
this.eventStorage = new EventStorage({
models: {
MemberCreatedEvent: models.MemberCreatedEvent,
SubscriptionCreatedEvent: models.SubscriptionCreatedEvent
},
labsService
});
this.lastSeenAtUpdater = new LastSeenAtUpdater({
services: {
settingsCache
},
async getMembersApi() {
return members.api;
}
});
this.eventStorage.subscribe(DomainEvents);
this.lastSeenAtUpdater.subscribe(DomainEvents);
}
}
module.exports = new MembersEventsServiceWrapper();

12
ghost/core/core/server/services/members/service.js
View File

@ -17,8 +17,6 @@ const models = require('../../models');
const {GhostMailer} = require('../mail');
const jobsService = require('../jobs');
const VerificationTrigger = require('@tryghost/verification-trigger');
const DomainEvents = require('@tryghost/domain-events');
const {LastSeenAtUpdater} = require('@tryghost/members-events-service');
const DatabaseInfo = require('@tryghost/database-info');
const settingsHelpers = require('../settings-helpers');
@ -133,16 +131,6 @@ module.exports = {
eventRepository: membersApi.events
});
new LastSeenAtUpdater({
services: {
domainEvents: DomainEvents,
settingsCache
},
async getMembersApi() {
return membersApi;
}
});
(async () => {
try {
const collection = await models.SingleUseToken.fetchAll();

1
ghost/member-attribution/index.js
View File

@ -1,6 +1,5 @@
module.exports = {
MemberAttributionService: require('./lib/service'),
EventHandler: require('./lib/event-handler'),
AttributionBuilder: require('./lib/attribution'),
UrlTranslator: require('./lib/url-translator')
};

21
ghost/member-attribution/lib/event-handler.js → ghost/members-events-service/lib/event-storage.js
View File

@ -1,23 +1,28 @@
const {MemberCreatedEvent, SubscriptionCreatedEvent} = require('@tryghost/member-events');
class MemberAttributionEventHandler {
/**
* Store events in the database
*/
class EventStorage {
/**
*
* @param {Object} deps
* @param {Object} deps.DomainEvents
* @param {Object} deps.labsService
* @param {Object} deps.models
* @param {Object} deps.models.MemberCreatedEvent
* @param {Object} deps.models.SubscriptionCreatedEvent
*/
constructor({DomainEvents, labsService, models}) {
constructor({labsService, models}) {
this.models = models;
this.DomainEvents = DomainEvents;
this.labsService = labsService;
}
subscribe() {
this.DomainEvents.subscribe(MemberCreatedEvent, async (event) => {
/**
* Subscribe to events of this domainEvents service
* @param {Object} domainEvents The DomainEvents service
*/
subscribe(domainEvents) {
domainEvents.subscribe(MemberCreatedEvent, async (event) => {
let attribution = event.data.attribution;
if (!this.labsService.isSet('memberAttribution')){
@ -36,7 +41,7 @@ class MemberAttributionEventHandler {
});
});
this.DomainEvents.subscribe(SubscriptionCreatedEvent, async (event) => {
domainEvents.subscribe(SubscriptionCreatedEvent, async (event) => {
let attribution = event.data.attribution;
if (!this.labsService.isSet('memberAttribution')){
@ -57,4 +62,4 @@ class MemberAttributionEventHandler {
}
}
module.exports = MemberAttributionEventHandler;
module.exports = EventStorage;

3
ghost/members-events-service/lib/index.js
View File

@ -1,3 +1,4 @@
module.exports = {
LastSeenAtUpdater: require('./last-seen-at-updater')
LastSeenAtUpdater: require('./last-seen-at-updater'),
EventStorage: require('./event-storage')
};

14
ghost/members-events-service/lib/last-seen-at-updater.js
View File

@ -10,13 +10,11 @@ class LastSeenAtUpdater {
* Initializes the event subscriber
* @param {Object} deps dependencies
* @param {Object} deps.services The list of service dependencies
* @param {any} deps.services.domainEvents The DomainEvents service
* @param {any} deps.services.settingsCache The settings service
* @param {() => object} deps.getMembersApi - A function which returns an instance of members-api
*/
constructor({
services: {
domainEvents,
settingsCache
},
getMembersApi
@ -26,14 +24,18 @@ class LastSeenAtUpdater {
}
this._getMembersApi = getMembersApi;
this._domainEventsService = domainEvents;
this._settingsCacheService = settingsCache;
this._domainEventsService.subscribe(MemberPageViewEvent, async (event) => {
}
/**
* Subscribe to events of this domainEvents service
* @param {any} domainEvents The DomainEvents service
*/
subscribe(domainEvents) {
domainEvents.subscribe(MemberPageViewEvent, async (event) => {
await this.updateLastSeenAt(event.data.memberId, event.data.memberLastSeenAt, event.timestamp);
});
this._domainEventsService.subscribe(MemberCommentEvent, async (event) => {
domainEvents.subscribe(MemberCommentEvent, async (event) => {
await this.updateLastCommentedAt(event.data.memberId, event.timestamp);
});
}

36
ghost/member-attribution/test/event-handler.test.js → ghost/members-events-service/test/event-storage.test.js
View File

@ -2,12 +2,12 @@
// const testUtils = require('./utils');
require('./utils');
const {MemberCreatedEvent, SubscriptionCreatedEvent} = require('@tryghost/member-events');
const MemberAttributionEventHandler = require('../lib/event-handler');
const EventStorage = require('../lib/event-storage');
describe('MemberAttributionEventHandler', function () {
describe('EventStorage', function () {
describe('Constructor', function () {
it('doesn\'t throw', function () {
new MemberAttributionEventHandler({});
new EventStorage({});
});
});
@ -27,12 +27,11 @@ describe('MemberAttributionEventHandler', function () {
const MemberCreatedEventModel = {add: sinon.stub()};
const labsService = {isSet: sinon.stub().returns(true)};
const subscribeSpy = sinon.spy(DomainEvents, 'subscribe');
const eventHandler = new MemberAttributionEventHandler({
DomainEvents,
const eventHandler = new EventStorage({
models: {MemberCreatedEvent: MemberCreatedEventModel},
labsService
});
eventHandler.subscribe();
eventHandler.subscribe(DomainEvents);
sinon.assert.calledOnceWithMatch(MemberCreatedEventModel.add, {
member_id: '123',
created_at: new Date(0),
@ -61,12 +60,11 @@ describe('MemberAttributionEventHandler', function () {
const MemberCreatedEventModel = {add: sinon.stub()};
const labsService = {isSet: sinon.stub().returns(true)};
const subscribeSpy = sinon.spy(DomainEvents, 'subscribe');
const eventHandler = new MemberAttributionEventHandler({
DomainEvents,
const eventHandler = new EventStorage({
models: {MemberCreatedEvent: MemberCreatedEventModel},
labsService
});
eventHandler.subscribe();
eventHandler.subscribe(DomainEvents);
sinon.assert.calledOnceWithMatch(MemberCreatedEventModel.add, {
member_id: '123',
created_at: new Date(0),
@ -98,12 +96,11 @@ describe('MemberAttributionEventHandler', function () {
const MemberCreatedEventModel = {add: sinon.stub()};
const labsService = {isSet: sinon.stub().returns(false)};
const subscribeSpy = sinon.spy(DomainEvents, 'subscribe');
const eventHandler = new MemberAttributionEventHandler({
DomainEvents,
const eventHandler = new EventStorage({
models: {MemberCreatedEvent: MemberCreatedEventModel},
labsService
});
eventHandler.subscribe();
eventHandler.subscribe(DomainEvents);
sinon.assert.calledOnceWithMatch(MemberCreatedEventModel.add, {
member_id: '123',
created_at: new Date(0),
@ -132,12 +129,11 @@ describe('MemberAttributionEventHandler', function () {
const SubscriptionCreatedEventModel = {add: sinon.stub()};
const labsService = {isSet: sinon.stub().returns(true)};
const subscribeSpy = sinon.spy(DomainEvents, 'subscribe');
const eventHandler = new MemberAttributionEventHandler({
DomainEvents,
const eventHandler = new EventStorage({
models: {SubscriptionCreatedEvent: SubscriptionCreatedEventModel},
labsService
});
eventHandler.subscribe();
eventHandler.subscribe(DomainEvents);
sinon.assert.calledOnceWithMatch(SubscriptionCreatedEventModel.add, {
member_id: '123',
subscription_id: '456',
@ -166,12 +162,11 @@ describe('MemberAttributionEventHandler', function () {
const SubscriptionCreatedEventModel = {add: sinon.stub()};
const labsService = {isSet: sinon.stub().returns(true)};
const subscribeSpy = sinon.spy(DomainEvents, 'subscribe');
const eventHandler = new MemberAttributionEventHandler({
DomainEvents,
const eventHandler = new EventStorage({
models: {SubscriptionCreatedEvent: SubscriptionCreatedEventModel},
labsService
});
eventHandler.subscribe();
eventHandler.subscribe(DomainEvents);
sinon.assert.calledOnceWithMatch(SubscriptionCreatedEventModel.add, {
member_id: '123',
subscription_id: '456',
@ -203,12 +198,11 @@ describe('MemberAttributionEventHandler', function () {
const SubscriptionCreatedEventModel = {add: sinon.stub()};
const labsService = {isSet: sinon.stub().returns(false)};
const subscribeSpy = sinon.spy(DomainEvents, 'subscribe');
const eventHandler = new MemberAttributionEventHandler({
DomainEvents,
const eventHandler = new EventStorage({
models: {SubscriptionCreatedEvent: SubscriptionCreatedEventModel},
labsService
});
eventHandler.subscribe();
eventHandler.subscribe(DomainEvents);
sinon.assert.calledOnceWithMatch(SubscriptionCreatedEventModel.add, {
member_id: '123',
subscription_id: '456',

140
ghost/members-events-service/test/last-seen-at-updater.test.js
View File

@ -19,8 +19,7 @@ describe('LastSeenAtUpdater', function () {
services: {
settingsCache: {
get: settingsCache
},
domainEvents: DomainEvents
}
},
async getMembersApi() {
return {
@ -30,6 +29,7 @@ describe('LastSeenAtUpdater', function () {
};
}
});
updater.subscribe(DomainEvents);
sinon.stub(updater, 'updateLastSeenAt');
DomainEvents.dispatch(MemberPageViewEvent.create({memberId: '1', memberLastSeenAt: previousLastSeen, url: '/'}, now.toDate()));
assert(updater.updateLastSeenAt.calledOnceWithExactly('1', previousLastSeen, now.toDate()));
@ -43,8 +43,7 @@ describe('LastSeenAtUpdater', function () {
services: {
settingsCache: {
get: settingsCache
},
domainEvents: DomainEvents
}
},
async getMembersApi() {
return {
@ -54,6 +53,7 @@ describe('LastSeenAtUpdater', function () {
};
}
});
updater.subscribe(DomainEvents);
sinon.stub(updater, 'updateLastCommentedAt');
DomainEvents.dispatch(MemberCommentEvent.create({memberId: '1'}, now.toDate()));
assert(updater.updateLastCommentedAt.calledOnceWithExactly('1', now.toDate()));
@ -68,8 +68,7 @@ describe('LastSeenAtUpdater', function () {
services: {
settingsCache: {
get: settingsCache
},
domainEvents: DomainEvents
}
},
async getMembersApi() {
return {
@ -92,8 +91,7 @@ describe('LastSeenAtUpdater', function () {
services: {
settingsCache: {
get: settingsCache
},
domainEvents: DomainEvents
}
},
async getMembersApi() {
return {
@ -124,8 +122,7 @@ describe('LastSeenAtUpdater', function () {
services: {
settingsCache: {
get: settingsCache
},
domainEvents: DomainEvents
}
},
async getMembersApi() {
return {
@ -152,8 +149,7 @@ describe('LastSeenAtUpdater', function () {
services: {
settingsCache: {
get: settingsCache
},
domainEvents: DomainEvents
}
},
async getMembersApi() {
return {
@ -169,15 +165,14 @@ describe('LastSeenAtUpdater', function () {
it('Doesn\'t update when last_commented_at is too recent', async function () {
const now = moment('2022-02-28T18:00:00Z');
const previousLastSeen = moment('2022-02-28T00:00:00Z').toISOString();
const previousLastSeen = moment('2022-02-28T00:00:00Z').toDate();
const stub = sinon.stub().resolves();
const settingsCache = sinon.stub().returns('Etc/UTC');
const updater = new LastSeenAtUpdater({
services: {
settingsCache: {
get: settingsCache
},
domainEvents: DomainEvents
}
},
async getMembersApi() {
return {
@ -196,7 +191,114 @@ describe('LastSeenAtUpdater', function () {
}
});
await updater.updateLastCommentedAt('1', now.toDate());
assert(stub.notCalled, 'The LastSeenAtUpdater should\'t update a member when the previous last_seen_at is close to the event timestamp.');
assert(stub.notCalled, 'The LastSeenAtUpdater should\'t update a member');
});
it('Does not update when last_commented_at is same date in timezone', async function () {
const now = moment.utc('2022-02-28T18:00:00Z');
const previousLastSeen = moment.utc('2022-02-27T23:59:00Z').toDate();
const stub = sinon.stub().resolves();
const settingsCache = sinon.stub().returns('Europe/Brussels');
const updater = new LastSeenAtUpdater({
services: {
settingsCache: {
get: settingsCache
}
},
async getMembersApi() {
return {
members: {
update: stub,
get: () => {
return {
id: '1',
get: () => {
return previousLastSeen;
}
};
}
}
};
}
});
await updater.updateLastCommentedAt('1', now.toDate());
assert(stub.notCalled, 'The LastSeenAtUpdater should\'t update a member.');
});
it('Does update when last_commented_at is different date', async function () {
const now = moment.utc('2022-02-28T18:00:00Z');
const previousLastSeen = moment.utc('2022-02-27T22:59:00Z').toDate();
const stub = sinon.stub().resolves();
const settingsCache = sinon.stub().returns('Europe/Brussels');
const updater = new LastSeenAtUpdater({
services: {
settingsCache: {
get: settingsCache
}
},
async getMembersApi() {
return {
members: {
update: stub,
get: () => {
return {
id: '1',
get: () => {
return previousLastSeen;
}
};
}
}
};
}
});
await updater.updateLastCommentedAt('1', now.toDate());
assert(stub.calledOnce, 'The LastSeenAtUpdater should attempt a member update');
assert(stub.calledOnceWithExactly({
last_seen_at: now.tz('utc').format('YYYY-MM-DD HH:mm:ss'),
last_commented_at: now.tz('utc').format('YYYY-MM-DD HH:mm:ss')
}, {
id: '1'
}), 'The LastSeenAtUpdater should attempt a member update with the current date.');
});
it('Does update when last_commented_at is null', async function () {
const now = moment.utc('2022-02-28T18:00:00Z');
const previousLastSeen = null;
const stub = sinon.stub().resolves();
const settingsCache = sinon.stub().returns('Etc/UTC');
const updater = new LastSeenAtUpdater({
services: {
settingsCache: {
get: settingsCache
}
},
async getMembersApi() {
return {
members: {
update: stub,
get: () => {
return {
id: '1',
get: () => {
return previousLastSeen;
}
};
}
}
};
}
});
await updater.updateLastCommentedAt('1', now.toDate());
assert(stub.calledOnce, 'The LastSeenAtUpdater should attempt a member update');
assert(stub.calledOnceWithExactly({
last_seen_at: now.tz('utc').format('YYYY-MM-DD HH:mm:ss'),
last_commented_at: now.tz('utc').format('YYYY-MM-DD HH:mm:ss')
}, {
id: '1'
}), 'The LastSeenAtUpdater should attempt a member update with the current date.');
});
it('Doesn\'t fire on other events', async function () {
@ -207,8 +309,7 @@ describe('LastSeenAtUpdater', function () {
services: {
settingsCache: {
get: settingsCache
},
domainEvents: DomainEvents
}
},
async getMembersApi() {
return {
@ -230,8 +331,7 @@ describe('LastSeenAtUpdater', function () {
services: {
settingsCache: {
get: settingsCache
},
domainEvents: DomainEvents
}
}
});
}, 'Missing option getMembersApi');