fix: Add proper suppor to the noReturn parameter on the ORM (#92)

packages/serverpod_offline_sync_client/lib/src/database/database.dart

@@ -323,20 +323,37 @@ class CrdtDatabase implements Database {
   }) async {
     if (rows.isEmpty) return [];
     await _ensureInitialized();
+    if (!_recorder.isCrdtTracked<T>(rows.first.table)) {
+      return _delegate.insert<T>(
+        rows,
+        transaction: transaction,
+        ignoreConflicts: ignoreConflicts,
+        noReturn: noReturn,
+      );
+    }
     return DatabaseUtil.runInTransactionOrSavepoint(
       _delegate,
       transaction,
       (tx) async {
         final prepared = _prepareRowsForInsert(rows, tx);
+        // Tracked rows can only skip RETURNING when the prepared rows are
+        // exactly what gets inserted: every id is caller-provided and a
+        // conflict throws instead of silently dropping rows.
+        final skipReturn =
+            noReturn && !ignoreConflicts && !rows.any((row) => row.id == null);
+
         final result = await _delegate.insert<T>(
           prepared.rows,
           transaction: tx,
           ignoreConflicts: ignoreConflicts,
+          noReturn: skipReturn,
         );
 
-        await _recorder.afterInsert<T>(result, tx);
+        final insertedRows = skipReturn ? prepared.rows : result;
-        _stripStampedRows(result, prepared);
+        await _recorder.afterInsert<T>(insertedRows, tx);
-        return result;
+        if (noReturn) return <T>[];
+        _stripStampedRows(insertedRows, prepared);
+        return insertedRows;
       },
     );
   }
@@ -417,6 +434,14 @@ class CrdtDatabase implements Database {
   }) async {
     if (rows.isEmpty) return [];
     await _ensureInitialized();
+    if (!_recorder.isCrdtTracked<T>(rows.first.table)) {
+      return _delegate.update<T>(
+        rows,
+        columns: columns,
+        transaction: transaction,
+        noReturn: noReturn,
+      );
+    }
     return DatabaseUtil.runInTransactionOrSavepoint(
       _delegate,
       transaction,
@@ -432,7 +457,7 @@ class CrdtDatabase implements Database {
         ];
 
         await _recorder.afterUpdate(updatedRows, columns, tx);
-        return updatedRows;
+        return noReturn ? <T>[] : updatedRows;
       },
     );
   }
@@ -530,10 +555,23 @@ class CrdtDatabase implements Database {
     bool noReturn = false,
   }) async {
     await _ensureInitialized();
-    if (_recorder.isCrdtTracked<T>()) {
+    if (!_recorder.isCrdtTracked<T>()) {
-      _assertNoScopeIdColumnValues<T>(columnValues);
+      return _delegate.updateWhere<T>(
+        columnValues: columnValues,
+        where: where,
+        limit: limit,
+        offset: offset,
+        orderBy: orderBy,
+        orderByList: orderByList,
+        // Remove this once the deprecated member is removed.
+        // ignore: deprecated_member_use
+        orderDescending: orderDescending,
+        transaction: transaction,
+        noReturn: noReturn,
+      );
     }
 
+    _assertNoScopeIdColumnValues<T>(columnValues);
     return DatabaseUtil.runInTransactionOrSavepoint(
       _delegate,
       transaction,
@@ -558,9 +596,8 @@ class CrdtDatabase implements Database {
 
         final columns = columnValues.map((e) => e.column).toList();
         await _recorder.afterUpdate(result, columns, tx);
-        if (_recorder.isCrdtTracked<T>()) {
+        if (noReturn) return <T>[];
-          result.forEach(_stripScopeId);
+        result.forEach(_stripScopeId);
-        }
         return result;
       },
     );
@@ -587,6 +624,7 @@ class CrdtDatabase implements Database {
       // ignore: deprecated_member_use
       orderDescending: orderDescending,
       transaction: transaction,
+      noReturn: noReturn,
     );
   }
 
@@ -629,6 +667,7 @@ class CrdtDatabase implements Database {
         // ignore: deprecated_member_use
         orderDescending: orderDescending,
         transaction: transaction,
+        noReturn: noReturn,
       );
     }
 
@@ -652,6 +691,7 @@ class CrdtDatabase implements Database {
         );
 
         await _recorder.insteadOfDelete<T>(rows, tx);
+        if (noReturn) return <T>[];
         return _stripScopeIdFromScopedRead(rows, null, tx);
       },
     );

test/serverpod_offline_sync_test_server/test/integration/crud/delete_test.dart

@@ -56,6 +56,62 @@ void main() {
         expect(deletedPerson.scopeId, isNull);
       });
     });
+
+    group('when deleting the Person with delete and noReturn,', () {
+      late List<Person> deleted;
+
+      setUp(() async {
+        deleted = await session.db.transactionForUser(
+          testCrdtUserId,
+          (tx) => Person.db.delete(
+            session,
+            [person],
+            transaction: tx,
+            noReturn: true,
+          ),
+        );
+      });
+
+      test('then an empty list is returned.', () async {
+        expect(deleted, isEmpty);
+      });
+
+      test('then a CRDT tombstone is created for the row.', () async {
+        final tombstone = await CrdtDataDeleted.db.findFirstRow(
+          session,
+          where: (t) => t.row.uuidRowId.equals(person.id),
+        );
+        expect(tombstone?.isDeleted, isTrue);
+      });
+    });
+
+    group('when deleting the Person with deleteWhere and noReturn,', () {
+      late List<Person> deleted;
+
+      setUp(() async {
+        deleted = await session.db.transactionForUser(
+          testCrdtUserId,
+          (tx) => Person.db.deleteWhere(
+            session,
+            where: (t) => t.id.equals(person.id),
+            transaction: tx,
+            noReturn: true,
+          ),
+        );
+      });
+
+      test('then an empty list is returned.', () async {
+        expect(deleted, isEmpty);
+      });
+
+      test('then a CRDT tombstone is created for the row.', () async {
+        final tombstone = await CrdtDataDeleted.db.findFirstRow(
+          session,
+          where: (t) => t.row.uuidRowId.equals(person.id),
+        );
+        expect(tombstone?.isDeleted, isTrue);
+      });
+    });
   });
 
   group('Given a person table with a deleted row,', () {

test/serverpod_offline_sync_test_server/test/integration/crud/insert_test.dart

@@ -172,6 +172,79 @@ void main() {
         expect(crdt.map((e) => e.uuidRowId).toSet(), insertedIds);
       });
     });
+
+    group('when inserting a Person with a caller-provided id and noReturn,', () {
+      late UuidValue id;
+      late List<Person> inserted;
+
+      setUp(() async {
+        id = const Uuid().v7obj();
+        inserted = await session.db.transactionForUser(
+          testCrdtUserId,
+          (tx) => Person.db.insert(
+            session,
+            [Person(id: id, name: 'fixed')],
+            transaction: tx,
+            noReturn: true,
+          ),
+        );
+      });
+
+      test('then an empty list is returned.', () async {
+        expect(inserted, isEmpty);
+      });
+
+      test('then the row exists in the person table.', () async {
+        final row = await Person.db.findById(session, id);
+        expect(row?.name, 'fixed');
+      });
+
+      test('then CRDT metadata is recorded for the row.', () async {
+        final crdtRow = await CrdtDataRow.db.findFirstRow(
+          session,
+          where: (t) => t.uuidRowId.equals(id),
+        );
+        expect(crdtRow, isNotNull);
+      });
+    });
+
+    group('when inserting a Person without a caller-provided id and noReturn,', () {
+      late List<Person> inserted;
+      late Person? row;
+
+      setUp(() async {
+        inserted = await session.db.transactionForUser(
+          testCrdtUserId,
+          (tx) => Person.db.insert(
+            session,
+            [Person(name: 'generated')],
+            transaction: tx,
+            noReturn: true,
+          ),
+        );
+
+        row = await Person.db.findFirstRow(
+          session,
+          where: (t) => t.name.equals('generated'),
+        );
+      });
+
+      test('then an empty list is returned.', () async {
+        expect(inserted, isEmpty);
+      });
+
+      test('then the row exists in the person table.', () async {
+        expect(row, isNotNull);
+      });
+
+      test('then CRDT metadata is recorded for the row.', () async {
+        final crdtRow = await CrdtDataRow.db.findFirstRow(
+          session,
+          where: (t) => t.uuidRowId.equals(row!.id),
+        );
+        expect(crdtRow, isNotNull);
+      });
+    });
   });
 
   group('Given a person row with a fixed id,', () {

test/serverpod_offline_sync_test_server/test/integration/crud/update_test.dart

@@ -194,6 +194,67 @@ void main() {
         );
       },
     );
+
+    group('when updating the Person with update and noReturn,', () {
+      late List<Person> updated;
+
+      setUp(() async {
+        updated = await session.db.transactionForUser(
+          testCrdtUserId,
+          (tx) => Person.db.update(
+            session,
+            [person.copyWith(name: 'updated')],
+            columns: (t) => [t.name],
+            transaction: tx,
+            noReturn: true,
+          ),
+        );
+      });
+
+      test('then an empty list is returned.', () async {
+        expect(updated, isEmpty);
+      });
+
+      test('then the person row reflects the new values.', () async {
+        final row = await Person.db.findById(session, person.id!);
+        expect(row?.name, 'updated');
+      });
+
+      test('then a CRDT field is created for the name column.', () async {
+        final field = await CrdtDataField.db.findFirstRow(
+          session,
+          where: (t) => t.row.uuidRowId.equals(person.id),
+          include: CrdtDataField.include(column: CrdtSchemaColumn.include()),
+        );
+        expect(field?.column?.name, 'name');
+      });
+    });
+
+    group('when updating the Person with updateWhere and noReturn,', () {
+      late List<Person> updated;
+
+      setUp(() async {
+        updated = await session.db.transactionForUser(
+          testCrdtUserId,
+          (tx) => Person.db.updateWhere(
+            session,
+            columnValues: (t) => [t.name('updated')],
+            where: (t) => t.id.equals(person.id),
+            transaction: tx,
+            noReturn: true,
+          ),
+        );
+      });
+
+      test('then an empty list is returned.', () async {
+        expect(updated, isEmpty);
+      });
+
+      test('then the person row reflects the new values.', () async {
+        final row = await Person.db.findById(session, person.id!);
+        expect(row?.name, 'updated');
+      });
+    });
   });
 
   group('Given a person table with a deleted row,', () {
@@ -513,4 +574,73 @@ void main() {
       });
     });
   });
+
+  group('Given a row on a table not tracked by CRDT,', () {
+    late CrdtSyncIntegrityViolation violation;
+
+    setUp(() async {
+      violation = await CrdtSyncIntegrityViolation.db.insertRow(
+        session,
+        CrdtSyncIntegrityViolation(
+          type: CrdtSyncViolationType.ownershipCollision,
+          domainTableName: Person.t.tableName,
+          uuidRowId: const Uuid().v7obj(),
+          incomingScopeUuid: testCrdtUserId,
+          operation: CrdtSyncViolationOperation.mergeInsert,
+          firstSeenAt: DateTime.now(),
+          lastSeenAt: DateTime.now(),
+          occurrences: 1,
+        ),
+      );
+    });
+
+    group('when updating the row with update and noReturn,', () {
+      late List<CrdtSyncIntegrityViolation> updated;
+
+      setUp(() async {
+        updated = await CrdtSyncIntegrityViolation.db.update(
+          session,
+          [violation.copyWith(occurrences: 2)],
+          noReturn: true,
+        );
+      });
+
+      test('then an empty list is returned.', () async {
+        expect(updated, isEmpty);
+      });
+
+      test('then the row reflects the new values.', () async {
+        final row = await CrdtSyncIntegrityViolation.db.findById(
+          session,
+          violation.id!,
+        );
+        expect(row?.occurrences, 2);
+      });
+    });
+
+    group('when updating the row with updateWhere and noReturn,', () {
+      late List<CrdtSyncIntegrityViolation> updated;
+
+      setUp(() async {
+        updated = await CrdtSyncIntegrityViolation.db.updateWhere(
+          session,
+          columnValues: (t) => [t.occurrences(3)],
+          where: (t) => t.id.equals(violation.id),
+          noReturn: true,
+        );
+      });
+
+      test('then an empty list is returned.', () async {
+        expect(updated, isEmpty);
+      });
+
+      test('then the row reflects the new values.', () async {
+        final row = await CrdtSyncIntegrityViolation.db.findById(
+          session,
+          violation.id!,
+        );
+        expect(row?.occurrences, 3);
+      });
+    });
+  });
 }