Skip to content

Commit

Permalink
fix: deep delete for views (#496)
Browse files Browse the repository at this point in the history
Co-authored-by: I543501 <56645452+larslutz96@users.noreply.github.com>
Co-authored-by: Patrice Bender <patrice.bender@sap.com>
Co-authored-by: I543501 <lars.lutz@sap.com>
Co-authored-by: Johannes Vogel <johannes.vogel@sap.com>
Co-authored-by: Johannes Vogel <31311694+johannes-vogel@users.noreply.github.com>
  • Loading branch information
6 people authored Oct 28, 2024
1 parent f4d7caf commit 82154ef
Show file tree
Hide file tree
Showing 3 changed files with 143 additions and 9 deletions.
35 changes: 31 additions & 4 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'),
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,14 +25,16 @@ 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) => {
operation.data ||
operation.entries?.reduce((acc, obj) => {
return Object.assign(acc, obj)
}, {}),
)
Expand Down Expand Up @@ -214,7 +216,31 @@ class SQLService extends DatabaseService {
// 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.target, this, false, req.query.cmd || 'DELETE')
if (transitions.target !== transitions.queryTarget) {
const keys = []
const transitionsTarget = transitions.queryTarget.keys || transitions.queryTarget.elements
for (const key in transitionsTarget) {
const exists = e => e && !e.virtual && !e.value && !e.isAssociation
if (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, target: transitions.target })
}
const table = getDBTable(req.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 @@ -241,6 +267,7 @@ class SQLService extends DatabaseService {
)
// Prepare and run deep query, à la CQL`DELETE from Foo[pred]:comp1.comp2...`
const query = DELETE.from({ ref: [...from.ref, c.name] })
query.target = c._target
return this.onDELETE({ query, depth, visited: [...visited], target: c._target })
}),
)
Expand Down
80 changes: 75 additions & 5 deletions test/compliance/DELETE.test.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,85 @@
const cds = require('../cds.js')
const cds = require('../../test/cds.js')
const complex = cds.utils.path.resolve(__dirname, '../compliance/resources')

Check warning on line 2 in test/compliance/DELETE.test.js

View workflow job for this annotation

GitHub Actions / Tests (22)

'complex' is assigned a value but never used
const Root = 'complex.Root'
const Child = 'complex.Child'
const GrandChild = 'complex.GrandChild'
const RootPWithKeys = 'complex.RootPWithKeys'
const ChildPWithWhere = 'complex.ChildPWithWhere'

describe('DELETE', () => {
const { data, expect } = cds.test(__dirname + '/resources')
data.autoIsolation(true)
data.autoReset()

describe.skip('from', () => {
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('ref', async () => {
const { globals } = cds.entities('basic.projection')
const changes = await cds.run(CQL`DELETE FROM ${globals}`)
expect(changes | 0).to.eq(3, 'Ensure that all rows are affected')
const { Authors } = cds.entities('complex.associations')
await INSERT.into(Authors).entries(new Array(9).fill().map((e,i) => ({ ID: 100+i, name: 'name'+i})))
const changes = await cds.run(DELETE.from(Authors))
expect(changes | 0).to.be.eq(10, 'Ensure that all rows are affected') // 1 from csv, 9 newly added
})
})

Expand Down
37 changes: 37 additions & 0 deletions test/compliance/resources/db/complex/index.cds
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,40 @@ using from './computed';
using from './associations';
using from './associationsUnmanaged';
using from './uniques';

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 82154ef

Please sign in to comment.