Type extracted types. Delete triggers should store previous values.

Author: stevensJourney

packages/common/src/client/triggers/TriggerManager.ts

@@ -31,7 +31,7 @@ export interface BaseTriggerDiffRecord {
 export interface TriggerDiffUpdateRecord extends BaseTriggerDiffRecord {
   operation: DiffTriggerOperation.UPDATE;
   value: string;
-  previous_value?: string;
+  previous_value: string;
 }
 
 /**
@@ -57,6 +57,21 @@ export interface TriggerDiffDeleteRecord extends BaseTriggerDiffRecord {
  */
 export type TriggerDiffRecord = TriggerDiffUpdateRecord | TriggerDiffInsertRecord | TriggerDiffDeleteRecord;
 
+/**
+ * Querying the DIFF table directly with {@link TriggerDiffHandlerContext#withExtractedDiff} will return records
+ * with the tracked columns extracted from the JSON value.
+ * This type represents the structure of such records.
+ * @example
+ * ```typescript
+ * const diffs = await context.withExtractedDiff<ExtractedTriggerDiffRecord<{id: string, name: string}>>('SELECT * FROM DIFF');
+ * ```
+ */
+export type ExtractedTriggerDiffRecord<T> = T & {
+  [K in keyof Omit<BaseTriggerDiffRecord, 'id'> as `__${string & K}`]: TriggerDiffRecord[K];
+} & {
+  __previous_value?: string;
+};
+
 /**
  * Hooks used in the creation of a table diff trigger.
  */
@@ -131,7 +146,7 @@ export interface TriggerDiffHandlerContext extends LockContext {
   /**
    * The name of the temporary destination table created by the trigger.
    */
-  destination_table: string;
+  destinationTable: string;
 
   /**
    * Allows querying the database with access to the table containing diff records.
@@ -158,7 +173,7 @@ export interface TriggerDiffHandlerContext extends LockContext {
    * WHERE json_extract(DIFF.value, '$.status') = 'active'
    * ```
    */
-  withDiff: <T = any>(query: string, params?: any[]) => Promise<T[]>;
+  withDiff: <T = any>(query: string, params?: ReadonlyArray<Readonly<any>>) => Promise<T[]>;
 
   /**
    * Allows querying the database with access to the table containing diff records.
@@ -189,7 +204,7 @@ export interface TriggerDiffHandlerContext extends LockContext {
    * WHERE DIFF.name = 'example'
    * ```
    */
-  withExtractedDiff: <T = any>(query: string, params?: any[]) => Promise<T[]>;
+  withExtractedDiff: <T = any>(query: string, params?: ReadonlyArray<Readonly<any>>) => Promise<T[]>;
 }
 
 /**

packages/common/src/client/triggers/TriggerManagerImpl.ts

@@ -173,12 +173,13 @@ export class TriggerManagerImpl implements TriggerManager {
             DiffTriggerOperation.DELETE
           ]} BEGIN
           INSERT INTO
-            ${destination} (id, operation, timestamp)
+            ${destination} (id, operation, timestamp, value)
           VALUES
             (
-              NEW.id,
+              OLD.id,
               'DELETE',
-              strftime ('%Y-%m-%dT%H:%M:%fZ', 'now')
+              strftime ('%Y-%m-%dT%H:%M:%fZ', 'now'),
+              OLD.data
             );
 
           END;
@@ -231,7 +232,7 @@ export class TriggerManagerImpl implements TriggerManager {
           await this.db.writeTransaction(async (tx) => {
             const callbackResult = await options.onChange({
               ...tx,
-              destination_table: destination,
+              destinationTable: destination,
               withDiff: async <T>(query, params) => {
                 // Wrap the query to expose the destination table
                 const wrappedQuery = /* sql */ `
@@ -241,6 +242,8 @@ export class TriggerManagerImpl implements TriggerManager {
                         *
                       FROM
                         ${destination}
+                      ORDER BY
+                        timestamp ASC
                     ) ${query}
                 `;
                 return tx.getAll<T>(wrappedQuery, params);
@@ -258,6 +261,8 @@ export class TriggerManagerImpl implements TriggerManager {
                         previous_value as __previous_value
                       FROM
                         ${destination}
+                      ORDER BY
+                        __timestamp ASC
                     ) ${query}
                 `;
                 return tx.getAll<T>(wrappedQuery, params);

packages/node/tests/trigger.test.ts

@@ -1,4 +1,4 @@
-import { DiffTriggerOperation, TriggerDiffRecord } from '@powersync/common';
+import { DiffTriggerOperation, ExtractedTriggerDiffRecord, TriggerDiffRecord } from '@powersync/common';
 // import 'source-map-support/register';
 import { describe, expect, vi } from 'vitest';
 import { Database, databaseTest } from './utils';
@@ -15,15 +15,15 @@ describe('Triggers', () => {
       source: 'lists',
       destination: tempTable,
       columns: ['name'],
-      operations: [DiffTriggerOperation.INSERT, DiffTriggerOperation.UPDATE]
+      operations: [DiffTriggerOperation.INSERT, DiffTriggerOperation.UPDATE, DiffTriggerOperation.DELETE]
     });
 
     const results = [] as TriggerDiffRecord[];
 
     database.onChange(
       {
         // This callback async processed. Invocations are sequential.
-        onChange: async (change) => {
+        onChange: async () => {
           await database.writeLock(async (tx) => {
             // API exposes a context to run things here.
             // using execute seems to be important on Node.js
@@ -48,13 +48,15 @@ describe('Triggers', () => {
     // Do some changes to the source table
     await database.execute('INSERT INTO lists (id, name) VALUES (uuid(), ?);', ['test list']);
     await database.execute(`UPDATE lists SET name = 'wooo'`);
+    await database.execute('DELETE FROM lists WHERE name = ?', ['wooo']);
 
     // Wait for the changes to be processed and results to be collected
     await vi.waitFor(
       () => {
-        expect(results.length).toEqual(2);
+        expect(results.length).toEqual(3);
         expect(results[0].operation).toEqual('INSERT');
         expect(results[1].operation).toEqual('UPDATE');
+        expect(results[2].operation).toEqual('DELETE');
       },
       { timeout: 1000 }
     );
@@ -63,7 +65,7 @@ describe('Triggers', () => {
   /**
    * Uses the automatic handlers for triggers to track changes.
    */
-  databaseTest('Should be able to handle table inserts', async ({ database }) => {
+  databaseTest('Should be able to track table inserts', async ({ database }) => {
     await database.execute(
       /* sql */ `
         INSERT INTO
@@ -98,8 +100,8 @@ describe('Triggers', () => {
           SELECT
             todos.*
           FROM
-            diff
-            JOIN todos ON diff.id = todos.id
+            DIFF
+            JOIN todos ON DIFF.id = todos.id
         `);
 
         results.push(...newTodos);
@@ -124,7 +126,7 @@ describe('Triggers', () => {
       () => {
         expect(results.length).toEqual(1);
       },
-      { timeout: 10000 }
+      { timeout: 1000 }
     );
 
     // Do further inserts
@@ -144,11 +146,11 @@ describe('Triggers', () => {
       () => {
         expect(results.length).toEqual(2);
       },
-      { timeout: 10000 }
+      { timeout: 1000 }
     );
   });
 
-  databaseTest('Should be able to handle table updates', async ({ database }) => {
+  databaseTest('Should be able to track table updates', async ({ database }) => {
     const { rows } = await database.execute(
       /* sql */ `
         INSERT INTO
@@ -161,18 +163,18 @@ describe('Triggers', () => {
 
     const list = rows!.item(0) as Database['lists'];
 
-    const changes: Database['lists'][] = [];
+    const changes: ExtractedTriggerDiffRecord<Database['lists']>[] = [];
 
     /**
      * Watch the todos table for changes. Only track the diff for rows belonging to the first list.
      */
     await database.triggers.trackTableDiff({
       source: 'lists',
-      when: { [DiffTriggerOperation.INSERT]: `NEW.id = '${list.id}'` },
-      operations: [DiffTriggerOperation.UPDATE],
+      when: { [DiffTriggerOperation.UPDATE]: `NEW.id = '${list.id}'` },
+      operations: [DiffTriggerOperation.UPDATE, DiffTriggerOperation.DELETE],
       onChange: async (context) => {
         // Fetches the todo records that were inserted during this diff
-        const diffs = await context.withExtractedDiff<Database['lists']>(/* sql */ `
+        const diffs = await context.withExtractedDiff<ExtractedTriggerDiffRecord<Database['lists']>>(/* sql */ `
           SELECT
             *
           FROM
@@ -203,7 +205,27 @@ describe('Triggers', () => {
         expect(changes.length).toEqual(updateCount);
         expect(changes.map((c) => c.name)).toEqual(Array.from({ length: updateCount }, (_, i) => `updated ${i}`));
       },
-      { timeout: 10000 }
+      { timeout: 1000 }
+    );
+
+    // clear the items
+    await database.execute(
+      /* sql */ `
+        DELETE FROM lists
+        WHERE
+          id = ?
+      `,
+      [list.id]
+    );
+
+    await vi.waitFor(
+      () => {
+        expect(changes.length).toEqual(updateCount + 1);
+        expect(changes[changes.length - 1].__operation).eq(DiffTriggerOperation.DELETE);
+        // The delete diff should contain the previous value
+        expect(changes[changes.length - 1].name).eq(`updated ${updateCount - 1}`);
+      },
+      { timeout: 1000 }
     );
   });