From 02168b41ceb5b75005584b791b80d5e6beaff11a Mon Sep 17 00:00:00 2001 From: Simon Backx Date: Mon, 22 Aug 2022 11:36:24 +0200 Subject: [PATCH] Improved dependency structure of member-attribution package refs https://github.com/TryGhost/Ghost/pull/15266#discussion_r950337271 - Moved dependency building to the the service wrapper - Don't listen for events inside the constructor - Used a models option to pass around models to make constructors more readable --- .../services/member-attribution/index.js | 41 +++++++++++++++---- ghost/member-attribution/index.js | 7 +++- ghost/member-attribution/lib/attribution.js | 5 --- ghost/member-attribution/lib/event-handler.js | 18 +++++--- ghost/member-attribution/lib/service.js | 30 ++++++-------- .../member-attribution/lib/url-translator.js | 2 +- .../test/event-handler.test.js | 12 +++--- 7 files changed, 70 insertions(+), 45 deletions(-) diff --git a/ghost/core/core/server/services/member-attribution/index.js b/ghost/core/core/server/services/member-attribution/index.js index 14159af0c0..85390a26fa 100644 --- a/ghost/core/core/server/services/member-attribution/index.js +++ b/ghost/core/core/server/services/member-attribution/index.js @@ -1,26 +1,49 @@ const urlService = require('../url'); const labsService = require('../../../shared/labs'); +const DomainEvents = require('@tryghost/domain-events'); class MemberAttributionServiceWrapper { init() { - if (this.service) { + if (this.eventHandler) { // Prevent creating duplicate DomainEvents subscribers return; } - const MemberAttributionService = require('@tryghost/member-attribution'); + // Wire up all the dependencies + const {MemberAttributionService, UrlTranslator, AttributionBuilder, EventHandler} = require('@tryghost/member-attribution'); const models = require('../../models'); - // For now we don't need to expose anything (yet) + const urlTranslator = new UrlTranslator({ + urlService, + models: { + Post: models.Post, + User: models.User, + Tag: models.Tag + } + }); + + const attributionBuilder = new AttributionBuilder({urlTranslator}); + + // Expose the service this.service = new MemberAttributionService({ - Post: models.Post, - User: models.User, - Tag: models.Tag, - MemberCreatedEvent: models.MemberCreatedEvent, - SubscriptionCreatedEvent: models.SubscriptionCreatedEvent, - urlService, + models: { + MemberCreatedEvent: models.MemberCreatedEvent, + SubscriptionCreatedEvent: models.SubscriptionCreatedEvent + }, + attributionBuilder, labsService }); + + // 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(); } } diff --git a/ghost/member-attribution/index.js b/ghost/member-attribution/index.js index ff50ccb46e..f49f7d515f 100644 --- a/ghost/member-attribution/index.js +++ b/ghost/member-attribution/index.js @@ -1 +1,6 @@ -module.exports = require('./lib/service'); +module.exports = { + MemberAttributionService: require('./lib/service'), + EventHandler: require('./lib/event-handler'), + AttributionBuilder: require('./lib/attribution'), + UrlTranslator: require('./lib/url-translator') +}; diff --git a/ghost/member-attribution/lib/attribution.js b/ghost/member-attribution/lib/attribution.js index 10958a6d48..a8f4865abb 100644 --- a/ghost/member-attribution/lib/attribution.js +++ b/ghost/member-attribution/lib/attribution.js @@ -16,13 +16,8 @@ class Attribution { * @param {'page'|'post'|'author'|'tag'|'url'} [data.type] */ constructor({id, url, type}, {urlTranslator}) { - /** @type {string|null} */ this.id = id; - - /** @type {string|null} */ this.url = url; - - /** @type {'page'|'post'|'author'|'tag'|'url'} */ this.type = type; /** diff --git a/ghost/member-attribution/lib/event-handler.js b/ghost/member-attribution/lib/event-handler.js index e34d6853d3..123c679581 100644 --- a/ghost/member-attribution/lib/event-handler.js +++ b/ghost/member-attribution/lib/event-handler.js @@ -1,9 +1,17 @@ const {MemberCreatedEvent, SubscriptionCreatedEvent} = require('@tryghost/member-events'); class MemberAttributionEventHandler { - constructor({MemberCreatedEvent: MemberCreatedEventModel, SubscriptionCreatedEvent: SubscriptionCreatedEventModel, DomainEvents, labsService}) { - this._MemberCreatedEventModel = MemberCreatedEventModel; - this._SubscriptionCreatedEvent = SubscriptionCreatedEventModel; + /** + * + * @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}) { + this.models = models; this.DomainEvents = DomainEvents; this.labsService = labsService; } @@ -18,7 +26,7 @@ class MemberAttributionEventHandler { attribution = {}; } - await this._MemberCreatedEventModel.add({ + await this.models.MemberCreatedEvent.add({ member_id: event.data.memberId, created_at: event.timestamp, attribution_id: attribution?.id ?? null, @@ -37,7 +45,7 @@ class MemberAttributionEventHandler { attribution = {}; } - await this._SubscriptionCreatedEvent.add({ + await this.models.SubscriptionCreatedEvent.add({ member_id: event.data.memberId, subscription_id: event.data.subscriptionId, created_at: event.timestamp, diff --git a/ghost/member-attribution/lib/service.js b/ghost/member-attribution/lib/service.js index 9be2496dc4..7c9d20f7b1 100644 --- a/ghost/member-attribution/lib/service.js +++ b/ghost/member-attribution/lib/service.js @@ -1,24 +1,18 @@ -const MemberAttributionEventHandler = require('./event-handler'); -const DomainEvents = require('@tryghost/domain-events'); -const UrlTranslator = require('./url-translator'); -const AttributionBuilder = require('./attribution'); const UrlHistory = require('./history'); class MemberAttributionService { - constructor({Post, User, Tag, MemberCreatedEvent, SubscriptionCreatedEvent, urlService, labsService}) { - const eventHandler = new MemberAttributionEventHandler({MemberCreatedEvent, SubscriptionCreatedEvent, DomainEvents, labsService}); - eventHandler.subscribe(); - - this.urlService = urlService; - this.models = {MemberCreatedEvent, SubscriptionCreatedEvent}; - - const urlTranslator = new UrlTranslator({ - urlService, - models: { - Post, User, Tag - } - }); - this.attributionBuilder = new AttributionBuilder({urlTranslator}); + /** + * + * @param {Object} deps + * @param {Object} deps.attributionBuilder + * @param {Object} deps.labsService + * @param {Object} deps.models + * @param {Object} deps.models.MemberCreatedEvent + * @param {Object} deps.models.SubscriptionCreatedEvent + */ + constructor({attributionBuilder, models}) { + this.models = models; + this.attributionBuilder = attributionBuilder; } /** diff --git a/ghost/member-attribution/lib/url-translator.js b/ghost/member-attribution/lib/url-translator.js index ba6bcfcda5..a3906a9116 100644 --- a/ghost/member-attribution/lib/url-translator.js +++ b/ghost/member-attribution/lib/url-translator.js @@ -1,6 +1,6 @@ /** * @typedef {Object} UrlService - * @prop {(resourceId: string) => Object} getResource + * @prop {(resourceId: string, options) => Object} getResource * @prop {(resourceId: string, options) => string} getUrlByResourceId * */ diff --git a/ghost/member-attribution/test/event-handler.test.js b/ghost/member-attribution/test/event-handler.test.js index 81e0ca2cdc..d26dd44b2c 100644 --- a/ghost/member-attribution/test/event-handler.test.js +++ b/ghost/member-attribution/test/event-handler.test.js @@ -29,7 +29,7 @@ describe('MemberAttributionEventHandler', function () { const subscribeSpy = sinon.spy(DomainEvents, 'subscribe'); const eventHandler = new MemberAttributionEventHandler({ DomainEvents, - MemberCreatedEvent: MemberCreatedEventModel, + models: {MemberCreatedEvent: MemberCreatedEventModel}, labsService }); eventHandler.subscribe(); @@ -63,7 +63,7 @@ describe('MemberAttributionEventHandler', function () { const subscribeSpy = sinon.spy(DomainEvents, 'subscribe'); const eventHandler = new MemberAttributionEventHandler({ DomainEvents, - MemberCreatedEvent: MemberCreatedEventModel, + models: {MemberCreatedEvent: MemberCreatedEventModel}, labsService }); eventHandler.subscribe(); @@ -100,7 +100,7 @@ describe('MemberAttributionEventHandler', function () { const subscribeSpy = sinon.spy(DomainEvents, 'subscribe'); const eventHandler = new MemberAttributionEventHandler({ DomainEvents, - MemberCreatedEvent: MemberCreatedEventModel, + models: {MemberCreatedEvent: MemberCreatedEventModel}, labsService }); eventHandler.subscribe(); @@ -134,7 +134,7 @@ describe('MemberAttributionEventHandler', function () { const subscribeSpy = sinon.spy(DomainEvents, 'subscribe'); const eventHandler = new MemberAttributionEventHandler({ DomainEvents, - SubscriptionCreatedEvent: SubscriptionCreatedEventModel, + models: {SubscriptionCreatedEvent: SubscriptionCreatedEventModel}, labsService }); eventHandler.subscribe(); @@ -168,7 +168,7 @@ describe('MemberAttributionEventHandler', function () { const subscribeSpy = sinon.spy(DomainEvents, 'subscribe'); const eventHandler = new MemberAttributionEventHandler({ DomainEvents, - SubscriptionCreatedEvent: SubscriptionCreatedEventModel, + models: {SubscriptionCreatedEvent: SubscriptionCreatedEventModel}, labsService }); eventHandler.subscribe(); @@ -205,7 +205,7 @@ describe('MemberAttributionEventHandler', function () { const subscribeSpy = sinon.spy(DomainEvents, 'subscribe'); const eventHandler = new MemberAttributionEventHandler({ DomainEvents, - SubscriptionCreatedEvent: SubscriptionCreatedEventModel, + models: {SubscriptionCreatedEvent: SubscriptionCreatedEventModel}, labsService }); eventHandler.subscribe();