From a100aa57382518e9525d0f98e7540ea5e8d09545 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Mon, 6 Apr 2026 17:55:16 -0400 Subject: [PATCH] =?UTF-8?q?=E2=9A=A1=20perf:=20Short-Circuit=20Config=20Ov?= =?UTF-8?q?erride=20Resolution=20for=20Empty=20Principals=20(#12549)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Skip the getApplicableConfigs DB query when buildPrincipals returns an empty array, since there are no principals to match against. --- packages/api/src/app/service.spec.ts | 27 ++++++++++++++++++++++++++- packages/api/src/app/service.ts | 6 ++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/packages/api/src/app/service.spec.ts b/packages/api/src/app/service.spec.ts index e5e076c8eb..f881b87354 100644 --- a/packages/api/src/app/service.spec.ts +++ b/packages/api/src/app/service.spec.ts @@ -201,14 +201,16 @@ describe('createAppConfigService', () => { const deps = createDeps({ getApplicableConfigs: mockGetConfigs }); const { getAppConfig } = createAppConfigService(deps); + // No role/userId → empty principals → short-circuits without calling getApplicableConfigs await getAppConfig(); + expect(mockGetConfigs).not.toHaveBeenCalled(); mockGetConfigs.mockResolvedValueOnce([ { priority: 10, overrides: { restricted: true }, isActive: true }, ]); const config = await getAppConfig({ role: 'ADMIN' }); - expect(mockGetConfigs).toHaveBeenCalledTimes(2); + expect(mockGetConfigs).toHaveBeenCalledTimes(1); expect((config as TestConfig).restricted).toBe(true); }); @@ -229,6 +231,29 @@ describe('createAppConfigService', () => { expect((config as TestConfig).x).toBe('admin-only'); }); + it('skips getApplicableConfigs when buildPrincipals returns empty', async () => { + const deps = createDeps({ + getUserPrincipals: jest.fn().mockResolvedValue([]), + }); + const { getAppConfig } = createAppConfigService(deps); + + const config = await getAppConfig({ userId: 'uid1', role: 'USER' }); + + expect(deps.getUserPrincipals).toHaveBeenCalledWith({ userId: 'uid1', role: 'USER' }); + expect(deps.getApplicableConfigs).not.toHaveBeenCalled(); + expect(config).toEqual(deps._baseConfig); + }); + + it('skips getApplicableConfigs when no role or userId is provided', async () => { + const deps = createDeps(); + const { getAppConfig } = createAppConfigService(deps); + + const config = await getAppConfig(); + + expect(deps.getApplicableConfigs).not.toHaveBeenCalled(); + expect(config).toEqual(deps._baseConfig); + }); + it('falls back to base config on getApplicableConfigs error', async () => { const deps = createDeps({ getApplicableConfigs: jest.fn().mockRejectedValue(new Error('DB down')), diff --git a/packages/api/src/app/service.ts b/packages/api/src/app/service.ts index 6c5d307709..29fda7df92 100644 --- a/packages/api/src/app/service.ts +++ b/packages/api/src/app/service.ts @@ -170,6 +170,12 @@ export function createAppConfigService(deps: AppConfigServiceDeps) { try { const principals = await buildPrincipals(role, userId); + + if (principals.length === 0) { + await cache.set(cacheKey, baseConfig, overrideCacheTtl); + return baseConfig; + } + const configs = await getApplicableConfigs(principals); if (configs.length === 0) {