From 32d019ecbe6cb9bf02fdde3095d08f8570533fed Mon Sep 17 00:00:00 2001 From: Jamie Howard Date: Thu, 9 Mar 2023 16:50:51 +0000 Subject: [PATCH 1/7] feature(ee): add db entries to persist ee feature tables --- packages/core/admin/ee/server/bootstrap.js | 31 +++++++++- .../admin/ee/server/utils/reserved-tables.js | 57 +++++++++++++++++++ 2 files changed, 87 insertions(+), 1 deletion(-) create mode 100644 packages/core/admin/ee/server/utils/reserved-tables.js diff --git a/packages/core/admin/ee/server/bootstrap.js b/packages/core/admin/ee/server/bootstrap.js index 293cfab84c..78a2b2ff78 100644 --- a/packages/core/admin/ee/server/bootstrap.js +++ b/packages/core/admin/ee/server/bootstrap.js @@ -2,11 +2,23 @@ // eslint-disable-next-line node/no-extraneous-require const { features } = require('@strapi/strapi/lib/utils/ee'); +const { mapAsync } = require('@strapi/utils'); const executeCEBootstrap = require('../../server/bootstrap'); const { getService } = require('../../server/utils'); const actions = require('./config/admin-actions'); +const { + createReservedTable, + findTablesThatStartWithPrefix, + doesReservedTableEntryExist, + createReservedTableEntries, +} = require('./utils/reserved-tables'); + +module.exports = async ({ strapi }) => { + const connection = strapi.db.getSchemaConnection(); + await createReservedTable(connection); + + const tablesToPersist = []; -module.exports = async () => { const { actionProvider } = getService('permission'); if (features.isEnabled('sso')) { @@ -14,9 +26,26 @@ module.exports = async () => { } if (features.isEnabled('audit-logs')) { + const auditLogTables = await findTablesThatStartWithPrefix('strapi_audit_logs'); + tablesToPersist.push(...auditLogTables); + await actionProvider.registerMany(actions.auditLogs); } + if (features.isEnabled('review-workflows')) { + const reviewWorkflowTables = await findTablesThatStartWithPrefix('strapi_workflows'); + tablesToPersist.push(...reviewWorkflowTables); + } + + let recordsToInsert = await mapAsync(tablesToPersist, (tableName) => + doesReservedTableEntryExist(tableName) + ); + recordsToInsert = recordsToInsert.filter((record) => record); + + if (recordsToInsert.length > 0) { + await createReservedTableEntries(recordsToInsert); + } + await getService('seat-enforcement').seatEnforcementWorkflow(); await executeCEBootstrap(); diff --git a/packages/core/admin/ee/server/utils/reserved-tables.js b/packages/core/admin/ee/server/utils/reserved-tables.js new file mode 100644 index 0000000000..a9d793c530 --- /dev/null +++ b/packages/core/admin/ee/server/utils/reserved-tables.js @@ -0,0 +1,57 @@ +'use strict'; + +const reservedTableName = 'strapi_reserved_table_names'; + +/** + * Finds all tables in the database that start with a prefix + * @param {string} prefix + * @returns {Array} + */ +const findTablesThatStartWithPrefix = async (prefix) => { + const tables = await strapi.db.dialect.schemaInspector.getTables(); + return tables.filter((tableName) => tableName.startsWith(prefix)); +}; + +/** + * Check whether an entry exists in the DB for a reserved table name + * If no entry does exist return the tableName to be added to the DB + * @param {string} tableName + * @returns {string|undefined} + */ +const doesReservedTableEntryExist = async (tableName) => { + const rows = await strapi.db.getConnection()(reservedTableName).select().where('name', tableName); + + if (rows.length === 0) { + return tableName; + } +}; + +/** + * Create entries in the DB for an array of reserved table names + * @param {Array} tableNames + */ +const createReservedTableEntries = async (tableNames) => + strapi.db + .getConnection()(reservedTableName) + .insert(tableNames.map((tableName) => ({ name: tableName }))); + +/** + * Create the DB table 'strapi_reserved_table_names' if it does not exist + * @param {SchemaBuilder} connection to the strapi db + */ +const createReservedTable = async (connection) => { + if (await connection.hasTable(reservedTableName)) { + return; + } + return connection.createTable(reservedTableName, (table) => { + table.increments('id'); + table.string('name'); + }); +}; + +module.exports = { + findTablesThatStartWithPrefix, + doesReservedTableEntryExist, + createReservedTableEntries, + createReservedTable, +}; From d3c6a7c927b61e7bb94d5d70e41eac6050b0e9cc Mon Sep 17 00:00:00 2001 From: Jamie Howard Date: Wed, 15 Mar 2023 11:02:44 +0000 Subject: [PATCH 2/7] feature(database): account for persisted tables in schema diffing --- packages/core/database/lib/schema/diff.js | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/packages/core/database/lib/schema/diff.js b/packages/core/database/lib/schema/diff.js index 63c9eeef06..7fe2ceb9f9 100644 --- a/packages/core/database/lib/schema/diff.js +++ b/packages/core/database/lib/schema/diff.js @@ -2,7 +2,11 @@ const _ = require('lodash/fp'); -const RESERVED_TABLE_NAMES = ['strapi_migrations', 'strapi_database_schema']; +const RESERVED_TABLE_NAMES = [ + 'strapi_migrations', + 'strapi_database_schema', + 'strapi_reserved_table_names', +]; const statuses = { CHANGED: 'CHANGED', @@ -322,7 +326,7 @@ module.exports = (db) => { }; }; - const diffSchemas = (srcSchema, destSchema) => { + const diffSchemas = async (srcSchema, destSchema) => { const addedTables = []; const updatedTables = []; const unchangedTables = []; @@ -344,11 +348,18 @@ module.exports = (db) => { } } + const reservedTablesFromDB = await db + .getConnection() + .select('name') + .from('strapi_reserved_table_names'); + + const reservedTables = [ + ...RESERVED_TABLE_NAMES, + ...reservedTablesFromDB.map((entry) => entry.name), + ]; + for (const srcTable of srcSchema.tables) { - if ( - !helpers.hasTable(destSchema, srcTable.name) && - !RESERVED_TABLE_NAMES.includes(srcTable.name) - ) { + if (!helpers.hasTable(destSchema, srcTable.name) && !reservedTables.includes(srcTable.name)) { removedTables.push(srcTable); } } From 7e4cea8a2410a97d3c6c7ba02af2b96601afd3d0 Mon Sep 17 00:00:00 2001 From: Jamie Howard Date: Wed, 15 Mar 2023 17:24:25 +0000 Subject: [PATCH 3/7] refactor(ee): use strapi core store to persist DB tables refactor(database) --- packages/core/admin/ee/server/bootstrap.js | 30 ++------- .../admin/ee/server/utils/reserved-tables.js | 54 +++++++-------- .../lib/schema/__tests__/schema-diff.test.js | 66 ++++++++++++++++--- packages/core/database/lib/schema/diff.js | 26 ++++---- packages/core/database/lib/schema/index.js | 2 +- 5 files changed, 97 insertions(+), 81 deletions(-) diff --git a/packages/core/admin/ee/server/bootstrap.js b/packages/core/admin/ee/server/bootstrap.js index 78a2b2ff78..8800903f82 100644 --- a/packages/core/admin/ee/server/bootstrap.js +++ b/packages/core/admin/ee/server/bootstrap.js @@ -2,23 +2,12 @@ // eslint-disable-next-line node/no-extraneous-require const { features } = require('@strapi/strapi/lib/utils/ee'); -const { mapAsync } = require('@strapi/utils'); const executeCEBootstrap = require('../../server/bootstrap'); const { getService } = require('../../server/utils'); const actions = require('./config/admin-actions'); -const { - createReservedTable, - findTablesThatStartWithPrefix, - doesReservedTableEntryExist, - createReservedTableEntries, -} = require('./utils/reserved-tables'); - -module.exports = async ({ strapi }) => { - const connection = strapi.db.getSchemaConnection(); - await createReservedTable(connection); - - const tablesToPersist = []; +const { reserveTablesWithPrefix } = require('./utils/reserved-tables'); +module.exports = async () => { const { actionProvider } = getService('permission'); if (features.isEnabled('sso')) { @@ -26,24 +15,13 @@ module.exports = async ({ strapi }) => { } if (features.isEnabled('audit-logs')) { - const auditLogTables = await findTablesThatStartWithPrefix('strapi_audit_logs'); - tablesToPersist.push(...auditLogTables); + await reserveTablesWithPrefix('strapi_audit_logs'); await actionProvider.registerMany(actions.auditLogs); } if (features.isEnabled('review-workflows')) { - const reviewWorkflowTables = await findTablesThatStartWithPrefix('strapi_workflows'); - tablesToPersist.push(...reviewWorkflowTables); - } - - let recordsToInsert = await mapAsync(tablesToPersist, (tableName) => - doesReservedTableEntryExist(tableName) - ); - recordsToInsert = recordsToInsert.filter((record) => record); - - if (recordsToInsert.length > 0) { - await createReservedTableEntries(recordsToInsert); + await reserveTablesWithPrefix('strapi_workflows'); } await getService('seat-enforcement').seatEnforcementWorkflow(); diff --git a/packages/core/admin/ee/server/utils/reserved-tables.js b/packages/core/admin/ee/server/utils/reserved-tables.js index a9d793c530..65444d1153 100644 --- a/packages/core/admin/ee/server/utils/reserved-tables.js +++ b/packages/core/admin/ee/server/utils/reserved-tables.js @@ -1,7 +1,5 @@ 'use strict'; -const reservedTableName = 'strapi_reserved_table_names'; - /** * Finds all tables in the database that start with a prefix * @param {string} prefix @@ -13,45 +11,39 @@ const findTablesThatStartWithPrefix = async (prefix) => { }; /** - * Check whether an entry exists in the DB for a reserved table name - * If no entry does exist return the tableName to be added to the DB - * @param {string} tableName - * @returns {string|undefined} + * Get all reserved table names from the core store + * @returns {Array} */ -const doesReservedTableEntryExist = async (tableName) => { - const rows = await strapi.db.getConnection()(reservedTableName).select().where('name', tableName); - - if (rows.length === 0) { - return tableName; - } -}; +const getReservedTables = async () => + strapi.store.get({ + type: 'core', + key: 'reserved_tables', + }); /** - * Create entries in the DB for an array of reserved table names - * @param {Array} tableNames + * Add all table names that start with a prefix to the reserved tables in + * core store + * @param {string} tableNamePrefix */ -const createReservedTableEntries = async (tableNames) => - strapi.db - .getConnection()(reservedTableName) - .insert(tableNames.map((tableName) => ({ name: tableName }))); +const reserveTablesWithPrefix = async (tableNamePrefix) => { + const reservedTables = (await getReservedTables()) || []; -/** - * Create the DB table 'strapi_reserved_table_names' if it does not exist - * @param {SchemaBuilder} connection to the strapi db - */ -const createReservedTable = async (connection) => { - if (await connection.hasTable(reservedTableName)) { + const tableNames = await findTablesThatStartWithPrefix(tableNamePrefix); + + if (!tableNames.length) { return; } - return connection.createTable(reservedTableName, (table) => { - table.increments('id'); - table.string('name'); + + reservedTables.push(...tableNames.filter((name) => !reservedTables.includes(name))); + + await strapi.store.set({ + type: 'core', + key: 'reserved_tables', + value: reservedTables, }); }; module.exports = { + reserveTablesWithPrefix, findTablesThatStartWithPrefix, - doesReservedTableEntryExist, - createReservedTableEntries, - createReservedTable, }; diff --git a/packages/core/database/lib/schema/__tests__/schema-diff.test.js b/packages/core/database/lib/schema/__tests__/schema-diff.test.js index 8b68f90b90..c01812ecf3 100644 --- a/packages/core/database/lib/schema/__tests__/schema-diff.test.js +++ b/packages/core/database/lib/schema/__tests__/schema-diff.test.js @@ -14,9 +14,15 @@ describe('diffSchemas', () => { }); diffSchemas = schemaDiff.diff.bind(schemaDiff); + + global.strapi = { + store: { + get: () => [], + }, + }; }); - test('New Table', () => { + test('New Table', async () => { const testTable = { name: 'my_table', }; @@ -29,7 +35,7 @@ describe('diffSchemas', () => { tables: [testTable], }; - expect(diffSchemas(srcSchema, destSchema)).toStrictEqual({ + expect(await diffSchemas(srcSchema, destSchema)).toStrictEqual({ status: 'CHANGED', diff: { tables: { @@ -42,7 +48,7 @@ describe('diffSchemas', () => { }); }); - test('Removed Table', () => { + test('Removed Table', async () => { const testTable = { name: 'my_table', }; @@ -55,7 +61,7 @@ describe('diffSchemas', () => { tables: [], }; - expect(diffSchemas(srcSchema, destSchema)).toStrictEqual({ + expect(await diffSchemas(srcSchema, destSchema)).toStrictEqual({ status: 'CHANGED', diff: { tables: { @@ -68,7 +74,7 @@ describe('diffSchemas', () => { }); }); - test('Unchanged Table', () => { + test('Unchanged Table', async () => { const testTable = { name: 'my_table', columns: [], @@ -84,7 +90,7 @@ describe('diffSchemas', () => { tables: [testTable], }; - expect(diffSchemas(srcSchema, destSchema)).toStrictEqual({ + expect(await diffSchemas(srcSchema, destSchema)).toStrictEqual({ status: 'UNCHANGED', diff: { tables: { @@ -98,7 +104,7 @@ describe('diffSchemas', () => { }); describe('Changed table', () => { - test('added column', () => { + test('added column', async () => { const srcSchema = { tables: [ { @@ -125,7 +131,7 @@ describe('diffSchemas', () => { ], }; - expect(diffSchemas(srcSchema, destSchema)).toStrictEqual({ + expect(await diffSchemas(srcSchema, destSchema)).toStrictEqual({ status: 'CHANGED', diff: { tables: { @@ -178,4 +184,48 @@ describe('diffSchemas', () => { test.todo('unchanged foreign key'); test.todo('removed foreign key'); }); + + test('With persisted DB tables', async () => { + const testTables = [ + { + name: 'my_table', + }, + { + name: 'my_table_1', + }, + ]; + + const coreStoreTable = { + name: 'strapi_core_store_settings', + columns: [], + indexes: [], + foreignKeys: [], + }; + + global.strapi = { + store: { + get: async () => [testTables[0].name, 'table2'], + }, + }; + + const srcSchema = { + tables: [...testTables, coreStoreTable], + }; + + const destSchema = { + tables: [coreStoreTable], + }; + + expect(await diffSchemas(srcSchema, destSchema)).toStrictEqual({ + status: 'CHANGED', + diff: { + tables: { + added: [], + updated: [], + unchanged: [coreStoreTable], + removed: [testTables[1]], + }, + }, + }); + }); }); diff --git a/packages/core/database/lib/schema/diff.js b/packages/core/database/lib/schema/diff.js index 7fe2ceb9f9..20e8b79900 100644 --- a/packages/core/database/lib/schema/diff.js +++ b/packages/core/database/lib/schema/diff.js @@ -2,11 +2,7 @@ const _ = require('lodash/fp'); -const RESERVED_TABLE_NAMES = [ - 'strapi_migrations', - 'strapi_database_schema', - 'strapi_reserved_table_names', -]; +const RESERVED_TABLE_NAMES = ['strapi_migrations', 'strapi_database_schema']; const statuses = { CHANGED: 'CHANGED', @@ -348,18 +344,18 @@ module.exports = (db) => { } } - const reservedTablesFromDB = await db - .getConnection() - .select('name') - .from('strapi_reserved_table_names'); - - const reservedTables = [ - ...RESERVED_TABLE_NAMES, - ...reservedTablesFromDB.map((entry) => entry.name), - ]; + const persistedTables = helpers.hasTable(srcSchema, 'strapi_core_store_settings') + ? await strapi.store.get({ + type: 'core', + key: 'reserved_tables', + }) + : []; for (const srcTable of srcSchema.tables) { - if (!helpers.hasTable(destSchema, srcTable.name) && !reservedTables.includes(srcTable.name)) { + if ( + !helpers.hasTable(destSchema, srcTable.name) && + ![...RESERVED_TABLE_NAMES, ...persistedTables].includes(srcTable.name) + ) { removedTables.push(srcTable); } } diff --git a/packages/core/database/lib/schema/index.js b/packages/core/database/lib/schema/index.js index b439dbb05b..a1dca13b23 100644 --- a/packages/core/database/lib/schema/index.js +++ b/packages/core/database/lib/schema/index.js @@ -50,7 +50,7 @@ const createSchemaProvider = (db) => { const DBSchema = await db.dialect.schemaInspector.getSchema(); - const { status, diff } = this.schemaDiff.diff(DBSchema, schema); + const { status, diff } = await this.schemaDiff.diff(DBSchema, schema); if (status === 'CHANGED') { await this.builder.updateSchema(diff); From 1ae35507fecdef72097eb39a082916668f355daa Mon Sep 17 00:00:00 2001 From: Jamie Howard Date: Thu, 16 Mar 2023 12:02:07 +0000 Subject: [PATCH 4/7] chore(ee): revert review-workflows bootstrap stage --- packages/core/admin/ee/server/bootstrap.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/core/admin/ee/server/bootstrap.js b/packages/core/admin/ee/server/bootstrap.js index 8800903f82..209f598450 100644 --- a/packages/core/admin/ee/server/bootstrap.js +++ b/packages/core/admin/ee/server/bootstrap.js @@ -20,10 +20,6 @@ module.exports = async () => { await actionProvider.registerMany(actions.auditLogs); } - if (features.isEnabled('review-workflows')) { - await reserveTablesWithPrefix('strapi_workflows'); - } - await getService('seat-enforcement').seatEnforcementWorkflow(); await executeCEBootstrap(); From b17dd815565836a26cefad51f04b926e31fbc731 Mon Sep 17 00:00:00 2001 From: Jamie Howard Date: Fri, 17 Mar 2023 13:31:55 +0000 Subject: [PATCH 5/7] chore: improve naming of persisted tables --- packages/core/admin/ee/server/bootstrap.js | 4 ++-- ...reserved-tables.js => persisted-tables.js} | 22 +++++++++---------- packages/core/database/lib/schema/diff.js | 6 ++--- 3 files changed, 16 insertions(+), 16 deletions(-) rename packages/core/admin/ee/server/utils/{reserved-tables.js => persisted-tables.js} (62%) diff --git a/packages/core/admin/ee/server/bootstrap.js b/packages/core/admin/ee/server/bootstrap.js index 209f598450..657d50e3d5 100644 --- a/packages/core/admin/ee/server/bootstrap.js +++ b/packages/core/admin/ee/server/bootstrap.js @@ -5,7 +5,7 @@ const { features } = require('@strapi/strapi/lib/utils/ee'); const executeCEBootstrap = require('../../server/bootstrap'); const { getService } = require('../../server/utils'); const actions = require('./config/admin-actions'); -const { reserveTablesWithPrefix } = require('./utils/reserved-tables'); +const { persistTablesWithPrefix } = require('./utils/persisted-tables'); module.exports = async () => { const { actionProvider } = getService('permission'); @@ -15,7 +15,7 @@ module.exports = async () => { } if (features.isEnabled('audit-logs')) { - await reserveTablesWithPrefix('strapi_audit_logs'); + await persistTablesWithPrefix('strapi_audit_logs'); await actionProvider.registerMany(actions.auditLogs); } diff --git a/packages/core/admin/ee/server/utils/reserved-tables.js b/packages/core/admin/ee/server/utils/persisted-tables.js similarity index 62% rename from packages/core/admin/ee/server/utils/reserved-tables.js rename to packages/core/admin/ee/server/utils/persisted-tables.js index 65444d1153..d35b0ca874 100644 --- a/packages/core/admin/ee/server/utils/reserved-tables.js +++ b/packages/core/admin/ee/server/utils/persisted-tables.js @@ -14,36 +14,36 @@ const findTablesThatStartWithPrefix = async (prefix) => { * Get all reserved table names from the core store * @returns {Array} */ -const getReservedTables = async () => +const getPersistedTables = async () => strapi.store.get({ type: 'core', - key: 'reserved_tables', - }); + key: 'persisted_tables', + }) ?? []; /** * Add all table names that start with a prefix to the reserved tables in * core store * @param {string} tableNamePrefix */ -const reserveTablesWithPrefix = async (tableNamePrefix) => { - const reservedTables = (await getReservedTables()) || []; +const persistTablesWithPrefix = async (tableNamePrefix) => { + const persistedTables = (await getPersistedTables()) || []; const tableNames = await findTablesThatStartWithPrefix(tableNamePrefix); + const notReservedTableNames = tableNames.filter((name) => !persistedTables.includes(name)); - if (!tableNames.length) { + if (!notReservedTableNames.length) { return; } - reservedTables.push(...tableNames.filter((name) => !reservedTables.includes(name))); - + persistedTables.push(...notReservedTableNames); await strapi.store.set({ type: 'core', - key: 'reserved_tables', - value: reservedTables, + key: 'persisted_tables', + value: persistedTables, }); }; module.exports = { - reserveTablesWithPrefix, + persistTablesWithPrefix, findTablesThatStartWithPrefix, }; diff --git a/packages/core/database/lib/schema/diff.js b/packages/core/database/lib/schema/diff.js index 20e8b79900..96f7018621 100644 --- a/packages/core/database/lib/schema/diff.js +++ b/packages/core/database/lib/schema/diff.js @@ -345,10 +345,10 @@ module.exports = (db) => { } const persistedTables = helpers.hasTable(srcSchema, 'strapi_core_store_settings') - ? await strapi.store.get({ + ? (await strapi.store.get({ type: 'core', - key: 'reserved_tables', - }) + key: 'persisted_tables', + })) ?? [] : []; for (const srcTable of srcSchema.tables) { From 2c7906cdd010a6a92117cde02d042644b5ebaf2e Mon Sep 17 00:00:00 2001 From: Jamie Howard Date: Fri, 17 Mar 2023 14:32:45 +0000 Subject: [PATCH 6/7] fix(ee): correct await --- packages/core/admin/ee/server/utils/persisted-tables.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core/admin/ee/server/utils/persisted-tables.js b/packages/core/admin/ee/server/utils/persisted-tables.js index d35b0ca874..db22f0c20c 100644 --- a/packages/core/admin/ee/server/utils/persisted-tables.js +++ b/packages/core/admin/ee/server/utils/persisted-tables.js @@ -15,10 +15,10 @@ const findTablesThatStartWithPrefix = async (prefix) => { * @returns {Array} */ const getPersistedTables = async () => - strapi.store.get({ + (await strapi.store.get({ type: 'core', key: 'persisted_tables', - }) ?? []; + })) ?? []; /** * Add all table names that start with a prefix to the reserved tables in From 04723bf727f4093091d7e93183917e6a7c87fed1 Mon Sep 17 00:00:00 2001 From: Jamie Howard Date: Mon, 20 Mar 2023 11:01:17 +0000 Subject: [PATCH 7/7] chore(ee): PR feedback --- packages/core/admin/ee/server/utils/persisted-tables.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/admin/ee/server/utils/persisted-tables.js b/packages/core/admin/ee/server/utils/persisted-tables.js index db22f0c20c..f811e9a4b8 100644 --- a/packages/core/admin/ee/server/utils/persisted-tables.js +++ b/packages/core/admin/ee/server/utils/persisted-tables.js @@ -27,7 +27,7 @@ const getPersistedTables = async () => */ const persistTablesWithPrefix = async (tableNamePrefix) => { - const persistedTables = (await getPersistedTables()) || []; + const persistedTables = await getPersistedTables(); const tableNames = await findTablesThatStartWithPrefix(tableNamePrefix); const notReservedTableNames = tableNames.filter((name) => !persistedTables.includes(name));