perf: resolve managed-group ids in one query for createUserDetails

[jason-p-pickering]

Background

While analysing the slow PATCH /api/users/{uid} traces, we confirmed that the 93s production cliff (Nigeria, ~300k users) is already fixed on master by #23103, #23123, #23126 — those removed the unbounded UserRole.members/UserGroup.members loads. A fresh v44 master trace on Sierra Leone (PATCH, 209ms, 76 jdbc queries) shows no query now scales with total users.

That trace did surface a bounded N+1 this PR removes:

SELECT ... FROM usergroupmanaged ... WHERE managedbygroupid = ?
   executions: 10   rows/exec: 0

Root cause

UserDetails.createUserDetails(User) builds managedGroupLongIds from User.getManagedGroups():

return groups.stream()
    .flatMap(group -> emptyIfNull(group.getManagedGroups()).stream())  // lazy-init per group
    .collect(...);

group.getManagedGroups() is a lazy collection, so this initialises one usergroupmanaged query per group the user belongs to — on every UserDetails construction from an entity (principal building, metadata import), not just PATCH.

Change

Resolve the managed-group primary keys in a single SQL query against the join table. The consumer (managedGroupLongIds, a Set<Long>) only needs PKs, so no UserGroup entities are hydrated and no usergroup join is needed:

SELECT managedgroupid FROM usergroupmanaged WHERE managedbygroupid IN (:userGroupPrimaryKeys)

• UserGroupStore.getManagedGroupIds(Collection<Long>) + HibernateUserGroupStore impl
• UserGroupService.getManagedGroupIds(...) delegate — keeps DefaultUserService's constructor unchanged
• UserDetails.createUserDetails(...) overload accepting pre-resolved managedGroupLongIds; null falls back to the entity walk, so fromUser / fromUserDontLoadOrgUnits / ApiTokenAuthManager are unchanged
• DefaultUserService.createUserDetails(User) computes the PKs from the already-loaded user.getGroups() and passes them in

Scope / risk

• Bounded win, not the Nigeria cliff fix. Scales with groups-per-user (tens), not total users. Worth doing because it runs on every entity-based UserDetails construction.
• Cache coherence: reads committed state via JDBC, consistent with the existing HibernateUserGroupStore SQL bypasses; introduces nothing new. The bypass is confined to principal construction — the deletion handler that mutates managedGroups still uses ORM.

Tests

• UserGroupServiceTest.testGetManagedGroupIds / …EmptyInputReturnsEmpty — store method (TDD).
• UserServiceTest.testCreateUserDetailsResolvesManagedGroupLongIds — characterization proving createUserDetails(...).getManagedGroupLongIds() is unchanged through the new path.
• Existing UserServiceTest.testManagedGroups regression — still green.

🤖 Generated with Claude Code

[sonarqubecloud]

Quality Gate failed

Failed conditions
2 New issues
2 New Code Smells (required ≤ 0)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

[codecov]

Codecov Report

❌ Patch coverage is 87.50000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 60.50%. Comparing base (17f8133) to head (3e0cda5).
⚠️ Report is 22 commits behind head on master.

Files with missing lines	Patch %	Lines
...in/java/org/hisp/dhis/user/DefaultUserService.java	50.00%	1 Missing and 1 partial ⚠️

❗ There is a different number of reports uploaded between BASE (17f8133) and HEAD (3e0cda5). Click for more details.

HEAD has 2 uploads less than BASE

Flag	BASE (17f8133)	HEAD (3e0cda5)
integration	4	3
integration-h2	1	0

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #24097      +/-   ##
============================================
- Coverage     69.04%   60.50%   -8.54%     
+ Complexity      709      619      -90     
============================================
  Files          3684     3686       +2     
  Lines        141646   141968     +322     
  Branches      16453    16511      +58     
============================================
- Hits          97797    85897   -11900     
- Misses        36243    48971   +12728     
+ Partials       7606     7100     -506     

Flag	Coverage Δ
integration	45.86% <75.00%> (-3.79%)	⬇️
integration-h2	?
unit	34.99% <25.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
.../src/main/java/org/hisp/dhis/user/UserDetails.java	80.16% <100.00%> (-4.32%)	⬇️
...main/java/org/hisp/dhis/user/UserGroupService.java	100.00% <ø> (ø)
...c/main/java/org/hisp/dhis/user/UserGroupStore.java	100.00% <ø> (ø)
...va/org/hisp/dhis/user/DefaultUserGroupService.java	42.62% <100.00%> (-40.72%)	⬇️
...p/dhis/user/hibernate/HibernateUserGroupStore.java	26.66% <100.00%> (-65.84%)	⬇️
...in/java/org/hisp/dhis/user/DefaultUserService.java	47.79% <50.00%> (-23.42%)	⬇️

... and 801 files with indirect coverage changes

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5a1b89b...3e0cda5. Read the comment docs.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[jason-p-pickering]

Marking this as draft. The SonarQube warning is a hint here. We basically went over the edge from 7-> 8 params. We have a total hodge-podge of pre-resolved entities which are passed in, and others which are resolved inside the factory. Maybe its better we make a decision where this is actually done?

[sonarqubecloud]

Quality Gate failed

Failed conditions
3 New issues
2 New Code Smells (required ≤ 0)
1 New Vulnerabilities (required ≤ 0)
C Security Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

💡 Need a hand with PR review? Try Gitar by Sonar!