diff --git a/packages/core/database/lib/entity-manager/__tests__/relations-orderer.test.js b/packages/core/database/lib/entity-manager/__tests__/relations-orderer.test.js index 45c1d65ef8..ee67bcd08e 100644 --- a/packages/core/database/lib/entity-manager/__tests__/relations-orderer.test.js +++ b/packages/core/database/lib/entity-manager/__tests__/relations-orderer.test.js @@ -58,7 +58,6 @@ describe('Given I have some relations in the database', () => { ); orderer.connect([ - { id: 4, position: { before: 2 } }, { id: 4, position: { before: 3 } }, { id: 5, position: { before: 4 } }, ]); diff --git a/packages/core/database/lib/entity-manager/__tests__/sort-connect-array.test.js b/packages/core/database/lib/entity-manager/__tests__/sort-connect-array.test.js index 6e81ba8268..5de024d9c6 100644 --- a/packages/core/database/lib/entity-manager/__tests__/sort-connect-array.test.js +++ b/packages/core/database/lib/entity-manager/__tests__/sort-connect-array.test.js @@ -61,4 +61,19 @@ describe('sortConnectArray', () => { 'A circular reference was found in the connect array. One relation is trying to connect before/after another one that is trying to connect before/after it' ); }); + + test('error when connecting same relation twice', () => { + const sortConnect = () => + sortConnectArray( + [ + { id: 1, position: { after: 2 } }, + { id: 1, position: { after: 3 } }, + ], + [] + ); + + expect(sortConnect).toThrowError( + 'The relation with id 1 is already connected. You cannot connect the same relation twice.' + ); + }); }); diff --git a/packages/core/database/lib/entity-manager/relations-orderer.js b/packages/core/database/lib/entity-manager/relations-orderer.js index 2d487848ae..508d235305 100644 --- a/packages/core/database/lib/entity-manager/relations-orderer.js +++ b/packages/core/database/lib/entity-manager/relations-orderer.js @@ -25,42 +25,36 @@ const sortConnectArray = (connectArr, initialArr = [], strictSort = true) => { // Boolean to know if we have to recalculate the order of the relations let needsSorting = false; // Map to validate if relation is already in sortedConnect or DB. - const relationInArray = initialArr.reduce((acc, rel) => ({ ...acc, [rel.id]: true }), {}); + const relationInInitialArray = initialArr.reduce((acc, rel) => ({ ...acc, [rel.id]: true }), {}); // Map to store the first index where a relation id is connected - const mappedRelationsIndex = connectArr.reduce((mapper, relation, index) => { + const mappedRelations = connectArr.reduce((mapper, relation) => { const adjacentRelId = relation.position?.before || relation.position?.after; - if (!relationInArray[adjacentRelId] && !mapper[adjacentRelId]) { + if (!relationInInitialArray[adjacentRelId] && !mapper[adjacentRelId]) { needsSorting = true; } + // If the relation is already in the array, throw an error + if (mapper[relation.id]) { + throw new InvalidRelationError( + `The relation with id ${relation.id} is already connected. ` + + 'You cannot connect the same relation twice.' + ); + } + return { - [relation.id]: mapper[relation.id] || index, // If the relation is already in the array, we keep the first index + [relation.id]: { ...relation, computed: false }, ...mapper, }; }, {}); - // Map to validate if connect relation has already been computed - const computedIdx = {}; // If we don't need to sort the connect array, we can return it as is if (!needsSorting) return connectArr; - // Add relation to sortedConnect and mark it as seen - const pushRelation = (relation) => { - sortedConnect.push(relation); - relationInArray[relation.id] = true; - }; - // Recursively compute in which order the relation should be connected - const computeRelation = (relation, idx, relationsSeenInBranch) => { + const computeRelation = (relation, relationsSeenInBranch) => { const adjacentRelId = relation.position?.before || relation.position?.after; - - // This connect has already been computed - if (idx in computedIdx) return; - - if (!adjacentRelId || relationInArray[adjacentRelId]) { - return pushRelation(relation); - } + const adjacentRelation = mappedRelations[adjacentRelId]; // If the relation has already been seen in the current branch, // it means there is a circular reference @@ -71,20 +65,24 @@ const sortConnectArray = (connectArr, initialArr = [], strictSort = true) => { ); } + // This relation has already been computed + if (mappedRelations[relation.id]?.computed) return; + + // Relation does not have a before or after attribute or is in the initial array + if (!adjacentRelId || relationInInitialArray[adjacentRelId]) { + mappedRelations[relation.id].computed = true; + sortedConnect.push(relation); + return; + } + // Look if id is referenced elsewhere in the array - const adjacentRelIdx = mappedRelationsIndex[adjacentRelId]; - - if (adjacentRelIdx) { - const adjacentRel = connectArr[adjacentRelIdx]; - - // Mark adjacent relation idx as computed, - // so it is not computed again later in the loop - computedIdx[adjacentRelIdx] = true; - computeRelation(adjacentRel, idx, { ...relationsSeenInBranch, [relation.id]: true }); - pushRelation(relation); + if (mappedRelations[adjacentRelId]) { + mappedRelations[relation.id].computed = true; + computeRelation(adjacentRelation, { ...relationsSeenInBranch, [relation.id]: true }); + sortedConnect.push(relation); } else if (strictSort) { // If we reach this point, it means that the adjacent relation is not in the connect array - // and it is not in the database. This should not happen. + // and it is not in the database. throw new InvalidRelationError( `There was a problem connecting relation with id ${ relation.id @@ -94,14 +92,13 @@ const sortConnectArray = (connectArr, initialArr = [], strictSort = true) => { ); } else { // We are in non-strict mode so we can push the relation. - pushRelation({ id: relation.id, position: { end: true } }); + mappedRelations[relation.id].computed = true; + sortedConnect.push({ id: relation.id, position: { end: true } }); } }; // Iterate over connectArr and populate sortedConnect - connectArr.forEach((relation, idx) => { - computeRelation(relation, idx, {}); - }); + connectArr.forEach((relation) => computeRelation(relation, {})); return sortedConnect; }; diff --git a/packages/core/strapi/tests/api/relations.test.api.js b/packages/core/strapi/tests/api/relations.test.api.js index 26ea2668e9..462dd1e904 100644 --- a/packages/core/strapi/tests/api/relations.test.api.js +++ b/packages/core/strapi/tests/api/relations.test.api.js @@ -804,7 +804,6 @@ describe('Relations', () => { { id: id1, position: { start: true } }, { id: id2, position: { start: true } }, { id: id3, position: { after: id1 } }, - { id: id1, position: { after: id2 } }, ], }); @@ -852,10 +851,7 @@ describe('Relations', () => { ], }); - const expectedShop = shopFactory({ - anyToManyRel: [{ id: id2 }, { id: id1 }], - }); - expect(updatedShop.data).toMatchObject(expectedShop); + expect(updatedShop.error).toMatchObject({ status: 400, name: 'ValidationError' }); }); test('Update relations with invalid connect array in strict mode', async () => {