On Di, 2011-08-16 at 17:18 +0200, Patrick Ohly wrote:
> On Tue, 2011-08-16 at 15:24 +0200, Lukas Zeller wrote:
> > On Aug 16, 2011, at 12:54 , Patrick Ohly wrote:
> > >> All but the first bullet point are not implemented so far.
> > > 
> > > I had to resolve the double negation before this sentence made sense to
> > > me ;-) So the "read back" bullet item is implemented, the rest isn't.
> > 
> > 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).
> 
> I would have to write a test case and try it out before I can comment on
> the patch.

I've written such a test and found some problems:
     1. compile issue
     2. TPluginApiDS::apiAddItem() treated DB_DataMerged as a failure
        and didn't return the local ID to
        TCustomImplDS::implProcessItem(), which caused a segfault later
        on (local ID NULL)

Patches attached.

But now I still see an unexpected Replace command. Here's the processing
of the client's Add in a SyncEvolution server where that Add maps to an
item that was new for this client:

[2011-08-29 16:43:57.474] adding "meeting on site, big meeting room"
[2011-08-29 16:43:57.479] InsertItemAsKey res=207
[2011-08-29 16:43:57.480] Database adapter indicates that added item was merged 
with pre-existing data (status 207)
[2011-08-29 16:43:57.480] TStdLogicDS::getConflictingItemByLocalID, found 
RemoteID='1234567890!@#$%^&*()<>@dummy-rid', 
LocalID='1234567890!@#$%^&*()<>@dummy-rid', syncop=add
[2011-08-29 16:43:57.480] Preventing localID='1234567890!@#$%^&*()<>@dummy-rid' 
to be sent to client
[2011-08-29 16:43:57.480] Item with localID='1234567890!@#$%^&*()<>@dummy-rid' 
will NOT be sent to client (slowsync match / duplicate prevention)
[2011-08-29 16:43:57.480] Created command 'Status' (outgoing)
[2011-08-29 16:43:57.484] ReadItemAsKey aID=(1234567890!@#$%^&*()<>@dummy-rid,) 
res=0
[2011-08-29 16:43:57.484] 'ScriptExecute' - Start executing Script, 
name=afterreadscript [--][++] [->end] [->enclosing]
[...]
[2011-08-29 16:43:57.489] Forced conflict with corresponding item from server DB
[2011-08-29 16:43:57.489] Deleted command 'Status' (outgoing MsgID=0, CmdID=0)
[2011-08-29 16:43:57.489] ModifyMap called: aEntryType=normal, 
aLocalID='1234567890!@#$%^&*()<>@dummy-rid, 
aRemoteid='1234567890!@#$%^&*()<>@dummy-rid', aMapflags=0x0, aDelete=0
[2011-08-29 16:43:57.490] - found no matching entry for localID 
'1234567890!@#$%^&*()<>@dummy-rid' - creating new one, added=true
[2011-08-29 16:43:57.490] - Operation add performed (regular), Remote 
ID=1234567890!@#$%^&*()<>@dummy-rid Local ID=1234567890!@#$%^&*()<>@dummy-rid, 
status=201
[2011-08-29 16:43:57.490] Deleted command 'Status' (outgoing MsgID=0, CmdID=0)
[...]

Later the server generates a Replace:

–[2011-08-29 16:43:57.494] 'Item_Generate' - generating SyncML item, 
SyncOp=wants-replace, RemoteID=1234567890!@#$%^&*()<>@dummy-rid [--][++] 
[->end] [->enclosing]
+–[2011-08-29 16:43:57.494] 'ScriptExecute' - Start executing Script, 
name=outgoingscript [--][++] [->end] [->enclosing]
[...]
+–[2011-08-29 16:43:57.505] 'issue' - issuing command, Cmd=Replace [--][++] 
[->end] [->enclosing]
[2011-08-29 16:43:57.505] Command 'Replace': is 2-th counted cmd, cmdsize(+tags 
needed to end msg)=500, available=38558 (maxfree=38558, freeaftersend=38058, 
notUsableBufferBytes()=0)
[2011-08-29 16:43:57.505] Item remoteID='1234567890!@#$%^&*()<>@dummy-rid', 
localID='', datasize=422
[2011-08-29 16:43:57.506] Replace: issued as (outgoing MsgID=2, CmdID=5), now 
queueing for status
[2011-08-29 16:43:57.506] Outgoing Message size is now 946 bytes
–[2011-08-29 16:43:57.506] End of 'issue' [->top] [->enclosing] 
+–[2011-08-29 16:43:57.506] 'DSStateChange' - Datastore changes state, 
datastore=calendar, oldstate=server_sync_gen_started, newstate=sync_gen_done 
[--][++] [->end] [->enclosing]
–[2011-08-29 16:43:57.506] End of 'DSStateChange' [->top] [->enclosing] 
[2011-08-29 16:43:57.506] engGenerateSyncCommands ended, state='sync_gen_done', 
sync generation done

Shouldn't this Item_Generate be suppressed in this case?

> > 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...
> 
> That sounds like it would solve the problem.

Is this something that you intend to work on or shall I give it a try
myself?

> I'm currently debating with myself whether I'll release 1.2 with the
> known issue around add<->add conflicts in CalDAV sync. It is very likely
> happen in an enterprise scenario where the calendar adds all meetings
> automatically. But there's also an easy workaround: sync the calendar
> before processing meeting invitations in email locally.

I'm still undecided. Depends a lot on the timing and the risk with these
patches.


-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

>From 82777e9eef3a5e048102233788dbd340b4be416b Mon Sep 17 00:00:00 2001
From: Patrick Ohly <patrick.o...@intel.com>
Date: Mon, 29 Aug 2011 16:56:24 +0200
Subject: [PATCH 1/2] dataconversion.cpp: added dummy getConflictingItemByLocalID() to resolve compile problem

After the recent change ("server engine: better support for backend
doing its own duplicate merging (status 207 from API)"),
dataconversion.cpp no longer compiled because the dummy engine didn't
implement the new pure virtual getConflictingItemByLocalID().
---
 src/sysync/dataconversion.cpp |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/src/sysync/dataconversion.cpp b/src/sysync/dataconversion.cpp
index 421bf90..5b51210 100644
--- a/src/sysync/dataconversion.cpp
+++ b/src/sysync/dataconversion.cpp
@@ -32,6 +32,7 @@ public:
   virtual void logicMarkItemForResend(const char*, const char*) {}
 #ifdef SYSYNC_SERVER
   virtual TSyncItem *getConflictingItemByRemoteID(TSyncItem *syncitemP) { return NULL; }
+  virtual TSyncItem *getConflictingItemByLocalID(TSyncItem *syncitemP) { return NULL; }
   virtual TSyncItem *getMatchingItem(TSyncItem *syncitemP, TEqualityMode aEqMode) { return NULL; }
   virtual void dontSendItemAsServer(TSyncItem *syncitemP) {}
   virtual void SendItemAsServer(TSyncItem *aSyncitemP) {}
-- 
1.7.2.5

>From fba12b4b490450143f8847ea6a2623042eb9411a Mon Sep 17 00:00:00 2001
From: Patrick Ohly <patrick.o...@intel.com>
Date: Mon, 29 Aug 2011 16:57:18 +0200
Subject: [PATCH 2/2] plugin API: treat DB_DataMerged as success

Plugins are now allowed to return LOCERR_OK and DB_DataMerged. The
latter must be treated like success at the API level. Special
treatment then happens in the caller.
---
 src/DB_interfaces/api_db/pluginapids.cpp |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/src/DB_interfaces/api_db/pluginapids.cpp b/src/DB_interfaces/api_db/pluginapids.cpp
index fe36cf4..54a63ad 100755
--- a/src/DB_interfaces/api_db/pluginapids.cpp
+++ b/src/DB_interfaces/api_db/pluginapids.cpp
@@ -1324,7 +1324,8 @@ localstatus TPluginApiDS::apiAddItem(TMultiFieldItem &aItem, string &aLocalID)
   return LOCERR_WRONGUSAGE; // completely wrong usage - should never happen as compatibility is tested at module connect
   #endif
   // now check result
-  if (dberr==LOCERR_OK) {
+  if (dberr==LOCERR_OK ||
+      dberr==DB_DataMerged) {
     // save new ID
     aLocalID = itemAndParentID.item.c_str();
     aItem.setLocalID(aLocalID.c_str()); // make sure item itself has correct ID as well
-- 
1.7.2.5

_______________________________________________
os-libsynthesis mailing list
os-libsynthesis@synthesis.ch
http://lists.synthesis.ch/mailman/listinfo/os-libsynthesis

Reply via email to