refactor(admin): dedicated /api/admin/stats endpoint returning counts only #453
Reference in New Issue
Block a user
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Context
The admin layout (
frontend/src/routes/admin/+layout.server.ts) currently fetches full entity lists (/api/users,/api/groups,/api/tags) solely to display counts on the System stats panel. It loads data it does not render.Problem
.lengthis usedAcceptance criteria
GET /api/admin/statsendpoint returning{ userCount, groupCount, tagCount }(counts only)@RequirePermission(Permission.ADMIN)on the new endpointnpm run generate:apirun after backend changeNotes
This is a tracked backlog item extracted from a TODO comment in
+layout.server.ts:29during CLEANUP-2 (#413).👨💻 Markus Keller — Application Architect
Observations
+layout.server.ts:29is correctly identified — threeJpaRepository.findAll()calls are being made solely to.lengththe result. This is a textbook over-fetch.AdminControllerat/api/adminalready uses@RequirePermission(Permission.ADMIN)at class level. The newGET /api/admin/statsendpoint fits cleanly into that controller rather than needing a new file, but the issue specifiesPermission.ADMINwhile the layout gate checksADMIN | ADMIN_USER | ADMIN_TAG | ADMIN_PERMISSION. A user with onlyADMIN_TAGwould be blocked from the new endpoint — a layering mismatch.StatsControllerat/api/statsusesPermission.READ_ALLand serves person/document counts for the dashboard. The new endpoint serves admin-level entity counts. These are different audiences and different permission levels — keeping them separate is correct; do not adduserCount/groupCount/tagCounttoStatsDTO.AppUserRepository,UserGroupRepository, andTagRepositoryall extendJpaRepositorywhich already exposescount()(a singleSELECT COUNT(*)) without any new query declaration needed. The implementation is trivially cheap.Recommendations
GET /api/admin/statsin the existingAdminControllerbut lower the permission toPermission.ADMIN_USERon the method (not class level) — or create a minimal newAdminStatsControllerwith a looser class-level permission that covers the full set of admin variants (ADMIN,ADMIN_USER,ADMIN_TAG,ADMIN_PERMISSION). The cleanest fit: newrecord AdminStatsDTO(long userCount, long groupCount, long tagCount)in theuserpackage (where user/group admin lives), wired to aAdminStatsServicethat callsuserRepository.count(),groupRepository.count(),tagRepository.count(). The controller gets@RequirePermission(Permission.ADMIN_USER)but that still blocksADMIN_TAG-only users from seeing the tag count in nav. Worth discussing with Elicit.CLAUDE.mddoes not need updating for a new method inside an existing controller, but ifAdminStatsControlleris created as a new class, the package table does not need updating since it stays withinuser/.userCount/groupCount/tagCountto the existingStatsDTO— those fields serve the dashboard (persons/documents). Mixing admin operational counts with dashboard analytics counts creates a DTO that serves two masters.+layout.server.tsspec test will need updating: the mock currently wires up threeapi.GET(...)calls for users/groups/tags; after this change it becomes one call for/api/admin/statsplus the invite fetch. Update the mock accordingly — do not leave the three-call mock in place.Open Decisions
Permission.ADMIN(maintenance-only admins see counts) vsPermission.ADMIN_USER(user admins see counts, but tag-only admins still blocked) vs a new composite check. The layout currently gates the entire admin area on any admin permission, so the stats endpoint must be accessible to any admin variant — consider a dedicatedPermission.ADMIN_STATSor accept that any authenticated admin can read counts (data sensitivity is low: counts only, no names).AdminController(simpler, but mixes maintenance actions with read endpoints) vs a newAdminStatsController(cleaner SRP but more files). Given the project's feature-package principle, both land inuser/— preference is a new controller.👨💻 Felix Brandt — Fullstack Developer
Observations
+layout.server.tsdoes three parallelPromise.allfetches then calls.lengthon each result. All three responses are otherwise discarded — the full entity arrays are never passed to the Svelte component. The TODO comment at line 29 correctly flags this.JpaRepository.count()already exists on all three repositories (AppUserRepository,UserGroupRepository,TagRepository) — no custom@Queryis needed.layout.server.spec.tsat line 9 mocks three sequentialGETcalls in order (users → groups → tags). After the refactor this mock signature changes: oneGETfor/api/admin/statsreplaces the three. The existing test at line 52 (returns entity counts and permission flags for a full admin) will need its mock and assertions updated.EntityNav.sveltecomponent receivesuserCount,groupCount,tagCountas individual number props — this interface does not need to change after the refactor, only how the layout server populates those props.fetch(not the typed API client) because it's via the internal URL. That fetch is separate and stays as-is — only the three typed API client calls are being replaced.Recommendations
record AdminStatsDTO(long userCount, long groupCount, long tagCount)— use a Javarecord, not a Lombok class, consistent with the existingStatsDTOrecord pattern. Place it in theuserpackage alongside the other admin types.AdminStatsService.getAdminStats()— one public method, under 10 lines, callsuserRepository.count(),groupRepository.count(),tagRepository.count(). Annotate@Service @RequiredArgsConstructor. No@Transactional— these are reads.GET /api/admin/statsreturningAdminStatsDTO. If adding toAdminController: remove the class-level@RequirePermission(Permission.ADMIN)guard on the method and add@RequirePermissionat method level only (so the maintenance actions keep theirADMINpermission and the stats method gets a lighter one). Cleaner: a newAdminStatsController.Promise.allblock with a singleapi.GET('/api/admin/stats')call. Runnpm run generate:apiafter backend changes — the new endpoint must appear in the OpenAPI spec before the frontend types will compile.AdminStatsControllerTestfirst (red → green): verify 401 unauthenticated, 403 without admin permission, 200 with correct JSON shape{ userCount, groupCount, tagCount }. Updatelayout.server.spec.tsmock from three GET calls to one.generate:api→ frontend load function test update → implementation update.Open Decisions
👨💻 Tobias Wendt — DevOps & Platform Engineer
Observations
JpaRepository.count()translates toSELECT COUNT(*) FROM table— a single indexed scan. No explain-plan concern at any foreseeable table size.api.GET('/api/admin/stats')call in+layout.server.tsruns server-side (SSR), so the HTTP round-trip stays internal (SvelteKit Node process → Spring Boot). No public-network latency added.AdminStatsControllerTest(or equivalent) — this runs in the existing@WebMvcTestslice pattern and adds minimal time. No new Testcontainers setup needed since count queries don't exercise anything Postgres-specific.Recommendations
docker-compose.yml, CI workflows, or deployment config.management.metrics.enabled=trueis the default in Boot 4), the new endpoint will automatically appear in/actuator/metrics/http.server.requests— no extra instrumentation needed. That's a nice monitoring win for free.Open Decisions
👨💻 Elicit — Requirements Engineer
Observations
+layout.server.ts:29extracted during CLEANUP-2 #413). Definition-of-Ready score is high.GET /api/admin/statsis unspecified. The issue says@RequirePermission(Permission.ADMIN)but the layout currently grants access to users with any ofADMIN | ADMIN_USER | ADMIN_TAG | ADMIN_PERMISSION. A user with onlyADMIN_TAG(tag administrator) currently sees their tag count in the nav bar. If the new endpoint usesPermission.ADMIN, that user will get a 403 and see no counts — a regression in observable behavior even though the AC doesn't call it out.fetch. That fetch is separate and not covered by this issue. Fine — but worth a note so it doesn't get accidentally touched during implementation..length. The AC is confirming no regression, not describing a new behavior. That's fine.Recommendations
@RequirePermission(Permission.ADMIN_USER)or a broader check. Do not leave this for the implementer to decide; the permission level is a product decision, not an engineering one.{ userCount, groupCount, tagCount }shape."layout.server.spec.tsmock updated to reflect single-endpoint fetch; all existing test assertions continue to pass."Open Decisions
/api/admin/stats? Options: (a) any of the four admin permissions — matches the current nav gate; (b)ADMIN_USERonly — user admins see all counts but tag-only admins see no counts in nav; (c) newPermission.ADMIN_STATS— maximum precision but adds enum value. The product owner should decide this before implementation.👨💻 Nora "NullX" Steiner — Security Engineer
Observations
/api/users,/api/groups,/api/tags) are each individually permission-gated: users and groups requireADMIN_USER/ADMIN_PERMISSIONrespectively; tags require no permission at all (GET /api/tagsis unguarded — it's the tag search/autocomplete endpoint used everywhere). The layout is leaking a read of the full tag list to establish a count, bypassing the fact that tag count is not sensitive data.@RequirePermission(Permission.ADMIN). Given that the full user list requiresADMIN_USERand the full group list requiresADMIN_PERMISSION, the new endpoint returning only counts is strictly less sensitive.Permission.ADMINis a reasonable lower bound; the data (integer counts) carries no PII. However, the permission must not accidentally be left off — without@RequirePermissionthe endpoint would be accessible to any authenticated user withREAD_ALL.AdminControllerhas@RequirePermission(Permission.ADMIN)at class level. If the new stats method is added there, it inheritsADMIN— which is the maintenance permission. A user withADMIN_USERbut notADMINwould be blocked. This is a silent security regression (user admins can currently see the count via the full list, but the new more-efficient endpoint would block them).Recommendations
AdminControllerwith its class-level@RequirePermission(Permission.ADMIN). Create a dedicated controller or use method-level annotation to set the correct permission for the stats endpoint independently.@RequirePermissionannotation must be present and tested: writeshould_return_403_when_user_lacks_any_admin_permission()andshould_return_401_when_unauthenticated()tests before writing the implementation.log.debug("Admin stats fetched: users={}, groups={}, tags={}", u, g, t)). Do not log entity names or IDs — not applicable here since the response contains only counts.Open Decisions
👨💻 Sara Holt — QA Engineer
Observations
layout.server.spec.tsis well-structured: it usesvi.mock, factory functions for users, and covers permission edge cases (no perms, undefined user, no groups, tag-admin-only access). After the refactor, themockApihelper and thereturns entity countstest will need updating — the three-sequential-GET mock becomes a single GET.StatsControllerTest(forGET /api/stats) is a good reference pattern for the new admin stats controller test: it uses@WebMvcTest, importsSecurityConfig + PermissionAspect + AopAutoConfiguration, and tests 401, 403, and 200. The new test should follow the same pattern exactly.AdminStatsServiceTest): mock the three repositories, verify the DTO is constructed correctly with correct counts.AdminStatsControllerTest):@WebMvcTest, verify 401/403/200 with response shape assertion.JpaRepository.count()is framework code; there's nothing domain-specific to test at the integration layer.mockResolvedValueOncechain with a singlemockResolvedValuereturning{ userCount: 2, groupCount: 1, tagCount: 3 }. All existing assertions (expect(result.userCount).toBe(2)etc.) continue to pass.Recommendations
AdminStatsControllerTestcovering:getStats_returns401_whenUnauthenticated,getStats_returns403_whenUserLacksAdminPermission,getStats_returns200_withCorrectShape. Mirror the pattern fromStatsControllerTest.AdminStatsServiceTestwith mocked repositories: one test for the happy path, one for all-zero counts.layout.server.spec.ts: themockApihelper should switch from threemockResolvedValueOncecalls to one that returns{ data: { userCount, groupCount, tagCount } }. The existing permission-gate tests (throws 403, etc.) don't invokemockApiso they require no changes.npm run generate:api, runnpm run checkto confirm the new API type resolves correctly in+layout.server.tsbefore committing.Open Decisions
👨💻 Leonie Voss — UI/UX Designer
Observations
2 users,1 group,5 tags) continue to render identically. No visual regression, no layout change, no Svelte component changes.EntityNav.sveltereceivesuserCount,groupCount,tagCountas individual number props — this prop interface is unchanged by the refactor. The counts are rendered as badge text insideEntityNavSectioncomponents. No touch-target, contrast, or font-size implications.hidden md:flex). This is a pre-existing design decision outside this issue's scope.Recommendations
0or missing during a future SSR error state (e.g., if the new stats endpoint is slow or unavailable), the nav badges should degrade gracefully. The current error handling in+layout.server.tsthrows a SvelteKiterror()which results in a full error page — that's fine and already matches existing behavior.Open Decisions
Decision Queue
Raised by Markus, Elicit, and Nora. One theme dominates:
Theme 1: Permission level on
GET /api/admin/statsThe issue body specifies
@RequirePermission(Permission.ADMIN)but three personas independently flagged this as a regression: users withADMIN_TAGorADMIN_USER(withoutADMIN) currently see entity counts in the nav bar — they reach those counts via the full-list endpoints they already have access to. A blanketADMINguard on the new endpoint would silently break the count display for those users.Options:
Permission.ADMINADMIN) can call the endpoint. Tag-only and user-only admins see no counts — silent regression.Permission.ADMIN_USERADMIN_TAGonly) still blocked. Partial regression for tag-only admins.ADMIN,ADMIN_USER,ADMIN_TAG,ADMIN_PERMISSIONcan call the endpoint. No regression. Requires either a newPermissionvalue or a composite@RequirePermissionapproach.Permission.ADMIN_STATSfrontend/src/lib/shared/errors.ts+ i18n keys. Most future-proof but most churn for a simple count endpoint.Recommended default (implement unless overridden): Option C — any admin permission grants access to counts. The data is non-sensitive (integers, no PII), and it matches the existing behavior exactly. Implement as: do not use the existing
AdminController(which hasADMINat class level); instead create a dedicatedAdminStatsControllerwith no class-level@RequirePermission, and apply the permission check at method level using whatever is the least-privileged admin permission the endpoint should accept.Theme 2: Which controller to put the new endpoint in
Markus and Nora both flagged that adding
GET /api/admin/statsto the existingAdminControllerwould inherit its class-level@RequirePermission(Permission.ADMIN), causing the permission regression above. The unanimous recommendation: create a newAdminStatsControllerin theuserpackage. This avoids touchingAdminController's permission model, keeps SRP, and is a trivial addition.