Missing columns

Development
1

Hi everyone! In the last 3 months I worked on switching from Hibernate Mappings to Annotations on OpenMRS domains. I had some problems with a few classes, because after annotating these classes I had some missing columns. How should I deal with that? I let you a google sheet where I wrote the classes that have missing columns. cc @ruhanga

2

Thanks @alinmihaila99 for the enormous effort in having all the openmrs domain annotated.

It’s quite unfortunate about the errors you are facing for a few domains when annotated. This is because there are a few java inheritance and composition inconsistencies in those classes that are actually overridden with their corresponding/associated hibernate mappings. Some of these domains whose switching to annotations work by @wyclif was reverted include Person, Patient, Provider, User, Obs.

Just for the record @alinmihaila99 you could list those domains that are sharing the above limitation here so a consented course of action can be take on these, and perhaps the solutions you have tried already for these.

It appears that some of the possible workarounds are:

  1. Introduce through liquibase changesets columns for the fields that are originally unmapped and inherited from the domains’ superclasses, marking them unused as was previously done in the above domains. The drawback is the introduction of redundant table columns in a domains database table which was undesirable in the first place.
  2. Redesign the domains’ base classes(MappedSuperClasses) to posses only common and relevant fields for all subclass domains.
  3. Allow for overriding un-required domain fields by using hibernate mappings in trickier cases such as the above.

@ibacher, @dkayiwa, @mksd, @wyclif, @burke, do you have any preference or perhaps ideas to implement?

1 Like
3

Do you have a pull request that reproduces this problem to serve as a play ground for investigative purposes?

4

Yes @dkayiwa, an example is this

5

This is closest to my preference. In particular, I’d like to see the relevant fields moved into BaseChangeableOpenmrsData and BaseChangeableOpenmrsMetadata (and have those classes implement the Changeable). This work is mostly done, i.e., they were deprecated in 2.2. That should at least take care of the domain objects which are not changeable (and, as an added bonus, actually reflect this in our API).

However, I don’t think there’s a single one-size-fits-all solution to this. For instance, Privilege and Role should probably actually have the creator / editor / voider properties for auditing purposes, but that probably requires a Liquibase changeset to handle properly (i.e. to add a date created / creator at a minimum to the records in those tables).

1 Like
6

Thanks for the feedback @ibacher.

It’s already helpful to think in this direction. I’d think for compatibility or some other reasons, the fields whose methods are being overridden were left in their superclasses. BaseChangeableOpenmrsData and BaseChangeableOpenmrsMetadata were also possibly introduced with the Hibernate mappings implementation in mind though their super classes are actually annotated.

Indeed @ibacher, so that for purposes of maintaining the only desired persist-able fields in a domain would then require that we keep some hibernate mapping around(Option 3). The hibernate mapping would override base class annotations/field mappings seemlessly.

I’d opt for 3 at the moment until we can have a robust structure of domain classes in relation to their base classes, since using Hibernate Annotations enforces a consistent design across domains unlike Hibernate Mappings. We have ~22% of the core domains facing this limitation to be successfully Annotated. I’m not sure what others have to say about this?

1 Like