mirror of
https://github.com/supabase/supabase.git
synced 2026-10-09 03:15:06 +03:00
Fourth of the stack; PRs 1–3 (#49069, #49070, #49072) have merged, so this now targets `master` directly. **Rebased onto latest `master`.** See "Conflict resolution" at the bottom for what was reconciled. ## The bug `QueryEditor` took `sql: string`, so a query's dialect brand died at the prop boundary and the component re-branded whatever it was handed based on a separately-passed `source`. Nothing tied the two together, which meant nothing stopped Postgres SQL from reaching the analytics endpoint. Explorer query drafts made it concrete. `explorer-query.ts` branded **every** draft with `untrustedSql` regardless of source: ```ts uncheckedSql: untrustedSql(sql) // even for a logs draft ``` and the editor then re-branded that same text with `untrustedLogSql` at run time for a logs draft — laundering a Postgres-branded value straight through the boundary that `safe-analytics-sql.ts` exists to defend. The brands are deliberately disjoint precisely so this can't happen; passing plain strings around defeated it. ## The fix Both carriers are now tagged by backend, so one `_tag` check narrows the SQL brand and that backend's parameters together. - **`ExplorerQueryDraft`** becomes `DatabaseQueryDraft | LogsQueryDraft`, and `toDraft` is the single place a persisted string re-enters the type system — branded for the backend its binding names. The draft is rebuilt rather than mutated in place, since a backend change changes which brand its SQL carries. - **`QueryEditor`** takes one discriminated `query` prop instead of `sql` + `source` + `rowLimit`. The tag picks both the brander at the editor boundary and the execution endpoint, so the mismatch is no longer expressible. - The two `acceptUntrusted*` promotions stay **inlined** in the run handler rather than factored into a shared helper, so each stays visible next to the user gesture that authorizes it, per the safe-SQL model. - **`rowLimit` moves onto the database member.** Logs execution has no use for it — `applyAutoLimit` is Postgres-specific — so it no longer sits on a shared type where it reads as meaningful for both. ## Local storage Existing query drafts shape-mismatch and fall back to a database binding via the existing `safeParse` guard — harmless, and notebooks are still behind the `explorer` flag so there is no saved server content in play. ## Conflict resolution `master` moved inside every file this PR touches. The type change is applied on top of that work; nothing was reverted. | Preserved from `master` | Where | |---|---| | zod parsing of persisted drafts (`persistedDraftsSchema`, `persistedDraftSchema`) | `explorer-query.ts` | | `MAX_PERSISTED_EXPLORER_QUERY_DRAFTS` cap, retaining most-recently-updated | `explorer-query.ts` | | debounced SQL persistence + `flushPendingPersistence`, immediate write-through for rename/source | `explorer-query.ts` | | `removeDraft` clearing pending timers | `explorer-query.ts` | | `getQuerySourceBinding(cell)` and the four source-change branches, incl. `database_identifier` / `time_range` propagation | `QueryCell/index.tsx` | | `restoredQueryKey` per `ref:id` and the `role="status"` loader | `QueryTab.tsx` | | `applyAutoLimit` relocated to `@/data/sql/utils` | `QueryEditor.tsx` | Two adaptations were needed: - `updateDraft` rebuilds the draft through `toDraft` instead of mutating it in place — required, because the object's shape depends on its tag. The debounced `persist` closure still re-reads `state.drafts[id]` at fire time, so behavior is unchanged. - Master's new test `falls back to the database source when persisted source data is invalid` asserted `draft.source`, which the tagged union replaces. Rewritten to assert the same intent against `_tag`. **Dropped from this PR's original description:** it previously claimed to fix a log cell always running against a synthesized default time range. Master fixed that itself by adopting `getQuerySourceBinding` (from #49072), so the claim no longer applies. ## Verification Typecheck, Prettier, and the lint ratchet clean. 736 tests pass across `state/`, the Explorer surfaces, notebooks, query sources, `data/sql`, and the SQL editor — including master's new `QueryTab.test.tsx`, `ExplorerQuerySourceMenu.test.tsx`, `ExplorerQueryTabCoordinator.test.tsx`, and the five draft-store tests added since this branch was cut. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Improved Explorer query handling across database and logs backends. - Preserved query text when switching backends while clearing incompatible results. - Retained results when changing parameters within the same backend. - Improved restoration of saved drafts, including fallback handling for legacy or invalid sources. - Added validation before executing edited SQL to help prevent invalid requests. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
220 lines
8.2 KiB
TypeScript
220 lines
8.2 KiB
TypeScript
import { LOCAL_STORAGE_KEYS } from 'common'
|
|
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
|
|
|
import {
|
|
createExplorerQueryState,
|
|
EXPLORER_QUERY_PERSIST_DELAY,
|
|
MAX_PERSISTED_EXPLORER_QUERY_DRAFTS,
|
|
} from './explorer-query'
|
|
|
|
const createMemoryStorage = () => {
|
|
const values = new Map<string, string>()
|
|
|
|
return {
|
|
getItem: (key: string) => values.get(key) ?? null,
|
|
setItem: vi.fn((key: string, value: string) => values.set(key, value)),
|
|
removeItem: (key: string) => values.delete(key),
|
|
}
|
|
}
|
|
|
|
const LOGS_SOURCE = {
|
|
_tag: 'logs',
|
|
time_range: { _tag: 'relative_time_range', amount: 3, unit: 'hour' },
|
|
} as const
|
|
|
|
describe('explorer query drafts', () => {
|
|
beforeEach(() => vi.useFakeTimers())
|
|
afterEach(() => vi.useRealTimers())
|
|
|
|
it('persists and restores drafts within their project', () => {
|
|
const storage = createMemoryStorage()
|
|
const firstState = createExplorerQueryState(storage)
|
|
|
|
firstState.createDraft({ id: 'query-1', projectRef: 'project-a' })
|
|
firstState.updateDraft({ id: 'query-1', name: 'Active users', sql: 'select * from users' })
|
|
|
|
const secondState = createExplorerQueryState(storage)
|
|
|
|
expect(secondState.restoreDraft({ id: 'query-1', projectRef: 'project-a' })).toBe(true)
|
|
expect(secondState.drafts['query-1']).toMatchObject({
|
|
_tag: 'database',
|
|
name: 'Active users',
|
|
uncheckedSql: 'select * from users',
|
|
projectRef: 'project-a',
|
|
})
|
|
expect(secondState.restoreDraft({ id: 'query-1', projectRef: 'project-b' })).toBe(false)
|
|
})
|
|
|
|
it('persists source parameters and clears stale results when the backend changes', () => {
|
|
const storage = createMemoryStorage()
|
|
const state = createExplorerQueryState(storage)
|
|
|
|
state.createDraft({ id: 'query-1', projectRef: 'project-a', sql: 'select 1' })
|
|
state.setResult({ id: 'query-1', result: { rows: [{ value: 1 }], executedAt: 1 } })
|
|
state.updateDraft({ id: 'query-1', source: LOGS_SOURCE })
|
|
|
|
expect(state.results['query-1']).toBeUndefined()
|
|
|
|
const restored = createExplorerQueryState(storage)
|
|
expect(restored.restoreDraft({ id: 'query-1', projectRef: 'project-a' })).toBe(true)
|
|
expect(restored.drafts['query-1']).toMatchObject(LOGS_SOURCE)
|
|
})
|
|
|
|
it('carries the query text over when the backend changes, rebranded for the new dialect', () => {
|
|
const storage = createMemoryStorage()
|
|
const state = createExplorerQueryState(storage)
|
|
|
|
state.createDraft({ id: 'query-1', projectRef: 'project-a', sql: 'select * from users' })
|
|
state.updateDraft({ id: 'query-1', source: LOGS_SOURCE })
|
|
|
|
expect(state.drafts['query-1']).toMatchObject({
|
|
_tag: 'logs',
|
|
uncheckedSql: 'select * from users',
|
|
})
|
|
})
|
|
|
|
it('keeps the query when only the parameters of the same backend change', () => {
|
|
const storage = createMemoryStorage()
|
|
const state = createExplorerQueryState(storage)
|
|
|
|
state.createDraft({ id: 'query-1', projectRef: 'project-a', sql: 'select * from users' })
|
|
state.setResult({ id: 'query-1', result: { rows: [{ value: 1 }], executedAt: 1 } })
|
|
state.updateDraft({
|
|
id: 'query-1',
|
|
source: { _tag: 'database', database_identifier: 'replica-1' },
|
|
})
|
|
|
|
expect(state.drafts['query-1']).toMatchObject({
|
|
_tag: 'database',
|
|
database_identifier: 'replica-1',
|
|
uncheckedSql: 'select * from users',
|
|
})
|
|
expect(state.results['query-1']).toBeDefined()
|
|
})
|
|
|
|
it('restores pre-source drafts as database queries', () => {
|
|
const storage = createMemoryStorage()
|
|
storage.setItem(
|
|
LOCAL_STORAGE_KEYS.EXPLORER_QUERY_DRAFTS('project-a'),
|
|
JSON.stringify({
|
|
'query-1': { name: 'Legacy query', sql: 'select 1', updatedAt: 1 },
|
|
})
|
|
)
|
|
|
|
const state = createExplorerQueryState(storage)
|
|
expect(state.restoreDraft({ id: 'query-1', projectRef: 'project-a' })).toBe(true)
|
|
expect(state.drafts['query-1']._tag).toBe('database')
|
|
})
|
|
|
|
it('ignores a malformed root value', () => {
|
|
const storage = createMemoryStorage()
|
|
storage.setItem(LOCAL_STORAGE_KEYS.EXPLORER_QUERY_DRAFTS('project-a'), JSON.stringify([]))
|
|
|
|
const state = createExplorerQueryState(storage)
|
|
expect(state.restoreDraft({ id: 'query-1', projectRef: 'project-a' })).toBe(false)
|
|
})
|
|
|
|
it('drops entries with malformed draft fields', () => {
|
|
const storage = createMemoryStorage()
|
|
storage.setItem(
|
|
LOCAL_STORAGE_KEYS.EXPLORER_QUERY_DRAFTS('project-a'),
|
|
JSON.stringify({
|
|
'query-1': { name: 'Invalid query', sql: 123, updatedAt: 1 },
|
|
})
|
|
)
|
|
|
|
const state = createExplorerQueryState(storage)
|
|
expect(state.restoreDraft({ id: 'query-1', projectRef: 'project-a' })).toBe(false)
|
|
})
|
|
|
|
it('falls back to the database source when persisted source data is invalid', () => {
|
|
const storage = createMemoryStorage()
|
|
storage.setItem(
|
|
LOCAL_STORAGE_KEYS.EXPLORER_QUERY_DRAFTS('project-a'),
|
|
JSON.stringify({
|
|
'query-1': {
|
|
name: 'Recoverable query',
|
|
sql: 'select 1',
|
|
updatedAt: 1,
|
|
source: { id: 'logs', type: 'logs', parameters: {} },
|
|
},
|
|
})
|
|
)
|
|
|
|
const state = createExplorerQueryState(storage)
|
|
expect(state.restoreDraft({ id: 'query-1', projectRef: 'project-a' })).toBe(true)
|
|
expect(state.drafts['query-1']).toMatchObject({
|
|
_tag: 'database',
|
|
uncheckedSql: 'select 1',
|
|
})
|
|
})
|
|
|
|
it('debounces SQL persistence while updating in-memory state immediately', () => {
|
|
const storage = createMemoryStorage()
|
|
const state = createExplorerQueryState(storage)
|
|
const key = LOCAL_STORAGE_KEYS.EXPLORER_QUERY_DRAFTS('project-a')
|
|
state.createDraft({ id: 'query-1', projectRef: 'project-a' })
|
|
storage.setItem.mockClear()
|
|
|
|
state.updateDraft({ id: 'query-1', sql: 's' })
|
|
state.updateDraft({ id: 'query-1', sql: 'se' })
|
|
state.updateDraft({ id: 'query-1', sql: 'select 1' })
|
|
|
|
expect(state.drafts['query-1'].uncheckedSql).toBe('select 1')
|
|
expect(storage.setItem).not.toHaveBeenCalled()
|
|
|
|
vi.advanceTimersByTime(EXPLORER_QUERY_PERSIST_DELAY)
|
|
|
|
expect(storage.setItem).toHaveBeenCalledOnce()
|
|
expect(JSON.parse(storage.getItem(key)!)['query-1'].sql).toBe('select 1')
|
|
})
|
|
|
|
it('flushes pending SQL persistence before the debounce elapses', () => {
|
|
const storage = createMemoryStorage()
|
|
const state = createExplorerQueryState(storage)
|
|
const key = LOCAL_STORAGE_KEYS.EXPLORER_QUERY_DRAFTS('project-a')
|
|
state.createDraft({ id: 'query-1', projectRef: 'project-a' })
|
|
storage.setItem.mockClear()
|
|
|
|
state.updateDraft({ id: 'query-1', sql: 'select 1' })
|
|
state.flushPendingPersistence({ projectRef: 'project-a' })
|
|
|
|
expect(storage.setItem).toHaveBeenCalledOnce()
|
|
expect(JSON.parse(storage.getItem(key)!)['query-1'].sql).toBe('select 1')
|
|
|
|
vi.advanceTimersByTime(EXPLORER_QUERY_PERSIST_DELAY)
|
|
expect(storage.setItem).toHaveBeenCalledOnce()
|
|
})
|
|
|
|
it('retains only the most recently updated persisted drafts', () => {
|
|
const storage = createMemoryStorage()
|
|
const state = createExplorerQueryState(storage)
|
|
const key = LOCAL_STORAGE_KEYS.EXPLORER_QUERY_DRAFTS('project-a')
|
|
|
|
for (let index = 0; index <= MAX_PERSISTED_EXPLORER_QUERY_DRAFTS; index++) {
|
|
vi.setSystemTime(index)
|
|
state.createDraft({ id: `query-${index}`, projectRef: 'project-a' })
|
|
}
|
|
|
|
const persisted = JSON.parse(storage.getItem(key)!)
|
|
expect(Object.keys(persisted)).toHaveLength(MAX_PERSISTED_EXPLORER_QUERY_DRAFTS)
|
|
expect(persisted['query-0']).toBeUndefined()
|
|
expect(persisted[`query-${MAX_PERSISTED_EXPLORER_QUERY_DRAFTS}`]).toBeDefined()
|
|
})
|
|
|
|
it('removes the persisted draft and its session result when its tab closes', () => {
|
|
const storage = createMemoryStorage()
|
|
const state = createExplorerQueryState(storage)
|
|
|
|
state.createDraft({ id: 'query-1', projectRef: 'project-a', sql: 'select 1' })
|
|
state.updateDraft({ id: 'query-1', sql: 'select 2' })
|
|
state.setResult({ id: 'query-1', result: { rows: [{ value: 1 }], executedAt: 1 } })
|
|
state.removeDraft({ id: 'query-1', projectRef: 'project-a' })
|
|
vi.advanceTimersByTime(EXPLORER_QUERY_PERSIST_DELAY)
|
|
|
|
expect(state.drafts['query-1']).toBeUndefined()
|
|
expect(state.results['query-1']).toBeUndefined()
|
|
expect(storage.getItem(LOCAL_STORAGE_KEYS.EXPLORER_QUERY_DRAFTS('project-a'))).toBeNull()
|
|
})
|
|
})
|