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

Polish Mono cache operators documentation #2790

Merged
merged 2 commits into from Sep 24, 2021

Conversation

simonbasle
Copy link
Member

This commit polishes the javadoc of most cache variants in Mono:

  • explicit the behavior in case of concurrent subscriptions for
    cache(), cache(Duration) and cache(Function)
  • polish (un)ordered lists and paragraph breaks in cacheInvalidateIf
    and cacheInvalidateWhen javadoc.

Fixes #2782.

This commit polishes the javadoc of most cache variants in Mono:
 - explicit the behavior in case of concurrent subscriptions for
 `cache()`, `cache(Duration)` and `cache(Function)`
 - polish (un)ordered lists and paragraph breaks in `cacheInvalidateIf`
 and `cacheInvalidateWhen` javadoc.

Fixes #2782.
@simonbasle simonbasle requested a review from a team as a code owner September 24, 2021 09:28
@simonbasle simonbasle added this to the 3.4.11 milestone Sep 24, 2021
@simonbasle simonbasle added the type/documentation A documentation update label Sep 24, 2021
@simonbasle simonbasle self-assigned this Sep 24, 2021
@simonbasle
Copy link
Member Author

cc @jzheaux for cache() wording
note that I took this PR as an opportunity to fix the javadoc of cacheInvalidateIf.

Copy link
Contributor

@OlegDokuka OlegDokuka left a comment

Choose a reason for hiding this comment

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

LGTM with some minor wording suggestions

Comment on lines 1822 to 1827
* Cache loading (ie. subscription to the source) can happen either at the very
* first subscription or after the cache has been cleared due to expiry.
* Once that upstream subscription is started, it cannot be cancelled.
* The operator will however prevent multiple concurrent subscriptions from triggering
* duplicated loading (only one load-triggering subscription can win at a time,
* and the others will get notified of the newly cached value when it arrives).
Copy link
Contributor

Choose a reason for hiding this comment

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

How about the following:

Suggested change
* Cache loading (ie. subscription to the source) can happen either at the very
* first subscription or after the cache has been cleared due to expiry.
* Once that upstream subscription is started, it cannot be cancelled.
* The operator will however prevent multiple concurrent subscriptions from triggering
* duplicated loading (only one load-triggering subscription can win at a time,
* and the others will get notified of the newly cached value when it arrives).
* Cache loading (ie. subscription to the source) is triggered by the very
* first subscription to empty or expired cache.
* Once the cache loading is started, it cannot be cancelled.
* The operator guarantees that only a single cash loading may happen at a time
* (only one load-triggering subscription can win at a time,
* and the others will get notified of the newly cached value when it arrives).

Copy link
Contributor

Choose a reason for hiding this comment

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

same should be applied to the following similar changes if makes sence

Copy link
Member Author

Choose a reason for hiding this comment

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

good suggestion. I actually took inspiration and reworded and simplified these sections even further in the last commit, let me know what you think

@OlegDokuka
Copy link
Contributor

@simonbasle Perfecto 👌.

@simonbasle simonbasle merged commit a050dcf into main Sep 24, 2021
@simonbasle simonbasle deleted the 2782-documentMonoCacheProtection branch September 24, 2021 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/documentation A documentation update
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mono.cache should clarify concurrency guarantees
2 participants