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

fix: avoid default limit 50 on catalog creation #2168

Merged

Conversation

ndr-brt
Copy link
Member

@ndr-brt ndr-brt commented Nov 7, 2022

What this PR changes/adds

Avoids the default limit of 50 when requesting catalog in certain circumstances.
To achieve that:

  • always set limit by default at the asset count retrieved by the countAssets call in the ContractOfferResolverImpl.queryContractOffers method.
  • ignore offset and limit in the countAssets implementation, as it should count items filtering by the query, but not restricting the output with a range.

Why it does that

Fix bug

Further notes

  • could we remove set the QuerySpec.limit's default value to Integer.MAX_VALUE? As the "security issue" is already handled by QuerySpecDto.

Linked Issue(s)

Closes #2064

Checklist

  • added appropriate tests?
  • performed checkstyle check locally?
  • added/updated copyright headers?
  • documented public classes/methods?
  • added/updated relevant documentation?
  • assigned appropriate label? (exclude from changelog with label no-changelog)
  • formatted title correctly? (take a look at the CONTRIBUTING and Etiquette for pull requests for details)

@ndr-brt ndr-brt added the bug Something isn't working label Nov 7, 2022
@ndr-brt ndr-brt requested a review from DominikPinsel November 7, 2022 10:55
@ndr-brt ndr-brt temporarily deployed to Azure-dev November 7, 2022 10:55 Inactive
@ndr-brt ndr-brt temporarily deployed to Azure-dev November 7, 2022 13:30 Inactive
Copy link
Member

@paullatzelsperger paullatzelsperger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ndr-brt ndr-brt force-pushed the fix/2064-avoid-default-limit-50 branch from d02c6d3 to 3927c58 Compare November 7, 2022 15:16
@ndr-brt ndr-brt temporarily deployed to Azure-dev November 7, 2022 15:17 Inactive
@ndr-brt ndr-brt merged commit 3aa192a into eclipse-edc:main Nov 8, 2022
@ndr-brt ndr-brt deleted the fix/2064-avoid-default-limit-50 branch November 8, 2022 07:30
ndr-brt added a commit to Think-iT-Labs/edc-connector that referenced this pull request Feb 10, 2023
* fix: avoid default limit to 50

* chore: refactor contract offer resolver

* chore: countAssets by criteria, not query
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Selector of ContractDefinition targets 50 Assets max. (= 50 ContractOffers max. per Definition)
2 participants