MemberJunction / MemberJunction/MJ

Fail-fast guard for the shared metadata graph: freeze Info objects (at least in dev) after Config

Open
#3,095 0 comments 0 reactions 0 assignees View on GitHub
enhancement
Dominant language
TSQL
Stars
29
Forks
6
Avg merge
2d 1h
Merged PRs (30d)
323

Description

## Context

Since #3083 / PR #3087, providers configured with `ignoreExistingMetadata: false` (MJServer's per-request RW/RO providers) share the global provider's metadata **Info object instances** by reference instead of deep-cloning the graph. The shell gives each provider its own top-level array containers (so `provider.Entities.sort(...)` etc. stays request-local), but everything below that level is shared process-wide:

- property writes on shared Info objects (`EntityInfo`, `EntityFieldInfo`, `ApplicationInfo`, ...)
- in-place mutation of their **nested arrays** (`entity.Fields`, `entity.RelatedEntities`, `application.ApplicationEntities`, ...)

The contract is "treat Info objects and everything they own as read-only" — but today that contract is **convention-only**. A repo-wide sweep during #3087 review found zero violators (the one in-place `provider.Entities` sort was fixed in that PR), but nothing stops a future MJ change or a downstream app's custom resolver from introducing one. The failure mode is silent cross-request/process-wide state corruption — exactly the class of bug that's cheap to prevent and expensive to debug.

## Proposal

Add a fail-fast guard so an illegal write throws at the mutation site instead of corrupting shared state:

1. **`Object.freeze` (deep) the metadata graph after the global provider's `Config()`/`Refresh()` completes** — the Info objects and their nested arrays, not just the top-level containers. Refreshes swap the whole `AllMetadata` object via `UpdateLocalMetadata`, so freezing the finished graph doesn't interfere with refresh flows.
2. **Consider dev-mode-only first.** Freezing changes observable behavior for any existing (out-of-repo) code that mutates Info objects on a client's global provider — such writes currently "work" (process-visible but tolerated). Gating the freeze behind an env flag / non-production default gets the bug-catching benefit without a compat cliff; promoting to always-on can be a later decision.

## Open questions

- **Lazy caches:** several Info classes memoize derived values post-construction (deterministic, provider/user-independent). A blanket deep freeze breaks those. Options: compute them eagerly before freezing, back them with a non-frozen side map (`WeakMap`), or exclude those specific slots from the freeze.
- **Perf:** deep-freezing a 600-entity graph is a one-time cost at Config/Refresh (not per-request) — should be measured but is expected to be negligible relative to the metadata load itself.
- **Scope:** the freeze should apply to the graph itself (shared by every shell), so it protects the client global-provider case too, not just server per-request providers.

## References

- #3083 (the per-request deep-clone tax), PR #3087 (the shared-shell fix — see its description's "one behavioral contract" section, where this guard was explicitly proposed as a deliberate follow-up rather than being bundled into the perf fix)
- `CreateSharedMetadataShell` / `CopyMetadataFromGlobalProvider` in `packages/MJCore/src/generic/providerBase.ts`
- The one real-world violator found and fixed: `entity-selector-with-grid.component.ts` in-place sort of `provider.Entities`

Contributor guide

Open the contributing guide

Research direction

Start in packages/MJCore/src/generic/providerBase.ts at CreateSharedMetadataShell and CopyMetadataFromGlobalProvider, then trace the global provider's Config()/Refresh() flow through UpdateLocalMetadata. Review the Info classes for lazy caches and determine how the proposed dev-mode deep freeze can protect the shared graph without breaking refreshes; done means the chosen behavior and compatibility scope are settled and illegal writes fail at their mutation site.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
backend
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Needs clarification
Newbie friendliness
32/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.