From 6389385f61f0ed8a2f9fb1f3cf4df8e5b9bc26c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivar=20Conradi=20=C3=98sthus?= Date: Fri, 12 Feb 2021 10:23:43 +0100 Subject: [PATCH] fix: refactor context to use service pattern (#721) --- lib/db/context-field-store.js | 55 ++--------- lib/db/index.js | 1 - lib/routes/admin-api/context.js | 70 ++++---------- lib/routes/admin-api/context.test.js | 8 +- .../admin-api => services}/context-schema.js | 2 +- lib/services/context-service.js | 92 +++++++++++++++++++ lib/services/index.js | 3 + test/e2e/helpers/database-init.js | 2 +- test/fixtures/fake-context-store.js | 10 +- 9 files changed, 138 insertions(+), 105 deletions(-) rename lib/{routes/admin-api => services}/context-schema.js (92%) create mode 100644 lib/services/context-service.js diff --git a/lib/db/context-field-store.js b/lib/db/context-field-store.js index 069cac04da..681662d2e3 100644 --- a/lib/db/context-field-store.js +++ b/lib/db/context-field-store.js @@ -1,11 +1,5 @@ 'use strict'; -const { - CONTEXT_FIELD_CREATED, - CONTEXT_FIELD_UPDATED, - CONTEXT_FIELD_DELETED, -} = require('../event-type'); - const COLUMNS = [ 'name', 'description', @@ -26,20 +20,10 @@ const mapRow = row => ({ }); class ContextFieldStore { - constructor(db, customContextFields, eventStore, getLogger) { + constructor(db, customContextFields, getLogger) { this.db = db; this.logger = getLogger('context-field-store.js'); this._createFromConfig(customContextFields); - - eventStore.on(CONTEXT_FIELD_CREATED, event => - this._createContextField(event.data), - ); - eventStore.on(CONTEXT_FIELD_UPDATED, event => - this._updateContextField(event.data), - ); - eventStore.on(CONTEXT_FIELD_DELETED, event => { - this._deleteContextField(event.data); - }); } async _createFromConfig(customContextFields) { @@ -48,12 +32,12 @@ class ContextFieldStore { 'Create custom context fields', customContextFields, ); - const conextFields = await this.getAll(); + const contextFields = await this.getAll(); customContextFields - .filter(c => !conextFields.some(cf => cf.name === c.name)) + .filter(c => !contextFields.some(cf => cf.name === c.name)) .forEach(async field => { try { - await this._createContextField(field); + await this.create(field); } catch (e) { this.logger.error(e); } @@ -89,39 +73,20 @@ class ContextFieldStore { .then(mapRow); } - async _createContextField(contextField) { - return this.db(TABLE) - .insert(this.fieldToRow(contextField)) - .catch(err => - this.logger.error( - 'Could not insert contextField, error: ', - err, - ), - ); + async create(contextField) { + return this.db(TABLE).insert(this.fieldToRow(contextField)); } - async _updateContextField(data) { + async update(data) { return this.db(TABLE) .where({ name: data.name }) - .update(this.fieldToRow(data)) - .catch(err => - this.logger.error( - 'Could not update context field, error: ', - err, - ), - ); + .update(this.fieldToRow(data)); } - async _deleteContextField({ name }) { + async delete(name) { return this.db(TABLE) .where({ name }) - .del() - .catch(err => { - this.logger.error( - 'Could not delete context field, error: ', - err, - ); - }); + .del(); } } diff --git a/lib/db/index.js b/lib/db/index.js index 1b51017159..caf941e24c 100644 --- a/lib/db/index.js +++ b/lib/db/index.js @@ -43,7 +43,6 @@ module.exports.createStores = (config, eventBus) => { contextFieldStore: new ContextFieldStore( db, config.customContextFields, - eventStore, getLogger, ), settingStore: new SettingStore(db, getLogger), diff --git a/lib/routes/admin-api/context.js b/lib/routes/admin-api/context.js index 4d76236a1e..03414ea948 100644 --- a/lib/routes/admin-api/context.js +++ b/lib/routes/admin-api/context.js @@ -2,17 +2,9 @@ const Controller = require('../controller'); -const { contextSchema, nameSchema } = require('./context-schema'); -const NameExistsError = require('../../error/name-exists-error'); const { handleErrors } = require('./util'); const extractUser = require('../../extract-user'); -const { - CONTEXT_FIELD_CREATED, - CONTEXT_FIELD_UPDATED, - CONTEXT_FIELD_DELETED, -} = require('../../event-type'); - const { CREATE_CONTEXT_FIELD, UPDATE_CONTEXT_FIELD, @@ -20,11 +12,10 @@ const { } = require('../../permissions'); class ContextController extends Controller { - constructor(config) { + constructor(config, { contextService }) { super(config); this.logger = config.getLogger('/admin-api/feature.js'); - this.eventStore = config.stores.eventStore; - this.store = config.stores.contextFieldStore; + this.contextService = contextService; this.get('/', this.getContextFields); this.post('/', this.createContextField, CREATE_CONTEXT_FIELD); @@ -43,7 +34,7 @@ class ContextController extends Controller { } async getContextFields(req, res) { - const fields = await this.store.getAll(); + const fields = await this.contextService.getAll(); res.status(200) .json(fields) .end(); @@ -52,7 +43,9 @@ class ContextController extends Controller { async getContextField(req, res) { try { const name = req.params.contextField; - const contextField = await this.store.get(name); + const contextField = await this.contextService.getContextField( + name, + ); res.json(contextField).end(); } catch (err) { res.status(404).json({ error: 'Could not find context field' }); @@ -60,17 +53,11 @@ class ContextController extends Controller { } async createContextField(req, res) { - const { name } = req.body; + const value = req.body; const userName = extractUser(req); try { - await this.validateUniqueName(name); - const value = await contextSchema.validateAsync(req.body); - await this.eventStore.store({ - type: CONTEXT_FIELD_CREATED, - createdBy: userName, - data: value, - }); + await this.contextService.createContextField(value, userName); res.status(201).end(); } catch (error) { handleErrors(res, this.logger, error); @@ -80,21 +67,15 @@ class ContextController extends Controller { async updateContextField(req, res) { const name = req.params.contextField; const userName = extractUser(req); - const updatedContextField = req.body; + const contextField = req.body; - updatedContextField.name = name; + contextField.name = name; try { - await this.store.get(name); - - const value = await contextSchema.validateAsync( - updatedContextField, + await this.contextService.updateContextField( + contextField, + userName, ); - await this.eventStore.store({ - type: CONTEXT_FIELD_UPDATED, - createdBy: userName, - data: value, - }); res.status(200).end(); } catch (error) { handleErrors(res, this.logger, error); @@ -103,40 +84,21 @@ class ContextController extends Controller { async deleteContextField(req, res) { const name = req.params.contextField; + const userName = extractUser(req); try { - await this.store.get(name); - await this.eventStore.store({ - type: CONTEXT_FIELD_DELETED, - createdBy: extractUser(req), - data: { name }, - }); + await this.contextService.deleteContextField(name, userName); res.status(200).end(); } catch (error) { handleErrors(res, this.logger, error); } } - async validateUniqueName(name) { - let msg; - try { - await this.store.get(name); - msg = 'A context field with that name already exist'; - } catch (error) { - // No conflict, everything ok! - return; - } - - // Interntional throw here! - throw new NameExistsError(msg); - } - async validate(req, res) { const { name } = req.body; try { - await nameSchema.validateAsync({ name }); - await this.validateUniqueName(name); + await this.contextService.validateName(name); res.status(200).end(); } catch (error) { handleErrors(res, this.logger, error); diff --git a/lib/routes/admin-api/context.test.js b/lib/routes/admin-api/context.test.js index e0da304bab..608959e647 100644 --- a/lib/routes/admin-api/context.test.js +++ b/lib/routes/admin-api/context.test.js @@ -4,6 +4,7 @@ const test = require('ava'); const supertest = require('supertest'); const { EventEmitter } = require('events'); const store = require('../../../test/fixtures/store'); +const { createServices } = require('../../services'); const getLogger = require('../../../test/fixtures/no-logger'); const getApp = require('../../app'); @@ -12,14 +13,17 @@ const eventBus = new EventEmitter(); function getSetup() { const base = `/random${Math.round(Math.random() * 1000)}`; const stores = store.createStores(); - const app = getApp({ + const config = { baseUriPath: base, stores, eventBus, extendedPermissions: false, customContextFields: [{ name: 'tenantId' }], getLogger, - }); + }; + + const services = createServices(stores, config); + const app = getApp(config, services); return { base, diff --git a/lib/routes/admin-api/context-schema.js b/lib/services/context-schema.js similarity index 92% rename from lib/routes/admin-api/context-schema.js rename to lib/services/context-schema.js index 698cec87c1..89e5205c2d 100644 --- a/lib/routes/admin-api/context-schema.js +++ b/lib/services/context-schema.js @@ -1,7 +1,7 @@ 'use strict'; const joi = require('joi'); -const { nameType } = require('./util'); +const { nameType } = require('../routes/admin-api/util'); const nameSchema = joi.object().keys({ name: nameType }); diff --git a/lib/services/context-service.js b/lib/services/context-service.js new file mode 100644 index 0000000000..ef29b175b0 --- /dev/null +++ b/lib/services/context-service.js @@ -0,0 +1,92 @@ +'use strict'; + +const { contextSchema, nameSchema } = require('./context-schema'); +const NameExistsError = require('../error/name-exists-error'); + +const { + CONTEXT_FIELD_CREATED, + CONTEXT_FIELD_UPDATED, + CONTEXT_FIELD_DELETED, +} = require('../event-type'); + +class ContextService { + constructor( + { projectStore, eventStore, contextFieldStore }, + { getLogger }, + ) { + this.projectStore = projectStore; + this.eventStore = eventStore; + this.contextFieldStore = contextFieldStore; + this.logger = getLogger('services/context-service.js'); + } + + async getAll() { + return this.contextFieldStore.getAll(); + } + + async getContextField(name) { + return this.contextFieldStore.get(name); + } + + async createContextField(value, userName) { + // validations + await this.validateUniqueName(value); + const contextField = await contextSchema.validateAsync(value); + + // creations + await this.contextFieldStore.create(value); + await this.eventStore.store({ + type: CONTEXT_FIELD_CREATED, + createdBy: userName, + data: contextField, + }); + } + + async updateContextField(updatedContextField, userName) { + // validations + await this.contextFieldStore.get(updatedContextField.name); + const value = await contextSchema.validateAsync(updatedContextField); + + // update + await this.contextFieldStore.update(value); + await this.eventStore.store({ + type: CONTEXT_FIELD_UPDATED, + createdBy: userName, + data: value, + }); + } + + async deleteContextField(name, userName) { + // validate existence + await this.contextFieldStore.get(name); + + // delete + await this.contextFieldStore.delete(name); + await this.eventStore.store({ + type: CONTEXT_FIELD_DELETED, + createdBy: userName, + data: { name }, + }); + } + + async validateUniqueName({ name }) { + let msg; + try { + await this.contextFieldStore.get(name); + msg = 'A context field with that name already exist'; + } catch (error) { + // No conflict, everything ok! + return; + } + + // Intentional throw here! + throw new NameExistsError(msg); + } + + async validateName(name) { + await nameSchema.validateAsync({ name }); + await this.validateUniqueName({ name }); + } +} + +module.exports = ContextService; diff --git a/lib/services/index.js b/lib/services/index.js index e72a3cf7ba..1d77698cc9 100644 --- a/lib/services/index.js +++ b/lib/services/index.js @@ -6,6 +6,7 @@ const TagTypeService = require('./tag-type-service'); const TagService = require('./tag-service'); const StrategyService = require('./strategy-service'); const AddonService = require('./addon-service'); +const ContextService = require('./context-service'); module.exports.createServices = (stores, config) => { const featureToggleService = new FeatureToggleService(stores, config); @@ -16,6 +17,7 @@ module.exports.createServices = (stores, config) => { const tagService = new TagService(stores, config); const clientMetricsService = new ClientMetricsService(stores, config); const addonService = new AddonService(stores, config, tagTypeService); + const contextService = new ContextService(stores, config); return { addonService, @@ -26,5 +28,6 @@ module.exports.createServices = (stores, config) => { tagTypeService, tagService, clientMetricsService, + contextService, }; }; diff --git a/test/e2e/helpers/database-init.js b/test/e2e/helpers/database-init.js index a4e9d7e5d0..432673674d 100644 --- a/test/e2e/helpers/database-init.js +++ b/test/e2e/helpers/database-init.js @@ -36,7 +36,7 @@ function createStrategies(store) { } function createContextFields(store) { - return dbState.contextFields.map(c => store._createContextField(c)); + return dbState.contextFields.map(c => store.create(c)); } function createApplications(store) { diff --git a/test/fixtures/fake-context-store.js b/test/fixtures/fake-context-store.js index 851d1e6f46..b45d67a58d 100644 --- a/test/fixtures/fake-context-store.js +++ b/test/fixtures/fake-context-store.js @@ -3,7 +3,7 @@ const NotFoundError = require('../../lib/error/notfound-error'); module.exports = () => { - const _contextFields = [ + let _contextFields = [ { name: 'environment' }, { name: 'userId' }, { name: 'appName' }, @@ -19,5 +19,13 @@ module.exports = () => { return Promise.reject(NotFoundError); }, create: contextField => _contextFields.push(contextField), + update: field => { + _contextFields = _contextFields.map(c => + c.name === field.name ? field : c, + ); + }, + delete: name => { + _contextFields = _contextFields.filter(c => c.name !== name); + }, }; };