weird behavior in ModuleUtil.matchRequiredVersions()

teleivo · September 19, 2016, 9:02am

Hello,

I stumbled on a to me weird behavior of

ModuleUtil.matchRequiredVersions(openmrsVersion, requiredOpenmrsVersion)

Which says that it

@return true if the <code>version</code> is within the <code>value</code>

This expression is true (as I would expect):

assertFalse(ModuleUtil.matchRequiredVersions("2.1.0", "1.9.*"));

But this is not:

assertFalse(ModuleUtil.matchRequiredVersions("2.1.0", "1.9.8"));

Im puzzled How can “2.1.0” match “1.9.8” ?

teleivo · September 19, 2016, 9:09am

mhmm. So if you pass in a single required version like “1.9.8”, no range then it ends up in

github.com

openmrs/openmrs-core/blob/2.0.x/api/src/main/java/org/openmrs/module/ModuleUtil.java#L308-L312


					if (compareVersion(version, range) < 0) {
						log.debug("Version " + version + " is below " + range);
					} else {
						return true;
					}

and since the current version in my example “2.1.0” is above the range value “1.9.8” this evaluates to true.

So I know where it comes from. Now the question is do you want this behavior ? I dont think I do

dkayiwa · September 19, 2016, 9:12am

Modules put a requires version of the platform. This method was created for such. it usually means the lowest required version. So if a module requires version 1.9.8, it should then be able to run on version 2.1.0

teleivo · September 19, 2016, 9:18am

should it? 2.1.0 can have backwards incompatible changes making a 1.9.8 module unable to run on 2.1.0.

But apart from that why is assertFalse(ModuleUtil.matchRequiredVersions("2.1.0", "1.9.*")); expression true then and “1.9.8” its not? Seems inconsistent to me

dkayiwa · September 19, 2016, 11:33am

@teleivo that is simply the minimum required version. It does not necessarily mean that it will run on all higher versions. If the module developer wants to specify an upper boundary, it is all up to them.

teleivo · September 19, 2016, 11:39am

ok, thanks for clarifying this.

But still the behavior of ModuleUtil.matchRequiredVersions doesnt make sense to me.

assertFalse(ModuleUtil.matchRequiredVersions(“2.1.0”, “1.9.*”)); evaluates to true and “1.9.8” does not, why?

dkayiwa · September 19, 2016, 12:00pm

When using a wild card, the meaning changes. So 1.9.* means the version is required to be in the 1.9.x series.

1.9 does mean 1.9 and above. 1.9.* means any version of the 1.9.x branch. That is 1.9.1, 1.9.2, but not 1.10.0, 1.11.0, etc

teleivo · September 19, 2016, 12:12pm

ahhh

this should go into the javadoc which isnt very clear.

dkayiwa · September 19, 2016, 12:27pm

I fully agree! Do you wanna add it?

teleivo · September 19, 2016, 12:28pm

am on it

teleivo · September 19, 2016, 12:42pm

@dkayiwa see PR here https://github.com/openmrs/openmrs-core/pull/1838

dkayiwa · September 19, 2016, 12:50pm

I trust your speed man!!!

reubenv · January 18, 2017, 7:57am

Hi, @teleivo @dkayiwa I feel this ticket https://issues.openmrs.org/browse/TRUNK-4866 would hamper backward compatibility right? Forgive me if this is out of the scope of this discussion

dkayiwa · January 18, 2017, 9:22am

@reubenv can you give an explanation regarding how it would affect backward compatibility?

reubenv · January 20, 2017, 10:40am

From what I understood, @Dengfeng Li proposes that the single version of each module within the openmrs installation should match that of the main software ,else an error would be thrown .If so then this would mean that the single required version of each module would have to updated with each new build. This in my opinion would not be very feasible each time. Even if a single version is below the openmrs version then the build fails. Now, if I were to install an old version of the module then currently the build won’t fail but after the proposed change it will. This is how it would prevent older versions of modules from running even if they were to run perfectly .Please do correct me if I am wrong