Monday, October 22, 2012

OpenMRS style tip: how you should expose module Global Properties

I've seen OpenMRS modules expose configurable functionality in about ten different ways. The most common is to expose a global property (e.g. "concept.weight") whose value you set to the primary key of an object in the database (e.g. "5089"), and then have the code littered with things like this:
Context.getConceptService().getConcept(Integer.valueOf(Context.getAdministrationService().getGlobalProperty("concept.weight")));
This is ugly for a few reasons:

  • a helper method would save a lot of typing, and allow consistent reporting of error messages if the GP is configured wrong.
  • proper unit-testing of any code that calls this requires you to use static mocks (thanks Mario and Alex for harping on this!)

Today Mark and I sat down (virtually) for a pairing session and wrote out our proposal for the "right way" to do this going forwards. Here's how it works in the EMR module:

  • An EmrProperties class
    • It has strongly-typed getter methods for the module's settings
    • These getters are not static. The class is annotated with @Component, so we can autowire it into services that need it.
    • This contains settings configurable via GP and hardcoded constants (so developers don't have to think about whether to look in a "properties" file or a "constants" file, and so if an initially-hardcoded setting is made configurable, this don't require structural refactoring)
  • A ModuleProperties helper class, so other modules that depend on the EMR module (which will be most PIH modules going forwards) don't have to reimplement code that fetches an object from the database based on a global property.
    • eventually we'll improve this to support PKs, UUIDs, names, etc.
The bolded point above is the key difference from things we've done before. So to use this class, we autowire an EmrProperties object (for production), providing a setter for testing. This lets us use the class in our code as conveniently as if we were making static calls, while letting us mock the EmrProperties object and inject it into our service, letting us write proper unit tests.

1 comment:

  1. Note: the link that says "we autowire" now points to a place where we no longer use the @Autowired annotation. This is because of https://tickets.openmrs.org/browse/TRUNK-3806 , where using @Autowired in an OpenmrsService slows down startup by 10x.

    You still can (and should, and we do) autowire EmrProperties in other places.

    ReplyDelete