Skip to content

Commit

Permalink
Merge pull request #8314 from strapi/fix/dpMigration
Browse files Browse the repository at this point in the history
fix d&p regressions
  • Loading branch information
alexandrebodin committed Oct 20, 2020
2 parents b1aa7b9 + d84284f commit 81c684d
Show file tree
Hide file tree
Showing 7 changed files with 238 additions and 111 deletions.
199 changes: 103 additions & 96 deletions packages/strapi-connector-bookshelf/lib/build-database-schema.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,22 @@ const _ = require('lodash');
const { singular } = require('pluralize');
const { contentTypes: contentTypesUtils } = require('strapi-utils');

const { storeDefinition, didDefinitionChange } = require('./utils/store-definition');
const { getDraftAndPublishMigrationWay, migrateDraftAndPublish } = require('./database-migration');
const { storeDefinition, getColumnsWhereDefinitionChanged } = require('./utils/store-definition');
const { migrateDraftAndPublish } = require('./database-migration');
const { getManyRelations } = require('./utils/associations');

module.exports = async ({ ORM, loadedModel, definition, connection, model }) => {
const definitionDidChange = await didDefinitionChange(definition, ORM);
if (!definitionDidChange) {
return;
}
// Update/create the tables in database
await createOrUpdateTables({ ORM, loadedModel, definition, connection, model });

const draftAndPublishMigrationWay = await getDraftAndPublishMigrationWay({ definition, ORM });
if (draftAndPublishMigrationWay === 'disable') {
await migrateDraftAndPublish({ definition, ORM, way: 'disable' });
}
// migrations
await migrateDraftAndPublish({ definition, ORM });

// store new definitions
await storeDefinition(definition, ORM);
};

const createOrUpdateTables = async ({ ORM, loadedModel, definition, connection, model }) => {
// Add created_at and updated_at field if timestamp option is true
if (loadedModel.hasTimestamps) {
definition.attributes[loadedModel.hasTimestamps[0]] = { type: 'currentTimestamp' };
Expand Down Expand Up @@ -59,9 +61,7 @@ module.exports = async ({ ORM, loadedModel, definition, connection, model }) =>
}

// Equilize many to many relations
const manyRelations = definition.associations.filter(({ nature }) =>
['manyToMany', 'manyWay'].includes(nature)
);
const manyRelations = getManyRelations(definition);

for (const manyRelation of manyRelations) {
const { plugin, collection, via, dominant, alias } = manyRelation;
Expand Down Expand Up @@ -103,12 +103,15 @@ module.exports = async ({ ORM, loadedModel, definition, connection, model }) =>
delete definition.attributes[loadedModel.hasTimestamps[0]];
delete definition.attributes[loadedModel.hasTimestamps[1]];
}
};

if (draftAndPublishMigrationWay === 'enable') {
await migrateDraftAndPublish({ definition, ORM, way: 'enable' });
}
const getColumnInfo = async (columnName, tableName, ORM) => {
const exists = await ORM.knex.schema.hasColumn(tableName, columnName);

await storeDefinition(definition, ORM);
return {
columnName,
exists,
};
};

const isColumn = ({ definition, attribute, name }) => {
Expand Down Expand Up @@ -278,23 +281,15 @@ const createOrUpdateTable = async ({ table, attributes, definition, ORM, model }
return;
}

const columns = Object.keys(attributes);
const attributesNames = Object.keys(attributes);

// Fetch existing column
const columnsExist = await Promise.all(
columns.map(attribute => ORM.knex.schema.hasColumn(table, attribute))
const columnsInfo = await Promise.all(
attributesNames.map(attributeName => getColumnInfo(attributeName, table, ORM))
);
const nameOfColumnsToAdd = columnsInfo.filter(info => !info.exists).map(info => info.columnName);

const columnsToAdd = {};

// Get columns to add
columnsExist.forEach((columnExist, index) => {
const attribute = attributes[columns[index]];

if (!columnExist) {
columnsToAdd[columns[index]] = attribute;
}
});
const columnsToAdd = _.pick(attributes, nameOfColumnsToAdd);

// Generate and execute query to add missing column
if (Object.keys(columnsToAdd).length > 0) {
Expand All @@ -303,83 +298,95 @@ const createOrUpdateTable = async ({ table, attributes, definition, ORM, model }
});
}

if (definition.client === 'sqlite3') {
const tmpTable = `tmp_${table}`;

const rebuildTable = async trx => {
await trx.schema.renameTable(table, tmpTable);
const attrsNameWithoutTimestamps = attributesNames.filter(
columnName => !(definition.options.timestamps || []).includes(columnName)
);

// drop possible conflicting indexes
await Promise.all(
columns.map(key => trx.raw('DROP INDEX IF EXISTS ??', uniqueColName(table, key)))
);
const columnsToAlter = getColumnsWhereDefinitionChanged(
attrsNameWithoutTimestamps,
definition,
ORM
);

// create the table
await createTable(table, { trx });
if (columnsToAlter.length > 0) {
if (definition.client === 'sqlite3') {
const tmpTable = `tmp_${table}`;

const attrs = Object.keys(attributes).filter(attribute =>
isColumn({
definition,
attribute: attributes[attribute],
name: attribute,
})
);
const rebuildTable = async trx => {
await trx.schema.renameTable(table, tmpTable);

const allAttrs = ['id', ...attrs];
// drop possible conflicting indexes
await Promise.all(
attributesNames.map(key => trx.raw('DROP INDEX IF EXISTS ??', uniqueColName(table, key)))
);

await trx.insert(qb => qb.select(allAttrs).from(tmpTable)).into(table);
await trx.schema.dropTableIfExists(tmpTable);
};
// create the table
await createTable(table, { trx });

try {
await ORM.knex.transaction(trx => rebuildTable(trx));
} catch (err) {
if (err.message.includes('UNIQUE constraint failed')) {
strapi.log.error(
`Unique constraint fails, make sure to update your data and restart to apply the unique constraint.\n\t- ${err.stack}`
const attrs = attributesNames.filter(attributeName =>
isColumn({
definition,
attribute: attributes[attributeName],
name: attributeName,
})
);
} else {
strapi.log.error(`Migration failed`);
strapi.log.error(err);
}

return false;
}
} else {
const alterTable = async trx => {
await Promise.all(
columns.map(col => {
return ORM.knex.schema
.alterTable(table, tbl => {
tbl.dropUnique(col, uniqueColName(table, col));
})
.catch(() => {});
})
);
await trx.schema.alterTable(table, tbl => {
alterColumns(tbl, _.pick(attributes, columns), {
tableExists,
});
});
};
const allAttrs = ['id', ...attrs];

try {
await ORM.knex.transaction(trx => alterTable(trx));
} catch (err) {
if (err.code === '23505' && definition.client === 'pg') {
strapi.log.error(
`Unique constraint fails, make sure to update your data and restart to apply the unique constraint.\n\t- ${err.message}\n\t- ${err.detail}`
);
} else if (definition.client === 'mysql' && err.errno === 1062) {
strapi.log.error(
`Unique constraint fails, make sure to update your data and restart to apply the unique constraint.\n\t- ${err.sqlMessage}`
);
} else {
strapi.log.error(`Migration failed`);
strapi.log.error(err);
await trx.insert(qb => qb.select(allAttrs).from(tmpTable)).into(table);
await trx.schema.dropTableIfExists(tmpTable);
};

try {
await ORM.knex.transaction(trx => rebuildTable(trx));
} catch (err) {
if (err.message.includes('UNIQUE constraint failed')) {
strapi.log.error(
`Unique constraint fails, make sure to update your data and restart to apply the unique constraint.\n\t- ${err.stack}`
);
} else {
strapi.log.error(`Migration failed`);
strapi.log.error(err);
}

return false;
}
} else {
const alterTable = async trx => {
await Promise.all(
columnsToAlter.map(col => {
return ORM.knex.schema
.alterTable(table, tbl => {
tbl.dropUnique(col, uniqueColName(table, col));
})
.catch(() => {});
})
);
await trx.schema.alterTable(table, tbl => {
alterColumns(tbl, _.pick(attributes, columnsToAlter), {
tableExists,
});
});
};

try {
await ORM.knex.transaction(trx => alterTable(trx));
} catch (err) {
if (err.code === '23505' && definition.client === 'pg') {
strapi.log.error(
`Unique constraint fails, make sure to update your data and restart to apply the unique constraint.\n\t- ${err.message}\n\t- ${err.detail}`
);
} else if (definition.client === 'mysql' && err.errno === 1062) {
strapi.log.error(
`Unique constraint fails, make sure to update your data and restart to apply the unique constraint.\n\t- ${err.sqlMessage}`
);
} else {
strapi.log.error(`Migration failed`);
strapi.log.error(err);
}

return false;
return false;
}
}
}
};
9 changes: 4 additions & 5 deletions packages/strapi-connector-bookshelf/lib/database-migration.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ const getDraftAndPublishMigrationWay = async ({ definition, ORM }) => {
}
};

const migrateDraftAndPublish = async ({ definition, ORM, way }) => {
const migrateDraftAndPublish = async ({ definition, ORM }) => {
const way = await getDraftAndPublishMigrationWay({ definition, ORM });

if (way === 'enable') {
const now = new Date();
let publishedAtValue = now;
Expand All @@ -42,13 +44,11 @@ const migrateDraftAndPublish = async ({ definition, ORM, way }) => {
.delete()
.where(PUBLISHED_AT_ATTRIBUTE, null);

// column are automatically deleted in sqlite because the table is recreated
// for other databases, we need to do it ourselves
const publishedAtColumnExists = await ORM.knex.schema.hasColumn(
definition.collectionName,
PUBLISHED_AT_ATTRIBUTE
);
if (definition.client !== 'sqlite3' && publishedAtColumnExists) {
if (publishedAtColumnExists) {
await ORM.knex.schema.table(definition.collectionName, table => {
table.dropColumn(PUBLISHED_AT_ATTRIBUTE);
});
Expand All @@ -57,6 +57,5 @@ const migrateDraftAndPublish = async ({ definition, ORM, way }) => {
};

module.exports = {
getDraftAndPublishMigrationWay,
migrateDraftAndPublish,
};
6 changes: 3 additions & 3 deletions packages/strapi-connector-bookshelf/lib/mount-models.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,11 @@ module.exports = async ({ models, target }, ctx, { selfFinalize = false } = {})

target[model].allAttributes = { ...definition.attributes };

const createAtCol = _.get(definition, 'options.timestamps.0', 'created_at');
const createdAtCol = _.get(definition, 'options.timestamps.0', 'created_at');
const updatedAtCol = _.get(definition, 'options.timestamps.1', 'updated_at');
if (_.get(definition, 'options.timestamps', false)) {
_.set(definition, 'options.timestamps', [createAtCol, updatedAtCol]);
target[model].allAttributes[createAtCol] = { type: 'timestamp' };
_.set(definition, 'options.timestamps', [createdAtCol, updatedAtCol]);
target[model].allAttributes[createdAtCol] = { type: 'timestamp' };
target[model].allAttributes[updatedAtCol] = { type: 'timestamp' };
} else {
_.set(definition, 'options.timestamps', false);
Expand Down
5 changes: 5 additions & 0 deletions packages/strapi-connector-bookshelf/lib/utils/associations.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@ const isPolymorphic = ({ assoc }) => {
return assoc.nature.toLowerCase().indexOf('morph') !== -1;
};

const getManyRelations = definition => {
return definition.associations.filter(({ nature }) => ['manyToMany', 'manyWay'].includes(nature));
};

module.exports = {
isPolymorphic,
getManyRelations,
};
16 changes: 10 additions & 6 deletions packages/strapi-connector-bookshelf/lib/utils/store-definition.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,16 +38,20 @@ const storeDefinition = async (definition, ORM) => {
return strapi.models['core_store'].forge(defData).save();
};

const didDefinitionChange = async (definition, ORM) => {
const previousDefRow = await getDefinitionFromStore(definition, ORM);
const previousDefJSON = _.get(previousDefRow, 'value', null);
const actualDefJSON = formatDefinitionToStore(definition);
const getColumnsWhereDefinitionChanged = async (columnsName, definition, ORM) => {
const previousDefinitionRow = await getDefinitionFromStore(definition, ORM);
const previousDefinition = JSON.parse(_.get(previousDefinitionRow, 'value', null));

return previousDefJSON !== actualDefJSON;
return columnsName.filter(columnName => {
const previousAttribute = _.get(previousDefinition, ['attributes', columnName], null);
const actualAttribute = _.get(definition, ['attributes', columnName], null);

return !_.isEqual(previousAttribute, actualAttribute);
});
};

module.exports = {
storeDefinition,
didDefinitionChange,
getDefinitionFromStore,
getColumnsWhereDefinitionChanged,
};
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const getDraftAndPublishMigrationWay = async (definition, ORM) => {
const previousDraftAndPublish = contentTypesUtils.hasDraftAndPublish(previousDef);
const actualDraftAndPublish = contentTypesUtils.hasDraftAndPublish(definition);

if (previousDraftAndPublish === actualDraftAndPublish) {
if (!previousDefRow || previousDraftAndPublish === actualDraftAndPublish) {
return 'none';
}
if (!previousDraftAndPublish && actualDraftAndPublish) {
Expand Down

0 comments on commit 81c684d

Please sign in to comment.