renamed syncUpdateCondition to shouldUpdateRecord and moved it before…

… conflictResolver where appropriate, added tests

src/sync/impl/__tests__/applyRemote.test.js

@@ -506,4 +506,86 @@ describe('applyRemoteChanges', () => {
     //   Record ID mock_tasks#tSynced was sent over the bridge, but it's not cached
     await database.get('mock_tasks').find('tSynced')
   })
+  describe('shouldUpdateRecord', () => {
+    it('can ignore record', async () => {
+      const { database, tasks } = makeDatabase()
+
+      await makeLocalChanges(database)
+      await testApplyRemoteChanges(database, {
+        mock_tasks: {
+          updated: [
+            // update / updated - will be ignored when any local change
+            { id: 'tUpdated', name: 'remote', description: 'remote' },
+          ],
+        },
+      }, {
+        shouldUpdateRecord: (_table, local, _remote) => {
+          return local._status !== 'updated'
+        },
+      })
+
+      await expectSyncedAndMatches(tasks, 'tUpdated', {
+        _status: 'updated',
+        _changed: 'name,position',
+        name: 'local', // local change preserved
+        position: 100,
+        description: 'orig', // orig should be preserved
+        project_id: 'orig', // unchanged
+      })
+    })
+    it('can still update', async () => {
+      const { database, tasks } = makeDatabase()
+
+      await makeLocalChanges(database)
+      await testApplyRemoteChanges(database, {
+        mock_tasks: {
+          updated: [
+            // update / updated - should not be ignored when local wasn't changed
+            { id: 'tSynced', name: 'remote', description: 'remote' },
+          ],
+        },
+      }, {
+        shouldUpdateRecord: (_table, local, _remote) => {
+          return local._status !== 'updated'
+        },
+      })
+
+      await expectSyncedAndMatches(tasks, 'tSynced', {
+        _status: 'synced',
+        name: 'remote', // remote change
+        description: 'remote', // remote change
+      })
+    })
+  })
+  describe('conflictResolver', () => {
+    it('can account for conflictResolver', async () => {
+      const { database, tasks } = makeDatabase()
+
+      await makeLocalChanges(database)
+      await testApplyRemoteChanges(database, {
+        mock_tasks: {
+          updated: [
+            // update / updated - resolve and update (description is concat of local/remote on change)
+            { id: 'tUpdated', name: 'remote', description: 'remote' },
+          ],
+        },
+      }, {
+        conflictResolver: (_table, local, remote, resolved) => {
+          if (local.name !== remote.name) {
+            resolved.name = `${remote.name} ${local.name}`
+          }
+          return resolved
+        },
+      })
+
+      await expectSyncedAndMatches(tasks, 'tUpdated', {
+        _status: 'updated',
+        _changed: 'name,position',
+        name: 'remote local', // concat of remote and local change
+        position: 100,
+        description: 'remote', // remote change
+        project_id: 'orig', // unchanged
+      })
+    })
+  })
 })

src/sync/impl/applyRemote.d.ts

@@ -4,15 +4,15 @@ import type {
   SyncDatabaseChangeSet,
   SyncLog,
   SyncConflictResolver,
-  SyncUpdateCondition,
+  SyncShouldUpdateRecord,
 } from '../index'
 
 export default function applyRemoteChanges(
   db: Database,
   remoteChanges: SyncDatabaseChangeSet,
   sendCreatedAsUpdated: boolean,
   log?: SyncLog,
+  shouldUpdateRecord?: SyncShouldUpdateRecord,
   conflictResolver?: SyncConflictResolver,
   _unsafeBatchPerCollection?: boolean,
-  syncUpdateCondition?: SyncUpdateCondition,
 ): Promise<void>

src/sync/impl/applyRemote.js

@@ -23,7 +23,7 @@ import type {
   SyncDatabaseChangeSet,
   SyncLog,
   SyncConflictResolver,
-  SyncUpdateCondition,
+  SyncShouldUpdateRecord,
   SyncPullStrategy,
 } from '../index'
 import { prepareCreateFromRaw, prepareUpdateFromRaw, recordFromRaw } from './helpers'
@@ -33,9 +33,9 @@ type ApplyRemoteChangesContext = $Exact<{
   strategy?: ?SyncPullStrategy,
   sendCreatedAsUpdated?: boolean,
   log?: SyncLog,
+  shouldUpdateRecord?: SyncShouldUpdateRecord,
   conflictResolver?: SyncConflictResolver,
   _unsafeBatchPerCollection?: boolean,
-  syncUpdateCondition?: SyncUpdateCondition,
 }>
 
 // NOTE: Creating JS models is expensive/memory-intensive, so we want to avoid it if possible
@@ -265,7 +265,7 @@ function prepareApplyRemoteChangesToCollection<T: Model>(
   collection: Collection<T>,
   context: ApplyRemoteChangesContext,
 ): Array<?T> {
-  const { db, sendCreatedAsUpdated, log, conflictResolver, syncUpdateCondition } = context
+  const { db, sendCreatedAsUpdated, log, conflictResolver, shouldUpdateRecord } = context
   const { table } = collection
   const {
     created,
@@ -299,8 +299,8 @@ function prepareApplyRemoteChangesToCollection<T: Model>(
           raw,
           collection,
           log,
+          shouldUpdateRecord,
           conflictResolver,
-          syncUpdateCondition,
         ),
       )
     } else if (locallyDeletedIds.includes(raw.id)) {
@@ -326,8 +326,8 @@ function prepareApplyRemoteChangesToCollection<T: Model>(
           raw,
           collection,
           log,
+          shouldUpdateRecord,
           conflictResolver,
-          syncUpdateCondition,
         ),
       )
     } else if (locallyDeletedIds.includes(raw.id)) {

src/sync/impl/helpers.d.ts

@@ -4,7 +4,7 @@ import type {
   SyncLog,
   SyncDatabaseChangeSet,
   SyncConflictResolver,
-  SyncUpdateCondition,
+  SyncShouldUpdateRecord,
 } from '../index'
 
 // Returns raw record with naive solution to a conflict based on local `_changed` field
@@ -21,8 +21,8 @@ export function prepareUpdateFromRaw<T extends Model = Model>(
   record: T,
   updatedDirtyRaw: DirtyRaw,
   log?: SyncLog,
+  shouldUpdateRecord?: SyncShouldUpdateRecord,
   conflictResolver?: SyncConflictResolver,
-  syncUpdateCondition?: SyncUpdateCondition,
 ): T
 
 export function prepareMarkAsSynced<T extends Model = Model>(record: T): T

src/sync/impl/helpers.js

@@ -9,8 +9,8 @@ import { type RawRecord, type DirtyRaw, sanitizedRaw } from '../../RawRecord'
 import type {
   SyncLog,
   SyncDatabaseChangeSet,
+  SyncShouldUpdateRecord,
   SyncConflictResolver,
-  SyncUpdateCondition,
 } from '../index'
 
 // Returns raw record with naive solution to a conflict based on local `_changed` field
@@ -64,10 +64,10 @@ export function requiresUpdate<T: Model>(
   collection: Collection<T>,
   local: RawRecord,
   dirtyRemote: DirtyRaw,
-  syncUpdateCondition?: SyncUpdateCondition,
+  shouldUpdateRecord?: SyncShouldUpdateRecord,
 ): boolean {
-  if (syncUpdateCondition) {
-    return syncUpdateCondition(collection.table, local, dirtyRemote)
+  if (shouldUpdateRecord) {
+    return shouldUpdateRecord(collection.table, local, dirtyRemote)
   }
 
   if (local._status !== 'synced') {
@@ -89,10 +89,10 @@ export function prepareUpdateFromRaw<T: Model>(
   remoteDirtyRaw: DirtyRaw,
   collection: Collection<T>,
   log: ?SyncLog,
+  shouldUpdateRecord?: SyncShouldUpdateRecord,
   conflictResolver?: SyncConflictResolver,
-  syncUpdateCondition?: SyncUpdateCondition,
 ): ?T {
-  if (!requiresUpdate(collection, localRaw, remoteDirtyRaw, syncUpdateCondition)) {
+  if (!requiresUpdate(collection, localRaw, remoteDirtyRaw, shouldUpdateRecord)) {
     return null
   }
 

src/sync/impl/synchronize.d.ts

@@ -8,6 +8,7 @@ export default function synchronize({
   sendCreatedAsUpdated,
   migrationsEnabledAtVersion,
   log,
+  shouldUpdateRecord,
   conflictResolver,
   _unsafeBatchPerCollection,
   unsafeTurbo,

src/sync/impl/synchronize.js

@@ -23,10 +23,10 @@ export default async function synchronize({
   sendCreatedAsUpdated = false,
   migrationsEnabledAtVersion,
   log,
+  shouldUpdateRecord,
   conflictResolver,
   _unsafeBatchPerCollection,
   unsafeTurbo,
-  syncUpdateCondition,
 }: SyncArgs): Promise<void> {
   const resetCount = database._resetCount
   log && (log.startedAt = new Date())
@@ -106,9 +106,9 @@ export default async function synchronize({
         strategy: ((pullResult: any).experimentalStrategy: ?SyncPullStrategy),
         sendCreatedAsUpdated,
         log,
+        shouldUpdateRecord,
         conflictResolver,
         _unsafeBatchPerCollection,
-        syncUpdateCondition,
       })
       onDidPullChanges && onDidPullChanges(resultRest)
     }

src/sync/index.d.ts

@@ -49,7 +49,7 @@ export type SyncLog = {
   error?: Error;
 }
 
-export type SyncUpdateCondition = (
+export type SyncShouldUpdateRecord = (
   table: TableName<any>,
   local: DirtyRaw,
   remote: DirtyRaw,
@@ -72,7 +72,7 @@ export type SyncArgs = $Exact<{
   log?: SyncLog;
   // Advanced (unsafe) customization point. Useful when doing per record conflict resolution and can
   // determine directly from remote and local if we can keep local.
-  syncUpdateCondition?: SyncUpdateCondition;
+  shouldUpdateRecord?: SyncShouldUpdateRecord;
   // Advanced (unsafe) customization point. Useful when you have subtle invariants between multiple
   // columns and want to have them updated consistently, or to implement partial sync
   // It's called for every record being updated locally, so be sure that this function is FAST.

src/sync/index.js

@@ -75,7 +75,7 @@ export type SyncLog = {
   error?: Error,
 }
 
-export type SyncUpdateCondition = (
+export type SyncShouldUpdateRecord = (
   table: TableName<any>,
   local: DirtyRaw,
   remote: DirtyRaw,
@@ -99,7 +99,7 @@ export type SyncArgs = $Exact<{
   log?: SyncLog,
   // Advanced (unsafe) customization point. Useful when doing per record conflict resolution and can
   // determine directly from remote and local if we can keep local.
-  syncUpdateCondition?: SyncUpdateCondition,
+  shouldUpdateRecord?: SyncShouldUpdateRecord,
   // Advanced (unsafe) customization point. Useful when you have subtle invariants between multiple
   // columns and want to have them updated consistently, or to implement partial sync
   // It's called for every record being updated locally, so be sure that this function is FAST.