-
Notifications
You must be signed in to change notification settings - Fork 11
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
fix: reject all path expressions w/o foreign keys #806
Merged
Merged
Changes from all commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
233af38
fix: dont use virtual key elements for `UPDATE` where condition
patricebender 893a863
add test
patricebender 1051200
Merge branch 'patrice/test-issue' into patrice/draft
patricebender 0c193c4
add end 2 end test
patricebender b7cec64
Merge branch 'main' into patrice/draft
patricebender a756755
trailing ws
patricebender d46bce8
remove annotation
patricebender 2564fb7
indent
patricebender 827bcf9
fix: reject all path expressions w/o foreign keys
patricebender 86a70cc
Merge branch 'main' into patrice/keyless
patricebender 033fc07
check also for expand and exists
patricebender 1a62434
align error message
patricebender d6af9ed
reject backlinks w/o fks for joins etc
patricebender 3e66e78
Merge branch 'main' into patrice/keyless
patricebender 2893d1a
Merge branch 'main' into patrice/keyless
patricebender 031e294
add test with calculated element
patricebender d9f214d
adapt tests after addition of calc elem
patricebender File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
'use strict' | ||
|
||
/** | ||
* Formats a ref array into a string representation. | ||
* If the first step is an entity, the separator is a colon, otherwise a dot. | ||
* | ||
* @param {Array} ref - The reference array to be formatted. | ||
* @param {Object} model - The model object containing definitions. | ||
* @returns {string} The formatted string representation of the reference. | ||
*/ | ||
function prettyPrintRef(ref, model = null) { | ||
return ref.reduce((acc, curr, j) => { | ||
if (j > 0) { | ||
if (j === 1 && model?.definitions[ref[0]]?.kind === 'entity') { | ||
acc += ':' | ||
} else { | ||
acc += '.' | ||
} | ||
} | ||
return acc + `${curr.id ? curr.id + '[…]' : curr}` | ||
}, '') | ||
} | ||
|
||
// export the function to be used in other modules | ||
module.exports = { | ||
prettyPrintRef, | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,112 @@ | ||
/** | ||
* Make sure we issue proper errors in case of path expression along keyless entities | ||
*/ | ||
'use strict' | ||
|
||
const cqn4sql = require('../../lib/cqn4sql') | ||
const cds = require('@sap/cds') | ||
const { expect } = cds.test | ||
describe('keyless entities', () => { | ||
let model | ||
|
||
beforeAll(async () => { | ||
model = await cds.load(__dirname + '/model/keyless').then(cds.linked) | ||
}) | ||
describe('managed assocs', () => { | ||
it('no foreign keys for join', () => { | ||
const { Books } = model.entities | ||
const q = SELECT.columns('ID').from(Books).where(`author[ID = 42].book[ID = 42].author.name LIKE 'King'`) | ||
expect(() => cqn4sql(q, model)).to.throw( | ||
'Path step “author” of “author[…].book[…].author.name” has no foreign keys', | ||
) | ||
// ok if explicit foreign key is used | ||
const qOk = SELECT.columns('ID').from(Books).where(`authorWithExplicitForeignKey[ID = 42].name LIKE 'King'`) | ||
expect(cqn4sql(qOk, model)).to.eql( | ||
CQL`SELECT Books.ID FROM Books as Books | ||
left join Authors as authorWithExplicitForeignKey | ||
on authorWithExplicitForeignKey.ID = Books.authorWithExplicitForeignKey_ID | ||
and authorWithExplicitForeignKey.ID = 42 | ||
where authorWithExplicitForeignKey.name LIKE 'King'`, | ||
) | ||
}) | ||
it('no foreign keys for join (2)', () => { | ||
const { Authors } = model.entities | ||
const q = SELECT.from(Authors).where(`book.authorWithExplicitForeignKey.book.my.author LIKE 'King'`) | ||
expect(() => cqn4sql(q, model)).to.throw( | ||
'Path step “author” of “book.authorWithExplicitForeignKey.book.my.author” has no foreign keys', | ||
) | ||
}) | ||
it('scoped query leading to where exists subquery cant be constructed', () => { | ||
const q = SELECT.from('Books:author') | ||
expect(() => cqn4sql(q, model)).to.throw(`Path step “author” of “Books:author” has no foreign keys`) | ||
|
||
// ok if explicit foreign key is used | ||
const qOk = SELECT.from('Books:authorWithExplicitForeignKey').columns('ID') | ||
expect(cqn4sql(qOk, model)).to.eql( | ||
CQL`SELECT authorWithExplicitForeignKey.ID FROM Authors as authorWithExplicitForeignKey | ||
where exists ( | ||
SELECT 1 from Books as Books where Books.authorWithExplicitForeignKey_ID = authorWithExplicitForeignKey.ID | ||
)`, | ||
) | ||
}) | ||
it('where exists predicate cant be transformed to subquery', () => { | ||
const q = SELECT.columns('ID').from('Books').where('exists author') | ||
expect(() => cqn4sql(q, model)).to.throw(`Path step “author” of “author” has no foreign keys`) | ||
// ok if explicit foreign key is used | ||
const qOk = SELECT.from('Books').columns('ID').where('exists authorWithExplicitForeignKey') | ||
expect(cqn4sql(qOk, model)).to.eql( | ||
CQL`SELECT Books.ID FROM Books as Books | ||
where exists ( | ||
SELECT 1 from Authors as authorWithExplicitForeignKey where authorWithExplicitForeignKey.ID = Books.authorWithExplicitForeignKey_ID | ||
)`, | ||
) | ||
}) | ||
it('correlated subquery for expand cant be constructed', () => { | ||
const q = CQL`SELECT author { name } from Books` | ||
expect(() => cqn4sql(q, model)).to.throw(`Can't expand “author” as it has no foreign keys`) | ||
// ok if explicit foreign key is used | ||
const qOk = CQL`SELECT authorWithExplicitForeignKey { name } from Books` | ||
expect(JSON.parse(JSON.stringify(cqn4sql(qOk, model)))).to.eql( | ||
CQL` | ||
SELECT | ||
( | ||
SELECT authorWithExplicitForeignKey.name from Authors as authorWithExplicitForeignKey | ||
where Books.authorWithExplicitForeignKey_ID = authorWithExplicitForeignKey.ID | ||
) as authorWithExplicitForeignKey | ||
from Books as Books`, | ||
) | ||
}) | ||
|
||
it('calculated element navigates along keyless assoc', () => { | ||
const q = SELECT.from('Books').columns('authorName') | ||
expect(() => cqn4sql(q, model)).to.throw(`Path step “author” of “author.name” has no foreign keys`) | ||
}) | ||
}) | ||
describe('managed assocs as backlinks', () => { | ||
it('backlink has no foreign keys for join', () => { | ||
const { Authors } = model.entities | ||
const q = SELECT.from(Authors).where(`bookWithBackLink.title LIKE 'Potter'`) | ||
expect(() => cqn4sql(q, model)).to.throw( | ||
`Path step “bookWithBackLink” is a self comparison with “author” that has no foreign keys`, | ||
) | ||
}) | ||
it('backlink has no foreign keys for scoped query', () => { | ||
const q = SELECT.columns('ID').from('Authors:bookWithBackLink') | ||
expect(() => cqn4sql(q, model)).to.throw( | ||
`Path step “bookWithBackLink” is a self comparison with “author” that has no foreign keys`, | ||
) | ||
}) | ||
it('backlink has no foreign keys for where exists subquery', () => { | ||
const q = SELECT.from('Authors').where('exists bookWithBackLink') | ||
expect(() => cqn4sql(q, model)).to.throw( | ||
`Path step “bookWithBackLink” is a self comparison with “author” that has no foreign keys`, | ||
) | ||
}) | ||
it('backlink has no foreign keys for expand subquery', () => { | ||
const q = CQL`SELECT bookWithBackLink { title } from Authors` | ||
expect(() => cqn4sql(q, model)).to.throw( | ||
`Path step “bookWithBackLink” is a self comparison with “author” that has no foreign keys`, | ||
) | ||
}) | ||
}) | ||
}) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
// path expressions along `Books:author` are not possible | ||
entity Books { | ||
key ID : Integer; | ||
title : String; | ||
stock : Integer; | ||
author : Association to Authors; | ||
authorName: String = author.name; | ||
authorWithExplicitForeignKey: Association to Authors { ID }; | ||
my: Association to Books; | ||
} | ||
|
||
entity Authors { | ||
ID : Integer; | ||
name : String; | ||
book: Association to Books; | ||
// backlink has no foreign keys... | ||
bookWithBackLink: Association to Books on bookWithBackLink.author = $self; | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there was a calculated element
Would selecting
authorName
in a query have to be handled separately?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point, I added this as a test case → does not need to be handled separately :)