MemberJunction / MemberJunction/MJ

Performance: entity and field permission aggregation is recomputed on every CRUD event (no memoization)

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

Description

## Summary

Neither entity-level nor field-level permission aggregation is memoized. Both recompute from scratch on every call, and both sit on the CRUD hot path. The only caches on `EntityInfo` / `EntityFieldInfo` today are structural (`_primaryKeysCache`, `_nameFieldCache`, `_foreignKeysCache`, …) — nothing caches a permission result.

Filing as a standalone item for a general memoization / speed pass rather than folding it into a feature PR.

## What recomputes

**`EntityInfo.GetUserPermisions(user)`** — `packages/MJCore/src/generic/entityInfo.ts:3565`

Walks `this.Permissions`, doing `user.UserRoles?.find(...)` per permission row, then folds Allow/Deny buckets. Runs on **every** entity permission check, for **every** entity, with no feature flag gating it.

**`EntityInfo.getDeniedFields(user, predicate)`** — `entityInfo.ts:3053`, behind `GetDeniedReadFields` / `GetDeniedUpdateFields` / `GetDeniedCreateFields`

Walks every field on the entity and calls `EntityFieldInfo.GetUserFieldPermissions` per field, which calls `AggregateFieldRulesForUser` (`entityInfo.ts:1495`), which does `user?.UserRoles?.find(...)` per rule.

So one call is roughly `O(fields × rulesPerField × userRoles)`. On an entity with ~100 fields, ~10 role rows per field and a user holding ~5 roles, that is on the order of 5,000 UUID comparisons per call — and a single request makes several.

## Where it is called from

`GetDeniedReadFields` alone has ~20 call sites across MJCore, MJServer, GenericDatabaseProvider, GraphQLDataProvider and Angular. On the hot path:

- `ProviderBase.ApplyFieldSecurityProjection` — every RunView result set
- `ProviderBase.AssertPredicatesRespectFieldSecurity` — every RunView with a predicate
- `GenericDatabaseProvider.getRunTimeViewFieldArray` / `getRunTimeViewFieldString` — SELECT narrowing per query
- `BaseEntity.CheckFieldLevelUpdatePermissions` + `ApplyFieldLevelCreateSuppression` — every save
- `BaseEntity` `Get()` / `Set()` gate
- `RecordChangeFieldSecurityProjector` — per Record Changes row, resolving a different entity each time
- `ResolverBase` — twice per update mutation

## Scope, stated honestly

`getDeniedFields` short-circuits on `EnableFieldLevelSecurity`, so the field-level cost lands **only on entities that have opted into FLS**. That flag is off by default, so this is not a live regression today — it becomes one as FLS adoption grows.

`GetUserPermisions` has no such gate and runs everywhere. That is the broader of the two.

## Why it is not a one-liner

The result depends on `(entity, user's role set, permission rows)`, and all three can change at runtime:

- permission rows change via `EntityPermission` / `EntityFieldPermission` saves, and via FLS reconciliation
- a user's roles change via `UserRole` saves
- metadata refreshes replace `EntityInfo` instances wholesale

So a cache needs an invalidation story, not just a `Map`. Options worth weighing: cache on `EntityInfo` keyed by a user-roles fingerprint and drop it on metadata refresh; or hang it off the request/session; or precompute per user at bootstrap the way `UserCache` does. Whatever is chosen has to fail closed — a stale *grant* is a security bug, a stale *denial* is only an annoyance.

## Related

- #3794 — memoize the order-by parse in materialized RunQuery (same flavour, different path)
- #2554 — BaseEngine lazy-load heavy columns with an LRU cache
- #4298 — `ProviderBase` record-name cache is not user-scoped (adjacent: caching + per-user correctness)

Surfaced while reviewing field-level security (#3367); no fix attempted there.

Contributor guide

Open the contributing guide

Research direction

Start with EntityInfo.GetUserPermisions at packages/MJCore/src/generic/entityInfo.ts:3565, getDeniedFields around line 3053, and AggregateFieldRulesForUser around line 1495. Trace the listed permission and role-save paths to define cache invalidation, then verify that repeated CRUD checks avoid recomputation without allowing stale grants; inspect the related call sites and security behavior when testing.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
backend-api-design, performance, security
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Needs clarification
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.