Infrastructure/ghost
0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-02-10 23:36:14 -05:00
Code Issues Activity

Cleaned up DB connection fallback in migrations commands

Browse source 
- throughout the migration utils we use the passed in DB connection or
  fallback to the `db.knex` instance
- this works, but it means we have places we need to make sure to
  implement `(transaction || db.knex)` everywhere
- as I'm working on refactoring the utils, this was also causing
  problems because I'd be checking the `transaction` instance but that may
  be null/undefined
- this commit pulls the fallback into the function parameters where it's
  evaluated at runtime
- this also has the added benefit that we get jsdoc typing now because
  the types are easy to figure out
- note: `transaction` should probably be renamed to `connection` because
  it's not necessary a transaction... but baby steps 🤓
This commit is contained in:
Daniel Lockyer 2022-03-02 13:03:56 +01:00
parent 11f64e91c0
commit bf45ef4a87
2 changed files with 39 additions and 41 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
core/server/data/schema/clients/mysql.js
View file

@ -1,8 +1,8 @@
const _ = require('lodash'); const _ = require('lodash');
const db = require('../../../data/db'); const db = require('../../../data/db');
const doRawAndFlatten = function doRaw(query, transaction, flattenFn) { const doRawAndFlatten = function doRaw(query, transaction = db.knex, flattenFn) {
return (transaction || db.knex).raw(query).then(function (response) { return transaction.raw(query).then(function (response) {
return _.flatten(flattenFn(response)); return _.flatten(flattenFn(response));
}); });
}; };
@ -31,8 +31,8 @@ const getColumns = function getColumns(table, transaction) {
// a wrong datatype in schema.js some installations using mysql could have been created using the // a wrong datatype in schema.js some installations using mysql could have been created using the
// data type text instead of mediumtext. // data type text instead of mediumtext.
// For details see: https://github.com/TryGhost/Ghost/issues/1947 // For details see: https://github.com/TryGhost/Ghost/issues/1947
const checkPostTable = function checkPostTable(transaction) { const checkPostTable = function checkPostTable(transaction = db.knex) {
return (transaction || db.knex).raw('SHOW FIELDS FROM posts where Field ="html" OR Field = "markdown"').then(function (response) { return transaction.raw('SHOW FIELDS FROM posts where Field ="html" OR Field = "markdown"').then(function (response) {
return _.flatten(_.map(response[0], function (entry) { return _.flatten(_.map(response[0], function (entry) {
if (entry.Type.toLowerCase() !== 'mediumtext') { if (entry.Type.toLowerCase() !== 'mediumtext') {
return (transaction || db.knex).raw('ALTER TABLE posts MODIFY ' + entry.Field + ' MEDIUMTEXT'); return (transaction || db.knex).raw('ALTER TABLE posts MODIFY ' + entry.Field + ' MEDIUMTEXT');

72
core/server/data/schema/commands.js
View file

@ -58,14 +58,14 @@ function addTableColumn(tableName, table, columnName, columnSpec = schema[tableN
} }
} }
function addColumn(tableName, column, transaction, columnSpec) { function addColumn(tableName, column, transaction = db.knex, columnSpec) {
return (transaction || db.knex).schema.table(tableName, function (table) { return transaction.schema.table(tableName, function (table) {
addTableColumn(tableName, table, column, columnSpec); addTableColumn(tableName, table, column, columnSpec);
}); });
} }
function dropColumn(tableName, column, transaction) { function dropColumn(tableName, column, transaction = db.knex) {
return (transaction || db.knex).schema.table(tableName, function (table) { return transaction.schema.table(tableName, function (table) {
table.dropColumn(column); table.dropColumn(column);
}); });
} }
@ -77,11 +77,11 @@ function dropColumn(tableName, column, transaction) {
* @param {string|[string]} columns - column(s) to form unique constraint with * @param {string|[string]} columns - column(s) to form unique constraint with
* @param {import('knex')} transaction - connection object containing knex reference * @param {import('knex')} transaction - connection object containing knex reference
*/ */
async function addUnique(tableName, columns, transaction) { async function addUnique(tableName, columns, transaction = db.knex) {
try { try {
logging.info(`Adding unique constraint for: ${columns} in table ${tableName}`); logging.info(`Adding unique constraint for: ${columns} in table ${tableName}`);
return await (transaction || db.knex).schema.table(tableName, function (table) { return await transaction.schema.table(tableName, function (table) {
table.unique(columns); table.unique(columns);
}); });
} catch (err) { } catch (err) {
@ -104,11 +104,11 @@ async function addUnique(tableName, columns, transaction) {
* @param {string|[string]} columns - column(s) unique constraint was formed * @param {string|[string]} columns - column(s) unique constraint was formed
* @param {import('knex')} transaction - connection object containing knex reference * @param {import('knex')} transaction - connection object containing knex reference
*/ */
async function dropUnique(tableName, columns, transaction) { async function dropUnique(tableName, columns, transaction = db.knex) {
try { try {
logging.info(`Dropping unique constraint for: ${columns} in table: ${tableName}`); logging.info(`Dropping unique constraint for: ${columns} in table: ${tableName}`);
return await (transaction || db.knex).schema.table(tableName, function (table) { return await transaction.schema.table(tableName, function (table) {
table.dropUnique(columns); table.dropUnique(columns);
}); });
} catch (err) { } catch (err) {
@ -134,9 +134,8 @@ async function dropUnique(tableName, columns, transaction) {
* @param {string} configuration.toColumn - column of the table to point the foreign key to * @param {string} configuration.toColumn - column of the table to point the foreign key to
* @param {import('knex')} configuration.transaction - connection object containing knex reference * @param {import('knex')} configuration.transaction - connection object containing knex reference
*/ */
async function hasForeignSQLite({fromTable, fromColumn, toTable, toColumn, transaction}) { async function hasForeignSQLite({fromTable, fromColumn, toTable, toColumn, transaction = db.knex}) {
const knex = (transaction || db.knex); const client = transaction.client.config.client;
const client = knex.client.config.client;
if (client !== 'sqlite3') { if (client !== 'sqlite3') {
throw new errors.InternalServerError({ throw new errors.InternalServerError({
@ -144,7 +143,7 @@ async function hasForeignSQLite({fromTable, fromColumn, toTable, toColumn, trans
}); });
} }
const foreignKeys = await knex.raw(`PRAGMA foreign_key_list('${fromTable}');`); const foreignKeys = await transaction.raw(`PRAGMA foreign_key_list('${fromTable}');`);
const hasForeignKey = foreignKeys.some(foreignKey => foreignKey.table === toTable && foreignKey.from === fromColumn && foreignKey.to === toColumn); const hasForeignKey = foreignKeys.some(foreignKey => foreignKey.table === toTable && foreignKey.from === fromColumn && foreignKey.to === toColumn);
@ -162,8 +161,8 @@ async function hasForeignSQLite({fromTable, fromColumn, toTable, toColumn, trans
* @param {Boolean} configuration.cascadeDelete - adds the "on delete cascade" option if true * @param {Boolean} configuration.cascadeDelete - adds the "on delete cascade" option if true
* @param {import('knex')} configuration.transaction - connection object containing knex reference * @param {import('knex')} configuration.transaction - connection object containing knex reference
*/ */
async function addForeign({fromTable, fromColumn, toTable, toColumn, cascadeDelete = false, transaction}) { async function addForeign({fromTable, fromColumn, toTable, toColumn, cascadeDelete = false, transaction = db.knex}) {
const isSQLite = db.knex.client.config.client === 'sqlite3'; const isSQLite = transaction.client.config.client === 'sqlite3';
if (isSQLite) { if (isSQLite) {
const foreignKeyExists = await hasForeignSQLite({fromTable, fromColumn, toTable, toColumn, transaction}); const foreignKeyExists = await hasForeignSQLite({fromTable, fromColumn, toTable, toColumn, transaction});
if (foreignKeyExists) { if (foreignKeyExists) {
@ -183,7 +182,7 @@ async function addForeign({fromTable, fromColumn, toTable, toColumn, cascadeDele
} }
} }
await (transaction || db.knex).schema.table(fromTable, function (table) { await transaction.schema.table(fromTable, function (table) {
if (cascadeDelete) { if (cascadeDelete) {
table.foreign(fromColumn).references(`${toTable}.${toColumn}`).onDelete('CASCADE'); table.foreign(fromColumn).references(`${toTable}.${toColumn}`).onDelete('CASCADE');
} else { } else {
@ -215,8 +214,8 @@ async function addForeign({fromTable, fromColumn, toTable, toColumn, cascadeDele
* @param {string} configuration.toColumn - column of the table to point the foreign key to * @param {string} configuration.toColumn - column of the table to point the foreign key to
* @param {import('knex')} configuration.transaction - connection object containing knex reference * @param {import('knex')} configuration.transaction - connection object containing knex reference
*/ */
async function dropForeign({fromTable, fromColumn, toTable, toColumn, transaction}) { async function dropForeign({fromTable, fromColumn, toTable, toColumn, transaction = db.knex}) {
const isSQLite = db.knex.client.config.client === 'sqlite3'; const isSQLite = transaction.client.config.client === 'sqlite3';
if (isSQLite) { if (isSQLite) {
const foreignKeyExists = await hasForeignSQLite({fromTable, fromColumn, toTable, toColumn, transaction}); const foreignKeyExists = await hasForeignSQLite({fromTable, fromColumn, toTable, toColumn, transaction});
if (!foreignKeyExists) { if (!foreignKeyExists) {
@ -236,7 +235,7 @@ async function dropForeign({fromTable, fromColumn, toTable, toColumn, transactio
} }
} }
await (transaction || db.knex).schema.table(fromTable, function (table) { await transaction.schema.table(fromTable, function (table) {
table.dropForeign(fromColumn); table.dropForeign(fromColumn);
}); });
@ -260,9 +259,8 @@ async function dropForeign({fromTable, fromColumn, toTable, toColumn, transactio
* @param {string} tableName - name of the table to check primary key constraint on * @param {string} tableName - name of the table to check primary key constraint on
* @param {import('knex')} transaction - connection object containing knex reference * @param {import('knex')} transaction - connection object containing knex reference
*/ */
async function hasPrimaryKeySQLite(tableName, transaction) { async function hasPrimaryKeySQLite(tableName, transaction = db.knex) {
const knex = (transaction || db.knex); const client = transaction.client.config.client;
const client = knex.client.config.client;
if (client !== 'sqlite3') { if (client !== 'sqlite3') {
throw new errors.InternalServerError({ throw new errors.InternalServerError({
@ -270,7 +268,7 @@ async function hasPrimaryKeySQLite(tableName, transaction) {
}); });
} }
const rawConstraints = await knex.raw(`PRAGMA index_list('${tableName}');`); const rawConstraints = await transaction.raw(`PRAGMA index_list('${tableName}');`);
const tablePrimaryKey = rawConstraints.find(c => c.origin === 'pk'); const tablePrimaryKey = rawConstraints.find(c => c.origin === 'pk');
return tablePrimaryKey; return tablePrimaryKey;
@ -283,8 +281,8 @@ async function hasPrimaryKeySQLite(tableName, transaction) {
* @param {string|[string]} columns - column(s) to form primary key constraint with * @param {string|[string]} columns - column(s) to form primary key constraint with
* @param {import('knex')} transaction - connection object containing knex reference * @param {import('knex')} transaction - connection object containing knex reference
*/ */
async function addPrimaryKey(tableName, columns, transaction) { async function addPrimaryKey(tableName, columns, transaction = db.knex) {
const isSQLite = db.knex.client.config.client === 'sqlite3'; const isSQLite = transaction.client.config.client === 'sqlite3';
if (isSQLite) { if (isSQLite) {
const primaryKeyExists = await hasPrimaryKeySQLite(tableName, transaction); const primaryKeyExists = await hasPrimaryKeySQLite(tableName, transaction);
if (primaryKeyExists) { if (primaryKeyExists) {
@ -294,7 +292,7 @@ async function addPrimaryKey(tableName, columns, transaction) {
} }
try { try {
logging.info(`Adding primary key constraint for: ${columns} in table ${tableName}`); logging.info(`Adding primary key constraint for: ${columns} in table ${tableName}`);
return await (transaction || db.knex).schema.table(tableName, function (table) { return await transaction.schema.table(tableName, function (table) {
table.primary(columns); table.primary(columns);
}); });
} catch (err) { } catch (err) {
@ -316,8 +314,8 @@ async function addPrimaryKey(tableName, columns, transaction) {
* @param {import('knex').Transaction} transaction - connection to the DB * @param {import('knex').Transaction} transaction - connection to the DB
* @param {Object} [tableSpec] - table schema to generate table with * @param {Object} [tableSpec] - table schema to generate table with
*/ */
function createTable(table, transaction, tableSpec = schema[table]) { function createTable(table, transaction = db.knex, tableSpec = schema[table]) {
return (transaction || db.knex).schema.createTable(table, function (t) { return transaction.schema.createTable(table, function (t) {
Object.keys(tableSpec) Object.keys(tableSpec)
.filter(column => !(column.startsWith('@@'))) .filter(column => !(column.startsWith('@@')))
.forEach(column => addTableColumn(table, t, column, tableSpec[column])); .forEach(column => addTableColumn(table, t, column, tableSpec[column]));
@ -331,12 +329,12 @@ function createTable(table, transaction, tableSpec = schema[table]) {
}); });
} }
function deleteTable(table, transaction) { function deleteTable(table, transaction = db.knex) {
return (transaction || db.knex).schema.dropTableIfExists(table); return transaction.schema.dropTableIfExists(table);
} }
function getTables(transaction) { function getTables(transaction = db.knex) {
const client = (transaction || db.knex).client.config.client; const client = transaction.client.config.client;
if (_.includes(_.keys(clients), client)) { if (_.includes(_.keys(clients), client)) {
return clients[client].getTables(transaction); return clients[client].getTables(transaction);
@ -345,8 +343,8 @@ function getTables(transaction) {
return Promise.reject(tpl(messages.noSupportForDatabase, {client: client})); return Promise.reject(tpl(messages.noSupportForDatabase, {client: client}));
} }
function getIndexes(table, transaction) { function getIndexes(table, transaction = db.knex) {
const client = (transaction || db.knex).client.config.client; const client = transaction.client.config.client;
if (_.includes(_.keys(clients), client)) { if (_.includes(_.keys(clients), client)) {
return clients[client].getIndexes(table, transaction); return clients[client].getIndexes(table, transaction);
@ -355,8 +353,8 @@ function getIndexes(table, transaction) {
return Promise.reject(tpl(messages.noSupportForDatabase, {client: client})); return Promise.reject(tpl(messages.noSupportForDatabase, {client: client}));
} }
function getColumns(table, transaction) { function getColumns(table, transaction = db.knex) {
const client = (transaction || db.knex).client.config.client; const client = transaction.client.config.client;
if (_.includes(_.keys(clients), client)) { if (_.includes(_.keys(clients), client)) {
return clients[client].getColumns(table); return clients[client].getColumns(table);
@ -365,8 +363,8 @@ function getColumns(table, transaction) {
return Promise.reject(tpl(messages.noSupportForDatabase, {client: client})); return Promise.reject(tpl(messages.noSupportForDatabase, {client: client}));
} }
function checkTables(transaction) { function checkTables(transaction = db.knex) {
const client = (transaction || db.knex).client.config.client; const client = transaction.client.config.client;
if (client === 'mysql') { if (client === 'mysql') {
return clients[client].checkPostTable(); return clients[client].checkPostTable();