Skip to content

Refresh cache in ReloadableResourceBundleMessageSource #31638

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

Closed
wants to merge 2 commits into from

Conversation

vershnik
Copy link

This change allows subclasses to call getMergedProperties(locale) when
cacheSeconds > 0 and get refreshed contents. Useful for SPA webapps to
deliver all translations to the browser.

Since all individual files can be refreshed concurrently, the thread that
refreshes mergedHolder can get stale data for any file being refreshed
so simply comparing maximal fileTimestamp is not enough to detect
change - used hashCode for this purpose.

andreibastun and others added 2 commits November 21, 2023 08:30
This change allows subclasses to call getMergedProperties(locale) when
cacheSeconds > 0 and get refreshed contents. Useful for SPA webapps to
deliver all translations to the browser.

Since all individual files are refreshed concurrently, the thread that
refreshes mergedHolder can get stale data for any file being refreshed
so simply comparing maximal fileTimestamp is not enough to detect
change - used hashCode for this purpose.
@pivotal-cla
Copy link

@vershnik Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@pivotal-cla
Copy link

@vershnik Thank you for signing the Contributor License Agreement!

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Nov 21, 2023
@sbrannen sbrannen added the in: core Issues in core modules (aop, beans, core, context, expression) label Nov 21, 2023
@sbrannen sbrannen changed the title Refresh cache in ReloadableResourceBundleMessageSource Refresh cache in ReloadableResourceBundleMessageSource Nov 21, 2023
@snicoll
Copy link
Member

snicoll commented Nov 21, 2023

@vershnik this PR has a a commit from @andreibastun. The author should be the one submitting the PR as they're the one signing the CLA.

@snicoll snicoll added the status: waiting-for-feedback We need additional information before we can continue label Nov 21, 2023
@andreibastun
Copy link
Contributor

@snicoll Sorry, I didn't notice that my second account was singed in the git when commiting. Now I accepthed CLA from both accounts.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Nov 21, 2023
@snicoll
Copy link
Member

snicoll commented Jan 25, 2024

Thanks for the PR but we're not keen to make ReloadableResourceBundleMessageSource more involved than it already is. We suggest that you implement the refresh in a custom sub-class of yours and if that turns out to be too convoluted, we can discuss how to add a hookpoint to make it easier.

@snicoll snicoll closed this Jan 25, 2024
@snicoll snicoll added status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-triage An issue we've not yet triaged or decided on in: core Issues in core modules (aop, beans, core, context, expression) status: feedback-provided Feedback has been provided labels Jan 25, 2024
@andreibastun
Copy link
Contributor

@snicoll Ok, that makes sense, thanks. I have created another PR 32118 that does not add functionality, but just extracs some methods from getMergedProperties to be more easily reused in subclasses.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants