History

Expand scope coverage (#427)

Rhys Sullivan committed 6 months ago

Commit e30e2131 parent 1c5d8523 files · +201 −5Browse files

Unified · Split

3 files
  1. apps/cloud/src/services/tenant-isolation.node.test.ts +52 −0
  2. packages/core/storage-drizzle/src/adapter.ts +17 −5
  3. packages/core/storage-postgres/src/index.test.ts +132 −0
apps/cloud/src/services/tenant-isolation.node.test.ts+52−0
@@ -155,4 +155,56 @@ describe("tenant isolation (HTTP)", () => { · 154 unmodified lines
155155 expect(result._tag).toBe("Left");
156156 }),
157157 );
158+
159+ it.effect("updating a same-namespace OpenAPI source in one org does not mutate another org", () =>
160+ Effect.gen(function* () {
161+ const orgA = `org_${crypto.randomUUID()}`;
162+ const orgB = `org_${crypto.randomUUID()}`;
163+ const namespace = `shared_${crypto.randomUUID().replace(/-/g, "_")}`;
164+
165+ yield* asOrg(orgA, (client) =>
166+ client.openapi.addSpec({
167+ path: { scopeId: ScopeId.make(orgA) },
168+ payload: {
169+ spec: MINIMAL_OPENAPI_SPEC,
170+ namespace,
171+ name: "Org A API",
172+ baseUrl: "https://org-a.example.com",
173+ },
174+ }),
175+ );
176+ yield* asOrg(orgB, (client) =>
177+ client.openapi.addSpec({
178+ path: { scopeId: ScopeId.make(orgB) },
179+ payload: {
180+ spec: MINIMAL_OPENAPI_SPEC,
181+ namespace,
182+ name: "Org B API",
183+ baseUrl: "https://org-b.example.com",
184+ },
185+ }),
186+ );
187+
188+ yield* asOrg(orgA, (client) =>
189+ client.openapi.updateSource({
190+ path: { scopeId: ScopeId.make(orgA), namespace },
191+ payload: {
192+ name: "Org A Updated API",
193+ baseUrl: "https://org-a-updated.example.com",
194+ },
195+ }),
196+ );
197+
198+ const orgASource = yield* asOrg(orgA, (client) =>
199+ client.openapi.getSource({ path: { scopeId: ScopeId.make(orgA), namespace } }),
200+ );
201+ const orgBSource = yield* asOrg(orgB, (client) =>
202+ client.openapi.getSource({ path: { scopeId: ScopeId.make(orgB), namespace } }),
203+ );
204+ expect(orgASource?.name).toBe("Org A Updated API");
205+ expect(orgASource?.config.baseUrl).toBe("https://org-a-updated.example.com");
206+ expect(orgBSource?.name).toBe("Org B API");
207+ expect(orgBSource?.config.baseUrl).toBe("https://org-b.example.com");
208+ }),
209+ );
158210 });
packages/core/storage-drizzle/src/adapter.ts+17−5
@@ -212,6 +212,17 @@ const compileWhere = ( · 211 unmodified lines
212212 return andClause ?? orClause;
213213 };
214214
215+const rowIdentityClause = (
216+ table: AnyTable,
217+ row: Record<string, unknown>,
218+): SQL => {
219+ const idClause = eq(table.id, row.id);
220+ if (table.scope_id && typeof row.scope_id === "string") {
221+ return and(eq(table.scope_id, row.scope_id), idClause) as SQL;
222+ }
223+ return idClause;
224+};
225+
215226 // ---------------------------------------------------------------------------
216227 // Join → drizzle `with` clause
217228 //
@@ -592,7 +603,8 @@ export const drizzleAdapter = (options: DrizzleAdapterOptions): DBAdapter => { · 374 unmodified lines
592603 if (matched.length === 0) return null;
593604 if (matched.length > 1) return null;
594605 const target = matched[0]!;
595− let updQ = db.update(table).set(update).where(eq(table.id, target.id));
606+ const identity = rowIdentityClause(table, target);
607+ let updQ = db.update(table).set(update).where(identity);
596608 if (provider !== "mysql") {
597609 const rows = (yield* runPromise(
598610 "update returning",
@@ -608,7 +620,7 @@ export const drizzleAdapter = (options: DrizzleAdapterOptions): DBAdapter => { · 9 unmodified lines
608620 );
609621 const reread = (yield* runPromise(
610622 "mysql update reread",
611− () => db.select().from(table).where(eq(table.id, target.id)).limit(1),
623+ () => db.select().from(table).where(identity).limit(1),
612624 model,
613625 )) as Record<string, unknown>[];
614626 return (reread[0] ?? null) as never;
@@ -652,18 +664,18 @@ export const drizzleAdapter = (options: DrizzleAdapterOptions): DBAdapter => { · 37 unmodified lines
652664 const table = getTable(model);
653665 const clause = compileWhere(table, where, provider);
654666 // Mirror in-memory semantics: delete first matching row only
655− let findQ = db.select({ id: table.id }).from(table);
667+ let findQ = db.select().from(table);
656668 if (clause) findQ = findQ.where(clause);
657669 const matched = (yield* runPromise(
658670 "delete pre-select",
659671 () => findQ.limit(1),
660672 model,
661− )) as { id: unknown }[];
673+ )) as Record<string, unknown>[];
662674 const first = matched[0];
663675 if (!first) return;
664676 yield* runPromise(
665677 "delete exec",
666− () => Promise.resolve(db.delete(table).where(eq(table.id, first.id))),
678+ () => Promise.resolve(db.delete(table).where(rowIdentityClause(table, first))),
667679 model,
668680 );
669681 }).pipe(
packages/core/storage-postgres/src/index.test.ts+132−0
@@ -6,12 +6,14 @@ · 5 unmodified lines
66 // pattern apps/cloud uses). Port 5435 so it doesn't clash with the
77 // cloud test DB on 5434.
88
9+import { describe, expect, it } from "@effect/vitest";
910 import { Effect } from "effect";
1011 import postgres from "postgres";
1112 import { drizzle } from "drizzle-orm/postgres-js";
1213 import { relations } from "drizzle-orm";
1314 import {
1415 pgTable,
16+ primaryKey,
1517 text,
1618 doublePrecision,
1719 boolean,
@@ -68,6 +70,16 @@ const with_defaults = pgTable("with_defaults", { · 50 unmodified lines
6870 touchedAt: timestamp("touchedAt"),
6971 });
7072
73+const scoped_item = pgTable(
74+ "scoped_item",
75+ {
76+ id: text("id").notNull(),
77+ scope_id: text("scope_id").notNull(),
78+ label: text("label"),
79+ },
80+ (table) => [primaryKey({ columns: [table.scope_id, table.id] })],
81+);
82+
7183 const sourceRelations = relations(source, ({ many }) => ({
7284 source_tag: many(source_tag),
7385 }));
@@ -157,3 +169,123 @@ const withAdapter = <A, E>( · 83 unmodified lines
157169 }) as Effect.Effect<A, E | Error>;
158170
159171 runAdapterConformance("postgres", withAdapter);
172+
173+const scopedSchema = {
174+ scoped_item: {
175+ fields: {
176+ scope_id: { type: "string", required: true, index: true },
177+ label: { type: "string", required: true },
178+ },
179+ },
180+} as const;
181+
182+const resetScopedTable = Effect.tryPromise({
183+ try: async () => {
184+ await sql.unsafe(`DROP TABLE IF EXISTS "scoped_item" CASCADE`);
185+ await sql.unsafe(
186+ `CREATE TABLE "scoped_item" (
187+ "id" TEXT NOT NULL,
188+ "scope_id" TEXT NOT NULL,
189+ "label" TEXT,
190+ PRIMARY KEY ("scope_id", "id")
191+ )`,
192+ );
193+ },
194+ catch: (cause) =>
195+ new Error(`failed to reset scoped_item table: ${String(cause)}`),
196+});
197+
198+const makeScopedAdapter = () =>
199+ makePostgresAdapter({
200+ db: drizzle(sql, { schema: { scoped_item } }),
201+ schema: scopedSchema,
202+ });
203+
204+describe("postgres scoped row identity", () => {
205+ it.effect("update pins composite identity when id is reused across scopes", () =>
206+ Effect.gen(function* () {
207+ yield* resetScopedTable;
208+ const adapter = makeScopedAdapter();
209+
210+ yield* adapter.create({
211+ model: "scoped_item",
212+ forceAllowId: true,
213+ data: { id: "shared", scope_id: "scope-a", label: "a" } as never,
214+ });
215+ yield* adapter.create({
216+ model: "scoped_item",
217+ forceAllowId: true,
218+ data: { id: "shared", scope_id: "scope-b", label: "b" } as never,
219+ });
220+
221+ yield* adapter.update({
222+ model: "scoped_item",
223+ where: [
224+ { field: "id", value: "shared" },
225+ { field: "scope_id", value: "scope-a" },
226+ ],
227+ update: { label: "a-updated" },
228+ });
229+
230+ const scopeA = yield* adapter.findOne<{ label: string }>({
231+ model: "scoped_item",
232+ where: [
233+ { field: "id", value: "shared" },
234+ { field: "scope_id", value: "scope-a" },
235+ ],
236+ });
237+ const scopeB = yield* adapter.findOne<{ label: string }>({
238+ model: "scoped_item",
239+ where: [
240+ { field: "id", value: "shared" },
241+ { field: "scope_id", value: "scope-b" },
242+ ],
243+ });
244+ expect(scopeA?.label).toBe("a-updated");
245+ expect(scopeB?.label).toBe("b");
246+ }),
247+ );
248+
249+ it.effect("delete pins composite identity when id is reused across scopes", () =>
250+ Effect.gen(function* () {
251+ yield* resetScopedTable;
252+ const adapter = makeScopedAdapter();
253+
254+ yield* adapter.create({
255+ model: "scoped_item",
256+ forceAllowId: true,
257+ data: { id: "shared", scope_id: "scope-a", label: "a" } as never,
258+ });
259+ yield* adapter.create({
260+ model: "scoped_item",
261+ forceAllowId: true,
262+ data: { id: "shared", scope_id: "scope-b", label: "b" } as never,
263+ });
264+
265+ yield* adapter.delete({
266+ model: "scoped_item",
267+ where: [
268+ { field: "id", value: "shared" },
269+ { field: "scope_id", value: "scope-a" },
270+ ],
271+ });
272+
273+ const scopeA = yield* adapter.findOne<{ label: string }>({
274+ model: "scoped_item",
275+ where: [
276+ { field: "id", value: "shared" },
277+ { field: "scope_id", value: "scope-a" },
278+ ],
279+ });
280+ const scopeB = yield* adapter.findOne<{ label: string }>({
281+ model: "scoped_item",
282+ where: [
283+ { field: "id", value: "shared" },
284+ { field: "scope_id", value: "scope-b" },
285+ ],
286+ });
287+ expect(scopeA).toBeNull();
288+ expect(scopeB?.label).toBe("b");
289+ }),
290+ );
291+});