Skip to content

Commit 6509ee5

Browse files
chore: Compatibility for unfolded CSN (#381)
- [x] support `assoc.elements` (instead or `.foreignKeys`) - [x] `localized` views are missing - [ ] calculating the full name of a deeply structured element by walking up its `.parent` yields faulty names for already unfolded structs with a `.parent` in their prototype. - [x] Smart Wildcards / overwrites: Structures are already unfolded ``` const input = CQL`SELECT from bookshop.Bar { 'first' as structure, * }` //> leafs of structure will still be rendered ``` - [ ] ... --------- Co-authored-by: Patrice Bender <[email protected]>
1 parent 4644483 commit 6509ee5

16 files changed

+142
-61
lines changed

.gitignore

+3
Original file line numberDiff line numberDiff line change
@@ -137,3 +137,6 @@ dist
137137

138138
# cds-typer
139139
@cds-models/
140+
141+
# cds temp folders
142+
**/_out

db-service/lib/cqn4sql.js

+19-7
Original file line numberDiff line numberDiff line change
@@ -972,7 +972,11 @@ function cqn4sql(originalQuery, model = cds.context?.model || cds.model) {
972972
* @returns {boolean} true if the element is a managed association and the model is flat
973973
*/
974974
function isManagedAssocInFlatMode(e) {
975-
return model.meta.transformation === 'odata' && e.isAssociation && e.keys
975+
return (
976+
(model.meta.transformation === 'odata' || model.meta.unfolded?.some(u => u === 'assocs')) &&
977+
e.isAssociation &&
978+
e.keys
979+
)
976980
}
977981
}
978982

@@ -1025,25 +1029,32 @@ function cqn4sql(originalQuery, model = cds.context?.model || cds.model) {
10251029
if (!column) return column
10261030
if (column.val || column.func || column.SELECT) return [column]
10271031

1032+
const structsAreUnfoldedAlready = model.meta.unfolded?.some(u => u === 'structs')
10281033
let { baseName, columnAlias, tableAlias } = names
10291034
const { exclude, replace } = excludeAndReplace || {}
10301035
const { $refLinks, flatName, isJoinRelevant } = column
10311036
let leafAssoc
10321037
let element = $refLinks ? $refLinks[$refLinks.length - 1].definition : column
10331038
if (isWildcard && element.type === 'cds.LargeBinary') return []
1034-
if (element.on) return [] // unmanaged doesn't make it into columns
1039+
if (element.on && !element.keys) return [] // unmanaged doesn't make it into columns
10351040
else if (element.virtual === true) return []
10361041
else if (!isJoinRelevant && flatName) baseName = flatName
10371042
else if (isJoinRelevant) {
10381043
const leaf = column.$refLinks[column.$refLinks.length - 1]
10391044
leafAssoc = [...column.$refLinks].reverse().find(link => link.definition.isAssociation)
1040-
const { foreignKeys } = leafAssoc.definition
1041-
if (foreignKeys && leaf.alias in foreignKeys) {
1045+
let elements
1046+
//> REVISIT: remove once UCSN is standard (no more .foreignKeys)
1047+
elements = leafAssoc.definition.elements || leafAssoc.definition.foreignKeys
1048+
if (elements && leaf.alias in elements) {
10421049
element = leafAssoc.definition
10431050
baseName = getFullName(leafAssoc.definition)
10441051
columnAlias = column.ref.slice(0, -1).map(idOnly).join('_')
10451052
} else baseName = getFullName(column.$refLinks[column.$refLinks.length - 1].definition)
1046-
} else baseName = baseName ? `${baseName}_${element.name}` : getFullName(element)
1053+
} else if (!baseName && structsAreUnfoldedAlready) {
1054+
baseName = element.name // name is already fully constructed
1055+
} else {
1056+
baseName = baseName ? `${baseName}_${element.name}` : getFullName(element)
1057+
}
10471058

10481059
// now we have the name of the to be expanded column
10491060
// it could be a structure, an association or a scalar
@@ -1621,7 +1632,7 @@ function cqn4sql(originalQuery, model = cds.context?.model || cds.model) {
16211632
* @returns {boolean}
16221633
*/
16231634
function isStructured(elt) {
1624-
return Boolean(elt?.elements && elt.kind === 'element')
1635+
return Boolean(elt?.kind !== 'entity' && elt?.elements && !elt.isAssociation)
16251636
}
16261637

16271638
/**
@@ -1994,7 +2005,8 @@ function cqn4sql(originalQuery, model = cds.context?.model || cds.model) {
19942005
* @returns the flat name of the element
19952006
*/
19962007
function getFullName(node, name = node.name) {
1997-
if (node.parent.kind === 'entity') return name
2008+
// REVISIT: this is an unfortunate implementation
2009+
if (!node.parent || node.parent.kind === 'entity') return name
19982010

19992011
return getFullName(node.parent, `${node.parent.name}_${name}`)
20002012
}

db-service/lib/infer/index.js

+31-17
Original file line numberDiff line numberDiff line change
@@ -99,12 +99,12 @@ function infer(originalQuery, model = cds.context?.model || cds.model) {
9999
if (!target) throw new Error(`"${first}" not found in the definitions of your model`)
100100
if (ref.length > 1) {
101101
target = from.ref.slice(1).reduce((d, r) => {
102-
const next = d.elements[r.id || r]?.elements ? d.elements[r.id || r] : d.elements[r.id || r]?._target
102+
const next = d.elements[r.id || r]?._target || d.elements[r.id || r]
103103
if (!next) throw new Error(`No association “${r.id || r}” in ${d.kind}${d.name}”`)
104104
return next
105105
}, target)
106106
}
107-
if (target.kind !== 'entity' && !target._isAssociation)
107+
if (target.kind !== 'entity' && !target.isAssociation)
108108
throw new Error('Query source must be a an entity or an association')
109109

110110
attachRefLinksToArg(from) // REVISIT: remove
@@ -165,7 +165,7 @@ function infer(originalQuery, model = cds.context?.model || cds.model) {
165165
// we need to search for first step in ´model.definitions[infixAlias]`
166166
if ($baseLink) {
167167
const { definition } = $baseLink
168-
const elements = definition.elements || definition._target?.elements
168+
const elements = definition._target?.elements || definition.elements
169169
const e = elements?.[id] || cds.error`"${id}" not found in the elements of "${definition.name}"`
170170
if (e.target) {
171171
// only fk access in infix filter
@@ -176,7 +176,7 @@ function infer(originalQuery, model = cds.context?.model || cds.model) {
176176
`"${e.name}" in path "${arg.ref.map(idOnly).join('.')}" must not be an unmanaged association`,
177177
)
178178
// no non-fk traversal in infix filter
179-
if (!expandOrExists && nextStep && !(nextStep in e.foreignKeys))
179+
if (!expandOrExists && nextStep && !isForeignKeyOf(nextStep, e))
180180
throw new Error(`Only foreign keys of "${e.name}" can be accessed in infix filter`)
181181
}
182182
arg.$refLinks.push({ definition: e, target: definition })
@@ -495,7 +495,7 @@ function infer(originalQuery, model = cds.context?.model || cds.model) {
495495
nameSegments.push(id)
496496
} else if ($baseLink) {
497497
const { definition, target } = $baseLink
498-
const elements = definition.elements || definition._target?.elements
498+
const elements = definition._target?.elements || definition.elements
499499
if (elements && id in elements) {
500500
const element = elements[id]
501501
rejectNonFkAccess(element)
@@ -530,7 +530,7 @@ function infer(originalQuery, model = cds.context?.model || cds.model) {
530530
}
531531
} else {
532532
const { definition } = column.$refLinks[i - 1]
533-
const elements = definition.elements || definition._target?.elements
533+
const elements = definition._target?.elements || definition.elements //> go for assoc._target first, instead of assoc as struct
534534
const element = elements?.[id]
535535

536536
if (firstStepIsSelf && element?.isAssociation) {
@@ -648,25 +648,25 @@ function infer(originalQuery, model = cds.context?.model || cds.model) {
648648
}
649649

650650
/**
651-
* Check if the next step in the ref is foreign key of `element`
651+
* Check if the next step in the ref is foreign key of `assoc`
652652
* if not, an error is thrown.
653-
*
654-
* @param {CSN.Element} element if this is an association, the next step must be a foreign key of the element.
653+
*
654+
* @param {CSN.Element} assoc if this is an association, the next step must be a foreign key of the element.
655655
*/
656-
function rejectNonFkAccess(element) {
657-
if (!inNestedProjection && !inCalcElement && element.target) {
656+
function rejectNonFkAccess(assoc) {
657+
if (!inNestedProjection && !inCalcElement && assoc.target) {
658658
// only fk access in infix filter
659659
const nextStep = column.ref[i + 1]?.id || column.ref[i + 1]
660660
// no unmanaged assoc in infix filter path
661-
if (!inExists && element.on)
661+
if (!inExists && assoc.on)
662662
throw new Error(
663-
`"${element.name}" in path "${column.ref
663+
`"${assoc.name}" in path "${column.ref
664664
.map(idOnly)
665665
.join('.')}" must not be an unmanaged association`
666666
)
667-
// no non-fk traversal in infix filter
668-
if (nextStep && element.foreignKeys && !(nextStep in element.foreignKeys))
669-
throw new Error(`Only foreign keys of "${element.name}" can be accessed in infix filter`)
667+
// no non-fk traversal in infix filter in non-exists path
668+
if (nextStep && !assoc.on && !isForeignKeyOf(nextStep, assoc))
669+
throw new Error(`Only foreign keys of "${assoc.name}" can be accessed in infix filter`)
670670
}
671671
}
672672
})
@@ -723,7 +723,7 @@ function infer(originalQuery, model = cds.context?.model || cds.model) {
723723
if (inlineCol === '*') {
724724
const wildCardElements = {}
725725
// either the `.elements´ of the struct or the `.elements` of the assoc target
726-
const leafLinkElements = $leafLink.definition.elements || $leafLink.definition._target.elements
726+
const leafLinkElements = $leafLink.definition._target?.elements || $leafLink.definition.elements
727727
Object.entries(leafLinkElements).forEach(([k, v]) => {
728728
const name = namePrefix ? `${namePrefix}_${k}` : k
729729
// if overwritten/excluded omit from wildcard elements
@@ -1130,6 +1130,20 @@ function infer(originalQuery, model = cds.context?.model || cds.model) {
11301130
}, '')
11311131
}
11321132
}
1133+
/**
1134+
* Returns true if e is a foreign key of assoc.
1135+
* this function is also compatible with unfolded csn (UCSN),
1136+
* where association do not have foreign keys anymore.
1137+
*
1138+
* @param {*} e
1139+
* @param {*} assoc
1140+
* @returns
1141+
*/
1142+
function isForeignKeyOf(e, assoc) {
1143+
if(!assoc.isAssociation) return false
1144+
if(assoc.foreignKeys) return e in assoc.foreignKeys
1145+
return assoc.elements && e in assoc.elements
1146+
}
11331147
const idOnly = ref => ref.id || ref
11341148

11351149
module.exports = infer

db-service/lib/infer/join-tree.js

+5-3
Original file line numberDiff line numberDiff line change
@@ -200,9 +200,11 @@ class JoinTree {
200200
}
201201
child.$refLink.alias = this.addNextAvailableTableAlias($refLink.alias, outerQueries)
202202
}
203-
204-
const foreignKeys = node.$refLink?.definition.foreignKeys
205-
if (node.$refLink && (!foreignKeys || !(child.$refLink.alias in foreignKeys)))
203+
//> REVISIT: remove fallback once UCSN is standard
204+
const elements =
205+
node.$refLink?.definition.isAssociation &&
206+
(node.$refLink.definition.elements || node.$refLink.definition.foreignKeys)
207+
if (node.$refLink && (!elements || !(child.$refLink.alias in elements)))
206208
// foreign key access
207209
node.$refLink.onlyForeignKeyAccess = false
208210

db-service/test/cds-infer/negative.test.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ describe('negative', () => {
117117
let query = CQL`SELECT from bookshop.Books:name { * }` // name does not exist
118118
expect(() => _inferred(query)).to.throw(/No association name in entity bookshop.Books/)
119119
let fromEndsWithScalar = CQL`SELECT from bookshop.Books:title { * }`
120-
expect(() => _inferred(fromEndsWithScalar)).to.throw(/No association title in entity bookshop.Books/)
120+
expect(() => _inferred(fromEndsWithScalar)).to.throw(/Query source must be a an entity or an association/)
121121
})
122122

123123
// queries with multiple sources are not supported for cqn4sql transformation (at least for now)

db-service/test/cqn2sql/insert.test.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@ const cds = require('@sap/cds/lib')
55
const cqn2sql = require('../../lib/cqn2sql')
66

77
beforeAll(async () => {
8-
cds.model = await cds.load(__dirname + '/testModel').then(cds.compile.for.nodejs)
8+
let model = await cds.load(__dirname + '/testModel').then(cds.linked)
9+
cds.model = cds.compile.for.nodejs(JSON.parse(JSON.stringify(model)))
910
})
1011

1112
describe('insert', () => {

db-service/test/cqn4sql/DELETE.test.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ describe('DELETE', () => {
7070
expect(query.DELETE).to.deep.equal(expected.DELETE)
7171
})
7272
it('DELETE with where exists expansion and path expression', () => {
73-
cds.model = cds.compile.for.nodejs(cds.model)
73+
cds.model = cds.compile.for.nodejs(JSON.parse(JSON.stringify(cds.model)))
7474
const { DELETE } = cds.ql
7575
let d = DELETE.from('bookshop.Books:author').where(`books.title = 'Harry Potter'`)
7676
const query = cqn4sql(d)

db-service/test/cqn4sql/calculated-elements.test.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -680,7 +680,7 @@ describe('Unfolding calculated elements in select list', () => {
680680
// at the leaf of a where exists path, there must be an association
681681
// calc elements can't end in an association, hence this does not work, yet.
682682
expect(() => cqn4sql(CQL`SELECT from booksCalc.Books:youngAuthorName { ID }`, model)).to.throw(
683-
'No association “youngAuthorName” in entity “booksCalc.Books”',
683+
'Query source must be a an entity or an association',
684684
)
685685
})
686686

db-service/test/cqn4sql/column.element.test.js

+9-3
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ describe('assign element onto columns with flat model', () => {
6969
let model
7070
beforeAll(async () => {
7171
model = cds.model = await cds.load('db/schema').then(cds.linked)
72-
model = cds.compile.for.nodejs(model)
72+
model = cds.compile.for.nodejs(JSON.parse(JSON.stringify(model)))
7373
})
7474

7575
it('foreign key is adjacent to its association in flat model', () => {
@@ -131,8 +131,14 @@ describe('assign element onto columns with flat model', () => {
131131
expect(query).to.deep.eql(expected)
132132
expect(query.SELECT.columns[0]).to.have.property('element').that.eqls(AssocWithStructuredKey.elements.ID)
133133
// foreign key is part of flat model
134-
expect(query.SELECT.columns[1]).to.have.property('element').that.eqls(AssocWithStructuredKey.elements.toStructuredKey_struct_mid_leaf)
135-
expect(query.SELECT.columns[2]).to.have.property('element').that.eqls(AssocWithStructuredKey.elements.toStructuredKey_struct_mid_anotherLeaf)
134+
if(model.meta.unfolded) { //> REVISIT: Remove once unfolded csn is standard
135+
expect(query.SELECT.columns[1]).to.have.property('element').that.eqls(AssocWithStructuredKey.elements.toStructuredKey_struct_mid_leaf.__proto__)
136+
expect(query.SELECT.columns[2]).to.have.property('element').that.eqls(AssocWithStructuredKey.elements.toStructuredKey_struct_mid_anotherLeaf.__proto__)
137+
} else {
138+
expect(query.SELECT.columns[1]).to.have.property('element').that.eqls(AssocWithStructuredKey.elements.toStructuredKey_struct_mid_leaf)
139+
expect(query.SELECT.columns[2]).to.have.property('element').that.eqls(AssocWithStructuredKey.elements.toStructuredKey_struct_mid_anotherLeaf)
140+
}
141+
136142

137143
expect(query.SELECT.columns[3]).to.have.property('element').that.eqls(AssocWithStructuredKey.elements.toStructuredKey_second)
138144
})

db-service/test/cqn4sql/expand.test.js

+3-3
Original file line numberDiff line numberDiff line change
@@ -306,7 +306,7 @@ describe('Unfold expands on associations to special subselects', () => {
306306
`
307307
// seems to only happen with the `for.nodejs(…)` compiled model
308308
expandQuery.SELECT.localized = true
309-
expect(JSON.parse(JSON.stringify(cqn4sql(expandQuery, cds.compile.for.nodejs(model))))).to.deep.equal(expected)
309+
expect(JSON.parse(JSON.stringify(cqn4sql(expandQuery, cds.compile.for.nodejs(JSON.parse(JSON.stringify(model))))))).to.deep.equal(expected)
310310
})
311311

312312
it('add where exists <assoc> shortcut to expand subquery where condition', () => {
@@ -1021,7 +1021,7 @@ describe('Unfold expands on associations to special subselects', () => {
10211021
}
10221022
}
10231023
`
1024-
let transformed = cqn4sql(q, cds.compile.for.nodejs(model))
1024+
let transformed = cqn4sql(q, cds.compile.for.nodejs(JSON.parse(JSON.stringify(model))))
10251025
expect(JSON.parse(JSON.stringify(transformed))).to.deep.eql(CQL`
10261026
SELECT from Collaborations as Collaborations {
10271027
Collaborations.id,
@@ -1121,7 +1121,7 @@ describe('expand on structure part II', () => {
11211121
(SELECT department.id, department.name from Department as department where Employee.department_id = department.id) as department,
11221122
(SELECT assets.id, assets.descr from Assets as assets where Employee.id = assets.owner_id) as assets
11231123
}`
1124-
expect(JSON.parse(JSON.stringify(cqn4sql(expandQuery, cds.compile.for.nodejs(model))))).to.eql(expected)
1124+
expect(JSON.parse(JSON.stringify(cqn4sql(expandQuery, cds.compile.for.nodejs(JSON.parse(JSON.stringify(model))))))).to.eql(expected)
11251125
})
11261126

11271127
it('multi expand with star but foreign key does not survive in structured mode', () => {

db-service/test/cqn4sql/flattening.test.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ describe('Flattening', () => {
164164
genre,
165165
genre_ID
166166
}`,
167-
cds.linked(cds.compile.for.odata(model)),
167+
cds.linked(cds.compile.for.nodejs(JSON.parse(JSON.stringify(model)))),
168168
)
169169
expect(query).to.deep.eql(CQL`SELECT from bookshop.Books as Books {
170170
Books.ID,

db-service/test/cqn4sql/search.test.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ describe('Replace attribute search by search predicate', () => {
9494
CQL`
9595
SELECT from bookshop.Books as Books
9696
left join bookshop.Authors as author on author.ID = Books.author_ID
97-
left join bookshop.Books as books2 on books2.author_ID = author.ID
97+
left join bookshop.Books as books2 on books2.author_ID = author.ID
9898
{
9999
Books.ID,
100100
books2.title as authorsBook

db-service/test/cqn4sql/table-alias.test.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -1002,7 +1002,7 @@ describe('table alias access', () => {
10021002
}
10031003
expect(error.message).to.match(/author_ID/)
10041004

1005-
const nodeModel = cds.compile.for.nodejs(model)
1005+
const nodeModel = cds.compile.for.nodejs(JSON.parse(JSON.stringify(model)))
10061006
const repeat = cqn4sql(resultCopy, nodeModel)
10071007

10081008
// Ensure sure that the where clause does not change

db-service/test/cqn4sql/tupleExpansion.test.js

+16-8
Original file line numberDiff line numberDiff line change
@@ -262,9 +262,9 @@ describe('Structural comparison', () => {
262262
})
263263

264264
it('expands comparison also in exists subquery', () => {
265-
const queryString = `SELECT from bookshop.AssocWithStructuredKey[toStructuredKey = null]:accessGroup { ID }`
266-
let query = cqn4sql(CQL(queryString), model)
267-
const expectedQueryString = `
265+
const queryString = `SELECT from bookshop.AssocWithStructuredKey[toStructuredKey = null]:accessGroup { ID }`
266+
let query = cqn4sql(CQL(queryString), model)
267+
const expectedQueryString = `
268268
SELECT from bookshop.AccessGroups as accessGroup
269269
{ accessGroup.ID }
270270
where exists (
@@ -275,7 +275,7 @@ describe('Structural comparison', () => {
275275
AssocWithStructuredKey.toStructuredKey_second = null
276276
)
277277
`
278-
expect(query).to.deep.equal(CQL(expectedQueryString))
278+
expect(query).to.deep.equal(CQL(expectedQueryString))
279279
})
280280

281281
it('compare assocs with multiple keys', () => {
@@ -405,7 +405,7 @@ describe('Structural comparison', () => {
405405
})
406406
})
407407
it('Struct needs to be unfolded in on-condition of join', () => {
408-
const queryString = `SELECT from bookshop.Unmanaged {
408+
const query = CQL`SELECT from bookshop.Unmanaged {
409409
toSelf.field
410410
}`
411411

@@ -415,8 +415,16 @@ describe('Structural comparison', () => {
415415
toSelf.field as toSelf_field
416416
}
417417
`
418-
// const unfolded = cds.compile.for.nodejs(JSON.parse(JSON.stringify(model)))
419-
const res = cqn4sql(CQL(queryString), model)
420-
expect(res).to.eql(expected)
418+
const unfolded = cds.compile.for.nodejs(JSON.parse(JSON.stringify(model)))
419+
const structuredRes = cqn4sql(query, model)
420+
const unfoldedRes = cqn4sql(query, unfolded)
421+
//> REVISIT: remove fallback once UCSN is the new standard
422+
if(unfolded.meta.unfolded) {
423+
expect(structuredRes).to.eql(expected).to.eql(unfoldedRes)
424+
} else {
425+
// with odata csn, the on condition of the assoc is wrapped in xpr
426+
expect(structuredRes.SELECT.from.on).to.eql(expected.SELECT.from.on).to.eql(unfoldedRes.SELECT.from.on[0].xpr)
427+
expect(structuredRes.SELECT.columns).to.eql(expected.SELECT.columns).to.eql(unfoldedRes.SELECT.columns)
428+
}
421429
})
422430
})

db-service/test/cqn4sql/where-exists.test.js

+3-2
Original file line numberDiff line numberDiff line change
@@ -715,7 +715,7 @@ describe('EXISTS predicate in infix filter', () => {
715715
)
716716
`
717717
expect(() => {
718-
cqn4sql(q, cds.compile.for.nodejs(model))
718+
cqn4sql(q, cds.compile.for.nodejs(JSON.parse(JSON.stringify(model))))
719719
}).to.throw(/Only foreign keys of "participant" can be accessed in infix filter/)
720720
})
721721
})
@@ -1294,11 +1294,12 @@ describe('Path expressions in from combined with `exists` predicate', () => {
12941294
})
12951295
})
12961296

1297+
12971298
describe('cap issue', () => {
12981299
let model
12991300
beforeAll(async () => {
13001301
model = cds.model = await cds.load(__dirname + '/model/cap_issue').then(cds.linked)
1301-
model = cds.compile.for.nodejs(model)
1302+
model = cds.compile.for.nodejs(JSON.parse(JSON.stringify(model)))
13021303
})
13031304
it('MUST ... two EXISTS both on same path in where with real life example', () => {
13041305
// make sure that in a localized scenario, all aliases

0 commit comments

Comments
 (0)