fix cleanOrder + use hasOrderColumn and hasInverseOrderColumn

This commit is contained in:
Pierre Noël 2022-09-27 11:56:46 +02:00
parent a56879e12b
commit 8e5543b21d
3 changed files with 139 additions and 90 deletions

View File

@ -24,7 +24,12 @@ const { createField } = require('../fields');
const { createQueryBuilder } = require('../query');
const { createRepository } = require('./entity-repository');
const { deleteRelatedMorphOneRelationsAfterMorphToManyUpdate } = require('./morph-relations');
const { isBidirectional, isManyToAny, isAnyToOne, isAnyToMany } = require('../metadata/relations');
const {
isBidirectional,
isAnyToOne,
hasOrderColumn,
hasInverseOrderColumn,
} = require('../metadata/relations');
const {
deletePreviousOneToAnyRelations,
deletePreviousAnyToOneRelations,
@ -511,7 +516,7 @@ const createEntityManager = (db) => {
const relsToAdd = cleanRelationData.set || cleanRelationData.connect;
const relIdsToadd = toIds(relsToAdd);
await deletePreviousOneToAnyRelations({ id, attribute, joinTable, relIdsToadd, db });
await deletePreviousOneToAnyRelations({ id, attribute, relIdsToadd, db });
// prepare new relations to insert
const insert = relsToAdd.map((data) => {
@ -524,13 +529,13 @@ const createEntityManager = (db) => {
});
// add order value
if (isAnyToMany(attribute)) {
if (hasOrderColumn(attribute)) {
insert.forEach((rel, idx) => {
rel[orderColumnName] = idx + 1;
});
}
// add inv_order value
if (isBidirectional(attribute) && isManyToAny(attribute)) {
if (hasInverseOrderColumn(attribute)) {
const maxResults = await db
.getConnection()
.select(inverseJoinColumn.name)
@ -715,16 +720,16 @@ const createEntityManager = (db) => {
const { joinColumn, inverseJoinColumn, orderColumnName, inverseOrderColumnName } =
joinTable;
const select = [joinColumn.name, inverseJoinColumn.name];
if (isAnyToMany(attribute)) {
if (hasOrderColumn(attribute)) {
select.push(orderColumnName);
}
if (isBidirectional(attribute) && isManyToAny(attribute)) {
if (hasInverseOrderColumn(attribute)) {
select.push(inverseOrderColumnName);
}
// only delete relations
if (isNull(cleanRelationData.set)) {
await deleteRelations({ id, attribute, joinTable, db }, { relIdsToDelete: 'all' });
await deleteRelations({ id, attribute, db, relIdsToDelete: 'all' });
} else {
const isPartialUpdate = !has('set', cleanRelationData);
let relIdsToaddOrMove;
@ -742,14 +747,11 @@ const createEntityManager = (db) => {
continue;
}
await deleteRelations({ id, attribute, joinTable, db }, { relIdsToDelete });
await deleteRelations({ id, attribute, db, relIdsToDelete });
// Fetch current relations to handle ordering
let currentMovingRels;
if (
isAnyToMany(attribute) ||
(isBidirectional(attribute) && isManyToAny(attribute))
) {
if (hasOrderColumn(attribute) || hasInverseOrderColumn(attribute)) {
currentMovingRels = await this.createQueryBuilder(joinTable.name)
.select(select)
.where({
@ -769,7 +771,7 @@ const createEntityManager = (db) => {
}));
// add order value
if (isAnyToMany(attribute)) {
if (hasOrderColumn(attribute)) {
const orderMax = (
await this.createQueryBuilder(joinTable.name)
.max(orderColumnName)
@ -785,7 +787,7 @@ const createEntityManager = (db) => {
}
// add inv order value
if (isBidirectional(attribute) && isManyToAny(attribute)) {
if (hasInverseOrderColumn(attribute)) {
const nonExistingRelsIds = difference(
relIdsToaddOrMove,
map(inverseJoinColumn.name, currentMovingRels)
@ -815,7 +817,7 @@ const createEntityManager = (db) => {
.insert(insert)
.onConflict(joinTable.pivotColumns);
if (isAnyToMany(attribute)) {
if (hasOrderColumn(attribute)) {
query.merge([orderColumnName]);
} else {
query.ignore();
@ -824,17 +826,20 @@ const createEntityManager = (db) => {
await query.execute();
// remove gap between orders
await cleanOrderColumns({ joinTable, attribute, db, id });
await cleanOrderColumns({ attribute, db, id });
} else {
if (isAnyToOne(attribute)) {
cleanRelationData.set = cleanRelationData.set.slice(-1);
}
// overwrite all relations
relIdsToaddOrMove = toIds(cleanRelationData.set);
await deleteRelations(
{ id, attribute, joinTable, db },
{ relIdsToDelete: 'all', relIdsToNotDelete: relIdsToaddOrMove }
);
await deleteRelations({
id,
attribute,
db,
relIdsToDelete: 'all',
relIdsToNotDelete: relIdsToaddOrMove,
});
if (isEmpty(cleanRelationData.set)) {
continue;
@ -848,14 +853,14 @@ const createEntityManager = (db) => {
}));
// add order value
if (isAnyToMany(attribute)) {
if (hasOrderColumn(attribute)) {
insert.forEach((row, idx) => {
row[orderColumnName] = idx + 1;
});
}
// add inv order value
if (isBidirectional(attribute) && isManyToAny(attribute)) {
if (hasInverseOrderColumn(attribute)) {
const existingRels = await this.createQueryBuilder(joinTable.name)
.select(inverseJoinColumn.name)
.where({
@ -894,7 +899,7 @@ const createEntityManager = (db) => {
.insert(insert)
.onConflict(joinTable.pivotColumns);
if (isAnyToMany(attribute)) {
if (hasOrderColumn(attribute)) {
query.merge([orderColumnName]);
} else {
query.ignore();
@ -908,7 +913,6 @@ const createEntityManager = (db) => {
await deletePreviousOneToAnyRelations({
id,
attribute,
joinTable,
relIdsToadd: relIdsToaddOrMove,
db,
});
@ -917,7 +921,6 @@ const createEntityManager = (db) => {
await deletePreviousAnyToOneRelations({
id,
attribute,
joinTable,
relIdToadd: relIdsToaddOrMove[0],
db,
});
@ -1037,9 +1040,7 @@ const createEntityManager = (db) => {
}
if (attribute.joinTable) {
const { joinTable } = attribute;
await deleteRelations({ id, attribute, joinTable, db }, { relIdsToDelete: 'all' });
await deleteRelations({ id, attribute, db, relIdsToDelete: 'all' });
}
}
},

View File

@ -6,11 +6,21 @@ const {
isOneToAny,
isManyToAny,
isAnyToOne,
isAnyToMany,
hasOrderColumn,
hasInverseOrderColumn,
} = require('../metadata/relations');
const { createQueryBuilder } = require('../query');
const deletePreviousOneToAnyRelations = async ({ id, attribute, joinTable, relIdsToadd, db }) => {
/**
* If some relations currently exist for this oneToX relation, on the one side, this function removes them and update the inverse order if needed.
* @param {Object} params
* @param {string} params.id - entity id on which the relations for entities relIdsToadd are created
* @param {string} params.attribute - attribute of the relation
* @param {string} params.inverseRelIds - entity ids of the inverse side for which the current relations will be deleted
* @param {string} params.db - database instance
*/
const deletePreviousOneToAnyRelations = async ({ id, attribute, relIdsToadd, db }) => {
const { joinTable } = attribute;
const { joinColumn, inverseJoinColumn } = joinTable;
// need to delete the previous relations for oneToAny relations
@ -25,16 +35,27 @@ const deletePreviousOneToAnyRelations = async ({ id, attribute, joinTable, relId
.where(joinTable.on || {})
.execute();
await cleanOrderColumns({ joinTable, attribute, db, inverseRelIds: relIdsToadd });
await cleanOrderColumns({ attribute, db, inverseRelIds: relIdsToadd });
}
};
const deletePreviousAnyToOneRelations = async ({ id, attribute, joinTable, relIdToadd, db }) => {
/**
* If a relation currently exists for this xToOne relations, this function removes it and update the inverse order if needed.
* @param {Object} params
* @param {string} params.id - entity id on which the relation for entity relIdToadd is created
* @param {string} params.attribute - attribute of the relation
* @param {string} params.relIdToadd - entity id of the new relation
* @param {string} params.db - database instance
*/
const deletePreviousAnyToOneRelations = async ({ id, attribute, relIdToadd, db }) => {
const { joinTable } = attribute;
const { joinColumn, inverseJoinColumn } = joinTable;
// Delete the previous relations for anyToOne relations
if (isBidirectional(attribute) && isAnyToOne(attribute)) {
// update orders for previous anyToOne relations that will be deleted if it has order (manyToOne)
// handling manyToOne
if (isManyToAny(attribute)) {
// if the database integrity was not broken relsToDelete is supposed to be of length 1
const relsToDelete = await createQueryBuilder(joinTable.name, db)
@ -48,7 +69,6 @@ const deletePreviousAnyToOneRelations = async ({ id, attribute, joinTable, relId
const relIdsToDelete = map(inverseJoinColumn.name, relsToDelete);
// delete previous anyToOne relations
await createQueryBuilder(joinTable.name, db)
.delete()
.where({
@ -58,9 +78,10 @@ const deletePreviousAnyToOneRelations = async ({ id, attribute, joinTable, relId
.where(joinTable.on || {})
.execute();
await cleanOrderColumns({ joinTable, attribute, db, inverseRelIds: relIdsToDelete });
await cleanOrderColumns({ attribute, db, inverseRelIds: relIdsToDelete });
// handling oneToOne
} else {
// delete previous anyToOne relations
await createQueryBuilder(joinTable.name, db)
.delete()
.where({
@ -73,15 +94,27 @@ const deletePreviousAnyToOneRelations = async ({ id, attribute, joinTable, relId
}
};
// INVERSE ORDER UPDATE
const deleteRelations = async (
{ id, attribute, joinTable, db },
{ relIdsToNotDelete = [], relIdsToDelete = [] }
) => {
/**
* Delete all or some relations of entity field
* @param {Object} params
* @param {string} params.id - entity id for which the relations will be deleted
* @param {string} params.attribute - attribute of the relation
* @param {string} params.db - database instance
* @param {string} params.relIdsToDelete - ids of entities to remove from the relations. Also accepts 'all'
* @param {string} params.relIdsToNotDelete - ids of entities to not remove from the relation when relIdsToDelete equals 'all'
*/
const deleteRelations = async ({
id,
attribute,
db,
relIdsToNotDelete = [],
relIdsToDelete = [],
}) => {
const { joinTable } = attribute;
const { joinColumn, inverseJoinColumn } = joinTable;
const all = relIdsToDelete === 'all';
if (isAnyToMany(attribute) || (isBidirectional(attribute) && isManyToAny(attribute))) {
if (hasOrderColumn(attribute) || hasInverseOrderColumn(attribute)) {
let lastId = 0;
let done = false;
const batchSize = 100;
@ -112,7 +145,7 @@ const deleteRelations = async (
.where(joinTable.on || {})
.execute();
await cleanOrderColumns({ joinTable, attribute, db, id, inverseRelIds: batchIds });
await cleanOrderColumns({ attribute, db, id, inverseRelIds: batchIds });
}
} else {
await createQueryBuilder(joinTable.name, db)
@ -130,69 +163,79 @@ const deleteRelations = async (
/**
* Clean the order columns by ensuring the order value are continuous (ex: 1, 2, 3 and not 1, 5, 10)
* @param {Object} params
* @param {string} params.joinTable - joinTable of the relation where the clean will be done
* @param {string} params.attribute - attribute on which the clean will be done
* @param {string} params.db - Database instance
* @param {string} params.id - Entity ID for which the clean will be done
* @param {string} params.inverseRelIds - Entity ids of the inverse side for which the clean will be done
* @param {string} params.id - entity id for which the clean will be done
* @param {string} params.attribute - attribute of the relation
* @param {string} params.db - database instance
* @param {string} params.inverseRelIds - entity ids of the inverse side for which the clean will be done
*/
const cleanOrderColumns = async ({ joinTable, attribute, db, id, inverseRelIds }) => {
const cleanOrderColumns = async ({ id, attribute, db, inverseRelIds }) => {
if (
!(isAnyToMany(attribute) && id) &&
!(isBidirectional(attribute) && isManyToAny(attribute) && !isEmpty(inverseRelIds))
!(hasOrderColumn(attribute) && id) &&
!(hasInverseOrderColumn(attribute) && !isEmpty(inverseRelIds))
) {
return;
}
const { joinTable } = attribute;
const { joinColumn, inverseJoinColumn, orderColumnName, inverseOrderColumnName } = joinTable;
const knex = db.getConnection();
const update = {};
const subQuery = knex(joinTable.name).select('id');
const update = [];
const updateBinding = [];
const select = ['??'];
const selectBinding = ['id'];
const where = [];
const whereBinding = [];
if (isAnyToMany(attribute) && id) {
update[orderColumnName] = knex.raw('t.src_order');
const on = [joinColumn.name, ...Object.keys(joinTable.on)];
subQuery
.select(
knex.raw(`ROW_NUMBER() OVER (PARTITION BY ${on.join(', ')} ORDER BY ??) AS src_order`, [
...on,
orderColumnName,
])
)
.where(joinColumn.name, id);
if (hasOrderColumn(attribute) && id) {
update.push('?? = t.src_order');
updateBinding.push(orderColumnName);
select.push('ROW_NUMBER() OVER (PARTITION BY ?? ORDER BY ??) AS src_order');
selectBinding.push(joinColumn.name, orderColumnName);
where.push('?? = ?');
whereBinding.push(joinColumn.name, id);
}
if (isBidirectional(attribute) && isManyToAny(attribute) && !isEmpty(inverseRelIds)) {
update[inverseOrderColumnName] = knex.raw('t.inv_order');
const on = [inverseJoinColumn.name, ...Object.keys(joinTable.on)];
subQuery
.select(
knex.raw(`ROW_NUMBER() OVER (PARTITION BY ${on.join(', ')} ORDER BY ??) AS inv_order`, [
...on,
inverseOrderColumnName,
])
)
.orWhereIn(inverseJoinColumn.name, inverseRelIds);
if (hasInverseOrderColumn(attribute) && !isEmpty(inverseRelIds)) {
update.push('?? = t.inv_order');
updateBinding.push(inverseOrderColumnName);
select.push('ROW_NUMBER() OVER (PARTITION BY ?? ORDER BY ??) AS inv_order');
selectBinding.push(inverseJoinColumn.name, inverseOrderColumnName);
where.push(`?? IN (${inverseRelIds.map(() => '?').join(', ')})`);
whereBinding.push(inverseJoinColumn.name, ...inverseRelIds);
}
await knex(joinTable.name)
.update(update)
.from(subQuery)
.where('t.id', knex.raw('??.id', joinTable.name));
// raw query as knex doesn't allow updating from a subquery
// https://github.com/knex/knex/issues/2504
/*
`UPDATE :joinTable:
SET :orderColumn: = t.order, :inverseOrderColumn: = t.inv_order
`UPDATE :joinTable:
SET :orderColumn: = t.src_order, :inverseOrderColumn: = t.inv_order
FROM (
SELECT
id,
ROW_NUMBER() OVER ( PARTITION BY :joinColumn: ORDER BY :orderColumn:) AS src_order,
ROW_NUMBER() OVER ( PARTITION BY :inverseJoinColumn: ORDER BY :inverseOrderColumn:) AS inv_order
FROM :joinTable:
WHERE :joinColumn: = :id OR :inverseJoinColumn: IN (:inverseRelIds)
) AS t
WHERE t.id = :joinTable:.id`,
*/
await db.getConnection().raw(
`UPDATE ??
SET ${update.join(', ')}
FROM (
SELECT
id,
ROW_NUMBER() OVER ( PARTITION BY :joinColumn: ORDER BY :orderColumn:) AS order,
ROW_NUMBER() OVER ( PARTITION BY :inverseJoinColumn: ORDER BY :inverseOrderColumn:) AS inv_order
FROM :joinTable:
WHERE :joinColumn: = :id OR :inverseJoinColumn: IN (:inverseRelIds)
SELECT ${select.join(', ')}
FROM ??
WHERE ${where.join(' OR ')}
) AS t
WHERE t.id = :joinTable:.id`,
*/
WHERE t.id = ??.id`,
[
joinTable.name,
...updateBinding,
...selectBinding,
joinTable.name,
...whereBinding,
joinTable.name,
]
);
};
module.exports = {

View File

@ -547,6 +547,9 @@ const createJoinTable = (metadata, { attributeName, attribute, meta }) => {
}
};
const hasOrderColumn = (attribute) => isAnyToMany(attribute);
const hasInverseOrderColumn = (attribute) => isBidirectional(attribute) && isManyToAny(attribute);
module.exports = {
createRelation,
@ -555,4 +558,6 @@ module.exports = {
isManyToAny,
isAnyToOne,
isAnyToMany,
hasOrderColumn,
hasInverseOrderColumn,
};