The only challenge I saw , since most components are initialised as beans by Spring auto-wiring , which are by default singleton scoped ,Spring only creates a single instance of those ,at the application initialization hence only takes up the Global property available at initialization.
When the Global property is changed , it means restarting the app to re-iniiatialise new instances that take those Global properties.
Probably the solution is to change the default scope of the beans
It’s probably preferable to have an alternative to using global properties for this. Fundamentally, they are the wrong place to store passwords, because they are essentially accessible by anyone (including unauthenticated users) in plain-text. Also, it would be nice to have a model that supports connections to more than one CR.
On the “liveness” issue, there are two patterns that we use:
Just load the GP value from the Admin service every time it’s needed. This usually performs well enough.
Load the properties, but then register a Global Property listener to be notified of updates to them.
In general, 1 is the pattern we most commonly use.
That means we need to directly inniatialise the FhirConfig object rather than autowiring it as a bean so that a new object is created on request or when the GP changes ??
First, I would never recommend using @Autowired in OpenMRS except in very specific circumstances. It sort of works in the FHIR2 module because we very carefully control when any autowired beans are actually materialised (basically, all the FHIR2 beans are materialised from the servlet(s), which are not themselves Spring components).
Unfortunately, because of how OpenMRS uses and configures Spring, relying on @Component, @Autowired and context:component-scan almost always results in Spring processing each bean more than once; sometimes twice, often many more times. Much of the reason for this is how the XML element context:component-scan actually works. If you then add your own context:component-scan, Spring does not do any de-depulication and just reprocesses your beans as new bean configurations (technical implementation; every context:component-scan element results in a call to parse() on that class).
There are a few other confounding factors that make this process more painful. One is that the ComponentScanBeanDefinitionParser creates it’s own ClassPathBeanDefinitionScanner, which means that each time the ComponentScanBeanDefinitionParser is invoked, it scans the full classpath. Our classpath size varies at execution time, but in practice, during the initial Spring refresh (which is when the “long, hanging” issue usually arises), scanning the classpath means loading each JAR provided by the application and each JAR provided by any OMODs installed in the application (Java—at least up until Java 8—does not provide an API for a JAR to provide metadata about which classes it includes, so classpath scans tend to be implemented as some variation of “expand the JAR and enumerate all the class files it contains”—Spring aggressively caches this, but the number of JARs on the classpath can be enormous, and fundamentally needs to run through the OpenmrsClassLoader, which is probably less efficient than it could be). The second is that it’s common for modules (including non-community modules) to use packages in the org.openmrs namespace, which is fine, but [we already define a component-scan that covers those elements](More precisely, if your module uses classes in a package starting with org.openmrs, we already have a component-scan running that covers these). This actually results in the Spring context containing multiple bean definitions for every bean defined in a module using @Component, and results in Spring having to do an expensive de-duplication procedure to process every @Autowired definition (I actually don’t know how this gets resolved).
The up-shot of the above is:
Prefer to just use Spring’s XML configuration over annotations in OpenMRS; it behaves much more closely to the way you will think it will behave.
If you must use @Autowired, be very careful about defining a new component-scan and only do that if you absolutely need to. Think very carefully about how your component-scan is configured.
(@mogoodrich and @mseaton: I know we’ve discussed the @Autowired thing many times; the above is my current understanding of some of the issues; @raff and @dkayiwa, if either of you have any insights—including why I’m wrong—I’d appreciate it; this is one of those things that results in bad DevX, because even Spring’s documentation favours describing annotation configuration rather than XML configuration…)
Don’t autowire admin service. Just change those to Context.getAdministrativeService() calls; this resolves to the admin bean and does not add much overhead versus autowiring it anyways. It’s not dependency injection, but it works much better in the OpenMRS context. If you follow the above (and remove any context-scan elements from the module), it will likely work ok.
@tendomart Pattern 1 is Context.getAdministrativeService().getGlobalProperty().
However, we’ve basically used @Autowired in freely outside of services. Based on your comments, we may have to reconsider this as well?
Doing a grep of my module code base I see many cases where were are doing conponent-scans within modules, are you saying we can and should remove these to potentially see faster startup time?
Well, if it hasn’t been causing noticeable issues, I think it’s fine not to worry about too much. I’m still doing at lot of guessing at what’s actually happening because tracing through Spring’s bean loading code is a bit vertigo-inducing.
Thanks @ibacher. I guess I should try removing them at some point and see it if affects performance. All we really know is that startup is slow, and that autowiring in services makes it really, really slow… it would be great to move the startup from “slow” to “not that slow”…
But the idea is that these should no longer be necessary, at least for more recent versions of OpenMRS?
Well, yes. I mean, they shouldn’t be necessary for any version of OpenMRS, really, as long as the packages starts with org.openmrs (we don’t, for instance, create a child Spring context per module; anything that’s in a webModuleApplicationContext.xml file or moduleApplicationContext.xml file just gets merged into the One Big Spring Context when the refresh is done; see here). Anything with org.pih, for example (and I have a few edu.brown things around, etc.) probably needs a component scan. Even in those case, though it would be good to copy the filter list from core:
<context:include-filter type="annotation" expression="org.openmrs.annotation.Handler"/>
<context:include-filter type="custom" expression="org.openmrs.annotation.OpenmrsProfileIncludeFilter"/>
<!-- Excludes classes with unit test super classes -->
<context:exclude-filter type="custom"
expression="org.openmrs.util.TestTypeFilter"/>
<context:exclude-filter type="custom" expression="org.openmrs.annotation.OpenmrsProfileExcludeFilter"/>
Since I think that’s how the @OpenmrsProfile annotation has any effect at all.
