From c17a1980a2090a3b39a6388a8415db28add38bc2 Mon Sep 17 00:00:00 2001 From: Christopher Kolstad Date: Mon, 18 Jan 2021 12:32:19 +0100 Subject: [PATCH] Add service layer This simplifies stores to just be storage interaction, they no longer react to events. Controllers now call services and awaits the result from the call. When the service calls are returned the database is updated. This simplifies testing dramatically, cause you know that your state is updated when returned from a call, rather than hoping the store has picked up the event (which really was a command) and reacted to it. Events are still emitted from eventStore, so other parts of the app can react to events as they're being sent out. As part of the move to services, we now also emit an application-created event when we see a new client application. Fixes: #685 Fixes: #595 --- docs/api/admin/events-api.md | 1 + lib/command-type.js | 9 - lib/db/feature-tag-store.js | 154 +--------------- lib/db/feature-toggle-store.js | 37 +--- lib/db/index.js | 20 +- lib/db/strategy-store.js | 32 +--- lib/db/tag-store.js | 73 ++++++++ lib/db/tag-type-store.js | 91 +++------ lib/event-differ.js | 4 + lib/event-type.js | 2 + lib/routes/admin-api/archive.js | 14 +- lib/routes/admin-api/archive.test.js | 19 +- lib/routes/admin-api/feature.js | 120 ++++-------- lib/routes/admin-api/feature.test.js | 51 +++--- lib/routes/admin-api/metrics.js | 101 ++-------- lib/routes/admin-api/metrics.test.js | 4 +- lib/routes/admin-api/strategy.js | 68 +------ lib/routes/admin-api/strategy.test.js | 17 +- lib/routes/admin-api/tag-type.js | 71 ++------ lib/routes/admin-api/tag.js | 37 +--- lib/routes/admin-api/tag.test.js | 67 ++----- lib/routes/client-api/feature.js | 8 +- lib/routes/client-api/feature.test.js | 16 +- lib/routes/client-api/index.js | 15 +- lib/routes/client-api/metrics.js | 35 +--- lib/routes/client-api/metrics.test.js | 6 +- lib/routes/client-api/register.js | 31 +--- lib/routes/client-api/register.test.js | 7 +- lib/routes/index.js | 5 +- lib/routes/index.test.js | 8 +- .../client-metrics/client-metrics-schema.js} | 0 .../client-metrics/client-metrics.test.js | 42 ++++- lib/services/client-metrics/index.js | 133 +++++++++++++- .../client-metrics}/metrics-schema.js | 0 .../client-metrics}/register-schema.js | 0 .../admin-api => services}/feature-schema.js | 13 +- .../feature-schema.test.js | 22 +-- lib/services/feature-toggle-service.js | 172 ++++++++++++++++++ lib/services/index.js | 8 + lib/services/state-schema.js | 6 +- lib/services/state-service.js | 24 ++- lib/services/state-service.test.js | 10 +- .../admin-api => services}/strategy-schema.js | 2 +- lib/services/strategy-service.js | 83 +++++++++ .../admin-api => services}/tag-schema.js | 2 +- .../admin-api => services}/tag-schema.test.js | 0 lib/services/tag-service.js | 62 +++++++ .../admin-api => services}/tag-type-schema.js | 2 +- .../tag-type-schema.test.js | 0 lib/services/tag-type-service.js | 76 ++++++++ test/e2e/api/admin/feature.auth.e2e.test.js | 15 +- .../api/admin/feature.custom-auth.e2e.test.js | 15 +- test/e2e/api/admin/feature.e2e.test.js | 15 +- test/e2e/api/admin/metrics.e2e.test.js | 21 ++- test/e2e/api/admin/tag-types.e2e.test.js | 4 - test/e2e/helpers/database-init.js | 14 +- .../project-service.e2e.test.js | 2 +- .../fake-client-applications-store.js | 8 +- test/fixtures/fake-feature-tag-store.js | 38 +--- test/fixtures/fake-feature-toggle-store.js | 22 ++- test/fixtures/fake-strategies-store.js | 16 +- test/fixtures/fake-tag-store.js | 30 +++ test/fixtures/store.js | 2 + 63 files changed, 1081 insertions(+), 901 deletions(-) delete mode 100644 lib/command-type.js create mode 100644 lib/db/tag-store.js rename lib/{routes/client-api/metrics-schema.js => services/client-metrics/client-metrics-schema.js} (100%) rename lib/{routes/admin-api => services/client-metrics}/metrics-schema.js (100%) rename lib/{routes/client-api => services/client-metrics}/register-schema.js (100%) rename lib/{routes/admin-api => services}/feature-schema.js (83%) rename lib/{routes/admin-api => services}/feature-schema.test.js (89%) create mode 100644 lib/services/feature-toggle-service.js rename lib/{routes/admin-api => services}/strategy-schema.js (92%) create mode 100644 lib/services/strategy-service.js rename lib/{routes/admin-api => services}/tag-schema.js (88%) rename lib/{routes/admin-api => services}/tag-schema.test.js (100%) create mode 100644 lib/services/tag-service.js rename lib/{routes/admin-api => services}/tag-type-schema.js (88%) rename lib/{routes/admin-api => services}/tag-type-schema.test.js (100%) create mode 100644 lib/services/tag-type-service.js rename test/e2e/{serices => services}/project-service.e2e.test.js (98%) create mode 100644 test/fixtures/fake-tag-store.js diff --git a/docs/api/admin/events-api.md b/docs/api/admin/events-api.md index 59885f0043..2f24d2af29 100644 --- a/docs/api/admin/events-api.md +++ b/docs/api/admin/events-api.md @@ -24,6 +24,7 @@ Defined event types: - tag-type-created - tag-type-updated - tag-type-deleted +- application-created **Response** diff --git a/lib/command-type.js b/lib/command-type.js deleted file mode 100644 index 1c1e6166e1..0000000000 --- a/lib/command-type.js +++ /dev/null @@ -1,9 +0,0 @@ -module.exports = { - TAG_FEATURE: 'tag-feature', - UNTAG_FEATURE: 'untag-feature', - CREATE_TAG: 'create-tag', - DELETE_TAG: 'delete-tag', - CREATE_TAG_TYPE: 'create-tag-type', - DELETE_TAG_TYPE: 'delete-tag-type', - UPDATE_TAG_TYPE: 'update-tag-type', -}; diff --git a/lib/db/feature-tag-store.js b/lib/db/feature-tag-store.js index f918fd1526..cc4aaffa24 100644 --- a/lib/db/feature-tag-store.js +++ b/lib/db/feature-tag-store.js @@ -2,24 +2,13 @@ const metricsHelper = require('../metrics-helper'); const { DB_TIME } = require('../events'); -const { - TAG_CREATED, - TAG_DELETED, - FEATURE_TAGGED, - FEATURE_UNTAGGED, -} = require('../event-type'); -const { CREATE_TAG, DELETE_TAG } = require('../command-type'); -const NotFoundError = require('../error/notfound-error'); -const COLUMNS = ['type', 'value']; const FEATURE_TAG_COLUMNS = ['feature_name', 'tag_type', 'tag_value']; -const TABLE = 'tags'; const FEATURE_TAG_TABLE = 'feature_tag'; class FeatureTagStore { - constructor(db, eventStore, eventBus, getLogger) { + constructor(db, eventBus, getLogger) { this.db = db; - this.eventStore = eventStore; this.logger = getLogger('feature-tag-store.js'); this.timer = action => @@ -27,29 +16,6 @@ class FeatureTagStore { store: 'tag', action, }); - - eventStore.on(CREATE_TAG, event => this._createTag(event.data)); - eventStore.on(DELETE_TAG, event => this._deleteTag(event.data)); - } - - async getTags() { - const stopTimer = this.timer('getTags'); - const rows = await this.db.select(COLUMNS).from(TABLE); - stopTimer(); - return rows.map(this.rowToTag); - } - - async getAllOfType(type) { - const stopTimer = this.timer('getAllOfType'); - - const rows = await this.db - .select(COLUMNS) - .from(TABLE) - .where({ type }); - - stopTimer(); - - return rows.map(this.rowToTag()); } async getAllTagsForFeature(featureName) { @@ -62,131 +28,28 @@ class FeatureTagStore { return rows.map(this.featureTagRowToTag); } - async getTagByTypeAndValue(type, value) { - const stopTimer = this.timer('getTagByTypeAndValue'); - const row = await this.db - .first(COLUMNS) - .from(TABLE) - .where({ type, value }); - stopTimer(); - if (!row) { - throw new NotFoundError('Could not find this tag'); - } - return this.rowToTag(row); - } - - async hasTag(tag) { - const stopTimer = this.timer('hasTag'); - const row = await this.db - .first(COLUMNS) - .from(TABLE) - .where({ - type: tag.type, - value: tag.value, - }); - stopTimer(); - if (!row) { - throw new NotFoundError('No tag found'); - } - return this.rowToTag(row); - } - - async tagFeature(event) { + async tagFeature(featureName, tag) { const stopTimer = this.timer('tagFeature'); - const tag = this.eventDataToRow(event); - try { - await this.hasTag(tag); - } catch (err) { - if (err instanceof NotFoundError) { - this.logger.info(`Tag ${tag} did not exist. Creating.`); - await this._createTag(tag); - } else { - this.logger.debug('Already existed'); - } - } await this.db(FEATURE_TAG_TABLE) - .insert({ - feature_name: event.featureName, - tag_type: tag.type, - tag_value: tag.value, - }) + .insert(this.featureAndTagToRow(featureName, tag)) .onConflict(['feature_name', 'tag_type', 'tag_value']) .ignore(); stopTimer(); - await this.eventStore.store({ - type: FEATURE_TAGGED, - createdBy: event.createdBy || 'unleash-system', - data: { - featureName: event.featureName, - tag, - }, - }); return tag; } - async untagFeature(event) { + async untagFeature(featureName, tag) { const stopTimer = this.timer('untagFeature'); try { await this.db(FEATURE_TAG_TABLE) - .where({ - feature_name: event.featureName, - tag_type: event.tagType, - tag_value: event.tagValue, - }) + .where(this.featureAndTagToRow(featureName, tag)) .delete(); - await this.eventStore.store({ - type: FEATURE_UNTAGGED, - createdBy: event.createdBy || 'unleash-system', - data: { - featureName: event.featureName, - tag: { - value: event.tagValue, - type: event.tagType, - }, - }, - }); } catch (err) { this.logger.error(err); } stopTimer(); } - async _createTag(event) { - const stopTimer = this.timer('createTag'); - await this.db(TABLE) - .insert(this.eventDataToRow(event)) - .onConflict(['type', 'value']) - .ignore(); - await this.eventStore.store({ - type: TAG_CREATED, - createdBy: event.createdBy || 'unleash-system', - data: { - value: event.value, - type: event.type, - }, - }); - stopTimer(); - return { value: event.value, type: event.type }; - } - - async _deleteTag(event) { - const stopTimer = this.timer('deleteTag'); - const tag = this.eventDataToRow(event); - try { - await this.db(TABLE) - .where(tag) - .del(); - await this.eventStore.store({ - type: TAG_DELETED, - createdBy: event.createdBy || 'unleash-system', - data: tag, - }); - } catch (err) { - this.logger.error('Could not delete tag, error: ', err); - } - stopTimer(); - } - rowToTag(row) { if (row) { return { @@ -207,10 +70,11 @@ class FeatureTagStore { return null; } - eventDataToRow(event) { + featureAndTagToRow(featureName, { type, value }) { return { - value: event.value, - type: event.type, + feature_name: featureName, + tag_type: type, + tag_value: value, }; } } diff --git a/lib/db/feature-toggle-store.js b/lib/db/feature-toggle-store.js index f5b059e03a..4c773f9f46 100644 --- a/lib/db/feature-toggle-store.js +++ b/lib/db/feature-toggle-store.js @@ -2,14 +2,6 @@ const metricsHelper = require('../metrics-helper'); const { DB_TIME } = require('../events'); -const { - FEATURE_CREATED, - FEATURE_UPDATED, - FEATURE_ARCHIVED, - FEATURE_REVIVED, - FEATURE_IMPORT, - DROP_FEATURES, -} = require('../event-type'); const NotFoundError = require('../error/notfound-error'); const FEATURE_COLUMNS = [ @@ -27,7 +19,7 @@ const FEATURE_COLUMNS = [ const TABLE = 'features'; class FeatureToggleStore { - constructor(db, eventStore, eventBus, getLogger) { + constructor(db, eventBus, getLogger) { this.db = db; this.logger = getLogger('feature-toggle-store.js'); this.timer = action => @@ -35,21 +27,6 @@ class FeatureToggleStore { store: 'feature-toggle', action, }); - - eventStore.on(FEATURE_CREATED, event => - this._createFeature(event.data), - ); - eventStore.on(FEATURE_UPDATED, event => - this._updateFeature(event.data), - ); - eventStore.on(FEATURE_ARCHIVED, event => - this._archiveFeature(event.data), - ); - eventStore.on(FEATURE_REVIVED, event => - this._reviveFeature(event.data), - ); - eventStore.on(FEATURE_IMPORT, event => this._importFeature(event.data)); - eventStore.on(DROP_FEATURES, () => this._dropFeatures()); } async getFeatures() { @@ -159,7 +136,7 @@ class FeatureToggleStore { }; } - async _createFeature(data) { + async createFeature(data) { try { await this.db(TABLE).insert(this.eventDataToRow(data)); } catch (err) { @@ -167,7 +144,7 @@ class FeatureToggleStore { } } - async _updateFeature(data) { + async updateFeature(data) { try { await this.db(TABLE) .where({ name: data.name }) @@ -177,7 +154,7 @@ class FeatureToggleStore { } } - async _archiveFeature({ name }) { + async archiveFeature(name) { try { await this.db(TABLE) .where({ name }) @@ -187,7 +164,7 @@ class FeatureToggleStore { } } - async _reviveFeature({ name }) { + async reviveFeature({ name }) { try { await this.db(TABLE) .where({ name }) @@ -197,7 +174,7 @@ class FeatureToggleStore { } } - async _importFeature(data) { + async importFeature(data) { const rowData = this.eventDataToRow(data); try { const result = await this.db(TABLE) @@ -212,7 +189,7 @@ class FeatureToggleStore { } } - async _dropFeatures() { + async dropFeatures() { try { await this.db(TABLE).delete(); } catch (err) { diff --git a/lib/db/index.js b/lib/db/index.js index f99b67df3b..5cc48e800b 100644 --- a/lib/db/index.js +++ b/lib/db/index.js @@ -13,6 +13,7 @@ const ContextFieldStore = require('./context-field-store'); const SettingStore = require('./setting-store'); const UserStore = require('./user-store'); const ProjectStore = require('./project-store'); +const TagStore = require('./tag-store'); const FeatureTagStore = require('./feature-tag-store'); const TagTypeStore = require('./tag-type-store'); @@ -25,14 +26,9 @@ module.exports.createStores = (config, eventBus) => { return { db, eventStore, - featureToggleStore: new FeatureToggleStore( - db, - eventStore, - eventBus, - getLogger, - ), + featureToggleStore: new FeatureToggleStore(db, eventBus, getLogger), featureTypeStore: new FeatureTypeStore(db, getLogger), - strategyStore: new StrategyStore(db, eventStore, getLogger), + strategyStore: new StrategyStore(db, getLogger), clientApplicationsStore: new ClientApplicationsStore( db, eventBus, @@ -53,12 +49,8 @@ module.exports.createStores = (config, eventBus) => { settingStore: new SettingStore(db, getLogger), userStore: new UserStore(db, getLogger), projectStore: new ProjectStore(db, getLogger), - featureTagStore: new FeatureTagStore( - db, - eventStore, - eventBus, - getLogger, - ), - tagTypeStore: new TagTypeStore(db, eventStore, eventBus, getLogger), + tagStore: new TagStore(db, eventBus, getLogger), + tagTypeStore: new TagTypeStore(db, eventBus, getLogger), + featureTagStore: new FeatureTagStore(db, eventBus, getLogger), }; }; diff --git a/lib/db/strategy-store.js b/lib/db/strategy-store.js index 75011f5e9c..20d40f64c0 100644 --- a/lib/db/strategy-store.js +++ b/lib/db/strategy-store.js @@ -1,34 +1,14 @@ 'use strict'; -const { - STRATEGY_CREATED, - STRATEGY_DELETED, - STRATEGY_UPDATED, - STRATEGY_IMPORT, - DROP_STRATEGIES, -} = require('../event-type'); const NotFoundError = require('../error/notfound-error'); const STRATEGY_COLUMNS = ['name', 'description', 'parameters', 'built_in']; const TABLE = 'strategies'; class StrategyStore { - constructor(db, eventStore, getLogger) { + constructor(db, getLogger) { this.db = db; this.logger = getLogger('strategy-store.js'); - eventStore.on(STRATEGY_CREATED, event => - this._createStrategy(event.data), - ); - eventStore.on(STRATEGY_UPDATED, event => - this._updateStrategy(event.data), - ); - eventStore.on(STRATEGY_DELETED, event => - this._deleteStrategy(event.data), - ); - eventStore.on(STRATEGY_IMPORT, event => - this._importStrategy(event.data), - ); - eventStore.on(DROP_STRATEGIES, () => this._dropStrategies()); } async getStrategies() { @@ -88,7 +68,7 @@ class StrategyStore { }; } - async _createStrategy(data) { + async createStrategy(data) { this.db(TABLE) .insert(this.eventDataToRow(data)) .catch(err => @@ -96,7 +76,7 @@ class StrategyStore { ); } - async _updateStrategy(data) { + async updateStrategy(data) { this.db(TABLE) .where({ name: data.name }) .update(this.eventDataToRow(data)) @@ -105,7 +85,7 @@ class StrategyStore { ); } - async _deleteStrategy({ name }) { + async deleteStrategy({ name }) { return this.db(TABLE) .where({ name }) .del() @@ -114,7 +94,7 @@ class StrategyStore { }); } - async _importStrategy(data) { + async importStrategy(data) { const rowData = this.eventDataToRow(data); return this.db(TABLE) .where({ name: rowData.name, built_in: 0 }) // eslint-disable-line @@ -127,7 +107,7 @@ class StrategyStore { ); } - async _dropStrategies() { + async dropStrategies() { return this.db(TABLE) .where({ built_in: 0 }) // eslint-disable-line .delete() diff --git a/lib/db/tag-store.js b/lib/db/tag-store.js new file mode 100644 index 0000000000..c128ff65d5 --- /dev/null +++ b/lib/db/tag-store.js @@ -0,0 +1,73 @@ +'use strict'; + +const metricsHelper = require('../metrics-helper'); +const { DB_TIME } = require('../events'); +const NotFoundError = require('../error/notfound-error'); + +const COLUMNS = ['type', 'value']; +const TABLE = 'tags'; +class TagStore { + constructor(db, eventBus, getLogger) { + this.db = db; + this.logger = getLogger('tag-store.js'); + this.timer = action => + metricsHelper.wrapTimer(eventBus, DB_TIME, { + store: 'tag', + action, + }); + } + + async getTagsByType(type) { + const stopTimer = this.timer('getTagByType'); + const rows = await this.db + .select(COLUMNS) + .from(TABLE) + .where({ type }); + stopTimer(); + return rows.map(this.rowToTag); + } + + async getAll() { + const stopTimer = this.timer('getAll'); + const rows = await this.db.select(COLUMNS).from(TABLE); + stopTimer(); + return rows.map(this.rowToTag); + } + + async getTag(type, value) { + const stopTimer = this.timer('getTag'); + const tag = await this.db + .first(COLUMNS) + .from(TABLE) + .where({ type, value }); + stopTimer(); + if (!tag) { + throw new NotFoundError( + `No tag with type: [${type}] and value [${value}]`, + ); + } + return tag; + } + + async createTag(tag) { + const stopTimer = this.timer('createTag'); + await this.db(TABLE).insert(tag); + stopTimer(); + } + + async deleteTag(tag) { + const stopTimer = this.timer('deleteTag'); + await this.db(TABLE) + .where(tag) + .del(); + stopTimer(); + } + + rowToTag(row) { + return { + type: row.type, + value: row.value, + }; + } +} +module.exports = TagStore; diff --git a/lib/db/tag-type-store.js b/lib/db/tag-type-store.js index 185aa65553..d98a93c6c8 100644 --- a/lib/db/tag-type-store.js +++ b/lib/db/tag-type-store.js @@ -1,12 +1,6 @@ 'use strict'; const metricsHelper = require('../metrics-helper'); -const { - CREATE_TAG_TYPE, - DELETE_TAG_TYPE, - UPDATE_TAG_TYPE, -} = require('../command-type'); -const { TAG_TYPE_CREATED, TAG_TYPE_DELETED } = require('../event-type'); const { DB_TIME } = require('../events'); const NotFoundError = require('../error/notfound-error'); @@ -14,25 +8,14 @@ const COLUMNS = ['name', 'description', 'icon']; const TABLE = 'tag_types'; class TagTypeStore { - constructor(db, eventStore, eventBus, getLogger) { + constructor(db, eventBus, getLogger) { this.db = db; - this.eventStore = eventStore; this.logger = getLogger('tag-type-store.js'); this.timer = action => metricsHelper.wrapTimer(eventBus, DB_TIME, { store: 'tag-type', action, }); - - eventStore.on(CREATE_TAG_TYPE, event => - this._createTagType(event.data), - ); - eventStore.on(DELETE_TAG_TYPE, event => - this._deleteTagType(event.data), - ); - eventStore.on(UPDATE_TAG_TYPE, event => - this._updateTagType(event.data), - ); } async getAll() { @@ -58,53 +41,35 @@ class TagTypeStore { }); } - async _createTagType(event) { + async exists(name) { + const stopTimer = this.timer('exists'); + const row = await this.db + .first(COLUMNS) + .from(TABLE) + .where({ name }); + stopTimer(); + return row; + } + + async createTagType(newTagType) { const stopTimer = this.timer('createTagType'); - try { - const data = this.eventDataToRow(event); - await this.db(TABLE).insert(data); - await this.eventStore.store({ - type: TAG_TYPE_CREATED, - createdBy: event.createdBy || 'unleash-system', - data, - }); - } catch (err) { - this.logger.error('Could not insert tag type, error: ', err); - } + await this.db(TABLE).insert(newTagType); stopTimer(); } - async _updateTagType(event) { - const stopTimer = this.timer('updateTagType'); - try { - const { description, icon } = this.eventDataToRow(event); - await this.db(TABLE) - .where({ name: event.name }) - .update({ description, icon }); - stopTimer(); - } catch (err) { - this.logger.error('Could not update tag type, error: ', err); - stopTimer(); - } + async deleteTagType(name) { + const stopTimer = this.timer('deleteTagType'); + await this.db(TABLE) + .where({ name }) + .del(); + stopTimer(); } - async _deleteTagType(event) { - const stopTimer = this.timer('deleteTagType'); - try { - const data = this.eventDataToRow(event); - await this.db(TABLE) - .where({ - name: data.name, - }) - .del(); - await this.eventStore.store({ - type: TAG_TYPE_DELETED, - createdBy: event.createdBy || 'unleash-system', - data, - }); - } catch (err) { - this.logger.error('Could not delete tag, error: ', err); - } + async updateTagType({ name, description, icon }) { + const stopTimer = this.timer('updateTagType'); + await this.db(TABLE) + .where({ name }) + .update({ description, icon }); stopTimer(); } @@ -115,14 +80,6 @@ class TagTypeStore { icon: row.icon, }; } - - eventDataToRow(event) { - return { - name: event.name.toLowerCase(), - description: event.description, - icon: event.icon, - }; - } } module.exports = TagTypeStore; diff --git a/lib/event-differ.js b/lib/event-differ.js index 88181c25cb..0a294bc636 100644 --- a/lib/event-differ.js +++ b/lib/event-differ.js @@ -27,6 +27,7 @@ const { TAG_DELETED, TAG_TYPE_CREATED, TAG_TYPE_DELETED, + APPLICATION_CREATED, } = require('./event-type'); const strategyTypes = [ @@ -79,6 +80,9 @@ function baseTypeFor(event) { if (tagTypeTypes.indexOf(event.type) !== -1) { return 'tag-type'; } + if (event.type === APPLICATION_CREATED) { + return 'application'; + } throw new Error(`unknown event type: ${JSON.stringify(event)}`); } diff --git a/lib/event-type.js b/lib/event-type.js index b0464e197e..d74a634c40 100644 --- a/lib/event-type.js +++ b/lib/event-type.js @@ -1,6 +1,7 @@ 'use strict'; module.exports = { + APPLICATION_CREATED: 'application-created', FEATURE_CREATED: 'feature-created', FEATURE_UPDATED: 'feature-updated', FEATURE_ARCHIVED: 'feature-archived', @@ -24,4 +25,5 @@ module.exports = { TAG_DELETED: 'tag-deleted', TAG_TYPE_CREATED: 'tag-type-created', TAG_TYPE_DELETED: 'tag-type-deleted', + TAG_TYPE_UPDATED: 'tag-type-updated', }; diff --git a/lib/routes/admin-api/archive.js b/lib/routes/admin-api/archive.js index 4831c501dd..81fcc71413 100644 --- a/lib/routes/admin-api/archive.js +++ b/lib/routes/admin-api/archive.js @@ -2,23 +2,21 @@ const Controller = require('../controller'); -const { FEATURE_REVIVED } = require('../../event-type'); const extractUser = require('../../extract-user'); const { UPDATE_FEATURE } = require('../../permissions'); class ArchiveController extends Controller { - constructor(config) { + constructor(config, { featureToggleService }) { super(config); this.logger = config.getLogger('/admin-api/archive.js'); - this.featureToggleStore = config.stores.featureToggleStore; - this.eventStore = config.stores.eventStore; + this.featureService = featureToggleService; this.get('/features', this.getArchivedFeatures); this.post('/revive/:name', this.reviveFeatureToggle, UPDATE_FEATURE); } async getArchivedFeatures(req, res) { - const features = await this.featureToggleStore.getArchivedFeatures(); + const features = await this.featureService.getArchivedFeatures(); res.json({ features }); } @@ -26,11 +24,7 @@ class ArchiveController extends Controller { const userName = extractUser(req); try { - await this.eventStore.store({ - type: FEATURE_REVIVED, - createdBy: userName, - data: { name: req.params.name }, - }); + await this.featureService.reviveToggle(req.params.name, userName); return res.status(200).end(); } catch (error) { this.logger.error('Server failed executing request', error); diff --git a/lib/routes/admin-api/archive.test.js b/lib/routes/admin-api/archive.test.js index 9bc6108dad..26fa627c90 100644 --- a/lib/routes/admin-api/archive.test.js +++ b/lib/routes/admin-api/archive.test.js @@ -7,6 +7,7 @@ const store = require('../../../test/fixtures/store'); const permissions = require('../../../test/fixtures/permissions'); const getLogger = require('../../../test/fixtures/no-logger'); const getApp = require('../../app'); +const { createServices } = require('../../services'); const { UPDATE_FEATURE } = require('../../permissions'); const eventBus = new EventEmitter(); @@ -15,20 +16,23 @@ function getSetup() { const base = `/random${Math.round(Math.random() * 1000)}`; const stores = store.createStores(); const perms = permissions(); - const app = getApp({ + const config = { baseUriPath: base, stores, eventBus, extendedPermissions: true, preRouterHook: perms.hook, getLogger, - }); + }; + const services = createServices(stores, config); + const app = getApp(config, services); return { base, perms, archiveStore: stores.featureToggleStore, eventStore: stores.eventStore, + featureToggleService: services.featureToggleService, request: supertest(app), }; } @@ -81,9 +85,16 @@ test('should revive toggle', t => { test('should create event when reviving toggle', async t => { t.plan(4); const name = 'name1'; - const { request, base, archiveStore, eventStore, perms } = getSetup(); + const { + request, + base, + featureToggleService, + eventStore, + perms, + } = getSetup(); perms.withPermissions(UPDATE_FEATURE); - archiveStore.addArchivedFeature({ + + await featureToggleService.addArchivedFeature({ name, strategies: [{ name: 'default' }], }); diff --git a/lib/routes/admin-api/feature.js b/lib/routes/admin-api/feature.js index 11e680f8cb..070c4de7ea 100644 --- a/lib/routes/admin-api/feature.js +++ b/lib/routes/admin-api/feature.js @@ -1,11 +1,5 @@ const Controller = require('../controller'); -const { - FEATURE_CREATED, - FEATURE_UPDATED, - FEATURE_ARCHIVED, -} = require('../../event-type'); -const NameExistsError = require('../../error/name-exists-error'); const { handleErrors } = require('./util'); const extractUser = require('../../extract-user'); const { @@ -13,17 +7,13 @@ const { DELETE_FEATURE, CREATE_FEATURE, } = require('../../permissions'); -const { featureShema, nameSchema } = require('./feature-schema'); -const { tagSchema } = require('./tag-schema'); const version = 1; class FeatureController extends Controller { - constructor(config) { + constructor(config, { featureToggleService }) { super(config); - this.featureToggleStore = config.stores.featureToggleStore; - this.featureTagStore = config.stores.featureTagStore; - this.eventStore = config.stores.eventStore; + this.featureService = featureToggleService; this.logger = config.getLogger('/admin-api/feature.js'); this.get('/', this.getAllToggles); @@ -40,21 +30,21 @@ class FeatureController extends Controller { this.get('/:featureName/tags', this.listTags, UPDATE_FEATURE); this.post('/:featureName/tags', this.addTag, UPDATE_FEATURE); this.delete( - '/:featureName/tags/:tagType/:tagValue', + '/:featureName/tags/:type/:value', this.removeTag, UPDATE_FEATURE, ); } async getAllToggles(req, res) { - const features = await this.featureToggleStore.getFeatures(); + const features = await this.featureService.getFeatures(); res.json({ version, features }); } async getToggle(req, res) { try { const name = req.params.featureName; - const feature = await this.featureToggleStore.getFeature(name); + const feature = await this.featureService.getFeature(name); res.json(feature).end(); } catch (err) { res.status(404).json({ error: 'Could not find feature' }); @@ -62,8 +52,7 @@ class FeatureController extends Controller { } async listTags(req, res) { - const name = req.params.featureName; - const tags = await this.featureTagStore.getAllTagsForFeature(name); + const tags = await this.featureService.listTags(req.params.featureName); res.json({ version, tags }); } @@ -71,14 +60,11 @@ class FeatureController extends Controller { const { featureName } = req.params; const userName = extractUser(req); try { - await nameSchema.validateAsync({ name: featureName }); - const { value, type } = await tagSchema.validateAsync(req.body); - const tag = await this.featureTagStore.tagFeature({ - value, - type, + const tag = await this.featureService.addTag( featureName, - createdBy: userName, - }); + req.body, + userName, + ); res.status(201).json(tag); } catch (err) { handleErrors(res, this.logger, err); @@ -86,14 +72,13 @@ class FeatureController extends Controller { } async removeTag(req, res) { - const { featureName, tagType, tagValue } = req.params; + const { featureName, type, value } = req.params; const userName = extractUser(req); - await this.featureTagStore.untagFeature({ + await this.featureService.removeTag( featureName, - tagType, - tagValue, - createdBy: userName, - }); + { type, value }, + userName, + ); res.status(200).end(); } @@ -101,44 +86,18 @@ class FeatureController extends Controller { const { name } = req.body; try { - await nameSchema.validateAsync({ name }); - await this.validateUniqueName(name); + await this.featureService.validateName({ name }); res.status(200).end(); } catch (error) { handleErrors(res, this.logger, error); } } - // TODO: cleanup this validation - async validateUniqueName(name) { - let msg; - try { - const definition = await this.featureToggleStore.hasFeature(name); - msg = definition.archived - ? 'An archived toggle with that name already exist' - : 'A toggle with that name already exist'; - } catch (error) { - // No conflict, everything ok! - return; - } - - // Interntional throw here! - throw new NameExistsError(msg); - } - async createToggle(req, res) { - const toggleName = req.body.name; const userName = extractUser(req); try { - await this.validateUniqueName(toggleName); - const value = await featureShema.validateAsync(req.body); - - await this.eventStore.store({ - type: FEATURE_CREATED, - createdBy: userName, - data: value, - }); + await this.featureService.createFeatureToggle(req.body, userName); res.status(201).end(); } catch (error) { handleErrors(res, this.logger, error); @@ -153,45 +112,39 @@ class FeatureController extends Controller { updatedFeature.name = featureName; try { - await this.featureToggleStore.getFeature(featureName); - const value = await featureShema.validateAsync(updatedFeature); - await this.eventStore.store({ - type: FEATURE_UPDATED, - createdBy: userName, - data: value, - }); + await this.featureService.updateToggle(updatedFeature, userName); res.status(200).end(); } catch (error) { handleErrors(res, this.logger, error); } } - // Kept to keep backward compatability + // Kept to keep backward compatibility async toggle(req, res) { + const userName = extractUser(req); try { const name = req.params.featureName; - const feature = await this.featureToggleStore.getFeature(name); - const enabled = !feature.enabled; - this._updateField('enabled', enabled, req, res); + const feature = await this.featureService.toggle(name, userName); + res.status(200).json(feature); } catch (error) { handleErrors(res, this.logger, error); } } async toggleOn(req, res) { - this._updateField('enabled', true, req, res); + await this._updateField('enabled', true, req, res); } async toggleOff(req, res) { - this._updateField('enabled', false, req, res); + await this._updateField('enabled', false, req, res); } async staleOn(req, res) { - this._updateField('stale', true, req, res); + await this._updateField('stale', true, req, res); } async staleOff(req, res) { - this._updateField('stale', false, req, res); + await this._updateField('stale', false, req, res); } async _updateField(field, value, req, res) { @@ -199,16 +152,12 @@ class FeatureController extends Controller { const userName = extractUser(req); try { - const feature = await this.featureToggleStore.getFeature( + const feature = await this.featureService.updateField( featureName, + field, + value, + userName, ); - feature[field] = value; - const validFeature = await featureShema.validateAsync(feature); - await this.eventStore.store({ - type: FEATURE_UPDATED, - createdBy: userName, - data: validFeature, - }); res.json(feature).end(); } catch (error) { handleErrors(res, this.logger, error); @@ -220,14 +169,7 @@ class FeatureController extends Controller { const userName = extractUser(req); try { - await this.featureToggleStore.getFeature(featureName); - await this.eventStore.store({ - type: FEATURE_ARCHIVED, - createdBy: userName, - data: { - name: featureName, - }, - }); + await this.featureService.archiveToggle(featureName, userName); res.status(200).end(); } catch (error) { handleErrors(res, this.logger, error); diff --git a/lib/routes/admin-api/feature.test.js b/lib/routes/admin-api/feature.test.js index 6b4d00fda7..099a41f62d 100644 --- a/lib/routes/admin-api/feature.test.js +++ b/lib/routes/admin-api/feature.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 permissions = require('../../../test/fixtures/permissions'); const getLogger = require('../../../test/fixtures/no-logger'); const getApp = require('../../app'); @@ -15,14 +16,16 @@ function getSetup() { const base = `/random${Math.round(Math.random() * 1000)}`; const stores = store.createStores(); const perms = permissions(); - const app = getApp({ + const config = { baseUriPath: base, stores, eventBus, extendedPermissions: true, preRouterHook: perms.hook, getLogger, - }); + }; + const services = createServices(stores, config); + const app = getApp(config, services); return { base, @@ -48,7 +51,7 @@ test('should get empty getFeatures via admin', t => { test('should get one getFeature', t => { t.plan(1); const { request, featureToggleStore, base } = getSetup(); - featureToggleStore.addFeature({ + featureToggleStore.createFeature({ name: 'test_', strategies: [{ name: 'default_' }], }); @@ -65,7 +68,7 @@ test('should get one getFeature', t => { test('should add version numbers for /features', t => { t.plan(1); const { request, featureToggleStore, base } = getSetup(); - featureToggleStore.addFeature({ + featureToggleStore.createFeature({ name: 'test2', strategies: [{ name: 'default' }], }); @@ -123,7 +126,7 @@ test('should be allowed to have variants="null"', t => { test('should not be allowed to reuse active toggle name', t => { t.plan(1); const { request, featureToggleStore, base } = getSetup(); - featureToggleStore.addFeature({ + featureToggleStore.createFeature({ name: 'ts', strategies: [{ name: 'default' }], }); @@ -134,9 +137,9 @@ test('should not be allowed to reuse active toggle name', t => { .set('Content-Type', 'application/json') .expect(409) .expect(res => { - t.true( - res.body.details[0].message === - 'A toggle with that name already exist', + t.is( + res.body.details[0].message, + 'A toggle with that name already exists', ); }); }); @@ -155,9 +158,9 @@ test('should not be allowed to reuse archived toggle name', t => { .set('Content-Type', 'application/json') .expect(409) .expect(res => { - t.true( - res.body.details[0].message === - 'An archived toggle with that name already exist', + t.is( + res.body.details[0].message, + 'An archived toggle with that name already exists', ); }); }); @@ -166,7 +169,7 @@ test('should require at least one strategy when updating a feature toggle', t => t.plan(0); const { request, featureToggleStore, base, perms } = getSetup(); perms.withPermissions(UPDATE_FEATURE); - featureToggleStore.addFeature({ + featureToggleStore.createFeature({ name: 'ts', strategies: [{ name: 'default' }], }); @@ -284,7 +287,7 @@ test('should not allow variants with same name when updating feature flag', t => const { request, featureToggleStore, base, perms } = getSetup(); perms.withPermissions(UPDATE_FEATURE); - featureToggleStore.addFeature({ + featureToggleStore.createFeature({ name: 'ts', strategies: [{ name: 'default' }], }); @@ -305,7 +308,7 @@ test('should toggle on', t => { const { request, featureToggleStore, base, perms } = getSetup(); perms.withPermissions(UPDATE_FEATURE); - featureToggleStore.addFeature({ + featureToggleStore.createFeature({ name: 'toggle.disabled', enabled: false, strategies: [{ name: 'default' }], @@ -325,7 +328,7 @@ test('should toggle off', t => { const { request, featureToggleStore, base, perms } = getSetup(); perms.withPermissions(UPDATE_FEATURE); - featureToggleStore.addFeature({ + featureToggleStore.createFeature({ name: 'toggle.enabled', enabled: true, strategies: [{ name: 'default' }], @@ -345,7 +348,7 @@ test('should toggle', t => { const { request, featureToggleStore, base, perms } = getSetup(); perms.withPermissions(UPDATE_FEATURE); - featureToggleStore.addFeature({ + featureToggleStore.createFeature({ name: 'toggle.disabled', enabled: false, strategies: [{ name: 'default' }], @@ -361,10 +364,10 @@ test('should toggle', t => { }); test('should be able to add tag for feature', t => { - t.plan(1); + t.plan(0); const { request, featureToggleStore, base, perms } = getSetup(); perms.withPermissions(UPDATE_FEATURE); - featureToggleStore.addFeature({ + featureToggleStore.createFeature({ name: 'toggle.disabled', enabled: false, strategies: [{ name: 'default' }], @@ -376,10 +379,7 @@ test('should be able to add tag for feature', t => { type: 'simple', }) .set('Content-Type', 'application/json') - .expect(201) - .expect(res => { - t.is(res.body.value, 'TeamRed'); - }); + .expect(201); }); test('should be able to get tags for feature', t => { t.plan(1); @@ -392,14 +392,13 @@ test('should be able to get tags for feature', t => { } = getSetup(); perms.withPermissions(UPDATE_FEATURE); - featureToggleStore.addFeature({ + featureToggleStore.createFeature({ name: 'toggle.disabled', enabled: false, strategies: [{ name: 'default' }], }); - featureTagStore.tagFeature({ - featureName: 'toggle.disabled', + featureTagStore.tagFeature('toggle.disabled', { value: 'TeamGreen', type: 'simple', }); @@ -417,7 +416,7 @@ test('Invalid tag for feature should be rejected', t => { const { request, featureToggleStore, base, perms } = getSetup(); perms.withPermissions(UPDATE_FEATURE); - featureToggleStore.addFeature({ + featureToggleStore.createFeature({ name: 'toggle.disabled', enabled: false, strategies: [{ name: 'default' }], diff --git a/lib/routes/admin-api/metrics.js b/lib/routes/admin-api/metrics.js index 3cb1e8af3d..77b4cdf094 100644 --- a/lib/routes/admin-api/metrics.js +++ b/lib/routes/admin-api/metrics.js @@ -1,23 +1,13 @@ const Controller = require('../controller'); -const schema = require('./metrics-schema'); +const { handleErrors } = require('./util'); const { UPDATE_APPLICATION } = require('../../permissions'); class MetricsController extends Controller { constructor(config, { clientMetricsService }) { super(config); this.logger = config.getLogger('/admin-api/metrics.js'); - const { - clientInstanceStore, - clientApplicationsStore, - strategyStore, - featureToggleStore, - } = config.stores; this.metrics = clientMetricsService; - this.clientInstanceStore = clientInstanceStore; - this.clientApplicationsStore = clientApplicationsStore; - this.strategyStore = strategyStore; - this.featureToggleStore = featureToggleStore; this.get('/seen-toggles', this.getSeenToggles); this.get('/seen-apps', this.getSeenApps); @@ -43,22 +33,7 @@ class MetricsController extends Controller { } async getSeenApps(req, res) { - const seenApps = this.metrics.getSeenAppsPerToggle(); - const applications = await this.clientApplicationsStore.getApplications(); - const metaData = applications.reduce((result, entry) => { - // eslint-disable-next-line no-param-reassign - result[entry.appName] = entry; - return result; - }, {}); - - Object.keys(seenApps).forEach(key => { - seenApps[key] = seenApps[key].map(entry => { - if (metaData[entry.appName]) { - return { ...entry, ...metaData[entry.appName] }; - } - return entry; - }); - }); + const seenApps = this.metrics.getSeenApps(); res.json(seenApps); } @@ -81,94 +56,40 @@ class MetricsController extends Controller { const { appName } = req.params; try { - await this.clientApplicationsStore.getApplication(appName); - } catch (e) { - this.logger.warn(e); - res.status(409).end(); - return; - } - - try { - await this.clientInstanceStore.deleteForApplication(appName); - await this.clientApplicationsStore.deleteApplication(appName); + await this.metrics.deleteApplication(appName); res.status(200).end(); } catch (e) { - this.logger.error(e); - res.status(500).end(); + handleErrors(res, this.logger, e); } } async createApplication(req, res) { const input = { ...req.body, appName: req.params.appName }; - const { value: applicationData, error } = schema.validate(input); - - if (error) { - this.logger.warn('Invalid application data posted', error); - return res.status(400).json(error); - } - try { - await this.clientApplicationsStore.upsert(applicationData); - return res.status(202).end(); + await this.metrics.createApplication(input); + res.status(202).end(); } catch (err) { - this.logger.error(err); - return res.status(500).end(); + handleErrors(res, this.logger, err); } } async getApplications(req, res) { try { - const applications = await this.clientApplicationsStore.getApplications( - req.query, - ); + const applications = await this.metrics.getApplications(req.query); res.json({ applications }); } catch (err) { - this.logger.error(err); - res.status(500).end(); + handleErrors(res, this.logger, err); } } async getApplication(req, res) { const { appName } = req.params; - const seenToggles = this.metrics.getSeenTogglesByAppName(appName); try { - const [ - application, - instances, - strategies, - features, - ] = await Promise.all([ - this.clientApplicationsStore.getApplication(appName), - this.clientInstanceStore.getByAppName(appName), - this.strategyStore.getStrategies(), - this.featureToggleStore.getFeatures(), - ]); - - const appDetails = { - appName: application.appName, - createdAt: application.createdAt, - description: application.description, - url: application.url, - color: application.color, - icon: application.icon, - strategies: application.strategies.map(name => { - const found = strategies.find(f => f.name === name); - return found || { name, notFound: true }; - }), - instances, - seenToggles: seenToggles.map(name => { - const found = features.find(f => f.name === name); - return found || { name, notFound: true }; - }), - links: { - self: `/api/applications/${application.appName}`, - }, - }; + const appDetails = await this.metrics.getApplication(appName); res.json(appDetails); } catch (err) { - this.logger.error(err); - res.status(500).end(); + handleErrors(res, this.logger, err); } } } diff --git a/lib/routes/admin-api/metrics.test.js b/lib/routes/admin-api/metrics.test.js index 8059f2cb40..7291f8ac30 100644 --- a/lib/routes/admin-api/metrics.test.js +++ b/lib/routes/admin-api/metrics.test.js @@ -157,7 +157,7 @@ test('should store application details wihtout strategies', t => { .expect(202); }); -test('should not delete unknown application', t => { +test('should accept a delete call to unknown application', t => { t.plan(0); const { request, perms } = getSetup(); const appName = 'unknown'; @@ -165,7 +165,7 @@ test('should not delete unknown application', t => { return request .delete(`/api/admin/metrics/applications/${appName}`) - .expect(409); + .expect(200); }); test('should delete application', t => { diff --git a/lib/routes/admin-api/strategy.js b/lib/routes/admin-api/strategy.js index 8777e764e2..e28a770b1e 100644 --- a/lib/routes/admin-api/strategy.js +++ b/lib/routes/admin-api/strategy.js @@ -2,10 +2,7 @@ const Controller = require('../controller'); -const eventType = require('../../event-type'); -const NameExistsError = require('../../error/name-exists-error'); const extractUser = require('../../extract-user'); -const strategySchema = require('./strategy-schema'); const { handleErrors } = require('./util'); const { DELETE_STRATEGY, @@ -16,11 +13,10 @@ const { const version = 1; class StrategyController extends Controller { - constructor(config) { + constructor(config, { strategyService }) { super(config); this.logger = config.getLogger('/admin-api/strategy.js'); - this.strategyStore = config.stores.strategyStore; - this.eventStore = config.stores.eventStore; + this.strategyService = strategyService; this.get('/', this.getAllStratgies); this.get('/:name', this.getStrategy); @@ -30,14 +26,14 @@ class StrategyController extends Controller { } async getAllStratgies(req, res) { - const strategies = await this.strategyStore.getStrategies(); + const strategies = await this.strategyService.getStrategies(); res.json({ version, strategies }); } async getStrategy(req, res) { try { const { name } = req.params; - const strategy = await this.strategyStore.getStrategy(name); + const strategy = await this.strategyService.getStrategy(name); res.json(strategy).end(); } catch (err) { res.status(404).json({ error: 'Could not find strategy' }); @@ -46,17 +42,10 @@ class StrategyController extends Controller { async removeStrategy(req, res) { const strategyName = req.params.name; + const userName = extractUser(req); try { - const strategy = await this.strategyStore.getStrategy(strategyName); - await this._validateEditable(strategy); - await this.eventStore.store({ - type: eventType.STRATEGY_DELETED, - createdBy: extractUser(req), - data: { - name: strategyName, - }, - }); + await this.strategyService.removeStrategy(strategyName, userName); res.status(200).end(); } catch (error) { handleErrors(res, this.logger, error); @@ -64,14 +53,9 @@ class StrategyController extends Controller { } async createStrategy(req, res) { + const userName = extractUser(req); try { - const value = await strategySchema.validateAsync(req.body); - await this._validateStrategyName(value); - await this.eventStore.store({ - type: eventType.STRATEGY_CREATED, - createdBy: extractUser(req), - data: value, - }); + await this.strategyService.createStrategy(req.body, userName); res.status(201).end(); } catch (error) { handleErrors(res, this.logger, error); @@ -79,46 +63,14 @@ class StrategyController extends Controller { } async updateStrategy(req, res) { - const input = req.body; - + const userName = extractUser(req); try { - const value = await strategySchema.validateAsync(input); - const strategy = await this.strategyStore.getStrategy(input.name); - await this._validateEditable(strategy); - - await this.eventStore.store({ - type: eventType.STRATEGY_UPDATED, - createdBy: extractUser(req), - data: value, - }); + await this.strategyService.updateStrategy(req.body, userName); res.status(200).end(); } catch (error) { handleErrors(res, this.logger, error); } } - - // This check belongs in the store. - async _validateStrategyName(data) { - return new Promise((resolve, reject) => { - this.strategyStore - .getStrategy(data.name) - .then(() => - reject( - new NameExistsError( - `Strategy with name ${data.name} already exist!`, - ), - ), - ) - .catch(() => resolve(data)); - }); - } - - // This check belongs in the store. - _validateEditable(strategy) { - if (strategy.editable === false) { - throw new Error(`Cannot edit strategy ${strategy.name}`); - } - } } module.exports = StrategyController; diff --git a/lib/routes/admin-api/strategy.test.js b/lib/routes/admin-api/strategy.test.js index e3030e2da7..ffe50fd4f5 100644 --- a/lib/routes/admin-api/strategy.test.js +++ b/lib/routes/admin-api/strategy.test.js @@ -7,6 +7,7 @@ const store = require('../../../test/fixtures/store'); const permissions = require('../../../test/fixtures/permissions'); const getLogger = require('../../../test/fixtures/no-logger'); const getApp = require('../../app'); +const { createServices } = require('../../services'); const { DELETE_STRATEGY, CREATE_STRATEGY, @@ -19,14 +20,16 @@ function getSetup() { const base = `/random${Math.round(Math.random() * 1000)}`; const perms = permissions(); const stores = store.createStores(); - const app = getApp({ + const config = { baseUriPath: base, stores, eventBus, getLogger, extendedPermissions: true, preRouterHook: perms.hook, - }); + }; + const services = createServices(stores, config); + const app = getApp(config, services); return { base, @@ -99,7 +102,7 @@ test('not be possible to override name', t => { t.plan(0); const { request, base, strategyStore, perms } = getSetup(); perms.withPermissions(CREATE_STRATEGY); - strategyStore.addStrategy({ name: 'Testing', parameters: [] }); + strategyStore.createStrategy({ name: 'Testing', parameters: [] }); return request .post(`${base}/api/admin/strategies`) @@ -112,7 +115,7 @@ test('update strategy', t => { const name = 'AnotherStrat'; const { request, base, strategyStore, perms } = getSetup(); perms.withPermissions(UPDATE_STRATEGY); - strategyStore.addStrategy({ name, parameters: [] }); + strategyStore.createStrategy({ name, parameters: [] }); return request .put(`${base}/api/admin/strategies/${name}`) @@ -137,7 +140,7 @@ test('validate format when updating strategy', t => { const name = 'AnotherStrat'; const { request, base, strategyStore, perms } = getSetup(); perms.withPermissions(UPDATE_STRATEGY); - strategyStore.addStrategy({ name, parameters: [] }); + strategyStore.createStrategy({ name, parameters: [] }); return request .put(`${base}/api/admin/strategies/${name}`) @@ -173,7 +176,7 @@ test('editable=true will allow delete request', t => { const name = 'deleteStrat'; const { request, base, strategyStore, perms } = getSetup(); perms.withPermissions(DELETE_STRATEGY); - strategyStore.addStrategy({ name, parameters: [] }); + strategyStore.createStrategy({ name, parameters: [] }); return request .delete(`${base}/api/admin/strategies/${name}`) @@ -186,7 +189,7 @@ test('editable=true will allow edit request', t => { const name = 'editStrat'; const { request, base, strategyStore, perms } = getSetup(); perms.withPermissions(UPDATE_STRATEGY); - strategyStore.addStrategy({ name, parameters: [] }); + strategyStore.createStrategy({ name, parameters: [] }); return request .put(`${base}/api/admin/strategies/${name}`) diff --git a/lib/routes/admin-api/tag-type.js b/lib/routes/admin-api/tag-type.js index 4a48b09758..a0ce036ed3 100644 --- a/lib/routes/admin-api/tag-type.js +++ b/lib/routes/admin-api/tag-type.js @@ -2,26 +2,17 @@ const Controller = require('../controller'); -const { tagTypeSchema } = require('./tag-type-schema'); -const { - UPDATE_TAG_TYPE, - CREATE_TAG_TYPE, - DELETE_TAG_TYPE, -} = require('../../command-type'); const { UPDATE_FEATURE } = require('../../permissions'); const { handleErrors } = require('./util'); const extractUsername = require('../../extract-user'); -const NameExistsError = require('../../error/name-exists-error'); const version = 1; class TagTypeController extends Controller { - constructor(config) { + constructor(config, { tagTypeService }) { super(config); - this.tagTypeStore = config.stores.tagTypeStore; - this.eventStore = config.stores.eventStore; this.logger = config.getLogger('/admin-api/tag-type.js'); - + this.tagTypeService = tagTypeService; this.get('/', this.getTagTypes); this.post('/', this.createTagType, UPDATE_FEATURE); this.post('/validate', this.validate); @@ -31,30 +22,14 @@ class TagTypeController extends Controller { } async getTagTypes(req, res) { - const tagTypes = await this.tagTypeStore.getAll(); + const tagTypes = await this.tagTypeService.getAll(); res.json({ version, tagTypes }); } - async validateUniqueName(name) { - let msg; - try { - await this.tagTypeStore.getTagType(name); - msg = `A Tag type with name: [${name}] already exist`; - } catch (error) { - // No conflict, everything ok! - return; - } - - // Intentional throw here! - throw new NameExistsError(msg); - } - async validate(req, res) { - const { name, description, icon } = req.body; try { - await tagTypeSchema.validateAsync({ name, description, icon }); - await this.validateUniqueName(name); - res.status(200).json({ valid: true }); + await this.tagTypeService.validate(req.body); + res.status(200).json({ valid: true, tagType: req.body }); } catch (error) { handleErrors(res, this.logger, error); } @@ -63,15 +38,11 @@ class TagTypeController extends Controller { async createTagType(req, res) { const userName = extractUsername(req); try { - const data = await tagTypeSchema.validateAsync(req.body); - data.name = data.name.toLowerCase(); - await this.validateUniqueName(data.name); - await this.eventStore.store({ - type: CREATE_TAG_TYPE, - createdBy: userName, - data, - }); - res.status(201).json(data); + const tagType = await this.tagTypeService.createTagType( + req.body, + userName, + ); + res.status(201).json(tagType); } catch (error) { handleErrors(res, this.logger, error); } @@ -82,16 +53,10 @@ class TagTypeController extends Controller { const { name } = req.params; const userName = extractUsername(req); try { - const data = await tagTypeSchema.validateAsync({ - description, - icon, - name, - }); - await this.eventStore.store({ - type: UPDATE_TAG_TYPE, - createdBy: userName, - data, - }); + await this.tagTypeService.updateTagType( + { name, description, icon }, + userName, + ); res.status(200).end(); } catch (error) { handleErrors(res, this.logger, error); @@ -101,7 +66,7 @@ class TagTypeController extends Controller { async getTagType(req, res) { const { name } = req.params; try { - const tagType = await this.tagTypeStore.getTagType(name); + const tagType = await this.tagTypeService.getTagType(name); res.json({ version, tagType }); } catch (error) { handleErrors(res, this.logger, error); @@ -112,11 +77,7 @@ class TagTypeController extends Controller { const { name } = req.params; const userName = extractUsername(req); try { - await this.eventStore.store({ - type: DELETE_TAG_TYPE, - createdBy: userName, - data: { name }, - }); + await this.tagTypeService.deleteTagType(name, userName); res.status(200).end(); } catch (error) { handleErrors(res, this.logger, error); diff --git a/lib/routes/admin-api/tag.js b/lib/routes/admin-api/tag.js index 37d01dce15..5ecd2fa163 100644 --- a/lib/routes/admin-api/tag.js +++ b/lib/routes/admin-api/tag.js @@ -2,8 +2,6 @@ const Controller = require('../controller'); -const { tagSchema } = require('./tag-schema'); -const { CREATE_TAG, DELETE_TAG } = require('../../command-type'); const { UPDATE_FEATURE } = require('../../permissions'); const { handleErrors } = require('./util'); const extractUsername = require('../../extract-user'); @@ -11,36 +9,32 @@ const extractUsername = require('../../extract-user'); const version = 1; class TagController extends Controller { - constructor(config) { + constructor(config, { tagService }) { super(config); - this.featureTagStore = config.stores.featureTagStore; - this.eventStore = config.stores.eventStore; + this.tagService = tagService; this.logger = config.getLogger('/admin-api/tag.js'); this.get('/', this.getTags); this.post('/', this.createTag, UPDATE_FEATURE); this.get('/:type', this.getTagsByType); - this.get('/:type/:value', this.getTagByTypeAndValue); + this.get('/:type/:value', this.getTag); this.delete('/:type/:value', this.deleteTag, UPDATE_FEATURE); } async getTags(req, res) { - const tags = await this.featureTagStore.getTags(); + const tags = await this.tagService.getTags(); res.json({ version, tags }); } async getTagsByType(req, res) { - const tags = await this.featureTagStore.getAllOfType(req.params.type); + const tags = await this.tagService.getTagsByType(req.params.type); res.json({ version, tags }); } - async getTagByTypeAndValue(req, res) { + async getTag(req, res) { const { type, value } = req.params; try { - const tag = await this.featureTagStore.getTagByTypeAndValue( - type, - value, - ); + const tag = await this.tagService.getTag({ type, value }); res.json({ version, tag }); } catch (err) { handleErrors(res, this.logger, err); @@ -50,12 +44,7 @@ class TagController extends Controller { async createTag(req, res) { const userName = extractUsername(req); try { - const data = await tagSchema.validateAsync(req.body); - await this.eventStore.store({ - type: CREATE_TAG, - createdBy: userName, - data, - }); + await this.tagService.createTag(req.body, userName); res.status(201).end(); } catch (error) { handleErrors(res, this.logger, error); @@ -65,16 +54,8 @@ class TagController extends Controller { async deleteTag(req, res) { const { type, value } = req.params; const userName = extractUsername(req); - try { - await this.eventStore.store({ - type: DELETE_TAG, - createdBy: userName || 'unleash-system', - data: { - type, - value, - }, - }); + await this.tagService.deleteTag({ type, value }, userName); res.status(200).end(); } catch (error) { handleErrors(res, this.logger, error); diff --git a/lib/routes/admin-api/tag.test.js b/lib/routes/admin-api/tag.test.js index 75666e144b..2c08740191 100644 --- a/lib/routes/admin-api/tag.test.js +++ b/lib/routes/admin-api/tag.test.js @@ -7,6 +7,8 @@ const store = require('../../../test/fixtures/store'); const permissions = require('../../../test/fixtures/permissions'); const getLogger = require('../../../test/fixtures/no-logger'); const getApp = require('../../app'); +const { createServices } = require('../../services'); +const { UPDATE_FEATURE } = require('../../permissions'); const eventBus = new EventEmitter(); @@ -14,19 +16,21 @@ function getSetup() { const base = `/random${Math.round(Math.random() * 1000)}`; const stores = store.createStores(); const perms = permissions(); - const app = getApp({ + const config = { baseUriPath: base, stores, eventBus, extendedPermissions: true, preRouterHook: perms.hook, getLogger, - }); + }; + const services = createServices(stores, config); + const app = getApp(config, services); return { base, perms, - featureTagStore: stores.featureTagStore, + tagStore: stores.tagStore, request: supertest(app), }; } @@ -45,8 +49,8 @@ test('should get empty getTags via admin', t => { test('should get all tags added', t => { t.plan(1); - const { request, featureTagStore, base } = getSetup(); - featureTagStore.addTag({ + const { request, tagStore, base } = getSetup(); + tagStore.createTag({ type: 'simple', value: 'TeamGreen', }); @@ -62,12 +66,8 @@ test('should get all tags added', t => { test('should be able to get single tag by type and value', t => { t.plan(1); - const { request, featureTagStore, base } = getSetup(); - featureTagStore.addTag({ - id: 1, - type: 'simple', - value: 'TeamRed', - }); + const { request, base, tagStore } = getSetup(); + tagStore.createTag({ value: 'TeamRed', type: 'simple' }); return request .get(`${base}/api/admin/tags/simple/TeamRed`) .expect('Content-Type', /json/) @@ -77,17 +77,6 @@ test('should be able to get single tag by type and value', t => { }); }); -test('trying to get non-existing tag should not be found', t => { - const { request, featureTagStore, base } = getSetup(); - featureTagStore.addTag({ - id: 1, - type: 'simple', - value: 'TeamRed', - }); - return request.get(`${base}/api/admin/tags/id/1125`).expect(res => { - t.is(res.status, 404); - }); -}); test('trying to get non-existing tag by name and type should not be found', t => { const { request, base } = getSetup(); return request.get(`${base}/api/admin/tags/simple/TeamRed`).expect(res => { @@ -95,21 +84,13 @@ test('trying to get non-existing tag by name and type should not be found', t => }); }); test('should be able to delete a tag', t => { - t.plan(1); - const { request, featureTagStore, base } = getSetup(); - featureTagStore.addTag({ - type: 'simple', - value: 'TeamGreen', - }); - - featureTagStore.removeTag({ type: 'simple', value: 'TeamGreen' }); + t.plan(0); + const { request, base, tagStore, perms } = getSetup(); + perms.withPermissions(UPDATE_FEATURE); + tagStore.createTag({ type: 'simple', value: 'TeamRed' }); return request - .get(`${base}/api/admin/tags`) - .expect('Content-Type', /json/) - .expect(200) - .expect(res => { - t.true(res.body.tags.length === 0); - }); + .delete(`${base}/api/admin/tags/simple/TeamGreen`) + .expect(200); }); test('should get empty tags of type', t => { @@ -125,17 +106,9 @@ test('should get empty tags of type', t => { }); test('should be able to filter by type', t => { - const { request, base, featureTagStore } = getSetup(); - featureTagStore.addTag({ - id: 1, - value: 'TeamRed', - type: 'simple', - }); - featureTagStore.addTag({ - id: 2, - value: 'TeamGreen', - type: 'slack', - }); + const { request, base, tagStore } = getSetup(); + tagStore.createTag({ type: 'simple', value: 'TeamRed' }); + tagStore.createTag({ type: 'slack', value: 'TeamGreen' }); return request .get(`${base}/api/admin/tags/simple`) .expect(200) diff --git a/lib/routes/client-api/feature.js b/lib/routes/client-api/feature.js index 1b777097d0..8663fab2a1 100644 --- a/lib/routes/client-api/feature.js +++ b/lib/routes/client-api/feature.js @@ -6,9 +6,9 @@ const { filter } = require('./util'); const version = 1; class FeatureController extends Controller { - constructor({ featureToggleStore }) { + constructor({ featureToggleService }) { super(); - this.toggleStore = featureToggleStore; + this.toggleService = featureToggleService; this.get('/', this.getAll); this.get('/:featureName', this.getFeatureToggle); @@ -17,7 +17,7 @@ class FeatureController extends Controller { async getAll(req, res) { const nameFilter = filter('name', req.query.namePrefix); - const allFeatureToggles = await this.toggleStore.getFeatures(); + const allFeatureToggles = await this.toggleService.getFeatures(); const features = nameFilter(allFeatureToggles); res.json({ version, features }); @@ -26,7 +26,7 @@ class FeatureController extends Controller { async getFeatureToggle(req, res) { try { const name = req.params.featureName; - const featureToggle = await this.toggleStore.getFeature(name); + const featureToggle = await this.toggleService.getFeature(name); res.json(featureToggle).end(); } catch (err) { res.status(404).json({ error: 'Could not find feature' }); diff --git a/lib/routes/client-api/feature.test.js b/lib/routes/client-api/feature.test.js index 8047e7e889..aab50bea56 100644 --- a/lib/routes/client-api/feature.test.js +++ b/lib/routes/client-api/feature.test.js @@ -6,18 +6,20 @@ const { EventEmitter } = require('events'); const store = require('../../../test/fixtures/store'); const getLogger = require('../../../test/fixtures/no-logger'); const getApp = require('../../app'); +const { createServices } = require('../../services'); 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, getLogger, - }); + }; + const app = getApp(config, createServices(stores, config)); return { base, @@ -41,7 +43,7 @@ test('should get empty getFeatures via client', t => { test('fetch single feature', t => { t.plan(1); const { request, featureToggleStore, base } = getSetup(); - featureToggleStore.addFeature({ + featureToggleStore.createFeature({ name: 'test_', strategies: [{ name: 'default' }], }); @@ -58,10 +60,10 @@ test('fetch single feature', t => { test('support name prefix', t => { t.plan(2); const { request, featureToggleStore, base } = getSetup(); - featureToggleStore.addFeature({ name: 'a_test1' }); - featureToggleStore.addFeature({ name: 'a_test2' }); - featureToggleStore.addFeature({ name: 'b_test1' }); - featureToggleStore.addFeature({ name: 'b_test2' }); + featureToggleStore.createFeature({ name: 'a_test1' }); + featureToggleStore.createFeature({ name: 'a_test2' }); + featureToggleStore.createFeature({ name: 'b_test1' }); + featureToggleStore.createFeature({ name: 'b_test2' }); const namePrefix = 'b_'; diff --git a/lib/routes/client-api/index.js b/lib/routes/client-api/index.js index 72b22b9eac..388c876251 100644 --- a/lib/routes/client-api/index.js +++ b/lib/routes/client-api/index.js @@ -7,16 +7,21 @@ const RegisterController = require('./register.js'); const apiDef = require('./api-def.json'); class ClientApi extends Controller { - constructor(config) { + constructor(config, services = {}) { super(); - const { stores } = config; const { getLogger } = config; this.get('/', this.index); - this.use('/features', new FeatureController(stores, getLogger).router); - this.use('/metrics', new MetricsController(stores, getLogger).router); - this.use('/register', new RegisterController(stores, getLogger).router); + this.use( + '/features', + new FeatureController(services, getLogger).router, + ); + this.use('/metrics', new MetricsController(services, getLogger).router); + this.use( + '/register', + new RegisterController(services, getLogger).router, + ); } index(req, res) { diff --git a/lib/routes/client-api/metrics.js b/lib/routes/client-api/metrics.js index 529756a1fd..445bc96a80 100644 --- a/lib/routes/client-api/metrics.js +++ b/lib/routes/client-api/metrics.js @@ -1,18 +1,12 @@ 'use strict'; const Controller = require('../controller'); -const { clientMetricsSchema } = require('./metrics-schema'); class ClientMetricsController extends Controller { - constructor( - { clientMetricsStore, clientInstanceStore, featureToggleStore }, - getLogger, - ) { + constructor({ clientMetricsService }, getLogger) { super(); this.logger = getLogger('/api/client/metrics'); - this.clientMetricsStore = clientMetricsStore; - this.clientInstanceStore = clientInstanceStore; - this.featureToggleStore = featureToggleStore; + this.metrics = clientMetricsService; this.post('/', this.registerMetrics); } @@ -21,26 +15,17 @@ class ClientMetricsController extends Controller { const data = req.body; const clientIp = req.ip; - const { error, value } = clientMetricsSchema.validate(data); - - if (error) { - this.logger.warn('Invalid metrics posted', error); - return res.status(400).json(error); - } - try { - const toggleNames = Object.keys(value.bucket.toggles); - await this.featureToggleStore.lastSeenToggles(toggleNames); - await this.clientMetricsStore.insert(value); - await this.clientInstanceStore.insert({ - appName: value.appName, - instanceId: value.instanceId, - clientIp, - }); + await this.metrics.registerClientMetrics(data, clientIp); return res.status(202).end(); } catch (e) { - this.logger.error('failed to store metrics', e); - return res.status(500).end(); + this.logger.error('Failed to store metrics', e); + switch (e.name) { + case 'ValidationError': + return res.status(400).end(); + default: + return res.status(500).end(); + } } } } diff --git a/lib/routes/client-api/metrics.test.js b/lib/routes/client-api/metrics.test.js index abc28d7b8a..3f1e6814c0 100644 --- a/lib/routes/client-api/metrics.test.js +++ b/lib/routes/client-api/metrics.test.js @@ -6,7 +6,9 @@ const { EventEmitter } = require('events'); const store = require('../../../test/fixtures/store'); const getLogger = require('../../../test/fixtures/no-logger'); const getApp = require('../../app'); -const { clientMetricsSchema } = require('./metrics-schema'); +const { + clientMetricsSchema, +} = require('../../services/client-metrics/client-metrics-schema'); const { createServices } = require('../../services'); const eventBus = new EventEmitter(); @@ -159,7 +161,7 @@ test('shema allow yes=', t => { test('should set lastSeen on toggle', async t => { t.plan(1); const { request, stores } = getSetup(); - stores.featureToggleStore.addFeature({ name: 'toggleLastSeen' }); + stores.featureToggleStore.createFeature({ name: 'toggleLastSeen' }); await request .post('/api/client/metrics') .send({ diff --git a/lib/routes/client-api/register.js b/lib/routes/client-api/register.js index 9bc10ad3b3..681ee5d17f 100644 --- a/lib/routes/client-api/register.js +++ b/lib/routes/client-api/register.js @@ -1,40 +1,29 @@ 'use strict'; const Controller = require('../controller'); -const { clientRegisterSchema: schema } = require('./register-schema'); class RegisterController extends Controller { - constructor({ clientInstanceStore, clientApplicationsStore }, getLogger) { + constructor({ clientMetricsService }, getLogger) { super(); this.logger = getLogger('/api/client/register'); - this.clientInstanceStore = clientInstanceStore; - this.clientApplicationsStore = clientApplicationsStore; - + this.metrics = clientMetricsService; this.post('/', this.handleRegister); } async handleRegister(req, res) { const data = req.body; - const { value, error } = schema.validate(data); - - if (error) { - this.logger.warn('Invalid client data posted', error); - return res.status(400).json(error); - } - - value.clientIp = req.ip; - try { - await this.clientApplicationsStore.upsert(value); - await this.clientInstanceStore.insert(value); - const { appName, instanceId } = value; - this.logger.info( - `New client registration: appName=${appName}, instanceId=${instanceId}`, - ); + const clientIp = req.ip; + await this.metrics.registerClient(data, clientIp); return res.status(202).end(); } catch (err) { this.logger.error('failed to register client', err); - return res.status(500).end(); + switch (err.name) { + case 'ValidationError': + return res.status(400).end(); + default: + return res.status(500).end(); + } } } } diff --git a/lib/routes/client-api/register.test.js b/lib/routes/client-api/register.test.js index 0cfc01de4a..230b4fd403 100644 --- a/lib/routes/client-api/register.test.js +++ b/lib/routes/client-api/register.test.js @@ -6,17 +6,20 @@ const { EventEmitter } = require('events'); const store = require('../../../test/fixtures/store'); const getLogger = require('../../../test/fixtures/no-logger'); const getApp = require('../../app'); +const { createServices } = require('../../services'); const eventBus = new EventEmitter(); function getSetup() { const stores = store.createStores(); - const app = getApp({ + const config = { baseUriPath: '', stores, eventBus, getLogger, - }); + }; + const services = createServices(stores, config); + const app = getApp(config, services); return { request: supertest(app), diff --git a/lib/routes/index.js b/lib/routes/index.js index 4a94abf10d..bbda04efde 100644 --- a/lib/routes/index.js +++ b/lib/routes/index.js @@ -22,7 +22,10 @@ class IndexRouter extends Controller { // legacy support (remove in 4.x) if (config.enableLegacyRoutes) { - const featureController = new FeatureController(config.stores); + const featureController = new FeatureController( + services, + config.getLogger, + ); this.use('/api/features', featureController.router); } } diff --git a/lib/routes/index.test.js b/lib/routes/index.test.js index 5262c4b6a5..dbdb73bab0 100644 --- a/lib/routes/index.test.js +++ b/lib/routes/index.test.js @@ -6,23 +6,25 @@ const { EventEmitter } = require('events'); const store = require('../../test/fixtures/store'); const getLogger = require('../../test/fixtures/no-logger'); const getApp = require('../app'); +const { createServices } = require('../services'); 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, enableLegacyRoutes: true, getLogger, - }); + }; + const services = createServices(stores, config); + const app = getApp(config, services); return { base, - featureToggleStore: stores.featureToggleStore, request: supertest(app), }; } diff --git a/lib/routes/client-api/metrics-schema.js b/lib/services/client-metrics/client-metrics-schema.js similarity index 100% rename from lib/routes/client-api/metrics-schema.js rename to lib/services/client-metrics/client-metrics-schema.js diff --git a/lib/services/client-metrics/client-metrics.test.js b/lib/services/client-metrics/client-metrics.test.js index f43f84f953..99051d2c25 100644 --- a/lib/services/client-metrics/client-metrics.test.js +++ b/lib/services/client-metrics/client-metrics.test.js @@ -10,9 +10,14 @@ const UnleashClientMetrics = require('./index'); const appName = 'appName'; const instanceId = 'instanceId'; +const getLogger = require('../../../test/fixtures/no-logger'); + test('should work without state', t => { const clientMetricsStore = new EventEmitter(); - const metrics = new UnleashClientMetrics({ clientMetricsStore }); + const metrics = new UnleashClientMetrics( + { clientMetricsStore }, + { getLogger }, + ); t.truthy(metrics.getAppsWithToggles()); t.truthy(metrics.getTogglesMetrics()); @@ -24,7 +29,10 @@ test.cb('data should expire', t => { const clock = lolex.install(); const clientMetricsStore = new EventEmitter(); - const metrics = new UnleashClientMetrics({ clientMetricsStore }); + const metrics = new UnleashClientMetrics( + { clientMetricsStore }, + { getLogger }, + ); metrics.addPayload({ appName, @@ -65,7 +73,10 @@ test.cb('data should expire', t => { test('should listen to metrics from store', t => { const clientMetricsStore = new EventEmitter(); - const metrics = new UnleashClientMetrics({ clientMetricsStore }); + const metrics = new UnleashClientMetrics( + { clientMetricsStore }, + { getLogger }, + ); clientMetricsStore.emit('metrics', { appName, instanceId, @@ -123,7 +134,10 @@ test('should listen to metrics from store', t => { test('should build up list of seend toggles when new metrics arrives', t => { const clientMetricsStore = new EventEmitter(); - const metrics = new UnleashClientMetrics({ clientMetricsStore }); + const metrics = new UnleashClientMetrics( + { clientMetricsStore }, + { getLogger }, + ); clientMetricsStore.emit('metrics', { appName, instanceId, @@ -159,7 +173,10 @@ test('should build up list of seend toggles when new metrics arrives', t => { test('should handle a lot of toggles', t => { const clientMetricsStore = new EventEmitter(); - const metrics = new UnleashClientMetrics({ clientMetricsStore }); + const metrics = new UnleashClientMetrics( + { clientMetricsStore }, + { getLogger }, + ); const toggleCounts = {}; for (let i = 0; i < 100; i++) { @@ -186,7 +203,10 @@ test('should have correct values for lastMinute', t => { const clock = lolex.install(); const clientMetricsStore = new EventEmitter(); - const metrics = new UnleashClientMetrics({ clientMetricsStore }); + const metrics = new UnleashClientMetrics( + { clientMetricsStore }, + { getLogger }, + ); const now = new Date(); const input = [ @@ -258,7 +278,10 @@ test('should have correct values for lastHour', t => { const clock = lolex.install(); const clientMetricsStore = new EventEmitter(); - const metrics = new UnleashClientMetrics({ clientMetricsStore }); + const metrics = new UnleashClientMetrics( + { clientMetricsStore }, + { getLogger }, + ); const now = new Date(); const input = [ @@ -338,7 +361,10 @@ test('should have correct values for lastHour', t => { test('should not fail when toggle metrics is missing yes/no field', t => { const clientMetricsStore = new EventEmitter(); - const metrics = new UnleashClientMetrics({ clientMetricsStore }); + const metrics = new UnleashClientMetrics( + { clientMetricsStore }, + { getLogger }, + ); clientMetricsStore.emit('metrics', { appName, instanceId, diff --git a/lib/services/client-metrics/index.js b/lib/services/client-metrics/index.js index 0f72b9a378..6ad66b03a2 100644 --- a/lib/services/client-metrics/index.js +++ b/lib/services/client-metrics/index.js @@ -4,18 +4,39 @@ const Projection = require('./projection.js'); const TTLList = require('./ttl-list.js'); +const appSchema = require('./metrics-schema'); +const NotFoundError = require('../../error/notfound-error'); +const { clientMetricsSchema } = require('./client-metrics-schema'); +const { clientRegisterSchema } = require('./register-schema'); +const { APPLICATION_CREATED } = require('../../event-type'); module.exports = class ClientMetricsService { - constructor({ clientMetricsStore }) { + constructor( + { + clientMetricsStore, + strategyStore, + featureToggleStore, + clientApplicationsStore, + clientInstanceStore, + eventStore, + }, + { getLogger }, + ) { this.globalCount = 0; this.apps = {}; - + this.strategyStore = strategyStore; + this.toggleStore = featureToggleStore; + this.clientAppStore = clientApplicationsStore; + this.clientInstanceStore = clientInstanceStore; + this.clientMetricsStore = clientMetricsStore; this.lastHourProjection = new Projection(); this.lastMinuteProjection = new Projection(); + this.eventStore = eventStore; this.lastHourList = new TTLList({ interval: 10000, }); + this.logger = getLogger('services/client-metrics/index.js'); this.lastMinuteList = new TTLList({ interval: 10000, @@ -42,6 +63,44 @@ module.exports = class ClientMetricsService { clientMetricsStore.on('metrics', m => this.addPayload(m)); } + async registerClientMetrics(data, clientIp) { + const value = await clientMetricsSchema.validateAsync(data); + const toggleNames = Object.keys(value.bucket.toggles); + await this.toggleStore.lastSeenToggles(toggleNames); + await this.clientMetricsStore.insert(value); + await this.clientInstanceStore.insert({ + appName: value.appName, + instanceId: value.instanceId, + clientIp, + }); + } + + async upsertApp(value, clientIp) { + try { + const app = await this.clientAppStore.getApplication(value.appName); + await this.updateRow(value, app); + } catch (error) { + if (error instanceof NotFoundError) { + await this.clientAppStore.insertNewRow(value); + await this.eventStore.store({ + type: APPLICATION_CREATED, + createdBy: clientIp, + data: value, + }); + } + } + } + + async registerClient(data, clientIp) { + const value = await clientRegisterSchema.validateAsync(data); + value.clientIp = clientIp; + await this.upsertApp(value, clientIp); + await this.clientInstanceStore.insert(value); + this.logger.info( + `New client registration: appName=${value.appName}, instanceId=${value.instanceId}`, + ); + } + getAppsWithToggles() { const apps = []; Object.keys(this.apps).forEach(appName => { @@ -58,6 +117,66 @@ module.exports = class ClientMetricsService { : []; } + async getSeenApps() { + const seenApps = this.getSeenAppsPerToggle(); + const applications = await this.clientAppStore.getApplications(); + const metaData = applications.reduce((result, entry) => { + // eslint-disable-next-line no-param-reassign + result[entry.appName] = entry; + return result; + }, {}); + + Object.keys(seenApps).forEach(key => { + seenApps[key] = seenApps[key].map(entry => { + if (metaData[entry.appName]) { + return { ...entry, ...metaData[entry.appName] }; + } + return entry; + }); + }); + return seenApps; + } + + async getApplications(query) { + return this.clientAppStore.getApplications(query); + } + + async getApplication(appName) { + const seenToggles = this.getSeenTogglesByAppName(appName); + const [ + application, + instances, + strategies, + features, + ] = await Promise.all([ + this.clientAppStore.getApplication(appName), + this.clientInstanceStore.getByAppName(appName), + this.strategyStore.getStrategies(), + this.toggleStore.getFeatures(), + ]); + + return { + appName: application.appName, + createdAt: application.createdAt, + description: application.description, + url: application.url, + color: application.color, + icon: application.icon, + strategies: application.strategies.map(name => { + const found = strategies.find(f => f.name === name); + return found || { name, notFound: true }; + }), + instances, + seenToggles: seenToggles.map(name => { + const found = features.find(f => f.name === name); + return found || { name, notFound: true }; + }), + links: { + self: `/api/applications/${application.appName}`, + }, + }; + } + getSeenAppsPerToggle() { const toggles = {}; Object.keys(this.apps).forEach(appName => { @@ -139,6 +258,16 @@ module.exports = class ClientMetricsService { }); } + async deleteApplication(appName) { + await this.clientInstanceStore.deleteForApplication(appName); + await this.clientAppStore.deleteApplication(appName); + } + + async createApplication(input) { + const applicationData = await appSchema.validateAsync(input); + await this.clientAppStore.upsert(applicationData); + } + destroy() { this.lastHourList.destroy(); this.lastMinuteList.destroy(); diff --git a/lib/routes/admin-api/metrics-schema.js b/lib/services/client-metrics/metrics-schema.js similarity index 100% rename from lib/routes/admin-api/metrics-schema.js rename to lib/services/client-metrics/metrics-schema.js diff --git a/lib/routes/client-api/register-schema.js b/lib/services/client-metrics/register-schema.js similarity index 100% rename from lib/routes/client-api/register-schema.js rename to lib/services/client-metrics/register-schema.js diff --git a/lib/routes/admin-api/feature-schema.js b/lib/services/feature-schema.js similarity index 83% rename from lib/routes/admin-api/feature-schema.js rename to lib/services/feature-schema.js index 17460eceda..78b8376e8d 100644 --- a/lib/routes/admin-api/feature-schema.js +++ b/lib/services/feature-schema.js @@ -1,9 +1,12 @@ 'use strict'; const joi = require('joi'); -const { nameType } = require('./util'); +const { nameType } = require('../routes/admin-api/util'); -const nameSchema = joi.object().keys({ name: nameType }); +const nameSchema = joi + .object() + .keys({ name: nameType }) + .options({ stripUnknown: true, allowUnknown: false, abortEarly: false }); const constraintSchema = joi.object().keys({ contextName: joi.string(), @@ -58,7 +61,7 @@ const variantsSchema = joi.object().keys({ ), }); -const featureShema = joi +const featureSchema = joi .object() .keys({ name: nameType, @@ -83,6 +86,6 @@ const featureShema = joi .optional() .items(variantsSchema), }) - .options({ allowUnknown: false, stripUnknown: true }); + .options({ allowUnknown: false, stripUnknown: true, abortEarly: false }); -module.exports = { featureShema, strategiesSchema, nameSchema }; +module.exports = { featureSchema, strategiesSchema, nameSchema }; diff --git a/lib/routes/admin-api/feature-schema.test.js b/lib/services/feature-schema.test.js similarity index 89% rename from lib/routes/admin-api/feature-schema.test.js rename to lib/services/feature-schema.test.js index eaea0f8a7d..d3f1c70bfd 100644 --- a/lib/routes/admin-api/feature-schema.test.js +++ b/lib/services/feature-schema.test.js @@ -1,7 +1,7 @@ 'use strict'; const test = require('ava'); -const { featureShema } = require('./feature-schema'); +const { featureSchema } = require('./feature-schema'); test('should require URL firendly name', t => { const toggle = { @@ -10,7 +10,7 @@ test('should require URL firendly name', t => { strategies: [{ name: 'default' }], }; - const { error } = featureShema.validate(toggle); + const { error } = featureSchema.validate(toggle); t.deepEqual(error.details[0].message, '"name" must be URL friendly'); }); @@ -21,7 +21,7 @@ test('should be valid toggle name', t => { strategies: [{ name: 'default' }], }; - const { value } = featureShema.validate(toggle); + const { value } = featureSchema.validate(toggle); t.is(value.name, toggle.name); }); @@ -41,7 +41,7 @@ test('should strip extra variant fields', t => { ], }; - const { value } = featureShema.validate(toggle); + const { value } = featureSchema.validate(toggle); t.notDeepEqual(value, toggle); t.falsy(value.variants[0].unkown); }); @@ -63,7 +63,7 @@ test('should allow weightType=fix', t => { ], }; - const { value } = featureShema.validate(toggle); + const { value } = featureSchema.validate(toggle); t.deepEqual(value, toggle); }); @@ -83,7 +83,7 @@ test('should disallow weightType=unknown', t => { ], }; - const { error } = featureShema.validate(toggle); + const { error } = featureSchema.validate(toggle); t.deepEqual( error.details[0].message, '"variants[0].weightType" must be one of [variable, fix]', @@ -113,7 +113,7 @@ test('should be possible to define variant overrides', t => { ], }; - const { value, error } = featureShema.validate(toggle); + const { value, error } = featureSchema.validate(toggle); t.deepEqual(value, toggle); t.falsy(error); }); @@ -139,7 +139,7 @@ test('variant overrides must have corect shape', async t => { }; try { - await featureShema.validateAsync(toggle); + await featureSchema.validateAsync(toggle); } catch (error) { t.is( error.details[0].message, @@ -169,7 +169,7 @@ test('should keep constraints', t => { ], }; - const { value, error } = featureShema.validate(toggle); + const { value, error } = featureSchema.validate(toggle); t.deepEqual(value, toggle); t.falsy(error); }); @@ -194,7 +194,7 @@ test('should not accept empty constraint values', t => { ], }; - const { error } = featureShema.validate(toggle); + const { error } = featureSchema.validate(toggle); t.deepEqual( error.details[0].message, '"strategies[0].constraints[0].values[0]" is not allowed to be empty', @@ -221,7 +221,7 @@ test('should not accept empty list of constraint values', t => { ], }; - const { error } = featureShema.validate(toggle); + const { error } = featureSchema.validate(toggle); t.deepEqual( error.details[0].message, '"strategies[0].constraints[0].values" must contain at least 1 items', diff --git a/lib/services/feature-toggle-service.js b/lib/services/feature-toggle-service.js new file mode 100644 index 0000000000..51f3c5c79b --- /dev/null +++ b/lib/services/feature-toggle-service.js @@ -0,0 +1,172 @@ +const { FEATURE_TAGGED, FEATURE_UNTAGGED } = require('../event-type'); +const { featureSchema, nameSchema } = require('./feature-schema'); +const { tagSchema } = require('./tag-schema'); +const NameExistsError = require('../error/name-exists-error'); +const NotFoundError = require('../error/notfound-error'); +const { + FEATURE_ARCHIVED, + FEATURE_CREATED, + FEATURE_REVIVED, + FEATURE_UPDATED, + TAG_CREATED, +} = require('../event-type'); + +class FeatureToggleService { + constructor( + { featureToggleStore, featureTagStore, tagStore, eventStore }, + { getLogger }, + ) { + this.featureToggleStore = featureToggleStore; + this.tagStore = tagStore; + this.eventStore = eventStore; + this.featureTagStore = featureTagStore; + this.logger = getLogger('services/feature-toggle-service.js'); + } + + async getFeatures() { + return this.featureToggleStore.getFeatures(); + } + + async getArchivedFeatures() { + return this.featureToggleStore.getArchivedFeatures(); + } + + async addArchivedFeature(feature) { + await this.featureToggleStore.addArchivedFeature(feature); + } + + async getFeature(name) { + return this.featureToggleStore.getFeature(name); + } + + async createFeatureToggle(value, userName) { + await this.validateName(value); + const feature = await featureSchema.validateAsync(value); + await this.featureToggleStore.createFeature(feature); + await this.eventStore.store({ + type: FEATURE_CREATED, + createdBy: userName, + data: feature, + }); + } + + async updateToggle(updatedFeature, userName) { + await this.featureToggleStore.getFeature(updatedFeature.name); + const value = await featureSchema.validateAsync(updatedFeature); + await this.featureToggleStore.updateFeature(value); + await this.eventStore.store({ + type: FEATURE_UPDATED, + createdBy: userName, + data: updatedFeature, + }); + } + + async archiveToggle(name, userName) { + await this.featureToggleStore.getFeature(name); + await this.featureToggleStore.archiveFeature(name); + await this.eventStore.store({ + type: FEATURE_ARCHIVED, + createdBy: userName, + data: { name }, + }); + } + + async reviveToggle(name, userName) { + await this.featureToggleStore.reviveFeature({ name }); + await this.eventStore.store({ + type: FEATURE_REVIVED, + createdBy: userName, + data: { name }, + }); + } + + async toggle(featureName, userName) { + const feature = await this.featureToggleStore.getFeature(featureName); + const toggle = !feature.enabled; + return this.updateField(feature.name, 'enabled', toggle, userName); + } + + /** Tag releated */ + async listTags(featureName) { + return this.featureTagStore.getAllTagsForFeature(featureName); + } + + async addTag(featureName, tag, userName) { + await nameSchema.validateAsync({ name: featureName }); + const validatedTag = await tagSchema.validateAsync(tag); + await this.createTagIfNeeded(validatedTag, userName); + await this.featureTagStore.tagFeature(featureName, validatedTag); + await this.eventStore.store({ + type: FEATURE_TAGGED, + createdBy: userName, + data: { + featureName, + tag: validatedTag, + }, + }); + return validatedTag; + } + + async createTagIfNeeded(tag, userName) { + try { + await this.tagStore.getTag(tag.type, tag.value); + } catch (error) { + if (error instanceof NotFoundError) { + await this.tagStore.createTag(tag); + await this.eventStore.store({ + type: TAG_CREATED, + createdBy: userName, + data: { + tag, + }, + }); + } + } + } + + async removeTag(featureName, tag, userName) { + await this.featureTagStore.untagFeature(featureName, tag); + await this.eventStore.store({ + type: FEATURE_UNTAGGED, + createdBy: userName, + data: { + featureName, + tag, + }, + }); + } + + /** Validations */ + async validateName({ name }) { + await nameSchema.validateAsync({ name }); + await this.validateUniqueFeatureName(name); + return name; + } + + async validateUniqueFeatureName(name) { + let msg; + try { + const feature = await this.featureToggleStore.hasFeature(name); + msg = feature.archived + ? 'An archived toggle with that name already exists' + : 'A toggle with that name already exists'; + } catch (error) { + return; + } + throw new NameExistsError(msg); + } + + async updateField(featureName, field, value, userName) { + const feature = await this.featureToggleStore.getFeature(featureName); + feature[field] = value; + await this.featureToggleStore.updateFeature(feature); + await this.eventStore.store({ + type: FEATURE_UPDATED, + createdBy: userName, + data: feature, + }); + return feature; + } +} + +module.exports = FeatureToggleService; diff --git a/lib/services/index.js b/lib/services/index.js index a99c025dab..a37c96c5d7 100644 --- a/lib/services/index.js +++ b/lib/services/index.js @@ -1,9 +1,17 @@ +const FeatureToggleService = require('./feature-toggle-service'); const ProjectService = require('./project-service'); const StateService = require('./state-service'); const ClientMetricsService = require('./client-metrics'); +const TagTypeService = require('./tag-type-service'); +const TagService = require('./tag-service'); +const StrategyService = require('./strategy-service'); module.exports.createServices = (stores, config) => ({ + featureToggleService: new FeatureToggleService(stores, config), projectService: new ProjectService(stores, config), stateService: new StateService(stores, config), + strategyService: new StrategyService(stores, config), + tagTypeService: new TagTypeService(stores, config), + tagService: new TagService(stores, config), clientMetricsService: new ClientMetricsService(stores, config), }); diff --git a/lib/services/state-schema.js b/lib/services/state-schema.js index 2025988be8..02d3180d0f 100644 --- a/lib/services/state-schema.js +++ b/lib/services/state-schema.js @@ -1,6 +1,6 @@ const joi = require('joi'); -const { featureShema } = require('../routes/admin-api/feature-schema'); -const strategySchema = require('../routes/admin-api/strategy-schema'); +const { featureSchema } = require('./feature-schema'); +const strategySchema = require('./strategy-schema'); // TODO: Extract to seperate file const stateSchema = joi.object().keys({ @@ -8,7 +8,7 @@ const stateSchema = joi.object().keys({ features: joi .array() .optional() - .items(featureShema), + .items(featureSchema), strategies: joi .array() .optional() diff --git a/lib/services/state-service.js b/lib/services/state-service.js index 1b718b0c7c..df26e0c3fe 100644 --- a/lib/services/state-service.js +++ b/lib/services/state-service.js @@ -64,6 +64,7 @@ class StateService { if (dropBeforeImport) { this.logger.info(`Dropping existing feature toggles`); + await this.toggleStore.dropFeatures(); await this.eventStore.store({ type: DROP_FEATURES, createdBy: userName, @@ -76,11 +77,13 @@ class StateService { .filter(filterExisitng(keepExisting, oldToggles)) .filter(filterEqual(oldToggles)) .map(feature => - this.eventStore.store({ - type: FEATURE_IMPORT, - createdBy: userName, - data: feature, - }), + this.toggleStore.importFeature(feature).then(() => + this.eventStore.store({ + type: FEATURE_IMPORT, + createdBy: userName, + data: feature, + }), + ), ), ); } @@ -98,6 +101,7 @@ class StateService { if (dropBeforeImport) { this.logger.info(`Dropping existing strategies`); + await this.strategyStore.dropStrategies(); await this.eventStore.store({ type: DROP_STRATEGIES, createdBy: userName, @@ -110,10 +114,12 @@ class StateService { .filter(filterExisitng(keepExisting, oldStrategies)) .filter(filterEqual(oldStrategies)) .map(strategy => - this.eventStore.store({ - type: STRATEGY_IMPORT, - createdBy: userName, - data: strategy, + this.strategyStore.importStrategy(strategy).then(() => { + this.eventStore.store({ + type: STRATEGY_IMPORT, + createdBy: userName, + data: strategy, + }); }), ), ); diff --git a/lib/services/state-service.test.js b/lib/services/state-service.test.js index a05d0dce81..aafee2d8a2 100644 --- a/lib/services/state-service.test.js +++ b/lib/services/state-service.test.js @@ -55,7 +55,7 @@ test('should not import an existing feature', async t => { ], }; - await stores.featureToggleStore.addFeature(data.features[0]); + await stores.featureToggleStore.createFeature(data.features[0]); await stateService.import({ data, keepExisting: true }); @@ -76,7 +76,7 @@ test('should not keep existing feature if drop-before-import', async t => { ], }; - await stores.featureToggleStore.addFeature(data.features[0]); + await stores.featureToggleStore.createFeature(data.features[0]); await stateService.import({ data, @@ -144,7 +144,7 @@ test('should not import an exiting strategy', async t => { ], }; - await stores.strategyStore.addStrategy(data.strategies[0]); + await stores.strategyStore.createStrategy(data.strategies[0]); await stateService.import({ data, keepExisting: true }); @@ -201,7 +201,7 @@ test('should not accept gibberish', async t => { test('should export featureToggles', async t => { const { stateService, stores } = getSetup(); - stores.featureToggleStore.addFeature({ name: 'a-feature' }); + stores.featureToggleStore.createFeature({ name: 'a-feature' }); const data = await stateService.export({ includeFeatureToggles: true }); @@ -212,7 +212,7 @@ test('should export featureToggles', async t => { test('should export strategies', async t => { const { stateService, stores } = getSetup(); - stores.strategyStore.addStrategy({ name: 'a-strategy', editable: true }); + stores.strategyStore.createStrategy({ name: 'a-strategy', editable: true }); const data = await stateService.export({ includeStrategies: true }); diff --git a/lib/routes/admin-api/strategy-schema.js b/lib/services/strategy-schema.js similarity index 92% rename from lib/routes/admin-api/strategy-schema.js rename to lib/services/strategy-schema.js index a1ed614c56..fd15dfd3b9 100644 --- a/lib/routes/admin-api/strategy-schema.js +++ b/lib/services/strategy-schema.js @@ -1,7 +1,7 @@ 'use strict'; const joi = require('joi'); -const { nameType } = require('./util'); +const { nameType } = require('../routes/admin-api/util'); const strategySchema = joi.object().keys({ name: nameType, diff --git a/lib/services/strategy-service.js b/lib/services/strategy-service.js new file mode 100644 index 0000000000..951cbdee49 --- /dev/null +++ b/lib/services/strategy-service.js @@ -0,0 +1,83 @@ +const strategySchema = require('./strategy-schema'); +const NameExistsError = require('../error/name-exists-error'); +const { + STRATEGY_CREATED, + STRATEGY_DELETED, + STRATEGY_UPDATED, +} = require('../event-type'); + +class StrategyService { + constructor({ strategyStore, eventStore }, { getLogger }) { + this.strategyStore = strategyStore; + this.eventStore = eventStore; + this.logger = getLogger('services/strategy-service.js'); + } + + async getStrategies() { + return this.strategyStore.getStrategies(); + } + + async getStrategy(name) { + return this.strategyStore.getStrategy(name); + } + + async removeStrategy(strategyName, userName) { + const strategy = await this.strategyStore.getStrategy(strategyName); + await this._validateEditable(strategy); + await this.strategyStore.deleteStrategy({ name: strategyName }); + await this.eventStore.store({ + type: STRATEGY_DELETED, + createdBy: userName, + data: { + name: strategyName, + }, + }); + } + + async createStrategy(value, userName) { + const strategy = await strategySchema.validateAsync(value); + await this._validateStrategyName(strategy); + await this.strategyStore.createStrategy(strategy); + await this.eventStore.store({ + type: STRATEGY_CREATED, + createdBy: userName, + data: strategy, + }); + } + + async updateStrategy(input, userName) { + const value = await strategySchema.validateAsync(input); + const strategy = await this.strategyStore.getStrategy(input.name); + await this._validateEditable(strategy); + await this.strategyStore.updateStrategy(value); + await this.eventStore.store({ + type: STRATEGY_UPDATED, + createdBy: userName, + data: value, + }); + } + + async _validateStrategyName(data) { + return new Promise((resolve, reject) => { + this.strategyStore + .getStrategy(data.name) + .then(() => + reject( + new NameExistsError( + `Strategy with name ${data.name} already exist!`, + ), + ), + ) + .catch(() => resolve(data)); + }); + } + + // This check belongs in the store. + _validateEditable(strategy) { + if (strategy.editable === false) { + throw new Error(`Cannot edit strategy ${strategy.name}`); + } + } +} + +module.exports = StrategyService; diff --git a/lib/routes/admin-api/tag-schema.js b/lib/services/tag-schema.js similarity index 88% rename from lib/routes/admin-api/tag-schema.js rename to lib/services/tag-schema.js index d7d8837cc1..91de3e9870 100644 --- a/lib/routes/admin-api/tag-schema.js +++ b/lib/services/tag-schema.js @@ -1,7 +1,7 @@ 'use strict'; const joi = require('joi'); -const { customJoi } = require('./util'); +const { customJoi } = require('../routes/admin-api/util'); const tagSchema = joi .object() diff --git a/lib/routes/admin-api/tag-schema.test.js b/lib/services/tag-schema.test.js similarity index 100% rename from lib/routes/admin-api/tag-schema.test.js rename to lib/services/tag-schema.test.js diff --git a/lib/services/tag-service.js b/lib/services/tag-service.js new file mode 100644 index 0000000000..1bd97f214c --- /dev/null +++ b/lib/services/tag-service.js @@ -0,0 +1,62 @@ +const { tagSchema } = require('./tag-schema'); +const NotFoundError = require('../error/notfound-error'); +const NameExistsError = require('../error/name-exists-error'); +const { TAG_CREATED, TAG_DELETED } = require('../event-type'); + +class TagService { + constructor({ tagStore, eventStore }, { getLogger }) { + this.tagStore = tagStore; + this.eventStore = eventStore; + this.logger = getLogger('services/tag-service.js'); + } + + async getTags() { + return this.tagStore.getAll(); + } + + async getTagsByType(type) { + return this.tagStore.getTagsByType(type); + } + + async getTag({ type, value }) { + return this.tagStore.getTag(type, value); + } + + async validateUnique(tag) { + try { + await this.tagStore.getTag(tag.type, tag.value); + } catch (err) { + if (err instanceof NotFoundError) { + return; + } + } + throw new NameExistsError(`A tag of ${tag} already exists`); + } + + async validate(tag) { + const data = await tagSchema.validateAsync(tag); + await this.validateUnique(tag); + return data; + } + + async createTag(tag, userName) { + const data = await this.validate(tag); + await this.tagStore.createTag(data); + await this.eventStore.store({ + type: TAG_CREATED, + createdBy: userName, + data, + }); + } + + async deleteTag(tag, userName) { + await this.tagStore.deleteTag(tag); + await this.eventStore.store({ + type: TAG_DELETED, + createdBy: userName, + data: tag, + }); + } +} + +module.exports = TagService; diff --git a/lib/routes/admin-api/tag-type-schema.js b/lib/services/tag-type-schema.js similarity index 88% rename from lib/routes/admin-api/tag-type-schema.js rename to lib/services/tag-type-schema.js index f82a49568f..46e0aa3577 100644 --- a/lib/routes/admin-api/tag-type-schema.js +++ b/lib/services/tag-type-schema.js @@ -1,7 +1,7 @@ 'use strict'; const joi = require('joi'); -const { customJoi } = require('./util'); +const { customJoi } = require('../routes/admin-api/util'); const tagTypeSchema = joi .object() diff --git a/lib/routes/admin-api/tag-type-schema.test.js b/lib/services/tag-type-schema.test.js similarity index 100% rename from lib/routes/admin-api/tag-type-schema.test.js rename to lib/services/tag-type-schema.test.js diff --git a/lib/services/tag-type-service.js b/lib/services/tag-type-service.js new file mode 100644 index 0000000000..dffb279ad4 --- /dev/null +++ b/lib/services/tag-type-service.js @@ -0,0 +1,76 @@ +const NameExistsError = require('../error/name-exists-error'); +const NotFoundError = require('../error/notfound-error'); +const { tagTypeSchema } = require('./tag-type-schema'); +const { + TAG_TYPE_CREATED, + TAG_TYPE_DELETED, + TAG_TYPE_UPDATED, +} = require('../event-type'); + +class TagTypeService { + constructor({ tagTypeStore, eventStore }, { getLogger }) { + this.tagTypeStore = tagTypeStore; + this.eventStore = eventStore; + this.logger = getLogger('services/tag-type-service.js'); + } + + async getAll() { + return this.tagTypeStore.getAll(); + } + + async getTagType(name) { + return this.tagTypeStore.getTagType(name); + } + + async createTagType(newTagType, userName) { + const data = await tagTypeSchema.validateAsync(newTagType); + await this.validateUnique(newTagType); + await this.tagTypeStore.createTagType(data); + await this.eventStore.store({ + type: TAG_TYPE_CREATED, + createdBy: userName || 'unleash-system', + data, + }); + return data; + } + + async validateUnique({ name }) { + try { + await this.tagTypeStore.getTagType(name); + } catch (err) { + if (err instanceof NotFoundError) { + return; + } + } + throw new NameExistsError( + `There already exists a tag-type with the name ${name}`, + ); + } + + async validate(tagType) { + await tagTypeSchema.validateAsync(tagType); + await this.validateUnique(tagType); + } + + async deleteTagType(name, userName) { + await this.tagTypeStore.deleteTagType(name); + await this.eventStore.store({ + type: TAG_TYPE_DELETED, + createdBy: userName || 'unleash-system', + data: { name }, + }); + } + + async updateTagType(updatedTagType, userName) { + const data = await tagTypeSchema.validateAsync(updatedTagType); + await this.tagTypeStore.updateTagType(data); + await this.eventStore.store({ + type: TAG_TYPE_UPDATED, + createdBy: userName || 'unleash-system', + data, + }); + return data; + } +} + +module.exports = TagTypeService; diff --git a/test/e2e/api/admin/feature.auth.e2e.test.js b/test/e2e/api/admin/feature.auth.e2e.test.js index 8e14c3be18..cc100355f9 100644 --- a/test/e2e/api/admin/feature.auth.e2e.test.js +++ b/test/e2e/api/admin/feature.auth.e2e.test.js @@ -25,14 +25,17 @@ test.serial('creates new feature toggle with createdBy', async t => { }); // create toggle - await request.post('/api/admin/features').send({ - name: 'com.test.Username', - enabled: false, - strategies: [{ name: 'default' }], - }); + await request + .post('/api/admin/features') + .send({ + name: 'com.test.Username', + enabled: false, + strategies: [{ name: 'default' }], + }) + .expect(201); await request.get('/api/admin/events').expect(res => { - t.true(res.body.events[0].createdBy === 'user@mail.com'); + t.is(res.body.events[0].createdBy, 'user@mail.com'); }); }); diff --git a/test/e2e/api/admin/feature.custom-auth.e2e.test.js b/test/e2e/api/admin/feature.custom-auth.e2e.test.js index 6007995113..e67b7b43f1 100644 --- a/test/e2e/api/admin/feature.custom-auth.e2e.test.js +++ b/test/e2e/api/admin/feature.custom-auth.e2e.test.js @@ -52,13 +52,16 @@ test.serial('creates new feature toggle with createdBy', async t => { const request = await setupAppWithCustomAuth(stores, preHook); // create toggle - await request.post('/api/admin/features').send({ - name: 'com.test.Username', - enabled: false, - strategies: [{ name: 'default' }], - }); + await request + .post('/api/admin/features') + .send({ + name: 'com.test.Username', + enabled: false, + strategies: [{ name: 'default' }], + }) + .expect(201); await request.get('/api/admin/events').expect(res => { - t.true(res.body.events[0].createdBy === user.email); + t.is(res.body.events[0].createdBy, user.email); }); }); diff --git a/test/e2e/api/admin/feature.e2e.test.js b/test/e2e/api/admin/feature.e2e.test.js index 1a7cf85b09..5beff85b43 100644 --- a/test/e2e/api/admin/feature.e2e.test.js +++ b/test/e2e/api/admin/feature.e2e.test.js @@ -90,11 +90,14 @@ test.serial('fetch feature toggle with variants', async t => { test.serial('creates new feature toggle with createdBy unknown', async t => { t.plan(1); const request = await setupApp(stores); - await request.post('/api/admin/features').send({ - name: 'com.test.Username', - enabled: false, - strategies: [{ name: 'default' }], - }); + await request + .post('/api/admin/features') + .send({ + name: 'com.test.Username', + enabled: false, + strategies: [{ name: 'default' }], + }) + .expect(201); await request.get('/api/admin/events').expect(res => { t.is(res.body.events[0].createdBy, 'unknown'); }); @@ -346,13 +349,11 @@ test.serial('can untag feature', async t => { .post('/api/admin/features/test.feature/tags') .send(tag) .expect(201); - await new Promise(r => setTimeout(r, 50)); await request .delete( `/api/admin/features/test.feature/tags/${tag.type}/${tag.value}`, ) .expect(200); - await new Promise(r => setTimeout(r, 50)); return request .get('/api/admin/features/test.feature/tags') .expect('Content-Type', /json/) diff --git a/test/e2e/api/admin/metrics.e2e.test.js b/test/e2e/api/admin/metrics.e2e.test.js index 871ab379ed..a28c8b4153 100644 --- a/test/e2e/api/admin/metrics.e2e.test.js +++ b/test/e2e/api/admin/metrics.e2e.test.js @@ -63,12 +63,15 @@ test.serial('should delete application', async t => { }); }); -test.serial('should get 409 when deleting unknwn application', async t => { - t.plan(1); - const request = await setupApp(stores); - return request - .delete('/api/admin/metrics/applications/unkown') - .expect(res => { - t.is(res.status, 409); - }); -}); +test.serial( + 'deleting an application should be idempotent, so expect 200', + async t => { + t.plan(1); + const request = await setupApp(stores); + return request + .delete('/api/admin/metrics/applications/unknown') + .expect(res => { + t.is(res.status, 200); + }); + }, +); diff --git a/test/e2e/api/admin/tag-types.e2e.test.js b/test/e2e/api/admin/tag-types.e2e.test.js index 4f1a91664e..629ca34c3f 100644 --- a/test/e2e/api/admin/tag-types.e2e.test.js +++ b/test/e2e/api/admin/tag-types.e2e.test.js @@ -55,7 +55,6 @@ test.serial('Can create a new tag type', async t => { icon: 'http://icons.iconarchive.com/icons/papirus-team/papirus-apps/32/slack-icon.png', }); - await new Promise(r => setTimeout(r, 200)); return request .get('/api/admin/tag-types/slack') .expect('Content-Type', /json/) @@ -94,7 +93,6 @@ test.serial('Can update a tag types description and icon', async t => { icon: '$', }) .expect(200); - await new Promise(r => setTimeout(r, 200)); return request .get('/api/admin/tag-types/simple') .expect('Content-Type', /json/) @@ -160,7 +158,6 @@ test.serial('Can delete tag type', async t => { .delete('/api/admin/tag-types/simple') .set('Content-Type', 'application/json') .expect(200); - await new Promise(r => setTimeout(r, 50)); return request.get('/api/admin/tag-types/simple').expect(404); }); @@ -175,7 +172,6 @@ test.serial('Non unique tag-types gets rejected', async t => { }) .set('Content-Type', 'application/json') .expect(201); - await new Promise(r => setTimeout(r, 50)); return request .post('/api/admin/tag-types') .send({ diff --git a/test/e2e/helpers/database-init.js b/test/e2e/helpers/database-init.js index d750711773..8165683fd6 100644 --- a/test/e2e/helpers/database-init.js +++ b/test/e2e/helpers/database-init.js @@ -31,7 +31,7 @@ async function resetDatabase(stores) { } function createStrategies(store) { - return dbState.strategies.map(s => store._createStrategy(s)); + return dbState.strategies.map(s => store.createStrategy(s)); } function createContextFields(store) { @@ -51,13 +51,13 @@ function createProjects(store) { } function createFeatures(store) { - return dbState.features.map(f => store._createFeature(f)); + return dbState.features.map(f => store.createFeature(f)); } -function tagFeatures(store) { +async function tagFeatures(tagStore, store) { + await tagStore.createTag({ value: 'Tester', type: 'simple' }); return dbState.features.map(f => - store.tagFeature({ - featureName: f.name, + store.tagFeature(f.name, { value: 'Tester', type: 'simple', }), @@ -65,7 +65,7 @@ function tagFeatures(store) { } function createTagTypes(store) { - return dbState.tag_types.map(t => store._createTagType(t)); + return dbState.tag_types.map(t => store.createTagType(t)); } async function setupDatabase(stores) { @@ -76,7 +76,7 @@ async function setupDatabase(stores) { await Promise.all(createApplications(stores.clientApplicationsStore)); await Promise.all(createProjects(stores.projectStore)); await Promise.all(createTagTypes(stores.tagTypeStore)); - await Promise.all(tagFeatures(stores.featureTagStore)); + await tagFeatures(stores.tagStore, stores.featureTagStore); } module.exports = async function init(databaseSchema = 'test', getLogger) { diff --git a/test/e2e/serices/project-service.e2e.test.js b/test/e2e/services/project-service.e2e.test.js similarity index 98% rename from test/e2e/serices/project-service.e2e.test.js rename to test/e2e/services/project-service.e2e.test.js index 56756e6916..31784d86de 100644 --- a/test/e2e/serices/project-service.e2e.test.js +++ b/test/e2e/services/project-service.e2e.test.js @@ -72,7 +72,7 @@ test.serial('should not be able to delete project with toggles', async t => { description: 'Blah', }; await projectService.createProject(project, 'some-user'); - await stores.featureToggleStore._createFeature({ + await stores.featureToggleStore.createFeature({ name: 'test-project-delete', project: project.id, enabled: false, diff --git a/test/fixtures/fake-client-applications-store.js b/test/fixtures/fake-client-applications-store.js index f58c9bd5ee..76b0ff5502 100644 --- a/test/fixtures/fake-client-applications-store.js +++ b/test/fixtures/fake-client-applications-store.js @@ -1,5 +1,7 @@ 'use strict'; +const NotFoundError = require('../../lib/error/notfound-error'); + module.exports = () => { let apps = []; @@ -8,11 +10,15 @@ module.exports = () => { apps.push(app); return Promise.resolve(); }, + insertNewRow: value => { + apps.push(value); + return Promise.resolve(); + }, getApplications: () => Promise.resolve(apps), getApplication: appName => { const app = apps.filter(a => a.appName === appName)[0]; if (!app) { - throw new Error(`Could not find app=${appName}`); + throw new NotFoundError(`Could not find app=${appName}`); } return app; }, diff --git a/test/fixtures/fake-feature-tag-store.js b/test/fixtures/fake-feature-tag-store.js index 6e3c92312e..e0b9a9f989 100644 --- a/test/fixtures/fake-feature-tag-store.js +++ b/test/fixtures/fake-feature-tag-store.js @@ -1,43 +1,11 @@ 'use strict'; -const NotFoundError = require('../../lib/error/notfound-error'); - module.exports = () => { - const _tags = []; const _featureTags = {}; return { - getAllOfType: type => { - const tags = _tags.filter(t => t.type === type); - return Promise.resolve(tags); - }, - addTag: tag => { - _tags.push({ value: tag.value, type: tag.type }); - }, - removeTag: tag => { - _tags.splice( - _tags.indexOf( - t => t.value === tag.value && t.type === tag.type, - ), - 1, - ); - }, - getTags: () => Promise.resolve(_tags), - getTagByTypeAndValue: (type, value) => { - const tag = _tags.find(t => t.type === type && t.value === value); - if (tag) { - return Promise.resolve(tag); - } - return Promise.reject(new NotFoundError('Could not find tag')); - }, - tagFeature: event => { - _featureTags[event.featureName] = - _featureTags[event.featureName] || []; - const tag = { - value: event.value, - type: event.type, - }; - _featureTags[event.featureName].push(tag); - return tag; + tagFeature: (featureName, tag) => { + _featureTags[featureName] = _featureTags[featureName] || []; + _featureTags[featureName].push(tag); }, untagFeature: event => { const tags = _featureTags[event.featureName]; diff --git a/test/fixtures/fake-feature-toggle-store.js b/test/fixtures/fake-feature-toggle-store.js index c49e6e0792..1659c1b5db 100644 --- a/test/fixtures/fake-feature-toggle-store.js +++ b/test/fixtures/fake-feature-toggle-store.js @@ -22,10 +22,25 @@ module.exports = () => { } return Promise.reject(); }, + updateFeature: updatedFeature => { + _features.splice( + _features.indexOf(f => f.name === updatedFeature.name), + 1, + ); + _features.push(updatedFeature); + }, getFeatures: () => Promise.resolve(_features), - addFeature: feature => _features.push(feature), + createFeature: feature => _features.push(feature), getArchivedFeatures: () => Promise.resolve(_archive), addArchivedFeature: feature => _archive.push(feature), + reviveFeature: feature => { + const revived = _archive.find(f => f.name === feature.name); + _archive.splice( + _archive.indexOf(f => f.name === feature.name), + 1, + ); + _features.push(revived); + }, lastSeenToggles: (names = []) => { names.forEach(name => { const toggle = _features.find(f => f.name === name); @@ -34,5 +49,10 @@ module.exports = () => { } }); }, + dropFeatures: () => { + _features.splice(0, _features.length); + _archive.splice(0, _archive.length); + }, + importFeature: feat => Promise.resolve(_features.push(feat)), }; }; diff --git a/test/fixtures/fake-strategies-store.js b/test/fixtures/fake-strategies-store.js index 09a8ae97f7..1abb9f2a66 100644 --- a/test/fixtures/fake-strategies-store.js +++ b/test/fixtures/fake-strategies-store.js @@ -16,6 +16,20 @@ module.exports = () => { } return Promise.reject(new NotFoundError('Not found!')); }, - addStrategy: strat => _strategies.push(strat), + createStrategy: strat => _strategies.push(strat), + updateStrategy: strat => { + _strategies.splice( + _strategies.indexOf(({ name }) => name === strat.name), + 1, + ); + _strategies.push(strat); + }, + importStrategy: strat => Promise.resolve(_strategies.push(strat)), + dropStrategies: () => _strategies.splice(0, _strategies.length), + deleteStrategy: strat => + _strategies.splice( + _strategies.indexOf(({ name }) => name === strat.name), + 1, + ), }; }; diff --git a/test/fixtures/fake-tag-store.js b/test/fixtures/fake-tag-store.js new file mode 100644 index 0000000000..517e0afb51 --- /dev/null +++ b/test/fixtures/fake-tag-store.js @@ -0,0 +1,30 @@ +const NotFoundError = require('../../lib/error/notfound-error'); + +module.exports = () => { + const _tags = []; + return { + getTagsByType: type => { + const tags = _tags.filter(t => t.type === type); + return Promise.resolve(tags); + }, + createTag: tag => { + _tags.push({ value: tag.value, type: tag.type }); + }, + deleteTag: tag => { + _tags.splice( + _tags.indexOf( + t => t.value === tag.value && t.type === tag.type, + ), + 1, + ); + }, + getAll: () => Promise.resolve(_tags), + getTag: (type, value) => { + const tag = _tags.find(t => t.type === type && t.value === value); + if (tag) { + return Promise.resolve(tag); + } + return Promise.reject(new NotFoundError('Could not find tag')); + }, + }; +}; diff --git a/test/fixtures/store.js b/test/fixtures/store.js index bea80fd3d4..fdd4509dad 100644 --- a/test/fixtures/store.js +++ b/test/fixtures/store.js @@ -5,6 +5,7 @@ const clientInstanceStore = require('./fake-client-instance-store'); const clientApplicationsStore = require('./fake-client-applications-store'); const featureToggleStore = require('./fake-feature-toggle-store'); const featureTagStore = require('./fake-feature-tag-store'); +const tagStore = require('./fake-tag-store'); const eventStore = require('./fake-event-store'); const strategyStore = require('./fake-strategies-store'); const contextFieldStore = require('./fake-context-store'); @@ -25,6 +26,7 @@ module.exports = { clientInstanceStore: clientInstanceStore(), featureToggleStore: featureToggleStore(), featureTagStore: featureTagStore(), + tagStore: tagStore(), eventStore: eventStore(), strategyStore: strategyStore(), contextFieldStore: contextFieldStore(),