Skip to content

Commit 936c9a6

Browse files
committed
fix: fix that child entity updates referencing non-existant IDs would add new child entities
Previously, we did not check whether the child entities we wanted to update actually existed. The bug has been introduced with the new dict-based child entity update implementation.
1 parent 75e0252 commit 936c9a6

File tree

4 files changed

+20
-4
lines changed

4 files changed

+20
-4
lines changed

spec/regression/logistics/tests/update-child-entities-dict.graphql

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,9 @@ mutation updateMultiple {
6161
{ id: "id_test_0007", itemNumber: "updated07" }
6262
{ id: "id_test_0008", itemNumber: "updated08" }
6363
{ id: "id_test_0009", itemNumber: "updated09" }
64+
# make sure this one does not end up in the list
65+
# (the initial implementation of the dict-based update had a bug that it was)
66+
{ id: "nonexistant", itemNumber: "updated10" }
6467
]
6568
}
6669
) {

spec/regression/logistics/tests/update-child-entities.graphql

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,9 @@ mutation updateMultiple {
6161
{ id: "id_test_0007", itemNumber: "updated07" }
6262
{ id: "id_test_0008", itemNumber: "updated08" }
6363
{ id: "id_test_0009", itemNumber: "updated09" }
64+
# make sure this one does not end up in the list
65+
# (the initial implementation of the dict-based update had a bug that it was)
66+
{ id: "nonexistant", itemNumber: "updated10" }
6467
]
6568
}
6669
) {

src/database/arangodb/aql-generator.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -853,9 +853,13 @@ register(UpdateChildEntitiesQueryNode, (node, context) => {
853853
),
854854
aql`})`,
855855

856-
// sort by the __index we stored, and unpack the dictionary into a list again
856+
// sort by the __index we stored,
857+
// filter out objects that were included in node.updates() but did not actually exist in node.originalList
858+
// (for them, __index is not set)
859+
// and unpack the dictionary into a list again
857860
aql`FOR ${itemVar}`,
858861
aql`IN VALUES(${updatedDictVar})`,
862+
aql`FILTER ${itemVar}.__index != null`,
859863
aql`SORT ${itemVar}.__index`,
860864
aql`RETURN UNSET(${itemVar}, '__index')`,
861865
);

src/database/inmemory/js-generator.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -528,7 +528,11 @@ register(UpdateChildEntitiesQueryNode, (node, context) => {
528528
// could be a complex expression, and we're using it multiple times -> store in a variable
529529
js`const ${itemsVar} = ${processNode(node.originalList, context)}`,
530530

531-
// add a __index property to each item so we can sort by this later
531+
// the aql implementation needs an __index property to sort the object after applying
532+
// the updates. the js implementation does not need to sort because {...obj, a: ...}
533+
// keeps the order of properties in obj. However, we still need to filter out updates
534+
// for which there are no items in originalList. To stay close to the aql implementation,
535+
// we just also set an __index here.
532536
// regular field names cannot start with an underscore, so we're safe to use __index as a
533537
// temporary property to store the index of the child entity in the list
534538
// convert the list into a dict object like { 'id1': { ...}, 'id2': { ... } }
@@ -566,8 +570,10 @@ register(UpdateChildEntitiesQueryNode, (node, context) => {
566570
),
567571
js`};`,
568572

569-
// sort by the __index we stored, and unpack the dictionary into a list again
570-
js`return Object.values(${updatedDictVar}).map(({ __index, ...${itemVar} }) => ${itemVar});`,
573+
// filter out objects that were included in node.updates() but did not actually exist in node.originalList
574+
// (for them, __index is not set)
575+
// and unpack the dictionary into a list again
576+
js`return Object.values(${updatedDictVar}).filter(({ __index }) => __index !== undefined).map(({ __index, ...${itemVar} }) => ${itemVar});`,
571577
);
572578
});
573579

0 commit comments

Comments
 (0)