From 9d39b1bd11fdfca4da67aaf533692caa44a4e4aa Mon Sep 17 00:00:00 2001 From: rcourtman Date: Wed, 5 Aug 2026 18:39:54 +0100 Subject: [PATCH] Share per-generation resource lists across API requests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every list-shaped resources request deep-cloned the whole registry: HandleListResources via ListForPresentation, the storage summary and incidents handlers and the k8s namespaces handler via List/ListByType, and the stats handler once more. With the frontend polling three pages plus summaries every few seconds, the same unchanged world was cloned dozens of times between snapshot updates. Cache the raw and presentation lists on the existing per-generation registry cache entry (same invalidation: entries rebuild when the seed lastUpdate moves) and hand requests a flat top-level copy instead. Every decorator in the request pipeline was audited to write only top-level fields on request-owned elements: action availability, discovery targets and readiness, metrics targets, canonical metadata refresh, and contract types all assign freshly built values. The one nested writer — the PMG list prune, which cleared relay domains through the shared pointer — now clones the PMG struct before clearing, with a regression test pinning both the clone-on-write and the cache's immunity to request decoration. Read-only consumers (stats aggregation, storage filtering, namespace counting) use the shared list with no copy at all. Single-resource lookups (presentationResourceByReference and its callers) still deep-clone per lookup; they are cold paths and stay as-is. Contract-Neutral: per-generation shared resource lists: response bytes pinned by contract tests, no payload delta --- internal/api/resources.go | 102 ++++++++++++++-- internal/api/resources_k8s_namespaces.go | 16 ++- internal/api/resources_shared_cache_test.go | 123 ++++++++++++++++++++ 3 files changed, 224 insertions(+), 17 deletions(-) create mode 100644 internal/api/resources_shared_cache_test.go diff --git a/internal/api/resources.go b/internal/api/resources.go index b012080c8..3179a8ac9 100644 --- a/internal/api/resources.go +++ b/internal/api/resources.go @@ -168,13 +168,15 @@ func (h *ResourceHandlers) HandleListResources(w http.ResponseWriter, r *http.Re } orgID := GetOrgID(r.Context()) - registry, err := h.buildRegistry(orgID) + sharedResources, registry, err := h.sharedPresentationResources(orgID) if err != nil { http.Error(w, sanitizeErrorForClient(err, "Internal server error"), http.StatusInternalServerError) return } - allResources := presentationResourcesFromRegistry(registry) + // Flat copy so the per-request decorations below stay top-level writes + // on request-owned elements while nested data remains shared. + allResources := flatCopyResources(sharedResources) h.applyActionAvailability(r.Context(), allResources) resources := allResources if unsupported := unsupportedResourceTypeFilterTokens(r.URL.Query().Get("type")); len(unsupported) > 0 { @@ -217,13 +219,12 @@ func (h *ResourceHandlers) HandleStorageSummary(w http.ResponseWriter, r *http.R } orgID := GetOrgID(r.Context()) - registry, err := h.buildRegistry(orgID) + resources, _, err := h.sharedRawResources(orgID) if err != nil { http.Error(w, sanitizeErrorForClient(err, "Internal server error"), http.StatusInternalServerError) return } - resources := registry.List() filters := parseListFilters(r) storageSubjects := make([]unified.Resource, 0, len(resources)) for _, resource := range resources { @@ -248,13 +249,12 @@ func (h *ResourceHandlers) HandleStorageIncidents(w http.ResponseWriter, r *http } orgID := GetOrgID(r.Context()) - registry, err := h.buildRegistry(orgID) + resources, _, err := h.sharedRawResources(orgID) if err != nil { http.Error(w, sanitizeErrorForClient(err, "Internal server error"), http.StatusInternalServerError) return } - resources := registry.List() filters := parseListFilters(r) incidentSubjects := make([]unified.Resource, 0, len(resources)) for _, resource := range resources { @@ -285,10 +285,15 @@ func pruneResourceForListResponse(resource *unified.Resource) { } // PMG domain stats can be very large; keep summary-only in list. + // Clone before clearing: the PMG struct is reachable through the shared + // per-generation resource cache, and writing through the pointer would + // corrupt it for every other request. if resource.PMG != nil { - resource.PMG.RelayDomains = nil - resource.PMG.DomainStats = nil - resource.PMG.DomainStatsAsOf = time.Time{} + pruned := *resource.PMG + pruned.RelayDomains = nil + pruned.DomainStats = nil + pruned.DomainStatsAsOf = time.Time{} + resource.PMG = &pruned } } @@ -742,13 +747,13 @@ func (h *ResourceHandlers) HandleStats(w http.ResponseWriter, r *http.Request) { } orgID := GetOrgID(r.Context()) - registry, err := h.buildRegistry(orgID) + allResources, _, err := h.sharedPresentationResources(orgID) if err != nil { http.Error(w, sanitizeErrorForClient(err, "Internal server error"), http.StatusInternalServerError) return } - allResources := presentationResourcesFromRegistry(registry) + // Both aggregations are read-only over the shared list. stats := computeResourceContractStats(allResources) stats.PolicyPosture = resourcePolicyPostureAggregation(allResources) @@ -1435,6 +1440,81 @@ type StorageIncidentSection struct { type registryCacheEntry struct { registry *unified.ResourceRegistry lastUpdate time.Time + // rawList and presentation are lazily built once per registry generation + // and SHARED between requests. Both the slices and the nested data of + // their elements are read-only: take a flat copy (flatCopyResources) + // before any top-level field writes, and never write through nested + // pointers or into nested maps/slices of shared elements. + rawList []unified.Resource + presentation []unified.Resource +} + +// flatCopyResources returns a top-level copy of a shared resource list. +// Element values are copies, so top-level field writes and in-place +// reordering are safe; nested data stays shared with the cache, so writes +// through nested pointers remain forbidden (clone the nested struct first, +// as pruneResourceForListResponse does for PMG). +func flatCopyResources(shared []unified.Resource) []unified.Resource { + out := make([]unified.Resource, len(shared)) + copy(out, shared) + return out +} + +// sharedPresentationResources returns the org's cached presentation-shape +// resource list for the current registry generation, building it at most +// once per generation instead of deep-cloning the registry on every request. +func (h *ResourceHandlers) sharedPresentationResources(orgID string) ([]unified.Resource, *unified.ResourceRegistry, error) { + registry, err := h.buildRegistry(orgID) + if err != nil { + return nil, nil, err + } + key := cacheKey(orgID) + + h.cacheMu.Lock() + if entry, ok := h.registryCache[key]; ok && entry.registry == registry && entry.presentation != nil { + list := entry.presentation + h.cacheMu.Unlock() + return list, registry, nil + } + h.cacheMu.Unlock() + + list := presentationResourcesFromRegistry(registry) + + h.cacheMu.Lock() + if entry, ok := h.registryCache[key]; ok && entry.registry == registry { + entry.presentation = list + h.registryCache[key] = entry + } + h.cacheMu.Unlock() + return list, registry, nil +} + +// sharedRawResources is sharedPresentationResources for the raw (uncoalesced) +// registry list. +func (h *ResourceHandlers) sharedRawResources(orgID string) ([]unified.Resource, *unified.ResourceRegistry, error) { + registry, err := h.buildRegistry(orgID) + if err != nil { + return nil, nil, err + } + key := cacheKey(orgID) + + h.cacheMu.Lock() + if entry, ok := h.registryCache[key]; ok && entry.registry == registry && entry.rawList != nil { + list := entry.rawList + h.cacheMu.Unlock() + return list, registry, nil + } + h.cacheMu.Unlock() + + list := registry.List() + + h.cacheMu.Lock() + if entry, ok := h.registryCache[key]; ok && entry.registry == registry { + entry.rawList = list + h.registryCache[key] = entry + } + h.cacheMu.Unlock() + return list, registry, nil } func buildStorageSummaryResponse(resources []unified.Resource) StorageSummaryResponse { diff --git a/internal/api/resources_k8s_namespaces.go b/internal/api/resources_k8s_namespaces.go index e6730c1df..234dedee3 100644 --- a/internal/api/resources_k8s_namespaces.go +++ b/internal/api/resources_k8s_namespaces.go @@ -54,7 +54,7 @@ func (h *ResourceHandlers) HandleK8sNamespaces(w http.ResponseWriter, r *http.Re } orgID := GetOrgID(r.Context()) - registry, err := h.buildRegistry(orgID) + resources, _, err := h.sharedRawResources(orgID) if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) return @@ -111,11 +111,15 @@ func (h *ResourceHandlers) HandleK8sNamespaces(w http.ResponseWriter, r *http.Re } } - for _, pod := range registry.ListByType(unified.ResourceTypePod) { - ingest(pod, false) - } - for _, dep := range registry.ListByType(unified.ResourceTypeK8sDeployment) { - ingest(dep, true) + // Read-only aggregation over the shared per-generation list; ingest only + // counts, so no copies are needed. + for i := range resources { + switch unified.CanonicalResourceType(resources[i].Type) { + case unified.ResourceTypePod: + ingest(resources[i], false) + case unified.ResourceTypeK8sDeployment: + ingest(resources[i], true) + } } namespaces := make([]string, 0, len(byNamespace)) diff --git a/internal/api/resources_shared_cache_test.go b/internal/api/resources_shared_cache_test.go new file mode 100644 index 000000000..e08149750 --- /dev/null +++ b/internal/api/resources_shared_cache_test.go @@ -0,0 +1,123 @@ +package api + +import ( + "context" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/rcourtman/pulse-go-rewrite/internal/config" + "github.com/rcourtman/pulse-go-rewrite/internal/models" + unifiedresources "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources" +) + +func TestPruneResourceForListResponseDoesNotMutateSharedPMG(t *testing.T) { + sharedPMG := &unifiedresources.PMGData{ + InstanceID: "pmg-1", + RelayDomains: []unifiedresources.PMGRelayDomainMeta{{Domain: "example.com"}}, + DomainStatsAsOf: time.Date(2026, 8, 5, 12, 0, 0, 0, time.UTC), + } + resource := unifiedresources.Resource{ID: "pmg-resource", PMG: sharedPMG} + + pruneResourceForListResponse(&resource) + + if resource.PMG == sharedPMG { + t.Fatal("expected prune to clone the PMG struct before clearing it") + } + if resource.PMG.RelayDomains != nil || !resource.PMG.DomainStatsAsOf.IsZero() { + t.Fatal("expected the pruned copy to be cleared") + } + if len(sharedPMG.RelayDomains) != 1 || sharedPMG.DomainStatsAsOf.IsZero() { + t.Fatal("expected the shared PMG struct to remain untouched") + } +} + +func TestSharedResourceListsCachedPerGenerationAndImmuneToRequestDecoration(t *testing.T) { + now := time.Date(2026, 8, 5, 12, 0, 0, 0, time.UTC) + h := newActionTestResourceHandlers(t, &config.Config{DataPath: t.TempDir()}) + h.SetStateProvider(resourceUnifiedSeedProvider{ + snapshot: models.StateSnapshot{LastUpdate: now}, + resources: []unifiedresources.Resource{ + { + ID: "vm-1", + Type: unifiedresources.ResourceTypeVM, + Name: "worker", + Status: unifiedresources.StatusOnline, + LastSeen: now, + Sources: []unifiedresources.DataSource{unifiedresources.SourceProxmox}, + }, + { + ID: "pmg-1", + Type: unifiedresources.ResourceTypePMG, + Name: "mailgw", + Status: unifiedresources.StatusOnline, + LastSeen: now, + Sources: []unifiedresources.DataSource{unifiedresources.SourceProxmox}, + PMG: &unifiedresources.PMGData{ + InstanceID: "pmg-1", + RelayDomains: []unifiedresources.PMGRelayDomainMeta{{Domain: "example.com"}}, + }, + }, + }, + }) + + orgID := "" + first, _, err := h.sharedPresentationResources(orgID) + if err != nil { + t.Fatalf("sharedPresentationResources: %v", err) + } + if len(first) == 0 { + t.Fatal("expected seeded resources") + } + second, _, err := h.sharedPresentationResources(orgID) + if err != nil { + t.Fatalf("sharedPresentationResources: %v", err) + } + if &first[0] != &second[0] { + t.Fatal("expected the cached shared list for an unchanged registry generation") + } + + // A full list request prunes PMG detail in its response; the shared cache + // must keep the untouched data for later consumers. + req := httptest.NewRequest(http.MethodGet, "/api/resources", nil) + req = req.WithContext(context.WithValue(req.Context(), OrgIDContextKey, orgID)) + rec := httptest.NewRecorder() + h.HandleListResources(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, body=%s", rec.Code, rec.Body.String()) + } + + after, _, err := h.sharedPresentationResources(orgID) + if err != nil { + t.Fatalf("sharedPresentationResources: %v", err) + } + if &first[0] != &after[0] { + t.Fatal("expected the request to serve from the cached list, not rebuild it") + } + foundPMG := false + for i := range after { + if after[i].PMG == nil { + continue + } + foundPMG = true + if len(after[i].PMG.RelayDomains) != 1 { + t.Fatal("expected the shared cache to retain PMG relay domains after a list request pruned its response copy") + } + } + if !foundPMG { + t.Fatal("expected a PMG-bearing resource in the shared list") + } + + rawFirst, _, err := h.sharedRawResources(orgID) + if err != nil { + t.Fatalf("sharedRawResources: %v", err) + } + rawSecond, _, err := h.sharedRawResources(orgID) + if err != nil { + t.Fatalf("sharedRawResources: %v", err) + } + if len(rawFirst) == 0 || &rawFirst[0] != &rawSecond[0] { + t.Fatal("expected the cached shared raw list for an unchanged registry generation") + } +}