server engine: better support for backend doing its own duplicate merging (status 207 from API)

So far, the DB backend could return DB_DataMerged (=207) for an add
to signal that the added data was merged with some pre-existing data
and thus the client should be updated with the merge result. This was
intended for merge with external data (augmenting entries with
lookups from other sources), but not yet for duplicate elimination.

This patch adds support for propagating merges that eliminate duplicates
in addition to merges that just add external data.

When a client sends a new item (new = client side ID not yet known
server side), and the DB backend returns status 207 for AddItem,
the following happens:

- current maps are searched for an item with the same localID as
  what the DB backend just returned as the "new" localID. If such
  an item is found, this means the add was completed by merging
  with an existing item.
- If so, this means that for this single localID, there are now
  two versions with different remoteIDs on the client. The item
  with the remoteID found in the map must be deleted, so a
  delete command is added to the list of changes to be sent to
  the client.
- It might also be that the item merged was just added to the
  server (and not yet known to the client), or had another
  change predating the merge. If so, the list of changes to be sent
  to the client will contain an add or a replace, resp.
  that must NOT propagate to the client, so it is removed
  from the list now.
- Finally, the result of the merge is fetched from the database
  and propagated to the client as a replace command.

6 files changed, 80 insertions, 9 deletions

diff --git a/src/sysync/customimplds.cpp b/src/sysync/customimplds.cpp
index 1f7e64e..b1df6e0 100755
--- a/src/sysync/customimplds.cpp
+++ b/src/sysync/customimplds.cpp
@@ -2796,11 +2796,52 @@ bool TCustomImplDS::implProcessItem(
         if (IS_SERVER) {
         	#ifdef SYSYNC_SERVER
           if (sta==DB_DataMerged) {
-            // while adding, data was merged with pre-existing data (external from the sync set)
-            // so we should retrieve the full data and send an update back to the client
+            // while adding, data was merged with pre-existing data from...
+            // ..either data external from the sync set, such as augmenting a contact with info from a third-party lookup
+            // ..or another item pre-existing in the sync set.
+            PDEBUGPRINTFX(DBG_DATA,("Database adapter indicates that added item was merged with pre-existing data (status 207)"));
+            myitemP->setLocalID(localID.c_str()); // following searches need to be based on new localID returned by add
+            // check if the item resulting from merrge is known by the client already (in it's pre-merge form, that is)
+            TMapContainer::iterator conflictingMapPos = findMapByLocalID(localID.c_str(), mapentry_normal);
+            bool remoteAlreadyKnowsItem = conflictingMapPos!=fMapTable.end();
+            // also check if we have a (pre-merge) operation pending for that item already
+            TSyncItem *conflictingItemP = getConflictingItemByLocalID(myitemP);
+            if (conflictingItemP) {
+              // cancel any pending operation for the original item.
+              dontSendItemAsServer(conflictingItemP);
+            }
+            // If client already knows that item, we must propagate the merge to the client
+            // by deleting the original item (in addition to sending the update of the merge)
+            if (remoteAlreadyKnowsItem) {
+              PDEBUGPRINTFX(DBG_DATA,(
+                "Merge occured with an item already known remotely (localID=%s, remoteID=%s) -> delete duplicate from client",
+                (*conflictingMapPos).localid.c_str(),
+                (*conflictingMapPos).remoteid.c_str()
+              ));
+              // client already knows an item with that server-side localID
+              // - check if it is the same item as the added one from a server's perspective
+              //   (this should not normally not be the case, as otherwise we should not have
+              //   tried to add it in the first place - check above should have generated 418 error)
+              bool sameRemoteItem = (*conflictingMapPos).remoteid==myitemP->getRemoteID();
+              if (sameRemoteItem) {
+                PDEBUGPRINTFX(DBG_ERROR,("Consistency error: despite being added new, this remoteID is already known!?"));
+              }
+              else {
+                // create delete for now duplicate item on client
+                TSyncItem *duplDelP = newItemForRemote(myitemP->getTypeID());
+                if (duplDelP) {
+                  // - setup delete item
+                  duplDelP->setRemoteID((*conflictingMapPos).remoteid.c_str());
+                  duplDelP->clearLocalID();
+                  duplDelP->setSyncOp(sop_delete);
+                  // - add it to the list of changes to be sent to the client later
+                  SendItemAsServer(duplDelP);
+                }
+              }
+            }
+            // now create a replace command to update the item added from the client with the merge result
             // - this is like forcing a conflict, i.e. this loads the item by local/remoteid and adds it to
             //   the to-be-sent list of the server.
-            PDEBUGPRINTFX(DBG_DATA,("Database adapter indicates that added item was merged with pre-existing data (status 207), so update client with merged item"));
             forceConflict(myitemP);
             sta = LOCERR_OK; // otherwise, treat as ok
           }
diff --git a/src/sysync/localengineds.cpp b/src/sysync/localengineds.cpp
index 1904670..9039be6 100644
--- a/src/sysync/localengineds.cpp
+++ b/src/sysync/localengineds.cpp
@@ -3173,7 +3173,7 @@ bool TLocalEngineDS::engHandleSyncOpStatus(TStatusCommand *aStatusCmdP,TSyncOpCo
         //     a map. But symbian cannot send early maps - it instead does
         //     it's own duplicate checking.
         // ... during resumed sync as client (as servers might issue 418 for
-        //     items send a second time after an implicit suspend)
+        //     items sent a second time after an implicit suspend)
         PDEBUGPRINTFX(DBG_ERROR,("Warning: received 418 status for add in resumed/slowsync session -> treat it as ok (200)"));
         dsConfirmItemOp(sop_replace,localID,remoteID,true); // kind of ok
         statuscode=200; // convert to ok (but no count incremented, as nothing changed)
diff --git a/src/sysync/localengineds.h b/src/sysync/localengineds.h
index dfee6e6..0c918f3 100755
--- a/src/sysync/localengineds.h
+++ b/src/sysync/localengineds.h
@@ -1009,8 +1009,9 @@ protected:
   /// get conflict resolution strategy.
   virtual TConflictResolution getConflictStrategy(bool aForSlowSync, bool aForFirstTime=false);
   #ifdef SYSYNC_SERVER
-  /// called to check if conflicting replace command from server exists
+  /// check if conflicting item already exist in list of items-to-be-sent-to-client
   virtual TSyncItem *getConflictingItemByRemoteID(TSyncItem *syncitemP) = 0;
+  virtual TSyncItem *getConflictingItemByLocalID(TSyncItem *syncitemP) = 0;
   /// called to check if content-matching item from server exists
   virtual TSyncItem *getMatchingItem(TSyncItem *syncitemP, TEqualityMode aEqMode) = 0;
   /// called to prevent item to be sent to client in subsequent engGenerateSyncCommands()
diff --git a/src/sysync/stdlogicds.cpp b/src/sysync/stdlogicds.cpp
index e79ac79..5eac4de 100644
--- a/src/sysync/stdlogicds.cpp
+++ b/src/sysync/stdlogicds.cpp
@@ -611,7 +611,7 @@ localstatus TStdLogicDS::startDataAccessForServer(void)
 // called to check if conflicting replace or delete command from server exists
 TSyncItem *TStdLogicDS::getConflictingItemByRemoteID(TSyncItem *syncitemP)
 {
-  // search for conflicting item by LUID
+  // search for conflicting item by remoteID
   TSyncItemPContainer::iterator pos;
   for (pos=fItems.begin(); pos!=fItems.end(); ++pos) {
     if (strcmp((*pos)->getRemoteID(),syncitemP->getRemoteID())==0) {
@@ -630,6 +630,31 @@ TSyncItem *TStdLogicDS::getConflictingItemByRemoteID(TSyncItem *syncitemP)
 } // TStdLogicDS::getConflictingItemByRemoteID
 
 
+
+// called to check if conflicting item (with same localID) already exists in the list of items
+// to be sent to the server
+TSyncItem *TStdLogicDS::getConflictingItemByLocalID(TSyncItem *syncitemP)
+{
+  // search for conflicting item by localID
+  TSyncItemPContainer::iterator pos;
+  for (pos=fItems.begin(); pos!=fItems.end(); ++pos) {
+    if (strcmp((*pos)->getLocalID(),syncitemP->getLocalID())==0) {
+      // same LUID exists in data from server
+      PDEBUGPRINTFX(DBG_DATA+DBG_CONFLICT,(
+        "TStdLogicDS::getConflictingItemByLocalID, found RemoteID='%s', LocalID='%s', syncop=%s",
+        syncitemP->getRemoteID(),
+        syncitemP->getLocalID(),
+        SyncOpNames[syncitemP->getSyncOp()]
+      ));
+      return (*pos); // return pointer to item in question
+    }
+  }
+  PDEBUGPRINTFX(DBG_DATA+DBG_CONFLICT,("TStdLogicDS::getConflictingItemByLocalID, no conflicting item"));
+  return NULL;
+} // TStdLogicDS::getConflictingItemByLocalID
+
+
+
 // called to check if content-matching item from server exists for slow sync
 TSyncItem *TStdLogicDS::getMatchingItem(TSyncItem *syncitemP, TEqualityMode aEqMode)
 {
@@ -684,7 +709,7 @@ void TStdLogicDS::dontSendItemAsServer(TSyncItem *syncitemP)
   for (pos=fItems.begin(); pos!=fItems.end(); ++pos) {
     if (*pos == syncitemP) {
       // it is in our list
-      PDEBUGPRINTFX(DBG_DATA+DBG_HOT,("Item with localID='%s' will NOT be sent to client (usually due to slowsync match)",syncitemP->getLocalID()));
+      PDEBUGPRINTFX(DBG_DATA+DBG_HOT,("Item with localID='%s' will NOT be sent to client (slowsync match / duplicate prevention)",syncitemP->getLocalID()));
       delete *pos; // delete item itself
       fItems.erase(pos); // remove from list
       break;
diff --git a/src/sysync/stdlogicds.h b/src/sysync/stdlogicds.h
index 723e03a..994593a 100755
--- a/src/sysync/stdlogicds.h
+++ b/src/sysync/stdlogicds.h
@@ -277,8 +277,10 @@ private:
 
 
   #ifdef SYSYNC_SERVER
-  // - called to check if conflicting replace or delete command from server exists
+protected:
+  // - check if conflicting item already exist in list of items-to-be-sent-to-client
   virtual TSyncItem *getConflictingItemByRemoteID(TSyncItem *syncitemP);
+  virtual TSyncItem *getConflictingItemByLocalID(TSyncItem *syncitemP);
   // - called to check if content-matching item from server exists
   virtual TSyncItem *getMatchingItem(TSyncItem *syncitemP, TEqualityMode aEqMode);
   // - called to prevent item to be sent to client in subsequent logicGenerateSyncCommandsAsServer()
@@ -286,6 +288,7 @@ private:
   virtual void dontSendItemAsServer(TSyncItem *syncitemP);
   // - called to have additional item sent to remote (DB takes ownership of item)
   virtual void SendItemAsServer(TSyncItem *aSyncitemP);
+private:
   // - end map operation (rollback if not aDoCommit)
   virtual bool MapFinishAsServer(
     bool aDoCommit,                // if not set, entire map operation must be undone
diff --git a/src/sysync/superdatastore.h b/src/sysync/superdatastore.h
index bfeefae..6db801e 100755
--- a/src/sysync/superdatastore.h
+++ b/src/sysync/superdatastore.h
@@ -257,8 +257,9 @@ protected:
   virtual void engRequestEnded(void);
   // Dummies, should never be called in Superdatastore, as all DB processing takes
   // place in subdatastores
-  // - called to check if conflicting replace command from server exists
+  // - check if conflicting item already exist in list of items-to-be-sent-to-client
   virtual TSyncItem *getConflictingItemByRemoteID(TSyncItem *syncitemP) { return NULL; };
+  virtual TSyncItem *getConflictingItemByLocalID(TSyncItem *syncitemP) { return NULL; };
   // - called to check if content-matching item from server exists
   virtual TSyncItem *getMatchingItem(TSyncItem *syncitemP, TEqualityMode aEqMode) { return NULL; };
   // - called to prevent item to be sent to client in subsequent engGenerateSyncCommands()