DB_Conflict (409): do age comparison before merging, avoid unneeded changes

Instead of always assuming that the incoming item is meant to win,
look at the age of the two items first.

Also tell the code which updates the DB item whether it was changed at
all and skip the DB write if it is not needed. Note that statistics
are still wrong: a server add is counted as "item added" in all cases,
even if it is turned into an update or becomes a nop.

2 files changed, 53 insertions, 14 deletions

diff --git a/src/sysync/customimplds.cpp b/src/sysync/customimplds.cpp
index f84b901..373cf4f 100755
--- a/src/sysync/customimplds.cpp
+++ b/src/sysync/customimplds.cpp
@@ -2718,9 +2718,12 @@ localstatus TCustomImplDS::implProcessMap(cAppCharP aRemoteID, cAppCharP aLocalI
 
 
 
-/// helper to merge database version of an item with the passed version of the same item
-TMultiFieldItem *TCustomImplDS::mergeWithDatabaseVersion(TSyncItem *aSyncItemP)
+/// helper to merge database version of an item with the passed version of the same item;
+/// does age comparison by default, with "local side wins" as fallback
+TMultiFieldItem *TCustomImplDS::mergeWithDatabaseVersion(TSyncItem *aSyncItemP, bool &aChangedDBVersion)
 {
+  aChangedDBVersion = false;
+
   TStatusCommand dummy(fSessionP);
   TMultiFieldItem *dbVersionItemP = (TMultiFieldItem *)newItemForRemote(aSyncItemP->getTypeID());
   if (!dbVersionItemP) return NULL;
@@ -2733,13 +2736,39 @@ TMultiFieldItem *TCustomImplDS::mergeWithDatabaseVersion(TSyncItem *aSyncItemP)
   bool ok=logicRetrieveItemByID(*dbVersionItemP,dummy);
   if (ok && dummy.getStatusCode()!=404) {
     // item found in DB, merge with original item
-    bool changedNewVersion, changedDBVersion;
-    aSyncItemP->mergeWith(*dbVersionItemP, changedNewVersion, changedDBVersion, this);
-    PDEBUGPRINTFX(DBG_DATA,(
-      "Merged incoming item (winning,relevant,%smodified) with version from database (loosing,to-be-replaced,%smodified)",
-      changedNewVersion ? "" : "NOT ",
-      changedDBVersion ? "" : "NOT "
-    ));
+    // TODO (?): make this configurable
+    TConflictResolution crstrategy = cr_newer_wins;
+
+    if (crstrategy==cr_newer_wins) {
+      sInt16 cmpRes = aSyncItemP->compareWith(*dbVersionItemP,
+                                              eqm_nocompare,this
+#ifdef SYDEBUG
+                                              ,PDEBUGTEST(DBG_CONFLICT+DBG_DETAILS) // show age comparisons only if we want to see details
+#endif
+                                              );
+      if (cmpRes==-1) crstrategy=cr_server_wins;
+      else crstrategy=cr_client_wins;
+      PDEBUGPRINTFX(DBG_DATA,(
+                "Newer item determined: %s",
+                crstrategy==cr_client_wins ?
+                "Incoming item is newer and wins" :
+                "DB item is newer and wins"));
+
+      bool changedNewVersion;
+      if (crstrategy==cr_client_wins) {
+        aSyncItemP->mergeWith(*dbVersionItemP, changedNewVersion, aChangedDBVersion, this);
+      } else {
+        dbVersionItemP->mergeWith(*aSyncItemP, aChangedDBVersion, changedNewVersion, this);
+      }
+      PDEBUGPRINTFX(DBG_DATA,(
+                              "Merged incoming item (%s,relevant,%smodified) with version from database (%s,%s,%smodified)",
+                              crstrategy==cr_client_wins ? "winning" : "loosing",
+                              changedNewVersion ? "" : "NOT ",
+                              crstrategy==cr_server_wins ? "winning" : "loosing",
+                              aChangedDBVersion ? "to-be-replaced" : "to-be-left-unchanged",
+                              aChangedDBVersion ? "" : "NOT "
+                              ));
+    }
   }
   else {
     // no item found, we cannot force a conflict
@@ -2834,11 +2863,16 @@ bool TCustomImplDS::implProcessItem(
         if (sta==DB_Conflict) {
           // DB has detected item conflicts with data already stored in the database and
           // request merging current data from the backend with new data before storing.
-          augmentedItemP = mergeWithDatabaseVersion(myitemP);
+          bool changedDBVersion;
+          augmentedItemP = mergeWithDatabaseVersion(myitemP, changedDBVersion);
           if (augmentedItemP==NULL)
             sta = DB_Error; // no item found, DB error 
           else {
-            sta = apiUpdateItem(*augmentedItemP); // store augmented version back to DB
+             // store augmented version back to DB only if modified
+            if (changedDBVersion)
+              sta = apiUpdateItem(*augmentedItemP);
+            else
+              sta = LOCERR_OK;
             // in server case, further process like backend merge (but no need to fetch again, we just keep augmentedItemP)
             if (IS_SERVER && sta==LOCERR_OK) sta = DB_DataMerged; 
           }
@@ -2944,11 +2978,16 @@ bool TCustomImplDS::implProcessItem(
           if (sta==DB_Conflict) {
             // DB has detected item conflicts with data already stored in the database and
             // request merging current data from the backend with new data before storing.
-            augmentedItemP = mergeWithDatabaseVersion(myitemP);
+            bool changedDBVersion;
+            augmentedItemP = mergeWithDatabaseVersion(myitemP, changedDBVersion);
             if (augmentedItemP==NULL)
               sta = DB_Error; // no item found, DB error 
             else {
-              sta = apiUpdateItem(*augmentedItemP); // store augmented version back to DB
+               // store augmented version back to DB only if modified
+              if (changedDBVersion)
+                sta = apiUpdateItem(*augmentedItemP);
+              else
+                sta = LOCERR_OK;
               delete augmentedItemP; // forget now
             }
           }
diff --git a/src/sysync/customimplds.h b/src/sysync/customimplds.h
index 0437866..939f1ad 100755
--- a/src/sysync/customimplds.h
+++ b/src/sysync/customimplds.h
@@ -773,7 +773,7 @@ protected:
   //   as a item copy with only finalisation-required fields
   void queueForFinalisation(TMultiFieldItem *aItemP);
   /// helper to merge database version of an item with the passed version of the same item
-  TMultiFieldItem *mergeWithDatabaseVersion(TSyncItem *aSyncItemP);
+  TMultiFieldItem *mergeWithDatabaseVersion(TSyncItem *aSyncItemP, bool &aChangedDBVersion);
 public:
   // - get last to-remote sync time
   lineartime_t getPreviousToRemoteSyncCmpRef(void) { return fPreviousToRemoteSyncCmpRef; };