EMRAPI support for OpenMRS 2.0

bharatak · September 6, 2016, 1:08pm

Hi, We are planning to upgrade Bahmni to use OpenMRS 2.0. So planning to upgrade emrapi to support it. So, 1.18 release of emrapi will have that support. Can you please let us know if everybody is happy with it? Do you see any issues/concerns that we should be aware of? Inviting some core devs of emrapi - @mogoodrich @raff @dkayiwa

Adding folks from bahmni team @angshuonline @darius @vinay @gsluthra

dkayiwa · September 6, 2016, 4:02pm

Isn’t this module already running on platform 2.0 according to: http://int02.openmrs.org/openmrs/admin/modules/module.list

Or did you mean something else?

bharatak · September 6, 2016, 4:23pm

Awesome. Didn’t realize its already ready for 2.0. Will test it with Bahmni after the upgrade. Thanks a lot.

gutkowski · September 7, 2016, 5:49am

Please note that although EMRAPI is compatibile with OpenMRS 2.x core, its REST resources: PatientProfileResource, and PersonImageResource are not supporting OpenMRS versions above 1.12.*.

bharatak · September 7, 2016, 8:04am

Thanks @gutkowski for pointing out. We will take care of that.

dkayiwa · September 7, 2016, 8:48am

Very good catch @gutkowski ! @bharatak do you have an estimate of when you think this will be done? Am talking in line with the reference application 2.5 which is supposed to be ready in a couple of days and this turns out to be a blocker for it.

cc @taremwatadeo

bharatak · September 7, 2016, 10:17am

I am not sure if these resources are used in reference app. They are definitely used in Bahmni. We have a plan of upgrading all the modules managed by Bahmni in next couple of weeks. We have plans to do some benchmarking with java7, java8 and then with openmrs2.0. Emrapi is planned to start after 3 days and will be completed somewhere in mid next week.

bharatak · September 22, 2016, 8:41am

Hi, We would like to cleanup the emrapi project structure and remove support for api-1* maven modules with openmrs 2.0 support. The current pom version is 1.18-SNAPSHOT. Can I suggest moving to 2.0-SNAPSHOT and clean up the project with references to openmrs 1.x. The 2.0 release of emrapi will support openmrs 2.0. What do you think? Do you see any new development happening currently in emrapi which supports backward compatibility? @mogoodrich @raff @dkayiwa @darius @gutkowski

cc: @preethi_s

darius · September 22, 2016, 1:35pm

@bharatak, in general we try to preserve support for (at least) all supported OpenMRS release lines. Especially since this is a shared module (and part of the reference application) we do need to maintain that support.

(At some point we should clean things up, and start dropping support for the older versions, but at the same time I imagine it takes more work to remove that support than to leave it.)

Currently the module supports OpenMRS Platform 1.9.9 and above. So if you do want to refactor things, I would think the requirement is to maintain support for platform 1.12.x, 1.11.x, and 1.10.x while adding 2.0.x support. (Normally we support “last 3”, but 1.12 was an out of band release, so I’d prefer not to drop 1.10.x support yet.)

mogoodrich · September 22, 2016, 2:38pm

The PIH EMR currently is running on OpenMRS 1.10 (and uses EMR-API) so we’d definitely need to maintain support for 1.10.x going forward. Hopefully we will be updating to 1.11 in the next 6-12 months, but we have not scheduled it yet.

Take care, Mark

preethi.s · September 27, 2016, 11:12am

Emr api would require reportingcompatibility omod to be loaded starting from openmrs 2.0

The require_module is specified over here:

github.com

openmrs/openmrs-module-emrapi/blob/master/omod/src/main/resources/config.xml#L22


    ${project.parent.description}
</description>
<updateURL>https://modules.openmrs.org/modules/download/${project.parent.artifactId}/update.rdf</updateURL>
	<require_version>1.11.3, 1.10.2 - 1.10.*, 1.9.9 - 1.9.*</require_version>
<!-- TRUNK-3751 prevents saying ${openmrsCoreVersion} when version is a SNAPSHOT-->
<!-- / Module Properties -->




<!-- Required Modules -->
<require_modules>
    <require_module version="${reportingModuleVersion}">org.openmrs.module.reporting</require_module>
    <require_module version="${providermanagementModuleVersion}">org.openmrs.module.providermanagement</require_module>
    <require_module version="${metadatasharingVersion}">org.openmrs.module.metadatasharing</require_module>
    <require_module version="${metadatamappingVersion}">org.openmrs.module.metadatamapping</require_module>
    <require_module version="${eventVersion}">org.openmrs.event</require_module>
    <require_module version="${webservicesRestModuleVersion}">org.openmrs.module.webservices.rest</require_module>
</require_modules>
<!-- / Required Modules -->


<!-- Conditional Resources -->
<conditionalResources>

Ideally reportingcompatibility should also be added over there but it is not necessary for implementations using openmrs 1.x. Is there a way to add require_module conditionally based on openmrs version? Any help?

dkayiwa · September 27, 2016, 11:22am

@preethi_s did you try using “aware_of_module”? https://wiki.openmrs.org/display/docs/Module+Config+File

darius · September 27, 2016, 3:26pm

(I’m surprised that we would have started requiring the reporting compatibility module…)

-Darius (by phone)

mseaton · September 27, 2016, 3:31pm

@preethi_s, are you sure we depend on reportingcompatibility in emrapi 2.0? This does not seem like something we would want.

Mike

preethi.s · September 28, 2016, 4:55am

It throws ClassNotFoundError: AbstractReportObject in runtime, when the compatibility omod is not present.

darius · September 28, 2016, 5:25am

@preethi_s can you share a fuller stack trace of this error? Our intention is definitely not to start requiring reportingcompatibility, so this seems like a mistake.

preethi.s · September 28, 2016, 5:38am

Below is the exception: Exception in thread "Thread-9" java.lang.NoClassDefFoundError: org/openmrs/reporting/AbstractReportObject at java.lang.Class.getDeclaredMethods0(Native Method) at java.lang.Class.privateGetDeclaredMethods(Class.java:2701) at java.lang.Class.getDeclaredMethods(Class.java:1975) at org.springframework.util.ReflectionUtils.getDeclaredMethods(ReflectionUtils.java:571) at org.springframework.util.ReflectionUtils.doWithMethods(ReflectionUtils.java:490) at org.springframework.util.ReflectionUtils.doWithMethods(ReflectionUtils.java:474) at org.springframework.beans.factory.annotation.AutowiredAnnotationBeanPostProcessor.determineCandidateConstructors(AutowiredAnnotationBeanPostProcessor.java:241) at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.determineConstructorsFromBeanPostProcessors(AbstractAutowireCapableBeanFactory.java:1057) at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBeanInstance(AbstractAutowireCapableBeanFactory.java:1030) at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.doCreateBean(AbstractAutowireCapableBeanFactory.java:504) at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBean(AbstractAutowireCapableBeanFactory.java:476) at org.springframework.beans.factory.support.AbstractBeanFactory$1.getObject(AbstractBeanFactory.java:303) at org.springframework.beans.factory.support.DefaultSingletonBeanRegistry.getSingleton(DefaultSingletonBeanRegistry.java:230) at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:299) at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:194) at org.springframework.beans.factory.support.DefaultListableBeanFactory.preInstantiateSingletons(DefaultListableBeanFactory.java:762) at org.springframework.context.support.AbstractApplicationContext.finishBeanFactoryInitialization(AbstractApplicationContext.java:757) at org.springframework.context.support.AbstractApplicationContext.refresh(AbstractApplicationContext.java:480) at org.openmrs.module.ModuleUtil.refreshApplicationContext(ModuleUtil.java:842) at org.openmrs.module.web.WebModuleUtil.refreshWAC(WebModuleUtil.java:866) at org.openmrs.web.Listener.performWebStartOfModules(Listener.java:653) at org.openmrs.web.Listener.performWebStartOfModules(Listener.java:632) at org.openmrs.web.Listener.startOpenmrs(Listener.java:267) at org.openmrs.web.WebDaemon$1.run(WebDaemon.java:42) Caused by: java.lang.ClassNotFoundException: org.openmrs.reporting.AbstractReportObject at org.apache.catalina.loader.WebappClassLoader.loadClass(WebappClassLoader.java:1320) at org.apache.catalina.loader.WebappClassLoader.loadClass(WebappClassLoader.java:1173) at org.openmrs.module.ModuleClassLoader.loadClass(ModuleClassLoader.java:563) at java.lang.ClassLoader.loadClass(ClassLoader.java:357) ... 24 more

reporting omod version that we use is reporting-0.10.2-SNAPSHOT.omod

darius · September 28, 2016, 8:14am

@mseaton, I did a bit of debugging and indeed if you try to start openmrs-core 2.0 with this set of modules, you get the exception that @preethi_s shared:

calculation-1.2.omod			providermanagement-2.5.0.omod
emrapi-1.18-SNAPSHOT.omod		reporting-0.10.2.omod
event-2.5-SNAPSHOT.omod			serialization.xstream-0.2.12.omod
htmlwidgets-1.7.3-SNAPSHOT.omod		uiframework-3.7.omod
legacyui-1.2-SNAPSHOT.omod		uilibrary-2.0.4.omod
metadatamapping-1.2.0.omod		webservices.rest-2.16.omod
metadatasharing-1.2.2.omod

Further it comes from loading the bean dataExportDataSetDefinitionPersister, which may come from this line:

https://github.com/openmrs/openmrs-module-reporting/blob/master/api/src/main/java/org/openmrs/module/reporting/dataset/definition/persister/DataExportDataSetDefinitionPersister.java#L71

(I don’t yet know why this behaves different now from how it did prior to 2.0.)

dkayiwa · September 28, 2016, 9:46am

FWIW, this class was deprecated https://github.com/openmrs/openmrs-core/blob/1.12.x/api/src/main/java/org/openmrs/reporting/AbstractReportObject.java and then removed from platform 2.0 During this ticket TRUNK-4337 and this commit, it was moved to the reportingcompatibility module https://github.com/openmrs/openmrs-module-reportingcompatibility/commit/59fe85db6d2f0e45041e3bf434b4d3e4c07dbc83

mseaton · September 28, 2016, 4:39pm

This doesn’t surprise me at all, and I would expect this to be a problem. The bigger issue is that there is not profile that tests the reporting module at all against version of OpenMRS beyond 1.10. There is the default setting that runs tests against 1.9, and there is a 1.10 profile (which I’m not sure is ever run in an automated or regular fashion). Beyond that, there is nothing that does any kind of unit testing against OpenMRS 1.11, 1.12, or anything in the 2.x line. If we had that, this problem would have been identified long ago. Thoughts on how we can do a better job with this? Is this something that Travis CI can help us with?

There was some effort put into adding 2.x compatiblity in the reporting module at one point - @dkayiwa I believe was involved in doing that. So there is a 2.x maven submodule where we could move this particular class. Should be straight-forward to do that. Adding the additional testing profiles for 1.11+ will be considerably more work I think.

Regardless, this should be considered a bug caused by 2.0 API changes, and should not be solved by adding reportingcompatibility as a dependency.

Mike