TRUNK-5899: Replace Hibernate XML mapping with JPA annotations for ConceptAttributeType #5008
Conversation
| @@ -26,7 +26,7 @@ | |||
| <mapping resource="org/openmrs/api/db/hibernate/Concept.hbm.xml" /> | |||
| <mapping resource="org/openmrs/api/db/hibernate/ConceptAnswer.hbm.xml" /> | |||
| <mapping resource="org/openmrs/api/db/hibernate/ConceptAttribute.hbm.xml" /> | |||
| <mapping resource="org/openmrs/api/db/hibernate/ConceptAttributeType.hbm.xml" /> | |||
| <!-- <mapping resource="org/openmrs/api/db/hibernate/ConceptAttributeType.hbm.xml" /> --> | |||
There was a problem hiding this comment.
| <!-- <mapping resource="org/openmrs/api/db/hibernate/ConceptAttributeType.hbm.xml" /> --> |
There was a problem hiding this comment.
I added this change into my updated pull request. Thank you for the suggestion!
api/pom.xml
Outdated
| <groupId>jakarta.persistence</groupId> | ||
| <artifactId>jakarta.persistence-api</artifactId> | ||
| <version>2.2.3</version> | ||
| </dependency> |
There was a problem hiding this comment.
struggling to understand this change, care to explain?
There was a problem hiding this comment.
Absolutely! This dependency was added to ensure that the JPA (Jakarta Persistence API) annotations maintain compatibility with current Java versions supported by OpenMRS. Since Java 8 is the current baseline requirement for OpenMRS 2.x, I chose JPA dependency version 2.2.3 as it is the last version compatible with Java 8. This allows the JPA annotations to be added to the ConceptAttributeType domain class without needing to upgrade the whole codebase to Java 11+.
There was a problem hiding this comment.
I dont see where youre using that library in your changes
There was a problem hiding this comment.
but those ones are being imported by javax.persistence.Id; and javax.persistence.Column; which are not coming from the dependency you have added above.
There was a problem hiding this comment.
I see. You are correct. Jakarta is the modernized version of javax, but does not contain the javax.persistence packages that I imported. I tested my changes both with and without the jakarta.persistence dependency, and it works correctly in both. I will update my pull request to reflect this change. Thank you for pointing this out!
There was a problem hiding this comment.
This dependency is now removed in my updated pull request. My update still passes all new and existing unit tests. Thank you for your help and review!
There was a problem hiding this comment.
If you feel like jumping into that kind of work of moving stuff from javax to jakarta, feel free to create a ticket on jira and geek yourself away 🥲
There was a problem hiding this comment.
I will definitely create a ticket! Anything other clarifications or changes you would recommend for this task?
…nceptAttributeType - add source code form to ConceptAttributeTypeTest.java - removed jakarta.persistence dependency in pom.xml based on code review
TRUNK-5899: Replace Hibernate XML mapping with JPA annotations for ConceptAttributeType
Description of what I changed
Issue I worked on
see https://issues.openmrs.org/browse/TRUNK-5899
Checklist: I completed these to help reviewers :)
My IDE is configured to follow the code style of this project.
I have added tests to cover my changes. (If you refactored
existing code that was well tested you do not have to add tests)
I ran
mvn clean packageright before creating this pull request andadded all formatting changes to my commit.
All new and existing tests passed.
My pull request is based on the latest changes of the master branch.