**refactor: migrate QueueEntryDaoImpl from deprecated Hibernate Criteria API to JPA CriteriaBuilder**#101
Conversation
…Builder in QueueEntryDaoImpl getQueueEntries() and getCountOfQueueEntries() were still using the legacy org.hibernate.Criteria API (deprecated since Hibernate 5.2). Migrated both to JPA CriteriaBuilder, consistent with getOverlappingQueueEntries() which already used it. Extracted shared predicate logic into buildPredicates().
|
Note: Will link a Jira ticket once I get access to issues.openmrs.org. This is part of a broader effort to modernise the DAO layer ahead of a potential Hibernate 6 migration. Happy to split further or adjust scope based on feedback. |
|
One suggestion: consider adding a brief Javadoc on buildPredicates() explaining the filter logic for future contributors unfamiliar with the original Criteria API behaviour. |
|
Hey @shubhangiisinghh! Great refactor! Migrating away from the legacy Criteria API is definitely the right move. I read through the changes and noticed a subtle behavioral difference regarding how the
Since the old code unconditionally created the alias anyway, a safer and more exact 1:1 translation would be to unconditionally create an Join<QueueEntry, Queue> queueJoin = root.join("queue", JoinType.INNER);
// Use the explicit join instead of root.get("queue")
limitCollection(predicates, queueJoin, searchCriteria.getQueues());
if (searchCriteria.getLocations() != null || searchCriteria.getServices() != null) {
limitCollection(predicates, queueJoin.get("location"), searchCriteria.getLocations());
limitCollection(predicates, queueJoin.get("service"), searchCriteria.getServices());
}Other than that, the limitCollection mapping for the empty list vs null checks is spot on. Nice work! |
Migrate QueueEntryDaoImpl to JPA CriteriaBuilder
getQueueEntries() and getCountOfQueueEntries() were still using the legacy Hibernate Criteria API, which has been deprecated since Hibernate 5.2. getOverlappingQueueEntries() in the same class already used JPA CriteriaBuilder — this PR brings the other two methods in line with that.
The main changes:
Replaced org.hibernate.Criteria / Restrictions / Order with CriteriaBuilder equivalents
Pulled the shared filter logic into a buildPredicates() helper to avoid duplicating it across both methods
Added a small limitCollection() utility that mirrors the behaviour of the old limitByCollectionProperty() from the parent class — null collection = no filter, empty collection = IS NULL, non-empty = IN (...)
No behaviour changes, all existing tests pass.