metadatasharing module not working with core

Tags: #<Tag:0x00007fe7f7695de8>

Logs openmrs.txt (1.2 MB)

I have figured out what’s wrong and tested it, the server runs after correcting that

<bean class="org.springframework.web.servlet.mvc.annotation.DefaultAnnotationHandlerMapping"/> this need to be removed from webModuleApplicationContext

@dkayiwa

2 Likes

This is probably true of a number of other community-supported modules. We should probably create an Epic in Jira to track the removal of this from the community-supported modules. We do need to be on the look out for where the DefaultAnnotationHandlerMapping was being configured to do more than this and we need to make sure that all @Controllers have a corresponding @RequestMapping, but otherwise, I think it’s a safe change to make (OpenMRS 1.11.0 appears to be the last pre-Spring 4 version).

2 Likes

I’ve created an Epic for this. @dkayiwa Thanks for all your work on this!

1 Like

Hello @ibacher, there are very many modules that are configured using this class,not only being them called as beans in webmoduleApplicationContext.xml file but also being called as a class. according to this https://github.com/search?p=3&q=org%3Aopenmrs+DefaultAnnotationHandlerMapping&type=Code. how can we go about this , Am guessing removing the DefaultAnnotationHandlerMpping class as it is stated would require to some files to be replaced by RequestMappingHandlerMapping class . More guidance will be highly appreciated thanks

@sharif I responded to this on the Epic, since the information is relevant for actually solving it. Thanks for pointing out the potential ambiguities; I hope I’ve provided enough clarification.

1 Like

Sure thanks for great clarification it was so helpful indeed

@ibacher , Due to this backward incompatible change because of the Spring Upgrade , and the coming Java Upgrade , Our core should be bumped to Version 3.0 rather than 2.4.0
cc @dkayiwa

@mozzy I don’t think there’s a change here that warrants a major version bump, but I understand the concern. Let me just clarify that the changes requested in the Epic will still be fully compatible with any version of OpenMRS Platform 2.x. Where things get a little muddy is with support for OpenMRS Platform 1.x. Versions of OpenMRS 1.12.0+ will be fully compatible with the change (remember the change we’re making reflects the state of things in Spring 3.2) but may be incompatible with OpenMRS 1.11.0 and lower. What this means is that some modules may become incompatible with OpenMRS 1.11.0 and lower and those modules should probably have their major version incremented to signal this.

Regarding the bump in Java versions, although I don’t quite know what the status of this is, my position on it is we should move towards being compatible with Java 11 (the LTS release) without losing compatibility with Java 8, which again should keep the platform consistent with the OpenMRS 2.x series.

In my view, neither of these changes, either separately or combined, rise to the level of demanding a new OpenMRS version, however, I am, as always, willing to be contradicted on this.

FTR, I have no real objection to bumping the platform to 3.0.0, I just think “modern Spring version and compatible with Java 11” is… not a major selling point.

cc: @burke @mseaton

@ibacher , thanks for the clarificaton on that point. At first it seemed to me that this change was nolonger compatible with the 2.x series :wink:
cool then …we can still go on with the 2.x

Have create a small PR here following the guidance from @ibacher and everything is right, The problem came to make metadatasharing module to depend on the current version of spring in openmrs core .

@sharif I don’t think that only removing these line will solve the problem we’ll need to remove the mappings as well

I completely agree with @ibacher

The changes we are making to modules are completely backwards compatible. So no module will lose support for an old version of the platform that it supported.

1 Like

You are right but according to what @ibacher suggested, there are some modules that will not necessary removing this line but taking a key note of @controllers followed by @requestMappings.Removing that link didn’t encounter any error irrespective on controllers in the module.

Thanks @dkayiwa i think thats why even the older support version of core in metadatasharing support the current upgrade of spring in core. Though running it encounters some logs here but they dont stop the success of the module

@sharif Could you turn that into two tickets for TRUNK please? One to fix these messages:

recognized obsolete hibernate namespace http://hibernate.sourceforge.net/. Use namespace http://www.hibernate.org/dtd/ instead. Refer to Hibernate 3.6 Migration Guide!

And one to address the exception:

schema export unsuccessful
org.h2.jdbc.JdbcSQLException: Database is already closed (to disable automatic closing at VM shutdown, add ";DB_CLOSE_ON_EXIT=FALSE" to the db URL) [90121-135]
        at org.h2.message.DbException.getJdbcSQLException(DbException.java:327)
        at org.h2.message.DbException.get(DbException.java:167)
        at org.h2.message.DbException.get(DbException.java:144)
        at org.h2.message.DbException.get(DbException.java:133)
        at org.h2.jdbc.JdbcConnection.checkClosed(JdbcConnection.java:1348)
        at org.h2.jdbc.JdbcConnection.checkClosed(JdbcConnection.java:1323)
        at org.h2.jdbc.JdbcConnection.getAutoCommit(JdbcConnection.java:386)
        at com.mchange.v2.c3p0.impl.NewProxyConnection.getAutoCommit(NewProxyConnection.java:985)
        at org.hibernate.connection.C3P0ConnectionProvider.getConnection(C3P0ConnectionProvider.java:82)
        at org.hibernate.tool.hbm2ddl.SuppliedConnectionProviderConnectionHelper.prepare(SuppliedConnectionProviderConnectionHelper.java:51)
        at org.hibernate.tool.hbm2ddl.SchemaExport.execute(SchemaExport.java:263)
        at org.hibernate.tool.hbm2ddl.SchemaExport.drop(SchemaExport.java:229)
        at org.hibernate.impl.SessionFactoryImpl.close(SessionFactoryImpl.java:961)
        at org.springframework.orm.hibernate3.AbstractSessionFactoryBean.destroy(AbstractSessionFactoryBean.java:251)
        at org.springframework.orm.hibernate3.LocalSessionFactoryBean.destroy(LocalSessionFactoryBean.java:899)
        at org.openmrs.api.db.hibernate.HibernateSessionFactoryBean.destroy(HibernateSessionFactoryBean.java:192)
        at org.springframework.beans.factory.support.DisposableBeanAdapter.destroy(DisposableBeanAdapter.java:184)
        at org.springframework.beans.factory.support.DefaultSingletonBeanRegistry.destroyBean(DefaultSingletonBeanRegistry.java:487)
        at org.springframework.beans.factory.support.DefaultSingletonBeanRegistry.destroySingleton(DefaultSingletonBeanRegistry.java:463)
        at org.springframework.beans.factory.support.DefaultSingletonBeanRegistry.destroySingletons(DefaultSingletonBeanRegistry.java:431)
        at org.springframework.context.support.AbstractApplicationContext.destroyBeans(AbstractApplicationContext.java:1048)
        at org.springframework.context.support.AbstractApplicationContext.doClose(AbstractApplicationContext.java:1022)
        at org.springframework.context.support.AbstractApplicationContext$3.run(AbstractApplicationContext.java:940)

ok then i will create them , Can i add the Trunk-5722 among the link reference @ibacher ,i think this is not related in openmrs core, because the error logs provided were after running metadatasharing module. so this tickets may be under metadatasharing module

The exception points to this: org.openmrs.api.db.hibernate.HibernateSessionFactoryBean.destroy, which is part of core, hence where I went with that. But you’re right that the warning is probably generated from the metadata module itself.

1 Like

Thanks again @ibacher, let me create them however with clear description of how error logs came, then we will decide where they reside, i think its possible to shift them where they fall

Have created then here, and here so would like for you review and also would recommend to reproduce the bug such that we can be on the same truck. Again if possible you can give me previlige to make them ready for work cc @dkayiwa @ibacher

1 Like

The issue here is that we’ve generally tried our hardest to avoid having to maintain multiple release branches of modules and backporting changes, etc.

So if I understand it, with this change we are saying that all modules would have to be bumped up to only support 1.12.x and above? If there’s an easy way to avoid having to do this, I’d like to do so.

1 Like