From 45da30dc64f983296d2c4fa409588855f32f0ec2 Mon Sep 17 00:00:00 2001 From: Kevin De Porre Date: Wed, 27 Nov 2024 09:19:55 +0100 Subject: [PATCH 1/3] Remove databaseId option from ShapeStream in favor of params option. --- packages/typescript-client/src/client.ts | 17 +---------------- .../test/__snapshots__/client.test.ts.snap | 2 +- packages/typescript-client/test/client.test.ts | 2 +- 3 files changed, 3 insertions(+), 18 deletions(-) diff --git a/packages/typescript-client/src/client.ts b/packages/typescript-client/src/client.ts index 5836e72ff2..68dc3aba62 100644 --- a/packages/typescript-client/src/client.ts +++ b/packages/typescript-client/src/client.ts @@ -34,13 +34,11 @@ import { SHAPE_HANDLE_QUERY_PARAM, SHAPE_SCHEMA_HEADER, WHERE_QUERY_PARAM, - DATABASE_ID_QUERY_PARAM, TABLE_QUERY_PARAM, REPLICA_PARAM, } from './constants' const RESERVED_PARAMS = new Set([ - DATABASE_ID_QUERY_PARAM, COLUMNS_QUERY_PARAM, LIVE_CACHE_BUSTER_QUERY_PARAM, SHAPE_HANDLE_QUERY_PARAM, @@ -54,7 +52,6 @@ const RESERVED_PARAMS = new Set([ type Replica = `full` | `default` type ReservedParamKeys = - | typeof DATABASE_ID_QUERY_PARAM | typeof COLUMNS_QUERY_PARAM | typeof LIVE_CACHE_BUSTER_QUERY_PARAM | typeof SHAPE_HANDLE_QUERY_PARAM @@ -84,12 +81,6 @@ export interface ShapeStreamOptions { */ url: string - /** - * Which database to use. - * This is optional unless Electric is used with multiple databases. - */ - databaseId?: string - /** * The root table for the shape. Passed as a query parameter. Not required if you set the table in your proxy. */ @@ -144,7 +135,7 @@ export interface ShapeStreamOptions { * Additional request parameters to attach to the URL. * These will be merged with Electric's standard parameters. * Note: You cannot use Electric's reserved parameter names - * (table, where, columns, offset, handle, live, cursor, database_id, replica). + * (table, where, columns, offset, handle, live, cursor, replica). */ params?: ParamsRecord @@ -246,7 +237,6 @@ export class ShapeStream = Row> #isUpToDate: boolean = false #connected: boolean = false #shapeHandle?: string - #databaseId?: string #schema?: Schema #onError?: ShapeStreamErrorHandler #replica?: Replica @@ -257,7 +247,6 @@ export class ShapeStream = Row> this.#lastOffset = this.options.offset ?? `-1` this.#liveCacheBuster = `` this.#shapeHandle = this.options.handle - this.#databaseId = this.options.databaseId this.#messageParser = new MessageParser(options.parser) this.#replica = this.options.replica this.#onError = this.options.onError @@ -347,10 +336,6 @@ export class ShapeStream = Row> ) } - if (this.#databaseId) { - fetchUrl.searchParams.set(DATABASE_ID_QUERY_PARAM, this.#databaseId!) - } - if ( (this.#replica ?? ShapeStream.Replica.DEFAULT) != ShapeStream.Replica.DEFAULT diff --git a/packages/typescript-client/test/__snapshots__/client.test.ts.snap b/packages/typescript-client/test/__snapshots__/client.test.ts.snap index 273ea4aaa2..7c8bb2bc64 100644 --- a/packages/typescript-client/test/__snapshots__/client.test.ts.snap +++ b/packages/typescript-client/test/__snapshots__/client.test.ts.snap @@ -1,3 +1,3 @@ // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html -exports[`Shape > should throw on a reserved parameter 1`] = `[ReservedParamError: Cannot use reserved Electric parameter names in custom params: database_id]`; +exports[`Shape > should throw on a reserved parameter 1`] = `[ReservedParamError: Cannot use reserved Electric parameter names in custom params: live]`; diff --git a/packages/typescript-client/test/client.test.ts b/packages/typescript-client/test/client.test.ts index e54b5c4426..a4c92e6d0d 100644 --- a/packages/typescript-client/test/client.test.ts +++ b/packages/typescript-client/test/client.test.ts @@ -30,7 +30,7 @@ describe(`Shape`, () => { url: `${BASE_URL}/v1/shape`, table: `foo`, params: { - database_id: `foo`, + live: `false`, }, }) new Shape(shapeStream) From 5690a27067fc1ee1a07e84289e785429b7171d3f Mon Sep 17 00:00:00 2001 From: Kevin De Porre Date: Wed, 27 Nov 2024 09:21:40 +0100 Subject: [PATCH 2/3] changeset --- .changeset/old-ligers-run.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/old-ligers-run.md diff --git a/.changeset/old-ligers-run.md b/.changeset/old-ligers-run.md new file mode 100644 index 0000000000..5939326b91 --- /dev/null +++ b/.changeset/old-ligers-run.md @@ -0,0 +1,5 @@ +--- +"@electric-sql/client": minor +--- + +[BREAKING]: Remove databaseId option from ShapeStream in favor of params option. From b9c36a2b7a8174f95629556884080db0a4c49588 Mon Sep 17 00:00:00 2001 From: Kevin De Porre Date: Wed, 27 Nov 2024 12:44:24 +0100 Subject: [PATCH 3/3] Remove all occurences of databaseId in the TS client and react hooks. --- .../react-hooks/test/support/global-setup.ts | 3 +- packages/typescript-client/src/constants.ts | 1 - .../test/integration.test.ts | 185 +----------------- .../test/support/global-setup.ts | 28 +-- .../test/support/test-context.ts | 100 +--------- 5 files changed, 9 insertions(+), 308 deletions(-) diff --git a/packages/react-hooks/test/support/global-setup.ts b/packages/react-hooks/test/support/global-setup.ts index bab5fd3156..039b4e0ce0 100644 --- a/packages/react-hooks/test/support/global-setup.ts +++ b/packages/react-hooks/test/support/global-setup.ts @@ -3,7 +3,6 @@ import { makePgClient } from './test-helpers' const url = process.env.ELECTRIC_URL ?? `http://localhost:3000` const proxyUrl = process.env.ELECTRIC_PROXY_CACHE_URL ?? `http://localhost:3002` -const databaseId = process.env.ELECTRIC_DATABASE_ID ?? `test_tenant` // name of proxy cache container to execute commands against, // see docker-compose.yml that spins it up for details @@ -30,7 +29,7 @@ function waitForElectric(url: string): Promise { ) const tryHealth = async () => - fetch(`${url}/v1/health?database_id=${databaseId}`) + fetch(`${url}/v1/health`) .then(async (res): Promise => { if (!res.ok) return tryHealth() const { status } = (await res.json()) as { status: string } diff --git a/packages/typescript-client/src/constants.ts b/packages/typescript-client/src/constants.ts index 5425dd2eaa..25b8843bd9 100644 --- a/packages/typescript-client/src/constants.ts +++ b/packages/typescript-client/src/constants.ts @@ -3,7 +3,6 @@ export const SHAPE_HANDLE_HEADER = `electric-handle` export const CHUNK_LAST_OFFSET_HEADER = `electric-offset` export const SHAPE_SCHEMA_HEADER = `electric-schema` export const CHUNK_UP_TO_DATE_HEADER = `electric-up-to-date` -export const DATABASE_ID_QUERY_PARAM = `database_id` export const COLUMNS_QUERY_PARAM = `columns` export const LIVE_CACHE_BUSTER_QUERY_PARAM = `cursor` export const SHAPE_HANDLE_QUERY_PARAM = `handle` diff --git a/packages/typescript-client/test/integration.test.ts b/packages/typescript-client/test/integration.test.ts index 2f0530a2c1..da10e7361b 100644 --- a/packages/typescript-client/test/integration.test.ts +++ b/packages/typescript-client/test/integration.test.ts @@ -9,14 +9,11 @@ import { IssueRow, testWithIssuesTable as it, testWithMultitypeTable as mit, - //testWithMultiTenantIssuesTable as multiTenantIt, } from './support/test-context' import * as h from './support/test-helpers' const BASE_URL = inject(`baseUrl`) -//const OTHER_DATABASE_URL = inject(`otherDatabaseUrl`) -//const databaseId = inject(`databaseId`) -//const otherDatabaseId = inject(`otherDatabaseId`) + it(`sanity check`, async ({ dbClient, issuesTableSql }) => { const result = await dbClient.query(`SELECT * FROM ${issuesTableSql}`) @@ -976,183 +973,3 @@ describe(`HTTP Sync`, () => { }) }) }) - -/* -describe.sequential(`Multi tenancy sync`, () => { - it(`should allow new databases to be added`, async () => { - const url = new URL(`${BASE_URL}/v1/admin/database`) - - // Add the database - const res = await fetch(url.toString(), { - method: `POST`, - headers: { - Accept: `application/json`, - 'Content-Type': `application/json`, - }, - body: JSON.stringify({ - database_id: otherDatabaseId, - database_url: OTHER_DATABASE_URL, - }), - }) - - expect(res.status).toBe(200) - const body = await res.json() - expect(body).toBe(otherDatabaseId) - }) - - it(`should serve original database`, async ({ - issuesTableUrl, - aborter, - insertIssues, - }) => { - const id = await insertIssues({ title: `test issue` }) - - const shapeData = new Map() - const issueStream = new ShapeStream({ - url: `${BASE_URL}/v1/shape`, - table: issuesTableUrl, - databaseId, - subscribe: false, - signal: aborter.signal, - }) - - await new Promise((resolve, reject) => { - issueStream.subscribe((messages) => { - messages.forEach((message) => { - if (isChangeMessage(message)) { - shapeData.set(message.key, message.value) - } - if (isUpToDateMessage(message)) { - aborter.abort() - return resolve() - } - }) - }, reject) - }) - - const values = [...shapeData.values()] - expect(values).toHaveLength(1) - expect(values[0]).toMatchObject({ - id: id[0], - title: `test issue`, - }) - }) - - multiTenantIt( - `should serve new database`, - async ({ issuesTableUrl, aborter, insertIssuesToOtherDb }) => { - const id = await insertIssuesToOtherDb({ title: `test issue in new db` }) - - const shapeData = new Map() - const issueStream = new ShapeStream({ - url: `${BASE_URL}/v1/shape`, - table: issuesTableUrl, - databaseId: otherDatabaseId, - subscribe: false, - signal: aborter.signal, - }) - - await new Promise((resolve, reject) => { - issueStream.subscribe((messages) => { - messages.forEach((message) => { - if (isChangeMessage(message)) { - shapeData.set(message.key, message.value) - } - if (isUpToDateMessage(message)) { - aborter.abort() - return resolve() - } - }) - }, reject) - }) - - const values = [...shapeData.values()] - expect(values).toHaveLength(1) - expect(values[0]).toMatchObject({ - id: id[0], - title: `test issue in new db`, - }) - } - ) - - multiTenantIt( - `should serve both databases in live mode`, - async ({ - issuesTableUrl, - aborter, - otherAborter, - insertIssues, - insertIssuesToOtherDb, - }) => { - // Set up streams for both databases - const defaultStream = new ShapeStream({ - url: `${BASE_URL}/v1/shape`, - table: issuesTableUrl, - databaseId, - subscribe: true, - signal: aborter.signal, - }) - - const otherStream = new ShapeStream({ - url: `${BASE_URL}/v1/shape`, - table: issuesTableUrl, - databaseId: otherDatabaseId, - subscribe: true, - signal: otherAborter.signal, - }) - - const defaultData = new Map() - const otherData = new Map() - - // Set up subscriptions - defaultStream.subscribe((messages) => { - messages.forEach((message) => { - if (isChangeMessage(message)) { - defaultData.set(message.key, message.value) - } - }) - }) - - otherStream.subscribe((messages) => { - messages.forEach((message) => { - if (isChangeMessage(message)) { - otherData.set(message.key, message.value) - } - }) - }) - - // Insert data into both databases - const defaultId = await insertIssues({ title: `default db issue` }) - const otherId = await insertIssuesToOtherDb({ title: `other db issue` }) - - // Give time for updates to propagate - await sleep(1000) - - // Verify data from default database - expect([...defaultData.values()]).toHaveLength(1) - expect([...defaultData.values()][0]).toMatchObject({ - id: defaultId[0], - title: `default db issue`, - }) - - // Verify data from other database - expect([...otherData.values()]).toHaveLength(1) - expect([...otherData.values()][0]).toMatchObject({ - id: otherId[0], - title: `other db issue`, - }) - } - ) - - it(`should allow databases to be deleted`, async () => { - const url = new URL(`${BASE_URL}/v1/admin/database/${otherDatabaseId}`) - - // Add the database - const res = await fetch(url.toString(), { method: `DELETE` }) - - expect(res.status).toBe(200) - const body = await res.json() - expect(body).toBe(otherDatabaseId) - }) -}) -*/ diff --git a/packages/typescript-client/test/support/global-setup.ts b/packages/typescript-client/test/support/global-setup.ts index 634f0c2ea4..a97f2e2667 100644 --- a/packages/typescript-client/test/support/global-setup.ts +++ b/packages/typescript-client/test/support/global-setup.ts @@ -1,14 +1,8 @@ import type { GlobalSetupContext } from 'vitest/node' import { makePgClient } from './test-helpers' -import { Client } from 'pg' const url = process.env.ELECTRIC_URL ?? `http://localhost:3000` const proxyUrl = process.env.ELECTRIC_PROXY_CACHE_URL ?? `http://localhost:3002` -const databaseId = process.env.ELECTRIC_DATABASE_ID ?? `test_tenant` -const otherDatabaseId = `other_test_tenant` -const otherDatabaseUrl = - process.env.OTHER_DATABASE_URL ?? - `postgresql://postgres:password@localhost:54322/electric?sslmode=disable` // name of proxy cache container to execute commands against, // see docker-compose.yml that spins it up for details @@ -24,9 +18,6 @@ declare module 'vitest' { testPgSchema: string proxyCacheContainerName: string proxyCachePath: string - databaseId: string - otherDatabaseId: string - otherDatabaseUrl: string } } @@ -38,7 +29,7 @@ function waitForElectric(url: string): Promise { ) const tryHealth = async () => - fetch(`${url}/v1/health?database_id=${databaseId}`) + fetch(`${url}/v1/health`) .then(async (res): Promise => { if (!res.ok) return tryHealth() const { status } = (await res.json()) as { status: string } @@ -63,27 +54,18 @@ export default async function ({ provide }: GlobalSetupContext) { await waitForElectric(url) const client = makePgClient() - const otherClient = new Client(otherDatabaseUrl) - const clients = [client, otherClient] - for (const c of clients) { - await c.connect() - await c.query(`CREATE SCHEMA IF NOT EXISTS electric_test`) - } + await client.connect() + await client.query(`CREATE SCHEMA IF NOT EXISTS electric_test`) provide(`baseUrl`, url) provide(`testPgSchema`, `electric_test`) provide(`proxyCacheBaseUrl`, proxyUrl) provide(`proxyCacheContainerName`, proxyCacheContainerName) provide(`proxyCachePath`, proxyCachePath) - provide(`databaseId`, databaseId) - provide(`otherDatabaseId`, otherDatabaseId) - provide(`otherDatabaseUrl`, otherDatabaseUrl) return async () => { - for (const c of clients) { - await c.query(`DROP SCHEMA electric_test CASCADE`) - await c.end() - } + await client.query(`DROP SCHEMA electric_test CASCADE`) + await client.end() } } diff --git a/packages/typescript-client/test/support/test-context.ts b/packages/typescript-client/test/support/test-context.ts index 965c980910..12fdb44362 100644 --- a/packages/typescript-client/test/support/test-context.ts +++ b/packages/typescript-client/test/support/test-context.ts @@ -4,10 +4,7 @@ import { Client, QueryResult } from 'pg' import { inject, test } from 'vitest' import { makePgClient } from './test-helpers' import { FetchError } from '../../src/error' -import { - DATABASE_ID_QUERY_PARAM, - SHAPE_HANDLE_QUERY_PARAM, -} from '../../src/constants' +import { SHAPE_HANDLE_QUERY_PARAM } from '../../src/constants' export type IssueRow = { id: string; title: string; priority?: string } export type GeneratedIssueRow = { id?: string; title: string } @@ -17,7 +14,7 @@ export type InsertIssuesFn = (...rows: GeneratedIssueRow[]) => Promise export type ClearIssuesShapeFn = (handle?: string) => Promise export type ClearShapeFn = ( table: string, - options?: { handle?: string; databaseId?: string } + options?: { handle?: string } ) => Promise export const testWithDbClient = test.extend<{ @@ -46,7 +43,6 @@ export const testWithDbClient = test.extend<{ async ( table: string, options: { - databaseId?: string handle?: string } = {} ) => { @@ -54,12 +50,6 @@ export const testWithDbClient = test.extend<{ const url = new URL(`${baseUrl}/v1/shape`) url.searchParams.set(`table`, table) - if (!options.databaseId) { - options.databaseId = inject(`databaseId`) - } - - url.searchParams.set(DATABASE_ID_QUERY_PARAM, options.databaseId) - if (options.handle) { url.searchParams.set(SHAPE_HANDLE_QUERY_PARAM, options.handle) } @@ -78,26 +68,6 @@ export const testWithDbClient = test.extend<{ }, }) -export const testWithDbClients = testWithDbClient.extend<{ - otherDbClient: Client - otherAborter: AbortController -}>({ - otherDbClient: async ({}, use) => { - const client = new Client({ - connectionString: inject(`otherDatabaseUrl`), - options: `-csearch_path=${inject(`testPgSchema`)}`, - }) - await client.connect() - await use(client) - await client.end() - }, - otherAborter: async ({}, use) => { - const controller = new AbortController() - await use(controller) - controller.abort(`Test complete`) - }, -}) - export const testWithIssuesTable = testWithDbClient.extend<{ issuesTableSql: string issuesTableUrl: string @@ -161,72 +131,6 @@ export const testWithIssuesTable = testWithDbClient.extend<{ }, }) -export const testWithMultiTenantIssuesTable = testWithDbClients.extend<{ - issuesTableSql: string - issuesTableUrl: string - insertIssues: InsertIssuesFn - insertIssuesToOtherDb: InsertIssuesFn -}>({ - issuesTableSql: async ({ dbClient, otherDbClient, task }, use) => { - const tableName = `"issues for ${task.id}_${Math.random().toString(16)}"` - const clients = [dbClient, otherDbClient] - const queryProms = clients.map((client) => - client.query(` - DROP TABLE IF EXISTS ${tableName}; - CREATE TABLE ${tableName} ( - id UUID PRIMARY KEY, - title TEXT NOT NULL, - priority INTEGER NOT NULL - ); - COMMENT ON TABLE ${tableName} IS 'Created for ${task.file?.name.replace(/'/g, `\``) ?? `unknown`} - ${task.name.replace(`'`, `\``)}'; - `) - ) - - await Promise.all(queryProms) - - await use(tableName) - - const cleanupProms = clients.map((client) => - client.query(`DROP TABLE ${tableName}`) - ) - await Promise.all(cleanupProms) - }, - issuesTableUrl: async ({ issuesTableSql, pgSchema, clearShape }, use) => { - const urlAppropriateTable = pgSchema + `.` + issuesTableSql - await use(urlAppropriateTable) - // ignore errors - clearShape has its own logging - // we don't want to interrupt cleanup - await Promise.allSettled([ - clearShape(urlAppropriateTable), - clearShape(urlAppropriateTable, { - databaseId: inject(`otherDatabaseId`), - }), - ]) - }, - insertIssues: ({ issuesTableSql, dbClient }, use) => - use(async (...rows) => { - const placeholders = rows.map( - (_, i) => `($${i * 3 + 1}, $${i * 3 + 2}, $${i * 3 + 3})` - ) - const { rows: result } = await dbClient.query( - `INSERT INTO ${issuesTableSql} (id, title, priority) VALUES ${placeholders} RETURNING id`, - rows.flatMap((x) => [x.id ?? uuidv4(), x.title, 10]) - ) - return result.map((x) => x.id) - }), - insertIssuesToOtherDb: ({ issuesTableSql, otherDbClient }, use) => - use(async (...rows) => { - const placeholders = rows.map( - (_, i) => `($${i * 3 + 1}, $${i * 3 + 2}, $${i * 3 + 3})` - ) - const { rows: result } = await otherDbClient.query( - `INSERT INTO ${issuesTableSql} (id, title, priority) VALUES ${placeholders} RETURNING id`, - rows.flatMap((x) => [x.id ?? uuidv4(), x.title, 10]) - ) - return result.map((x) => x.id) - }), -}) - export const testWithMultitypeTable = testWithDbClient.extend<{ tableSql: string tableUrl: string