I am trying to fix our document type definition files (we refer to undeclared elements)
for our modules config.xml that we have uploaded at
http://resources.openmrs.org/doctype/config-1.X.dtd and put them under version
control in openmrs-core. So they are useful to devs when writing config.xml in their IDEs and also so we can potentially also use them for validation in our ModuleFileParser (better error messages to implementer, admins and devs, less code for us to write).
Variation 2 introduces another module element but the module is already defined in the DTD, it is our root element,
which is why we cannot declare it again https://www.w3.org/TR/REC-xml/#EDUnique
We cannot create a valid DTD for 1.6 if we adhere to our wikiâs descriptions.
I was searching for conditionalResources on all github (not only openmrs org) and the only usages I found were of Variant 1
I wonder if @dkayiwa, @wyclif, or @raff know of anyplace that variant #2 is used?
If that variant is never used, and it makes our XML invalid, then I guess we should change the naming of that xml element. Itâs actually a different use case than variation 1 â I think it lets you load a resource if a given module is loaded â so we should still preserve it. Assuming thatâs what itâs for we could give it a very precise name like <loadIfModulePresent>.
I have never used the DTD for conditional resources loading, I always did it through class annotations. So this is no concern for me at this juncture. I have a feeling anyway that this hasnât been used much for modules as your GitHub search seems to confirm, so itâs probably a good time to streamline it.
Iâm saying this because âversionâ already refers to the Maven version, and it could be that the module ID might in fact just always be the Maven artifact ID.
I now also found out that the conditionalResource element has
the child element openmrsVersion as shown in the wiki but it also has kind of an âaliasâ element named openmrsPlatformVersion although the wiki doesnt mention this. In openmrs-core the ModuleFileParser treats it the same as openmrsVersion and takes either or to set the fieldfield openmrsPlatformVersion of object ModuleConditionalResource. github shows no use of openmrsPlatformVersion instead openmrsVersion is used exclusively. I see 2 options
keep both elements as possible children to conditionalResource, since removing openmrsVersion as child from conditionalResource would break existing config.xml
I could remove openmrsPlatformVersion without breaking existing config.xml, but then I would also change ModuleConditionalResource.openmrsPlatformVersion to ModuleConditionalResource.openmrsVersion so our DTD is closest to actual java objects
The ModuleConditionalResource class is not really part of the public API that clients code against, itâs used internally by the module engine therefore I think itâs fair to rename the openmrsPlatformVersion field to openmrsVersion becaise technically it shouldnât break any client code.
If I had a time machine I would go back and tell us that the correct name for the attribute should be âopenmrsCoreVersionâ.
I guess that since âopenmrsPlatformVersionâ is not documented anywhere, and you couldnât find any uses on github, I would choose #2 and remove it. (This is a weak preference though.)
@wyclif and @darius I researched more and found https://issues.openmrs.org/browse/TRUNK-4578
where we renamed the annotations OpenmrsProfile field openmrsVersion to openmrsPlatformVersion, so it seems as if we kept the openmrsVersion element in config.xml to not break compatibility but OpenmrsProfile now only uses openmrsPlatformVersion.
Therefore I donât think I should rename ModuleConditionalResource.openmrsPlatformVersion and also allow the element openmrsPlatformVersion as alias to openmrsVersion in the config.xml. Since thats what is also implemented in the ModuleFileParser.
My team has picked up the ticket https://issues.openmrs.org/browse/TRUNK-5403 recently, and we are not sure what there is left to do besides add openmrsPlatformVersion as a possible child of conditionalResource and create a pull request for the new dtd. Is there anything else that needs to be done besides that?
My team wants to understand more about what these config files are used for. Could someone give a brief explanation of the purpose of these config files?
Furthermore, we tested config_1.5 and found that there was a syntax error in the file. Which I believe we plan on fixing before releasing a config_1.6. @dkayiwa@teleivo@darius@wyclif