From cefdde18f4fc4fa531ce1354fad993bf8d629719 Mon Sep 17 00:00:00 2001 From: Alexandre Bodin Date: Tue, 16 Feb 2021 12:08:35 +0100 Subject: [PATCH 1/5] Init db lifecycles manager --- .../lib/build-database-schema.js | 2 +- .../lib/mount-models.js | 2 +- .../strapi-database/lib/database-manager.js | 2 + .../strapi-database/lib/lifecycle-manager.js | 35 ++++++++++++++++ .../strapi-database/lib/migration-manager.js | 2 +- .../queries/__tests__/create-query.test.js | 8 ++++ .../strapi-database/lib/utils/lifecycles.js | 4 ++ .../config/functions/bootstrap.js | 40 ++++++++++--------- 8 files changed, 73 insertions(+), 22 deletions(-) create mode 100644 packages/strapi-database/lib/lifecycle-manager.js diff --git a/packages/strapi-connector-bookshelf/lib/build-database-schema.js b/packages/strapi-connector-bookshelf/lib/build-database-schema.js index 31de44eaf3..9f727713ce 100644 --- a/packages/strapi-connector-bookshelf/lib/build-database-schema.js +++ b/packages/strapi-connector-bookshelf/lib/build-database-schema.js @@ -398,7 +398,7 @@ const createOrUpdateTable = async ({ table, attributes, definition, ORM, model } module.exports = async ({ ORM, loadedModel, definition, connection, model }) => { // run migrations - await strapi.db.migrations.runMigration(migrateSchemas, { + await strapi.db.migrations.run(migrateSchemas, { ORM, loadedModel, definition, diff --git a/packages/strapi-connector-mongoose/lib/mount-models.js b/packages/strapi-connector-mongoose/lib/mount-models.js index 2be0cc1ee7..e75dfa6e07 100644 --- a/packages/strapi-connector-mongoose/lib/mount-models.js +++ b/packages/strapi-connector-mongoose/lib/mount-models.js @@ -301,7 +301,7 @@ module.exports = async ({ models, target }, ctx) => { const definitionDidChange = await didDefinitionChange(definition, instance); // run migrations - await strapi.db.migrations.runMigration(migrateSchema, { + await strapi.db.migrations.run(migrateSchema, { definition, model: modelInstance, ORM: instance, diff --git a/packages/strapi-database/lib/database-manager.js b/packages/strapi-database/lib/database-manager.js index 204c92b0b0..f777de90c1 100644 --- a/packages/strapi-database/lib/database-manager.js +++ b/packages/strapi-database/lib/database-manager.js @@ -7,6 +7,7 @@ const createConnectorRegistry = require('./connector-registry'); const constants = require('./constants'); const { validateModelSchemas } = require('./validation'); const createMigrationManager = require('./migration-manager'); +const createLifecycleManager = require('./lifecycle-manager'); class DatabaseManager { constructor(strapi) { @@ -23,6 +24,7 @@ class DatabaseManager { this.models = new Map(); this.migrations = createMigrationManager(this); + this.lifecycles = createLifecycleManager(this); } async initialize() { diff --git a/packages/strapi-database/lib/lifecycle-manager.js b/packages/strapi-database/lib/lifecycle-manager.js new file mode 100644 index 0000000000..094a234aa2 --- /dev/null +++ b/packages/strapi-database/lib/lifecycle-manager.js @@ -0,0 +1,35 @@ +'use strict'; +const debug = require('debug')('strapi-database:lifecycle'); +const { isFunction, isNil } = require('lodash/fp'); + +class LifecycleManager { + constructor(db) { + debug('Initialize lifecycle manager'); + this.db = db; + this.lifecycles = []; + } + + register(lifecycle) { + debug('Register lifecycle'); + + this.lifecycles.push(lifecycle); + } + + async run(action, model, ...args) { + for (const lifecycle of this.lifecycles) { + if (!isNil(lifecycle.model) && lifecycle.model !== model.uid) { + continue; + } + + if (isFunction(lifecycle[action])) { + debug(`Run lifecycle ${action} for model ${model.uid}`); + + await lifecycle[action](...args); + } + } + } +} + +module.exports = strapi => { + return new LifecycleManager(strapi); +}; diff --git a/packages/strapi-database/lib/migration-manager.js b/packages/strapi-database/lib/migration-manager.js index a1d1f49970..8a29cbf39f 100644 --- a/packages/strapi-database/lib/migration-manager.js +++ b/packages/strapi-database/lib/migration-manager.js @@ -14,7 +14,7 @@ class MigrationManager { this.migrations.push(migration); } - async runMigration(fn, options, context = {}) { + async run(fn, options, context = {}) { debug('Run migration'); await this.runBefore(options, context); await fn(options, context); diff --git a/packages/strapi-database/lib/queries/__tests__/create-query.test.js b/packages/strapi-database/lib/queries/__tests__/create-query.test.js index 033189021b..0ea7cd5db0 100644 --- a/packages/strapi-database/lib/queries/__tests__/create-query.test.js +++ b/packages/strapi-database/lib/queries/__tests__/create-query.test.js @@ -4,6 +4,14 @@ const _ = require('lodash'); const createQuery = require('../create-query'); describe('Database queries', () => { + global.strapi = { + db: { + lifecycles: { + run() {}, + }, + }, + }; + describe('Substitute id with primaryKey in parameters', () => { test.each(['create', 'update', 'delete', 'find', 'findOne', 'search', 'count', 'countSearch'])( 'Calling "%s" replaces id by the primaryKey in the params of the model before calling the underlying connector', diff --git a/packages/strapi-database/lib/utils/lifecycles.js b/packages/strapi-database/lib/utils/lifecycles.js index 5875fd89f8..155a67711d 100644 --- a/packages/strapi-database/lib/utils/lifecycles.js +++ b/packages/strapi-database/lib/utils/lifecycles.js @@ -3,6 +3,10 @@ const _ = require('lodash'); const executeLifecycle = async (lifecycle, model, ...args) => { + // Run registered lifecycles + await strapi.db.lifecycles.run(lifecycle, model, ...args); + + // Run user lifecycles if (_.has(model, `lifecycles.${lifecycle}`)) { await model.lifecycles[lifecycle](...args); } diff --git a/packages/strapi-plugin-i18n/config/functions/bootstrap.js b/packages/strapi-plugin-i18n/config/functions/bootstrap.js index 66f273810c..92603ab565 100644 --- a/packages/strapi-plugin-i18n/config/functions/bootstrap.js +++ b/packages/strapi-plugin-i18n/config/functions/bootstrap.js @@ -1,6 +1,5 @@ 'use strict'; -const _ = require('lodash'); const { capitalize } = require('lodash/fp'); const { getService } = require('../../utils'); @@ -26,23 +25,26 @@ module.exports = async () => { await getService('locales').setDefaultLocale(DEFAULT_LOCALE); } - Object.values(strapi.models).forEach(model => { - if (getService('content-types').isLocalized(model)) { - // TODO: support adding lifecycles programmatically or connecting to a database event handler to avoid conflicts with existing lifecycles fonctions - - _.set(model, 'lifecycles.beforeCreate', async data => { - if (!data.locale) { - data.locale = await getService('locales').getDefaultLocale(); - } + Object.values(strapi.models) + .filter(model => getService('content-types').isLocalized(model)) + .forEach(model => { + strapi.db.lifecycles.register({ + model: model.uid, + async beforeCreate(data) { + if (!data.locale) { + data.locale = await getService('locales').getDefaultLocale(); + } + }, + async afterCreate(entry) { + await getService('localizations').addLocalizations(entry, { model }); + }, + async afterUpdate(entry) { + await getService('localizations').updateNonLocalizedFields(entry, { model }); + }, + async afterDelete() { + // TODO/ implement + // await getService('localizations').updateDeletedLocalization(entry, { model }); + }, }); - - _.set(model, 'lifecycles.afterCreate', async entry => { - await getService('localizations').addLocalizations(entry, { model }); - }); - - _.set(model, 'lifecycles.afterUpdate', async entry => { - await getService('localizations').updateNonLocalizedFields(entry, { model }); - }); - } - }); + }); }; From 9020fc4e9159ff3bcb67962ccb47932789e4467c Mon Sep 17 00:00:00 2001 From: Alexandre Bodin Date: Wed, 17 Feb 2021 13:00:49 +0100 Subject: [PATCH 2/5] Add lifecycles to manage locale & localizations fields --- .../config/functions/bootstrap.js | 9 +- .../services/__tests__/localizations.test.js | 85 ++++++++++++++++++- .../services/localizations.js | 49 ++++++++++- 3 files changed, 131 insertions(+), 12 deletions(-) diff --git a/packages/strapi-plugin-i18n/config/functions/bootstrap.js b/packages/strapi-plugin-i18n/config/functions/bootstrap.js index 92603ab565..891009e0ed 100644 --- a/packages/strapi-plugin-i18n/config/functions/bootstrap.js +++ b/packages/strapi-plugin-i18n/config/functions/bootstrap.js @@ -31,9 +31,7 @@ module.exports = async () => { strapi.db.lifecycles.register({ model: model.uid, async beforeCreate(data) { - if (!data.locale) { - data.locale = await getService('locales').getDefaultLocale(); - } + await getService('lcoalizations').assignDefaultLocale(data); }, async afterCreate(entry) { await getService('localizations').addLocalizations(entry, { model }); @@ -41,9 +39,8 @@ module.exports = async () => { async afterUpdate(entry) { await getService('localizations').updateNonLocalizedFields(entry, { model }); }, - async afterDelete() { - // TODO/ implement - // await getService('localizations').updateDeletedLocalization(entry, { model }); + async afterDelete(entry) { + await getService('localizations').removeEntryFromLocalizations(entry, { model }); }, }); }); diff --git a/packages/strapi-plugin-i18n/services/__tests__/localizations.test.js b/packages/strapi-plugin-i18n/services/__tests__/localizations.test.js index 97f119eb1e..e28f25895a 100644 --- a/packages/strapi-plugin-i18n/services/__tests__/localizations.test.js +++ b/packages/strapi-plugin-i18n/services/__tests__/localizations.test.js @@ -1,6 +1,11 @@ 'use strict'; -const { addLocalizations, updateNonLocalizedFields } = require('../localizations'); +const { + assignDefaultLocale, + addLocalizations, + updateNonLocalizedFields, + removeEntryFromLocalizations, +} = require('../localizations'); const model = { uid: 'test-model', @@ -20,12 +25,43 @@ const model = { }; describe('localizations service', () => { + describe('assignDefaultLocale', () => { + test('Does not change the input if locale is already defined', async () => { + const input = { locale: 'myLocale' }; + await assignDefaultLocale(input); + + expect(input).toStrictEqual({ locale: 'myLocale' }); + }); + + test('Use default locale to set the locale on the input data', async () => { + const getDefaultLocaleMock = jest.fn(() => 'defaultLocale'); + + global.strapi = { + plugins: { + i18n: { + services: { + locales: { + getDefaultLocale: getDefaultLocaleMock, + }, + }, + }, + }, + }; + + const input = {}; + await assignDefaultLocale(input); + + expect(input).toStrictEqual({ locale: 'defaultLocale' }); + expect(getDefaultLocaleMock).toHaveBeenCalled(); + }); + }); + describe('addLocalizations', () => { test('Does nothing if entry already as a localizations array', async () => { const entry = { localizations: [] }; await addLocalizations(entry, { model }); - expect(entry).toEqual({ localizations: [] }); + expect(entry).toStrictEqual({ localizations: [] }); }); test('Updates entry in db', async () => { @@ -58,7 +94,7 @@ describe('localizations service', () => { await addLocalizations(entry, { model }); - expect(entry).toEqual({ + expect(entry).toStrictEqual({ id: entry.id, locale: entry.locale, localizations: [ @@ -127,4 +163,47 @@ describe('localizations service', () => { expect(update).toHaveBeenCalledWith({ id: 2 }, { stars: 1 }); }); }); + + describe('removeEntryFromLocalizations', () => { + test('Does nothing if no localizations set', async () => { + const update = jest.fn(); + global.strapi = { + query() { + return { update }; + }, + }; + + const entry = { id: 1, locale: 'test' }; + + await removeEntryFromLocalizations(entry, { model }); + + expect(update).not.toHaveBeenCalled(); + }); + + test('Removes entry from localizations', async () => { + const update = jest.fn(); + global.strapi = { + query() { + return { update }; + }, + }; + + const entry = { + id: 1, + locale: 'mainLocale', + localizations: [ + { id: 1, locale: 'mainLocale' }, + { id: 2, locale: 'otherLocale' }, + ], + }; + + await removeEntryFromLocalizations(entry, { model }); + + expect(update).toHaveBeenCalledTimes(1); + expect(update).toHaveBeenCalledWith( + { id: 2 }, + { localizations: [{ id: 2, locale: 'otherLocale' }] } + ); + }); + }); }); diff --git a/packages/strapi-plugin-i18n/services/localizations.js b/packages/strapi-plugin-i18n/services/localizations.js index 87d22257a3..3425996536 100644 --- a/packages/strapi-plugin-i18n/services/localizations.js +++ b/packages/strapi-plugin-i18n/services/localizations.js @@ -1,8 +1,26 @@ 'use strict'; const { pick, isNil } = require('lodash/fp'); + +const { getService } = require('../utils'); const { getNonLocalizedFields } = require('./content-types'); +/** + * Adds the default locale to an object if it isn't defined set yet + * @param {Object} data a data object before being persisted into db + */ +const assignDefaultLocale = async data => { + if (isNil(data.locale)) { + data.locale = await getService('locales').getDefaultLocale(); + } +}; + +/** + * Create default localizations for an entry if it isn't defined yet + * @param {Object} entry entry to update + * @param {Object} options + * @param {Object} options.model corresponding model + */ const addLocalizations = async (entry, { model }) => { if (isNil(entry.localizations)) { const localizations = [{ locale: entry.locale, id: entry.id }]; @@ -12,10 +30,16 @@ const addLocalizations = async (entry, { model }) => { } }; +/** + * Update non localized fields of all the related localizations of an entry with the entry values + * @param {Object} entry entry to update + * @param {Object} options + * @param {Object} options.model corresponding model + */ const updateNonLocalizedFields = async (entry, { model }) => { - const fieldsToUpdate = pick(getNonLocalizedFields(model), entry); - if (Array.isArray(entry.localizations)) { + const fieldsToUpdate = pick(getNonLocalizedFields(model), entry); + const updateQueries = entry.localizations .filter(({ id }) => id != entry.id) .map(({ id }) => strapi.query(model.uid).update({ id }, fieldsToUpdate)); @@ -24,8 +48,27 @@ const updateNonLocalizedFields = async (entry, { model }) => { } }; +/** + * Remove entry from localizations & udpate realted localizations + * @param {Object} entry entry to remove from localizations + * @param {Object} options + * @param {Object} options.model corresponding model + */ +const removeEntryFromLocalizations = async (entry, { model }) => { + if (Array.isArray(entry.localizations)) { + const newLocalizations = entry.localizations.filter(({ id }) => id != entry.id); + + const updateQueries = newLocalizations.map(({ id }) => { + return strapi.query(model.uid).update({ id }, { localizations: newLocalizations }); + }); + + await Promise.all(updateQueries); + } +}; + module.exports = { + assignDefaultLocale, addLocalizations, updateNonLocalizedFields, - getNonLocalizedFields, + removeEntryFromLocalizations, }; From c1da5a568501a33f68e14f82d522c5caaf123ad1 Mon Sep 17 00:00:00 2001 From: Alexandre Bodin Date: Wed, 17 Feb 2021 15:51:47 +0100 Subject: [PATCH 3/5] Add lifecyle manager tests --- .../lib/__tests__/lifecycle-manager.test.js | 54 +++++++++++++++++++ .../strapi-database/lib/database-manager.js | 2 +- .../strapi-database/lib/lifecycle-manager.js | 4 +- 3 files changed, 57 insertions(+), 3 deletions(-) create mode 100644 packages/strapi-database/lib/__tests__/lifecycle-manager.test.js diff --git a/packages/strapi-database/lib/__tests__/lifecycle-manager.test.js b/packages/strapi-database/lib/__tests__/lifecycle-manager.test.js new file mode 100644 index 0000000000..a0c3d3291c --- /dev/null +++ b/packages/strapi-database/lib/__tests__/lifecycle-manager.test.js @@ -0,0 +1,54 @@ +'use strict'; + +const createLifecycleManager = require('../lifecycle-manager'); + +describe('Lifecycle Manager', () => { + test('Allows registering lifecycles', () => { + const manager = createLifecycleManager(); + + const lifecycle = {}; + manager.register(lifecycle); + + expect(manager.lifecycles).toEqual([lifecycle]); + }); + + test('Will run all the lifecycles if no model specified', async () => { + const lifecycleA = { + find: jest.fn(), + }; + + const lifecycleB = { + find: jest.fn(), + }; + + const manager = createLifecycleManager(); + + manager.register(lifecycleA).register(lifecycleB); + + await manager.run('find', { uid: 'test-uid' }); + + expect(lifecycleA.find).toHaveBeenCalled(); + expect(lifecycleB.find).toHaveBeenCalled(); + }); + + test('Will match on model if specified', async () => { + const lifecycleA = { + model: 'test-uid', + find: jest.fn(), + }; + + const lifecycleB = { + model: 'other-uid', + find: jest.fn(), + }; + + const manager = createLifecycleManager(); + + manager.register(lifecycleA).register(lifecycleB); + + await manager.run('find', { uid: 'test-uid' }); + + expect(lifecycleA.find).toHaveBeenCalled(); + expect(lifecycleB.find).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/strapi-database/lib/database-manager.js b/packages/strapi-database/lib/database-manager.js index f777de90c1..f610c22374 100644 --- a/packages/strapi-database/lib/database-manager.js +++ b/packages/strapi-database/lib/database-manager.js @@ -24,7 +24,7 @@ class DatabaseManager { this.models = new Map(); this.migrations = createMigrationManager(this); - this.lifecycles = createLifecycleManager(this); + this.lifecycles = createLifecycleManager(); } async initialize() { diff --git a/packages/strapi-database/lib/lifecycle-manager.js b/packages/strapi-database/lib/lifecycle-manager.js index 094a234aa2..2781618633 100644 --- a/packages/strapi-database/lib/lifecycle-manager.js +++ b/packages/strapi-database/lib/lifecycle-manager.js @@ -3,9 +3,8 @@ const debug = require('debug')('strapi-database:lifecycle'); const { isFunction, isNil } = require('lodash/fp'); class LifecycleManager { - constructor(db) { + constructor() { debug('Initialize lifecycle manager'); - this.db = db; this.lifecycles = []; } @@ -13,6 +12,7 @@ class LifecycleManager { debug('Register lifecycle'); this.lifecycles.push(lifecycle); + return this; } async run(action, model, ...args) { From e7189d25789ff68d61b0473f02cfff5584c04fe4 Mon Sep 17 00:00:00 2001 From: Alexandre Bodin Date: Wed, 17 Feb 2021 16:59:46 +0100 Subject: [PATCH 4/5] Fix typos & remove unecessary parameter --- packages/strapi-database/lib/lifecycle-manager.js | 4 +--- packages/strapi-plugin-i18n/config/functions/bootstrap.js | 4 ++-- .../services/__tests__/localizations.test.js | 8 ++++---- packages/strapi-plugin-i18n/services/localizations.js | 5 +++-- 4 files changed, 10 insertions(+), 11 deletions(-) diff --git a/packages/strapi-database/lib/lifecycle-manager.js b/packages/strapi-database/lib/lifecycle-manager.js index 2781618633..1c7d9540a2 100644 --- a/packages/strapi-database/lib/lifecycle-manager.js +++ b/packages/strapi-database/lib/lifecycle-manager.js @@ -30,6 +30,4 @@ class LifecycleManager { } } -module.exports = strapi => { - return new LifecycleManager(strapi); -}; +module.exports = () => new LifecycleManager(); diff --git a/packages/strapi-plugin-i18n/config/functions/bootstrap.js b/packages/strapi-plugin-i18n/config/functions/bootstrap.js index 891009e0ed..e5e4595a50 100644 --- a/packages/strapi-plugin-i18n/config/functions/bootstrap.js +++ b/packages/strapi-plugin-i18n/config/functions/bootstrap.js @@ -31,7 +31,7 @@ module.exports = async () => { strapi.db.lifecycles.register({ model: model.uid, async beforeCreate(data) { - await getService('lcoalizations').assignDefaultLocale(data); + await getService('localizations').assignDefaultLocale(data); }, async afterCreate(entry) { await getService('localizations').addLocalizations(entry, { model }); @@ -40,7 +40,7 @@ module.exports = async () => { await getService('localizations').updateNonLocalizedFields(entry, { model }); }, async afterDelete(entry) { - await getService('localizations').removeEntryFromLocalizations(entry, { model }); + await getService('localizations').removeEntryFromRelatedLocalizations(entry, { model }); }, }); }); diff --git a/packages/strapi-plugin-i18n/services/__tests__/localizations.test.js b/packages/strapi-plugin-i18n/services/__tests__/localizations.test.js index e28f25895a..7051142fa9 100644 --- a/packages/strapi-plugin-i18n/services/__tests__/localizations.test.js +++ b/packages/strapi-plugin-i18n/services/__tests__/localizations.test.js @@ -4,7 +4,7 @@ const { assignDefaultLocale, addLocalizations, updateNonLocalizedFields, - removeEntryFromLocalizations, + removeEntryFromRelatedLocalizations, } = require('../localizations'); const model = { @@ -164,7 +164,7 @@ describe('localizations service', () => { }); }); - describe('removeEntryFromLocalizations', () => { + describe('removeEntryFromRelatedLocalizations', () => { test('Does nothing if no localizations set', async () => { const update = jest.fn(); global.strapi = { @@ -175,7 +175,7 @@ describe('localizations service', () => { const entry = { id: 1, locale: 'test' }; - await removeEntryFromLocalizations(entry, { model }); + await removeEntryFromRelatedLocalizations(entry, { model }); expect(update).not.toHaveBeenCalled(); }); @@ -197,7 +197,7 @@ describe('localizations service', () => { ], }; - await removeEntryFromLocalizations(entry, { model }); + await removeEntryFromRelatedLocalizations(entry, { model }); expect(update).toHaveBeenCalledTimes(1); expect(update).toHaveBeenCalledWith( diff --git a/packages/strapi-plugin-i18n/services/localizations.js b/packages/strapi-plugin-i18n/services/localizations.js index 3425996536..1b772f9a25 100644 --- a/packages/strapi-plugin-i18n/services/localizations.js +++ b/packages/strapi-plugin-i18n/services/localizations.js @@ -50,11 +50,12 @@ const updateNonLocalizedFields = async (entry, { model }) => { /** * Remove entry from localizations & udpate realted localizations + * This method should be used only after an entry is deleted * @param {Object} entry entry to remove from localizations * @param {Object} options * @param {Object} options.model corresponding model */ -const removeEntryFromLocalizations = async (entry, { model }) => { +const removeEntryFromRelatedLocalizations = async (entry, { model }) => { if (Array.isArray(entry.localizations)) { const newLocalizations = entry.localizations.filter(({ id }) => id != entry.id); @@ -70,5 +71,5 @@ module.exports = { assignDefaultLocale, addLocalizations, updateNonLocalizedFields, - removeEntryFromLocalizations, + removeEntryFromRelatedLocalizations, }; From 77906863aa11ce2e95533a2cea9fcd16db95cfd4 Mon Sep 17 00:00:00 2001 From: Alexandre Bodin Date: Wed, 17 Feb 2021 18:40:45 +0100 Subject: [PATCH 5/5] fix typos --- packages/strapi-plugin-i18n/services/localizations.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/strapi-plugin-i18n/services/localizations.js b/packages/strapi-plugin-i18n/services/localizations.js index 1b772f9a25..052caa0232 100644 --- a/packages/strapi-plugin-i18n/services/localizations.js +++ b/packages/strapi-plugin-i18n/services/localizations.js @@ -6,7 +6,7 @@ const { getService } = require('../utils'); const { getNonLocalizedFields } = require('./content-types'); /** - * Adds the default locale to an object if it isn't defined set yet + * Adds the default locale to an object if it isn't defined yet * @param {Object} data a data object before being persisted into db */ const assignDefaultLocale = async data => { @@ -49,7 +49,7 @@ const updateNonLocalizedFields = async (entry, { model }) => { }; /** - * Remove entry from localizations & udpate realted localizations + * Remove entry from localizations & udpate related localizations * This method should be used only after an entry is deleted * @param {Object} entry entry to remove from localizations * @param {Object} options