Skip to content

Commit

Permalink
fix: deep delete for views without navigation (#434)
Browse files Browse the repository at this point in the history
Support deep delete when navigation is not included in the view.
Rewrite first incoming query in `deep_delete()` to a delete from
database table where keys in select from view and call `onDELETE` again
with this query.

---------

Co-authored-by: Patrice Bender <patrice.bender@sap.com>
Co-authored-by: I543501 <lars.lutz@sap.com>
  • Loading branch information
3 people authored Mar 5, 2024
1 parent cd3115b commit 3ebc9c2
Show file tree
Hide file tree
Showing 3 changed files with 161 additions and 18 deletions.
57 changes: 45 additions & 12 deletions db-service/lib/SQLService.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
const cds = require('@sap/cds/lib'),
DEBUG = cds.debug('sql|db')
const { Readable } = require('stream')
const { resolveView } = require('@sap/cds/libx/_runtime/common/utils/resolveView')
const { resolveView, getDBTable, getTransition } = require('@sap/cds/libx/_runtime/common/utils/resolveView')
const DatabaseService = require('./common/DatabaseService')
const cqn4sql = require('./cqn4sql')

Expand All @@ -25,16 +25,18 @@ class SQLService extends DatabaseService {
this.on(['INSERT', 'UPSERT', 'UPDATE'], require('./deep-queries').onDeep)
if (cds.env.features.db_strict) {
this.before(['INSERT', 'UPSERT', 'UPDATE'], ({ query }) => {
const elements = query.target?.elements; if (!elements) return
const elements = query.target?.elements
if (!elements) return
const kind = query.kind || Object.keys(query)[0]
const operation = query[kind]
if (!operation.columns && !operation.entries && !operation.data) return
const columns =
operation.columns ||
Object.keys(
operation.data || operation.entries?.reduce((acc, obj) => {
return Object.assign(acc, obj)
}, {}),
operation.data ||
operation.entries?.reduce((acc, obj) => {
return Object.assign(acc, obj)
}, {}),
)
const invalidColumns = columns.filter(c => !(c in elements))

Expand Down Expand Up @@ -115,7 +117,11 @@ class SQLService extends DatabaseService {
*/
async onSELECT({ query, data }) {
if (!query.target) {
try { this.infer(query) } catch (e) { /**/ }
try {
this.infer(query)
} catch (e) {
/**/
}
}
if (query.target && !query.target._unresolved) {
// Will return multiple rows with objects inside
Expand Down Expand Up @@ -195,11 +201,38 @@ class SQLService extends DatabaseService {
return (await ps.run(values)).changes
}

exists(e) {
return e && !e.virtual && !e.value && !e.isAssociation
}

get onDELETE() {
// REVISIT: It's not yet 100 % clear under which circumstances we can rely on db constraints
return (super.onDELETE = /* cds.env.features.assert_integrity === 'db' ? this.onSIMPLE : */ deep_delete)
async function deep_delete(/** @type {Request} */ req) {
let { compositions } = req.target
const transitions = getTransition(req.query.target, this)
if (transitions.target !== transitions.queryTarget) {
const keys = []
const transitionsTarget = transitions.queryTarget.keys || transitions.queryTarget.elements
for (const key in transitionsTarget) {
if (this.exists(transitionsTarget[key])) keys.push(key)
}
const matchedKeys = keys.filter(key => transitions.mapping.has(key)).map(k => ({ ref: [k] }))
const query = DELETE.from({
ref: [
{
id: transitions.target.name,
where: [
{ list: matchedKeys.map(k => transitions.mapping.get(k.ref[0])) },
'in',
SELECT.from(req.query.DELETE.from).columns(matchedKeys).where(req.query.DELETE.where),
],
},
],
})
return this.onDELETE({ query })
}
const table = getDBTable(req.query.target)
const { compositions } = table
if (compositions) {
// Transform CQL`DELETE from Foo[p1] WHERE p2` into CQL`DELETE from Foo[p1 and p2]`
let { from, where } = req.query.DELETE
Expand All @@ -211,22 +244,22 @@ class SQLService extends DatabaseService {
}
// Process child compositions depth-first
let { depth = 0, visited = [] } = req
visited.push(req.target.name)
visited.push(req.query.target.name)
await Promise.all(
Object.values(compositions).map(c => {
if (c._target['@cds.persistence.skip'] === true) return
if (c._target === req.target) {
if (c._target === req.query.target) {
// the Genre.children case
if (++depth > (c['@depth'] || 3)) return
} else if (visited.includes(c._target.name))
throw new Error(
`Transitive circular composition detected: \n\n` +
` ${visited.join(' > ')} > ${c._target.name} \n\n` +
`These are not supported by deep delete.`,
` ${visited.join(' > ')} > ${c._target.name} \n\n` +
`These are not supported by deep delete.`,
)
// Prepare and run deep query, à la CQL`DELETE from Foo[pred]:comp1.comp2...`
const query = DELETE.from({ ref: [...from.ref, c.name] })
return this.onDELETE({ query, depth, visited: [...visited], target: c._target })
return this.onDELETE({ query, depth, visited: [...visited] })
}),
)
}
Expand Down
72 changes: 72 additions & 0 deletions test/compliance/DELETE.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,77 @@
const cds = require('../../test/cds.js')
const complex = cds.utils.path.resolve(__dirname, '../compliance/resources')
const Root = 'complex.Root'
const Child = 'complex.Child'
const GrandChild = 'complex.GrandChild'
const RootPWithKeys = 'complex.RootPWithKeys'
const ChildPWithWhere = 'complex.ChildPWithWhere'

describe('DELETE', () => {
const { expect } = cds.test(complex)
describe('from', () => {
describe('deep', () => {
beforeEach(async () => {
const inserts = [
INSERT.into(Root).entries([
{
ID: 5,
fooRoot: 'bar',
children: [
{
ID: 6,
fooChild: 'bar',
children: [
{
ID: 8,
fooGrandChild: 'bar',
},
],
},
{
ID: 7,
fooChild: 'foo',
children: [
{
ID: 9,
fooGrandChild: 'foo',
},
],
},
],
},
]),
]
const insertsResp = await cds.run(inserts)
expect(insertsResp[0].affectedRows).to.be.eq(1)
})

test('on root with keys', async () => {
const deepDelete = await cds.run(DELETE.from(RootPWithKeys).where({ ID: 5 }))
expect(deepDelete).to.be.eq(1)

const root = await cds.run(SELECT.one.from(Root).where({ ID: 5 }))
expect(root).to.not.exist

const child = await cds.run(SELECT.from(Child).where({ ID: 6, or: { ID: 7 } }))
expect(child.length).to.be.eq(0)

const grandchild = await cds.run(SELECT.from(GrandChild).where({ ID: 8, or: { ID: 9 } }))
expect(grandchild.length).to.be.eq(0)
})

test('on child with where', async () => {
// only delete entries where fooChild = 'bar'
const deepDelete = await cds.run(DELETE.from(ChildPWithWhere))
expect(deepDelete).to.be.eq(1)

const child = await cds.run(SELECT.from(Child).where({ ID: 6, or: { ID: 7 } }))
expect(child[0].ID).to.be.eq(7)

const grandchild = await cds.run(SELECT.from(GrandChild).where({ ID: 8, or: { ID: 9 } }))
expect(grandchild[0].ID).to.be.eq(9)
})
})

test.skip('missing', () => {
throw new Error('not supported')
})
Expand Down
50 changes: 44 additions & 6 deletions test/compliance/resources/db/complex/index.cds
Original file line number Diff line number Diff line change
@@ -1,13 +1,51 @@
namespace complex;

entity Books {
key ID : Integer;
title : String(111);
author : Association to Authors;
key ID : Integer;
title : String(111);
author : Association to Authors;
}

entity Authors {
key ID : Integer;
name : String(111);
books : Association to many Books on books.author = $self;
key ID : Integer;
name : String(111);
books : Association to many Books
on books.author = $self;
}

entity Root {
key ID : Integer;
fooRoot : String;
children : Composition of many Child
on children.parent = $self;
}

entity Child {
key ID : Integer;
fooChild : String;
parent : Association to one Root;
children : Composition of many GrandChild
on children.parent = $self
}

entity GrandChild {
key ID : Integer;
fooGrandChild : String;
parent : Association to one Child;
}

entity RootPWithKeys as
projection on Root {
key ID,
fooRoot,
children
}

entity ChildP as
projection on Child {
key ID,
fooChild,
parent
}

entity ChildPWithWhere as projection on Child where fooChild = 'bar'

0 comments on commit 3ebc9c2

Please sign in to comment.