From ec0c74afdf627eab43310c987f009a7a3a1a89d4 Mon Sep 17 00:00:00 2001 From: Andrei Zimin Date: Thu, 14 May 2026 14:45:14 +0300 Subject: [PATCH] feat(user-display): persistent DB cache + Keycloak resolver hook (Phase 1+2 wiring) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes "ток мой UUID отображается" pain reported on staging today: any reviewer / maker / publisher / actor whose JWT sub never landed in the JVM cache shows up as a short UUID in audit / reviews / changelog / events / history / record drawer / webhook detail / record version history. Restart the pod and even captured users disappear. Three-tier resolve, replacing the in-memory-only cache that was the "Альтернатива (на будущее)" TODO in the original UserDisplayService: 1. JVM ConcurrentHashMap hot cache (~µs, capped at 10k). 2. user_display_cache Postgres table (~ms, persistent across restart). New migration 0023 — sub PK + preferred_username + name + email + source enum + synced_at + updated_at, plus an index on synced_at for the scheduled bulk sync that lands later. 3. Keycloak Admin API on-demand (Phase 2) — dispatched to optional KeycloakUserResolver bean. When the bean isn't wired (no Keycloak admin creds yet), the third tier is a no-op and UserCell falls back to short UUID exactly like before. Source enum on every row — JWT_CAPTURE / KEYCLOAK_SYNC / KEYCLOAK_ON_DEMAND — so the eventual scheduled sync can prefer JWT- captured rows (richest claims as the user actually saw them) and the on-demand fallback can be distinguished from bulk sync in audits. JwtUserCaptureFilter and UserDisplayController are unchanged — they already used UserDisplayService.put / find with the same signatures. The behavior change is invisible at the API layer: same /admin/users/ {sub}/display endpoint, same 404 user_not_cached on miss; persistence just survives pod restart now. mergeFrom on UserDisplayCacheEntry deliberately keeps the existing non-blank field when an update brings null — Keycloak sync may have a richer email than JWT capture; don't blank it out. Phase 2 will land a KeycloakAdminUserResolver impl + vault secret plumbing on this same branch. Phase 3 (scheduled bulk sync via @Scheduled) is a separate sprint — it needs the Keycloak admin client already in place. --- .../userdisplay/UserDisplayCacheEntry.java | 124 +++++++++++++ .../UserDisplayCacheRepository.java | 9 + .../changes/0023-user-display-cache.xml | 81 ++++++++ .../main/resources/db/changelog/master.xml | 1 + .../restapi/service/KeycloakUserResolver.java | 24 +++ .../restapi/service/UserDisplayService.java | 174 ++++++++++++++---- 6 files changed, 374 insertions(+), 39 deletions(-) create mode 100644 ordinis-domain/src/main/java/cloud/nstart/terravault/ordinis/domain/userdisplay/UserDisplayCacheEntry.java create mode 100644 ordinis-domain/src/main/java/cloud/nstart/terravault/ordinis/domain/userdisplay/UserDisplayCacheRepository.java create mode 100644 ordinis-migrations/src/main/resources/db/changelog/changes/0023-user-display-cache.xml create mode 100644 ordinis-rest-api/src/main/java/cloud/nstart/terravault/ordinis/restapi/service/KeycloakUserResolver.java diff --git a/ordinis-domain/src/main/java/cloud/nstart/terravault/ordinis/domain/userdisplay/UserDisplayCacheEntry.java b/ordinis-domain/src/main/java/cloud/nstart/terravault/ordinis/domain/userdisplay/UserDisplayCacheEntry.java new file mode 100644 index 0000000..a2cc86a --- /dev/null +++ b/ordinis-domain/src/main/java/cloud/nstart/terravault/ordinis/domain/userdisplay/UserDisplayCacheEntry.java @@ -0,0 +1,124 @@ +package cloud.nstart.terravault.ordinis.domain.userdisplay; + +import jakarta.persistence.Column; +import jakarta.persistence.Entity; +import jakarta.persistence.EnumType; +import jakarta.persistence.Enumerated; +import jakarta.persistence.Id; +import jakarta.persistence.Table; +import org.hibernate.annotations.UpdateTimestamp; + +import java.time.OffsetDateTime; + +/** + * Persistent cache entry for JWT sub → display info resolve. See + * migration 0023-user-display-cache.xml for the table contract and + * the multi-tier resolve strategy. + * + *

Source semantics: + *

+ */ +@Entity +@Table(name = "user_display_cache") +public class UserDisplayCacheEntry { + + @Id + @Column(name = "sub", nullable = false, length = 64) + private String sub; + + @Column(name = "preferred_username", length = 255) + private String preferredUsername; + + @Column(name = "name", length = 255) + private String name; + + @Column(name = "email", length = 320) + private String email; + + @Enumerated(EnumType.STRING) + @Column(name = "source", nullable = false, length = 32) + private Source source = Source.JWT_CAPTURE; + + @Column(name = "synced_at", nullable = false) + private OffsetDateTime syncedAt; + + @UpdateTimestamp + @Column(name = "updated_at", nullable = false) + private OffsetDateTime updatedAt; + + protected UserDisplayCacheEntry() { + // JPA + } + + public UserDisplayCacheEntry(String sub, String preferredUsername, String name, + String email, Source source, OffsetDateTime syncedAt) { + this.sub = sub; + this.preferredUsername = preferredUsername; + this.name = name; + this.email = email; + this.source = source; + this.syncedAt = syncedAt; + } + + public String getSub() { + return sub; + } + + public String getPreferredUsername() { + return preferredUsername; + } + + public String getName() { + return name; + } + + public String getEmail() { + return email; + } + + public Source getSource() { + return source; + } + + public OffsetDateTime getSyncedAt() { + return syncedAt; + } + + public OffsetDateTime getUpdatedAt() { + return updatedAt; + } + + /** + * Idempotent merge from a fresh capture / sync. Updates fields that + * have a new non-null value; leaves existing data otherwise (e.g. + * Keycloak sync may have richer email than JWT capture, don't blank). + */ + public void mergeFrom(String preferredUsername, String name, String email, + Source source, OffsetDateTime syncedAt) { + if (preferredUsername != null && !preferredUsername.isBlank()) { + this.preferredUsername = preferredUsername; + } + if (name != null && !name.isBlank()) { + this.name = name; + } + if (email != null && !email.isBlank()) { + this.email = email; + } + this.source = source; + this.syncedAt = syncedAt; + } + + public enum Source { + JWT_CAPTURE, + KEYCLOAK_SYNC, + KEYCLOAK_ON_DEMAND + } +} diff --git a/ordinis-domain/src/main/java/cloud/nstart/terravault/ordinis/domain/userdisplay/UserDisplayCacheRepository.java b/ordinis-domain/src/main/java/cloud/nstart/terravault/ordinis/domain/userdisplay/UserDisplayCacheRepository.java new file mode 100644 index 0000000..83f98f0 --- /dev/null +++ b/ordinis-domain/src/main/java/cloud/nstart/terravault/ordinis/domain/userdisplay/UserDisplayCacheRepository.java @@ -0,0 +1,9 @@ +package cloud.nstart.terravault.ordinis.domain.userdisplay; + +import org.springframework.data.jpa.repository.JpaRepository; + +public interface UserDisplayCacheRepository extends JpaRepository { + // PK is sub (String). Built-in findById(sub), save, etc. covers Phase 1+2. + // Phase 3 (scheduled bulk sync) will add `findAllBySyncedAtBefore(...)` + // and `findAllByOrderBySyncedAtAsc(Pageable)` for stale-first refresh. +} diff --git a/ordinis-migrations/src/main/resources/db/changelog/changes/0023-user-display-cache.xml b/ordinis-migrations/src/main/resources/db/changelog/changes/0023-user-display-cache.xml new file mode 100644 index 0000000..63c99dc --- /dev/null +++ b/ordinis-migrations/src/main/resources/db/changelog/changes/0023-user-display-cache.xml @@ -0,0 +1,81 @@ + + + + + + + user_display_cache — persistent sub → display info, three-tier resolve + + + + + + + + + + + + + + + + + + + + + + + + + + + Enforce source enum values inline в DB чтобы insert с typo шумел + + ALTER TABLE user_display_cache + ADD CONSTRAINT user_display_cache_source_check + CHECK (source IN ('JWT_CAPTURE', 'KEYCLOAK_SYNC', 'KEYCLOAK_ON_DEMAND')); + + + ALTER TABLE user_display_cache DROP CONSTRAINT user_display_cache_source_check; + + + + + Index for Phase 3 nightly sync — pull stale entries by synced_at + + + + + diff --git a/ordinis-migrations/src/main/resources/db/changelog/master.xml b/ordinis-migrations/src/main/resources/db/changelog/master.xml index 9ce0d6f..c5bba02 100644 --- a/ordinis-migrations/src/main/resources/db/changelog/master.xml +++ b/ordinis-migrations/src/main/resources/db/changelog/master.xml @@ -32,5 +32,6 @@ + diff --git a/ordinis-rest-api/src/main/java/cloud/nstart/terravault/ordinis/restapi/service/KeycloakUserResolver.java b/ordinis-rest-api/src/main/java/cloud/nstart/terravault/ordinis/restapi/service/KeycloakUserResolver.java new file mode 100644 index 0000000..9b7cb87 --- /dev/null +++ b/ordinis-rest-api/src/main/java/cloud/nstart/terravault/ordinis/restapi/service/KeycloakUserResolver.java @@ -0,0 +1,24 @@ +package cloud.nstart.terravault.ordinis.restapi.service; + +import java.util.Optional; + +/** + * Phase 2 contract: resolve sub → user info via Keycloak Admin API. The + * implementation (Phase 2) will be an HTTP client using a service-account + * client + role {@code view-users} on realm {@code nstart}. + * + *

{@link UserDisplayService} consumes this as an optional dependency — + * if no bean exists in the context, the third resolve tier is skipped and + * UserCell falls back to short UUID. This keeps the Phase 1 (DB cache only) + * deployment runnable without Keycloak admin creds. + * + *

Implementations must throw {@link RuntimeException} on transient errors + * (network, 5xx) so the caller can log and continue. Return {@link Optional#empty()} + * for definitive 404 — sub does not exist in the realm. + */ +public interface KeycloakUserResolver { + + Optional findById(String sub); + + record KeycloakUser(String sub, String preferredUsername, String name, String email) {} +} diff --git a/ordinis-rest-api/src/main/java/cloud/nstart/terravault/ordinis/restapi/service/UserDisplayService.java b/ordinis-rest-api/src/main/java/cloud/nstart/terravault/ordinis/restapi/service/UserDisplayService.java index 2bac04f..8108ad1 100644 --- a/ordinis-rest-api/src/main/java/cloud/nstart/terravault/ordinis/restapi/service/UserDisplayService.java +++ b/ordinis-rest-api/src/main/java/cloud/nstart/terravault/ordinis/restapi/service/UserDisplayService.java @@ -1,69 +1,165 @@ package cloud.nstart.terravault.ordinis.restapi.service; +import cloud.nstart.terravault.ordinis.domain.userdisplay.UserDisplayCacheEntry; +import cloud.nstart.terravault.ordinis.domain.userdisplay.UserDisplayCacheEntry.Source; +import cloud.nstart.terravault.ordinis.domain.userdisplay.UserDisplayCacheRepository; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; import java.time.OffsetDateTime; import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; /** - * In-memory кэш user display info (sub → preferredUsername / name / email), - * заполняемый из JWT claims на каждом authenticated запросе через - * {@link cloud.nstart.terravault.ordinis.restapi.auth.JwtUserCaptureFilter}. + * Three-tier resolve sub → display info: + *

    + *
  1. JVM ConcurrentHashMap hot cache (~µs).
  2. + *
  3. {@code user_display_cache} Postgres table (~ms, persistent across pod restart).
  4. + *
  5. Keycloak Admin API on-demand fallback (~10-50ms, Phase 2 — wired + * via optional {@link KeycloakUserResolver} bean).
  6. + *
* - *

Зачем: audit log + record drafts хранят actor как JWT - * {@code sub} UUID. Frontend (UserCell в audit / reviews) показывает short - * UUID + tooltip — плохой UX. Этот сервис позволяет резолвить sub → - * читаемое имя для display, не запрашивая Keycloak Admin API на каждое - * отображение. + *

JwtUserCaptureFilter calls {@link #put} on every authenticated request + * → updates both hot cache and DB (idempotent merge). * - *

Trade-offs: - *

    - *
  • In-memory — пропадает на pod restart. Постепенно заполняется - * обратно по мере того как users делают запросы. Для display это OK.
  • - *
  • Без Keycloak Admin API — не видим users которые ни разу не - * логинились в Ordinis. Display fallback → short UUID.
  • - *
  • Stale: если user сменит preferred_username в Keycloak, кэш - * обновится только на следующий его login + request.
  • - *
+ *

Phase 3 (TBD): scheduled bulk sync from Keycloak will call + * {@link #upsertFromKeycloak} for every realm user every 6h to refill + * stale entries. * - *

Альтернатива (на будущее): миграция 0023 user_display_cache table - * + periodic Keycloak sync. Сделать когда станет реально нужно - * (пока display name никто не редактирует постоянно). + *

Hot cache reads from DB lazily on miss — no warm-up at startup + * (would block readiness probe + Keycloak might be cold). First request + * for each sub after restart pays a single DB hit. */ @Service public class UserDisplayService { - /** Cap чтобы не держать миллионы entries в RAM при abuse. ~100 байт per - * entry → 10k = 1MB. Eviction policy — LRU не нужен, hash без bound — fine. */ - private static final int SOFT_CAP = 10_000; - - private final ConcurrentHashMap cache = new ConcurrentHashMap<>(); + private static final Logger log = LoggerFactory.getLogger(UserDisplayService.class); /** - * Обновляет / создаёт запись для {@code sub}. Idempotent. Вызывается - * из {@code JwtUserCaptureFilter} на каждом authenticated request — - * стоимость ~O(1) hash put. + * Cap чтобы не держать миллионы entries в RAM при abuse. ~100 байт per + * entry → 10k = 1MB. Eviction policy — LRU не нужен, hash без bound — fine. + * DB table сама разрастается без cap (см. migration 0023 design notes). */ + private static final int HOT_CACHE_CAP = 10_000; + + private final ConcurrentHashMap hotCache = new ConcurrentHashMap<>(); + private final UserDisplayCacheRepository repository; + private final KeycloakUserResolver keycloakResolver; + + public UserDisplayService(UserDisplayCacheRepository repository, + @Autowired(required = false) KeycloakUserResolver keycloakResolver) { + this.repository = repository; + this.keycloakResolver = keycloakResolver; + if (keycloakResolver == null) { + log.info("UserDisplayService: KeycloakUserResolver not wired — three-tier resolve " + + "will fall back to short-UUID for users that never made an authenticated " + + "request to ordinis."); + } + } + + /** + * Capture from JWT (called per-request by JwtUserCaptureFilter). Cheap and + * idempotent. Writes to hot cache + DB. Source is always JWT_CAPTURE here — + * KEYCLOAK_SYNC / KEYCLOAK_ON_DEMAND have their own write paths. + */ + @Transactional public void put(String sub, String preferredUsername, String name, String email) { if (sub == null || sub.isBlank()) return; - if (cache.size() >= SOFT_CAP) return; // soft cap, don't grow unbounded - cache.put(sub, new UserDisplayInfo( - sub, - nullIfBlank(preferredUsername), - nullIfBlank(name), - nullIfBlank(email), - OffsetDateTime.now())); + upsert(sub, preferredUsername, name, email, Source.JWT_CAPTURE); } + /** + * Three-tier lookup. Returns empty only if sub absent in all three tiers + * AND Keycloak is unreachable (transient — caller's UserCell will fall + * back to short UUID, retry next time it renders). + */ + @Transactional public Optional find(String sub) { if (sub == null || sub.isBlank()) return Optional.empty(); - return Optional.ofNullable(cache.get(sub)); + + // Tier 1: hot cache + UserDisplayInfo hot = hotCache.get(sub); + if (hot != null) return Optional.of(hot); + + // Tier 2: DB + Optional dbHit = repository.findById(sub); + if (dbHit.isPresent()) { + UserDisplayInfo info = toInfo(dbHit.get()); + cacheHot(sub, info); + return Optional.of(info); + } + + // Tier 3: Keycloak Admin on-demand (Phase 2) + if (keycloakResolver != null) { + try { + Optional kc = keycloakResolver.findById(sub); + if (kc.isPresent()) { + KeycloakUserResolver.KeycloakUser u = kc.get(); + UserDisplayInfo info = upsert(sub, u.preferredUsername(), u.name(), u.email(), + Source.KEYCLOAK_ON_DEMAND); + return Optional.of(info); + } + } catch (RuntimeException e) { + log.warn("Keycloak on-demand lookup failed for sub={}: {}", sub, e.getMessage()); + } + } + + return Optional.empty(); } - /** Stats для observability — сколько users закэшировано. */ - public int size() { - return cache.size(); + /** + * Phase 3 entry point — scheduled sync writes every realm user with + * Source.KEYCLOAK_SYNC. Same idempotent merge as {@link #put}. + */ + @Transactional + public void upsertFromKeycloak(String sub, String preferredUsername, String name, String email) { + if (sub == null || sub.isBlank()) return; + upsert(sub, preferredUsername, name, email, Source.KEYCLOAK_SYNC); + } + + /** Stats для observability. */ + public int hotCacheSize() { + return hotCache.size(); + } + + // --- internals --- + + private UserDisplayInfo upsert(String sub, String preferredUsername, String name, + String email, Source source) { + OffsetDateTime now = OffsetDateTime.now(); + UserDisplayCacheEntry entry = repository.findById(sub).orElse(null); + if (entry == null) { + entry = new UserDisplayCacheEntry(sub, + nullIfBlank(preferredUsername), + nullIfBlank(name), + nullIfBlank(email), + source, + now); + } else { + entry.mergeFrom(preferredUsername, name, email, source, now); + } + repository.save(entry); + UserDisplayInfo info = toInfo(entry); + cacheHot(sub, info); + return info; + } + + private void cacheHot(String sub, UserDisplayInfo info) { + if (hotCache.size() < HOT_CACHE_CAP) { + hotCache.put(sub, info); + } + } + + private static UserDisplayInfo toInfo(UserDisplayCacheEntry e) { + return new UserDisplayInfo( + e.getSub(), + e.getPreferredUsername(), + e.getName(), + e.getEmail(), + e.getUpdatedAt()); } private static String nullIfBlank(String s) {