From 82154ef8b837f17e81e2516056e03ff215f1dff8 Mon Sep 17 00:00:00 2001 From: Bob den Os <108393871+BobdenOs@users.noreply.github.com> Date: Mon, 28 Oct 2024 17:08:38 +0100 Subject: [PATCH] fix: deep delete for views (#496) Co-authored-by: I543501 <56645452+larslutz96@users.noreply.github.com> Co-authored-by: Patrice Bender Co-authored-by: I543501 Co-authored-by: Johannes Vogel Co-authored-by: Johannes Vogel <31311694+johannes-vogel@users.noreply.github.com> --- db-service/lib/SQLService.js | 35 +++++++- test/compliance/DELETE.test.js | 80 +++++++++++++++++-- .../compliance/resources/db/complex/index.cds | 37 +++++++++ 3 files changed, 143 insertions(+), 9 deletions(-) diff --git a/db-service/lib/SQLService.js b/db-service/lib/SQLService.js index ecf4f7d02..16be87f75 100644 --- a/db-service/lib/SQLService.js +++ b/db-service/lib/SQLService.js @@ -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') @@ -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) }, {}), ) @@ -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 @@ -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 }) }), ) diff --git a/test/compliance/DELETE.test.js b/test/compliance/DELETE.test.js index ce23f6424..bfda99319 100644 --- a/test/compliance/DELETE.test.js +++ b/test/compliance/DELETE.test.js @@ -1,15 +1,85 @@ -const cds = require('../cds.js') +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 { 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 }) }) diff --git a/test/compliance/resources/db/complex/index.cds b/test/compliance/resources/db/complex/index.cds index ce7b0e6f3..686cba74d 100644 --- a/test/compliance/resources/db/complex/index.cds +++ b/test/compliance/resources/db/complex/index.cds @@ -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'