Supported experimentalAcceptedIds as safer alternative to rejectedIds

When design is set that each resource must be confirmed as synced it is safer to look at it from whitelist perspective than blacklist to prevent possible miss-sync.
Nozbe · Mar 3, 2023 · 52d958a · 52d958a
1 parent 5d9efec
commit 52d958a
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 6 deletions.
diff --git a/src/sync/impl/markAsSynced.js b/src/sync/impl/markAsSynced.js
@@ -5,18 +5,21 @@ import { logError } from '../../utils/common'
 import type { Database, Model, TableName } from '../..'
 
 import { prepareMarkAsSynced } from './helpers'
-import type { SyncLocalChanges, SyncRejectedIds } from '../index'
+import type { SyncLocalChanges, SyncRejectedIds, SyncAcceptedIds } from '../index'
 
 const recordsToMarkAsSynced = (
   { changes, affectedRecords }: SyncLocalChanges,
   allRejectedIds: SyncRejectedIds,
+  allAcceptedIds: ?SyncAcceptedIds,
+  allowOnlyAcceptedIds: boolean,
 ): Model[] => {
   const syncedRecords = []
 
   Object.keys(changes).forEach((table) => {
     const { created, updated } = changes[(table: any)]
     const raws = created.concat(updated)
     const rejectedIds = new Set(allRejectedIds[(table: any)])
+    const acceptedIds = new Set(allAcceptedIds[(table: any)] || [])
 
     raws.forEach((raw) => {
       const { id } = raw
@@ -27,7 +30,8 @@ const recordsToMarkAsSynced = (
         )
         return
       }
-      if (areRecordsEqual(record._raw, raw) && !rejectedIds.has(id)) {
+      const isAccepted = (!allAcceptedIds && allowOnlyAcceptedIds) || acceptedIds.has(id);
+      if (areRecordsEqual(record._raw, raw) && !rejectedIds.has(id) && isAccepted) {
         syncedRecords.push(record)
       }
     })
@@ -39,10 +43,13 @@ const destroyDeletedRecords = (
   db: Database,
   { changes }: SyncLocalChanges,
   allRejectedIds: SyncRejectedIds,
+  allAcceptedIds: ?SyncAcceptedIds,
+  allowOnlyAcceptedIds: boolean,
 ): Promise<any>[] =>
   Object.keys(changes).map((_tableName) => {
     const tableName: TableName<any> = (_tableName: any)
     const rejectedIds = new Set(allRejectedIds[tableName])
+    const acceptedIds = new Set(allAcceptedIds[(table: any)] || [])
     const deleted = changes[tableName].deleted.filter((id) => !rejectedIds.has(id))
     return deleted.length ? db.adapter.destroyDeletedRecords(tableName, deleted) : Promise.resolve()
   })
@@ -51,14 +58,18 @@ export default function markLocalChangesAsSynced(
   db: Database,
   syncedLocalChanges: SyncLocalChanges,
   rejectedIds?: ?SyncRejectedIds,
+  allAcceptedIds: ?SyncAcceptedIds,
+  allowOnlyAcceptedIds: boolean,
 ): Promise<void> {
   return db.write(async () => {
     // update and destroy records concurrently
     await Promise.all([
       db.batch(
-        recordsToMarkAsSynced(syncedLocalChanges, rejectedIds || {}).map(prepareMarkAsSynced),
+        recordsToMarkAsSynced(syncedLocalChanges, rejectedIds || {}, allAcceptedIds,
+          allowOnlyAcceptedIds).map(prepareMarkAsSynced),
       ),
-      ...destroyDeletedRecords(db, syncedLocalChanges, rejectedIds || {}),
+      ...destroyDeletedRecords(db, syncedLocalChanges, rejectedIds || {}, allAcceptedIds,
+        allowOnlyAcceptedIds),
     ])
   }, 'sync-markLocalChangesAsSynced')
 }
diff --git a/src/sync/impl/synchronize.js b/src/sync/impl/synchronize.js
@@ -26,6 +26,7 @@ export default async function synchronize({
   conflictResolver,
   _unsafeBatchPerCollection,
   unsafeTurbo,
+  pushShouldConfirmOnlyAccepted, // TODO add
 }: SyncArgs): Promise<void> {
   const resetCount = database._resetCount
   log && (log.startedAt = new Date())
@@ -134,9 +135,14 @@ export default async function synchronize({
         (await pushChanges({ changes: localChanges.changes, lastPulledAt: newLastPulledAt })) || {}
       log && (log.phase = 'pushed')
       log && (log.rejectedIds = pushResult.experimentalRejectedIds)
+      // TODO log.acceptedIds = pushResult.experimentalAcceptedIds
+      //  or log.rejectedIds = localChanges - pushResult.experimentalAcceptedIds but can be more
+      //    expensive
+      //  or log.acceptedIds and just count with localChanges - pushResult.experimentalAcceptedIds
 
       ensureSameDatabase(database, resetCount)
-      await markLocalChangesAsSynced(database, localChanges, pushResult.experimentalRejectedIds)
+      await markLocalChangesAsSynced(database, localChanges, pushResult.experimentalRejectedIds,
+        pushResult.experimentalAcceptedIds, pushShouldConfirmOnlyAccepted)
       log && (log.phase = 'marked local changes as synced')
     }
   } else {

diff --git a/src/sync/index.d.ts b/src/sync/index.d.ts
@@ -29,9 +29,14 @@ export type SyncPullResult =
 
 export type SyncRejectedIds = { [tableName: TableName<any>]: RecordId[] }
 
+export type SyncAcceptedIds = { [tableName: TableName<any>]: RecordId[] }
+
 export type SyncPushArgs = $Exact<{ changes: SyncDatabaseChangeSet; lastPulledAt: Timestamp }>
 
-export type SyncPushResult = $Exact<{ experimentalRejectedIds?: SyncRejectedIds }>
+export type SyncPushResult = $Exact<{
+  experimentalRejectedIds?: SyncRejectedIds,
+  experimentalAcceptedIds?: SyncAcceptedIds,
+}>
 
 type SyncConflict = $Exact<{ local: DirtyRaw; remote: DirtyRaw; resolved: DirtyRaw }>
 export type SyncLog = {