mekomsolutions / openmrs-module-initializer

The OpenMRS Initializer module is an API-only module that processes the content of the configuration folder when it is found inside OpenMRS' application data directory.
MIT License
23 stars 79 forks source link

[#255] Fixing 'liquibase' domain issues against Core 2.5.5+. #256

Closed mseaton closed 1 year ago

mseaton commented 1 year ago

The genesis of this is trying to use the liquibase domain on an OpenMRS 2.5.x instance to load a liquibase file that contains "sqlFile" changesets which refer to sql files at a relative path, which fails due to an NPE.

This is something I had noticed a long time ago, and we worked around in our main PIH EMR distribution by a) patching core, and b) using a custom liquibase loader at startup. You can see that code here, which has been used successfully in that distribution for years. This also addresses an issue that I brought up on the core ticket which provided the necessary fixes in 2.5.x and beyond, where we discovered an issue with the previous use of absolute file paths in the liquibase changelog. Basically, the use of absolute paths made the changesets non-portable, so migrating (say) from /home/tomcat7/.OpenMRS to /home/tomcat8/.OpenMRS and then restarting your sever would result in all of the changes in the liquibase file getting re-executed, whether they already had been or not, since the change in file would be interpreted by liquibase as a totally new set of changesets. This could have very bad consequences if, for example, there are data migration or data cleanup changesets that were intended to run only once and never again and don't have sufficient preconditions on them.

This PR aims to solve these issues for implementations that have reached OpenMRS 2.5 (although this will only work for OpenMRS 2.5.5+ due to where the core patch was installed).

@mks-d / @Ruhanga / @ibacher / @rbuisson let me know what you think.

mseaton commented 1 year ago

@ibacher / @Ruhanga - thoughts on this PR?

ibacher commented 8 months ago

@mseaton So, I'm running into issues with this change. Specifically, the relativeFilePath doesn't seem to work at all. The code in the PIH EMR seems to work because here you're still passing an absolute path, but in this PR, we're only passing a relative path and this doesn't seem to work. What am I missing?

mseaton commented 8 months ago

because here you're still passing an absolute path

That isn't an absolute path @ibacher . It's a path relative to the application data directory, specifically "configuration/pih/liquibase/xxx.xml"

If it is helpful @ibacher , we are successfully using the Iniz liquibase domain in our Rwanda distribution here.

ibacher commented 8 months ago

@mseaton Yeah, I discovered a different bug in core that needs fixing.