I was hoping to refactor and move this line: this.cachedProperties.put(filename, propHolder);

from line 461 to line 395

Rationale: This allows me to extend ReloadableResourceBundleMessageSource and override just refreshProperties() and loadProperties() in the child class to extend reading and loading resources from a different source than .xml or .properties. My use case requires me to use JSON or db query

@Override
protected PropertiesHolder refreshProperties(String filename, @Nullable PropertiesHolder propHolder){
    PropertiesHolder prop = super.refreshProperties(filename, propHolder);
    if (propHolder.getProperties().isEmpty()){
        //try refreshing as json. since XML and  prop are not present)
    }
}

And This Class has a lot of useful reload logic which is pointless to reimplement if the functionality is already present.

Comment From: snicoll

Thanks for sharing your use case @Sahilshetye.

Comment From: simonbasle

(edit: oops, I had this issue opened in a stale tab and didn't see it was assigned 😉 )

@Sahilshetye do you absolutely need to override refreshProperties? Moving that line makes the logic a bit harder to follow and to prove that the correct propHolder has been added to the cache, so I'm not a fan of that proposal.

Maybe instead we could make the set of extensions tried within refreshProperties overridable. You would then just override this new getResourceExtensions method as well as loadProperties, and that's it.

I need to discuss this with the team, but does that sound like a plausible solution to your particular use case?

Comment From: snicoll

Closing in favor of PR #30369

Comment From: Sahilshetye

der to follow and to prove that the correct propHolder has been added to the cache, so I'm not a fan of that proposal.

Works for me.