Review property placement behavior in context builder
Description
Environment
Confluence content
mentioned on
Activity
This is substantially cleaned up. The one outstanding issue is that injecting a post processor into the builder that already has an ApplicationContext or Environment in place via a parent context is a problem, but that's "coder error" at this point, will add a note to the javadoc about it. Most processors don't need that stuff since they access the context at runtime via the defined hooks.
We control this generally. In fact, we already were injecting the PropertyConfigurer via servces-system.xml which I forgot about, so this is kind of “rationalizing” all this to an extent.
The problem we have is that our tests are very sensitive to these settings and we don’t use them consistently, so it leads one to draw the wrong conclusions about the problems.
I still don’t really understand how the replacement “works” when a Spring Resource is used but not ours, but I assume it’s buried in the differences between a Converter and a PropertyEditor, whatever that is.
The secondary issue I hit is that if for some cases the injected processor classes need access to the Spring context or environment, that wasn’t happening. When you declare it as a bean inside, Spring auto-injects those via Aware interfaces. When you create them on your own and pass them into a builder, it doesn’t. So that was easy to fix as long as it’s understood that the context isn’t necessarily started at the time you inject them.
This is not done because what I did was just a quick fix for the tests, but we need to clean this up and control it better.
OIC you can get the code to do it - the the factory is applicationcontextaware you tell it about it?
How do we find out what BeanFactoryPostProcessor we are using? It’s going to be boring reading all that xml with no IDE support?
Subject to further testing, I believe what I got wrong was that the PropertySourcesPlaceholderConfigurer
class is really just a BeanFactoryPostProcessor, so we can inject our expected version of it into the context when we build it.
I also think we should be checking any injected bean/factory post processors for the need to inject the ApplicationContext and Environment because Spring won’t do that if they’re added by hand, only when they’re located within the context itself at runtime.
So far this change is fixing the tests I saw break, and hasn’t broken anything else. I believe we will be able to remove a lot of those shibboleth.PropertySourcesPlaceholderConfigurer
beans we have all over now, at least from the child contexts we build with this class, but that includes SWF contexts now as well as our services.
While investigating other issues with the converters, I discovered that it appears they all run before property replacement happens in Spring, which I don’t exactly understand since that seems like it would never work right.
Pending whether that’s a sign of some other problem, we need to manually perform the replacement inside the converter to get it to work properly. At a minimum this is crucial for the StringToResource converter.