SessionFactory issue after Hibernate 4 upgrade

Scenario 4 works fine - upgrading from a version of core without DbSession to a version of core with DbSession, while the hibernatecompatibility module is still installed.

1 Like

Cool. That’s a plus. Thanks for testing.

Is there an easy recipe (e.g., one-liner) we could give to module authors to include in their startup code that would fail loudly in a well-explained way? For example:

// Copy this into your module's startup code
if (/* pre-hibernate4 & no compatibility module */) {
  throw ModuleException("Hibernate compatibility module required. See http://om.rs/hibernate4");
}

We could point om.rs/hibernate4 to a wiki page summarizing the issue with instructions on how to fix it (i.e., how to install the hibernate compatibility module).

I have summarised what we came up with on the following wiki: https://wiki.openmrs.org/display/docs/Supporting+Platform+1.12+and+below

@kristopherschmidt, could you please include the message suggested by Burke in your changes for the Atlas module? I would also like to ask you to review the wiki I’ve created. Please feel free to edit the page if you think anything is missing or could be improved. Would you be able to join the design call on Monday to share your experience with that?

Thanks @raff and @kristopherschmidt! Hope you can still make the design call today from 4-5pm UTC

I am planning to attend

@kristopherschmidt,

I’m sorry you couldn’t make it to the design forum. We appreciate a lot your work on this!

Before the final decision on the preferred solution we would like to try yet another approach. What if we created a jar with DbSessionFactory that would load the bean if necessary and declared it as a dependency in pom.xml in a module?

We would have to check, if 2 or more modules containing such a jar, can coexist and run fine along one another. The worry here is that our module framework may not support well a situation when one module is using a bean loaded by a different module without declaring require or aware of dependency and we will run into issues.

It is instead of using the hibernatecompatibility module approach, which we find a bit misleading for implementations. Basically they will be able to upgrade to a newer version of a module only to find out that the module will not run after the upgrade. They will need to read up in a documentation on how to fix it and that the hibernatecompatibility module is required.

What do you think about this? Would you like to give it a try?

Sorry I was not able to attend. I got called into a critical meeting at work. I think the idea of a jar makes sense to avoid an extra module dependency for every module out there. In order to activate the code in the jar, it could be as simple as defining the DbSessionFactory bean inside every module’s applicationcontext instead of in the hibernatecompatibility module’s applicationcontext? Or did you have some other way in mind to bootstrap the jar’s code?

Also aside from this particular technical approach, was there any discussion about continuing on master as opposed to a branch, or if anyone will be leading the charge to get modules updated? My concern is that modules need to be updated in a particular order. Modules that depend on emrapi for instance will need to wait for it to be updated before they can be updated and tested (and emrapi in turn would need to wait for modules it depends on). This makes for a process that may not happen naturally without someone dedicated to getting all of the bundled modules updated, or at least laying out a roadmap of what all the modules are that need to be updated and an optimal order , and push the community toward closing some of these things out.

Hopefully these steps will follow shortly after the revised proof-of-concept of how to update modules. Unfortunately I won’t be able to lead the charge on this one, as my free time to work on things has been dramatically reduced of late.

2 Likes

If it simply works that way then it’s great. I was thinking that it might be necessary to ask Spring to load the bean conditionally i.e. if it hasn’t been loaded yet. It may be possible to do it inside a bean factory (annotated with @Configuration). Please let us know if you have some extra time aside to try it in the next few days.

We haven’t discussed working on a branch other than master. Personally, I would not consider that at this point since Hibernate 4 is already in master and it would be hard to revert.

You are correct that some modules will have to wait for others to fully test them out. However, that is only if you want to test them out on the master branch. They should continue to work on all other branches of openmrs-core without having to wait so I think it is safe to just merge in changes in modules and simply note that to test them fully on master, we will have to wait for a few other modules. Determining dependencies prior to that would be nice, but it requires extra work and I do not think it is absolutely necessary as long as we can merge in all changes in a week or two, which should be feasible.

After further thought on this discussion and the resulting Supporting Platform 1.12 and below page…

  • I agree with Darius that we don’t want to put implementations into the situation where they upgrade a module and it breaks their system (i.e., because introducing a new dependency on the hibernate compatibility module).

  • While I thought we might be able to create a jar that could conditionally introduce DbSession for each module, I’m growing nervous that introducing multiple copies of the same class across module classloaders could introduce problems.

So, option 1 (requiring 2.4+, 1.12+, 1.11.3+, 1.10.2+, or 1.9.9+) seems the most robust.

If we allow option 2 (use hibernate compatibility module with existing/older versions of OpenMRS), then users could upgrade a module and break their system (if they don’t have hibernate compatibility installed). Asking module authors to document the requirement is good, but admins can easily upgrade a module without viewing the module documentation.

Should we get rid of option 2? Or can we provide further guidance to module authors – i.e., add a snippet of code that should be added to their module’s startup that will test for need & existence of hibernate compatibility and give the user a very clear error message pointing them to a wiki page that walks them through installing the hibernate compatibility module (as I mentioned above).

fwiw, I still haven’t merged in the pull request for HFE that introduces option #1 (and would make 2.4+, 1.12+, 1.11.3+, 1.10.2+, or 1.9.9+). I can go ahead and do this once we make a final decision… :slight_smile:

FYI – We’re scheduled to make a final decision on this Thursday’s (2015-07-23) dev forum.

1 Like

Was there a decision made about this in last week’s developers forum?

I asked on today’s PM call and it wasn’t discussed on last week’s developer forum.

Here is the decision that needs to made…


Do we allow use of the Hibernate Compatibility module?

  • Require OpenMRS Upgrade – modules that use Hibernate 4 should require 2.4+, 1.12+, 1.11.3+, 1.10.2+, or 1.9.9+. We do not use the Hibernate Compatibility module.

  • Allow use of Hibernate Compatibility module – modules that use Hibernate 4 can choose either to require an upgrade or not require an upgrade in older versions of OpenMRS that install the Hibernate Compatibility module.

Currently, on the wiki, we’re giving module authors the option of using the Hibernate Compatibility module. While this is flexibile, it could surprise/confuse implementations if they upgrade a module that requires Hibernate 4 and do not know about the Hibernate Compatibility module.

Proposal

We update this page and stop permitting use of the Hibernate Compatibility module. If we come up with a way to alert implementations before upgrading a module or fail with a clear error that directs the implementer to the solution (i.e., download & install Hibernate Compatibility module), then we could give module authors the additional option of using Hibernate Compatibility.

1 Like

My modified proposal:

  1. The main distribution of any module should not use the Hibernate Compatibility module, but rather should require an upgrade to openmrs-core.

  2. But on the wiki, we give advanced instructions to cover the small use case of someone who cannot upgrade openmrs-core, but can install new modules, i.e. we tell them how to do a custom build of a module that requires a lower OpenMRS version and also requires the hibernatecompatibility module.

FWIW, for the xforms module, i simply used reflection to support both hibernate 3 and 4 as per this commit:

It’s brilliant! :slight_smile: I thought in Hibernate 3 we only had org.hibernate.classic.Session, but it actually extended org.hibernate.Session so casting what getCurrentSession returns does work.

We can remove more dependencies by moving createQuery etc. to the abstract DAO so the specific DAOs won’t need to use getCurrentSession(). In fact a lot of code could be moved to the parent DAOs.

@dkayiwa, could you please add your solution to https://wiki.openmrs.org/display/docs/Supporting+Platform+2.0+and+below ? It is a good solution for module developers that do not want to require upgrading to the latest maintenance version of a platform.

1 Like

Done. Thanks Rafal for reminding me to update this page!

1 Like