ICU-23309 Add ClassLoader to caching key in ResourceBundleWrapper#3843
ICU-23309 Add ClassLoader to caching key in ResourceBundleWrapper#3843own-mind wants to merge 1 commit intounicode-org:mainfrom
Conversation
d32a296 to
25ca1e8
Compare
|
Notice: the branch changed across the force-push!
~ Your Friendly Jira-GitHub PR Checker Bot |
mihnita
left a comment
There was a problem hiding this comment.
I have a few nitpicks, nothing big.
Thank you!
Mihai
icu4j/main/core/src/test/resources/com/ibm/icu/dev/test/locale/first/localization.properties
Outdated
Show resolved
Hide resolved
icu4j/main/core/src/test/resources/com/ibm/icu/dev/test/locale/first/localization_de.properties
Outdated
Show resolved
Hide resolved
icu4j/main/core/src/test/resources/com/ibm/icu/dev/test/locale/second/localization.properties
Outdated
Show resolved
Hide resolved
...j/main/core/src/test/resources/com/ibm/icu/dev/test/locale/second/localization_de.properties
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
This file is generated.
So these changes will be lost next time.
I would say don't touch this file.
It is not related to anything this PR is doing.
It is not your problem to solve :-)
And if you want to solve it, great, I appreciate it!
But it would be in a different PR, and would also update the generator.
There was a problem hiding this comment.
Why reformat this file?
I know it is enforced now.
But I don't see any changes relevant to this PR.
So there is no need to reformat if it is not part of the PR.
If there are changes that are PR related then I don't see them
(one of the reasons we usually ask for these kind of changes to have 2 commits, one with change proper and one with the re-formatting.
Too late now...
But tldr:
- if there is a change here that is related to the PR, then please help me see it, because I can't find it
- if there is nothing PR related then just drop the file
There was a problem hiding this comment.
I have force-pushed the fixes, now without those extra files. I was complying with PR checks regarding formatting and single-commit, but I guess those checks aren't as important :)
icu4j/main/core/src/main/java/com/ibm/icu/impl/ResourceBundleWrapper.java
Outdated
Show resolved
Hide resolved
icu4j/main/core/src/main/java/com/ibm/icu/impl/ResourceBundleWrapper.java
Outdated
Show resolved
Hide resolved
25ca1e8 to
8c2dd91
Compare
|
Notice: the branch changed across the force-push!
~ Your Friendly Jira-GitHub PR Checker Bot |
When using
UResourceBundle.getBundleInstance(baseName, locale, classloader), in cases whenRootTypeof the bundles isRootType.JAVA(for example,.propertybundles), ResourceBundleWrapper is used as implementation forUResourceBundle. Theclassloaderis provided to tell the implementation where to get the resources for the bundle.However, the caching used in
ResourceBundleWrapperfor caching already discovered bundles does not respect the used classloader for those bundles, only the base name. Thus, if there are multiple bundles are requested with the same base name but different classloader, only the first discovered will be returned.Changes made:
BundleCacheKeyclass was added that storesbaseNameandclassLoaderto be used inResourceBundleWrapper.BUNDLE_CACHEinstead ofStringkey (that only reflected thebaseName)Checklist