chore(zero): Change the CRUD protocol (#2648)

to pass primaryKey and value instead of id and value. The value already contains the id so all we really need is the primaryKey.
rocicorp · Oct 11, 2024 · 4a06ddd · 4a06ddd
1 parent 262a43d
commit 4a06ddd
Show file tree

Hide file tree

Showing 8 changed files with 180 additions and 139 deletions.
diff --git a/packages/zero-cache/src/services/mutagen/mutagen.test.ts b/packages/zero-cache/src/services/mutagen/mutagen.test.ts
@@ -1,11 +1,11 @@
 import {resolver} from '@rocicorp/resolver';
 import {afterEach, beforeEach, describe, expect, test} from 'vitest';
-import {Mode} from '../../db/transaction-pool.js';
 import {
   ErrorKind,
   MutationType,
   type CRUDMutation,
 } from '../../../../zero-protocol/src/mod.js';
+import {Mode} from '../../db/transaction-pool.js';
 import {expectTables, testDBs} from '../../test/db.js';
 import type {PostgresDB} from '../../types/pg.js';
 import {processMutation} from './mutagen.js';
@@ -109,9 +109,9 @@ describe('processMutation', () => {
               ops: [
                 {
                   op: 'create',
-                  entityType: 'idonly',
-                  id: {id: '1'},
-                  value: {},
+                  tableName: 'idonly',
+                  primaryKey: ['id'],
+                  value: {id: '1'},
                 },
               ],
             },
@@ -161,9 +161,9 @@ describe('processMutation', () => {
             ops: [
               {
                 op: 'create',
-                entityType: 'idonly',
-                id: {id: '1'},
-                value: {},
+                tableName: 'idonly',
+                primaryKey: ['id'],
+                value: {id: '1'},
               },
             ],
           },
@@ -211,9 +211,9 @@ describe('processMutation', () => {
             ops: [
               {
                 op: 'create',
-                entityType: 'idonly',
-                id: {id: '1'},
-                value: {},
+                tableName: 'idonly',
+                primaryKey: ['id'],
+                value: {id: '1'},
               },
             ],
           },
@@ -263,9 +263,9 @@ describe('processMutation', () => {
             ops: [
               {
                 op: 'create',
-                entityType: 'idonly',
-                id: {id: '1'}, // This would result in a duplicate key value if applied.
-                value: {},
+                tableName: 'idonly',
+                primaryKey: ['id'],
+                value: {id: '1'}, // This would result in a duplicate key value if applied.
               },
             ],
           },
@@ -314,9 +314,9 @@ describe('processMutation', () => {
               ops: [
                 {
                   op: 'create',
-                  entityType: 'idonly',
-                  id: {id: '1'},
-                  value: {},
+                  tableName: 'idonly',
+                  primaryKey: ['id'],
+                  value: {id: '1'},
                 },
               ],
             },
@@ -368,9 +368,9 @@ describe('processMutation', () => {
               ops: [
                 {
                   op: 'create',
-                  entityType: 'idonly',
-                  id: {id: '1'},
-                  value: {},
+                  tableName: 'idonly',
+                  primaryKey: ['id'],
+                  value: {id: '1'},
                 },
               ],
             },
@@ -422,9 +422,9 @@ describe('processMutation', () => {
               ops: [
                 {
                   op: 'create',
-                  entityType: 'idonly',
-                  id: {id: '1'},
-                  value: {},
+                  tableName: 'idonly',
+                  primaryKey: ['id'],
+                  value: {id: '1'},
                 },
               ],
             },
@@ -469,42 +469,47 @@ describe('processMutation', () => {
             ops: [
               {
                 op: 'create',
-                entityType: 'id_and_cols',
-                id: {id: '1'},
+                tableName: 'id_and_cols',
+                primaryKey: ['id'],
                 value: {
+                  id: '1',
                   col1: 'create',
                   col2: 'create',
                 },
               },
               {
                 op: 'set',
-                entityType: 'id_and_cols',
-                id: {id: '2'},
+                tableName: 'id_and_cols',
+                primaryKey: ['id'],
                 value: {
+                  id: '2',
                   col1: 'set',
                   col2: 'set',
                 },
               },
               {
                 op: 'update',
-                entityType: 'id_and_cols',
-                id: {id: '1'},
-                partialValue: {
+                tableName: 'id_and_cols',
+                primaryKey: ['id'],
+                value: {
+                  id: '1',
                   col1: 'update',
                 },
               },
               {
                 op: 'set',
-                entityType: 'id_and_cols',
-                id: {id: '1'},
+                tableName: 'id_and_cols',
+                primaryKey: ['id'],
                 value: {
+                  id: '1',
                   col2: 'set',
                 },
               },
               {
                 op: 'delete',
-                entityType: 'id_and_cols',
-                id: {id: '2'},
+                tableName: 'id_and_cols',
+                primaryKey: ['id'],
+                value: {id: '2'},
               },
             ],
           },
@@ -554,9 +559,10 @@ describe('processMutation', () => {
             ops: [
               {
                 op: 'create',
-                entityType: 'fk_ref',
-                id: {id: '1'},
+                tableName: 'fk_ref',
+                primaryKey: ['id'],
                 value: {
+                  id: '1',
                   ref: '1',
                 },
               },
@@ -622,9 +628,11 @@ describe('processMutation', () => {
             ops: [
               {
                 op: 'create',
-                entityType: 'idonly',
-                id: {id: '1'},
-                value: {},
+                tableName: 'idonly',
+                primaryKey: ['id'],
+                value: {
+                  id: '1',
+                },
               },
             ],
           },

diff --git a/packages/zero-cache/src/services/mutagen/mutagen.ts b/packages/zero-cache/src/services/mutagen/mutagen.ts
@@ -4,7 +4,12 @@ import {resolver} from '@rocicorp/resolver';
 import type {JWTPayload} from 'jose';
 import postgres from 'postgres';
 import {assert, unreachable} from '../../../../shared/src/asserts.js';
+import * as v from '../../../../shared/src/valita.js';
 import {ErrorKind} from '../../../../zero-protocol/src/mod.js';
+import {
+  primaryKeyValueSchema,
+  type PrimaryKeyValue,
+} from '../../../../zero-protocol/src/primary-key.js';
 import {
   MutationType,
   type CRUDMutation,
@@ -302,27 +307,17 @@ export function getCreateSQL(
   tx: postgres.TransactionSql,
   create: CreateOp,
 ): postgres.PendingQuery<postgres.Row[]> {
-  const table = create.entityType;
-  const {id, value} = create;
-
-  const valueWithIdColumns = {
-    ...value,
-    ...id,
-  };
-
-  return tx`INSERT INTO ${tx(table)} ${tx(valueWithIdColumns)}`;
+  return tx`INSERT INTO ${tx(create.tableName)} ${tx(create.value)}`;
 }
 
 export function getSetSQL(
   tx: postgres.TransactionSql,
   set: SetOp,
 ): postgres.PendingQuery<postgres.Row[]> {
-  const table = set.entityType;
-  const {id, value} = set;
-
+  const {tableName, primaryKey, value} = set;
   return tx`
-    INSERT INTO ${tx(table)} ${tx({...value, ...id})}
-    ON CONFLICT (${tx(Object.keys(id))})
+    INSERT INTO ${tx(tableName)} ${tx(value)}
+    ON CONFLICT (${tx(primaryKey)})
     DO UPDATE SET ${tx(value)}
   `;
 }
@@ -331,28 +326,30 @@ function getUpdateSQL(
   tx: postgres.TransactionSql,
   update: UpdateOp,
 ): postgres.PendingQuery<postgres.Row[]> {
-  const table = update.entityType;
-  const {id, partialValue} = update;
-
-  return tx`UPDATE ${tx(table)} SET ${tx(partialValue)} WHERE ${tx(id)}`;
+  const table = update.tableName;
+  const {primaryKey, value} = update;
+  const id: Record<string, PrimaryKeyValue> = {};
+  for (const key of primaryKey) {
+    id[key] = v.parse(value[key], primaryKeyValueSchema);
+  }
+  return tx`UPDATE ${tx(table)} SET ${tx(value)} WHERE ${tx(id)}`;
 }
 
 function getDeleteSQL(
   tx: postgres.TransactionSql,
   deleteOp: DeleteOp,
 ): postgres.PendingQuery<postgres.Row[]> {
-  const table = deleteOp.entityType;
-  const {id} = deleteOp;
+  const {tableName, primaryKey, value} = deleteOp;
 
   const conditions = [];
-  for (const [key, value] of Object.entries(id)) {
+  for (const key of primaryKey) {
     if (conditions.length > 0) {
       conditions.push(tx`AND`);
     }
-    conditions.push(tx`${tx(key)} = ${value}`);
+    conditions.push(tx`${tx(key)} = ${value[key]}`);
   }
 
-  return tx`DELETE FROM ${tx(table)} WHERE ${conditions}`;
+  return tx`DELETE FROM ${tx(tableName)} WHERE ${conditions}`;
 }
 
 async function checkSchemaVersionAndIncrementLastMutationID(

diff --git a/packages/zero-cache/src/services/mutagen/write-authorizer.test.ts b/packages/zero-cache/src/services/mutagen/write-authorizer.test.ts
@@ -180,8 +180,9 @@ describe('can insert/update/delete/upsert', () => {
       (['Insert', 'Update', 'Delete', 'Upsert'] as const)) {
       expect(
         authorizer[`can${op}`](jwtPayload, {
-          id: {id: id ?? 1},
-          entityType: 'foo',
+          tableName: 'foo',
+          primaryKey: ['id'] as const,
+          value: {id: id ?? 1},
           // eslint-disable-next-line @typescript-eslint/no-explicit-any
         } as any),
       ).toBe(expected);

diff --git a/packages/zero-cache/src/services/mutagen/write-authorizer.ts b/packages/zero-cache/src/services/mutagen/write-authorizer.ts
@@ -1,3 +1,4 @@
+import type {SQLQuery} from '@databases/sql';
 import {LogContext} from '@rocicorp/logger';
 import type {JWTPayload} from 'jose';
 import {tmpdir} from 'node:os';
@@ -6,14 +7,20 @@ import {pid} from 'node:process';
 import {assert} from '../../../../shared/src/asserts.js';
 import type {JSONValue} from '../../../../shared/src/json.js';
 import {randInt} from '../../../../shared/src/rand.js';
+import * as v from '../../../../shared/src/valita.js';
 import type {
   CreateOp,
   DeleteOp,
   SetOp,
   UpdateOp,
 } from '../../../../zero-protocol/src/mod.js';
+import {
+  primaryKeyValueSchema,
+  type PrimaryKeyValue,
+} from '../../../../zero-protocol/src/primary-key.js';
 import type {BuilderDelegate} from '../../../../zql/src/zql/builder/builder.js';
 import {buildPipeline} from '../../../../zql/src/zql/builder/builder.js';
+import {MissingParameterError} from '../../../../zql/src/zql/builder/error.js';
 import type {Row} from '../../../../zql/src/zql/ivm/data.js';
 import {Database} from '../../../../zqlite/src/db.js';
 import {compile, sql} from '../../../../zqlite/src/internal/sql.js';
@@ -29,7 +36,6 @@ import {mapLiteDataTypeToZqlSchemaValue} from '../../types/lite.js';
 import {DatabaseStorage} from '../view-syncer/database-storage.js';
 import type {NormalizedTableSpec} from '../view-syncer/pipeline-driver.js';
 import {normalize} from '../view-syncer/pipeline-driver.js';
-import {MissingParameterError} from '../../../../zql/src/zql/builder/error.js';
 
 export interface WriteAuthorizer {
   canInsert(authData: JWTPayload, op: CreateOp): boolean;
@@ -89,16 +95,16 @@ export class WriteAuthorizerImpl {
     if (preMutationRow) {
       return this.canUpdate(authData, {
         op: 'update',
-        entityType: op.entityType,
-        id: op.id,
-        partialValue: op.value,
+        tableName: op.tableName,
+        primaryKey: op.primaryKey,
+        value: op.value,
       });
     }
 
     return this.canInsert(authData, {
       op: 'create',
-      entityType: op.entityType,
-      id: op.id,
+      tableName: op.tableName,
+      primaryKey: op.primaryKey,
       value: op.value,
     });
   }
@@ -145,10 +151,10 @@ export class WriteAuthorizerImpl {
         action,
         'duration:',
         performance.now() - start,
-        'entityType:',
-        op.entityType,
-        'id:',
-        op.id,
+        'tableName:',
+        op.tableName,
+        'primaryKey:',
+        op.primaryKey,
       );
     }
   }
@@ -168,7 +174,7 @@ export class WriteAuthorizerImpl {
     authData: JWTPayload,
     op: ActionOpMap[A],
   ) {
-    const rules = this.#authorizationConfig[op.entityType];
+    const rules = this.#authorizationConfig[op.tableName];
     if (!rules) {
       return true;
     }
@@ -216,9 +222,22 @@ export class WriteAuthorizerImpl {
   }
 
   #getPreMutationRow(op: SetOp | UpdateOp | DeleteOp) {
+    const {value} = op;
+    const conditions: SQLQuery[] = [];
+    const values: PrimaryKeyValue[] = [];
+    for (const pk of op.primaryKey) {
+      conditions.push(sql`${sql.ident(pk)}=?`);
+      values.push(v.parse(value[pk], primaryKeyValueSchema));
+    }
+
     return this.#statementCache.use(
-      compile(sql`SELECT * FROM ${sql.ident(op.entityType)} WHERE id = ?`),
-      stmt => stmt.statement.get<Row | undefined>(op.id.id),
+      compile(
+        sql`SELECT * FROM ${sql.ident(op.tableName)} WHERE ${sql.join(
+          conditions,
+          sql` AND `,
+        )}`,
+      ),
+      stmt => stmt.statement.get<Row | undefined>(...values),
     );
   }