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 explicit composite cache key elements values and improve doc #11046

Merged

Conversation

gwenneg
Copy link
Member

@gwenneg gwenneg commented Jul 28, 2020

Fixes #10436.

cc @gsmet @geoand

@@ -327,6 +327,49 @@ method annotated with `@CacheResult` or `@CacheInvalidate`.

This annotation is optional and should only be used when some of the method arguments are NOT part of the cache key.

=== Composite cache key building logic

When a cache key is built from several method arguments, whether they are explicitly identified with `@CacheKey` or not, the building logic depends on these arguments order in the method signature. On the other hand, the arguments names are not used at all and do not have any effect on the cache key.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
When a cache key is built from several method arguments, whether they are explicitly identified with `@CacheKey` or not, the building logic depends on these arguments order in the method signature. On the other hand, the arguments names are not used at all and do not have any effect on the cache key.
When a cache key is built from several method arguments, whether they are explicitly identified with `@CacheKey` or not, the building logic depends on the order of these arguments in the method signature. On the other hand, the arguments names are not used at all and do not have any effect on the cache key.

Copy link
Contributor

@geoand geoand left a comment

Choose a reason for hiding this comment

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

Thanks!

I added a small doc suggestion

@gwenneg gwenneg force-pushed the issue-10436-cache-key-parameters-positions branch from 304c08a to 133263b Compare July 29, 2020 08:59
@gwenneg
Copy link
Member Author

gwenneg commented Jul 29, 2020

Suggestion applied, thanks for the review @geoand!

Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

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

Nice work, thanks!

@gsmet gsmet added this to the 1.7.0 - master milestone Jul 29, 2020
@gsmet gsmet added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Jul 29, 2020
@machi1990 machi1990 merged commit 8b69ba0 into quarkusio:master Jul 29, 2020
@gwenneg gwenneg deleted the issue-10436-cache-key-parameters-positions branch July 29, 2020 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/cache area/documentation triage/waiting-for-ci Ready to merge when CI successfully finishes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

@CacheKey depending on order of properties
4 participants