Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

upsertGraph: allow marking individual models in graph to be unrelated or deleted #2410

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions lib/model/Model.js
Original file line number Diff line number Diff line change
Expand Up @@ -729,6 +729,8 @@ Model.uidProp = '#id';
Model.uidRefProp = '#ref';
Model.dbRefProp = '#dbRef';
Model.propRefRegex = /#ref{([^\.]+)\.([^}]+)}/g;
Model.graphUnrelateProp = '#unrelate';
Model.graphDeleteProp = '#delete';
Model.jsonAttributes = null;
Model.cloneObjectAttributes = true;
Model.virtualAttributes = null;
Expand Down
8 changes: 8 additions & 0 deletions lib/model/graph/ModelGraphNode.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,14 @@ class ModelGraphNode {
return this.obj[this.modelClass.dbRefProp];
}

get toBeUnrelated() {
return this.obj[this.modelClass.graphUnrelateProp] === true;
}

get toBeDeleted() {
return this.obj[this.modelClass.graphDeleteProp] === true;
}

get parentNode() {
if (this.parentEdge) {
return this.parentEdge.ownerNode;
Expand Down
4 changes: 3 additions & 1 deletion lib/queryBuilder/graph/GraphFetcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,9 @@ function shouldSelectColumn(column, selects, node) {
!selects.has(column) &&
column !== modelClass.propertyNameToColumnName(modelClass.dbRefProp) &&
column !== modelClass.propertyNameToColumnName(modelClass.uidRefProp) &&
column !== modelClass.propertyNameToColumnName(modelClass.uidProp)
column !== modelClass.propertyNameToColumnName(modelClass.uidProp) &&
column !== modelClass.propertyNameToColumnName(modelClass.graphUnrelateProp) &&
column !== modelClass.propertyNameToColumnName(modelClass.graphDeleteProp)
);
}

Expand Down
22 changes: 20 additions & 2 deletions lib/queryBuilder/graph/GraphOptions.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ class GraphOptions {

// Like `shouldRelate` but ignores settings that explicitly disable relate operations.
shouldRelateIgnoreDisable(node, graphData) {
if (node.toBeUnrelated || node.toBeDeleted) {
return false;
}

if (node.isReference || node.isDbReference) {
return true;
}
Expand All @@ -71,6 +75,10 @@ class GraphOptions {

// Like `shouldInsert` but ignores settings that explicitly disable insert operations.
shouldInsertIgnoreDisable(node, graphData) {
if (node.toBeUnrelated || node.toBeDeleted) {
return false;
}

return (
!getCurrentNode(node, graphData) &&
!this.shouldRelateIgnoreDisable(node, graphData) &&
Expand All @@ -89,6 +97,10 @@ class GraphOptions {
// Like `shouldPatch() || shouldUpdate()` but ignores settings that explicitly disable
// update or patch operations.
shouldPatchOrUpdateIgnoreDisable(node, graphData) {
if (node.toBeUnrelated || node.toBeDeleted) {
return false;
}

if (this.shouldRelate(node, graphData)) {
// We should update all nodes that are going to be related. Note that
// we don't actually update anything unless there is something to update
Expand Down Expand Up @@ -121,16 +133,22 @@ class GraphOptions {
}

shouldUnrelate(currentNode, graphData) {
const node = getNode(currentNode, graphData.graph);
if (node && node.toBeUnrelated) return true;

return (
!getNode(currentNode, graphData.graph) &&
!node &&
!this._hasOption(currentNode, NO_UNRELATE) &&
this.shouldUnrelateIgnoreDisable(currentNode)
);
}

shouldDelete(currentNode, graphData) {
const node = getNode(currentNode, graphData.graph);
if (node && node.toBeDeleted) return true;

return (
!getNode(currentNode, graphData.graph) &&
!node &&
!this._hasOption(currentNode, NO_DELETE) &&
!this.shouldUnrelateIgnoreDisable(currentNode)
);
Expand Down
2 changes: 2 additions & 0 deletions lib/queryBuilder/graph/GraphUpsert.js
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,8 @@ function checkForNotFoundErrors(graphData, builder) {
for (const node of graph.nodes) {
if (
node.obj.$hasId() &&
!node.toBeUnrelated &&
!node.toBeDeleted &&
!graphOptions.shouldInsertIgnoreDisable(node, graphData) &&
!graphOptions.shouldRelateIgnoreDisable(node, graphData) &&
!currentGraph.nodeForNode(node)
Expand Down
223 changes: 223 additions & 0 deletions tests/integration/upsertGraph.js
Original file line number Diff line number Diff line change
Expand Up @@ -1296,6 +1296,229 @@ module.exports = (session) => {
});
});

it('should respect noDelete flag and special #unrelate and #delete model props', () => {
const upsert = {
// the root gets updated because it has an id
id: 2,
model1Prop1: 'updated root 2',

// unrelate
model1Relation1: null,

// update idCol=1
// delete idCol=2 with `#delete: true` special prop
// and insert one new
model1Relation2: [
{
idCol: 1,
model2Prop1: 'updated hasMany 1',

// unrelate id=4 with `#unrelate: true` special prop
// don't unrelate id=5 because of `noUnrelate`
// relate id=6
// and insert one new
model2Relation1: [
{
id: 4,
'#unrelate': true,
},
{
// This is the new row.
model1Prop1: 'inserted manyToMany',
},
{
id: 6,
},
],
},
{
idCol: 2,
'#delete': true,
},
{
// This is the new row.
model2Prop1: 'inserted hasMany',
},
],
};

return transaction(session.knex, (trx) => {
return Model1.query(trx)
.upsertGraph(upsert, {
fetchStrategy,
relate: true,
noDelete: true,
unrelate: ['model1Relation1'],
})
.then((result) => {
// Fetch the graph from the database.
return Model1.query(trx)
.findById(2)
.withGraphFetched(
'[model1Relation1, model1Relation2(orderById).model2Relation1(orderById)]'
);
})
.then(omitIrrelevantProps)
.then((result) => {
expect(result).to.eql({
id: 2,
model1Id: null,
model1Prop1: 'updated root 2',

model1Relation1: null,

model1Relation2: [
{
idCol: 1,
model1Id: 2,
model2Prop1: 'updated hasMany 1',

model2Relation1: [
{
id: 5,
model1Id: null,
model1Prop1: 'manyToMany 2',
},
{
id: 6,
model1Id: null,
model1Prop1: 'manyToMany 3',
},
{
id: 8,
model1Id: null,
model1Prop1: 'inserted manyToMany',
},
],
},
{
idCol: 3,
model1Id: 2,
model2Prop1: 'inserted hasMany',
model2Relation1: [],
},
],
});

return Promise.all([trx('Model1'), trx('model2')]).then(
([model1Rows, model2Rows]) => {
// Row 3 should NOT be deleted.
expect(model1Rows.find((it) => it.id == 3)).to.eql({
id: 3,
model1Id: null,
model1Prop1: 'belongsToOne',
model1Prop2: null,
});

// Row 4 should NOT be deleted.
expect(model1Rows.find((it) => it.id == 4)).to.eql({
id: 4,
model1Id: null,
model1Prop1: 'manyToMany 1',
model1Prop2: null,
});

// Row 2 should be deleted.
expect(model2Rows.find((it) => it.id_col == 2)).to.equal(undefined);
}
);
});
});
});

it('should not fail when trying to #unrelate or #delete a model which either does not exist or is linked elsewhere', () => {
const upsert = {
id: 2,
model1Relation2: [
{
idCol: 1,
model2Relation1: [
// ignore unrelating/deleting models which are linked to a different base model
// id 6&7 are linked to model1Relation2.idCol=2
{ id: 6, '#unrelate': true },
{ id: 7, '#delete': true },
// ignore unrelating/deleting non-existing models
{ id: 900, '#unrelate': true },
{ id: 901, '#delete': true },
// add new model
{ model1Prop1: 'inserted manyToMany' },
],
},
// ignore delete (non-existing) idCol=3 with `#delete: true` special prop
// ignore unrelate (non-existing) idCol=4 with `#unrelate: true` special prop
{ idCol: 3, '#delete': true },
{ idCol: 4, '#unrelate': true },
],
};

return transaction(session.knex, (trx) => {
return Model1.query(trx)
.upsertGraph(upsert, {
fetchStrategy,
relate: true,
noDelete: true,
})
.then((result) => {
// Fetch the graph from the database.
return Model1.query(trx)
.findById(2)
.withGraphFetched('[model1Relation2(orderById).model2Relation1(orderById)]');
})
.then(omitIrrelevantProps)
.then((result) => {
expect(result).to.eql({
id: 2,
model1Id: 3,
model1Prop1: 'root 2',

model1Relation2: [
{
idCol: 1,
model1Id: 2,
model2Prop1: 'hasMany 1',

model2Relation1: [
{
id: 4,
model1Id: null,
model1Prop1: 'manyToMany 1',
},
{
id: 5,
model1Id: null,
model1Prop1: 'manyToMany 2',
},
{
id: 8,
model1Id: null,
model1Prop1: 'inserted manyToMany',
},
],
},
{
idCol: 2,
model1Id: 2,
model2Prop1: 'hasMany 2',

model2Relation1: [
{
id: 6,
model1Id: null,
model1Prop1: 'manyToMany 3',
},
{
id: 7,
model1Id: null,
model1Prop1: 'manyToMany 4',
},
],
},
],
});
});
});
});

it('should relate and unrelate some models if `unrelate` and `relate` are arrays of relation paths', () => {
const upsert = {
// the root gets updated because it has an id
Expand Down
4 changes: 4 additions & 0 deletions typings/objection/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1437,6 +1437,8 @@ declare namespace Objection {
uidRefProp: string;
dbRefProp: string;
propRefRegex: RegExp;
graphUnrelateProp: string;
graphDeleteProp: string;
pickJsonSchemaProperties: boolean;
relatedFindQueryMutates: boolean;
relatedInsertQueryMutates: boolean;
Expand Down Expand Up @@ -1543,6 +1545,8 @@ declare namespace Objection {
static uidRefProp: string;
static dbRefProp: string;
static propRefRegex: RegExp;
static graphUnrelateProp: string;
static graphDeleteProp: string;
static pickJsonSchemaProperties: boolean;
static relatedFindQueryMutates: boolean;
static relatedInsertQueryMutates: boolean;
Expand Down