Dependencies in EMRAPI module

Hi,

Currently, emrapi depends on reporting, providermanagement, metadatasharing, event, webservices and appframework. Bahmni just about needs providermanagement functionalities. Is it alright to make all of these aware of modules instead of require modules?

-Shruthi

@shruthidipali I didn’t quite understand from your email which ones Bahmni does need vs does not.

Here’s my recollection of the reasoning behind these, and current thoughts:

  • webservices.rest
  • this is a core part of the platform, and this dependency should stay
  • providermanagement
  • this is a too-heavy way of providing “provider type” in the reference application.
  • ideally we would add provider type to openmrs-core, and remove this module from the reference application (and remove this dependency from emrapi)
  • metadatasharing
  • emrapi provides some utility methods to load MDS packages
  • but really those methods ought to live somewhere else => this dependency is misguided, and I’d welcome someone doing the work to move the code to another module
  • event
  • @wyclif added this dependency to support recently-viewed patients. See RA-200.
  • I could live with having that feature moved to another module, but I’m not sure which one. But I’d prefer to leave this where it is.
  • appframework
  • I never wanted this dependency – uiframework, appframework, and emrapi were all supposed to be the independent base pieces
  • @mogoodrich added it to support feature toggles. see EA-13, where he says “If we change our mind, we can always pull this dependency out later.”
  • reporting
  • the original intent was that each of the higher-level constructs that are built on top of obs (e.g. Diagnoses) should reporting/query functionality alongside them in the same module.
  • I still generally stick by this.

I might be mistaken, but I’m pretty sure the metadatasharing utility methods should be removed from emrapi, as they are re-introduced (and better belong) in the metadatadeploy module. But someone would need to confirm.

Thanks @darius it looks like all feature toggle references have been removed from emr-api, but I neglected to remove the dependency on appframework itself. I just pulled it from the pom and the module still compiles and passes, so I’ll commit the change.

@shruthidipali are you asking if we can just make the modules you don’t use “aware of” in the config.xml so emr-api can be started without them even those the dependency still exists, I think that’s something we want to avoid.

@mogoodrich, thanks! Can you also please clean up the state of EA-13? (I presume this was actually in released code.)

@darius @mogoodrich I was wondering if we can make all dependant modules to be “aware of” modules in emrapi module’s config.xml. we would save some startup time that way.

-Shruthi

@shruthidipali I’ll defer to @darius on that one… :smile:

In general I don’t like that idea. They are real dependencies for the module, and I think we are a lot better off having these be explicit, and fail fast, even if it increases startup time a bit.

Looking at the specifics:

  • webservices.rest => everyone should be running this module, so it gains nothing to make it aware-of
  • metadatasharing => the correct thing is to refactor and remove this dependency
  • appframework => @mogoodrich already removed this dependency
  • providermanagement => I suspect this will break things to remove (and the correct fix is to add one field to openmrs-core)

The last two are “event” and “reporting”. I guess I could live with these being optional dependencies. E.g. if you don’t have the reporting module loaded, then presumably nothing will try to use the reporting/querying classes. And I suppose it could be okay to not fire events if the event module isn’t loaded.

Even so, I would vote against making these changes for the sake of startup time (it really feels like a hack), and instead focus on the real improvements we have identified around removing dependencies.

Apologies for responding late!

Any thoughts on further dividing emrapi into smaller omods? that way, the implementation is free to choose a smaller set of dependencies. We could also have a blanket omod which includes everything as well. The choice would then be that of an implementer to either include everything or just a subset of the dependencies.

-Shruthi

If Bahmni wants to do a fork of emrapi that has fewer dependencies, that’s fine (and it’s fine if this is not a code fork, but happens as part of the build process of that module). But I don’t approve of releasing multiple smaller variations of emrapi, because the confusion this introduces generally will outweigh the benefits of slightly-improved startup times.

But go back to my previous email: most of these dependencies are okay to remove, and I would really prefer that we remove them in the right way, rather than attempt hacks that will probably take more time to implement and test.

As I said:

  • remove metadatasharing by moving some utility methods to the metadatadeploy module instead
  • remove providermanagment by adding a “provider type” in openmrs-core
  • event and reporting could be aware-of dependencies if you really want