How to segregate imports of module being 'aware of' away from Spring wiring?

Hi @raff and @dkayiwa,

I would need your science about the usage and patterns around ‘aware of’. Here is the current real world use case:

  • Ext I18N defines an interface: AddressHierarchyI18nCache.
  • Ext I18N wires up a bean for it with AddressHierarchyI18nCacheImpl.
  • AH depends on Ext I18N API.
  • AH is ‘aware of’ Ext I18N.

Is it possible for AH to know and use the interface AddressHierarchyI18nCache when Ext I18N is not loaded? I guess I’m looking for some sort of pattern that hides Ext I18N imports away enough so that they don’t ever come in the way of Spring when Ext I18N is not prsent. Is there such a pattern, would there be examples around?

The only idea I have had so far consists in duplicating Ext I18N’s AddressHierarchyI18nCache interface within AH, and implement each of the duplicated methods using reflection (that’s to get rid of the imports). Roughly:

@Override
public String getterDuplicated() {
 if (ModuleFactory.isModuleStarted("exti18n")) {
  // get the bean using reflection
  // call the original method from the bean using reflection
  return ...
 }
 else {
  return "whatever is ok without Ext I18N";
 }
}

That’s tedious. Or is there any other smart Spring wiring way to do that? So something that could wire an interface defined in AH to

  • An implementation that mentions Ext I18N imports when Ext I18N is loaded.
  • An implementation agnostic to Ext I18N when Ext I18N is not loaded.

Cc @angshuonline, @mseaton

Is this of any help? https://wiki.openmrs.org/display/docs/Supporting+different+OpenMRS+versions

@mksd, the key thing you will likely want to research are these capabilities. Specifically, look at the section titled “Conditional loading based on running modules”.

What I might recommend is to move the AddressHierarchyI18nCache class into the addresshierarchy module, but annotate it with @OpenmrsProfile(modules = {"exti18n:*"}) (or whatever the right syntax is here), then change your “AddressHierarchyServiceImpl.getI18nCache” method to return this bean if it is successfully found, and return an EmptyI18nCache otherwise. This might be enough to fix your issues.

Mike

@dkayiwa and @mseaton, I gave it a bit of a try.

I introduced a new AH-only interface I18nCache that has got two possible implementations (see here):

  • EmptyI18nCache (that’s not a Spring bean).
  • I18nCacheImpl (that’s the conditional Spring bean).

I18nCacheImpl is the implementation that should be used when Ext I18N is loaded, see here:

if (ModuleFactory.isModuleStarted("exti18n")) {
 return Context.getRegisteredComponent("ah.i18nCache", I18nCache.class);
}
else {
 return new EmptyI18nCache(); // an inert/disabled cache that does nothing
}

And the bean definition is here in moduleApplicationContext.xml. Note that I also have tried to not do that and annotate it with @Component("ah.i18nCache") instead, as well as not using @Autowired on the AddressHierarchyI18nCache member but set it through an init method instead.

Per design, I18nCacheImpl is the only class that refers to Ext I18N imports. Despite all the above, somehow Spring bumps into trying to resolve those imports even when Ext I18N is not around. Here is the log trace that is generated when the context is refreshed:

Error creating bean with name 'ah.i18nCache' defined in URL [jar:file:/openmrs/platform-2-0-3/.openmrs-lib-cache/addresshierarchy/lib/addresshierarchy-api-2.11.0-SNAPSHOT.jar!/moduleApplicationContext.xml]: Post-processing failed of bean type [class org.openmrs.module.addresshierarchy.i18n.I18nCacheImpl] failed; nested exception is java.lang.IllegalStateException: Failed to introspect bean class [org.openmrs.module.addresshierarchy.i18n.I18nCacheImpl] for persistence metadata: could not find class that it depends on

I would prefer that the address-hierarchy module exposes an extension and provides an interface, which other omods can supply implementations for, and with Spring @autowired, the supplied bean(s) can be automatically injected into a controller or such.

AddressHierarchyAjaxController in openmrs-module-addresshiearchy is responsible for getting the entries. I can imagine a simple @Autowired annotation over a field type of matching interface will help inject any matching bean.

If you look into “AddressHierarchyAjaxController.getPossibleAddressHierarchyEntriesWithParents()”, you can just do something like

> getAddresses(transform(retrieveAddressHierarchyEntries(ahService, level, searchString, parentEntry, limit)))

the transform() method can just run through the interface if any is defined.

For using in Bahmni, say while fetching through the patient Profile [NOTE: Written in context of Bahmni]

  • let Bahmni core not depend on another module.
  • let Bahmni open up an extension for i18n of PersonAddress. – assume an interface provided from BahmniCore like “I18PersonAddress”, having a method like “transform(PersonAddress, userLocale)”
  • let BahmniPatientProfileResource expose that as extension point by either having autowiring of any object matching “I18PersonAddress” interfaces or even doing a Context.getRegisteredComponents(I18PersonAddress.class), and if found calling the transform method.

No Magic in the above mechanisms, just simple ways to make things extensible.

If things were done “the other way around” as you describe. So if Ext I18N depends on Bahmni Core and provides the implementation for the API that Bahmni Core exposes, then what would happen at runtime when Bahmni Core is not loaded? I believe we would just bump on the exact same issue as the one described above on this thread. Ext I18N would face a ClassNotFoundException for every interface of Bahmni Core that it is meant to implement.


Also things are a lot more intricate than that when it comes to AH and hierarchical address entries, and this exact instruction illustrates it well:

getAddresses(transform(retrieveAddressHierarchyEntries(ahService, level, searchString, parentEntry, limit)))

When address entries are internationalized (and saved as language-independent codes) something like a search string becomes a bit of a nightmare as you can imagine. There is no way anymore to leverage on Hibernate or Lucene queries directly, a whole new layer must be introduced to allow for such searches.

This is one of the multiple reasons why nothing much happens at the controller level but rather deeper at the service implementation level. See also around this post for the rationale behind the directions that were taken a few months back.

@mseaton, IF there is no good Spring way around this, would it be conceivable that a handful of interfaces (two basically) of Ext I18N could be JAR packaged into AH?

I would create a separate submodule in Ext I18N that would contain a pure API (not sure how I would name it tho, ‘exti18n-api-api’ :wink: maybe?) And that one would be JAR-embedded inside AH.

@mksd does this mean that our documentation https://wiki.openmrs.org/display/docs/Supporting+different+OpenMRS+versions in regards to “Conditional loading based on running modules” is not correct? Or is this use case more complicated than what we documented?

I’m not sure 100% yet, what I can say is this: Spring complains about issues arising from a component that should not be loaded.

I think that Spring actually does not intend to load it, but the exceptions are thrown solely because it is inspecting it. I would have wished for it to just skip it and go away from it as soon as it realises that it is not meant to be loaded.

OK. If you are not getting any further, you can raise a pull request for us to take a look.

I think we should handle this case as either a bug in core, or something that needs to be better documented if we are simply not utilizing the feature correctly.

@dkayiwa or @raff or @darius, do any of you know of any successful usage of the OpenmrsProfile annodation when the condition is based on a running module like this? I feel like what @mksd is trying to do should work. He should be able to have a bean with an @OpenmrsProfile annotation (perhaps with a Component annotation, perhaps not), and OpenMRS would instantiate and register this with Spring if the profile pattern matches the specified modules.

Mike

This bean was defined correctly and should load only when the expected module is started: https://github.com/mekomsolutions/openmrs-module-addresshierarchy/blob/tmp/api/src/main/java/org/openmrs/module/addresshierarchy/i18n/I18nCacheImpl.java#L13-L14

So @mksd, all you need to do is simply remove this: https://github.com/mekomsolutions/openmrs-module-addresshierarchy/blob/tmp/api/src/main/resources/moduleApplicationContext.xml#L70

Hi @dkayiwa, thanks for looking into it. However I already mentioned above that this option doesn’t work either:

When I annotated it with @Component I also did remove the entry in moduleApplicationContext.xml, to no avail in the end (same stack trace).

It seems that either I have to put a bean entry in moduleApplicationContext.xml or to add @Component to my class, the @OpenmrsProfile annotation doesn’t take care of that, which makes sense. If I fail to do any I get this:

No qualifying bean of type [org.openmrs.module.addresshierarchy.i18n.I18nCache] found for dependency: expected at least 1 bean which qualifies as autowire candidate for this dependency. Dependency annotations: {}

@mksd that problem is happing because of the way modules are started and spring application context initialized during tests. But all should be well at runtime. For the sake of making your tests pass, just put that bean in this file https://github.com/mekomsolutions/openmrs-module-addresshierarchy/blob/tmp/api/src/test/resources/TestingApplicationContext.xml

Yeah sorry, I forgot to tell you. For the sake of this troubleshooting, please skip tests, I’ll fix that later. I know that a test starts failing when we touch the way this bean is being wired.

This is not working at runtime, all the things I said above are relevant logs and comments about runtime.

Can you also change this https://github.com/mekomsolutions/openmrs-module-addresshierarchy/blob/tmp/api/src/main/java/org/openmrs/module/addresshierarchy/service/AddressHierarchyServiceImpl.java#L1078 to this? Context.getRegisteredComponents(I18nCache.class).get(0)

Not sure if I understand this. :frowning:

If another module is dependent on AH, then AH will be loaded first. So the interface definition that the other module relies on will always exist and not throw ClassNotFoundException. Same for BahmniCore being dependency to another module. I am not suggesting that you make Ext I18N module to depend on Bahmni. The interface for i18 needs to be in AH module. Bahmni depends on AH, Exti18n depends on AH - no conflict. If you are saying that the order of loading of Ext118 and BahmiCore can cause the issue, then do a runtime discovery. It can be referenced runtime using calling “Context.getRegisteredComponent(I18Interface.class)” - it does not need to be a class level variable at all.

Also not sure why the transform() function is a problem at the controller level. Maybe I am not getting the context. But idea is simple - the core service call fetches me the entries - that should not change. (I make no assumotion that any codified address entries) - and then I just apply a transformation, which again is applied if a transformer is provided or not. The Transformer figures out if it should apply any transformation or not - if not just return the original. The base of things do not change, only surfacial transformation does.

If we need to take action within deeper internal service layers, well then I would tend to believe that its a smell that the design in not quite right.

Btw, we should really meetup early next week to get a better sense of your contexts and requirements in Bahmni context.

Hi @angshuonline,

My understanding is that exti18n will not depend on addresshierarchy - rather, addresshierarchy will be awareof exti18n. We want addresshierarchy to utilize exti18n when it is present, and not to use it when it is not.

Mike

I think adding @OpenmrsProfile only isn’t enough for a bean to get picked up, you would probably need to add @Component too. Don’t you need to specify the openmrs version too?

@wyclif, yes @mksd has tried this. I don’t think one should need to specify the openmrs version if this is not a condition. The only condition is whether a module is loaded or not.

@dkayiwa / @wyclif - is someone able to actually try this out and see if the functionality works? We can speculate here all we want, but as far as I can tell so far, we have documented support for a feature, but we have no working examples that we can point anyone to. It should be straightforward to fire up a new SDK, install addresshierarchy and exti18n and try this out. Does anyone have cycles to do that?

Mike