Skip to content
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

use NonEmpty for all arguments to Queries methods where empty lists don't work #11155

Closed
S11001001 opened this issue Oct 6, 2021 · 0 comments · Fixed by #12341
Closed

use NonEmpty for all arguments to Queries methods where empty lists don't work #11155

S11001001 opened this issue Oct 6, 2021 · 0 comments · Fixed by #12341
Assignees
Labels
component/json-api HTTP JSON API gardening Cleanup code, improve tests, better automation, upgrade deps, fix flaky tests, address TODOs, etc. good first issue Good for newcomers team/ledger-clients Related to the Ledger Clients team's components. wip-issue This issue is being worked on. Use draft PRs for work in progress PRs.

Comments

@S11001001
Copy link
Contributor

S11001001 commented Oct 6, 2021

There are several functions like selectContractsMultiTemplate in Queries that will always fail if given an empty input of e.g. template/predicate pairs. Currently this isn't apparent from the signature, but it hasn't been changed because the callers in ContractDao aren't trivial.

We should just percolate the NonEmpty requirement via NonEmpty types all the way back to where we parse client arguments from JSON. This might entail extending the method enrichments in NonEmpty (e.g. map is a major missing method map now exists 🎉 ), or adding overrides to the Foldable1 instance therein (also done in #11592), and of course eliminating any needless code where we handle an impossible empty-data case, starting with where we make these assertions in Queries in the first place.

With respect to #11156, any party sets should use NonEmpty too, even though the current examples use OneAnd.

@S11001001 S11001001 added good first issue Good for newcomers component/json-api HTTP JSON API team/ledger-clients Related to the Ledger Clients team's components. labels Oct 6, 2021
@S11001001 S11001001 self-assigned this Jan 10, 2022
@S11001001 S11001001 added the wip-issue This issue is being worked on. Use draft PRs for work in progress PRs. label Jan 10, 2022
@S11001001 S11001001 added the gardening Cleanup code, improve tests, better automation, upgrade deps, fix flaky tests, address TODOs, etc. label Mar 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/json-api HTTP JSON API gardening Cleanup code, improve tests, better automation, upgrade deps, fix flaky tests, address TODOs, etc. good first issue Good for newcomers team/ledger-clients Related to the Ledger Clients team's components. wip-issue This issue is being worked on. Use draft PRs for work in progress PRs.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant