Skip to content

Commit 17726cb

Browse files
Bill LeoutsakosBill Leoutsakos
authored andcommitted
fix(selectors): refresh caches after environment changes
1 parent e8e0a06 commit 17726cb

8 files changed

Lines changed: 316 additions & 19 deletions

File tree

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import {
5+
auditMock,
6+
authMockFns,
7+
createMockRequest,
8+
dbChainMockFns,
9+
environmentUtilsMockFns,
10+
posthogServerMock,
11+
resetDbChainMock,
12+
} from '@sim/testing'
13+
import { beforeEach, describe, expect, it, vi } from 'vitest'
14+
15+
const { mockEncryptSecret, mockSyncPersonalEnvCredentialsForUser } = vi.hoisted(() => ({
16+
mockEncryptSecret: vi.fn(),
17+
mockSyncPersonalEnvCredentialsForUser: vi.fn(),
18+
}))
19+
20+
vi.mock('@sim/audit', () => auditMock)
21+
vi.mock('@/lib/posthog/server', () => posthogServerMock)
22+
vi.mock('@/lib/core/security/encryption', () => ({
23+
decryptSecret: vi.fn(),
24+
encryptSecret: mockEncryptSecret,
25+
}))
26+
vi.mock('@/lib/credentials/environment', () => ({
27+
syncPersonalEnvCredentialsForUser: mockSyncPersonalEnvCredentialsForUser,
28+
}))
29+
30+
import { POST } from '@/app/api/environment/route'
31+
32+
describe('POST /api/environment', () => {
33+
beforeEach(() => {
34+
vi.clearAllMocks()
35+
resetDbChainMock()
36+
authMockFns.mockGetSession.mockResolvedValue({
37+
user: { id: 'user-1', name: 'Test User', email: 'test@example.com' },
38+
})
39+
mockEncryptSecret.mockResolvedValue({ encrypted: 'encrypted-value' })
40+
mockSyncPersonalEnvCredentialsForUser.mockResolvedValue(undefined)
41+
})
42+
43+
it('invalidates the effective environment cache immediately after the database update', async () => {
44+
const response = await POST(
45+
createMockRequest('POST', { variables: { JIRA_DOMAIN: 'example.atlassian.net' } })
46+
)
47+
48+
expect(response.status).toBe(200)
49+
expect(environmentUtilsMockFns.mockInvalidateEffectiveDecryptedEnvCache).toHaveBeenCalledWith({
50+
userId: 'user-1',
51+
})
52+
expect(dbChainMockFns.onConflictDoUpdate.mock.invocationCallOrder[0]).toBeLessThan(
53+
environmentUtilsMockFns.mockInvalidateEffectiveDecryptedEnvCache.mock.invocationCallOrder[0]
54+
)
55+
expect(
56+
environmentUtilsMockFns.mockInvalidateEffectiveDecryptedEnvCache.mock.invocationCallOrder[0]
57+
).toBeLessThan(mockSyncPersonalEnvCredentialsForUser.mock.invocationCallOrder[0])
58+
})
59+
})

apps/sim/app/api/environment/route.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import { generateRequestId } from '@/lib/core/utils/request'
1414
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
1515
import { syncPersonalEnvCredentialsForUser } from '@/lib/credentials/environment'
1616
import type { EnvironmentVariable } from '@/lib/environment/api'
17+
import { invalidateEffectiveDecryptedEnvCache } from '@/lib/environment/utils'
1718
import { captureServerEvent } from '@/lib/posthog/server'
1819

1920
const logger = createLogger('EnvironmentAPI')
@@ -69,6 +70,8 @@ export const POST = withRouteHandler(async (req: NextRequest) => {
6970
},
7071
})
7172

73+
invalidateEffectiveDecryptedEnvCache({ userId: session.user.id })
74+
7275
await syncPersonalEnvCredentialsForUser({
7376
userId: session.user.id,
7477
envKeys: Object.keys(variables),

apps/sim/hooks/queries/dynamic-subblock-options.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { useQueries } from '@tanstack/react-query'
33
import { buildSelectorContextFromBlock } from '@/lib/workflows/subblocks/context'
44
import { summarizeNames } from '@/lib/workflows/subblocks/display'
55
import type { SubBlockConfig } from '@/blocks/types'
6+
import { environmentDependentSelectorKeys } from '@/hooks/selectors/cache-invalidation'
67
import {
78
createSelectorCacheScopeRegistry,
89
scopeServerResolvedSelectorContext,
@@ -18,7 +19,7 @@ export const DYNAMIC_SUBBLOCK_OPTION_STALE_TIME = 30 * 1000
1819

1920
export const dynamicSubBlockOptionKeys = {
2021
all: ['dynamic-subblock-options'] as const,
21-
details: () => [...dynamicSubBlockOptionKeys.all, 'detail'] as const,
22+
details: () => environmentDependentSelectorKeys.dynamicDetails,
2223
/**
2324
* `selectorScope` is the selector's OWN query key for this context — every context field its
2425
* result depends on, named by the selector rather than restated here. Without it a label

apps/sim/hooks/queries/environment.test.tsx

Lines changed: 204 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,33 @@
11
/**
22
* @vitest-environment jsdom
33
*/
4-
import { act } from 'react'
5-
import { QueryClient, QueryClientProvider } from '@tanstack/react-query'
6-
import { createRoot } from 'react-dom/client'
7-
import { afterEach, describe, expect, it, vi } from 'vitest'
4+
import { act, type ReactNode } from 'react'
5+
import { sleep } from '@sim/utils/helpers'
6+
import { QueryClient, QueryClientProvider, useQuery } from '@tanstack/react-query'
7+
import { createRoot, type Root } from 'react-dom/client'
8+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
89

9-
const { mockFetchWorkspaceEnvironment } = vi.hoisted(() => ({
10+
const { mockFetchWorkspaceEnvironment, mockRequestJson } = vi.hoisted(() => ({
1011
mockFetchWorkspaceEnvironment: vi.fn(),
12+
mockRequestJson: vi.fn(),
1113
}))
1214

15+
vi.mock('@/lib/api/client/request', () => ({ requestJson: mockRequestJson }))
16+
1317
vi.mock('@/lib/environment/api', () => ({
1418
fetchPersonalEnvironment: vi.fn(),
1519
fetchWorkspaceEnvironment: mockFetchWorkspaceEnvironment,
1620
}))
1721

18-
import { useWorkspaceEnvironment } from '@/hooks/queries/environment'
22+
import type { QueryKey } from '@tanstack/react-query'
23+
import {
24+
environmentKeys,
25+
useRemoveWorkspaceEnvironment,
26+
useSavePersonalEnvironment,
27+
useUpsertWorkspaceEnvironment,
28+
useWorkspaceEnvironment,
29+
} from '@/hooks/queries/environment'
30+
import { environmentDependentSelectorKeys } from '@/hooks/selectors/cache-invalidation'
1931

2032
function renderWorkspaceEnvironment(workspaceId: string, enabled?: boolean) {
2133
;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true
@@ -41,10 +53,112 @@ function renderWorkspaceEnvironment(workspaceId: string, enabled?: boolean) {
4153
return () => act(() => root.unmount())
4254
}
4355

44-
afterEach(() => {
56+
interface CacheQueryFns {
57+
environment: ReturnType<typeof vi.fn>
58+
primary: ReturnType<typeof vi.fn>
59+
dynamicDetails: ReturnType<typeof vi.fn>
60+
workflowDetails: ReturnType<typeof vi.fn>
61+
workflowReplacementOptions: ReturnType<typeof vi.fn>
62+
unrelated: ReturnType<typeof vi.fn>
63+
}
64+
65+
function createCacheQueryFns(): CacheQueryFns {
66+
return {
67+
environment: vi.fn().mockResolvedValue('environment'),
68+
primary: vi.fn().mockResolvedValue('primary'),
69+
dynamicDetails: vi.fn().mockResolvedValue('dynamic'),
70+
workflowDetails: vi.fn().mockResolvedValue('workflow-detail'),
71+
workflowReplacementOptions: vi.fn().mockResolvedValue('workflow-options'),
72+
unrelated: vi.fn().mockResolvedValue('unrelated'),
73+
}
74+
}
75+
76+
function renderMutationWithCaches<T>(
77+
useMutationHook: () => T,
78+
environmentQueryKey: QueryKey,
79+
queryFns: CacheQueryFns
80+
): { result: () => T; unmount: () => void } {
81+
;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true
82+
const queryClient = new QueryClient({
83+
defaultOptions: {
84+
queries: { retry: false, staleTime: Number.POSITIVE_INFINITY },
85+
mutations: { retry: false },
86+
},
87+
})
88+
const container = document.createElement('div')
89+
const root: Root = createRoot(container)
90+
let latest: T
91+
92+
function Probe() {
93+
useQuery({ queryKey: environmentQueryKey, queryFn: queryFns.environment })
94+
useQuery({
95+
queryKey: [...environmentDependentSelectorKeys.primary, 'jira.projects', 'scope'],
96+
queryFn: queryFns.primary,
97+
})
98+
useQuery({
99+
queryKey: [...environmentDependentSelectorKeys.dynamicDetails, 'jira.project', 'scope'],
100+
queryFn: queryFns.dynamicDetails,
101+
})
102+
useQuery({
103+
queryKey: [...environmentDependentSelectorKeys.workflowDetails, 'jira.projects', 'scope'],
104+
queryFn: queryFns.workflowDetails,
105+
})
106+
useQuery({
107+
queryKey: [
108+
...environmentDependentSelectorKeys.workflowReplacementOptions,
109+
'jira.projects',
110+
'scope',
111+
],
112+
queryFn: queryFns.workflowReplacementOptions,
113+
})
114+
useQuery({ queryKey: ['unrelated-user-data'], queryFn: queryFns.unrelated })
115+
latest = useMutationHook()
116+
return null
117+
}
118+
119+
function Wrapper({ children }: { children: ReactNode }) {
120+
return <QueryClientProvider client={queryClient}>{children}</QueryClientProvider>
121+
}
122+
123+
act(() => {
124+
root.render(
125+
<Wrapper>
126+
<Probe />
127+
</Wrapper>
128+
)
129+
})
130+
131+
return {
132+
result: () => latest,
133+
unmount: () => act(() => root.unmount()),
134+
}
135+
}
136+
137+
async function flush() {
138+
await act(async () => {
139+
for (let i = 0; i < 5; i++) {
140+
await Promise.resolve()
141+
await sleep(0)
142+
}
143+
})
144+
}
145+
146+
function expectOnlyEnvironmentAndSelectorCachesRefetched(queryFns: CacheQueryFns) {
147+
expect(queryFns.environment).toHaveBeenCalledTimes(2)
148+
expect(queryFns.primary).toHaveBeenCalledTimes(2)
149+
expect(queryFns.dynamicDetails).toHaveBeenCalledTimes(2)
150+
expect(queryFns.workflowDetails).toHaveBeenCalledTimes(2)
151+
expect(queryFns.workflowReplacementOptions).toHaveBeenCalledTimes(2)
152+
expect(queryFns.unrelated).toHaveBeenCalledTimes(1)
153+
}
154+
155+
beforeEach(() => {
45156
vi.clearAllMocks()
157+
mockRequestJson.mockResolvedValue({ success: true })
46158
})
47159

160+
afterEach(() => vi.restoreAllMocks())
161+
48162
describe('useWorkspaceEnvironment', () => {
49163
it('does not run without a workspace ID even when the caller enables it', () => {
50164
const unmount = renderWorkspaceEnvironment('', true)
@@ -60,3 +174,86 @@ describe('useWorkspaceEnvironment', () => {
60174
unmount()
61175
})
62176
})
177+
178+
describe('environment mutation selector freshness', () => {
179+
it('refetches every selector-bearing cache after a successful personal save', async () => {
180+
const queryFns = createCacheQueryFns()
181+
const view = renderMutationWithCaches(
182+
useSavePersonalEnvironment,
183+
environmentKeys.personal(),
184+
queryFns
185+
)
186+
await flush()
187+
188+
await act(async () => {
189+
await view.result().mutateAsync({ variables: { DOMAIN: 'new-value' } })
190+
})
191+
await flush()
192+
193+
expectOnlyEnvironmentAndSelectorCachesRefetched(queryFns)
194+
view.unmount()
195+
})
196+
197+
it.each([
198+
{
199+
name: 'upsert',
200+
useMutationHook: useUpsertWorkspaceEnvironment,
201+
variables: { workspaceId: 'workspace-1', variables: { DOMAIN: 'new-value' } },
202+
},
203+
{
204+
name: 'removal',
205+
useMutationHook: useRemoveWorkspaceEnvironment,
206+
variables: { workspaceId: 'workspace-1', keys: ['DOMAIN'] },
207+
},
208+
])('refetches every selector-bearing cache after a successful workspace $name', async (test) => {
209+
const queryFns = createCacheQueryFns()
210+
const view = renderMutationWithCaches(
211+
test.useMutationHook,
212+
environmentKeys.workspace('workspace-1'),
213+
queryFns
214+
)
215+
await flush()
216+
217+
await act(async () => {
218+
const mutation = view.result() as {
219+
mutateAsync: (variables: unknown) => Promise<unknown>
220+
}
221+
await mutation.mutateAsync(test.variables)
222+
})
223+
await flush()
224+
225+
expectOnlyEnvironmentAndSelectorCachesRefetched(queryFns)
226+
view.unmount()
227+
})
228+
229+
it('does not invalidate selector caches after a failed environment mutation', async () => {
230+
const requestError = new Error('save failed')
231+
mockRequestJson.mockRejectedValueOnce(requestError)
232+
const queryFns = createCacheQueryFns()
233+
const view = renderMutationWithCaches(
234+
useSavePersonalEnvironment,
235+
environmentKeys.personal(),
236+
queryFns
237+
)
238+
await flush()
239+
240+
let caught: unknown
241+
await act(async () => {
242+
try {
243+
await view.result().mutateAsync({ variables: { DOMAIN: 'new-value' } })
244+
} catch (error) {
245+
caught = error
246+
}
247+
})
248+
await flush()
249+
250+
expect(caught).toBe(requestError)
251+
expect(queryFns.environment).toHaveBeenCalledTimes(2)
252+
expect(queryFns.primary).toHaveBeenCalledTimes(1)
253+
expect(queryFns.dynamicDetails).toHaveBeenCalledTimes(1)
254+
expect(queryFns.workflowDetails).toHaveBeenCalledTimes(1)
255+
expect(queryFns.workflowReplacementOptions).toHaveBeenCalledTimes(1)
256+
expect(queryFns.unrelated).toHaveBeenCalledTimes(1)
257+
view.unmount()
258+
})
259+
})

apps/sim/hooks/queries/environment.ts

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
} from '@/lib/api/contracts'
1010
import type { WorkspaceEnvironmentData } from '@/lib/environment/api'
1111
import { fetchPersonalEnvironment, fetchWorkspaceEnvironment } from '@/lib/environment/api'
12+
import { invalidateEnvironmentDependentSelectorQueries } from '@/hooks/selectors/cache-invalidation'
1213

1314
const logger = createLogger('EnvironmentQueries')
1415

@@ -75,11 +76,12 @@ export function useSavePersonalEnvironment() {
7576

7677
logger.info('Saved personal environment variables')
7778
},
78-
onSettled: async () => {
79+
onSettled: async (_data, error) => {
7980
await Promise.all([
8081
queryClient.invalidateQueries({ queryKey: environmentKeys.personal() }),
8182
queryClient.invalidateQueries({ queryKey: environmentKeys.workspaces() }),
8283
])
84+
if (!error) await invalidateEnvironmentDependentSelectorQueries(queryClient)
8385
},
8486
})
8587
}
@@ -103,10 +105,12 @@ export function useUpsertWorkspaceEnvironment() {
103105
logger.info(`Upserted workspace environment variables for workspace: ${workspaceId}`)
104106
return data
105107
},
106-
onSettled: (_data, _error, variables) =>
107-
queryClient.invalidateQueries({
108+
onSettled: async (_data, error, variables) => {
109+
await queryClient.invalidateQueries({
108110
queryKey: environmentKeys.workspace(variables.workspaceId),
109-
}),
111+
})
112+
if (!error) await invalidateEnvironmentDependentSelectorQueries(queryClient)
113+
},
110114
})
111115
}
112116

@@ -129,9 +133,11 @@ export function useRemoveWorkspaceEnvironment() {
129133
logger.info(`Removed ${keys.length} workspace environment keys for workspace: ${workspaceId}`)
130134
return data
131135
},
132-
onSettled: (_data, _error, variables) =>
133-
queryClient.invalidateQueries({
136+
onSettled: async (_data, error, variables) => {
137+
await queryClient.invalidateQueries({
134138
queryKey: environmentKeys.workspace(variables.workspaceId),
135-
}),
139+
})
140+
if (!error) await invalidateEnvironmentDependentSelectorQueries(queryClient)
141+
},
136142
})
137143
}

0 commit comments

Comments
 (0)