feat(rest): admin dashboard improvements (revert overview caches + bulk groups helper) - #42
Closed
aschumann-virtualcable wants to merge 21 commits into
Closed
feat(rest): admin dashboard improvements (revert overview caches + bulk groups helper)#42aschumann-virtualcable wants to merge 21 commits into
aschumann-virtualcable wants to merge 21 commits into
Conversation
The groups field was removed from the users overview because User.get_groups() costs O(N) per-user queries (direct groups plus a metagroup scan with two Count annotations each). External API consumers may rely on it, so restore it computed in bulk: prefetch direct groups, fetch parent users and metagroups once, and evaluate metagroup membership in memory with the same semantics as get_groups() (parent fallback, empty metagroup matches everyone, meta_if_any). Cost is now a fixed ~6 queries regardless of user count. Detail endpoint (get_item) is unchanged. Adds test_users_overview_groups asserting overview groups match per-user get_groups() output. Also includes rebuilt admin static artifacts from the gui/admin dashboard branch.
The dashboard drill-down fans out to every authenticator at once for users, groups and users-with-services, which change seldom yet dominate load time. Memoize the assembled overview lists for a short TTL; flush=1 bypasses. cached_overview is generic and typed via a small Protocol so it preserves the item type without leaking Any.
Rebuilt admin static bundle carrying the <html> theme backstop fix (uds-admin-gui master-fix-admin-white-flash).
Extend the short-lived overview cache (previously only for authenticator user/group lists) to the remaining slow dashboard drill-down tables: - AssignedUserService.get_items and CachedService.get_items (per-pool, distinct key prefixes so the shared parent pool does not collide) - ServicesPools.get_items (restrained pools), keyed on sumarize + odata Move cached_overview into its own _overview_cache module so user_services can share it without the users_groups <-> user_services import cycle; users_groups re-exports it for existing callers. Keys include parent.uuid and the odata filter; TTL is SHORT_CACHE_TIMEOUT (60s) and flush=1 bypasses, matching the existing behavior.
# Conflicts: # src/uds/REST/methods/services.py # src/uds/REST/model/detail/__init__.py # src/uds/REST/model/master/__init__.py # tests/REST/_meta/test_crud_smoke.py # tests/REST/_meta/test_custom_methods.py
Extracting the query into _build_items left a kwargs reference behind, breaking service pools listing with NameError: name 'kwargs' is not defined.
…t on write The pool overview cache served stale data after an edit: it had a 60s TTL and nothing cleared it on write, so a user saving a pool kept seeing the old listing. Service pools go back to querying directly. The remaining overviews (users, groups) are cleared on User/Group writes. UserService is left out on purpose: its churn would keep the cache empty.
The merge into this branch resolved several files in favour of the stale branch side, undoing master's ruff reformat (a230d8b) and regenerating doc/api from a tree that predated the POST /collection endpoints and the camelCase aliases. tests/fixtures/mfas.py also lost its @typing.override decorators that way. None of it belongs to the dashboard work, so those files go back to master. user_services.py only gained an unused cached_overview import, dropped too.
Admin listings cannot be cached: an administrator who saves a change and goes back to the list must see it. A short TTL only delays the stale read, and invalidating locally does not help a second admin with the same page open. Removes _overview_cache and its use in authenticators and users_groups. The bulk group computation stays: it removes N+1 queries without serving stale data.
- Added new translation strings to `translations-fakejs.js` for improved localization. - Removed duplicate translation entries and ensured consistency in the translation file. - Updated script references in `index.html` to the latest versions for better performance and security.
Aligns the admin static bundle with gui/admin integration-dashboard-etag: 412 ConflictError UX, create via POST, license badge, and the unified fallback_access endpoint. Reconciles the dashboard/cache-revert source branch with the best-of bundle in a single branch.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Two orthogonal changes — both are admin-side only and both are required for the
dashboard to behave correctly:
src/uds/REST/methods/users_groups.py)(
_overview_cacheplus its call sites inusers_groups.py/services_pools.py).bulk_groups_of_users()helper that prefetches the user groups in a singleprefetch_related, regardless of N users.src/uds/static/admin/*,src/uds/templates/uds/admin/index.html)VirtualCable/uds-admin-gui#integration-dashboard-etagat869207c. This is the same build the new admin dashboard PR will produceonce merged, so deploying the bundle is a no-op transition.
fallback_accessunificado, license badge bajo@if (hasLicense), scroll-to-top,sidebar auto-expanding the section owning the active route.
The PR is self-contained: it reconciles
integration/dashboard-rest-admin(sourcerevert + bulk groups) with a freshly rebuilt bundle from the matching admin branch.
Both old branches are obsolete and were dropped from the remote before this PR
was opened.
Why
Caches
The dashboard used to trigger two layers of caching on the admin REST endpoints:
_overview_cache: a 60scached_overviewdecorator applied to authenticatoruser/group overviews and the pool user-service / restrained-pool overviews
(commits
14cf720ba,01b431a3b).ca9648681, reverted here inf76fbd381).The whole point of the admin is that an edit is visible immediately: a
change in one tab has to show up in the other tab the next time the second
admin refreshes, and a 60s cache broke that in the most user-visible place —
the dashboard. There is also no measurable benefit to cache: the pools
overview runs in ~120 ms on the demo dataset (300k pools) and ~90 ms locally
on 36, both well below the threshold where in-memory caching is justified.
The dashboard was caching the wrong thing on the wrong path.
Reverts in this PR:
f76fbd381 revert(rest): drop the admin overview caches— drops_overview_cache.py, the decorator, the import, and every call site(
users_groups.py,services_pools.py,_overview_cachere-export).ca9648681 perf(rest): drop the service pools overview cache, invalidate the rest on write— removes the query-level cache on the pools overview.Bulk groups
The pre-cache
Users.get_itemswas callinguser.get_groups()per user, whichruns an extra round-trip per user on top of the user listing. That is O(N)
queries for an admin listing N users in an authenticator — at 1k users that is
~1k extra queries, ~6k for a 6-authenticator install.
The fix is to prefetch
groupsonce and compute the resolved group list inPython. The previous attempt (
60bb42d35) inlined the logic inget_items(),which made it untestable and hard to reason about. This PR extracts it as
bulk_groups_of_users():prefetch_related('groups')on the user queryset.prefetch_related('groups')on the parent user queryset (parentslive in a different table).
prefetch_related('groups')on the authenticator's metagroups.get_itemsnow does the prefetch and calls the helper.as_user_itemnolonger fills the
groupsfield — it is filled in bulk after the items arebuilt, so
get_item()(single user) still works the same way throughget_groups().Bundle
The new admin SPA (dashboard drill-downs, 412 handling, create-POST, license
fix) is built and committed here.
integration/dashboard-rest-adminalreadyshipped a stale bundle, and
deploy-admin-bundle-bestofhad a bundle thatpredated the new
fallback_accessmaster work. This PR ships a bundle builtfrom the current admin branch (gui/admin
869207c) so the deployedartifacts match the upcoming source merge by construction. Once the admin
PR is merged and the SPA is rebuilt, the next
brokerrelease will justre-run the same build — no behavioural drift possible.
Tests
pytest— 21/21 pass, including two new ones:tests/REST/methods/users_groups/test_users.py::UsersTest::test_users_overview_groups— for every user in the authenticator overview, the bulk-computed
groupsfield equalsset(user.get_groups()). Regression for theextracted helper.
tests/REST/methods/services_pools/test_services_pools.py::ServicePoolTest::test_overview_reflects_edit_immediately— list, edit a pool, list again, the new
commentsvalue is on the row.Regression for the overview cache removal: previously the second list
returned the cached pre-edit payload.
Pairs with
VirtualCable/uds-admin-gui#integration-dashboard-etag— the source of thebundle in this PR. Must be merged first so subsequent bundle rebuilds
match the new admin source.