diff --git a/packages/agent-bff/src/build-bff.ts b/packages/agent-bff/src/build-bff.ts index 3a977c2ec9..a0650f290a 100644 --- a/packages/agent-bff/src/build-bff.ts +++ b/packages/agent-bff/src/build-bff.ts @@ -68,6 +68,11 @@ export interface BuildBffOptions { export interface Bff { callback: BffCallback; + /** + * Forget what was read from the SaaS. A host that knows the schema just moved — an agent + * restarting on a customization refresh — calls this instead of waiting out the 24h TTL. + */ + invalidate(): void; } const SESSION_TTL_SECONDS = 24 * 60 * 60; @@ -318,6 +323,7 @@ function buildAgentRouteMiddlewares( bundle: ReadModelBundle | undefined, config: BFFConfig, logger: Logger, + permissionsCache: PermissionsCache, ): Middleware[] { if (!bundle) { logger( @@ -337,7 +343,7 @@ function buildAgentRouteMiddlewares( forestServerUrl: apiKeyConfig.forestServerUrl, envSecret: apiKeyConfig.forestEnvSecret, }), - cache: new PermissionsCache(), + cache: permissionsCache, logger, }); @@ -381,19 +387,24 @@ function buildAiMiddlewares(config: BFFConfig, oauth: OAuthEdge, logger: Logger) ]; } +interface AgentEdge { + middlewares: Middleware[]; + invalidate(): void; +} + function buildAgentMiddlewares( config: BFFConfig, logger: Logger, oauth: OAuthEdge, aiMiddlewares: Middleware[], basePath: string, -): Middleware[] { +): AgentEdge { const { forestAuthSecret, defaultTimezone } = config; if (!forestAuthSecret) { logger('Warn', 'Agent edge disabled: FOREST_AUTH_SECRET is missing'); - return []; + return { middlewares: [], invalidate: () => undefined }; } const apiKeyStep = buildApiKeyMiddleware(config, logger) ?? createApiKeyUnavailableGuard(logger); @@ -401,6 +412,7 @@ function buildAgentMiddlewares( // AGENT_URL every data path answers 501, so concrete paths would advertise a dead surface. const bundle = resolveReadModelBundle(config, logger); const source = toUnfoldSource(bundle, config, logger); + const permissionsCache = new PermissionsCache(); const chain: Middleware[] = [ createAuthModeMiddleware({ authSecret: forestAuthSecret }), @@ -430,10 +442,19 @@ function buildAgentMiddlewares( : []), ...aiMiddlewares, createTimezoneMiddleware({ defaultTimezone }), - ...buildAgentRouteMiddlewares(bundle, config, logger), + ...buildAgentRouteMiddlewares(bundle, config, logger, permissionsCache), ]; - return chain.map(agentScoped); + return { + middlewares: chain.map(agentScoped), + invalidate: () => { + // The one trace an invalidation leaves: the operator asked "the schema is wrong" needs to + // establish that one happened and when, and the age gauge cannot say it. + logger('Info', 'Dropping the SaaS read caches on host request'); + bundle?.store.invalidate(); + permissionsCache.clear(); + }, + }; } /** @@ -462,7 +483,8 @@ export default async function buildBff({ const oauth = buildOAuthMiddlewares(config, logger); const aiMiddlewares = buildAiMiddlewares(config, oauth, logger); - const agentMiddlewares = buildAgentMiddlewares(config, logger, oauth, aiMiddlewares, mountPath); + const agentEdge = buildAgentMiddlewares(config, logger, oauth, aiMiddlewares, mountPath); + const agentMiddlewares = agentEdge.middlewares; const hasAgentEdge = agentMiddlewares.length > 0; const agentErrorMiddleware = hasAgentEdge ? [agentScoped(createErrorMiddleware({ logger }))] : []; const agentJsonOnlyGuard = hasAgentEdge ? [agentScoped(createJsonOnlyGuard())] : []; @@ -490,5 +512,5 @@ export default async function buildBff({ const app = new Koa(); for (const middleware of middlewares) app.use(middleware); - return { callback: app.callback() }; + return { callback: app.callback(), invalidate: agentEdge.invalidate }; } diff --git a/packages/agent-bff/src/permissions/permissions-cache.ts b/packages/agent-bff/src/permissions/permissions-cache.ts index 587ae8cad3..6dda9be477 100644 --- a/packages/agent-bff/src/permissions/permissions-cache.ts +++ b/packages/agent-bff/src/permissions/permissions-cache.ts @@ -31,6 +31,7 @@ export default class PermissionsCache { private readonly ttlMs: number; private entry: CacheEntry | undefined; + private generationValue = 0; constructor({ now = Date.now, ttlMs = PERMISSIONS_CACHE_TTL_MS }: PermissionsCacheOptions = {}) { this.now = now; @@ -49,7 +50,22 @@ export default class PermissionsCache { return this.getFreshEntry()?.permissions; } - set(permissions: EvaluatedPermissions): void { + /** + * The generation a fetch must be started against. Read it before the fetch and hand it back to + * `set`, so a response that crossed a `clear()` cannot become the shared entry. + */ + get generation(): number { + return this.generationValue; + } + + /** + * Store a fetched payload, unless a `clear()` landed while the fetch was in flight: it read the + * permissions the invalidation declared stale, and with a single shared entry one late write would + * hand a revoked access to every caller for a full TTL. + */ + set(permissions: EvaluatedPermissions, generation: number): void { + if (generation !== this.generationValue) return; + const previous = this.getFreshEntry(); const carriesTheSameUsers = previous !== undefined && sameUsers(previous.permissions.users, permissions.users); @@ -67,6 +83,7 @@ export default class PermissionsCache { clear(): void { this.entry = undefined; + this.generationValue += 1; } private getFreshEntry(): CacheEntry | undefined { diff --git a/packages/agent-bff/src/permissions/permissions-routes-middleware.ts b/packages/agent-bff/src/permissions/permissions-routes-middleware.ts index 4c0b1b778b..c0c6e067ed 100644 --- a/packages/agent-bff/src/permissions/permissions-routes-middleware.ts +++ b/packages/agent-bff/src/permissions/permissions-routes-middleware.ts @@ -70,6 +70,7 @@ async function resolvePermissions({ logger: Logger; }): Promise<{ permissions: EvaluatedPermissions; fromFreshFetch: boolean }> { let resolved: EnvironmentAndUserPermissions; + const { generation } = cache; try { resolved = await client.fetchPermissions(); @@ -90,7 +91,7 @@ async function resolvePermissions({ users: resolved.users, }; - cache.set(permissions); + cache.set(permissions, generation); return { permissions, fromFreshFetch: true }; } diff --git a/packages/agent-bff/src/read-model/create-read-model.ts b/packages/agent-bff/src/read-model/create-read-model.ts index 45c3aec4c4..197795653c 100644 --- a/packages/agent-bff/src/read-model/create-read-model.ts +++ b/packages/agent-bff/src/read-model/create-read-model.ts @@ -32,7 +32,7 @@ export default function createReadModel({ // Wrap once so a throwing metrics backend can never break business logic anywhere in the bundle. const resolvedMetrics = safeMetrics(metrics ?? createConsoleMetrics(logger)); const fetcher = new ForestSchemaClient({ forestServerUrl, envSecret }); - const schemaCache = new SchemaCache({ fetcher, metrics: resolvedMetrics, now }); + const schemaCache = new SchemaCache({ fetcher, metrics: resolvedMetrics, logger, now }); const capabilitiesCache = new CapabilitiesCache({ now }); const store = new ReadModelStore(schemaCache, capabilitiesCache); const actionEndpointResolver = new ActionEndpointResolver( diff --git a/packages/agent-bff/src/read-model/read-model-store.ts b/packages/agent-bff/src/read-model/read-model-store.ts index 3f3b677d53..e3814508b0 100644 --- a/packages/agent-bff/src/read-model/read-model-store.ts +++ b/packages/agent-bff/src/read-model/read-model-store.ts @@ -34,6 +34,18 @@ export default class ReadModelStore { return (await this.getSchemaSnapshot()).readModel; } + /** + * Forget the cached schema and the capabilities that belong to it, then force the read-model to be + * rebuilt. The capabilities are dropped here rather than left to the next revision change: a + * `clear()` also drops a schema write that was in flight, and a dropped write moves nothing the + * snapshot can notice, so inferring the invalidation from the revision would miss it. + */ + invalidate(): void { + this.schemaCache.clear(); + this.capabilitiesCache.clear(); + this.builtRevision = -1; + } + async getSchemaSnapshot(): Promise { const collections = await this.schemaCache.get(); const { revision } = this.schemaCache; diff --git a/packages/agent-bff/src/read-model/schema-cache.ts b/packages/agent-bff/src/read-model/schema-cache.ts index 3b8351e297..52aa351ee7 100644 --- a/packages/agent-bff/src/read-model/schema-cache.ts +++ b/packages/agent-bff/src/read-model/schema-cache.ts @@ -1,4 +1,5 @@ import type { SchemaFetcher } from './forest-schema-client'; +import type { Logger } from '../ports/logger-port'; import type { Metrics } from '../ports/metrics-port'; import type { ForestSchemaCollection } from '@forestadmin/forestadmin-client'; @@ -6,12 +7,23 @@ import SchemaUnavailableError from './errors'; export const ONE_DAY_MS = 24 * 60 * 60 * 1000; +/** + * How long after a `clear()` the cache keeps re-reading the schema, and how often. An invalidation + * says "the agent just changed its schema", but the SaaS the BFF reads from may not have finished + * recording it: caching whatever comes back on the very next read would pin the old schema for a + * full day. Within this window a read costs one request every few seconds, which is nothing next to + * serving a collection the agent exposes and the BFF 404s. + */ +export const REVALIDATION_WINDOW_MS = 60 * 1000; +export const REVALIDATION_TTL_MS = 5 * 1000; + export const SCHEMA_CACHE_REFRESH_ERROR = 'schema_cache_refresh_error'; export const SCHEMA_CACHE_AGE_SECONDS = 'schema_cache_age_seconds'; export interface SchemaCacheOptions { fetcher: SchemaFetcher; metrics: Metrics; + logger?: Logger; now?: () => number; ttlMs?: number; } @@ -19,6 +31,12 @@ export interface SchemaCacheOptions { interface CacheEntry { collections: ForestSchemaCollection[]; fetchedAt: number; + /** + * Decided when the entry is written, not when it is read: a read taken while the revalidation + * window was open cannot be trusted for a day, and must not be promoted to a long-lived entry the + * moment the window closes. + */ + expiresAt: number; } /** @@ -30,22 +48,32 @@ interface CacheEntry { export default class SchemaCache { private readonly fetcher: SchemaFetcher; private readonly metrics: Metrics; + private readonly logger: Logger; private readonly now: () => number; private readonly ttlMs: number; private entry: CacheEntry | null = null; private inFlight: Promise | null = null; private revisionValue = 0; - - constructor({ fetcher, metrics, now = Date.now, ttlMs = ONE_DAY_MS }: SchemaCacheOptions) { + private generation = 0; + private revalidatingUntil = 0; + + constructor({ + fetcher, + metrics, + logger = () => undefined, + now = Date.now, + ttlMs = ONE_DAY_MS, + }: SchemaCacheOptions) { this.fetcher = fetcher; this.metrics = metrics; + this.logger = logger; this.now = now; this.ttlMs = ttlMs; } async get(): Promise { - if (this.entry && this.now() - this.entry.fetchedAt < this.ttlMs) { + if (this.entry && this.now() < this.entry.expiresAt) { this.emitAge(); return this.entry.collections; @@ -54,6 +82,23 @@ export default class SchemaCache { return this.refresh(); } + /** + * Expire what is cached and re-read it eagerly for a while. Called when something outside knows + * the schema moved — an agent restarting on a customization refresh, say. + * + * The entry is expired, not dropped: every read now goes through a refresh, so nothing the host + * declared stale is served while the SaaS answers, but the class keeps its safety net — a refresh + * that fails still has a last good schema to fall back on. Dropping it would turn a restart + * during a SaaS blip into a `503 schema_unavailable` on every data route. + */ + clear(): void { + this.generation += 1; + this.inFlight = null; + this.revalidatingUntil = this.now() + REVALIDATION_WINDOW_MS; + + if (this.entry) this.entry = { ...this.entry, expiresAt: this.now() }; + } + ageSeconds(): number | undefined { if (!this.entry) return undefined; @@ -66,15 +111,21 @@ export default class SchemaCache { private async refresh(): Promise { if (!this.inFlight) { - this.inFlight = this.doRefresh().finally(() => { - this.inFlight = null; + // Identity-guarded, because `clear()` detaches the in-flight fetch: a read that lands after an + // invalidation must start its own, not join the one that read the invalidated schema. + const pending: Promise = this.doRefresh().finally(() => { + if (this.inFlight === pending) this.inFlight = null; }); + + this.inFlight = pending; } return this.inFlight; } private async doRefresh(): Promise { + const { generation } = this; + try { const collections = await this.fetcher.fetchSchema(); @@ -83,16 +134,29 @@ export default class SchemaCache { // failed fetch and fall through to the failure path below. if (collections.length === 0) throw new Error('Forest returned an empty schema'); - this.entry = { collections, fetchedAt: this.now() }; - this.revisionValue += 1; - this.emitAge(); + // Skip the write if a clear() happened while this fetch was in flight: it read the schema the + // invalidation declared stale, and caching it now would undo the invalidation. + if (this.generation === generation) { + const fetchedAt = this.now(); + + this.entry = { collections, fetchedAt, expiresAt: fetchedAt + this.ttlFor(fetchedAt) }; + this.revisionValue += 1; + this.emitAge(); + } return collections; } catch (error) { this.metrics.increment(SCHEMA_CACHE_REFRESH_ERROR); + // The counter alone cannot tell "the SaaS returned an empty array" from "the SaaS is down" + // from "the env secret is wrong", and the cause is otherwise swallowed: it becomes the `cause` + // of a `SchemaUnavailableError` the error middleware serialises without logging. + this.logger('Warn', 'Schema refresh failed', { + cause: error instanceof Error ? `${error.name}: ${error.message}` : String(error), + servedStale: this.entry !== null, + }); // Warm cache: keep serving the last good schema (stale), do not poison — the next read - // re-attempts because `fetchedAt` is unchanged and the entry stays expired. + // re-attempts because `expiresAt` is unchanged and the entry stays expired. if (this.entry) { this.emitAge(); @@ -104,6 +168,10 @@ export default class SchemaCache { } } + private ttlFor(fetchedAt: number): number { + return fetchedAt < this.revalidatingUntil ? REVALIDATION_TTL_MS : this.ttlMs; + } + private emitAge(): void { const age = this.ageSeconds(); if (age !== undefined) this.metrics.gauge(SCHEMA_CACHE_AGE_SECONDS, age); diff --git a/packages/agent-bff/test/build-bff-invalidate.test.ts b/packages/agent-bff/test/build-bff-invalidate.test.ts new file mode 100644 index 0000000000..03f044f7a0 --- /dev/null +++ b/packages/agent-bff/test/build-bff-invalidate.test.ts @@ -0,0 +1,92 @@ +import type { EvaluatedPermissions } from '../src/permissions/permissions-cache'; +import type { PermissionsRoutesMiddlewareOptions } from '../src/permissions/permissions-routes-middleware'; +import type { Logger } from '../src/ports/logger-port'; + +import buildBff from '../src/build-bff'; +import { restoreFetchAfterEach, stubEnvironmentIdFetch } from './helpers/fetch-stub'; +import { parseConfig } from '../src/config/env-config'; +import createPermissionsRoutesMiddleware from '../src/permissions/permissions-routes-middleware'; + +jest.mock('../src/permissions/permissions-routes-middleware', () => ({ + __esModule: true, + default: jest.fn(() => async (_ctx: unknown, next: () => Promise) => next()), +})); + +const VALID_ENV = { + FOREST_AUTH_SECRET: 'auth-secret', + FOREST_ENV_SECRET: 'env-secret', + FOREST_SERVER_URL: 'https://api.forestadmin.com', + FOREST_APP_URL: 'https://app.forestadmin.com', + AGENT_URL: 'https://agent.example.com', + BFF_TOKEN_ENCRYPTION_KEY: Buffer.alloc(32).toString('base64'), +} satisfies NodeJS.ProcessEnv; + +const PERMISSIONS = { + actionPermissions: { + isDevelopment: false, + actionsGloballyAllowed: new Set(), + actionsByRole: new Map(), + }, + users: [{ id: 1, roleId: 7 }], +} as unknown as EvaluatedPermissions; + +const noopLogger: Logger = () => undefined; + +const mounted = createPermissionsRoutesMiddleware as unknown as jest.Mock; + +function permissionsRouteOptions(): PermissionsRoutesMiddlewareOptions { + return mounted.mock.calls[0][0] as PermissionsRoutesMiddlewareOptions; +} + +describe('buildBff invalidate', () => { + restoreFetchAfterEach(); + + beforeEach(() => { + mounted.mockClear(); + stubEnvironmentIdFetch(); + }); + + describe('when the agent edge is mounted', () => { + it('should drop the entry from the very cache the permissions route reads', async () => { + const bff = await buildBff({ config: parseConfig(VALID_ENV), logger: noopLogger }); + const { cache } = permissionsRouteOptions(); + cache.set(PERMISSIONS, cache.generation); + + bff.invalidate(); + + expect(cache.getFresh()).toBeUndefined(); + }); + + it('should invalidate the very store the permissions route reads', async () => { + const bff = await buildBff({ config: parseConfig(VALID_ENV), logger: noopLogger }); + const { store } = permissionsRouteOptions(); + const invalidate = jest.spyOn(store, 'invalidate'); + + bff.invalidate(); + + expect(invalidate).toHaveBeenCalledTimes(1); + }); + + it('should log the invalidation, the only trace an operator can correlate against', async () => { + const logger = jest.fn(); + const bff = await buildBff({ config: parseConfig(VALID_ENV), logger }); + logger.mockClear(); + + bff.invalidate(); + + expect(logger).toHaveBeenCalledWith('Info', 'Dropping the SaaS read caches on host request'); + }); + }); + + describe('when the read-model bundle is absent but the auth secret is present', () => { + it('should mount no permissions route and stay safe to invalidate', async () => { + const bff = await buildBff({ + config: parseConfig({ ...VALID_ENV, FOREST_ENV_SECRET: undefined }), + logger: noopLogger, + }); + + expect(() => bff.invalidate()).not.toThrow(); + expect(mounted).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/packages/agent-bff/test/build-bff.test.ts b/packages/agent-bff/test/build-bff.test.ts index 5ac7d15cad..5d34b1ca7b 100644 --- a/packages/agent-bff/test/build-bff.test.ts +++ b/packages/agent-bff/test/build-bff.test.ts @@ -175,6 +175,17 @@ describe('buildBff', () => { }); }); + describe('invalidate', () => { + it('should be safe on a deployment whose agent edge is not mounted', async () => { + const bff = await buildBff({ + config: parseConfig({ ...VALID_ENV, FOREST_AUTH_SECRET: undefined }), + logger: noopLogger, + }); + + expect(() => bff.invalidate()).not.toThrow(); + }); + }); + describe('when BFF_ALLOWED_ORIGINS carries a malformed entry', () => { it('should warn once with the rejected entries', async () => { const logger = jest.fn(); diff --git a/packages/agent-bff/test/permissions/permissions-cache.test.ts b/packages/agent-bff/test/permissions/permissions-cache.test.ts index 14ee5953fe..2c51d8cac9 100644 --- a/packages/agent-bff/test/permissions/permissions-cache.test.ts +++ b/packages/agent-bff/test/permissions/permissions-cache.test.ts @@ -25,7 +25,7 @@ describe('PermissionsCache', () => { let clock = 1_000; const cache = new PermissionsCache({ now: () => clock }); - cache.set(PERMISSIONS); + cache.set(PERMISSIONS, cache.generation); clock += PERMISSIONS_CACHE_TTL_MS - 1; expect(cache.getFresh()).toBe(PERMISSIONS); @@ -37,7 +37,7 @@ describe('PermissionsCache', () => { let clock = 1_000; const cache = new PermissionsCache({ now: () => clock }); - cache.set(PERMISSIONS); + cache.set(PERMISSIONS, cache.generation); clock += PERMISSIONS_CACHE_TTL_MS; expect(cache.getFresh()).toBeUndefined(); @@ -49,8 +49,8 @@ describe('PermissionsCache', () => { const cache = new PermissionsCache(); const latest = { ...PERMISSIONS }; - cache.set(PERMISSIONS); - cache.set(latest); + cache.set(PERMISSIONS, cache.generation); + cache.set(latest, cache.generation); expect(cache.size).toBe(1); expect(cache.getFresh()).toBe(latest); @@ -61,11 +61,32 @@ describe('PermissionsCache', () => { it('should drop the stored entry', () => { const cache = new PermissionsCache(); - cache.set(PERMISSIONS); + cache.set(PERMISSIONS, cache.generation); cache.clear(); expect(cache.getFresh()).toBeUndefined(); expect(cache.size).toBe(0); }); }); + + describe('when a fetch started before a clear tries to store its result', () => { + it('should refuse the write, since it read the permissions the clear declared stale', () => { + const cache = new PermissionsCache(); + const { generation } = cache; + + cache.clear(); + cache.set(PERMISSIONS, generation); + + expect(cache.getFresh()).toBeUndefined(); + }); + + it('should accept a fetch started after that clear', () => { + const cache = new PermissionsCache(); + + cache.clear(); + cache.set(PERMISSIONS, cache.generation); + + expect(cache.getFresh()).toBe(PERMISSIONS); + }); + }); }); diff --git a/packages/agent-bff/test/permissions/permissions-routes-middleware.test.ts b/packages/agent-bff/test/permissions/permissions-routes-middleware.test.ts index 2a43d4d28a..44831ee50f 100644 --- a/packages/agent-bff/test/permissions/permissions-routes-middleware.test.ts +++ b/packages/agent-bff/test/permissions/permissions-routes-middleware.test.ts @@ -82,6 +82,13 @@ function fetcherOf( }; } +function seed(cache: PermissionsCache, users: UserPermissionV4[]): void { + cache.set( + { actionPermissions: generateActionsFromPermissions(NORMAL_MODE), users }, + cache.generation, + ); +} + function principalMiddleware(id: number | null): Middleware { return async (ctx, next) => { if (id !== null) { @@ -203,10 +210,7 @@ describe('createPermissionsRoutesMiddleware', () => { describe('when a caller absent from the cached users payload calls in', () => { it('should refetch once and serve them rather than returning a stale 403', async () => { const cache = new PermissionsCache(); - cache.set({ - actionPermissions: generateActionsFromPermissions(NORMAL_MODE), - users: [{ id: CALLER_ID, roleId: ADMIN_ROLE } as UserPermissionV4], - }); + seed(cache, [{ id: CALLER_ID, roleId: ADMIN_ROLE } as UserPermissionV4]); const client = fetcherOf({ environmentPermissions: NORMAL_MODE, users: USERS }); const response = await request(appOf({ client, cache, callerId: VIEWER_ID }).callback()) @@ -239,10 +243,7 @@ describe('createPermissionsRoutesMiddleware', () => { const app = appOf({ client, cache, callerId: VIEWER_ID }); await request(app.callback()).get(ROUTE); - cache.set({ - actionPermissions: generateActionsFromPermissions(NORMAL_MODE), - users: USERS, - }); + seed(cache, USERS); const afterRefresh = await request(app.callback()).get(ROUTE); expect(afterRefresh.status).toBe(200); @@ -293,10 +294,7 @@ describe('createPermissionsRoutesMiddleware', () => { describe('when the SaaS is down and the stale cache does not cover the caller', () => { it('should keep refetching rather than recording a rejection it never confirmed', async () => { const cache = new PermissionsCache(); - cache.set({ - actionPermissions: generateActionsFromPermissions(NORMAL_MODE), - users: [{ id: CALLER_ID, roleId: ADMIN_ROLE } as UserPermissionV4], - }); + seed(cache, [{ id: CALLER_ID, roleId: ADMIN_ROLE } as UserPermissionV4]); const client = fetcherOf(new Error('SaaS down')); const app = appOf({ client, cache, callerId: VIEWER_ID }); @@ -311,10 +309,7 @@ describe('createPermissionsRoutesMiddleware', () => { describe('when the refetch triggered by an unknown caller fails', () => { it('should keep serving the cached permissions to the callers they cover', async () => { const cache = new PermissionsCache(); - cache.set({ - actionPermissions: generateActionsFromPermissions(NORMAL_MODE), - users: [{ id: CALLER_ID, roleId: ADMIN_ROLE } as UserPermissionV4], - }); + seed(cache, [{ id: CALLER_ID, roleId: ADMIN_ROLE } as UserPermissionV4]); const client = fetcherOf(new Error('SaaS down')); const unknown = await request(appOf({ client, cache, callerId: VIEWER_ID }).callback()) @@ -415,10 +410,7 @@ describe('createPermissionsRoutesMiddleware', () => { const cache = new PermissionsCache(); const client: PermissionsFetcher = { fetchPermissions: jest.fn(async () => { - cache.set({ - actionPermissions: generateActionsFromPermissions(NORMAL_MODE), - users: USERS, - }); + seed(cache, USERS); throw new Error('SaaS down'); }), @@ -556,7 +548,7 @@ describe('createPermissionsRoutesMiddleware', () => { }); describe('when the schema is refreshed while the permissions are being fetched', () => { - it('should answer from the refreshed read model, not the superseded one', async () => { + function invalidatedMidFetch() { const cache = new PermissionsCache(); const refreshed = new ReadModel([ collection('orders', [column('id')], [action('Refund order', '/refund')]), @@ -574,11 +566,24 @@ describe('createPermissionsRoutesMiddleware', () => { }), }; + return { cache, client, store }; + } + + it('should answer from the refreshed read model, not the superseded one', async () => { + const { cache, client, store } = invalidatedMidFetch(); + const response = await request(appOf({ client, cache, store }).callback()).get(ROUTE); expect(response.status).toBe(200); expect(Object.keys(response.body.collections)).toEqual(['orders']); - expect(cache.size).toBe(1); + }); + + it('should not repopulate the shared cache with what that fetch read', async () => { + const { cache, client, store } = invalidatedMidFetch(); + + await request(appOf({ client, cache, store }).callback()).get(ROUTE); + + expect(cache.size).toBe(0); }); }); diff --git a/packages/agent-bff/test/read-model/read-model-store.test.ts b/packages/agent-bff/test/read-model/read-model-store.test.ts index 5fa5b90d5f..4b3da75769 100644 --- a/packages/agent-bff/test/read-model/read-model-store.test.ts +++ b/packages/agent-bff/test/read-model/read-model-store.test.ts @@ -274,4 +274,46 @@ describe('ReadModelStore', () => { expect(store.ageSeconds()).toBe(7); }); }); + + describe('invalidate', () => { + it('should re-read the schema on the next snapshot', async () => { + const fetchSchema = jest.fn().mockResolvedValue(makeSchema('users')); + const store = build(fetchSchema); + await store.getSchemaSnapshot(); + + store.invalidate(); + await store.getSchemaSnapshot(); + + expect(fetchSchema).toHaveBeenCalledTimes(2); + }); + + it('should drop the capabilities with it, since they belong to the schema generation', async () => { + const fetchSchema = jest.fn().mockResolvedValue(makeSchema('users')); + const store = build(fetchSchema); + const capabilities = jest.fn().mockResolvedValue({ fields: [] }); + await store.getCapabilities('users', capabilities); + + store.invalidate(); + await store.getCapabilities('users', capabilities); + + expect(capabilities).toHaveBeenCalledTimes(2); + }); + + it('should drop the capabilities even when the revision does not move', async () => { + const fetchSchema = jest + .fn() + .mockResolvedValueOnce(makeSchema('users')) + .mockRejectedValue(new Error('boom')); + const store = build(fetchSchema); + const capabilities = jest.fn().mockResolvedValue({ fields: [] }); + await store.getCapabilities('users', capabilities); + const before = await store.getSchemaSnapshot(); + + store.invalidate(); + await store.getCapabilities('users', capabilities); + + expect((await store.getSchemaSnapshot()).revision).toBe(before.revision); + expect(capabilities).toHaveBeenCalledTimes(2); + }); + }); }); diff --git a/packages/agent-bff/test/read-model/schema-cache.test.ts b/packages/agent-bff/test/read-model/schema-cache.test.ts index b0cfc1fe7a..ca8b900217 100644 --- a/packages/agent-bff/test/read-model/schema-cache.test.ts +++ b/packages/agent-bff/test/read-model/schema-cache.test.ts @@ -1,3 +1,4 @@ +import type { Logger } from '../../src/ports/logger-port'; import type { Metrics } from '../../src/ports/metrics-port'; import type { SchemaFetcher } from '../../src/read-model/forest-schema-client'; import type { ForestSchemaCollection } from '@forestadmin/forestadmin-client'; @@ -6,6 +7,8 @@ import { makeMetrics, makeSchema } from './fixtures'; import SchemaUnavailableError from '../../src/read-model/errors'; import SchemaCache, { ONE_DAY_MS, + REVALIDATION_TTL_MS, + REVALIDATION_WINDOW_MS, SCHEMA_CACHE_AGE_SECONDS, SCHEMA_CACHE_REFRESH_ERROR, } from '../../src/read-model/schema-cache'; @@ -22,8 +25,8 @@ describe('SchemaCache', () => { fetcher = { fetchSchema: jest.fn() }; }); - function build(): SchemaCache { - return new SchemaCache({ fetcher: fetcher as SchemaFetcher, metrics, now }); + function build(logger: Logger = () => undefined): SchemaCache { + return new SchemaCache({ fetcher: fetcher as SchemaFetcher, metrics, logger, now }); } describe('cold cache', () => { @@ -121,6 +124,23 @@ describe('SchemaCache', () => { expect(metrics.increment).toHaveBeenCalledWith(SCHEMA_CACHE_REFRESH_ERROR); }); + it('should log the cause and that the stale schema was served', async () => { + const logger = jest.fn(); + fetcher.fetchSchema + .mockResolvedValueOnce(makeSchema('users')) + .mockRejectedValueOnce(new Error('boom')); + const cache = build(logger); + + await cache.get(); + clock += ONE_DAY_MS; + await cache.get(); + + expect(logger).toHaveBeenCalledWith('Warn', 'Schema refresh failed', { + cause: 'Error: boom', + servedStale: true, + }); + }); + it('should re-attempt on the next read and serve the fresh schema once it succeeds', async () => { const good = makeSchema('users'); const fresh = makeSchema('users-v2'); @@ -186,6 +206,18 @@ describe('SchemaCache', () => { expect(cache.ageSeconds()).toBeUndefined(); }); + it('should log the empty schema as the cause, since the counter cannot tell it from an outage', async () => { + const logger = jest.fn(); + fetcher.fetchSchema.mockResolvedValue([]); + + await expect(build(logger).get()).rejects.toBeInstanceOf(SchemaUnavailableError); + + expect(logger).toHaveBeenCalledWith('Warn', 'Schema refresh failed', { + cause: 'Error: Forest returned an empty schema', + servedStale: false, + }); + }); + it('should keep serving the last good schema when a refresh returns empty', async () => { const good = makeSchema('users'); fetcher.fetchSchema.mockResolvedValueOnce(good).mockResolvedValueOnce([]); @@ -242,4 +274,152 @@ describe('SchemaCache', () => { expect(cache.revision).toBe(1); }); }); + + describe('clear', () => { + it('should re-read the schema on the next get', async () => { + const cache = build(); + fetcher.fetchSchema.mockResolvedValue(makeSchema('users')); + await cache.get(); + + cache.clear(); + await cache.get(); + + expect(fetcher.fetchSchema).toHaveBeenCalledTimes(2); + }); + + it('should keep re-reading during the revalidation window, since the SaaS may still be catching up', async () => { + const cache = build(); + fetcher.fetchSchema.mockResolvedValue(makeSchema('users')); + await cache.get(); + cache.clear(); + await cache.get(); + + clock += REVALIDATION_TTL_MS; + await cache.get(); + + expect(fetcher.fetchSchema).toHaveBeenCalledTimes(3); + }); + + it('should go back to the long TTL once the window is over', async () => { + const cache = build(); + fetcher.fetchSchema.mockResolvedValue(makeSchema('users')); + await cache.get(); + cache.clear(); + await cache.get(); + + clock += REVALIDATION_WINDOW_MS; + await cache.get(); + const afterWindow = fetcher.fetchSchema.mock.calls.length; + clock += REVALIDATION_TTL_MS; + await cache.get(); + + expect(fetcher.fetchSchema).toHaveBeenCalledTimes(afterWindow); + }); + + it('should not let a fetch started before the clear repopulate the cache', async () => { + const cache = build(); + const stale = makeSchema('stale'); + let releaseStale: (collections: ForestSchemaCollection[]) => void = () => undefined; + fetcher.fetchSchema.mockReturnValueOnce( + new Promise(resolve => { + releaseStale = resolve; + }), + ); + + const pending = cache.get(); + cache.clear(); + releaseStale(stale); + + await expect(pending).resolves.toEqual(stale); + + fetcher.fetchSchema.mockResolvedValue(makeSchema('fresh')); + const result = await cache.get(); + + expect(result).toEqual(makeSchema('fresh')); + expect(fetcher.fetchSchema).toHaveBeenCalledTimes(2); + }); + + it('should start its own fetch for a read that lands after the clear, not join the invalidated one', async () => { + const cache = build(); + const stale = makeSchema('stale'); + let releaseStale: (collections: ForestSchemaCollection[]) => void = () => undefined; + fetcher.fetchSchema.mockReturnValueOnce( + new Promise(resolve => { + releaseStale = resolve; + }), + ); + + const beforeClear = cache.get(); + cache.clear(); + fetcher.fetchSchema.mockResolvedValue(makeSchema('fresh')); + const afterClear = cache.get(); + releaseStale(stale); + + await expect(beforeClear).resolves.toEqual(stale); + await expect(afterClear).resolves.toEqual(makeSchema('fresh')); + expect(fetcher.fetchSchema).toHaveBeenCalledTimes(2); + }); + + it('should not promote a read taken inside the window to the long TTL once the window closes', async () => { + const cache = build(); + fetcher.fetchSchema.mockResolvedValue(makeSchema('users')); + await cache.get(); + + cache.clear(); + clock += REVALIDATION_TTL_MS; + await cache.get(); + clock += REVALIDATION_WINDOW_MS; + await cache.get(); + + expect(fetcher.fetchSchema).toHaveBeenCalledTimes(3); + }); + + it('should keep the last good schema as a fallback when the refresh after a clear fails', async () => { + const good = makeSchema('users'); + fetcher.fetchSchema.mockResolvedValueOnce(good).mockRejectedValue(new Error('boom')); + const cache = build(); + await cache.get(); + + cache.clear(); + const result = await cache.get(); + + expect(result).toBe(good); + expect(metrics.increment).toHaveBeenCalledWith(SCHEMA_CACHE_REFRESH_ERROR); + }); + + it('should keep re-reading after serving that fallback, so the invalidation is not defeated', async () => { + const good = makeSchema('users'); + const fresh = makeSchema('users-v2'); + fetcher.fetchSchema + .mockResolvedValueOnce(good) + .mockRejectedValueOnce(new Error('boom')) + .mockResolvedValueOnce(fresh); + const cache = build(); + await cache.get(); + + cache.clear(); + await cache.get(); + const result = await cache.get(); + + expect(result).toBe(fresh); + expect(fetcher.fetchSchema).toHaveBeenCalledTimes(3); + }); + + it('should not bump the revision for a fetch the clear invalidated', async () => { + const cache = build(); + let releaseStale: (collections: ForestSchemaCollection[]) => void = () => undefined; + fetcher.fetchSchema.mockReturnValueOnce( + new Promise(resolve => { + releaseStale = resolve; + }), + ); + + const pending = cache.get(); + cache.clear(); + releaseStale(makeSchema('stale')); + await pending; + + expect(cache.revision).toBe(0); + }); + }); });