HV-1131 Deferring retrieval of default MI #556
Conversation
| @@ -400,7 +404,7 @@ public ClassLoader getExternalClassLoader() { | |||
|
|
|||
| @Override | |||
| public final MessageInterpolator getDefaultMessageInterpolator() { | |||
| return defaultMessageInterpolator; | |||
| return new ResourceBundleMessageInterpolator( defaultResourceBundleLocator ); | |||
There was a problem hiding this comment.
I thought about this approach but I must admit I really disliked the fact that we instantiate a new ResourceBundleMessageInterpolator each time we call this method (and getMessageInterpolator). I would expect us to return the same object for each call.
Thinking about it and as I noticed Configuration is not supposed to be thread safe, couldn't we store the interpolators the first time the methods are called? That would solve this issue.
There was a problem hiding this comment.
I'm not really concerned about it (these methods are not expected to be invoked gazillion of times). Is there anything specific you base your expectation for the same instance to be returned upon?
Storing them as you say should work; ConfigurationState (unlike Configuration) doesn't explicitly state that it's not thread-safe, but I think it's a reasonable assumption then that it isn't.
There was a problem hiding this comment.
Just to be clear on my worries: it's named like a getter and it used to work like a getter so I didn't think it was such a trivial change to make it return a different instance for each call. I don't think it's an inappropriate expectation to have it behave like a getter.
Nice! |
… it's actually used
gunnarmorling
force-pushed
the
HV-1131
branch
from
96b223f
to
c9948da
Compare
|
@gsmet Pushed an update. |
|
Ok, I see. FWIW, I don't think that the JavaBeans spec mandates that a
getter always returns the same value. But it's changed as per your request
now.
|
|
Merged, thanks! |
@gsmet I've found an alternative approach for the EL issue. The idea is to defer the retrieval of the default message interpolator until it's actually needed (either by calling
getDefaultMessageInterpolator()or when bootstrapping the VF and no other MI has been given). That way the instantiation of the expression factory can simply be done in the 'ctor ofResourceBundleMessageInterpolator, not requiring any synchronization.I've also rewritten the test so it doesn't need reflection in the actual test routine. I've reworked your commit so it adds the new test and your fixes around the test class loader, but ommitted your original approach for fixing the issue for the sake of a clean history.