Re: [SyncEvolution] UID matching during add<->add conflict

On Aug 16, 2011, at 12:54 , Patrick Ohly wrote: Yes, I guess that would be useful. We'll see... Agreed. Yes - it can check in the maps. I created a patch that enhances support for handling the different merge cases (see below). Correct. I was wrong in the first mail about that. The backend cannot do that, the engine can. Correct! That was a case of too many edits ;-) With the patch below the entire list (plus also handling not just add<->add conflicts) is implemented (but not yet tested). So what's missing right now is a simple way to actually do the merge in the backend, without re-implementing half of libsynthesis. The (hopefully not so) longterm solution would be libvxxx. As a short term workaround, which is not exactly super elegant but not dangerous either, we could do the following: * we define another special return code for AddItem, say 419. The backend can return it when it detects that the added data SHOULD be merged with what the backend already has, but can't do that itself. * the engine would do the same things as (details see commit msg in the patch below) for 207 but additionally it would: * merge the incoming data with the data fetched from the backend with the localID returned from AddItem() * call UpdateItem() with the merge result in the backend Let me know what you think... Best Regards, Lukas Zeller --- the patch --- From a6ff628dc1833c6eacfa307c4fbd1ce7a4757077 Mon Sep 17 00:00:00 2001 From: Lukas Zeller <luz(a)plan44.ch&gt; Date: Tue, 16 Aug 2011 15:02:43 +0200 Subject: [PATCH] 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. --- sysync/customimplds.cpp | 47 +++++++++++++++++++++++++++++++++++++++++++-- sysync/localengineds.cpp | 2 +- sysync/localengineds.h | 3 +- sysync/stdlogicds.cpp | 29 ++++++++++++++++++++++++++- sysync/stdlogicds.h | 5 +++- sysync/superdatastore.h | 3 +- 6 files changed, 80 insertions(+), 9 deletions(-) diff --git a/sysync/customimplds.cpp b/sysync/customimplds.cpp index 9d19f89..45051b4 100755 --- a/sysync/customimplds.cpp +++ b/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/sysync/localengineds.cpp b/sysync/localengineds.cpp index f566ce7..50ef0c3 100755 --- a/sysync/localengineds.cpp +++ b/sysync/localengineds.cpp @@ -3177,7 +3177,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/sysync/localengineds.h b/sysync/localengineds.h index dfee6e6..0c918f3 100755 --- a/sysync/localengineds.h +++ b/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/sysync/stdlogicds.cpp b/sysync/stdlogicds.cpp index e79ac79..5eac4de 100644 --- a/sysync/stdlogicds.cpp +++ b/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/sysync/stdlogicds.h b/sysync/stdlogicds.h index 723e03a..994593a 100644 --- a/sysync/stdlogicds.h +++ b/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/sysync/superdatastore.h b/sysync/superdatastore.h index cf6ec14..8549a37 100755 --- a/sysync/superdatastore.h +++ b/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() -- 1.7.5.4+GitX