I’m implementing a custom module to deploy metadata to a custom distro, using metadata deploy module . Right now, I’m just loading Locations and some PersonAttributes. However, whilst I can create locations, I’m not able to create PersonAttributes, since when I load the module, I get the following exception:
After that I compared, both LocationAttributeTypeDeployHandler and PersonAttributeTypeDeployHandler classes, and I realised that whilst LocationAttributeTypeDeployHandler delegates persistence to a service, which delegates to a dao, PersonAttributeTypeDeployHandler tries to do it directly using the following code:
I also realised, indeed, that there are a PersonDAO.savePersonAttributeType, with a hibernate implementation, so it seems for me that someone forgot to join it together, invoking the dao (directly or indirectly) from PersonAttributeTypeDeployHandler.
@dkayiwa, thanks for fixing this so quickly! A few comments:
I don’t see a ticket in the metadata deploy project in jira: https://issues.openmrs.org/browse/DPLY. Can you please create one and link this commit to it?
I notice that you wrote custom session handling functionality to bridge the Hibernate 3 vs. 4 differences. Isn’t that what the DbSessionFactory / DbSession classes were built to solve for us? I would think the right solution is to change our session to reference DbSession instead, use that in our DAOs, and ensure that config.xml supports OpenMRS versions that contain this class. This is what we did for most of the other modules that span these versions, from what I recall.
Hi @mseaton, i did it as part of RA-954 as you can see from the commit message.
Apart from the one test class, i found only once instance of sessionFactory.getCurrentSession() and i decided to do the simpler fix to make sure @fmalmeida is unblocked as quickly as possible.
I did not have time to do the longer DbSession route. May be a GSOC student could take on that.
Thanks @dkayiwa. I did see that on the commit. That being said, I think @burke’s comment (channeling me) from last February on that ticket still applies:
I definitely appreciate you moving quickly on this to help move the blocker. I think we should adopt a consistent approach when at all possible, particularly if there are best practice conventions that we are asking the community to adopt. Let’s get it ticketed, as an intro ticket, to get that moved over to use DbSession, which perhaps a GSoC Applicant can take on, as you suggest.
