O3-5445: Fix duplicate SQL query in AbstractBaseQueueDaoImpl#get(String uuid)

[sanks011]

Description

AbstractBaseQueueDaoImpl#get(String uuid) was calling criteria.add(eq("uuid", uuid)).uniqueResult() twice on the same mutable Criteria object - firing two SQL queries per UUID lookup and adding a redundant duplicate WHERE uuid = ? clause on the second query.

This affected every UUID-based entity lookup across the module (QueueEntry, Queue, QueueRoom, RoomProviderMap).

Fix removes the orphaned first call and keeps a single criteria.add(...) → uniqueResult() chain.

Related

https://openmrs.atlassian.net/jira/software/c/projects/O3/boards/37?selectedIssue=O3-5445

[bhavya-jpg]

@sanks011 Hey, great catch! Silent duplicate queries like this in Hibernate are super easy to miss since standard DAO tests only check if the right entity comes back, and they completely ignore the extra JDBC overhead. This is a solid performance fix.

I was looking at the changes and they look good to me. I just had two quick thoughts:

Should we add a test to make sure this doesn't happen again? Since standard assertions don't catch duplicate queries, maybe we could enable Hibernate Statistics for this specific query test and assert that statistics.getEntityLoadCount() == 1. That way if anyone adds a duplicate Criteria fetch in the future, the test will actually fail.

Since BaseDao classes get copy-pasted a lot when setting up new entities, is it possible this exact same .uniqueResult() glitch exists in other DAOs in this module? It might be worth doing a quick global search for uniqueResult() duplicate calls just to be absolutely sure we got all of them.

Awesome debugging on this, nice work cleaning it up!

[sanks011]

@sanks011 Hey, great catch! Silent duplicate queries like this in Hibernate are super easy to miss since standard DAO tests only check if the right entity comes back, and they completely ignore the extra JDBC overhead. This is a solid performance fix.

I was looking at the changes and they look good to me. I just had two quick thoughts:

Should we add a test to make sure this doesn't happen again? Since standard assertions don't catch duplicate queries, maybe we could enable Hibernate Statistics for this specific query test and assert that statistics.getEntityLoadCount() == 1. That way if anyone adds a duplicate Criteria fetch in the future, the test will actually fail.

Since BaseDao classes get copy-pasted a lot when setting up new entities, is it possible this exact same .uniqueResult() glitch exists in other DAOs in this module? It might be worth doing a quick global search for uniqueResult() duplicate calls just to be absolutely sure we got all of them.

Awesome debugging on this, nice work cleaning it up!

Thanks for the review! Addressing both points:

1. Hibernate Statistics test: The original bug was a one-off copy-paste artifact (orphaned statement before the return). SessionStatistics.getEntityLoadCount() tracks Session.get()/load() calls, not Criteria.uniqueResult() invocations - so it wouldn't actually catch this class of bug. Query-counting tests also tend to be fragile across Hibernate versions and caching behavior changes. Standard code review is the right safeguard here.

2. Other DAOs: Already checked - none of the concrete DAO implementations (QueueDaoImpl, QueueRoomDaoImpl, RoomProviderMapDaoImpl, QueueEntryDaoImpl) override get(String uuid). They all inherit from AbstractBaseQueueDaoImpl, so this single fix covers all entity lookups. The only other uniqueResult() call in the module is QueueEntryDaoImpl#getCountOfQueueEntries(), which is used correctly (single invocation with a row count projection). No duplicate query patterns exist elsewhere.

TL;DR - the fix as-is is complete and sufficient. No additional test or DAO changes needed.