Loading Resources conditionally

Development
1

Was working with Mike today to debug an issue with the Reporting module after it had been updated to the support OpenMRS 2.0, and I believe we ran into an issue with the documentation of conditional resource loading. The documentation suggests that if you specify a conditional resource 1.10 then it will load that resource when using OpenMRS 1.10 or above. I believe that, in reality, it will only load the specified resource if using OpenMRS 1.10.x. We had to make the following change to the Reporting module config to get it to run properly with OpenMRS 1.10.4

We should update the documentation https://wiki.openmrs.org/display/docs/Supporting+different+OpenMRS+versions but first I wanted to check that it’s the documentation that is in error, not the code. It does seem to me that what the code is doing is what I’d intuitively expect if I hadn’t read the docs.

2

1.10 does mean 1.10 and above. 1.10.* means any version of the 1.10.x branch that is 1.10.1, 1.10.2, but not 1.11.0, 1.12.0

When implementing the feature I used the same behavior as for defining required modules’ versions what we had ever since.

In the commit you quoted I see you changed 1.9.* to 1.. 1.9. refers to any version of the 1.9.x branch, whereas what you correctly noticed what you want there is 1.* that is any version of the 1.x branch.

4

Sorry, I withdrew my previous post because when I went to make the change, I realized that I liked things better the way I had them… :slightly_smiling:

If that is our convention, we should use it, but I think it’s not as clear when using conditional resources (where there can be multiple “conditional” entries) rather than something like “required module” where there is only one entry and an implied “and above”.

For instance, this seems much clearer to me:

<conditionalResources>
		<conditionalResource>
			<path>/lib/reporting-api-1.9.*</path>
			<openmrsVersion>1.*</openmrsVersion>
		</conditionalResource>
		<conditionalResource>
			<path>/lib/reporting-api-2.0.*</path>
			<openmrsVersion>2.*</openmrsVersion>
		</conditionalResource>
	</conditionalResources>

than:

<conditionalResources>
		<conditionalResource>
			<path>/lib/reporting-api-1.9.*</path>
			<openmrsVersion>1.10</openmrsVersion>
		</conditionalResource>
		<conditionalResource>
			<path>/lib/reporting-api-2.0.*</path>
			<openmrsVersion>2.0</openmrsVersion>
		</conditionalResource>
	</conditionalResources>

Note that I don’t think we need to change the implementation of the feature, just recommend the first approach as best practice. Thoughts/opinions anyone?

5

1.10 should support 1.10 and above and if it isn’t then it’s a bug, 1.10.* as @raff says would only support 1.10.x versions only and not 1.11 and later

6

Yes, but of the two examples above (which both should achieve the same thing), I think that the first one (with 1.* and 2.*) is more clear.

7

1.* i think would mean all 1.x.x versions

8

@mogoodrich, I agree with you that the first one is more clear, assuming that your goal is always to include just one of the two resources.

I don’t think those options do the same thing. Your first option always imports exactly one of the two sets of resources, whereas in your second option: (a) nothing is included if you’re running 1.9.x. (b) only /lib/reporting-api-1.9.* is included if you’re running 1.10.x or 1.11.x © both are included if you’re running 2.x

9

@darius thanks, now I think I’m understanding where I am confused… I was assuming both options did the same thing (included just one of the two resources) because that is what I was assuming we wanted to do. But it makes much more sense the way you described it.

@dkayiwa to be clear, when running platform 2.0 do we want to:

(a) only include /lib/reporting-api-2.0.* (b) include both reporting-api-1.9 and reporting-api-2.0

I was assuming the intent was (a). If the intent is (b) then the second option I specified above is correct, and the first option is wrong.

@dkayiwa I also have the same question about the conditional jars in htmlformentry.

10

I haven’t looked at the specifics of the reporting or HFE modules, but in general I think it’s a bit more complicated, and it may be that special-case solutions help limit the complexity.

For a contrived example, take the introduction of Visits in 1.9, and how this would have affected the reporting module.

  • Adding new reporting classes to handle Visits could be done by adding a new resource only loaded in 1.9+.
  • But if you wanted to modify EncounterCohortDefinition to support an “inVisitOfType” parameter (not actually done in real life) you would need to move the old EncounterCohortDefinition class to a [1.0-1.9) resource, and put a new EncounterCohortDefinition class in a 1.9+ resource.
  • However to avoid needing to have 3 separate maven modules (one for all versions, one for [1.0-1.9), and one for 1.9+) you might instead choose to leave EncounterCohortDefinition unchanged, and introduce a new EncounterWithVisitDetailsCohortDefinition.

So, the general pattern to having a module support 1.x and 2.x lines of the OpenMRS platform involves having a bunch of maven submodules:

  • one for classes that work on all versions
  • one for 1.*
  • one for 2.*
  • (optionally) one for 1.9+
  • (optionally) one for [1.9-2.0)
  • (optionally) one for 1.10+
  • (optionally) one for [1.10-2.0)
  • (optionally) one for 1.11+
  • (optionally) one for [1.11-2.0)
  • etc

As you can see, this is straightforward if you are only dealing with classes that changed from 1.x to 2.x, but it can get complicated fast if you were already doing conditional loading for pre-1.10 vs 1.10+ (for order) and the resources previously in 1.10+ also end up needing to be split to deal with 1.x vs 2.x.

So, my advice is that if you’ve got a module that already supports a mix of OpenMRS versions, and it needs further changes to support 1.x vs 2.x, then you should judiciously use hacky reflection approaches to limit the explosion of maven submodules. But doing this requires careful thought, and I don’t think there is a general pattern for it.

11

@darius ugh

@dkayiwa just a ping again re: what the intended conditional loading should be for reporting and html form entry

@mseaton fyi

12

@mogoodrich the intention was for the /lib/reporting-api-1.9 jar to be loaded for platform version 1.9 to < 2.0 While the /lib/reporting-api-2.0 jar is for platform version 2.0 and above. The rest are shared.

For the htmlformentry module, the /lib/htmlformentry-api-1.10 jar is to be loaded for version 1.10 and above. While the /lib/htmlformentry-api-2.0 jar is for platform version 2.0 and above. The rest are shared.

13

Perfect, thanks @dkayiwa.