fix: Author column values written by set-based updates (#147)

packages/serverpod_offline_sync/lib/src/database/database.dart

@@ -804,7 +804,12 @@ class OfflineSyncDatabase implements Database {
 
         final columns = columnValues.map((e) => e.column).toList();
         _recorder.validateAuthoredRows(result, columns);
-        await _recorder.afterUpdate(result, columns, tx);
+        await _recorder.afterUpdate(
+          result,
+          columns,
+          tx,
+          authoredColumnValues: true,
+        );
         if (noReturn) return <T>[];
         result.forEach(_stripSpaceId);
         return result;

packages/serverpod_offline_sync/lib/src/database/recorder.dart

@@ -837,11 +837,19 @@ class CrdtMutationRecorder {
   }
 
   /// Records CRDT field metadata for updated rows.
+  ///
+  /// [authoredColumnValues] marks a write that reached the domain without a
+  /// planning pass, which is the set-based `updateWhere`. Every accepted column
+  /// value it carries is authored, so it overlays the attempted value a
+  /// projection is holding for the same field. Without the overlay the pass
+  /// below would read that stale attempt as the authored value and restore it
+  /// over the write.
   Future<void> afterUpdate<T extends TableRow>(
     List<T> updatedRows,
     List<Column>? columns,
     Transaction transaction, {
     bool projectionUnchanged = false,
+    bool authoredColumnValues = false,
     Map<MergeRowKey, Map<String, Object?>> domainBeforeUpsert = const {},
   }) async {
     await _foreignKeyProjector.assertVisibleTargets(updatedRows, columns, transaction);
@@ -857,22 +865,29 @@ class CrdtMutationRecorder {
         'updated',
         transaction,
       );
-      // Only rows returned by the physical upsert were accepted by updateWhere.
       // Explicit columns author even an unchanged null; full-row passthrough
       // keeps the claim behind an unchanged displayed alternative.
       final authoredOverlays = <MergeFieldKey, Object?>{
-        for (final row in updatedRows)
-          if (domainBeforeUpsert.containsKey((tableName, row.id)))
+        if (authoredColumnValues)
+          for (final row in updatedRows)
             for (final MapEntry(key: columnName, value: value)
                 in _authoredValuesFromRow(row, columns).entries)
-              if ((domainBeforeUpsert[(tableName, row.id)]?.containsKey(columnName) ??
-                      false) &&
-                  (columns != null ||
-                      !projectionValuesEqual(
-                        domainBeforeUpsert[(tableName, row.id)]![columnName],
-                        value,
-                      )))
-                (tableName, row.id as UuidValue, columnName): value,
+              (tableName, row.id as UuidValue, columnName): value
+        else
+          // Only rows returned by the physical upsert were accepted by its
+          // `updateWhere` predicate.
+          for (final row in updatedRows)
+            if (domainBeforeUpsert.containsKey((tableName, row.id)))
+              for (final MapEntry(key: columnName, value: value)
+                  in _authoredValuesFromRow(row, columns).entries)
+                if ((domainBeforeUpsert[(tableName, row.id)]?.containsKey(columnName) ??
+                        false) &&
+                    (columns != null ||
+                        !projectionValuesEqual(
+                          domainBeforeUpsert[(tableName, row.id)]![columnName],
+                          value,
+                        )))
+                  (tableName, row.id as UuidValue, columnName): value,
       };
       _uniqueResolver.validateAuthoredFields(authoredOverlays);
       final implicitForeignKeyRepairFields = columns == null

test/serverpod_offline_sync_test_server/test/integration/sync/sync_local_projection_test.dart

@@ -463,6 +463,84 @@ void main() {
     });
   });
 
+  group('Given a merged town with a withheld mayor,', () {
+    late SyncNode node;
+    late Person mayor;
+    late Town town;
+
+    setUpAll(() async {
+      node = await syncNode(await createAdditionalTestSession(), testSyncTables);
+      mayor = Person(id: const Uuid().v7obj(), name: 'mayor');
+      town = Town(id: const Uuid().v7obj(), name: 'town', mayorId: mayor.id);
+      final hlc = Hlc(DateTime.now().toUtc(), 0, const Uuid().v7obj());
+      await node.offlineSync.db.mergeChanges([
+        CrdtMergeInsert(
+          uuidSpaceId: testCrdtUserId,
+          tableName: Town.t.tableName,
+          uuidRowId: town.id!,
+          uuidNodeId: hlc.nodeId,
+          hlcDatetime: hlc.datetime,
+          hlcCounter: hlc.counter,
+          data: town,
+        ),
+      ], spaceId: testCrdtUserId);
+    });
+
+    group('when a set-based write clears the mayor,', () {
+      setUpAll(() async {
+        // A narrowed write authors the column it names, so it replaces the claim
+        // the projection is holding rather than passing over it. The column
+        // already displays null, which is exactly when a write that did not
+        // author would leave the withheld mayor to reappear later.
+        await node.offlineSync.db.transactionForUser(
+          testCrdtUserId,
+          (tx) => Town.db.updateWhere(
+            node.offlineSync,
+            columnValues: (t) => [t.mayorId(null)],
+            where: (t) => t.id.equals(town.id),
+            transaction: tx,
+          ),
+        );
+      });
+
+      test('then the town no longer holds a withheld mayor.', () async {
+        expect(
+          await attemptedValue(
+            rowId: town.id!,
+            columnName: 'mayorId',
+            databaseSession: node.offlineSync,
+          ),
+          isNull,
+        );
+      });
+    });
+
+    group(
+      'when a set-based write clears the mayor and the missing person is inserted locally,',
+      () {
+        setUpAll(() async {
+          await node.offlineSync.db.transactionForUser(
+            testCrdtUserId,
+            (tx) => Town.db.updateWhere(
+              node.offlineSync,
+              columnValues: (t) => [t.mayorId(null)],
+              where: (t) => t.id.equals(town.id),
+              transaction: tx,
+            ),
+          );
+          await node.offlineSync.db.transactionForUser(
+            testCrdtUserId,
+            (tx) => Person.db.insertRow(node.offlineSync, mayor, transaction: tx),
+          );
+        });
+
+        test('then the cleared mayor stays cleared.', () async {
+          expect((await Town.db.findById(node.offlineSync, town.id!))!.mayorId, isNull);
+        });
+      },
+    );
+  });
+
   group(
     'Given two merged unique children waiting for unavailable parents and a missing fallback,',
     () {

test/serverpod_offline_sync_test_server/test/integration/sync/sync_unique_projection_contract_test.dart

@@ -231,6 +231,91 @@ void main() {
           expect((await Unique.db.findById(session, winner.id!))!.name, 'shared-name');
         },
       );
+
+      test(
+        'when a set-based write gives the loser a free name, '
+        'then the write is authored and the released claim is not restored.',
+        () async {
+          await session.db.transactionForUser(
+            testCrdtUserId,
+            (tx) => Unique.db.updateWhere(
+              session,
+              columnValues: (t) => [t.name('renamed')],
+              where: (t) => t.id.equals(loser.id),
+              transaction: tx,
+            ),
+          );
+
+          expect((await Unique.db.findById(session, loser.id!))!.name, 'renamed');
+          expect(
+            await attemptedValue(
+              rowId: loser.id!,
+              columnName: Unique.t.name.columnName,
+            ),
+            isNull,
+          );
+          expect((await Unique.db.findById(session, winner.id!))!.name, 'shared-name');
+        },
+      );
+    },
+  );
+
+  group(
+    'Given a nullable unique loser whose claim was released to null, ',
+    () {
+      late UniqueNullable winner;
+      late UniqueNullable loser;
+
+      setUp(() async {
+        winner = await session.db.transactionForUser(
+          testCrdtUserId,
+          (tx) => UniqueNullable.db.insertRow(
+            session,
+            UniqueNullable(id: const Uuid().v7obj(), value: 1),
+            transaction: tx,
+          ),
+        );
+        loser = UniqueNullable(id: const Uuid().v7obj(), value: 1);
+        await mergeIndependentInsert(
+          session,
+          loser,
+          space: testCrdtUserId,
+          tables: [UniqueNullable.t],
+        );
+      });
+
+      test(
+        'when a set-based write authors null on both rows, '
+        'then the loser keeps the written null rather than reclaiming its value.',
+        () async {
+          // The loser displays the null its release wrote, so writing null is
+          // an unchanged domain value. A narrowed write authors it anyway:
+          // otherwise the freed claim behind it would win the reprojection the
+          // winner's own null triggers, and reappear as a value nobody wrote.
+          await session.db.transactionForUser(
+            testCrdtUserId,
+            (tx) => UniqueNullable.db.updateWhere(
+              session,
+              columnValues: (t) => [t.value(null)],
+              where: (t) => t.id.inSet(<UuidValue>{winner.id!, loser.id!}),
+              transaction: tx,
+            ),
+          );
+
+          expect((await UniqueNullable.db.findById(session, loser.id!))!.value, isNull);
+          expect(
+            await attemptedValue(
+              rowId: loser.id!,
+              columnName: UniqueNullable.t.value.columnName,
+            ),
+            isNull,
+          );
+          expect(
+            (await UniqueNullable.db.findById(session, winner.id!))!.value,
+            isNull,
+          );
+        },
+      );
     },
   );