From 94f5add1c0f19edf7f211df80faf28bb7fe996b5 Mon Sep 17 00:00:00 2001 From: Daniel Lockyer Date: Tue, 8 Feb 2022 09:04:09 +0100 Subject: [PATCH] Refactored default settings population to reduce unnecessary DB queries refs https://github.com/TryGhost/Toolbox/issues/202 - this code suffers from two problems: - when we don't have any new settings to insert, we still end up fetching the columnInfo and owner info, even though we only need them if we're inserting data. This results in 3 extra queries upon boot - secondly, we insert every setting with a separate query - MySQL and SQLite both support batch inserts and knex has a utility to help us that I've [used before](https://github.com/TryGhost/knex-migrator/commit/38821c52426f57a502b3ac859a2970bfa0b1e08f). With 95 settings at the time of writing, this adds 94 extra queries during the DB init - this commit refactors the code so that we only fetch the columnInfo and owner data if we've got new settings to insert, and batches the inserts using knex's batchInsert util - this query results in ~95 less queries during DB init and saves a couple of queries during boot --- core/server/models/settings.js | 102 ++++++++++++++++----------------- 1 file changed, 50 insertions(+), 52 deletions(-) diff --git a/core/server/models/settings.js b/core/server/models/settings.js index f10254be91..fe8c03b2fe 100644 --- a/core/server/models/settings.js +++ b/core/server/models/settings.js @@ -260,62 +260,60 @@ Settings = ghostBookshelf.Model.extend({ await ghostBookshelf.knex.destroy(); await ghostBookshelf.knex.initialize(); - // fetch available columns to avoid populating columns not yet created by migrations - const columnInfo = await ghostBookshelf.knex.table('settings').columnInfo(); - const columns = Object.keys(columnInfo); + const allSettings = await this.findAll(options); - // fetch other data that is used when inserting new settings - const date = ghostBookshelf.knex.raw('CURRENT_TIMESTAMP'); - let owner; - try { - owner = await ghostBookshelf.model('User').getOwnerUser(); - } catch (e) { - // in some tests the owner is deleted and not recreated before setup - if (e.errorType === 'NotFoundError') { - owner = {id: 1}; - } else { - throw e; + const usedKeys = allSettings.models.map(function mapper(setting) { + return setting.get('key'); + }); + + const settingsToInsert = []; + + _.each(getDefaultSettings(), function forEachDefault(defaultSetting, defaultSettingKey) { + const isMissingFromDB = usedKeys.indexOf(defaultSettingKey) === -1; + if (isMissingFromDB) { + defaultSetting.value = defaultSetting.getDefaultValue(); + settingsToInsert.push(defaultSetting); } + }); + + if (settingsToInsert.length > 0) { + // fetch available columns to avoid populating columns not yet created by migrations + const columnInfo = await ghostBookshelf.knex.table('settings').columnInfo(); + const columns = Object.keys(columnInfo); + + // fetch other data that is used when inserting new settings + const date = ghostBookshelf.knex.raw('CURRENT_TIMESTAMP'); + let owner; + try { + owner = await ghostBookshelf.model('User').getOwnerUser(); + } catch (e) { + // in some tests the owner is deleted and not recreated before setup + if (e.errorType === 'NotFoundError') { + owner = {id: 1}; + } else { + throw e; + } + } + + const settingsDataToInsert = settingsToInsert.map((setting) => { + const settingValues = Object.assign({}, setting, { + id: ObjectID().toHexString(), + created_at: date, + created_by: owner.id, + updated_at: date, + updated_by: owner.id + }); + + return _.pick(settingValues, columns); + }); + + await ghostBookshelf.knex + .batchInsert('settings', settingsDataToInsert); + + return self.findAll(options); } - return this - .findAll(options) - .then(function checkAllSettings(allSettings) { - const usedKeys = allSettings.models.map(function mapper(setting) { - return setting.get('key'); - }); - - const insertOperations = []; - - _.each(getDefaultSettings(), function forEachDefault(defaultSetting, defaultSettingKey) { - const isMissingFromDB = usedKeys.indexOf(defaultSettingKey) === -1; - if (isMissingFromDB) { - defaultSetting.value = defaultSetting.getDefaultValue(); - - const settingValues = Object.assign({}, defaultSetting, { - id: ObjectID().toHexString(), - created_at: date, - created_by: owner.id, - updated_at: date, - updated_by: owner.id - }); - - insertOperations.push( - ghostBookshelf.knex - .table('settings') - .insert(_.pick(settingValues, columns)) - ); - } - }); - - if (insertOperations.length > 0) { - return Promise.all(insertOperations).then(function fetchAllToReturn() { - return self.findAll(options); - }); - } - - return allSettings; - }); + return allSettings; }, permissible: function permissible(modelId, action, context, unsafeAttrs, loadedPermissions, hasUserPermission, hasApiKeyPermission) {