Re: [os-libsynthesis] xml instead of wbxml to format the syncML network stream

2015-01-20 Thread Patrick Ohly
On Tue, 2015-01-20 at 19:04 +0800, zhou lei wrote:
> Dear Lukas,
> 
> 
> I installing one syncML server which did not support WBXML, I have to
> format the syncML CMD with XML, not WBXML. 
> 
> 
> As I know, libsynthesis already support XML, but I don't know how to
> switch WBXML to XML. Please kindly help on this.

In the sync profile, set the "encoding" key to 2 instead of 1. See
SDK_manual.pdf for setting keys. I'm not sure where that key
specifically is documented.

-- 
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.




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


Re: [os-libsynthesis] aborting a sync -> slow sync

2014-09-24 Thread Patrick Ohly
On Wed, 2014-09-24 at 21:14 +0200, Lukas Zeller wrote:
> Hi Patrick,
> 
> On 24.09.2014, at 17:33, Patrick Ohly  wrote:
> 
> > Let me pick your brain for a second... :-)
> 
> It has gotten a bit rusty as far as sync is concerned. But I'll try :-)
> 
> > When SyncEvolution runs a local sync (i.e. both client and server are
> > provided by SyncEvolution) and the sync gets aborted (= STEPCMD_ABORT is
> > passed to SessionStep()), I noticed that the next sync starts again as
> > if nothing happened.
> > 
> > For example, aborting a normal two-way sync and then syncing again does
> > another normal sync using the same sync anchors because the datastores's
> > SaveAdminData has never been called during the aborted sync.
> 
> What I *remember* (=not verified by looking at the code recently) is
> that the engine tries to find out if "anything has happened" already
> when the sync is aborted. 
> 
> If so, it would save state. Otherwise, it would just stop and next
> sync would start "as if nothing happened" (because it thinks in fact
> nothing of importance *has* happened, and avoid the relatively
> expensive process of saving state).

Would this "save state" include resetting the sync anchor? I can't think
of any other way how the server could avoid duplicates, because it
cannot know whether the client has added the items and if so, under
which ID... except that there is also suspend/resume.

It's different on the client side because the client can detect the
duplicate item based on the server's ID.

> > The result is that the server can end up adding the same items twice,
> > once during the aborted sync and again during the next one. This causes
> > duplicates, because the client side has no means of detecting the
> > duplicate.
> 
> Did you actually see such problems happeing or do you just anticipate
> them because they *would* occur if the engine *did* abort without
> saving state in all cases?

I've seen it happen in the the case that I described (message sent with
several s inside, server aborts before receiving a response).

> Now I could not resist looking at the code - two of these "early
> abort" checks are in TSyncAgent::NextMessage(), look for "happened" in
> the comments, but these only apply to early suspends, not abort. I
> guess another relevant check is in
> TLocalEngineDS::engAbortDataStoreSync(), look for "preventResuming". I
> don't have the complete picture however from just this quick look
> up...

So the expected result is that a suspend state needs to be written on
both sides and then the next sync must resume? 

> > How is this case supposed to be handled? Is SyncEvolution doing
> > something wrong in its change or meta data handling?
> 
> The only thing I could imagine going wrong would be not to continue
> calling SessionStep() after the STEPCMD_ABORT until SessionStep()
> actually signals session done, because it might take another step to
> completely clean up the session, which is needed to make all
> datastores call saveAdminData.

I need to check this on both server and client side. I suspect that
SyncEvolution is too aggressive on the client side and doesn't let it
finish he session normally.

If STEPCMD_ABORT leads to suspend state being written and resuming the
sync in the next session, why is STEPCMD_RESUME needed? My understanding
is that STEPCMD_RESUME will take longer because it relies on further
SyncML message exchanges. Does this perhaps make it more reliable?


-- 
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.




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


[os-libsynthesis] aborting a sync -> slow sync

2014-09-24 Thread Patrick Ohly
Hello Lukas!

Let me pick your brain for a second... :-)

When SyncEvolution runs a local sync (i.e. both client and server are
provided by SyncEvolution) and the sync gets aborted (= STEPCMD_ABORT is
passed to SessionStep()), I noticed that the next sync starts again as
if nothing happened.

For example, aborting a normal two-way sync and then syncing again does
another normal sync using the same sync anchors because the datastores's
SaveAdminData has never been called during the aborted sync.

The result is that the server can end up adding the same items twice,
once during the aborted sync and again during the next one. This causes
duplicates, because the client side has no means of detecting the
duplicate.

How is this case supposed to be handled? Is SyncEvolution doing
something wrong in its change or meta data handling?

My current workaround consists of detecting when the server side sends
data to the client and resetting the admin data. If the sync does not
complete, the next sync will be a slow sync because the server's sync
anchor is gone. Is there a better way?

-- 
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.




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


[os-libsynthesis] avoiding disk writes during sync without changes

2014-08-29 Thread Patrick Ohly
Hello!

For IVI, I am working on cloud syncing. When syncing against a WebDAV
server, SyncEvolution runs a two-way sync with binfile client on the
WebDAV side and a server with admin data handled by SyncEvolution on the
other side.

One goal for IVI is to minimize or better, avoid disk writes, because
flash storage must last as long as possible.

The most common case is that nothing changed on either side. In this
case, libsynthesis unnecessarily updates nonce (even if not used; I've
already patched that (1)), sync anchors (again, I have a patch for this:
skip writing of admin data after detecting the special case (2)) and the
change log in the binfile client.

This last write happens in TBinfileImplDS::changeLogPreflight(). The
changes are minor, just a few bytes change. I suspect that these are the
time stamps and modcount embedded in the log.

Would it be possible to check in changeLogPreflight() how significant
the changes are? If there were no item changes, what would be the effect
of not updating the header?

There are two cases where that can happen with the attached patch:
- the processes crashes
- SyncEvolution skips the session shutdown, see (2).

(1) For local sync, none and
none are used. Patch attached. Okay?

(2) This happens in SyncEvolution: the SaveAdminData implementation
detects that nothing happened during the sync, then returns an error to
libsynthesis and tells the SyncEvolution event loop to stop the sync
without sending the final reply message to the client. The server side
then finishes the sync successfully while the client side simply aborts
without doing its own session shutdown. This also eliminates
SaveAdminData on the binfile client side.

-- 
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 4683fceb72cef0e7301b735a23b1f40323da52ba Mon Sep 17 00:00:00 2001
From: Patrick Ohly 
Date: Fri, 29 Aug 2014 10:35:47 +0200
Subject: [PATCH 1/2] syncsession: avoid unnecessary nonce update

When the configure auth type needs no nonce, don't generate one
and return NULL immediately, like newChallenge() would do.

That avoids unnecessary disk writes for storing the new nonce, which
is important for local syncs in IVI. It also avoids debug output about
a challenge that would not get sent.
---
 src/sysync/syncsession.cpp | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/src/sysync/syncsession.cpp b/src/sysync/syncsession.cpp
index 54c119c..f7c7a11 100644
--- a/src/sysync/syncsession.cpp
+++ b/src/sysync/syncsession.cpp
@@ -4061,15 +4061,22 @@ bool TSyncSession::processSyncOpItem(
 // generate challenge for session
 SmlChalPtr_t TSyncSession::newSessionChallenge(void)
 {
+  sysync::TAuthTypes auth = requestedAuthType();
+
+  // Avoid misleading debug output (there is no challenge)
+  // and more importantly, creating a new nonce that is not
+  // going to be used. That causes unnecessary disk writes.
+  if (auth == sysync::auth_none) return NULL;
+
   string nonce;
   getNextNonce(fRemoteURI.c_str(),nonce);
   PDEBUGPRINTFX(DBG_PROTO,(
 "Challenge for next auth: AuthType=%s, Nonce='%s', binary %sallowed",
-authTypeSyncMLNames[requestedAuthType()],
+authTypeSyncMLNames[auth],
 nonce.c_str(),
 getEncoding()==SML_WBXML ? "" : "NOT "
   ));
-  return newChallenge(requestedAuthType(),nonce,getEncoding()==SML_WBXML);
+  return newChallenge(auth,nonce,getEncoding()==SML_WBXML);
 } // TSyncSession::newSessionChallenge
 
 
-- 
2.1.0.rc1

>From 115cef8a0ec1823b58808bbb9820f7cb82c2a313 Mon Sep 17 00:00:00 2001
From: Patrick Ohly 
Date: Fri, 29 Aug 2014 10:39:17 +0200
Subject: [PATCH 2/2] binfileclient: avoid disk writes in changeLogPreflight()

During a normal sync where nothing changed, only the header gets
updated. This change is not critical and thus does not have to be
flushed to disk unless also some entries get added or updated.

The advantage is that when SyncEvolution detects a sync where nothing
changed on either side and skips the client's session shutdown, the
.bfi is left unchanged, which reduces flash wearout.

To detect item changes, a brute-force byte comparison is used. This
requires less changes to the code and is less error-prone than adding
"modified=true" to all places where "existingentries" gets modified.
---
 src/sysync/binfileimplds.cpp | 24 +++-
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/src/sysync/binfileimplds.cpp b/src/sysync/binfileimplds.cpp
index 314ccc7..947fa0c 100755
--- a/src/sysync/binfileimplds.cpp
+++ b/src/sysync/binfileimplds.cpp
@@ -741,9 +741,9 @@ localstatus TBinfileImplDS::changeLogPreflight(bool &aValidChangelog)
 lsd.c_str()
   ));
   #endif
-  // - s

Re: [os-libsynthesis] comparison of PHOTO data

2014-07-14 Thread Patrick Ohly
On Mon, 2014-07-14 at 18:15 +0200, Lukas Zeller wrote:
> Hello Patrick,
> 
> On 11 Jul 2014, at 15:27, Patrick Ohly  wrote:
> 
> > I'm currently investigating why a comparison of two PHOTO fields of
> > different length returns "field equal". PHOTO is defined as:
> > 
> >   > merge="fillempty"/>
> 
> My first question would be why PHOTO was defined "string" here, not "blob"?

I changed that in this commit:

commit 6d3b1cf64b09cf1603b813137d14d529a31dda8b
Author: Patrick Ohly 
Date:   Fri Jul 5 09:45:15 2013 +0200

EDS: update PHOTO+GEO during slow sync, avoid rewriting PHOTO file

If PHOTO and/or GEO were the only modified properties during a slow
sync, the updated item was not written into local storage because
they were marked as compare="never" = "not relevant".

For PHOTO this was intentional in the sample config, with the
rationale that local storages often don't store the data exactly as
requested. When that happens, comparing the data would lead to
unnecessary writes. But EDS and probably all other local SyncEvolution
storages (KDE, file) store the photo exactly as requested, so not
considering changes had the undesirable effect of not always writing
new photo data.

For GEO, ignoring it was accidental.

A special merge script handles EDS file:// photo URIs. When the
loosing item has the data in a file and the winning item has binary
data, the data in the file may still be up-to-date, so before allowing
MERGEFIELDS() to overwrite the file reference with binary data and
thus forcing EDS to write a new file, check the content. If it
matches, use the file reference in both items.

Unfortunately I did not write down why I changed the type, instead of
merely changing the compare setting.

Does it matter at all?

> IMHO The real bug is that TBlobField does not override compareWith.
> TBlobField's compareWith could ignore aCaseInsensitive and use memcmp,
> for the case of actually comparing two blobs, and fall back to
> TStringField for comparison with other types.
> 
> > We have std::string as value and therefore can store null bytes as part
> > of the data, but the actual comparison falls back to C-string
> > operations, which only work for null bytes at the end.
> 
> There's to much C strings throughout the entire project, so for text,
> C string semantics are assumed pretty much everywhere.
> 
> TBlobField however makes use of the true binary byte string capability of 
> std::string exactly this way.

That's also true for TStringField, except that it occasionally falls
back to plain C operations, which is only correct for real strings.

> > I think the strcmp() needs to be replaced with something that also looks
> > at the rest of the string if no difference is found. Agreed?
> 
> A fully std::string compatible comparison would be nicer here, and fix the 
> problem.

Here's the patch that I ended up using:

commit 9278e054e9a9a2aa8c73aed98cb42bf1f9bfd0fe
Author: Patrick Ohly 
Date:   Mon Jul 14 05:00:54 2014 -0700

string fields: full compare

String fields also get used for arbitrary binary data, like
photos. In that case we need to compare the entire std::string,
not just the part before any embedded null byte.

This gets done using std::string::compare. Case-insensitive
comparison still uses C strucmp() and thus should never be used
for non-string data.

diff --git a/src/sysync/itemfield.cpp b/src/sysync/itemfield.cpp
index e6f7d71..64867e0 100644
--- a/src/sysync/itemfield.cpp
+++ b/src/sysync/itemfield.cpp
@@ -801,7 +801,7 @@ bool TStringField::merge(TItemField &aItemField, const char 
aSep)
 // Note: Both fields must be assigned. NO TEST IS DONE HERE!
 sInt16 TStringField::compareWith(TItemField &aItemField, bool aCaseInsensitive)
 {
-  sInt16 result;
+  int result;
   PULLFROMPROXY;
   if (aItemField.isBasedOn(fty_string)) {
 TStringField *sfP = static_cast(&aItemField);
@@ -812,7 +812,7 @@ sInt16 TStringField::compareWith(TItemField &aItemField, 
bool aCaseInsensitive)
 if (aCaseInsensitive)
   result=strucmp(fString.c_str(),sfP->fString.c_str());
 else
-  result=strcmp(fString.c_str(),sfP->fString.c_str());
+  result=fString.compare(sfP->fString);
   }
   else {
 // convert other field to string
@@ -821,7 +821,7 @@ sInt16 TStringField::compareWith(TItemField &aItemField, 
bool aCaseInsensitive)
 if (aCaseInsensitive)
   result=strucmp(fString.c_str(),s.c_str());
 else
-  result=strcmp(fString.c_str(),s.c_str());
+  result=fString.compare(s);
   }
   return result >0 ? 1 : (result<0 ? -1 : 0);
 } // TStringField::compareWith


-- 
Best Regards, Patri

[os-libsynthesis] comparison of PHOTO data

2014-07-11 Thread Patrick Ohly
Hello!

I'm currently investigating why a comparison of two PHOTO fields of
different length returns "field equal". PHOTO is defined as:

  

I think the specific situation is that the two values contain a null
byte somewhere in the middle, and the part before that is equal.

I think the following code does the comparison, doesn't it?

sInt16 TStringField::compareWith(TItemField &aItemField, bool aCaseInsensitive)
{
  sInt16 result;
  PULLFROMPROXY;
  if (aItemField.isBasedOn(fty_string)) {
TStringField *sfP = static_cast(&aItemField);
#ifdef STREAMFIELD_SUPPORT
sfP->pullFromProxy(); // make sure we have all chars
#endif
// direct compare possible, return strcmp
if (aCaseInsensitive)
  result=strucmp(fString.c_str(),sfP->fString.c_str());
else
  result=strcmp(fString.c_str(),sfP->fString.c_str());
  }
...

We have std::string as value and therefore can store null bytes as part
of the data, but the actual comparison falls back to C-string
operations, which only work for null bytes at the end.

I think the strcmp() needs to be replaced with something that also looks
at the rest of the string if no difference is found. Agreed?

Or do we need a real "binary" type? Using string type for PHOTO also has
the risk that during a merge operation, the two values will get
concatenated (thus breaking the images) unless some custom merge script
resolves the conflict differently first.

-- 
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.




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


Re: [os-libsynthesis] merging of winning and loosing items

2014-05-22 Thread Patrick Ohly
On Wed, 2014-04-09 at 11:36 +0200, Patrick Ohly wrote:
> The problem that I was alluding to earlier is that merging
> 
> winning: TEL  = [ '1', '2' ] and
> loosing: TEL = [ '1', '2', '3' ]
> has no unambiguous solution. Was '3' added on the loosing side or was it
> removed on the winning one?
> 
> If in doubt, probably the best approach is to pick the one which avoids
> data loss, i.e. preserve '3'.
> 
> Now, consider reordering:
> winning: TEL = [ '1', '2' ] and
> loosing: TEL = [ '2', '1' ].
> 
> Were the entries merely reordered or were the values changed on the
> loosing side? In the former case, nothing needs to be done and no
> information gets lost. In the later case, the changes made on the client
> get lost.
[...]
> Copying extra values from the loosing side can work with reordering by
> looking at each value and not just at the end of the array. It's not
> perfect, though, when values were edited.

I implemented the "copy from loosing side" for any of the repeating
values. This will appear in 1.4.99.2.

-- 
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.




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


Re: [os-libsynthesis] How to run syncevolution at bare minimum processing.

2014-05-19 Thread Patrick Ohly
On Mon, 2014-05-19 at 18:46 +0530, anuj chauhan wrote:

> I have a quad-core machine with 8gb ram and these processes are
> consuming about 90-94% of the cpu usage.I want to tune syncevolution
> so that it could run on bare minimum processing.For this i have done
> following modification :
> 1.  Commented out the code which gets the Server DevInf. I
> suspect, this will reduce load on Server and client as well.
> 
> 2.  Removed the file synccompare. During these concurrent runs,
> top command showed high CPU usage for Perl just after syncevolution
> completed its exercise. By removing this file, this reduced the client
> load and processing.

Instead it would have been better to use "printChanges=0". This disables
the invocation of synccompare.

> 3.  Reduced the log level to 0. Number of IO went down along with
> the disk usage.

You can decrease it further by also using "dumpData=0".

-- 
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.




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


Re: [os-libsynthesis] mixing properties with and without group tag (was: Re: suppressempty + arrays)

2014-05-14 Thread Patrick Ohly
On Wed, 2014-05-14 at 17:44 +0200, Lukas Zeller wrote:
> Hello Patrick,
> 
> On 12.05.2014, at 18:25, Patrick Ohly  wrote:
> 
> > [...] 
> >> and a property which has a group tag must not reuse any of these
> >> unassigned group tag values.
> > 
> > Actually, a "property which has a group *field*" - it doesn't matter
> > whether the current property has a group tag value. This check was
> > missing. Attached a patch adding it, in a brute-force manner. Does that
> > look right?
> 
> Yes, it looks right to me. I guess you've found out in the meantime if it 
> also *works* right :-)

Yes, it works for me. But it's good that you had a look at it anyway,
because my vCard profile certainly isn't representative.

I'll include this and the other patches (including the removal of
backslash escaping in parameters!) in the master branch of libsynthesis
on freedesktop.org once it passed all my tests. There's also a memory
leak fix for SWAP().

-- 
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.




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


Re: [os-libsynthesis] mixing properties with and without group tag (was: Re: suppressempty + arrays)

2014-05-12 Thread Patrick Ohly
On Mon, 2014-05-12 at 17:29 +0200, Patrick Ohly wrote:
> I think the code which deals with group tags must use the same logic
> that I introduced for "sharedfield": a property which has a group field
> array, but no group tag, must set an unassigned value in the group field
> array,

That was already done:
  if (aPropP->groupFieldID!=FID_NOT_SUPPORTED) {
TItemField *g_fldP =  
aItem.getArrayFieldAdjusted(aPropP->groupFieldID+baseoffset,repoffset,false);
if (g_fldP)
  g_fldP->setAsString(aGroupName,aGroupNameLen); // store the group name 
(aGroupName might be NULL, that's ok)
  }

>  and a property which has a group tag must not reuse any of these
> unassigned group tag values.

Actually, a "property which has a group *field*" - it doesn't matter
whether the current property has a group tag value. This check was
missing. Attached a patch adding it, in a brute-force manner. Does that
look right?

-- 
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 f03962222958e9311db0bcf44a1810ee89afc99b Mon Sep 17 00:00:00 2001
From: Patrick Ohly 
Date: Mon, 12 May 2014 09:03:56 -0700
Subject: [PATCH] mimedir parser: avoid reusing group tag

When two properties (TEL and X-ABLabel in this example) share the same group
field, then storing the property where a group tag was used for the first time
must not reuse unassigned group field entries.

There was some logic for that already (see "someGroups"), but it failed to
handle this case. This approach here uses brute-force instead of trying to me
smart: the reuse check in the ngetArrayField(e_rep,true); // get leaf field, if it exists
-  if (!e_basefldP || (e_fldP && e_fldP->isAssigned())) {
-// base field of one of the main fields does not exist or leaf field is already assigned
+  if (!e_basefldP || (e_fldP && e_fldP->isAssigned()) ||
+  (aPropP->groupFieldID!=FID_NOT_SUPPORTED && !valuelist &&
+   aItem.getArrayFieldAdjusted(aPropP->groupFieldID+baseoffset,e_rep,true))) {
+// base field of one of the main fields does not exist or leaf field is already assigned,
+// or the group field entry is already in use (doesn't matter whether it is empty)
 // -> skip that repetition
 dostore = false;
 break;
-- 
1.7.10.4

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


[os-libsynthesis] mixing properties with and without group tag (was: Re: suppressempty + arrays)

2014-05-12 Thread Patrick Ohly
On Mon, 2014-05-12 at 12:55 +0200, Patrick Ohly wrote:
> The X-ABRELATEDNAMES properties were not generated. The labels should be
> redundant, but some peers get confused. Google preserves them as
> stand-alone X-ABLabel without tag. DAViCal preserves them with tag,
> which then happened to confuse SyncEvolution's conversion code (separate
> issue).

This second issue actually is in the groupfield support of libsynthesis:

[2014-05-12 15:02:45.501] Parsing: 
  * [2014-05-12 15:02:45.501] 
BEGIN:VCARD
VERSION:3.0
PRODID:-//Synthesis AG//NONSGML SyncML Engine V3.4.0.47//EN
REV:20140512T150240Z
UID:syuid974165.212266710163478
N:Doe;John;;;
FN:John Doe
X-EVOLUTION-FILE-AS:Doe\, John
TITLE:tester
TEL;TYPE=WORK,VOICE:business 1
X-MOZILLA-HTML:FALSE
item3.X-ABLabel:Spouse
item2.X-ABLabel:Manager
item1.X-ABLabel:Assistant
END:VCARD
  * [2014-05-12 15:02:45.501] Successfully parsed: 
  * [2014-05-12 15:02:45.501] Item
LocalID='syuid974165.212266710163478.vcf', RemoteID='',
operation=wants-add, size: [maxlocal,maxremote,actual]
  * [2014-05-12 15:02:45.501] 
-  0 :integer SYNCLVL [   0, n/a, 0] : 
-  1 :  timestamp REV [   0,   0, 0] : 2014-05-12T15:02:40Z 
(TZ: UTC)
-  2 : string UID [   0, n/a,27] : 
"syuid974165.212266710163478"
-  3 : string GROUP_TAG   [   0, n/a, 0] : 
 -- element0 : "item3"
 -- element1 : "item2"
 -- element2 : "item1"
-  4 : string N_LAST  [   0,   0, 3] : "Doe"
-  5 : string N_FIRST [   0,   0, 4] : "John"
...
- 23 :  telephone TEL [   0,   0, 0] : 
 -- element0 : "business 1"
- 24 :integer TEL_FLAGS   [   0,   0, 0] : 
 -- element0 : 10
- 25 :integer TEL_SLOT[   0,   0, 0] : 
...
- 83 : string LABEL   [   0,   0, 0] : 
 -- element0 : "Spouse"
 -- element1 : "Manager"
 -- element2 : "Assistant"
- 84 : string XPROPS  [   0,   0, 0] : 

This field list makes it look like TEL "business 1" at index #0 had the
same group tag as LABEL "Spouse", thus adding a label to a TEL which had
no label.

I think the code which deals with group tags must use the same logic
that I introduced for "sharedfield": a property which has a group field
array, but no group tag, must set an unassigned value in the group field
array, and a property which has a group tag must not reuse any of these
unassigned group tag values.

-- 
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.




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


Re: [os-libsynthesis] suppressempty + arrays

2014-05-12 Thread Patrick Ohly
On Mon, 2014-05-12 at 15:37 +0200, Lukas Zeller wrote:
> Hello Patrick,
> 
> On 12.05.2014, at 12:55, Patrick Ohly  wrote:
> 
> > What's the intended behavior of suppressempty for arrays?
> > 
> >"suppressempty": This optional boolean value. can be set to true if 
> > this property must
> >never be sent empty (without values). This is the case for most 
> > vCalendar fields, for example.
> > 
> > The default seems to be suppressempty=no (judging from the "can be set
> > to true"). When I have arrays with empty entries, will those be
> > generated?
> 
> It also depends on "minShow" and "fDontSendEmptyProperties", and also
> on "mandatory", but basically empty array elements will generate a
> property when "suppressempty" is not set. At least that's the
> intention :-)

Thanks for pointing out "fDontSendEmptyProperties". I have that set,
which is probably the reason why they don't get generated.

-- 
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.




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


[os-libsynthesis] suppressempty + arrays

2014-05-12 Thread Patrick Ohly
Hello!

What's the intended behavior of suppressempty for arrays?

"suppressempty": This optional boolean value. can be set to true if 
this property must
never be sent empty (without values). This is the case for most 
vCalendar fields, for example.

The default seems to be suppressempty=no (judging from the "can be set
to true"). When I have arrays with empty entries, will those be
generated?

Example:

- 35 : string RELATEDNAMES[   0,   0, 0] : 
 -- element0 : 
 -- element1 : 
 -- element2 : 
...
- 83 : string LABEL   [   0,   0, 0] : 
 -- element0 : "Spouse"
 -- element1 : "Manager"
 -- element2 : "Assistant"



  
  
  

  

...

  
  


This leads to:

BEGIN:VCARD
VERSION:3.0
PRODID:-//Synthesis AG//NONSGML SyncML Engine V3.4.0.47//EN
REV:20140512T093706Z
UID:syuid384197.212266690650521
N:parserbug=
FN:parserbug=
X-EVOLUTION-FILE-AS:parserbug=
NICKNAME:user17
NOTE:triggers parser bug in Funambol 3.0: trailing = is mistaken for soft l
 ine break=
item3.X-ABLabel:Spouse
item2.X-ABLabel:Manager
item1.X-ABLabel:Assistant
END:VCARD

The X-ABRELATEDNAMES properties were not generated. The labels should be
redundant, but some peers get confused. Google preserves them as
stand-alone X-ABLabel without tag. DAViCal preserves them with tag,
which then happened to confuse SyncEvolution's conversion code (separate
issue).

-- 
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.




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


[os-libsynthesis] [+ x] for next field

2014-05-07 Thread Patrick Ohly
Hello!

The documentation mentions:

In addition there is a special form of array index that can be
used to access fields in a field list
(see 10.1) by index instead of by name. This special form of
array index starts with a + sign as the
first character after the opening [ as shown in the following
example:








/* sample script to access the telephone numbers by index
instead of by name */
integer i;
telephone a,b,c;
a = TEL_1[+0]; // this is the same as: a=TEL_1
b = TEL_1[+1]; // this is the same as: b=TEL_2
c = TEL_1[+2]; // this is the same as: c=TEL_3

Does this also work for fields which are arrays? Like this:

  
   
   
   

  

  a = TEL[+1][3]; // same as TEL_FLAGS[3]

I tried it briefly, but got a syntax error which implied that the first
square brackets were treated like a normal array index in TEL.

What would it take to make this work?

-- 
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.




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


Re: [os-libsynthesis] Syncevoltion throwing error on log level < 4

2014-05-07 Thread Patrick Ohly
On Wed, 2014-05-07 at 16:37 +0530, anuj chauhan wrote:
> Hi ,
> 
> 
> I am facing a issue on reducing log level<4.Syncevolution is
> 
> throwing following error:
> 
> 
> #[ERROR] basic_string::_S_construct null not valid
> 
> 
>  Please suggest how to resolve this problem.

Run it under a debugger, catch exceptions ("catch throw" in gdb),
continue until you hit the problem (some exceptions can be ignored),
then provide the stack backtrace ("where").


-- 
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.




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


Re: [os-libsynthesis] folding parameter values (was: Re: vCard group tags)

2014-05-06 Thread Patrick Ohly
On Tue, 2014-05-06 at 13:29 +0200, Patrick Ohly wrote:
> On Mon, 2014-05-05 at 14:42 +0200, Patrick Ohly wrote:
> > On Fri, 2014-05-02 at 11:59 +0200, Lukas Zeller wrote:
> > > On 02.05.2014, at 10:38, Patrick Ohly  wrote:
> > > > I noticed another problem with the "use X-ABLabel parameter" approach:
> > > > storing complex strings (spaces, quotation marks) in a parameter value
> > > > is harder.
> > > 
> > > That's probably why Apple chose the X-ABLabel property approach. An
> > > unparseable parameter could ruin the real data, a unknown property is
> > > less dangerous.
> > > 
> > > > The EDS vCard parser gets it wrong and fails to parse:
> > > > 
> > > > X-ABRELATEDNAMES;X-ABLabel=domestic partner:domestic partner
> > > > 
> > > > That is valid according to http://tools.ietf.org/html/rfc2425#page-5
> > > > because the space is a SAFE-CHAR and thus may appear in a ptext, but the
> > > > EDS parser does not expect the space. To work around this, we could
> > > > voluntarily quote the string even though it is not required. 
> > > > 
> > > > Now, the conceptual problem with "X-ABLabel parameter" is that a quoted
> > > > string cannot contain double quotes. It is probably rare that a user
> > > > enters double quotes as part of a label, but it cannot be ruled out
> > > > either. Line breaks are also only allowed in property values and not
> > > > parameter values.
> > 
> > I've looked into TMimeDirProfileHandler::generateValue() some more to
> > understand under which circumstances libsynthesis uses quoted strings
> > (with double quotes at start and end) as parameter value. At first
> > glance it doesn't seem to do that at all. Instead special values are
> > escaped with backslash.
> > 
> > item29.X-ABLabel:custom-label5\nUmlaut-ä\nSemicolon\;
> > ->
> > X-ABRELATEDNAMES;X-ABLabel=custom-label5\nUmlaut-ä\nSemicolon\;:custom 
> > relationship
> > 
> > Where is it specified that the backslash escape mechanism can be used in
> > parameter values?
> > 
> > http://tools.ietf.org/html/rfc2425#page-5 says:
> > 
> > 
> >param= param-name "=" param-value *("," param-value)
> > 
> >param-name   = x-name / iana-token
> > 
> >param-value  = ptext / quoted-string
> > 
> >ptext  = *SAFE-CHAR
> > 
> >SAFE-CHAR= WSP / %x21 / %x23-2B / %x2D-39 / %x3C-7E / NON-ASCII
> >   ; Any character except CTLs, DQUOTE, ";", ":", ","
> > 
> > My reading of that is that special characters must not appear in a ptext
> > at all, not even when escaped with backslash. One has to use a quoted
> > string, which (unfortunately) cannot hold all characters either.
> 
> Furthermore, folding is not described for parameter values, is it?
> 
> X-ABRELATEDNAMES;X-ABLabel="custom-label5 Umlaut ä Semicolon ; Backslash \ 
>  newline  tab \t end of label":custom relationship
> 
> This is what libsynthesis produces for a long parameter value. The \t
> was part of the original value. With the revised parser/generator it
> just gets passed through.
> 
> I think the generator should be changed to not fold a line unless the
> property value has started.

I'm not sure about that anymore. The EDS parser/generator has no
problems with folding inside parameters.

Attached is a patch which does what I had in mind, but we can probably
ignore it unless folding inside parameters really turns out to be wrong
and/or cause problems.

-- 
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.


diff --git a/src/sysync/mimedirprofile.cpp b/src/sysync/mimedirprofile.cpp
index 83acbca..929ef5c 100644
--- a/src/sysync/mimedirprofile.cpp
+++ b/src/sysync/mimedirprofile.cpp
@@ -2101,6 +2101,7 @@ static void finalizeProperty(
   const char *proptext,
   string &aString,
   TMimeDirMode aMimeMode, // MIME mode (older or newer vXXX format compatibility)
+  size_t foldStartingAt, // First byte offset where folding is allowed.
   bool aDoNotFold, // set to prevent folding
   bool aDoSoftBreak // set to insert QP-softbreaks when \r is encountered, otherwise do a full hard break (which essentially inserts a space for mimo_old)
 )
@@ -2112,6 +2113,7 @@ static void finalizeProperty(
   ssize_t foldLoc = -1; // possible break location - linear whit

[os-libsynthesis] folding parameter values (was: Re: vCard group tags)

2014-05-06 Thread Patrick Ohly
On Mon, 2014-05-05 at 14:42 +0200, Patrick Ohly wrote:
> On Fri, 2014-05-02 at 11:59 +0200, Lukas Zeller wrote:
> > On 02.05.2014, at 10:38, Patrick Ohly  wrote:
> > > I noticed another problem with the "use X-ABLabel parameter" approach:
> > > storing complex strings (spaces, quotation marks) in a parameter value
> > > is harder.
> > 
> > That's probably why Apple chose the X-ABLabel property approach. An
> > unparseable parameter could ruin the real data, a unknown property is
> > less dangerous.
> > 
> > > The EDS vCard parser gets it wrong and fails to parse:
> > > 
> > > X-ABRELATEDNAMES;X-ABLabel=domestic partner:domestic partner
> > > 
> > > That is valid according to http://tools.ietf.org/html/rfc2425#page-5
> > > because the space is a SAFE-CHAR and thus may appear in a ptext, but the
> > > EDS parser does not expect the space. To work around this, we could
> > > voluntarily quote the string even though it is not required. 
> > > 
> > > Now, the conceptual problem with "X-ABLabel parameter" is that a quoted
> > > string cannot contain double quotes. It is probably rare that a user
> > > enters double quotes as part of a label, but it cannot be ruled out
> > > either. Line breaks are also only allowed in property values and not
> > > parameter values.
> 
> I've looked into TMimeDirProfileHandler::generateValue() some more to
> understand under which circumstances libsynthesis uses quoted strings
> (with double quotes at start and end) as parameter value. At first
> glance it doesn't seem to do that at all. Instead special values are
> escaped with backslash.
> 
> item29.X-ABLabel:custom-label5\nUmlaut-ä\nSemicolon\;
> ->
> X-ABRELATEDNAMES;X-ABLabel=custom-label5\nUmlaut-ä\nSemicolon\;:custom 
> relationship
> 
> Where is it specified that the backslash escape mechanism can be used in
> parameter values?
> 
> http://tools.ietf.org/html/rfc2425#page-5 says:
> 
> 
>param= param-name "=" param-value *("," param-value)
> 
>param-name   = x-name / iana-token
> 
>param-value  = ptext / quoted-string
> 
>ptext  = *SAFE-CHAR
> 
>SAFE-CHAR= WSP / %x21 / %x23-2B / %x2D-39 / %x3C-7E / NON-ASCII
>   ; Any character except CTLs, DQUOTE, ";", ":", ","
> 
> My reading of that is that special characters must not appear in a ptext
> at all, not even when escaped with backslash. One has to use a quoted
> string, which (unfortunately) cannot hold all characters either.

Furthermore, folding is not described for parameter values, is it?

X-ABRELATEDNAMES;X-ABLabel="custom-label5 Umlaut ä Semicolon ; Backslash \ 
 newline  tab \t end of label":custom relationship

This is what libsynthesis produces for a long parameter value. The \t
was part of the original value. With the revised parser/generator it
just gets passed through.

I think the generator should be changed to not fold a line unless the
property value has started.

-- 
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.




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


Re: [os-libsynthesis] vCard group tags

2014-05-06 Thread Patrick Ohly
On Tue, 2014-05-06 at 12:09 +0200, Lukas Zeller wrote:
> On 05.05.2014, at 14:42, Patrick Ohly  wrote:
> 
> >>> Now, the conceptual problem with "X-ABLabel parameter" is that a quoted
> >>> string cannot contain double quotes. It is probably rare that a user
> >>> enters double quotes as part of a label, but it cannot be ruled out
> >>> either. Line breaks are also only allowed in property values and not
> >>> parameter values.
> > 
> > I've looked into TMimeDirProfileHandler::generateValue() some more to
> > understand under which circumstances libsynthesis uses quoted strings
> > (with double quotes at start and end) as parameter value. At first
> > glance it doesn't seem to do that at all. Instead special values are
> > escaped with backslash.
> > 
> > item29.X-ABLabel:custom-label5\nUmlaut-ä\nSemicolon\;
> > ->
> > X-ABRELATEDNAMES;X-ABLabel=custom-label5\nUmlaut-ä\nSemicolon\;:custom 
> > relationship
> > 
> > Where is it specified that the backslash escape mechanism can be used in
> > parameter values?
> 
> That's a tough question :-) You are probably right regarding the specs.
> 
> What I faintly remember is that the problem of escaping parameter values came 
> into the scene when Oracle's put some lengthy internal ID into parameters. 
> These IDs had all sorts of nasty characters. That's when the parameter value 
> escaping using backslash was implemented.
> 
> What I don't remember and also did not find in the history is the rationale 
> for using the backslash. Most probably, their ID contained doublequotes, so 
> doublequouting them would not have helped. I know we discussed variants with 
> the Oracle guys. Also, probably this was pre-iCalendar, and the vCalendar 
> "specs" were more than vague in many of these details. So probably this was 
> just the solution we found for vCalendar and it was never checked to conform 
> with RFC2425 later.
> 
> > http://tools.ietf.org/html/rfc2425#page-5 says:
> > 
> > 
> >   param= param-name "=" param-value *("," param-value)
> > 
> >   param-name   = x-name / iana-token
> > 
> >   param-value  = ptext / quoted-string
> > 
> >   ptext  = *SAFE-CHAR
> > 
> >   SAFE-CHAR= WSP / %x21 / %x23-2B / %x2D-39 / %x3C-7E / NON-ASCII
> >  ; Any character except CTLs, DQUOTE, ";", ":", ","
> > 
> > My reading of that is that special characters must not appear in a ptext
> > at all, not even when escaped with backslash. One has to use a quoted
> > string, which (unfortunately) cannot hold all characters either.
> 
> Especially the double quote itself remains a problem. 

In the patch that I just sent I substitute it with a single quote.

> > IMHO libsynthesis is currently producing broken vCards. I consider
> > changing the code so that it uses quoted strings if it detects unsafe
> > characters in the value and filters out invalid ones. "unsafe" would be
> > more conservative than in the RFC itself and also include spaces, to
> > work around the EDS parser bug.
> 
> Makes sense. Of course, TMimeDirProfileHandler::parseValue() would need to be 
> updated as well.

Done, see the patch in my other mail.

> To maximize backwards compatibility I would however still generate
> backslash escapes when the value actually contains a doublequote,
> newline or backslash itself.

This will cause problems with peer parsers which do implement backslash
escaping. For example:

TEL;X-ABLabel="some string \" more string":1234

The EDS parser will end the parameter at the backslash and then get
confused (and rightly so!) by the " more string" text.

> And the parser should generally allow backslash-escaped characters.

Again, that would lead to issues. Consider:

TEL;X-ABLabel="some string \":1234

If we allow escaping, the parameter continues and includes the property
value.

What could work is to allow escaping anything which is not a double
quote and which cannot be stored in a normal quoted-string. The only
downside is that this will get shown to users literally if the peer's
parser does no unescaping.


-- 
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.




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


[os-libsynthesis] parameter value escaping (was: Re: vCard group tags)

2014-05-06 Thread Patrick Ohly
On Mon, 2014-05-05 at 14:42 +0200, Patrick Ohly wrote:
> On Fri, 2014-05-02 at 11:59 +0200, Lukas Zeller wrote:
> > On 02.05.2014, at 10:38, Patrick Ohly  wrote:
> > > I noticed another problem with the "use X-ABLabel parameter" approach:
> > > storing complex strings (spaces, quotation marks) in a parameter value
> > > is harder.
> > 
> > That's probably why Apple chose the X-ABLabel property approach. An
> > unparseable parameter could ruin the real data, a unknown property is
> > less dangerous.
> > 
> > > The EDS vCard parser gets it wrong and fails to parse:
> > > 
> > > X-ABRELATEDNAMES;X-ABLabel=domestic partner:domestic partner
> > > 
> > > That is valid according to http://tools.ietf.org/html/rfc2425#page-5
> > > because the space is a SAFE-CHAR and thus may appear in a ptext, but the
> > > EDS parser does not expect the space. To work around this, we could
> > > voluntarily quote the string even though it is not required. 
> > > 
> > > Now, the conceptual problem with "X-ABLabel parameter" is that a quoted
> > > string cannot contain double quotes. It is probably rare that a user
> > > enters double quotes as part of a label, but it cannot be ruled out
> > > either. Line breaks are also only allowed in property values and not
> > > parameter values.
> 
> I've looked into TMimeDirProfileHandler::generateValue() some more to
> understand under which circumstances libsynthesis uses quoted strings
> (with double quotes at start and end) as parameter value. At first
> glance it doesn't seem to do that at all. Instead special values are
> escaped with backslash.
> 
> item29.X-ABLabel:custom-label5\nUmlaut-ä\nSemicolon\;
> ->
> X-ABRELATEDNAMES;X-ABLabel=custom-label5\nUmlaut-ä\nSemicolon\;:custom 
> relationship
> 
> Where is it specified that the backslash escape mechanism can be used in
> parameter values?
> 
> http://tools.ietf.org/html/rfc2425#page-5 says:
> 
> 
>param= param-name "=" param-value *("," param-value)
> 
>param-name   = x-name / iana-token
> 
>param-value  = ptext / quoted-string
> 
>ptext  = *SAFE-CHAR
> 
>SAFE-CHAR= WSP / %x21 / %x23-2B / %x2D-39 / %x3C-7E / NON-ASCII
>   ; Any character except CTLs, DQUOTE, ";", ":", ","
> 
> My reading of that is that special characters must not appear in a ptext
> at all, not even when escaped with backslash. One has to use a quoted
> string, which (unfortunately) cannot hold all characters either.
> 
> IMHO libsynthesis is currently producing broken vCards. I consider
> changing the code so that it uses quoted strings if it detects unsafe
> characters in the value and filters out invalid ones. "unsafe" would be
> more conservative than in the RFC itself and also include spaces, to
> work around the EDS parser bug.

Attached is a patch which makes the libsynthesis parser and generator
behave according to my current understanding of the RFCs. The risk of
course is that there are cases where backslashes are used in parameter
values and the peer (unpatched libsynthesis, other implementations)
expect that backslash escaping is used.

The Evolution Data Server parser does not use backslash escaping for
parameters. To exchange values containing backslashes, the patch is
needed.

This becomes relevant in the context of the X-ABLabel parameter. My hope
is that all other parameters are simple enough that the ambiguity never
arises.

Or do we need an on/off switch for backslash escaping depending on the
peer?

-- 
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 8a7356c1518aca405eb29ddf12a2d1052375da68 Mon Sep 17 00:00:00 2001
From: Patrick Ohly 
Date: Tue, 6 May 2014 12:13:19 +0200
Subject: [PATCH 2/2] MIME parser + encoder: no backslash quoting in parameter
 values
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

RFC 2425 (MIME DIR) and RFC 6350 (vCard) do not describe backslash
escaping for special characters in parameter values. Only the more
limited quoted-string (double quotes at start and end, no line breaks,
no double quotes inside value) is specified.

The following two examples both contain a literal backslash:

URL;X-ABLabel="Custom-label6 Backslash \":http://custom.com
X-ABRELATEDNAMES;X-ABLabel="custom-label5 Umlaut ä Semicolon ; Backslash \ end of label":custom relationship

This commit limits backslash escapin

Re: [os-libsynthesis] vCard group tags

2014-05-06 Thread Patrick Ohly
On Fri, 2014-05-02 at 22:08 +0200, Lukas Zeller wrote:
> Hello Patrick,
> 
> On 02.05.2014, at 13:44, Patrick Ohly  wrote:
> > Here's the patch:
> 
> There's one problem with this - it would break the case where the
> target of a repeating property is not an array field but multiple
> plain fields accessed via offset. Plain fields always exist, so you'll
> always end with dostore==false.
> 
> I would recommend to play safe and add a new option on
> TParameterDefinition to enable this check (and possibly other related
> checks). Maybe a bool called "sharedfield", with a default of "false".
> If it turns out you need changes in the behaviour for property field
> checking as well, the same flag could also be added to
> TPropertyDefinition.

I've implemented that approach. I think more flexibility is not needed.
Patch attached. Okay?

-- 
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 ab9ee87347c3efeb98cfb2b46549272a070f04d3 Mon Sep 17 00:00:00 2001
From: Patrick Ohly 
Date: Tue, 6 May 2014 10:48:58 +0200
Subject: [PATCH 1/2] profile: 

Traditionally, the value of a property and its parameter values get
stored in fields reserved for that property such that there is no
overlap with other properties. In such a scenario it is enough to
check for unused fields used for the property value, because the
corresponding parameter fields are guaranteed to be available once a
repetition is found.

However, once the same field is used for different properties the
check and creation becomes more complicated: when parsing properties,
the shared field must also be checked. It is not enough for it to be
empty, it also must not be in use at all, because otherwise a value might
get added to the repetition used already by some other property.

This check only makes sense (and only works) for array fields. It is
disabled by default and needs to be enabled explicitly with the new
parameter attribute "sharedfield".

Storing values must ensure that the shared array field gets resized if
the parameter was not set, otherwise the check would fail to detect
used repetitions.

The effect of using a shared field is that arrays contain a lot of
unassigned values because index overlaps between otherwise unrelated
properties get avoided.

This feature was needed for converting between a profile with custom
labels in a X-ABLabel property attached to various contact properties
(ADR, TEL, X-ABDate, etc.) via group tags and a profile using a
property parameter instead. The label array field is the shared field
in this case.
---
 src/sysync/mimedirprofile.cpp |   57 ++---
 src/sysync/mimedirprofile.h   |7 +++--
 2 files changed, 58 insertions(+), 6 deletions(-)

diff --git a/src/sysync/mimedirprofile.cpp b/src/sysync/mimedirprofile.cpp
index 5b0ecc7..a1f88b8 100644
--- a/src/sysync/mimedirprofile.cpp
+++ b/src/sysync/mimedirprofile.cpp
@@ -504,7 +504,9 @@ bool TMIMEProfileConfig::localStartElement(const char *aElementName, const char
   bool defparam=false;
   bool shownonempty=false; // don't show properties that have only param values, but no main value
   bool showinctcap=false; // don't show parameter in CTCap by default
+  bool sharedfield=false; // assume traditional, unshared field for parameter
   if (
+!getAttrBool(aAttributes,"sharedfield",sharedfield,true) ||
 !getAttrBool(aAttributes,"positional",positional,true) ||
 !getAttrBool(aAttributes,"default",defparam,true) ||
 !getAttrBool(aAttributes,"shownonempty",shownonempty,true) ||
@@ -513,7 +515,7 @@ bool TMIMEProfileConfig::localStartElement(const char *aElementName, const char
   )
 return fail("bad boolean value");
   // - add parameter
-  fOpenParameter = fOpenProperty->addParam(nam,defparam,positional,shownonempty,showinctcap,modeDep);
+  fOpenParameter = fOpenProperty->addParam(nam,defparam,positional,shownonempty,showinctcap,modeDep,sharedfield);
   #ifndef NO_REMOTE_RULES
   const char *depRuleName = getAttr(aAttributes,"rule");
   TCFG_ASSIGN(fOpenParameter->dependencyRuleName,depRuleName); // save name for later resolving
@@ -936,13 +938,14 @@ void TConversionDef::addEnumNameExt(TPropertyDefinition *aProp, const char *aEnu
 
 TParameterDefinition::TParameterDefinition(
   const char *aName, bool aDefault, bool aExtendsName, bool aShowNonEmpty, bool aShowInCTCap, TMimeDirMode aModeDep
-) {
+, bool aSharedField) {
   next=NULL;
   TCFG_ASSIGN(paramname,aName);
   defaultparam=aDefault;
   extendsname=aExtendsName;
   shownonempty=aShowNonEmpt

Re: [os-libsynthesis] vCard group tags

2014-05-05 Thread Patrick Ohly
On Fri, 2014-05-02 at 11:59 +0200, Lukas Zeller wrote:
> On 02.05.2014, at 10:38, Patrick Ohly  wrote:
> > I noticed another problem with the "use X-ABLabel parameter" approach:
> > storing complex strings (spaces, quotation marks) in a parameter value
> > is harder.
> 
> That's probably why Apple chose the X-ABLabel property approach. An
> unparseable parameter could ruin the real data, a unknown property is
> less dangerous.
> 
> > The EDS vCard parser gets it wrong and fails to parse:
> > 
> > X-ABRELATEDNAMES;X-ABLabel=domestic partner:domestic partner
> > 
> > That is valid according to http://tools.ietf.org/html/rfc2425#page-5
> > because the space is a SAFE-CHAR and thus may appear in a ptext, but the
> > EDS parser does not expect the space. To work around this, we could
> > voluntarily quote the string even though it is not required. 
> > 
> > Now, the conceptual problem with "X-ABLabel parameter" is that a quoted
> > string cannot contain double quotes. It is probably rare that a user
> > enters double quotes as part of a label, but it cannot be ruled out
> > either. Line breaks are also only allowed in property values and not
> > parameter values.

I've looked into TMimeDirProfileHandler::generateValue() some more to
understand under which circumstances libsynthesis uses quoted strings
(with double quotes at start and end) as parameter value. At first
glance it doesn't seem to do that at all. Instead special values are
escaped with backslash.

item29.X-ABLabel:custom-label5\nUmlaut-ä\nSemicolon\;
->
X-ABRELATEDNAMES;X-ABLabel=custom-label5\nUmlaut-ä\nSemicolon\;:custom 
relationship

Where is it specified that the backslash escape mechanism can be used in
parameter values?

http://tools.ietf.org/html/rfc2425#page-5 says:


   param= param-name "=" param-value *("," param-value)

   param-name   = x-name / iana-token

   param-value  = ptext / quoted-string

   ptext  = *SAFE-CHAR

   SAFE-CHAR= WSP / %x21 / %x23-2B / %x2D-39 / %x3C-7E / NON-ASCII
  ; Any character except CTLs, DQUOTE, ";", ":", ","

My reading of that is that special characters must not appear in a ptext
at all, not even when escaped with backslash. One has to use a quoted
string, which (unfortunately) cannot hold all characters either.

IMHO libsynthesis is currently producing broken vCards. I consider
changing the code so that it uses quoted strings if it detects unsafe
characters in the value and filters out invalid ones. "unsafe" would be
more conservative than in the RFC itself and also include spaces, to
work around the EDS parser bug.

-- 
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.




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


Re: [os-libsynthesis] vCard group tags

2014-05-02 Thread Patrick Ohly
On Fri, 2014-05-02 at 11:59 +0200, Lukas Zeller wrote:
> Hello Patrick,
> 
> On 02.05.2014, at 10:38, Patrick Ohly  wrote:
> 
> >> What do you mean by "independent" properties?
> > 
> > For example, ADR and TEL are independent in the sense that their values
> > are stored in different field arrays. Sharing the LABEL field for the
> > new parameter creates a dependency (or risk of collision) that did not
> > exist before.
> 
> But how can this work at all? The index in the field arrays is what
> relates the components (e.g. TEL, TEL_FLAGS and LABEL) to each other.
> 
> This would mean all of the field arrays would need to have rows for
> the *sum* of all the TEL+ADR+EMAIL+URL+xxx, with most rows empty, to
> be able to share a single LABEL array.
> 
> Now I see why you want to check the availability of LABEL row even if 
> assigned via a property parameter.
> 
> Hmmm - indeed this might work, at the expense of a lot of empty
> elements in the actual property field arrays.

Exactly, that's what I need and get with the patch.

I suspect that a groupfield would cause less empty elements if (and only
if) no groups are used, because then values from different properties
can use the same UNASSIGNED entry in the group field. But I haven't
checked whether that's really how it works; it is not very relevant with
Google because Google uses many group tags, even if the label is just
"Other".

If different properties end up sharing the same UNASSIGNED group tag,
then adding labels is harder in scripts (must move entries before
setting the tag), but that's not necessary at the moment.

>  I'm just not sure right now if the generator part of MimeProfile is
> prepared for properly skipping entirely empty rows to avoid generating
> empty property lines in the vCard output. But that would be fixable as
> well.

That part already works.

> > That's because the code which checks where to store the ADR value does
> > not check whether the LABEL at the position is available.
> > 
> > There's another problem:
> > 
> > TEL:123456789
> > ADR;X-ABLabel=labelAdr:xxx
> > 
> > A simplistic check "LABEL[0] empty" will accept position #0 for ADR. But
> > doing so will add the labelAdr also to the TEL, which was previously
> > created at position #0.
> > 
> > That means that the check has to be "LABEL[0] does not exist".
> 
> Yes. Fortunately, TItemField differentiates between empty and unassigned; the 
> check needs to be for the latter.

Unassigned is not good enough. If the LABEL array gets extended at the
end (for example, because the third ADR had a X-ABLabel parameter), then
I have unassigned entries at the beginning of the array which must not
be used, because there are already other properties associated with
them.

Here's the patch:

diff --git a/src/sysync/mimedirprofile.cpp b/src/sysync/mimedirprofile.cpp
index 5b0ecc7..3699ad0 100644
--- a/src/sysync/mimedirprofile.cpp
+++ b/src/sysync/mimedirprofile.cpp
@@ -4154,6 +4154,35 @@ bool TMimeDirProfileHandler::parseProperty(
   else
 dostore = true; // at least one field exists, we might 
store
 }
+// - check if fields used for parameters are empty
+const TParameterDefinition *paramP = aPropP->parameterDefs;
+while (paramP) {
+  if (mimeModeMatch(paramP->modeDependency)
+#ifndef NO_REMOTE_RULES
+  && (!paramP->ruleDependency || 
isActiveRule(paramP->ruleDependency))
+#endif
+  ) {
+if (paramP->convdef.fieldid==FID_NOT_SUPPORTED)
+  continue; // no field, no need to check it
+sInt16 e_fid = paramP->convdef.fieldid /* +baseoffset */;
+sInt16 e_rep = repoffset;
+aItem.adjustFidAndIndex(e_fid,e_rep);
+// - get base field
+TItemField *e_basefldP = aItem.getField(e_fid);
+TItemField *e_fldP = NULL;
+if (e_basefldP)
+  e_fldP=e_basefldP->getArrayField(e_rep,true); // get 
leaf field, if it exists
+if (!e_basefldP || e_fldP) {
+  // base field of one of the main fields does not exist 
or leaf field is already in use
+  // (unassigned is not good enough, otherwise we might 
end up adding information
+  // to some other, previously parsed property using the 
same array field)
+  // -> skip that repetition
+  dostore=false;
+  break;
+}
+  }
+  paramP=par

Re: [os-libsynthesis] vCard group tags

2014-05-02 Thread Patrick Ohly
On Thu, 2014-05-01 at 16:53 +0200, Lukas Zeller wrote:
> Hello Patrick,
> 
> On 30.04.2014, at 19:10, Patrick Ohly  wrote:
> 
> >> What you have here is probably the most elaborate profile ever
> >> specified for libsynthesis :-) , but I see no reason why it should not
> >> work once the  is correct.
> > 
> > That's what I had tried earlier. It leads to the situation where the
> > labels of independent properties overwrite labels of other, earlier
> > properties.
> 
> What do you mean by "independent" properties?

For example, ADR and TEL are independent in the sense that their values
are stored in different field arrays. Sharing the LABEL field for the
new parameter creates a dependency (or risk of collision) that did not
exist before.

> > That's because the position checking seems to focus
> > exclusively on property values and ignores property parameters.
> 
> Yes.
> 
> Somehow I don't see how a label parameter value can get stored without
> also writing an associated property value.
> 
> Only when using the grouped tag mode with a separate X-ABLabel
> property, it might happen that you only have the label, but not (yet)
> the values, depending on the order of the group members in the vCard.
> Only if you mix both variants in the input vCard, I can imagine
> something like the following could go wrong:
> 
>   A.X-ABLabel:labelA
>   TEL;X-ABLabel=labelTel:123456789
>   A.TEL:987654321
> 
> In this case there is a label-only situation after the first line is
> parsed. When the second line is parsed, it will overwrite the first
> label (because it does not check the parameter array for being empty
> at that index), so labelA would be lost.

It's simpler.

TEL;X-ABLabel=labelTel:123456789
ADR;X-ABLabel=labelAdr:xxx

stores only the second label at position #0
=> LABEL = [ 'labelAdr' ]

That's because the code which checks where to store the ADR value does
not check whether the LABEL at the position is available.

There's another problem:

TEL:123456789
ADR;X-ABLabel=labelAdr:xxx

A simplistic check "LABEL[0] empty" will accept position #0 for ADR. But
doing so will add the labelAdr also to the TEL, which was previously
created at position #0.

That means that the check has to be "LABEL[0] does not exist".

> > Similar to "for (sInt16 e=0; enumValues; e++) {" the code would
> > also need to iterate over aPropP->parameterDefs.
> 
> Yes.

This helped, but so far I have only implemented the too simplistic
check.

I noticed another problem with the "use X-ABLabel parameter" approach:
storing complex strings (spaces, quotation marks) in a parameter value
is harder. The EDS vCard parser gets it wrong and fails to parse:

X-ABRELATEDNAMES;X-ABLabel=domestic partner:domestic partner

That is valid according to http://tools.ietf.org/html/rfc2425#page-5
because the space is a SAFE-CHAR and thus may appear in a ptext, but the
EDS parser does not expect the space. To work around this, we could
voluntarily quote the string even though it is not required. 

Now, the conceptual problem with "X-ABLabel parameter" is that a quoted
string cannot contain double quotes. It is probably rare that a user
enters double quotes as part of a label, but it cannot be ruled out
either. Line breaks are also only allowed in property values and not
parameter values.

I'm undecided now how to proceed. Simplify X-ABLabel property values so
that they can be stored as parameter? Use the more complex X-ABLabel
property and grouping? Bummer.

-- 
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.




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


Re: [os-libsynthesis] vCard group tags

2014-04-30 Thread Patrick Ohly
 -- element0 : "PO"
 -- element1 : 
 -- element2 : 
- 78 :  multiline ADR_CITY[   0,   0, 0] : 
 -- element0 : "City"
 -- element1 : 
 -- element2 : 
- 79 :  multiline ADR_REG [   0,   0, 0] : 
 -- element0 : "State"
 -- element1 : 
 -- element2 : 
- 80 :  multiline ADR_ZIP [   0,   0, 0] : 
 -- element0 : "ZIP"
 -- element1 : 
 -- element2 : 
- 81 :  multiline ADR_COUNTRY [   0,   0, 0] : 
 -- element0 : "Country"
 -- element1 : 
 -- element2 : 
- 82 :  multiline NOTE[   0,   0,15] : "A test contact."
- 83 : string PHOTO   [   0,   0,   516] : "�PNG

- 84 : string PHOTO_TYPE  [   0,   0, 0] : 
- 85 : string PHOTO_VALUE [   0,   0, 0] : 
- 86 : string GEO_LAT [   0,   0, 0] : 
- 87 : string GEO_LONG[   0,   0, 0] : 
- 88 : string CRYPTOENCRYPTPREF [   0,   0, 0] : 
- 89 : string CRYPTOPROTOPREF [   0,   0, 0] : 
- 90 : string CRYPTOSIGNPREF  [   0,   0, 0] : 
- 91 : string OPENPGPFP   [   0,   0, 0] : 
- 92 : string XPROPS  [   0,   0, 0] : 
 -- element0 : 
"X-PHONETIC-FIRST-NAME:John"
 -- element1 : 
"X-PHONETIC-LAST-NAME:Doe"

Do I need to modify the source code to ensure that parameters do not
exist yet?

Similar to "for (sInt16 e=0; enumValues; e++) {" the code would
also need to iterate over aPropP->parameterDefs.

-- 
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.




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


Re: [os-libsynthesis] vCard group tags

2014-04-30 Thread Patrick Ohly
On Wed, 2014-04-30 at 15:19 +0200, Patrick Ohly wrote:
> It's permanently increasing repoffset.
> 
> I've not looked further, but I suspect that this is because of using
> LABEL twice, once indirectly via the group field and once via the
> position field.
> 
> I'll try applying different  elements, even if that means more
> duplication in the profile.

That had even weirder effects, so I am back at the  approach and trying to figure out where the endless loop
comes from.

Attached my current field list and profile.

It hangs parsing:

ADR;TYPE=HOME:PO;neighborhood;home address\n;City;State;ZIP;Country

The (almost) endless loop comes from this code:

  // note: repArray will be updated below (if property not empty or 
!overwriteempty)
  dostore=true; // we can store
  do {
repoffset = aRepArray[repid]*repinc;
// - set flag if repeat offset should be incremented after 
storing an empty property or not
overwriteempty = propnameextP->overwriteEmpty;
// - check if target property main value is empty (must be, or 
we will skip that repetition)
dostore = false; // if no field exists, we do not store
for (sInt16 e=0; enumValues; e++) {
  if (aPropP->convdefs[e].fieldid==FID_NOT_SUPPORTED)
continue; // no field, no need to check it
  sInt16 e_fid = aPropP->convdefs[e].fieldid+baseoffset;
  sInt16 e_rep = repoffset;
  aItem.adjustFidAndIndex(e_fid,e_rep);
  // - get base field
  TItemField *e_basefldP = aItem.getField(e_fid);
  TItemField *e_fldP = NULL;
  if (e_basefldP)
e_fldP=e_basefldP->getArrayField(e_rep,true); // get leaf 
field, if it exists
  if (!e_basefldP || (e_fldP && e_fldP->isAssigned())) {
// base field of one of the main fields does not exist or 
leaf field is already assigned
// -> skip that repetition
===>dostore = false;
break;
  }
  else
dostore = true; // at least one field exists, we might store
}
// check if we can test more repetitions
if (!dostore) {
  if (aRepArray[repid]+1
  
  
  
  
  
  
  
  
  

  
  
   

  
  

  
  

  


The ADR_* fields are consecutive. LABEL is somewhere else entirely
(almost at the end of my field list).

Here's what I see when at the dostore = false line:

(gdb) p *aPropP
$20 = { = {}, next = 0x19d5c80, propname = 
"ADR", nameExts = 0x19d5990, 
  groupFieldID = 90, numValues = 7, convdefs = 0x19d58d8, unprocessed = false, 
valuelist = false, 
  expandlist = false, valuesep = 59 ';', altvaluesep = 0 '\000', allowFoldAtSep 
= false, parameterDefs = 0x19d59c0, 
  mandatory = false, showInCTCap = true, canFilter = false, suppressEmpty = 
false, delayedProcessing = 0, 
  modeDependency = sysync::numMimeModes, nextNameExt = 0, propGroup = 66, 
dependsOnRemoterule = false, 
  ruleDependency = 0x0, dependencyRuleName = ""}
(gdb) p e
$21 = 4
(gdb) p aPropP->convdefs[0]
$22 = { = {}, fieldid = 75, enumdefs = 
0x0, convmode = 0, 
  combineSep = 0 '\000'}
(gdb) p aPropP->convdefs[1]
$23 = { = {}, fieldid = 71, enumdefs = 
0x0, convmode = 0, 
  combineSep = 0 '\000'}
(gdb) p aPropP->convdefs[2]
$24 = { = {}, fieldid = 70, enumdefs = 
0x0, convmode = 0, 
  combineSep = 0 '\000'}
(gdb) p aPropP->convdefs[3]
$25 = { = {}, fieldid = 76, enumdefs = 
0x0, convmode = 0, 
  combineSep = 0 '\000'}
(gdb) p aPropP->convdefs[4]
$26 = { = {}, fieldid = 77, enumdefs = 
0x0, convmode = 0, 
  combineSep = 0 '\000'}
(gdb) p baseoffset
$27 = 16
(gdb) p e_fid
$28 = 93
(gdb) p *propnameextP
$29 = { = {}, next = 0x0, musthave_ids = 
0, forbidden_ids = 0, 
  addtlSend_ids = 0, fieldidoffs = 16, maxRepeat = 32767, repeatInc = 1, 
minShow = 0, overwriteEmpty = false, 
  readOnly = false, repeatID = 27}

This last fieldid = 77  corresponds to ADR_REG, but because we add
baseoffset = 16 (from propnameextP->fieldidoffs) we end up with an
invalid index.

Does the code perhaps expect that the  is one of the
fields that the propertie's values get stored in? It seems so.

And does the code check for existence of values in the fields for
parameters at all? It doesn't seem so, which foils my current plan to
reuse the LABEL field for multiple different properties.

-- 
Best Regards, Patrick Ohly

The content of this mess

Re: [os-libsynthesis] vCard group tags

2014-04-30 Thread Patrick Ohly
On Tue, 2014-04-29 at 18:00 +0200, Lukas Zeller wrote:
> Hi Patrick,
> 
> you are faster in understanding the details than I could look them
> up :-)

Live and learn. Or use the source, Luke ;-)

However, it helps to spell out observations and thoughts occasionally,
so let me continue with a related problem.

I decided that a separate X-ABLabel for every TEL, ADR, X-ABDate,
X-ABRELATEDNAMES, etc. is too verbose and too different from
"traditional" usage of vCard. Recipients of such a vCard must be able to
understand and support groups, which is not guaranteed. For example, the
Evolution vCard parser supports groups, but the higher level of
abstraction does not (or not easily).

I also don't see what advantage X-ABLabel as property has over X-ABLabel
as parameter. I understand that a property could itself have parameters
or complex values, but that's not the case here. A simple string might
just as well be attached as parameter.

So that's what I am trying to do:
 1. Parse with groups: X-ABLabel property enabled.
 2. Throw away group tags.
 3. Generate with parameter: X-ABLabel parameter enabled.

This direction works. What fails is reading such a generated vCard,
because different properties store their X-ABLabel parameter in the same
"LABEL" field array, overwriting each others' values.

The logic for choosing a position must now also (or instead?!) check
whether the LABEL position is unused when adding a new ADR. In other
words, when parsing an ADR, look at LABEL to determine the position. I
thought I could achieve that with:



  
  



  
  
  
  
  
  
  
  
  

  
  
   

  
  

  
  

  


However, when parsing a vCard with HAVE-ABLABEL-PARAMETER unset and
HAVE-ABLABEL-PROPERTY set,
sysync::TMimeDirProfileHandler::parseProperty() goes into an endless
loop involving sysync::TMultiFieldItem::adjustFidAndIndex() directly on
the first ADR:

ADR;TYPE=HOME:PO;neighborhood;home address\n;City;State;ZIP;Country

It's permanently increasing repoffset.

I've not looked further, but I suspect that this is because of using
LABEL twice, once indirectly via the group field and once via the
position field.

I'll try applying different  elements, even if that means more
duplication in the profile.

-- 
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.




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


Re: [os-libsynthesis] SyncML Server Performance using Syncevolution

2014-04-29 Thread Patrick Ohly
On Tue, 2014-04-29 at 23:22 +0530, Sachin Gupta wrote:
> one more thing. everytime i invoke syncevolution, it has to load all
> the xmls, build up its data structures, and all the other stuff.
> Can i implement threads in it and run it as a daemon. This will i can
> save time from doing all this.
> 
> From within, can i then implement threads and launch multiple sync operations?

Multithreading is not going to work. You should be able to run a sync
repeatedly in the same process, but reusing the XML loading will require
putting the loop fairly deeply into the SyncEvolution stack (see
SyncContext.cpp).

I'm still not sure why you want to do this when you can simply use more
client machines. It might safe you some time for setting up a cluster if
(and only if!) you can manage to run all clients from the same machine,
but I have my doubts whether that will be possible.

-- 
Best Regards

Patrick Ohly
Senior Software Engineer

Intel GmbH
Open Source Technology Center   
Usenerstr. 5a   Phone: +49-228-2493652
53129 Bonn
Germany


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


Re: [os-libsynthesis] vCard group tags

2014-04-29 Thread Patrick Ohly
On Tue, 2014-04-29 at 17:18 +0200, Patrick Ohly wrote:
> On Tue, 2014-04-29 at 09:50 +0200, Patrick Ohly wrote:
> > On Mon, 2014-04-28 at 23:18 +0200, Lukas Zeller wrote:
> > > Hi Patrick,
> > > 
> > > > Unfortunately I am not getting the group-tag field populated. I'm
> > > > attaching my field list and profile.
> > > 
> > > small oversight on my and your part :-)
> > > 
> > > The attribute to be added to  is called "groupfield", not 
> > > "grouptag"...
> > 
> > I fixed that (see attached profile + field list). It fills the GROUP_TAG
> > array now, but the LABEL array only has one entry, apparently the last
> > one. The array positions also don't match:
> > 
> > item34.URL:http\://custom.com
> > item34.X-ABLabel:Custom-label
> > 
> > -  3 : string GROUP_TAG   [   0, n/a, 0] :  > elements>
> >  -- element0 : "item34"
> > -  4 : string LABEL   [   0,   0, 0] :  > elements>
> >  -- element0 : "Custom-label"
> > - 34 : string WEB [   0,   0, 0] :  > elements>
> >  -- element0 : 
> > ...
> >  -- element   18 : "http://custom.com";
> 
> I've stepped through TMimeDirProfileHandler::parseProperty() when
> parsing a simpler example:
> 
> BEGIN:VCARD
> VERSION:3.0
> N:Doe;John;1;Mr.;Sr.
> FN:Mr. John 1 Doe Sr.
> UID:6f354d698b7ccd22
> item1.URL:http\://company.com
> item1.X-ABLabel:Work
> item2.URL:http\://custom.com
> item2.X-ABLabel:Custom-label
> END:VCARD
> 
> When parsing item1.X-ABLabel:Work,
> TMimeDirProfileHandler::parseProperty() immediately skips over the
> parameter parsing because there is none and fieldoffsetfound was set to
> true in
> fieldoffsetfound = (aPropP->nameExts==NULL); // no first pass needed at 
> all w/o nameExts, just use offs=0
> 
> It then has repoffset == 0 when storing the group tag:
> 
>   // parameters are all processed by now, decision made to store data (if 
> !dostore, routine exits above)
>   // - store the group tag value if we have one
>   if (aPropP->groupFieldID!=FID_NOT_SUPPORTED) {
> TItemField *g_fldP =  
> aItem.getArrayFieldAdjusted(aPropP->groupFieldID+baseoffset,repoffset,false);
> if (g_fldP)
>   g_fldP->setAsString(aGroupName,aGroupNameLen); // store the group name 
> (aGroupName might be NULL, that's ok)
>   }
> 
> This happens to be correct (accidentally) for item1.X-ABLabel:Work.

Looking further at aPropP->nameExts it seems that this what relates to
 in the profile config. I did not have that for my X-ABLabel.
Adding it seems to fix the problem. So it seems that "groupfield" can
only be used reliably with a  which has a , correct?


  
  


-- 
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.




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


Re: [os-libsynthesis] vCard group tags

2014-04-29 Thread Patrick Ohly
On Tue, 2014-04-29 at 09:50 +0200, Patrick Ohly wrote:
> On Mon, 2014-04-28 at 23:18 +0200, Lukas Zeller wrote:
> > Hi Patrick,
> > 
> > > Unfortunately I am not getting the group-tag field populated. I'm
> > > attaching my field list and profile.
> > 
> > small oversight on my and your part :-)
> > 
> > The attribute to be added to  is called "groupfield", not 
> > "grouptag"...
> 
> I fixed that (see attached profile + field list). It fills the GROUP_TAG
> array now, but the LABEL array only has one entry, apparently the last
> one. The array positions also don't match:
> 
> item34.URL:http\://custom.com
> item34.X-ABLabel:Custom-label
> 
> -  3 : string GROUP_TAG   [   0, n/a, 0] :  elements>
>  -- element0 : "item34"
> -  4 : string LABEL   [   0,   0, 0] : 
>  -- element0 : "Custom-label"
> - 34 : string WEB [   0,   0, 0] :  elements>
>  -- element0 : 
> ...
>  -- element   18 : "http://custom.com";

I've stepped through TMimeDirProfileHandler::parseProperty() when
parsing a simpler example:

BEGIN:VCARD
VERSION:3.0
N:Doe;John;1;Mr.;Sr.
FN:Mr. John 1 Doe Sr.
UID:6f354d698b7ccd22
item1.URL:http\://company.com
item1.X-ABLabel:Work
item2.URL:http\://custom.com
item2.X-ABLabel:Custom-label
END:VCARD

When parsing item1.X-ABLabel:Work,
TMimeDirProfileHandler::parseProperty() immediately skips over the
parameter parsing because there is none and fieldoffsetfound was set to
true in
fieldoffsetfound = (aPropP->nameExts==NULL); // no first pass needed at all 
w/o nameExts, just use offs=0

It then has repoffset == 0 when storing the group tag:

  // parameters are all processed by now, decision made to store data (if 
!dostore, routine exits above)
  // - store the group tag value if we have one
  if (aPropP->groupFieldID!=FID_NOT_SUPPORTED) {
TItemField *g_fldP =  
aItem.getArrayFieldAdjusted(aPropP->groupFieldID+baseoffset,repoffset,false);
if (g_fldP)
  g_fldP->setAsString(aGroupName,aGroupNameLen); // store the group name 
(aGroupName might be NULL, that's ok)
  }

This happens to be correct (accidentally) for item1.X-ABLabel:Work.

When repeating this for item2.X-ABLabel, the setAsString() above
overwrites "item1" with "item2" at offset 0, leading to:

-  3 : string GROUP_TAG   [   0, n/a, 0] : 
 -- element0 : "item2"
 -- element1 : "item2"

The second element here came from item2.URL.

The group tag matching is buried in the parameter parsing loop. Skipping
it via the (aPropP->nameExts==NULL) is incorrect if the property has a
group name.

I'm not sure what the correct way of executing that code in this
particular case is. The following patch does not work:

diff --git a/src/sysync/mimedirprofile.cpp b/src/sysync/mimedirprofile.cpp
index 5b0ecc7..16a5aac 100644
--- a/src/sysync/mimedirprofile.cpp
+++ b/src/sysync/mimedirprofile.cpp
@@ -3835,7 +3835,7 @@ bool TMimeDirProfileHandler::parseProperty(
   encoding = enc_none; // no encoding by default
   charset = aMimeMode==mimo_standard ? chs_utf8 : fDefaultInCharset; // always 
UTF8 for real MIME-DIR (same as enclosing SyncML doc), for mimo_old depends on 
 remote rule option (normally UTF-8)
   nameextmap = 0; // no name extensions detected so far
-  fieldoffsetfound = (aPropP->nameExts==NULL); // no first pass needed at all 
w/o nameExts, just use offs=0
+  fieldoffsetfound = (aPropP->nameExts==NULL) && !aGroupNameLen; // no first 
pass needed at all w/o nameExts, just use offs=0, except when we have to match 
a group name
   valuelist = aPropP->valuelist; // cache flag
   // prepare storage as unprocessed value
   if (aPropP->unprocessed) {

It loops twice, but still never reaches the group name matching because
that is inside a "while (propnameextP)" loop that never gets entered.

Should that code be copied out for the simpler case of a property with
no aPropP->nameExts?

-- 
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.




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


Re: [os-libsynthesis] vCard group tags

2014-04-29 Thread Patrick Ohly
 [   0,   0, 5] : "FALSE"
- 47 : string IMPP[   0,   0, 0] : 
 -- element0 : "xmpp:google%20talk"
 -- element1 : "aim:aim"
 -- element2 : "ymsgr:yahoo"
 -- element3 : "skype:skype"
 -- element4 : "x-apple:QQ"
 -- element5 : "msnim:MSN"
 -- element6 : "aim:ICQ"
 -- element7 : "xmpp:Jabber"
 -- element8 : "x-apple:custom%20chat"
- 48 : string IMPP_SERVICE[   0,   0, 0] : 
- 49 : string IMPP_SLOT   [   0,   0, 0] : 
- 50 : string AIM_HANDLE  [   0,   0, 0] : 
- 51 : string AIM_SLOT[   0,   0, 0] : 
- 52 : string GADUGADU_HANDLE [   0,   0, 0] : 
- 53 : string GADUGADU_SLOT   [   0,   0, 0] : 
- 54 : string GROUPWISE_HANDLE [   0,   0, 0] : 
- 55 : string GROUPWISE_SLOT  [   0,   0, 0] : 
- 56 : string ICQ_HANDLE  [   0,   0, 0] : 
- 57 : string ICQ_SLOT[   0,   0, 0] : 
- 58 : string JABBER_HANDLE   [   0,   0, 0] : 
- 59 : string JABBER_SLOT [   0,   0, 0] : 
- 60 : string MSN_HANDLE  [   0,   0, 0] : 
- 61 : string MSN_SLOT[   0,   0, 0] : 
- 62 : string YAHOO_HANDLE[   0,   0, 0] : 
- 63 : string YAHOO_SLOT  [   0,   0, 0] : 
- 64 : string SKYPE_HANDLE[   0,   0, 0] : 
- 65 : string SKYPE_SLOT  [   0,   0, 0] : 
- 66 : string SIP_HANDLE  [   0,   0, 0] : 
- 67 : string SIP_SLOT[   0,   0, 0] : 
- 68 : string IM_ADDRESS  [   0,   0, 0] : 
- 69 : string MEANWHILE_HANDLE [   0,   0, 0] : 
- 70 : string IRC_HANDLE  [   0,   0, 0] : 
- 71 : string SMS_HANDLE  [   0,   0, 0] : 
- 72 :  multiline ADR_STREET  [   0,   0, 0] : 
 -- element0 : "home address
"
 -- element1 : "work address"
 -- element2 : "custom address"
- 73 :  multiline ADR_ADDTL   [   0,   0, 0] : 
 -- element0 : "neighborhood"
 -- element1 : 
 -- element2 : 
- 74 :integer ADR_STREET_FLAGS [   0,   0, 0] : 
 -- element0 : 1
 -- element1 : 2
 -- element2 : 
- 75 : string ADR_STREET_LABEL [   0, n/a, 0] : 
- 76 :integer ADR_STREET_ID   [   0, n/a, 0] : 
- 77 :  multiline ADR_POBOX   [   0,   0, 0] : 
 -- element0 : "PO"
 -- element1 : 
 -- element2 : 
- 78 :  multiline ADR_CITY[   0,   0, 0] : 
 -- element0 : "City"
 -- element1 : 
 -- element2 : 
- 79 :  multiline ADR_REG [   0,   0, 0] : 
 -- element0 : "State"
 -- element1 : 
 -- element2 : 
- 80 :  multiline ADR_ZIP [   0,   0, 0] : 
 -- element0 : "ZIP"
 -- element1 : 
 -- element2 : 
- 81 :  multiline ADR_COUNTRY [   0,   0, 0] : 
 -- element0 : "Country"
 -- element1 : 
 -- element2 : 
- 82 :  multiline NOTE[   0,   0,15] : "A test contact."
- 83 : string PHOTO   [   0,   0,   516] : "�PNG

- 84 : string PHOTO_TYPE  [   0,   0, 0] : 
- 85 : string PHOTO_VALUE [   0,   0, 0] : 
- 86 : string GEO_LAT [   0,   0, 0] : 
- 87 : string GEO_LONG[   0,   0, 0] : 
- 88 : string CRYPTOENCRYPTPREF [   0,   0, 0] : 
- 89 : string CRYPTOPROTOPREF [   0,   0, 0] : 
- 90 : string CRYPTOSIGNPREF  [   0,   0, 0] : 
- 91 : string OPENPGPFP   [   0,   0, 0] : 
- 92 : string XPROPS  [   0,   0, 0] : 
 -- element0 : 
"X-PHONETIC-FIRST-NAME:John"
 -- element1 : 
"X-PHONETIC-LAST-NAME:Doe"


-- 
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.




00vcard-fieldlist.xml
Description: XML document


01vcard-profile.xml
Description: XML document
___
os-libsynthesis mailing list
os-libsynthesis@synthesis.ch
http://lists.synthesis.ch/mailman/listinfo/os-libsynthesis


Re: [os-libsynthesis] vCard group tags

2014-04-29 Thread Patrick Ohly
On Wed, 2014-04-16 at 17:34 +0200, Lukas Zeller wrote:
> Hello Patrick,
> 
> On 11.04.2014, at 12:18, Patrick Ohly  wrote:
> 
> > Google CardDAV and iOS/OS X use group tags to represent custom labels in
> > vCards. Attached is an example. Note that the Google vCard format used
> > by CardDAV is not the same as the one used by Gmail's export feature.
> > The example is from CardDAV.
> > 
> > I am trying to understand how I can support that using libsynthesis.
> > 
> > The doc mentions:
> > 
> > "groupfield"
> > [...]
> > Is there an example using this feature?
> 
> I implemented that feature back at Synthesis in 2010 for Beat's
> Android client, and what is shown in the docs (the TITLE/ORG case) is
> in fact taken from that client's config.
> 
> > Is the following field list and profile correct?
> > 
> > Field list:
> > 
> >  
> >  
> >  
> >  
> >  
> >  
> >  
> > 
> > Profile:
> > 
> >
> >  
> >
> >
> >  
> >  
> >  
> >  
> >
> > 
> > 
> > My understanding is that parsing the example from the documentation will 
> > lead to:
> > ORG = [ "myOwnCompany", "myEmployer" ]
> > TITLE = [ "boss", "employee" ]
> > GROUP-TAG-TITLE-ORG = [ "A", "B" ]
> 
> Yes, that's excactly what is supposed to happen.

Unfortunately I am not getting the group-tag field populated. I'm
attaching my field list and profile.

When parsing, I get:

[2014-04-28 19:31:37.575] Parsing: 
  * [2014-04-28 19:31:37.575] 
BEGIN:VCARD
VERSION:3.0
N:Doe;John;1;Mr.;Sr.
FN:Mr. John 1 Doe Sr.
NICKNAME:Johnny
TITLE:tester
ORG:at company
REV:2014-04-28T17:31:23Z
UID:6a5bd79f8b1cdd58
BDAY;VALUE=DATE:1970-12-30
ADR;TYPE=HOME:PO;neighborhood;home address\n;City;State;ZIP;Country
ADR;TYPE=WORK:;;work address
ADR;TYPE=OTHER:;;custom address
TEL;TYPE=WORK:business 1
TEL;TYPE=CELL:mobile
TEL;TYPE=HOME:home
TEL:main
TEL;TYPE=FAX,WORK:work fax
TEL;TYPE=FAX,HOME:home fax
item13.TEL:google voice
TEL;TYPE=PAGER:pager
item14.TEL:custom
EMAIL;TYPE=HOME,PREF:john@home.com
EMAIL;TYPE=WORK:d...@work.com
item1.EMAIL:j...@custom.com
NOTE:A test contact.
PHOTO;ENCODING=B:iVBORw0KGgoNSUhEUgAAACQXCAYAAABj7u2bBmJLR0QA/w
 D/AP+gvaeTCXBIWXMAAAsTAAALEwEAmpwYB3RJTUUH1gEICjgdiWkBOQAAAB10RVh0Q
 29tbWVudABDcmVhdGVkIHdpdGggVGhlIEdJTVDvZCVuAAABaElEQVRIx+3Wu0tcURAG8F98gRKT
 YGORRqwksJV/QOqFFIFgKgsRYbHV1larDQQCKQxpUscyhUmXJuCSNpYWPsAU6wPxHW6aWbgsu+v
 e3RUs7geHc+fON3O+M4c5HHLkyHG/eISkg5heIGmUr++hVWigyY6THlejbWSt0Bv8QBXX2MF7jK
 U4IyjjJ45xg31sYKZuw7Xv9Gh6vvXO9QbBtbGNJ8Ert+AlTURkFjQX9g5e4ykGUcBm+FaDexx2M
 UQOYhIL2Lpj09oV9CvsQgPuePj+hP037BL6M6yRSdDZHWVOcBHcEv7FvyN8xxqmeynovA1Baf4U
 VvANhyn/Uq8E/Q57ssNufhvx1QZrDHfS9p9i3sQsnscdNowXWEQlOBXMYyI4j3EavqFUzpOYl4O
 TqUJ9+NzmkbXyb6Ryfumm7Wso4it2cYXL6K6PeBmcV8E5iEvxPDjv8CyVaxQfsIfbqGIlf17k6B
 b/Ae0cnahfg6KuAElFTkSuQmCC
item4.IMPP;X-SERVICE-TYPE=GoogleTalk:xmpp:google%20talk
item5.IMPP;X-SERVICE-TYPE=AIM:aim:aim
item6.IMPP;X-SERVICE-TYPE=Yahoo:ymsgr:yahoo
item7.IMPP;X-SERVICE-TYPE=Skype:skype:skype
item8.IMPP;X-SERVICE-TYPE=QQ:x-apple:QQ
item9.IMPP;X-SERVICE-TYPE=MSN:msnim:MSN
item10.IMPP;X-SERVICE-TYPE=ICQ:aim:ICQ
item11.IMPP;X-SERVICE-TYPE=Jabber:xmpp:Jabber
item12.IMPP;X-SERVICE-TYPE=Chat-label:x-apple:custom%20chat
X-EVOLUTION-FILE-AS:Doe\, John
X-MOZILLA-HTML:FALSE
X-ABLABEL:custom-label
X-PHONETIC-FIRST-NAME:John
X-PHONETIC-LAST-NAME:Doe
item1.X-ABLabel:custom-label
item2.X-ABDATE:1971-01-01
item2.X-ABLabel:Anniversary
item3.X-ABDATE:2000-02-01
item3.X-ABLabel:custom-label
item4.X-ABLabel:Other
item5.X-ABLabel:Other
item6.X-ABLabel:Other
item7.X-ABLabel:Other
item8.X-ABLabel:Other
item9.X-ABLabel:Other
item10.X-ABLabel:Other
item11.X-ABLabel:Other
item12.X-ABLabel:Other
item13.X-ABLabel:Google Voice
item14.X-ABLabel:custom-label
item15.X-ABRELATEDNAMES:spouse
item15.X-ABLabel:Spouse
item16.X-ABRELATEDNAMES:child
item16.X-ABLabel:Child
item17.X-ABRELATEDNAMES:mother
item17.X-ABLabel:Mother
item18.X-ABRELATEDNAMES:father
item18.X-ABLabel:Father
item19.X-ABRELATEDNAMES:parent
item19.X-ABLabel:Parent
item20.X-ABRELATEDNAMES:brother
item20.X-ABLabel:Brother
item21.X-ABRELATEDNAMES:sister
item21.X-ABLabel:Sister
item22.X-ABRELATEDNAMES:friend
item22.X-ABLabel:Friend
item23.X-ABRELATEDNAMES:relative
item23.X-ABLabel:relative
item24.X-ABRELATEDNAMES:manager
item24.X-ABLabel:Manager
item25.X-ABRELATEDNAMES:assistant
item25.X-ABLabel:Assistant
item26.X-ABRELATEDNAMES:referred-by
item26.X-ABLabel:referred by
item27.X-ABRELATEDNAMES:partner
item27.X-ABLabel:Partner
item28.X-ABRELATEDNAMES:domestic partner
item28.X-ABLabel:domestic partner
it

Re: [os-libsynthesis] Duplication of data during a merge scenario

2014-04-28 Thread Patrick Ohly
On Mon, 2014-04-28 at 18:23 +0530, Rajesh Kumar Pawar wrote:
> I've been trying to work on a merging  scenario using syncevolution as
> a client.
>
> The steps that I followed in this scenario are as follows
> 
> 1.   Created a contact with TEL;CELL:12456399
> 
> 2.   Perform slow sync with server
> 
> 3.   Modify the field in client vCard as TEL;CELL:12456300
> 
> 4.   Wait for 5min to ensure sufficient difference between the
> modification time stamps.
> 
> 5.   Modified the field as TEL;CELL:12456391 on server. 
> 
> 6.   Perform a two-way sync.

In this scenario, the server decides how to handle the conflict. What
SyncML implementation are you using as server? You only mention that you
use SyncEvolution as client.

If the server is not SyncEvolution or libsynthesis based, then you need
to talk to the server implementer; there's not much that SyncEvolution
or libsynthesis can do.

At least not when sticking to a standard SyncML sync. There was an idea
a while back to do a SyncML sync in two phases: in the first phase
pretend that the client has no changes, and just retrieve changes from
the server. This way conflict resolution can be done in the client. Then
in the second phase, send the client changes (including merge results)
back to the server, hoping that no further changes had been made there
in the meantime. This never got implemented, though.

In SyncEvolution there is a similar (but not quite the same) data loss
issue when adding an address on one side and a telephone number on the
other side. I'm currently looking at that.

-- 
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.




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


Re: [os-libsynthesis] mapping between individual X- extensions and grouped extensions

2014-04-26 Thread Patrick Ohly
On Sat, 2014-04-26 at 12:30 +0200, Lukas Zeller wrote:
> The afterread/beforewrite script could do such a conversion as well,
> however for normalizing data these are executed too late on the server
> side for normalized data to be used in slow sync matching, so it'll be
> more complicated to correctly match and merge records.

What do you mean with that? The afterread script gets called after
reading and before using the field list, right? So whatever
transformation is necessary (for example, X-JABBER -> IMPP) can be done
in time before the engine processes the IMPP field.

-- 
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.




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


Re: [os-libsynthesis] SyncML Server Performance using Syncevolution

2014-04-26 Thread Patrick Ohly
On Sat, 2014-04-26 at 23:42 +0530, Sachin Gupta wrote:
> On Fri, Apr 25, 2014 at 3:29 PM, Patrick Ohly  wrote:
> > On Fri, 2014-04-25 at 15:03 +0530, Sachin Gupta wrote:
> > > Can you suggest how i can test SyncML Server performance and have 2500
> > > users/syncevolutions connecting simultaneously?
> >
> > There's no ready-made solution. You'll have to write your own scripts
> > for configuring SyncEvolution and running the desired benchmark.
> >
> > Note that each context in SyncEvolution gets its own device ID. So if
> > you want to simulate n different devices, use:
> >
> > syncevolution --configure ... client-1@client-1
> > ...
> > syncevolution --configure ... client-n@client-n
> >
> I figured so. So wrote scripts which will launch syncevo each with
> unique device ids and seperate user accounts.
> But the concern is managing these number of syncs through a time
> period. Exploring if JMETER can help me out in this.\
> Also being a process, it would not be possible to launch so many
> processes in parallel.
> Memory and CPU would be issues, right? Would need very high end
> systems for this?

I have not measured this. Try it and you'll see. My expection is that
you will need multiple client machines, though.

> Would it be possible doing this launching processes or shall i look
> into creating threads within the Syncevolution launching and
> controlling sessions from there?

I would just use multiple client machines. Much simpler and scales
perfectly.

-- 
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.




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


[os-libsynthesis] mapping between individual X- extensions and grouped extensions

2014-04-25 Thread Patrick Ohly
Hello!

I am trying to figure out how I can translate between Evolution/SyncML
vCard format and CardDAV (in particular, Google Contacts).

Google Contacts seems to follow the format as used by Apple, which is
not surprising considering that Apple iOS and OS X are probably the main
clients using CardDAV.

The key difference is that CardDAV uses IMPP (rfc 4770) for all kinds of
chat protocols, whereas Evolution and several SyncML implementations use
one kind of X- extension per protocol (X-AIM, X-JABBER, ...).

Evolution:
X-JABBER;TYPE=HOME;X-EVOLUTION-UI-SLOT=2:JABBER DOE

Google CardDAV:
item5.IMPP;X-SERVICE-TYPE=GoogleTalk:xmpp:google%20talk
item5.X-ABLabel:Other

Note that X-SERVICE-TYPE=GoogleTalk is the provider of the service.
"xmpp:" is part of the URL value and defines the chat protocol used by
the service; the rest of the value uses URL encoding of special
characters (a space in this case). The label is just that, a label
chosen by the user.

Evolution doesn't have support for custom labels, so there is nothing
exactly corresponding to X-ABLabel yet. TYPE comes close.

Same for some dates (X-EVOLUTION-ANNIVERSARY vs. X-ABDate):

Evolution:
X-EVOLUTION-ANNIVERSARY:2006-01-09

Google CardDAV:
item3.X-ABDATE:1971-01-01
item3.X-ABLabel:Anniversary
item4.X-ABDATE:2000-02-01
item4.X-ABLabel:custom-label

The exception is BDAY, which is used by both sides.

Finally, the same difference exists for related persons:

Evolution:
X-EVOLUTION-SPOUSE:Joan Doe

Google CardDAV:
item16.X-ABRELATEDNAMES:spouse
item16.X-ABLabel:Spouse

At the moment, SyncEvolution uses separate fields for AIM (AIM_HANDLE,
an array) and spouse (a single value). I'm leaning towards changing that
into a more generic field list where there is one array for IMPP, one
array for DATE, and one array for related NAMES. I think that can be
mapped fairly directly to the vCard format used by Google CardDAV. For
example:

   
   

   

   

The bigger problem will be on the Evolution side. I don't see how I can
teach libsynthesis that a IMPP entry whose protocol (encoded as part of
the value!) is xmpp maps to X-JABBER.

Should I keep the traditional JABBER_HANDLE array and move entries back
and forth between it and the IMPP array? This could be done with
incoming/outgoing resp. afterread/beforewrite scripts.

Then the traditional profile will only use the JABBER_HANDLE, as before,
whereas the new profile only uses IMPP.

For SyncEvolution<->SyncEvolution syncing the new profile should be used
because it will be more complete. Perhaps I can achieve that by offering
multiple datatypes and then letting the sync engines negotiate the most
suitable one.

-- 
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.




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


Re: [os-libsynthesis] SyncML Server Performance using Syncevolution

2014-04-25 Thread Patrick Ohly
On Fri, 2014-04-25 at 15:03 +0530, Sachin Gupta wrote:
> Hi Lukas/Patrick,
> 
> 
> I have to run some test cases using syncevolution as client to test a
> SyncML server performance. Expectation is to put load of around 2500
> users syncing concurrently for an hour.

That's a bit vague. What kind of changes are supposed to be synced
during that hour? The load will depend a lot on that. At the low end you
just have 2500 users connecting repeatedly without data changing on
client or server. The high end is open-ended; lots of items and slow
syncs will be more expensive than few items and incremental changes.

> Can you suggest how i can test SyncML Server performance and have 2500
> users/syncevolutions connecting simultaneously?

There's no ready-made solution. You'll have to write your own scripts
for configuring SyncEvolution and running the desired benchmark.

Note that each context in SyncEvolution gets its own device ID. So if
you want to simulate n different devices, use:

syncevolution --configure ... client-1@client-1
...
syncevolution --configure ... client-n@client-n

-- 
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.




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


[os-libsynthesis] vCard group tags

2014-04-16 Thread Patrick Ohly
Hello!

Google CardDAV and iOS/OS X use group tags to represent custom labels in
vCards. Attached is an example. Note that the Google vCard format used
by CardDAV is not the same as the one used by Gmail's export feature.
The example is from CardDAV.

I am trying to understand how I can support that using libsynthesis.

The doc mentions:

"groupfield"

New in 3.4: This optional attribute can be used to specify a string 
field (usually
a array or a repeating field) which represents the vCard or vCalendar 
group tag (a prefix to
the property name, separated by a dot). The group tag can be used to 
link properties together
which can occur multiple times. For example, some vCards might contain 
more than one
ORG and TITLE. Now each title belongs to a particular organisation, so 
the group tag is
used to represent that:

A.ORG:myOwnCompany
B.ORG:myEmployer

B.TITLE:employee
A.TITLE:boss

The groupfield mechanism makes sure that TITLE and ORG repetitions will 
be stored in the
same repetition index (array position if ORG and TITLE are mapped to 
arrays) according to
their group tag, even if occuring out of order in the incoming vCard.

Is there an example using this feature?

Is the following field list and profile correct?

Field list:

  
  
  
  
  
  
  

Profile:


  


  
  
  
  



My understanding is that parsing the example from the documentation will lead 
to:
ORG = [ "myOwnCompany", "myEmployer" ]
TITLE = [ "boss", "employee" ]
GROUP-TAG-TITLE-ORG = [ "A", "B" ]

The challenge will be to convert between
item6.IMPP;X-SERVICE-TYPE=AIM:aim:aim
item6.X-ABLabel:Other
and
X-AIM:aim

Custom labels will also be "fun", if that's what Evolution decides to
use (currently it is unsupported).

-- 
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.


BEGIN:VCARD
VERSION:3.0
N:Doe;John;1;Mr.;Sr.
FN:Mr. John 1 Doe Sr.
NICKNAME:Johnny
TITLE:tester
ORG:at company
REV:2014-04-11T08:43:42Z
UID:6f354d698b7ccd22
BDAY;VALUE=DATE:1970-12-30
ADR;TYPE=HOME:PO;neighborhood;home address\n;City;State;ZIP;Country
ADR;TYPE=WORK:;;work address
item1.ADR:;;custom address
TEL;TYPE=WORK:business 1
TEL;TYPE=CELL:mobile
TEL;TYPE=HOME:home
TEL:main
TEL;TYPE=FAX,WORK:work fax
TEL;TYPE=FAX,HOME:home fax
item14.TEL:google voice
TEL;TYPE=PAGER:pager
item15.TEL:custom
EMAIL;TYPE=HOME,PREF:john@home.com
EMAIL;TYPE=WORK:d...@work.com
item2.EMAIL:j...@custom.com
NOTE:A test contact.
PHOTO;ENCODING=B:/9j/4AAQSkZJRgABAQAAAQABAAD/4QDSRXhpZgAASUkqAAgCADEBAg
 AHJgAAAGmHBAABLgBQaWNhc2EAAAIAAJAHAAQwMjIwhpIHAH0AAABMA
 AAAICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
 ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICA
 gICAgICAgICAgICAgICAgICAgAP/bAIQAAwICCAgICAgICAgICAgICAgICAgICAgICAgICAgIBw
 gIBwcHBwcHBwcHBwcHCgcHBwgJCQkHBwwMCggMBwgJCAEDBAQGBQYKBgYKDQwMDA0MDAwMDAgIC
 AgIDAgIDAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgI/8AAEQgCgAKAAwEiAAIR
 AQMRAf/EAB0AAAICAwEBAQIDBAUBBgcACAn/xABFEAACAQMDAwIEBAQEBAMHBQE
 AAQIDBBEFITEGEkFRYQcTInEIMoGRFEKhsSNSwfAVYtHhM3LxFhgkQ0SCkgk0U2PSF//EABsBAA
 IDAQEBAAACAQMEBQYH/8QAKxEBAQACAQQDAAICAQQDAAECEQMEEiExBRNBI
 lEyYRQjQoGxUpGh/9oADAMBAAIRAxEAPwD5IyEjOAkj1rmMIZFHkgoocMoykZSCigKzBDFEwojH
 EEUAaPYMpExErMUGonohJDDbyPSQUUZcQGwxQ1I9BBpAXYcBJBJBKIDbBlIJRM9oxWIoOKMxiMh
 EAxFBRRlRDjEDb09FDInoxCSDRbRQPJYCiYk/ATwJXnPJ6nAFQwZVX6sDWoo0tsHp1CJ/E/U0Rq
 txnZC2T2skXtlVzBnu/CyI0uf0uOdyJe3vY8N7Ln7Fdz0jVWDnwZUe2e7xng1266uoucYKaxjd5
 Nc1L4kU4N9zz2vZ+wt5sf03ZXSatfH3FT1KK5a/7nGKvxpeW8ZXCNX6h+JlSrtH6UZ8+q456XTp
 9u+aj1nSpLum1+5RT+Mdo0/D+5843GqVpbym5L38EaKT3Zhz62z0unTvoH//ALPa5xLj1JVP42W
 8WllYfufOPYm8I9s+FuimdfyG+h9SaD8VqEqm81hm+2+vUJ7xqxa+58QUrlLjZr3LGy6krQbcak
 kvuaMevv6S8D7dtLiEt4tE7HufGmkfE+6pbxm2l4Z0rRPxEvEfnRx9tzbxdbjldVVl076CUMLKe
 56Fxl5xn1OSW/x1t6jSb7Y+pc1/iDSh2zp1FKL533Nc6nCqvprplX8ufPgfaSTjnG65OZ6L8VaN
 RuDlzsmb1ouqR7eUWTmwpbx1bSiNoLBFp105LcbVvI96SaLcbFdx0mxQyKFjEWUWsNBRR7AcYi7
 pdsxCSMxiHGI2gFozCAztDjEaIoYwDSGxgeUSUPRQxRMxgOpwAAhEZGAyEBigAKjEbGIUYhxgAZ
 SM9oxRCSABURkIBxpjYwABhEZGAUIDYwABpw3GpBwgGogC1AfgGMB3aAAkHgyojVAA/PtBpGEg0
 jktr2A4nlEKMRwzFBxR5IJRGKJBRPJBJEor3aEkeCSArKRlhRM4BGwxQaR5IPtAbeijKPBpDaKw
 kGkeSDUQDyiZwEkZUQD0EHg9CIaQB5IKKMxiecgAkgu7+oiVyhbuVtl4wRbpMxTpvGCJKq0z2oX
 Ee3Kllo03qfWKjhmHKW5Xlyye1mOG203eoSjHfnwyih1nHOG8yXk5vffE6o49suVtk0mtrs1JzT
 /N4OZy9Vpqx4HdLrrelF9ykn6lFrPxFhFZg8532OQq/eHjLzyQqkpepkv

Re: [os-libsynthesis] UID + CardDAV

2014-04-16 Thread Patrick Ohly
On Wed, 2014-04-16 at 11:21 +0200, Patrick Ohly wrote:
> This means that I can't put properties that are never meant to be sent
> over SyncML into the profile used for syncing. I need to maintain a
> separate profile with UID enabled that gets used for 
> MAKETEXTWITHPROFILE/PARSETEXTWITHPROFILE.

Actual, I can also simply disable the property in CtCap with
showindev="no". That works fine for the UID in vCard case.

I also looked into the profile pre-processing and might end up
implementing that, too, because it is much nicer to figure out what the
actual profile is that gets used. Compare this:


 

  

 
 

  


  



With this:



  

  


  

  


  

  


It's not necessarily shorter, but much easier to read and understand. I
made up the if/else here. The "if" attributes are meant to be combined
with "or".

-- 
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.




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


Re: [os-libsynthesis] UID + CardDAV

2014-04-16 Thread Patrick Ohly
On Fri, 2014-04-11 at 16:52 +0200, Patrick Ohly wrote:
[ shows up in CtCap]
> Could it be a bug that the disable property shows up in the CtCap?
> 
> Darn, probably a circular dependency again: we have to be ready to send
> CtCap before we know the peer, so the rule mechanism (which depends on
> knowing the peer) can't be used to influence the CtCap. Right?

A closer look at the CtCap seems to confirm that. For example,
SyncEvolution knows that KDE uses X-KADDRESSBOOK-X-Profession instead of
ROLE, so it has:




  


  


This is a somewhat convoluted way of saying that vCard ROLE is not
active for KDE (no "value" specied) and that X-KADDRESSBOOK-X-Profession
is to be used instead.

In the CtCap, both properties show up:

ROLE1
X-KADDRESSBOOK-X-Profession1

This means that I can't put properties that are never meant to be sent
over SyncML into the profile used for syncing. I need to maintain a
separate profile with UID enabled that gets used for 
MAKETEXTWITHPROFILE/PARSETEXTWITHPROFILE.

I wonder whether it makes sense to have X-KADDRESSBOOK-X-Profession
appear in the CtCap. Probably not, because SyncEvolution will not use it
during syncing, only during storing in the datastore. I probably need to
change the way how SyncEvolution adapts the main profile for specific
storages: instead of setting rules in MAKETEXTWITHPROFILE or
PARSETEXTWITHPROFILE, it needs to do its own custom pre-processing to
turn the main profile into a "KDE" profile or an "EDS" profile.

-- 
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.




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


Re: [os-libsynthesis] Photo Data getting truncated

2014-04-09 Thread Patrick Ohly
On Wed, 2014-04-09 at 19:13 +0530, anuj chauhan wrote:
> Hi Patrick,
> 
> 
> I am trying to syncronize image data with funambol server but while
> sending the data to server syncevolution is truncating the image data
> to somewhere
> 200-300 bytes.The actual image data was 2423 bytes.
> 
> 
> Below is snippet of what  am trying to send(length of image data is
> 2423 bytes) :
> 
> 
> BEGIN:VCARD
> N:lastname3;firstname3;Kumar;Mr;Phd
> EMAIL;WORK;INTERNET:wo...@hotmail.com
> EMAIL;HOME;INTERNET:ho...@hotmail.com
> URL:http://www.care1.com
> ADR;HOME:;;Caprio;Demalio;Romania;RD3258;USA
> ADR;WORK:;;Caprio;Demalio;Romania;RD3258;USA
> PHOTO;ENCODING=b;TYPE=JPEG:/9j/4AAQSkZJRgABAQAAAQABAAD/2wCEAAkGBxMTEhQUExQWFRQXGBUUFxUUFxQUFRUUFBQXFhUVFBQYHCggGBwlHBQUITEhJSkrLi4uFx8zODMsNygtLisBCgoKDg0OGhAQGywkHxwsLCwsLCwsLCwsLCwsLCwsLCwsLCwsLCwsLCwsLCwsLCwsLCwsLCwsLCwsLCwsLCwsLP/AABEIALcBEwMBIgACEQEDEQH/xAAcAAABB<...remainder
>  of "B" encoded binary data...>
> END:VCARD

There is no VERSION:3.0 in this example. Are you sure that the engine is
parsing it as vCard 3.0?

Have you checked that base64 decoding worked as expected? Look at the
field list dump at loglevel=4.

If all of that worked, can you attach the full test data?


-- 
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.




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


[os-libsynthesis] merging of winning and loosing items

2014-04-09 Thread Patrick Ohly
Hello!

I'm currently writing tests for handling of conflicts. This wasn't done
earlier because SyncEvolution's test suite primarily targeted syncing
with SyncML servers, and behavior of those was out of control anyway.
With local syncing and/or SyncEvolution as HTTP server that's different.

My main concern at the moment are update/update conflicts where both
sides updated the same common item before syncing. libsynthesis
determines the winning item based on age (REV in vCard, LAST-MODIFIED in
iCalendar). That determines which side's single-value properties are
preserved when merging them isn't possible. I already noticed that I
need tests for both cases, because a particular SyncEvolution bug
(https://bugs.freedesktop.org/show_bug.cgi?id=77065) only shows up in
one of the two cases.

How does this apply to arrays? Consider the following scenario: the
client adds a new TEL, the server a new EMAIL (in this order). What I
observe is that the server's item wins, preserving the server's EMAIL,
while the client's TEL array gets overwritten with the server's less
complete TEL array. The new TEL from the client thus gets lost.

That is with the following profile:

  
  
   
   
   

  

  
  
   
   
   

  

I suspect that in this case the engine really has no choice other than
treating the entire TEL array as one value. Even if it knew how to merge
the two TEL arrays (which isn't that simple when one also takes
reordering and intentional removal of entries into account), would it
know that the consecutive arrays (TEL and TEL_* resp. EMAIL and EMAIL_*)
are related?

I don't think so. That's something that is specified in the profile, but
not in the field list.

So a merge script will be needed if special handling of TEL and EMAIL is
desired, right?

The problem that I was alluding to earlier is that merging

winning: TEL  = [ '1', '2' ] and
loosing: TEL = [ '1', '2', '3' ]
has no unambiguous solution. Was '3' added on the loosing side or was it
removed on the winning one?

If in doubt, probably the best approach is to pick the one which avoids
data loss, i.e. preserve '3'.

Now, consider reordering:
winning: TEL = [ '1', '2' ] and
loosing: TEL = [ '2', '1' ].

Were the entries merely reordered or were the values changed on the
loosing side? In the former case, nothing needs to be done and no
information gets lost. In the later case, the changes made on the client
get lost. I don't see a way around that (*).

Copying extra values from the loosing side can work with reordering by
looking at each value and not just at the end of the array. It's not
perfect, though, when values were edited.

Any comments so far?

(*) A three-way merge where previous copies of client and server items
are available to compute a diff would work, but that increases storage
requirements for rare situations and would  require significant changes
to the engine.

-- 
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.




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


Re: [os-libsynthesis] How to synchronise image with server

2014-04-07 Thread Patrick Ohly
On Mon, 2014-04-07 at 18:00 +0530, Anuj wrote:
> Hi Patrick,
> Thanks for replying ,
> I have a doubt on where to define value =uri
> In mime profile or somewhere else.please specify.

Please follow http://www.ietf.org/rfc/rfc2426.txt for formatting your
PHOTO data.

-- 
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.




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


Re: [os-libsynthesis] How to synchronise image with server

2014-04-04 Thread Patrick Ohly
On Fri, 2014-04-04 at 05:28 -0500, anuj chauhan wrote:
> Hi Patrick,
> 
> 
> I created a photo field as mentioned
> on https://tools.ietf.org/html/rfc6350#page-30.Below is sample for
> that
> 
> 
> PHOTO:file:///home/droot/evol_store1/photos/Blue hills.jpg.
> 
> 
> 
> but when i tracked the outgoing log I found that syncevolution has
> converted the string "file:///home/droot/evol_store1/photos/Blue
> hills.jpg." into base64 format and sent the url for syncronization,

A full log at loglevel=4 would be useful in cases like this.

However, after looking at 04vcard-photo-inlining.xml I have a hunch: you
don't specify VALUE=uri explicitly, and the script checks for that.

That follows http://www.ietf.org/rfc/rfc2426.txt which says "The default
is binary value." You used vCard 4.0, which is not used or supported by
SyncEvolution. vCard 4.0 has a different definition of the PHOTO
property (value is always a URI and inline data is represented as a
special URI).

-- 
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.




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


Re: [os-libsynthesis] How to synchronise image with server

2014-04-02 Thread Patrick Ohly
On Wed, 2014-04-02 at 13:32 +0530, Anuj wrote:
> Hi ,
> I want to synchronise image with server but have no clue of its 
> representation in 
> vCard.Do I need to put URL of image in vCard photo field or entire
> image data need to be put in vCard.kindly guide me on this issue 
> I am using file based backend for syncevolution.

You can either base64 encode the photo data as specified by
https://tools.ietf.org/html/rfc6350#page-30 or you can use a file URL.
SyncEvolution then will read the file data and inline it before sending
to the server.

-- 
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.




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


Re: [os-libsynthesis] issues with combination of TYPE

2014-03-12 Thread Patrick Ohly
On Wed, 2014-03-12 at 19:52 +0530, Rajesh Kumar Pawar wrote:
> Hi Patrick,
> 
> Thank you for the quick reply,
> 
> I am not clear on where exactly I could find the entries in the log
> related to the data being parsed and scripts manipulating the data in
> vCard
> 
> if you could suggest me some keywords that might help.

Run with loglevel=4, then unfold the entire log (++ buttons at the top
of the log), then search for "Parsing". That shows the incoming data.
Below it is how the engine processes the data.

-- 
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.




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


Re: [os-libsynthesis] issues with combination of TYPE

2014-03-12 Thread Patrick Ohly
On Wed, 2014-03-12 at 15:30 +0530, rajesh kumar pawar wrote:
> Before sync:
>EMAIL;WORK;INTERNET:w...@email.com
>EMAIL;HOME;INTERNET:h...@email.com
>EMAIL;INTERNET:inter...@email.com
> 
> After sync:
>EMAIL;INTERNET:w...@email.com
>EMAIL;INTERNET:h...@email.com
>EMAIL;INTERNET:inter...@email.com
> 
> Log shows the following :
> 
> - 17 :  multiline EMAIL   [1000, 100, 0] : 
>  -- element0 : "w...@email.com"
> -- element1 : "h...@email.com"
>  -- element2 : "inter...@email.com"
> - 18 :  integer EMAIL_FLAGS   [   0, 100, 0] : 
>  -- element0 : 4
>  -- element1 : 4
>  -- element2 : 4

What does the log say about the data that it is parsing and then
manipulating via scripts, eventually leading to the part you quoted
here?

-- 
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.




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


Re: [os-libsynthesis] How to build a installation package for syncevoltion client for ubuntu

2014-03-05 Thread Patrick Ohly
On Tue, 2014-03-04 at 16:33 +0530, anuj chauhan wrote:
> w.r.t previous mail, i want to ask that how do i build a .deb package
> from the source compiled locally?
> 
> I can see that there are options to make or make install, but could
> not find any command to build a .deb package.
> 
> 
> normally there are options to build a package using buildpkg command,
> but i could not find the same here.

It is not normal for autotool's based projects to have make targets for
packaging in distro-specific formats like .rpm or .deb. This is
something that distros add when taking upstream source from a git repo
or source tar balls.

In the case of SyncEvolution, there is a "make deb" target. It's meant
to be used as part of the nightly testing and depends on CheckInstall
being installed. Your mileage may vary.

You are probably better off learning how to build a .deb package and
doing that the normal way - see Debian HOWTOs.

-- 
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.




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


[os-libsynthesis] race condition with multithreaded engine

2014-02-04 Thread Patrick Ohly
Hello!

In SyncEvolution, I am allowing the usage of a background thread for the
initialization of a datastore. One of the nightly tests ran into a crash
related to that. The TestHTTP.testAbortThread test aborts a sync before
or while it still starts up. It looks like, depending on timing (failure
is rare - I don't remember seeing it before), the background thread
still runs while the engine shuts down. The exact call stack when that
goes bad is this:

==17106== Invalid write of size 2
==17106==at 0x486A1EB: sysync::TScriptContext::ExecuteScript(std::string 
const&, sysync::TItemField**, bool, sysync::TFuncTable const*, void*, 
sysync::TMultiFieldItem*, bool, sysync::TMultiFieldItem*, bool, bool) 
(scriptcontext.cpp:4405)
==17106==by 0x486DDDC: sysync::TScriptContext::executeTest(bool, 
sysync::TScriptContext*, std::string const&, sysync::TFuncTable const*, void*, 
sysync::TMultiFieldItem*, bool, sysync::TMultiFieldItem*, bool) 
(scriptcontext.cpp:3545)
==17106==by 0x47D8206: sysync::TPluginApiDS::apiReadSyncSet(bool) 
(pluginapids.cpp:1047)
==17106==by 0x4810798: sysync::TCustomImplDS::implStartDataRead() 
(customimplds.cpp:1708)
==17106==by 0x4878A41: sysync::TStdLogicDS::performStartSync() 
(stdlogicds.cpp:313)
==17106==by 0x4879434: sysync::StartSyncThreadFunc(sysync::TThreadObject*, 
unsigned long) (stdlogicds.cpp:464)
==17106==by 0x47F636D: sysync::TThreadObject::execute() 
(platform_thread.cpp:254)
==17106==by 0x47F68B7: PosixThreadFunc (platform_thread.cpp:51)
==17106==by 0x44E9CF0: start_thread (pthread_create.c:311)
==17106==by 0x46F3C3D: clone (clone.S:131)
==17106==  Address 0xaa54860 is 72 bytes inside a block of size 584 free'd
==17106==at 0x402A0E8: operator delete(void*) (in 
/usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==17106==by 0x4868C53: sysync::TScriptContext::~TScriptContext() 
(scriptcontext.cpp:2394)
==17106==by 0x4813046: sysync::TCustomImplDS::InternalResetDataStore() 
(customimplds.cpp:1006)
==17106==by 0x47DBE11: sysync::TPluginApiDS::dsResetDataStore() 
(customimplds.h:399)
==17106==by 0x483DB64: sysync::TLocalEngineDS::engResetDataStore() 
(localengineds.cpp:1370)
==17106==by 0x482DF41: 
sysync::TLocalEngineDS::engTerminateDatastore(unsigned short) 
(localengineds.cpp:4520)
==17106==by 0x47D3481: sysync::TPluginApiDS::announceAgentDestruction() 
(pluginapids.cpp:312)
==17106==by 0x48A1682: sysync::TSyncSession::announceDestruction() 
(syncsession.cpp:1334)
==17106==by 0x47D046A: sysync::TPluginApiAgent::TerminateSession() 
(pluginapiagent.cpp:338)
==17106==by 0x47F14DD: 
sysync::TServerEngineInterface::CloseSession(sysync::SessionType*) 
(enginesessiondispatch.cpp:434)
==17106==by 0x48229AA: sysync::CloseSession(void*, sysync::SessionType*) 
(engineentry.cpp:113)
==17106==by 0x42E4379: 
sysync::TEngineModuleBridge::CloseSession(sysync::SessionType*) 
(enginemodulebridge.cpp:149)
==17106==by 0x417FE8A: 
boost::detail::sp_counted_impl_pd::dispose() (SynthesisEngine.cpp:64)
==17106==by 0x807DCB7: boost::detail::shared_count::~shared_count() 
(sp_counted_base_gcc_x86.hpp:145)
==17106==by 0x4269036: SyncEvo::SyncContext::doSync() (shared_ptr.hpp:169)
==17106==by 0x426FCFC: SyncEvo::SyncContext::sync(SyncEvo::SyncReport*) 
(SyncContext.cpp:3422)
==17106==by 0x8083B17: 
SyncEvo::SessionHelper::doSync(SyncEvo::SessionCommon::SyncParams const&, 
boost::shared_ptr > const&) 
(session-helper.cpp:222)
==17106==by 0x808B55E: 
boost::detail::function::function_obj_invoker0 > const&>, 
boost::_bi::list3, 
boost::_bi::value, 
boost::_bi::value > > > >, 
bool>::invoke(boost::detail::function::function_buffer&) 
(mem_fn_template.hpp:274)
==17106==by 0x8083664: SyncEvo::SessionHelper::run() 
(function_template.hpp:1013)
==17106==by 0x807C7D7: main (sync-helper.cpp:187)

In other words, the background thread is still using the script context
that the main thread has already destroyed.

Somewhere in the main thread's call chain there needs to be a "tell
helper thread to abort (optional) and wait for it (required)". I am not
sure where to put that. Any suggestion?

doSync() should have called SessionStep(sysync::STEPCMD_ABORT) and
waited for sysync::STEPCMD_DONE or sysync::STEPCMD_ERROR, so the engine
should have had a chance to wait for the background thread before the
CloseSession() call.

-- 
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.



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


Re: [os-libsynthesis] how to configure libsynthesis syncronizing a group of contactcs.

2014-01-27 Thread Patrick Ohly
On Mon, 2014-01-27 at 07:16 -0600, anuj chauhan wrote:

> I want to synchronize a group of contacts .Is there any group-filed
> tag in libsynthesis
> 
> for contacts.or any subsection in xml configuraton for
> group-synchronization.I am completly blank on how to achieve this
> through synthesis.

Defining a group of contacts is not standardized in vCard. If you want
to synchronize it, you probably have at least one system where groups of
contacts are supported. Which system is that and how does it exchange a
group?

-- 
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.



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


Re: [os-libsynthesis] NumberOfChanges + binfileclient

2014-01-22 Thread Patrick Ohly
On Wed, 2014-01-22 at 11:41 +0100, Lukas Zeller wrote:
> So I see no easy way to run the preflight earlier :-(

Perhaps we don't have to. What about adding the NumberOfChanges later
on? That should be SyncML compliant, shouldn't it?

Below is a patch which achieves that. It results in a sync where the
first Sync command of the client has no NumberOfChanges yet. Then in the
second message, the Sync command gets continued, but this time with a
NumberOfChanges value.

That's good enough for my purposes. If the sync is so short that all
items fit into one message, there's only a short period of time without
information about the total number of changes, which is okay.

diff --git a/src/sysync/synccommand.cpp b/src/sysync/synccommand.cpp
index 3b27d63..c37e9d3 100755
--- a/src/sysync/synccommand.cpp
+++ b/src/sysync/synccommand.cpp
@@ -742,7 +742,8 @@ TSyncCommand::TSyncCommand(
   fSyncElementP->meta=NULL; // %%% no search grammar for now
   // add number of changes for SyncML 1.1 if remote supports it
   fSyncElementP->noc=NULL; // default to none
-  if (aSessionP->fRemoteWantsNOC) {
+  fRemoteWantsNOC=aSessionP->fRemoteWantsNOC;
+  if (fRemoteWantsNOC) {
 sInt32 noc = fLocalDataStoreP->getNumberOfChanges();
 if (noc>=0) {
   // we have a valid NOC value, add it
@@ -954,6 +955,17 @@ bool TSyncCommand::generateOpen(void)
   // issue command Start  with SyncML toolkit
   fPrepared=false; // force re-preparing in all cases (as this might be 
continuing a command)
   PrepareIssue(&fSyncElementP->cmdID,&fSyncElementP->flags);
+
+  // (re-)check NumberOfChanges, because in a SyncML client that number might 
not
+  // have been available right away when constructing the TSyncCommand.
+  if (!fSyncElementP->noc &&
+  fRemoteWantsNOC) {
+sInt32 noc = fLocalDataStoreP->getNumberOfChanges();
+if (noc>=0) {
+  fSyncElementP->noc=newPCDataLong(noc);
+}
+  }
+
   if (!fEvalMode) {
 #ifdef SYDEBUG
 if (fSessionP->fXMLtranslate && fSessionP->fOutgoingXMLInstance)
diff --git a/src/sysync/synccommand.h b/src/sysync/synccommand.h
index 8d9e41d..dc12877 100755
--- a/src/sysync/synccommand.h
+++ b/src/sysync/synccommand.h
@@ -365,6 +365,7 @@ private:
   // involved datastores and mode
   TLocalEngineDS *fLocalDataStoreP;
   TRemoteDataStore *fRemoteDataStoreP;
+  bool fRemoteWantsNOC;
 }; // TSyncCommand
 
 

-- 
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.



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


[os-libsynthesis] NumberOfChanges + binfileclient

2014-01-21 Thread Patrick Ohly
Hello!

I am wondering about NumberOfChanges information sent from a Synthesis
client to a Synthessi server. Normally this is not really interesting,
but in my case the client and server are both running locally and the
user is actually controlling the server side, so having the client's
information would be useful in that case.

What I see is that the client side calls
TBinfileImplDS::getNumberOfChanges() inside
TLocalEngineDS::engClientStartOfSyncMessage(). If getNumberOfChanges()
returned a valid number here, it would get added to the Sync header. But
TBinfileImplDS does not have a value yet. The value gets computed only
later, in TBinfileImplDS::changeLogPreflight().

The engine only calls TBinfileImplDS::getNumberOfChanges() once more for
local progress events.

Is there a chance to make the SyncML NumberOfChanges feature work for
binfile clients? Any idea what change might be needed?

To me it looks like a circular dependency: DB initialization doesn't
start unless syncing started, and having the information available when
syncing starts depends on initialization.

-- 
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.



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


Re: [os-libsynthesis] PHOTO + compare="never"

2013-07-10 Thread Patrick Ohly
On Fri, 2013-07-05 at 14:05 +0200, Lukas Zeller wrote:
> On 05.07.2013, at 11:58, Patrick Ohly  wrote:
> 
> > [...]
> > I broke MERGEFIELDS() when introducing that mode, see attached patch. It
> > is now still not backward-compatible. Is there a way to have a builtin
> > function with an optional parameter?
> 
> There is, see the OPTVAL() in the param definitions, for example param_find 
> in scriptcontext.cpp.
> In the function implementation, you can test for isAssigned() to see if the 
> parameter was specified
> in the call or not.

I've implemented that now in the patch that'll appear in the FDO master
git branch soon.

-- 
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.



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


Re: [os-libsynthesis] PHOTO + compare="never"

2013-07-05 Thread Patrick Ohly
On Thu, 2013-07-04 at 15:56 +0200, Lukas Zeller wrote:
> On 04.07.2013, at 12:01, Patrick Ohly  wrote:
> 
> > I just noticed one aspect of the example configs that I wasn't aware of:
> > syncclient_sample_config.xml:   > compare="never" merge="fillempty"/>
> > 
> > [...]
> > 
> > What was the rationale for using that mode for PHOTO? Is it for storages
> > which store photo data after re-encoding it?
> 
> Exactly. For the mobile platforms I wrote clients, I never had an
> address book which did not somehow mangle (re-encode, resize, etc.)
> the data.

I see. For SyncEvolution, all local storages faithfully store the data
as-is, so it makes more sense to really compare data, even if it is more
costly.

Defining PHOTO as "blob" did not have any advantages for me, because in
SyncEvolution its content always comes from parsing a text item and thus
it is in memory anyway. I switched to "string" to get rid of the
"Winning and loosing Field 'PHOTO' not equal: '' <> ''" that I mentioned in my other email and to simplify the script
(now I can compare against EMPTY instead of having to check the size).

> > But with a storage that stores the data as-is, comparing it is
> > better (IMHO), because modified data actually gets stored. The downside
> > is that comparisons become more expensive.
> 
> That's another reason. Comparison causes the BLOB contents to be
> pulled from storage (server side), which can be quite expensive.

Actually, it seems that comparisons are simply not done, instead giving
a sometimes false "does not match" result.

> > Speaking of comparisons,  with EDS this is slightly tricky. Inlined data
> > gets replaced with a file reference, so what the comparison really needs
> > to do is not comparing
> > "file:///tmp/testing/temp-testpim/data/evolution/addressbook/pim-manager-testsync-testcontacts-foo/photos/pas_id_51D53D180001_photo-file1.image%252Fjpeg"
> >  against the binary data in the incoming item, but rather the content of 
> > that file.
> > 
> > I probably need to write a  for that, right? In that case
> > compare="never" may be the right thing to do again, but I am not sure.
> 
> There's a compare="script" for that (might not be in the docs because
> that was added later, if I correctly remember somehow related to
> SyncEvolution...)

I remember vaguely that we discussed it, but I am not using at the
moment and as far as I can tell, also don't need it for PHOTO. I don't
even need a special comparescript. Instead I copy the fields in a
special mergescript.

Here's my field list:


  
  
  

And here's the merge script:

   
   

Does that make sense? When would I need a comparescript for PHOTO?

In the script above, the "mode == 1" happens when caching items and
completely overwriting the server side during a slow sync.

Note that this script depends on a few changes and fixes for mergescript
support. In particular, the merge mode must be available and passed
through to MERGEFIELDS().

I broke MERGEFIELDS() when introducing that mode, see attached patch. It
is now still not backward-compatible. Is there a way to have a builtin
function with an optional parameter? If not, then should I introduce a
MERGEFIELDS() without parameter and a MERGEFIELDS_WITH_MODE(mode)?

-- 
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 c122950f3cca690e68c913eaaa3ae623e294c443 Mon Sep 17 00:00:00 2001
From: Patrick Ohly 
Date: Fri, 5 Jul 2013 09:04:15 +0200
Subject: [PATCH 3/4] engine: fix MERGEFIELDS()

Commit de7ff9 introduced a new mode of operation for merging, where
one side gets updated to be identical to the other side, regardless of
what the field list says.

That commit broke MERGEFIELDS() is several ways:
- The extra parameter was not declared in TBuiltInFuncDef
  and therefore all attempts to call MERGEFIELDS() from
  a script caused a parsing error (something like "closing
  bracket expected").
- The parameter was meant to be optional, but not passing it
  caused a crash because aFuncContextP->getLocalVar(0) then
  is NULL.

This commit fixes these issues, but it still doesn't make the new
parameter optional. That doesn't seem to be possible with the current
TBuiltInFuncDef? Therefore old scripts calling MERGEFIELDS() without
parameter need to be changed to MERGEFIELDS(0).
---
 src/sysync/multifielditemtype.cpp |8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/sysync/mul

Re: [os-libsynthesis] temporary local ID + FinalizeLocalID

2013-07-05 Thread Patrick Ohly
On Wed, 2013-06-26 at 16:10 +0200, Patrick Ohly wrote:
> On Mon, 2013-06-10 at 12:35 +0200, Lukas Zeller wrote:
> > On 10.06.2013, at 11:57, Patrick Ohly  wrote:
> > > I'll probably try something else: if commands were delayed in a
> > > message which is marked Final, then force execution of the commands at
> > > the end of processing that message and issue their Status commands, just
> > > as syncing normally would.
> > 
> > Sounds like a good compromise to me.
> 
> I've implemented that. I've run this through a full test against
> different peers, without problems. Or rather, there were problems
> earlier, which shows that the tests covered relevant corner cases ;-)
> These problems have been addressed.

After adding more tests, I found one more problem: the combination of
 and queuing preceding items failed. Patch attached. I intend
to squash it into the "preserve status order" patch before including FDO
master branch.

-- 
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 1f01b69ee1aa597924f90c48a299db254a0cd5d9 Mon Sep 17 00:00:00 2001
From: Patrick Ohly 
Date: Mon, 1 Jul 2013 08:52:47 +0200
Subject: [PATCH 1/4] merge: ordered status

Add/Replace commands which contain incomplete items are special: they
must trigger a Status 213 response in the same message, which cannot
be done without processing the previously queued commands first.

We could try to queue the incomplete command without processing it at
all, but that would change the response in a way that might be
unexpected by the peer (although legal). Better play it safe and
finish all queued commands, then process the command.
---
 src/sysync/synccommand.cpp |   18 ++
 src/sysync/synccommand.h   |4 
 src/sysync/syncsession.cpp |7 +++
 3 files changed, 29 insertions(+)

diff --git a/src/sysync/synccommand.cpp b/src/sysync/synccommand.cpp
index ef363d1..3b27d63 100755
--- a/src/sysync/synccommand.cpp
+++ b/src/sysync/synccommand.cpp
@@ -2289,6 +2289,24 @@ missingeoc:
 } // TSyncOpCommand::AddNextChunk
 
 
+// SyncOp commands can execute out of order except when they
+// contain chunked items, because then we would have to issue
+// a 213 Status immediately, which would violate the ordering
+// of Status replies.
+bool TSyncOpCommand::canExecuteOutOfOrder()
+{
+  SmlItemListPtr_t *itemnodePP=&(fSyncOpElementP->itemList);
+  while (*itemnodePP) {
+SmlItemListPtr_t thisitemnode = *itemnodePP;
+if (thisitemnode->item &&
+thisitemnode->item->flags & SmlMoreData_f) {
+  return false;
+}
+itemnodePP = &(thisitemnode->next);
+  }
+  return true;
+}
+
 // execute command (perform real actions, generate status)
 // returns true if command has executed and can be deleted
 bool TSyncOpCommand::execute(void)
diff --git a/src/sysync/synccommand.h b/src/sysync/synccommand.h
index bc5869c..c070905 100755
--- a/src/sysync/synccommand.h
+++ b/src/sysync/synccommand.h
@@ -90,6 +90,9 @@ public:
   #endif
   // - analyze command (but do not yet execute)
   virtual bool analyze(TPackageStates aPackageState) { fPackageState=aPackageState; return true; }; // returns false if command is bad and cannot be executed
+  // - execute() can be called even if there are other already queued commands.
+  //   True by default, exceptions must be defined explicitly.
+  virtual bool canExecuteOutOfOrder() { return true; }
   // - execute command (perform real actions, generate status)
   virtual bool execute(void); // returns true if command could execute, false if it must be queued for later finishing (next message)
   // - get number of bytes that will be still available in the workspace after
@@ -413,6 +416,7 @@ public:
   virtual ~TSyncOpCommand();
   virtual bool isSyncOp() { return true; };
   virtual bool analyze(TPackageStates aPackageState);
+  virtual bool canExecuteOutOfOrder();
   virtual bool execute(void);
   #ifndef USE_SML_EVALUATION
   // - get (approximated) message size required for sending it
diff --git a/src/sysync/syncsession.cpp b/src/sysync/syncsession.cpp
index 870342a..a394bed 100644
--- a/src/sysync/syncsession.cpp
+++ b/src/sysync/syncsession.cpp
@@ -2549,6 +2549,13 @@ Ret_t TSyncSession::process(TSmlCommand *aSyncCommandP)
 delayExecUntilNextRequest(aSyncCommandP);
   }
   else {
+if (fDelayedExecutionCommands.size()>0 &&
+!aSyncCommandP->canExecuteOutOfOrder()) {
+  PDEBUGPRINTFX(DBG_SESSION,("%s: command cannot be executed with other commands already delayed -> flush queue",aSyncCommandP->getName()));
+  fCmdIncoming = NU

Re: [os-libsynthesis] PHOTO + compare="never"

2013-07-04 Thread Patrick Ohly
On Thu, 2013-07-04 at 13:01 +0300, Patrick Ohly wrote:
> Hello!
> 
> I just noticed one aspect of the example configs that I wasn't aware of:
> syncclient_sample_config.xml:   compare="never" merge="fillempty"/>
> 
> What I see in a slow is that if all fields are equal except for the
> photo, the modified photo is not stored because merging considers it not
> relevant (compare="never" => eqm_none).
> 
> According to the doc:
> "never": field is not compared at all. This is for fields that do not 
> contain user data, such
> as "REV" in vCard. It would not make sense to compare these fields, 
> as they are not rele-
> vant for finding out if two objects have the same data or not.
> 
> 
> What was the rationale for using that mode for PHOTO? Is it for storages
> which store photo data after re-encoding it? With such a storage, the
> comparison would yield a false "field is different", causing unnecessary
> writes. But with a storage that stores the data as-is, comparing it is
> better (IMHO), because modified data actually gets stored. The downside
> is that comparisons become more expensive.

When using compare="conflict" fot PHOTO, I get

[2013-07-04 14:35:45.739] Winning and loosing Field 'PHOTO' not equal: '' <> ''
[2013-07-04 14:35:45.739] - updated fields such that both have same value 
''

and PHOTO == EMPTY in a script also does not work. Shouldn't a blob
comparison compare the content (= byte string) and treat a blob of size
0 as empty?

-- 
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.



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


[os-libsynthesis] PHOTO + compare="never"

2013-07-04 Thread Patrick Ohly
Hello!

I just noticed one aspect of the example configs that I wasn't aware of:
syncclient_sample_config.xml:  

What I see in a slow is that if all fields are equal except for the
photo, the modified photo is not stored because merging considers it not
relevant (compare="never" => eqm_none).

According to the doc:
"never": field is not compared at all. This is for fields that do not 
contain user data, such
as "REV" in vCard. It would not make sense to compare these fields, as 
they are not rele-
vant for finding out if two objects have the same data or not.


What was the rationale for using that mode for PHOTO? Is it for storages
which store photo data after re-encoding it? With such a storage, the
comparison would yield a false "field is different", causing unnecessary
writes. But with a storage that stores the data as-is, comparing it is
better (IMHO), because modified data actually gets stored. The downside
is that comparisons become more expensive.

Speaking of comparisons,  with EDS this is slightly tricky. Inlined data
gets replaced with a file reference, so what the comparison really needs
to do is not comparing
"file:///tmp/testing/temp-testpim/data/evolution/addressbook/pim-manager-testsync-testcontacts-foo/photos/pas_id_51D53D180001_photo-file1.image%252Fjpeg"
 against the binary data in the incoming item, but rather the content of that 
file.

I probably need to write a  for that, right? In that case
compare="never" may be the right thing to do again, but I am not sure.
Won't it lead to the situation again where MERGEFIELDS() incorrectly not
marks an item as changed even though it was?

-- 
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.



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


[os-libsynthesis] redundant DB updates - script for processing incoming items from SyncML peer?

2013-06-11 Thread Patrick Ohly
Hello!

When storing contacts in Evolution Data Server, SyncEvolution applies a
 which massages the contact such that it meets
Evolution conventions. For example, the VOICE flag must be set for TEL,
because otherwise Evolution ignores the number.

This leads to the following situation:
  * Contact with TEL;TYPE=WORK:1234 is received.
  * It gets processed and before writing, the flag is set =>
TEL;TYPE=WORK,VOICE:1234
  * The next sync is a slow sync, so the incoming item (absolutely
unmodified!) gets compared against the one from the datastore.
  * Because the datastores' and the incoming items TEL flags are
different, the engine concludes that the DB item must be merged
and updated in the DB.
  * Because the same  is again applied after the
merge, the exact same item gets sent as update to the DB,
without changing anything.

This leads to one unnecessary database operation, which is undesirable
when slow syncs are frequent and the storage resides on flash storage.
This is the case for syncing with PBAP in IVI.

IMHO the "adapt item to what is expected by data store" step should be
applied to the incoming item *before* processing it, in other words, in
the . If processing then doesn't break the item again,
the script does not need to run again before writing.

But the SySync_config_reference.pdf explicitly warns against that:
"Note that this is not the place to implement database specific
conversions, because this is better done in the  and
 in the  section".

Is that because normally, datatypes are shared between different data
stores?

I could avoid that by defining datatypes such that they only get used in
exactly one data store. The putting data store specific transformations
into the datatype's  would be okay, wouldn't it?

-- 
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.



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


Re: [os-libsynthesis] temporary local ID + FinalizeLocalID

2013-06-10 Thread Patrick Ohly
On Mon, 2013-06-10 at 12:35 +0200, Lukas Zeller wrote:
> On 10.06.2013, at 11:57, Patrick Ohly  wrote:
> > I'll probably try something else: if commands were delayed in a
> > message which is marked Final, then force execution of the commands at
> > the end of processing that message and issue their Status commands, just
> > as syncing normally would.
> 
> Sounds like a good compromise to me.

It seems to be working, too :-)

> > The effect will be that commands only get delayed across messages if
> > there will be more of them coming in anyway. The hope is that this will
> > avoid or at least minimize state change issues.
> > 
> > Note that the problem that I am running into also exists in the
> > unmodified code: I expect similar issues when the server delays
> > processing because of timing.
> 
> Probably not, because the server can only delay processing at the very
> beginning of the sync, while it loads the sync set in the background.
> So far, this was the only (but well tested with very long lasting sync
> set loads) case of delaying command execution. Delaying execution
> after sync has actually started is a new case.

Ah, I see. I had skipped over the
fLocalSyncDatastoreP->engIsStarted(false) check in the relevant code:

TSyncSession::processSyncOpItem()
...
  // check if we can process it now
  // Note: request time limit is active in server only.
  if (!fLocalSyncDatastoreP->engIsStarted(false) || RemainingRequestTime()<0) {
aQueueForLater=true; // re-execute later...
return true; // ...but otherwise ok
  }

-- 
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.



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


Re: [os-libsynthesis] temporary local ID + FinalizeLocalID

2013-06-10 Thread Patrick Ohly
On Mon, 2013-06-10 at 11:45 +0200, Patrick Ohly wrote:
> On Mon, 2013-06-10 at 11:27 +0200, Patrick Ohly wrote:
> > On Sun, 2013-06-09 at 12:59 +0200, Lukas Zeller wrote:
> > > Hello Patrick,
> > > 
> > > On 07.06.2013, at 16:37, Patrick Ohly  wrote:
> > > 
> > > > Couldn't the ordering of statuses be enforced by queuing the response of
> > > > commands which complete out of order, without forcing the strict
> > > > ordering of command execution?
> > > 
> > > I'd say yes - as long as the statuses arrive in the order the commands
> > > were sent, the peer doesn't see in what order the commands were
> > > actually executed.
> > > 
> > > However, there are tons of edge cases to consider, especially for
> > > split message commands () and multi-message issues in
> > > general. The SyncML state machine was already a mess in the specs, and
> > > got even more so by all the workarounds for peers that didn't work
> > > correctly (or interpreted the weak specs differently) in one or the
> > > other case.
> > > 
> > > So while out of order execution and re-ordering statuses afterwards is
> > > certainly thinkable, I don't have a firm idea how difficult that would
> > > be to implement without breaking compatibility with all the weird
> > > peers out there. My general feeling is: more complicated than it looks
> > > on the first sight.
> > 
> > My idea was to use the same delayed command queue for commands that
> > finished and those that still need to execute: when encountering a
> > finished command in that queue, the level processing the queue would
> > issue the status command already prepared earlier.
> > 
> > That way correct ordering  would always be guaranteed.
> > 
> > > > The server expected another message whereas the client closed
> > > > the connection, leading to an error in SyncEvolution's message
> > > > transport:
> > > [...]
> > > 
> > > > I am a bit at a loss stepping through the client and server states here
> > > > that led to this situation. Lukas, can you help?
> > > 
> > > I looked through the logs, but can't see right away what's going
> > > wrong. I'd guess that the problem is the updating of the fNeedToAnswer
> > > flag, which does not get properly set or reset due to the out-of-order
> > > execution.
> > 
> > What exactly does "fNeedToAnswer" mean? Initially I thought it meant "my
> > peer needs to send me an answer", because you were talking about it in
> > the context of the server sending a message and then expecting an answer
> > that the client doesn't send.
> > 
> > But after looking at the code more closely, it seems to have the meaning
> > of "I need to send my answer". Right?
> > 
> > In that case, the flag is irrelevant on the server except for (perhaps)
> > some state changes, because the server always has to reply to the
> > client.
> > 
> > The problem in my case is not that the flag is set when it shouldn't be
> > set (or vice versa), the problem is that the server expects another
> > message. I think the client is correct in not sending that message.
> > 
> > The key difference in the logs is that in the normal case, the server
> > stays in the "map" phase, while in the failure case it goes 
> > 
> > 
> > Okay:
> > 
> > [2013-06-07 15:55:13.697] ---> MessageEnded starts : old incoming 
> > state='sync', old outgoing state='sync', NeedToAnswer
> > 
> > [2013-06-07 16:14:55.625] ---> MessageEnded finishes : new incoming 
> > state='map', new outgoing state='map', NeedToAnswer
> > 
> > 
> > Failed:
> > [2013-06-07 15:55:13.697] ---> MessageEnded starts : old incoming 
> > state='sync', old outgoing state='sync', NeedToAnswer
> > 
> > [2013-06-07 15:55:13.697] ---> MessageEnded finishes : new incoming 
> > state='map', new outgoing state='sync', NeedToAnswer
> > 
> > [2013-06-07 15:55:13.777] ---> MessageEnded starts : old incoming 
> > state='map', old outgoing state='sync', NeedToAnswer
> > 
> > [2013-06-07 15:55:13.777] ---> MessageEnded finishes : new incoming 
> > state='map', new outgoing state='map', NeedToAnswer
> >  error>
> > 
> > Looking at it like that, the question bec

Re: [os-libsynthesis] temporary local ID + FinalizeLocalID

2013-06-10 Thread Patrick Ohly
On Mon, 2013-06-10 at 11:27 +0200, Patrick Ohly wrote:
> On Sun, 2013-06-09 at 12:59 +0200, Lukas Zeller wrote:
> > Hello Patrick,
> > 
> > On 07.06.2013, at 16:37, Patrick Ohly  wrote:
> > 
> > > Couldn't the ordering of statuses be enforced by queuing the response of
> > > commands which complete out of order, without forcing the strict
> > > ordering of command execution?
> > 
> > I'd say yes - as long as the statuses arrive in the order the commands
> > were sent, the peer doesn't see in what order the commands were
> > actually executed.
> > 
> > However, there are tons of edge cases to consider, especially for
> > split message commands () and multi-message issues in
> > general. The SyncML state machine was already a mess in the specs, and
> > got even more so by all the workarounds for peers that didn't work
> > correctly (or interpreted the weak specs differently) in one or the
> > other case.
> > 
> > So while out of order execution and re-ordering statuses afterwards is
> > certainly thinkable, I don't have a firm idea how difficult that would
> > be to implement without breaking compatibility with all the weird
> > peers out there. My general feeling is: more complicated than it looks
> > on the first sight.
> 
> My idea was to use the same delayed command queue for commands that
> finished and those that still need to execute: when encountering a
> finished command in that queue, the level processing the queue would
> issue the status command already prepared earlier.
> 
> That way correct ordering  would always be guaranteed.
> 
> > > The server expected another message whereas the client closed
> > > the connection, leading to an error in SyncEvolution's message
> > > transport:
> > [...]
> > 
> > > I am a bit at a loss stepping through the client and server states here
> > > that led to this situation. Lukas, can you help?
> > 
> > I looked through the logs, but can't see right away what's going
> > wrong. I'd guess that the problem is the updating of the fNeedToAnswer
> > flag, which does not get properly set or reset due to the out-of-order
> > execution.
> 
> What exactly does "fNeedToAnswer" mean? Initially I thought it meant "my
> peer needs to send me an answer", because you were talking about it in
> the context of the server sending a message and then expecting an answer
> that the client doesn't send.
> 
> But after looking at the code more closely, it seems to have the meaning
> of "I need to send my answer". Right?
> 
> In that case, the flag is irrelevant on the server except for (perhaps)
> some state changes, because the server always has to reply to the
> client.
> 
> The problem in my case is not that the flag is set when it shouldn't be
> set (or vice versa), the problem is that the server expects another
> message. I think the client is correct in not sending that message.
> 
> The key difference in the logs is that in the normal case, the server
> stays in the "map" phase, while in the failure case it goes 
> 
> 
> Okay:
> 
> [2013-06-07 15:55:13.697] ---> MessageEnded starts : old incoming 
> state='sync', old outgoing state='sync', NeedToAnswer
> 
> [2013-06-07 16:14:55.625] ---> MessageEnded finishes : new incoming 
> state='map', new outgoing state='map', NeedToAnswer
> 
> 
> Failed:
> [2013-06-07 15:55:13.697] ---> MessageEnded starts : old incoming 
> state='sync', old outgoing state='sync', NeedToAnswer
> 
> [2013-06-07 15:55:13.697] ---> MessageEnded finishes : new incoming 
> state='map', new outgoing state='sync', NeedToAnswer
> 
> [2013-06-07 15:55:13.777] ---> MessageEnded starts : old incoming 
> state='map', old outgoing state='sync', NeedToAnswer
> 
> [2013-06-07 15:55:13.777] ---> MessageEnded finishes : new incoming 
> state='map', new outgoing state='map', NeedToAnswer
>  error>
> 
> Looking at it like that, the question becomes: how does the server
> decided that it still needs an answer from the client?

Perhaps the problem is on the client side after all: in message msgID=2,
the client client sent the Add command with a Final flag. In msgID=3, it
acknowledges the server's SyncCmd with a Status, *without* a Final flag.
The server therefore (in it's MessageEnded checks) continues with the
session, whereas the client quits it.

Should the client continue to set the Final flag as long as it is
waiting for the server to finish?

-- 
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.



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


Re: [os-libsynthesis] temporary local ID + FinalizeLocalID

2013-06-10 Thread Patrick Ohly
On Sun, 2013-06-09 at 12:59 +0200, Lukas Zeller wrote:
> Hello Patrick,
> 
> On 07.06.2013, at 16:37, Patrick Ohly  wrote:
> 
> > Couldn't the ordering of statuses be enforced by queuing the response of
> > commands which complete out of order, without forcing the strict
> > ordering of command execution?
> 
> I'd say yes - as long as the statuses arrive in the order the commands
> were sent, the peer doesn't see in what order the commands were
> actually executed.
> 
> However, there are tons of edge cases to consider, especially for
> split message commands () and multi-message issues in
> general. The SyncML state machine was already a mess in the specs, and
> got even more so by all the workarounds for peers that didn't work
> correctly (or interpreted the weak specs differently) in one or the
> other case.
> 
> So while out of order execution and re-ordering statuses afterwards is
> certainly thinkable, I don't have a firm idea how difficult that would
> be to implement without breaking compatibility with all the weird
> peers out there. My general feeling is: more complicated than it looks
> on the first sight.

My idea was to use the same delayed command queue for commands that
finished and those that still need to execute: when encountering a
finished command in that queue, the level processing the queue would
issue the status command already prepared earlier.

That way correct ordering  would always be guaranteed.

> > The server expected another message whereas the client closed
> > the connection, leading to an error in SyncEvolution's message
> > transport:
> [...]
> 
> > I am a bit at a loss stepping through the client and server states here
> > that led to this situation. Lukas, can you help?
> 
> I looked through the logs, but can't see right away what's going
> wrong. I'd guess that the problem is the updating of the fNeedToAnswer
> flag, which does not get properly set or reset due to the out-of-order
> execution.

What exactly does "fNeedToAnswer" mean? Initially I thought it meant "my
peer needs to send me an answer", because you were talking about it in
the context of the server sending a message and then expecting an answer
that the client doesn't send.

But after looking at the code more closely, it seems to have the meaning
of "I need to send my answer". Right?

In that case, the flag is irrelevant on the server except for (perhaps)
some state changes, because the server always has to reply to the
client.

The problem in my case is not that the flag is set when it shouldn't be
set (or vice versa), the problem is that the server expects another
message. I think the client is correct in not sending that message.

The key difference in the logs is that in the normal case, the server
stays in the "map" phase, while in the failure case it goes 


Okay:

[2013-06-07 15:55:13.697] ---> MessageEnded starts : old incoming state='sync', 
old outgoing state='sync', NeedToAnswer

[2013-06-07 16:14:55.625] ---> MessageEnded finishes : new incoming 
state='map', new outgoing state='map', NeedToAnswer


Failed:
[2013-06-07 15:55:13.697] ---> MessageEnded starts : old incoming state='sync', 
old outgoing state='sync', NeedToAnswer

[2013-06-07 15:55:13.697] ---> MessageEnded finishes : new incoming 
state='map', new outgoing state='sync', NeedToAnswer

[2013-06-07 15:55:13.777] ---> MessageEnded starts : old incoming state='map', 
old outgoing state='sync', NeedToAnswer

[2013-06-07 15:55:13.777] ---> MessageEnded finishes : new incoming 
state='map', new outgoing state='map', NeedToAnswer
 error>

Looking at it like that, the question becomes: how does the server
decided that it still needs an answer from the client?

-- 
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.



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


Re: [os-libsynthesis] temporary local ID + FinalizeLocalID

2013-06-07 Thread Patrick Ohly
On Mon, 2013-06-03 at 22:17 +0200, Lukas Zeller wrote:
> > How about yet another approach: the store is allowed to return a new
> > error code, LOCERR_AGAIN, for add/update/delete operations. The engine
> > then remembers all operations with that return code and calls them again
> > (with the exact same parameters and without freeing resources!) at a
> > suitable time, for example the end of the current message. In the second
> > call, the store must execute or finish the operation.
> > 
> > The engine must not call an operation on an item for which it has
> > already something pending. If that happens, it has to complete the first
> > operation first. That way the store can keep a list of pending
> > operations with the ItemID as key.
> 
> Sounds like a good solution indeed. I like it better than
> late-handling return codes especially because it avoids any risk of
> breaking existing datastores (and possible unsafe assumptions in their
> implementations), because for those nothing would change outside *and*
> inside the engine.
> 
> It's also somewhat similar to the  mechanism,
> which also keeps items (or parts of them) in memory for using them
> again at the end of the session, usually to resolve cross references
> (parent/child tasks for example). Maybe some of the mechanisms can be
> re-used for LOCERR_AGAIN. 

I found a different mechanism: TSyncSession::processSyncOpItem() can
decide to delay execution of the command by setting a flag. Currently
this is done when processing the message already took too long.

I have changed the command and item processing call chain and my backend
so that it can kick of the operation and continue when the chain is
invoked again. For that, I am setting the aQueueForLater=true to use the
existing queuing mechanism for SyncML commands.

Now I found one problem with that: after the first of several Add or
Update commands got queued, all following commands are also queued. What
I'd like to see instead is that they all get processed.

Then in the level above the engine, right before sending the response, I
would gather all pending operations and combine them into a batched,
asynchronous add or update operation. The batching is expected to be
much more efficient with EDS. It also allows overlapping local
processing in the PIM storage with network IO. But right now, I always
only get one item to be batched because everything else is still in the
queue for later processing.

Is the "command received after other commands needed to be delayed ->
must be delayed, too" something which is imposed by SyncML?

I tried to think of situations where the engine needs to enforce
completion of the pending operation before triggering another one, but
couldn't think of such a situation. My expectation is that either the
backend will properly serialize the item changes or the item changes are
independent (update "foo", delete "bar").

But what could happen is that "update foo" gets started, does not
complete, and then "delete bar" gets processed right away. That would
re-order the status messages such that the status for the later command
gets sent first. That's because my backend currently can only do
insert/update asynchronously, but not deletes.

-- 
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.



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


Re: [os-libsynthesis] temporary local ID + FinalizeLocalID

2013-06-03 Thread Patrick Ohly
On Mo, 2013-06-03 at 15:12 +0200, Patrick Ohly wrote:
> Hello!
> 
> I'm trying to batch local database adds together to increase performance
> in SyncEvolution (https://bugs.freedesktop.org/show_bug.cgi?id=52669 and
> https://bugs.freedesktop.org/show_bug.cgi?id=64838#c5).
> 
> My hope was that I can postpone the actual modification and return a
> temporary local ID. Then later or in parallel, combine multiple items
> together in a batch add. Finally, the temporary ID was meant to be
> replaced by the final one in FinalizeLocalID.
> 
> But I hit the first snag pretty early: FinalizeLocalID() doesn't seem to
> get called on the server side where I need this feature primarily (for
> PBAP, CalDAV, ActiveSync the server side is the one with the local
> database).
> 
> dsFinalizeLocalID, the wrapper around the store's FinalizeLocalID, gets
> called in three functions:
> (gdb) b sysync::TBinfileImplDS::changeLogPostflight
> Breakpoint 2 at 0x106f956: file 
> /home/pohly/syncevolution/libsynthesis/src/sysync/binfileimplds.cpp, line 462.
> (gdb) b sysync::TBinfileImplDS::SaveAdminData
> Breakpoint 3 at 0x10739fd: file 
> /home/pohly/syncevolution/libsynthesis/src/sysync/binfileimplds.cpp, line 
> 2389.
> (gdb) b sysync::TLocalEngineDS::engGenerateMapItems
> Breakpoint 4 at 0x109c79c: file 
> /home/pohly/syncevolution/libsynthesis/src/sysync/localengineds.cpp, line 
> 6635.
> 
> None of these get called on the server side during a
> refresh-from-client, slow or normal two-way sync. That makes sense, map
> items are only relevant on the client, and the server is not using the
> binfile class.
> 
> But shouldn't the server also call FinalizeLocalID somewhere? Where
> would be the right place for it?
> 
> Regarding FinalizeLocalID() in the client, I see some conceptual issues
> with it, at least for what I am trying to achieve. My main concern is
> about error handling.
> 
> FinalizeLocalID() can return an error code, but dsFinalizeLocalID does
> not check for errors. It merely checks for "ID modified". So in case of
> an error, the local store itself has to remember that writing failed and
> "do something".
> 
> What that "something" could be is not clear. It could return an error in
> EndDataWrite(), in which case I would expect the engine to enforce a
> slow sync in the next sync session. I can't think of other, realistic
> options right now.
> 
> I think a better solution would be to delay handling the result of a
> database write until the entire SyncML message has been processed. Then
> give the store a chance to finalize the ID of each item that was touched
> as part of that SyncML message. The store can use that opportunity to
> flush the pending operations and return error codes for each item, just
> as it would normally do in the synchronous write operations. Then the
> engine continues with the same error handling that it already does.
> 
> The advantage, besides better error handling, is that the store has a
> natural place to flush pending operations. Without the intermediate
> FinalizeLocalID() calls a new "max pending operations" setting
> independent of the SyncML message size would be needed.

... while the extension proposed above probably would work for adding or
updating items, for deleting it's questionable - why should
FinalizeLocalID be called for a deleted item?!

How about yet another approach: the store is allowed to return a new
error code, LOCERR_AGAIN, for add/update/delete operations. The engine
then remembers all operations with that return code and calls them again
(with the exact same parameters and without freeing resources!) at a
suitable time, for example the end of the current message. In the second
call, the store must execute or finish the operation.

The engine must not call an operation on an item for which it has
already something pending. If that happens, it has to complete the first
operation first. That way the store can keep a list of pending
operations with the ItemID as key.

-- 
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.



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


[os-libsynthesis] temporary local ID + FinalizeLocalID

2013-06-03 Thread Patrick Ohly
Hello!

I'm trying to batch local database adds together to increase performance
in SyncEvolution (https://bugs.freedesktop.org/show_bug.cgi?id=52669 and
https://bugs.freedesktop.org/show_bug.cgi?id=64838#c5).

My hope was that I can postpone the actual modification and return a
temporary local ID. Then later or in parallel, combine multiple items
together in a batch add. Finally, the temporary ID was meant to be
replaced by the final one in FinalizeLocalID.

But I hit the first snag pretty early: FinalizeLocalID() doesn't seem to
get called on the server side where I need this feature primarily (for
PBAP, CalDAV, ActiveSync the server side is the one with the local
database).

dsFinalizeLocalID, the wrapper around the store's FinalizeLocalID, gets
called in three functions:
(gdb) b sysync::TBinfileImplDS::changeLogPostflight
Breakpoint 2 at 0x106f956: file 
/home/pohly/syncevolution/libsynthesis/src/sysync/binfileimplds.cpp, line 462.
(gdb) b sysync::TBinfileImplDS::SaveAdminData
Breakpoint 3 at 0x10739fd: file 
/home/pohly/syncevolution/libsynthesis/src/sysync/binfileimplds.cpp, line 2389.
(gdb) b sysync::TLocalEngineDS::engGenerateMapItems
Breakpoint 4 at 0x109c79c: file 
/home/pohly/syncevolution/libsynthesis/src/sysync/localengineds.cpp, line 6635.

None of these get called on the server side during a
refresh-from-client, slow or normal two-way sync. That makes sense, map
items are only relevant on the client, and the server is not using the
binfile class.

But shouldn't the server also call FinalizeLocalID somewhere? Where
would be the right place for it?

Regarding FinalizeLocalID() in the client, I see some conceptual issues
with it, at least for what I am trying to achieve. My main concern is
about error handling.

FinalizeLocalID() can return an error code, but dsFinalizeLocalID does
not check for errors. It merely checks for "ID modified". So in case of
an error, the local store itself has to remember that writing failed and
"do something".

What that "something" could be is not clear. It could return an error in
EndDataWrite(), in which case I would expect the engine to enforce a
slow sync in the next sync session. I can't think of other, realistic
options right now.

I think a better solution would be to delay handling the result of a
database write until the entire SyncML message has been processed. Then
give the store a chance to finalize the ID of each item that was touched
as part of that SyncML message. The store can use that opportunity to
flush the pending operations and return error codes for each item, just
as it would normally do in the synchronous write operations. Then the
engine continues with the same error handling that it already does.

The advantage, besides better error handling, is that the store has a
natural place to flush pending operations. Without the intermediate
FinalizeLocalID() calls a new "max pending operations" setting
independent of the SyncML message size would be needed.

Comments?

-- 
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.



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


Re: [os-libsynthesis] Sync mode extensions must be optional

2013-03-10 Thread Patrick Ohly
On Thu, 2012-10-11 at 12:27 +0200, Lukas Zeller wrote:
> Hi Patrick,
> 
> I found that some servers don't like the sync mode extensions in the
> devInf you introduced for SyncEvolution a while ago (crash syncs with
> misleading errors as apparently their wbxml decoders fail or
> something).
> 
> As I see no way to automatically control this, I added a session level
> config flag  that must be set explicitly to enable
> the extensions. The default is off, to make sure current libsynthesis
> (master branch) users will no see any difference in behaviour once
> they update. For SyncEvolution, you need to add this flag to your
> config.

For SyncEvolution 1.3.99.3, I've extended the SyncEvolution
"SyncMLVersion" setting to control whether the extensions are enabled.
They are enabled by default because I don't expect many users to use
affected servers, whereas more user will want it on for use with
SyncEvolution as server.

I had already done this a while ago, but then noticed a problem during
release testing which caused me to back out the changes again because I
didn't have time to investigate.

It turned out that I didn't enable the feature on the server side.
Strictly speaking, that shouldn't be necessary, because the server will
only add the extensions when the client already sent his, so there
should not be any problem. But it makes sense to have this configurable,
just in case.

The master branch of libsynthesis on freedesktop.org is now based on the
latest gitorious master branch. Lukas, it contains two patches which you
might want to include:


commit 9b0c01cf2fff6701bdb026518048fc4b06a7072e
Author: Patrick Ohly 
Date:   Thu Feb 21 00:51:53 2013 -0800

caching mode: fix memory leak

When deleting unmatched local sync items, the TSyncItem instance was
leaked after removing it from fItems.


commit 99159e0991664f8c8319e634598ea6c9bd73fcc2
Author: Patrick Ohly 
Date:   Mon Sep 10 09:15:20 2012 +0200

autotools: bumped minor version

Bumped minor version so that SyncEvolution can ensure that the version
it is compiled against really fixes the VJOURNAL<->plain text
conversion problem.

BTW, did you tag 3.4.0.47? There is a comment about the version, but not
the tag itself.

-- 
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.



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


Re: [os-libsynthesis] PBAP: one-way sync modes

2012-08-31 Thread Patrick Ohly
On Tue, 2012-08-21 at 11:31 +0200, Patrick Ohly wrote:
> For the PBAP caching mechanism in SyncEvolution I'd like to use the
> Synthesis engine. I think that makes sense because the engine does a few
> things which would have to be done manually otherwise (translate between
> formats, finding pairs). The improvements below would also make sense in
> other use cases.
> 
> At the moment the engine (or rather, SyncML) lacks a few things which I
> need to change. I'm focusing on one-way-from-client syncing because the
> server has a bit more control over what it does and because it matches
> the SyncEvolution use case.
> 
> In SyncML, incremental one-way syncs fall back to a two-way slow sync
> after a failure. If the server has an item which the client no longer
> has, then the item will be recreated on the client, despite the
> intention to only transfer data in one direction. Right?

As Lukas mentioned in another mail thread ("one-way sync + sync tokens
not updated"), the one-way sync modes are in fact meant to be part of a
two-way sync setup. They just temporarily disable sending changes in one
direction. With that intention in mind the current behavior (falling
back to two-way slow sync) makes sense again.

However, SyncEvolution doesn't support these modes properly, because its
change tracking only supports reporting of changes since the last sync.
That's a problem for another time, let's focus on one-way syncing for
PBAP.

> As a first step I would disable sending changes to the client if the
> client asked for a one-way-from-client sync. The server's datastore can
> already be marked read-only ( config option, SETREADONLY());
> something similar for the peer's datastore would make sense. Then I
> could leave the default behavior as it is and use a script function to
> trigger the desired behavior.
> 
> Actually, there is SETREFRESHONLY(). According to the docs, it is meant
> to be used for turning a two-way sync requested by the client into a
> refresh-from-remote. I need to check whether I can use that or still
> need to add/modify something.

No, SETREFRESHONLY() just changes to traditional one-way sync.

> Suppose a slow sync was done in that modified refresh-only mode. Any
> item that the server has which are not on the client need to be removed
> if the server's storage is meant to mirror the client.
> 
> At the moment, the engine cannot know whether it is meant to mirror the
> data and thus it will leave the extra items on the server unchanged. I
> intend to add a "" config option similar to "". If
> set, the engine will not only avoid sending changes to the client, it
> will also remove those extra local items.

I've done it a bit differently: instead of adding a config option,
there's now only a macro call. The full code is in the "pbap" branch. I
also pushed the patches which use this feature into the "pbap"
SyncEvolution branch.

I'm attaching the libsynthesis patches for review. Lukas, does this make
sense? Can it get included upstream?

If yes, then I would rebase onto your latest code first. I held back
with that because I wanted to release 1.3 with minimal changes.

> Finally, during a slow sync where a match was found, extra properties in
> the server's copy of an item must be removed to avoid keeping stale
> data. This can already be done by configuring the merging of items
> accordingly (instead of preserving as much data as possible, ensure that
> the client's item is always considered the "winning" one and that its
> version completely overwrites the server's).

Also done and attached.

It will be necessary to revise this handling a bit once more to cover
another PBAP use case: sometimes updating the cache is done with photos,
sometimes without (to reduce the amount of Bluetooth traffic). In the
later case the local PHOTO property must be preserved. Currently it gets
removed.

-- 
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 758c470e929594f7f5330758103fe27c285fa657 Mon Sep 17 00:00:00 2001
From: Patrick Ohly 
Date: Thu, 23 Aug 2012 13:50:36 +0200
Subject: [PATCH 1/3] engine: initial support for caching of remote data

This patch introduces support for true one-way syncing ("caching"):
the local datastore is meant to be an exact copy of the data on the
remote side. The assumption is that no modifications are ever made
locally outside of syncing. This is different from one-way sync modes,
which allows local changes and only temporarily disables sending them
to the remote side.

Another goal of t

[os-libsynthesis] PBAP: one-way sync modes

2012-08-21 Thread Patrick Ohly
Hello!

For the PBAP caching mechanism in SyncEvolution I'd like to use the
Synthesis engine. I think that makes sense because the engine does a few
things which would have to be done manually otherwise (translate between
formats, finding pairs). The improvements below would also make sense in
other use cases.

At the moment the engine (or rather, SyncML) lacks a few things which I
need to change. I'm focusing on one-way-from-client syncing because the
server has a bit more control over what it does and because it matches
the SyncEvolution use case.

In SyncML, incremental one-way syncs fall back to a two-way slow sync
after a failure. If the server has an item which the client no longer
has, then the item will be recreated on the client, despite the
intention to only transfer data in one direction. Right?

This is undesirable in general (IMHO), and an absolute no-no for PBAP as
the client, because it cannot write the changes.

As a first step I would disable sending changes to the client if the
client asked for a one-way-from-client sync. The server's datastore can
already be marked read-only ( config option, SETREADONLY());
something similar for the peer's datastore would make sense. Then I
could leave the default behavior as it is and use a script function to
trigger the desired behavior.

Actually, there is SETREFRESHONLY(). According to the docs, it is meant
to be used for turning a two-way sync requested by the client into a
refresh-from-remote. I need to check whether I can use that or still
need to add/modify something.

Suppose a slow sync was done in that modified refresh-only mode. Any
item that the server has which are not on the client need to be removed
if the server's storage is meant to mirror the client.

At the moment, the engine cannot know whether it is meant to mirror the
data and thus it will leave the extra items on the server unchanged. I
intend to add a "" config option similar to "". If
set, the engine will not only avoid sending changes to the client, it
will also remove those extra local items.

Finally, during a slow sync where a match was found, extra properties in
the server's copy of an item must be removed to avoid keeping stale
data. This can already be done by configuring the merging of items
accordingly (instead of preserving as much data as possible, ensure that
the client's item is always considered the "winning" one and that its
version completely overwrites the server's).


-- 
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.



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


[os-libsynthesis] one-way sync + sync tokens not updated

2012-08-21 Thread Patrick Ohly
Hello!

SyncEvolution's ActiveSync backend is the first one which uses the
string tokens in StartDataRead() and EndDataWrite(). The backend always
runs inside a binfile-based SyncML client.

In one particular test I am using a "one-way-from-server" sync,
initiated by the client, and noticed a problem: the new token created by
the backend in EndDataWrite() was not passed to the StartDataRead() in
the next sync.

The backend cannot handle that, because the tokens come directly from
ActiveSync, which only allows reusing old tokens in a few limited cases.
In this case, the server rejected the obsolete token, causing an
unexpected slow sync.

Full log is here:
http://syncev.meego.com/2012-08-16-17-03_all/testing-amd64/14-exchange/Client_Sync_eds_contact_testOneWayFromLocal.send.client.B/syncevolution-log.html

It has no debug output explaining the problem. I tracked it down to
this:

// save end of session state
localstatus TCustomImplDS::implSaveEndOfSession(bool aUpdateAnchors)
{
  localstatus sta=LOCERR_OK;
  PDEBUGBLOCKCOLL("SaveEndOfSession");
  // update TCustomImplDS dsSavedAdmin variables (other levels have already 
updated their variables
  if (aUpdateAnchors) {
if (!fRefreshOnly || fSlowSync) {
...
  // also update opaque reference string possibly needed in DS API 
implementations
  fPreviousToRemoteSyncIdentifier = fCurrentSyncIdentifier;
=>PDEBUGPRINTFX(DBG_ADMIN+DBG_DBAPI+DBG_EXOTIC,("updating sync token 
(fPreviousToRemoteSyncIdentifier) from %s to current sync token 
%s",fPreviousToRemoteSyncIdentifier.c_str(),fCurrentSyncIdentifier.c_str()));
} else {
=>PDEBUGPRINTFX(DBG_ADMIN+DBG_DBAPI+DBG_EXOTIC,("keeping old sync token 
(fPreviousToRemoteSyncIdentifier) %s instead of updating to current sync token 
%s",fPreviousToRemoteSyncIdentifier.c_str(),fCurrentSyncIdentifier.c_str()));
}

I added that debug output. It confirms that the "keeping old sync token"
branch is taken.

What is the rationale here? Is the goal perhaps that if the client
switches back to a two-way sync, all changes since the last two-way sync
get sent, despite the intermediate one-way sync?

Those changes include the changes made on behalf of the server during
the one-way-from-server sync. Is that filtered out?

My test is probably broken. IMHO one-way sync only makes sense into a
storage which never changes on the receiving side. In the ActiveSync
case, that cannot be guaranteed, so I might as well just skip the test.

OTOH, a user might decide to use an ActiveSync server as remote backup,
in which case one-way syncing makes sense again. Would it be acceptable
to always take the "updating sync token" branch above?

-- 
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.



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


Re: [os-libsynthesis] [SyncEvolution] Recurrences with Nth weekday of the month

2012-06-22 Thread Patrick Ohly
On Thu, 2012-06-21 at 10:51 +1000, Mathew McBride wrote:
> Hi,
> 
> I'm testing SyncEvolution against my Funambol connector (GroupDAV) at 
> the moment. I've been testing some recurrence combinations and it seems 
> recurrences of type 'N' weekday of the month (i.e second tuesday of the 
> month) aren't supported?
[...]
> The original Evolution event has this:
> RRULE:BYDAY=TU;BYSETPOS=2;COUNT=2;FREQ=MONTHLY;WKST=SU
> 
> The environment is Ubuntu 12.04 with the provided Evolution and 
> SyncEvolution versions (1.2.2).
> 
> Are these type of recurrences supported with Evolution?

In Evolution, yes, in libsynthesis, no. I can reproduce the problem and
noticed that the code (src/sysync/rrules.cpp, RRULE2toInternal()) bails
out because it doesn't recognize the "BYSETPOS" keyword.

Lukas, was that a conscious decision, perhaps because it cannot be
supported in combination with vCalendar 1.0, or just an oversight? What
would it take to support that?

-- 
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.



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


Re: [os-libsynthesis] Funambol and large SyncML messages?

2012-06-18 Thread Patrick Ohly
On Mon, 2012-06-18 at 09:30 +0200, Lukas Zeller wrote:
> occasionally, I get error reports from libsynthesis (iOS client) users
> which experience SyncML Error 511 ("server error") with
> Funambol/OneMediaHub servers. Analyzing these, it always boils down to
> a big (99kBytes) client message that the server apparently cannot
> handle. As the server does not send a MaxMsgSize, libsynthesis makes
> the message as big as fits into it's local buffer which is 100k by
> default.
> 
> My question - did you experience this large message size problem with
> Funambol as well? Or are you using a smaller message size than the
> default in SyncEvolution anyway?

The default message size is 15 bytes. I vaguely remember such issues
for myFunambol, but chalked it up to "server isn't reliable" instead of
investigating.

Testing against OneMedia has not been successful at all, as in "most
tests don't run". First there was the issue with slow syncs causing the
server to report the 411 "try again later" error, then it started
throwing XML parser errors for a (as far as I can tell so far) correct
DevInf.

> I'm just wondering if reducing the message size could reliably fix
> that Error 511 problem - and if so how small the message size should
> be.

I don't know. I guess you could try asking Funambol (their mailing list
is now on SourceForge), but be prepared to be told to install a Funambol
client and emulate its behavior (that was the response that I got for
the DevInf issue).

-- 
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.



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


Re: [os-libsynthesis] Funambol + refresh-from-server => 417

2012-06-11 Thread Patrick Ohly
On Thu, 2012-06-07 at 14:43 +0200, Lukas Zeller wrote:
> Just changing getSyncStateAlertCode() call with the second argument
> set to false does the same trick. Because that second parameter
> switches between "simplified" (true) and "standard conformant" (false)
> client alert codes already.

Ah, now I see what that parameter does. I've added a configuration
option, as you suggested. It turned out that I was overly optimistic
about all current servers supporting it: of course, Google didn't bother
supporting refresh-from-server.

Unfortunately one cannot use DevInf+remote rules to trigger the right
behavior, because the Alert is generated before DevInf is available. 

Grr. Looks like I will have to introduce a SyncEvolution configure
option for this.

-- 
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.



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


Re: [os-libsynthesis] [PATCH] Improve readability of SyncML TK error messages

2012-06-08 Thread Patrick Ohly
On Thu, 2012-06-07 at 12:31 +0200, Lukas Zeller wrote:
> Hi Andris,
> 
> thanks for the patch. However, the SMLERRPRINTFX() macro, like all
> debug output macros across the entire library, is supposed to output a
> linefeed by itself. It also actually does so in my builds of
> libsynthesis, so if the LFs are missing in your output, it must be
> something along the path from SMLERRPRINTFX() to the actual output
> function.
> 
> I think the problem is in the CONSOLEINFO_LIBC shortcut output mode
> Patrick added this February (13ff1e4149 (logging + Linux: enable
> console output)), where CONSOLEPRINTF() is mapped to
> fprintf(stderr,...) without adding a LF. In my (iOS) build this new
> mode is not used, but output ends in a plain puts(), which does add
> the LF.

Indeed. I wasn't aware of that convention when adding the code.

> diff --git a/src/sysync/sysync_debug.h b/src/sysync/sysync_debug.h
> index 61d4cdd..68dce5e 100755
> --- a/src/sysync/sysync_debug.h
> +++ b/src/sysync/sysync_debug.h
> @@ -110,7 +110,7 @@ TDebugLogger *getDbgLogger(void);
>// Because a lot of libs log to stderr, include a unique prefix.
>// Assumes that all printf format strings are plain strings.
>#define CONSOLEPUTS(m) CONSOLE_PRINTF_VARARGS("%s", (m))
> -#define CONSOLE_PRINTF_VARARGS(_m, _args...) fprintf(stderr, "SYSYNC " _m, 
> ##_args)
> +#define CONSOLE_PRINTF_VARARGS(_m, _args...) fprintf(stderr, "SYSYNC " _m 
> "\n", ##_args)
>#define CONSOLEPRINTF(m) CONSOLE_PRINTF_VARARGS m
>  # else // CONSOLEINFO_LIBC
>#define CONSOLEPUTS(m) ConsolePuts(m)

I've added that to my repo and also refined the CONSOLEINFO_LIBC code a
bit: instead of always using stdio, it lets the app override the actual
function.

I'm adding that because I get quite a bit of error output from the
SyncML TK which seems to be harmless. Some of reporting inside
libsynthesis duplicates my own reporting. So I'd rather disable console
printing dynamically and only enable it when the log level in
SyncEvolution is sufficiently high.

-- 
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.



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


[os-libsynthesis] Funambol + refresh-from-server => 417

2012-06-06 Thread Patrick Ohly
Hello!

It seems that Funambol has implemented some kind of protection against
excessive slow syncs: I can do a slow sync once fine. Doing it again
shortly afterwards (say, a few seconds later, as in automated testing)
results in a 417 "retry later" status for the Alert command (in the
SyncML response message).

They have not responded to my question about this behavior, so I don't
have an official confirmation that my interpretation is correct.

This server behavior makes one (otherwise useful) feature of
libsynthesis a bit troublesome: when an app requests a refresh from
server ("forceslow" = 1, "syncmode" = 1 in the binfile client API), then
the engine will ask the server for a slow sync instead of telling it to
do a refresh-from-server. In other words, the Funambol server has to
assume the worst (a slow sync) and applies its throttling even though
the client only wants to do a relatively benign "refresh from server".

I've checked that the Funambol server does many "refresh from server"
syncs without 417. For that I patched libsynthesis:

diff --git a/src/sysync/localengineds.cpp b/src/sysync/localengineds.cpp
index ce3d034..98f15dc 100644
--- a/src/sysync/localengineds.cpp
+++ b/src/sysync/localengineds.cpp
@@ -3481,6 +3481,11 @@ localstatus TLocalEngineDS::engGenerateClientSyncAlert(
   ));
   // create appropriate initial alert command
   uInt16 alertCode = getSyncStateAlertCode(fServerAlerted,true);
+  if (alertCode == 201 &&
+  fSyncMode == smo_fromserver &&
+  true) {
+alertCode = 205;
+  }
   // check for resume
   if (fResuming) {
 // check if what we resume is same as what we wanted to do

Does that patch look right in principle?

The "true" part needs to be replaced with a configuration option that
chooses between old (use "slow sync") and new behavior (use "refresh
from server"). Or has enough time passed that we no longer have to care
about servers which implement slow sync, but not refresh from server?
Then we could switch to the new behavior unconditionally.

-- 
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.



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


Re: [os-libsynthesis] [SyncEvolution] Sync Results for HTC-Vision (T-Mobile G2 or Desire-Z)

2012-06-01 Thread Patrick Ohly
On Fri, 2012-06-01 at 09:10 +0300, Andris Pavenis wrote:
> On 05/31/2012 04:01 PM, Patrick Ohly wrote:
> > Hello!
> >
> > Let me add the Synthesis list where compatibility with the Funambol
> > client is also currently getting discussed.
> 
> I have at least some success with Funambol Client from Android market
> 
> About says:
> FunV10 Version 10.1.3
> 
> 1) added datastore 'configuration'. Due to absence of hints what to use
>  took copy/paste from 'notes' and edited name after that
> 2) workarounded XML parse errors in SyncML TK by converting XML to
>  WBXML using libwbxml (tests done on Fedora 16, libwbxml version
>  0.11.0). After that I'm getting error message about NULL dereference
>  from Funambol client log on Android phone. Suspected that even when
>  client provided 'Accept-Encoding: gzip' it failed to decode gzip 
> compressed
>  message (server returned 'Content-Encoding: x-gzip')

This doesn't apply to the Funambol client which Onyeibo Oku was testing,
which still used XML. I could try to replicate it in syncevo-http-server
(a Python script which provides the HTTP front-end), if there is some
real need for it.

> 3) Added a diagnostic feature to server to disable compression of outgoing
>  messages even if client says it is OK to compress. After enabling this
>  diagnostic feature and specifying correct remote datastore names on
>  FunV10 client synchronization finally works
>
> One additional note:
> 
> Funamol uses different data-store names as Synthesis clients by default.
> data-store name alias support would be nice feature to have in libsynthesis.
> In our (IPNetworks) server development version such alias support is already
> implemented on data-store level, but then one must copy/paste related
> sections in server XML configuration files. Something like
> 
>  ...
> 
> would look much nicer.

Something like that exists already:

11.34.1 : alternate name for this datastore
Contained in:  of 
Can contain: alternate data store name
Attributes: none
Default: none
This can be used to make the same datastore accessible with an alternate name. 
In scripts the
LOCALDBNAME() function (see 11.34.21) can be used to find out what name was 
used by the
remote (client) to address the datastore – and possibly behave differently 
depending on what
name was used.

However, I noticed a problem with it that I still need to investigate
further: DevInf only includes the main name of a datastore. A peer which
expects a different name can sync if that different name was set as
alias, but it will have to do so without matching CTCaps. I've seen
warnings in the Synthesis logs about "blind syncs" because of that.

-- 
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.



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


Re: [os-libsynthesis] [SyncEvolution] Sync Results for HTC-Vision (T-Mobile G2 or Desire-Z)

2012-05-31 Thread Patrick Ohly
Hello!

Let me add the Synthesis list where compatibility with the Funambol
client is also currently getting discussed.

On Thu, 2012-05-31 at 09:25 +0200, Patrick Ohly wrote:
> On Thu, 2012-05-31 at 08:01 +0100, Onyeibo Oku wrote:
> > Greetings
> > 
> > I attempted to sync my HTC-Vision with SyncEvolution today for the first
> > time over WLAN.  The Server was setup according to
> > https://syncevolution.org/wiki/http-server-howto
> > 
> > The output:
> > http://pastebin.com/1kPcKQc1
> > 
> > Summary:
> > As much as the device established a clean connection, the
> > synchronization did not happen. The process terminates with an error
> > message: [ERROR] sync: could not extract LocURI=deviceID from initial
> > message.  
> > 
> > The device client also exits with and error message: Invalid URL.
> 
> There have been all kinds of problems with that client. The last time I
> looked at the official Funambol app, it required the server to be set up
> in a special way - in other words, it was only meant to work with
> onemedia.com.
> 
> > I used the community edition of Funambol Client for Android.  The server
> > is SyncEvolution version 1.2.2 running on Fedora Linux (beefy
> > Miracle-17). How do I begin to rectify this scenario?
> 
> There seems to be a valid, initial SyncML session. But it doesn't sync
> against any of the configured sources. It would be useful to set
> loglevel=10
> in /home/twohot/.config/syncevolution/default/peers/htc-desire-z/config.ini 
> and try again. Send me the entire session directory (look in 
> ~/.cache/syncevolution/) as archive (in particular including the message 
> dumps).
> 
> Then there is another message from the device which cannot be matched
> against that config. Run "syncevo-http-server -d", that will dump that
> message if it is plain text, and quote the output.

I got logs and found the root cause for this problem: after the first
message from the client, the server sends a challenge and asks the
client to authenticate with a MD5 hash. This does not create a session.
Instead the resend by the client with MD5 authentication is treated as a
separate, new session.

The Funambol client does not implement MD5 authentication properly. It's
second message lags the declaration that the password is b64 encoded.

It did this correctly for basic authentication. Here's a diff between
the messages:

***
*** 16,25 
  
  
  
! b64
! syncml:auth-basic
  
! dHdvaG90OioyMW45ZjE1cnJtOXQyMCo=
  
  
  
--- 16,24 
  
  
  
! syncml:auth-md5
  
! D+x9Y+zGniTegAmOjbDPrw==
  
  
  

If I add the "b64" line to the
second message, SyncEvolution can decode the message correctly.

As a workaround it is possible to change the SyncEvolution configuration
so that it asks for basic authentication. For that, set
  clientAuthType = basic
in
your /home/twohot/.config/syncevolution/default/peers/htc-desire-z/config.ini 
(without the # quotation mark in front of it).

I don't know how far that will get you. I see that the client is asking
for a sync against "configuration", which doesn't exist in
SyncEvolution. And even if it existed it is uncertain what should be put
there.

FWIW, this version of the client is still using XML instead of WBXML.

-- 
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.



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


Re: [os-libsynthesis] Funambol client on Android and libsynthesis based server

2012-05-30 Thread Patrick Ohly
On Wed, 2012-05-30 at 15:35 +0300, Andris Pavenis wrote:
> I have already earlier tried to use Android Funambol client with 
> libsynthesis based server.

I also tried with Android Funambol client + SyncEvolution +
libsynthesis, but although I made some progress I did not quite succeed.

Make sure that you have the following commit in your code:

commit 9b7164cfb0328c89aada0a3ee195493f31f561b7
Author: Patrick Ohly 
Date:   Fri Mar 2 17:04:05 2012 +0100

SyncML TK: support (hexa-)decimal character entities in XML

The Funambol Android client uses XML instead of WBXML and encodes
some characters, like the at sign, with decimal character
entities: @

This was not handled by the SyncML TK, which only supports some named
entities and #43 as a special decimal entity, thus resulting in
a 0x200b = SML_ERR_XLT_INVAL_SYNCML_DOC error.

Fixed by fully supporting all decimal and hexadecimal character
entities by converting the string value into integer and, with sanity
checks, into a char. xmlHTMLEntity() now has to return a computed
value and therefore its signature had to be changed away from
returning a pointer to the result. Works because it never returned
anything other than a char anyway.

It's in the master branch of
http://meego.gitorious.org/meego-middleware/libsynthesis

Not sure whether that is the issue that you are seeing. I don't remember
what issues I still had when I gave up.

Speaking of that master branch: Lukas, there are some other bug fixes
that you might want to merge back.

-- 
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.



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


Re: [os-libsynthesis] scripts + datastore

2012-05-02 Thread Patrick Ohly
On Fri, 2012-04-27 at 16:27 +0200, Lukas Zeller wrote:
> On Apr 25, 2012, at 15:11 , Patrick Ohly wrote:
> > But in practice that pointer is always NULL. I wasn't sure anyway
> > whether I would get the pointer to the local or remote datastore.
> 
> from fDsP you'd get the pointer to the local datastore (TLocalEngineDS *).
> 
> So to reach the remote datastore from a given TMultiFieldItem (which
> you certainly have at comparescript), I'd rather use
> 
>   yourMultiFieldItem->getRemoteItemType->getRelatedDatastore()
> 
> to get to the datastore that has defined that item's remote type.

Thanks, that worked.

-- 
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.



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


[os-libsynthesis] scripts + datastore

2012-04-25 Thread Patrick Ohly
Hello!

I am trying to solve one issue in SyncEvolution: when trying to find
pairs, it needs to know on a per-datastore basis whether both remote and
local storage have truly unique UID/RECURRENCE-ID that can be relied
upon (iCalendar 2.0 semantic).

So far, I am using a compare script for that, but it has to make
assumptions about the peer. To overcome that I added code that allows
clients to add SyncCap entries to the CTCap (similar to the "can restart
flag"). This information is stored at the receiving end in the
TSyncDataStore base class by the TRemoteDataStore while parsing the
SyncCap (again, very similar to "fCanRestart").

But now my problem is: how can the compare script access that
information?

It runs inside the "datatype context". Does that mean that all
datastores sharing the same type also share the same context and that
the  for the type is only invoked once?

The script functions in multifielditemtype.cpp (like SYNCOP()) looked
promising. It's possible to get a pointer to some kind of datastore:

TSyncDataStore *related = static_cast(aFuncContextP->getCallerContext())->getRelatedDatastore();

But in practice that pointer is always NULL. I wasn't sure anyway
whether I would get the pointer to the local or remote datastore.

Any hints?

-- 
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.



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


Re: [os-libsynthesis] Sync server with oracle database

2012-04-24 Thread Patrick Ohly
On Mon, 2012-04-23 at 22:23 +0200, Lukas Zeller wrote:
> To make a server out of libsynthesis, what needs to be done outside
> libsynthesis is the connection to the HTTP transport, and session
> handling (SyncML session objects must be kept around between HTTP
> requests until the entire session completes or is aborted).
> 
> The opensource SyncEvolution project has done this, however
> SyncEvolution has quite some complexity of its own so I don't know how
> easy it would be to use the HTTP transport code from SyncEvolution.

It would be pretty hard. The HTTP server in SyncEvolution is written in
Python using the Twisted framework and depends on the D-Bus API provided
by the SyncEvolution core, so one also inherits all the rest of
SyncEvolution (own configuration system, backends, etc.).

For writing one's own server, the Synthesis SDK and server will be
easier.

-- 
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.



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


Re: [os-libsynthesis] 409 "item merged" in client + multiple sync cycles in a single session

2012-03-09 Thread Patrick Ohly
On Tue, 2012-03-06 at 14:50 +0100, Patrick Ohly wrote:
> I haven't look into this yet, but still have it on my radar.

Done in the meego.gitorious.org master branch. I found that checking for
collisions is hard (not all records are in memory), so I settled for
making the chance of collisions smaller in the string case by including
a running count.

-- 
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.



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


Re: [os-libsynthesis] URI decoding

2012-03-09 Thread Patrick Ohly
On Wed, 2012-03-07 at 16:40 +0100, Lukas Zeller wrote:
> looks ok to me except that readConnectHost() should also be updated to
> avoid change of semantics (must still return host:port).

Duh, no idea why I missed that. Fixed in the patch as it was included in
meego.gitorious.org's master.

-- 
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.



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


Re: [os-libsynthesis] URI decoding

2012-03-07 Thread Patrick Ohly
On Tue, 2012-03-06 at 20:19 +0100, Lukas Zeller wrote:
> Hi Patrick,
> 
> On Mar 6, 2012, at 14:55 , Patrick Ohly wrote:
> 
> > Is there some utility code for decoding an URI into its individual
> > parts?
> 
> Yes, it's called splitURL in sysync_utils.h/.cpp
> 
> > Simple string operations don't work for an uri like this:
> > 
> > file:///tmp/abc%25def => file:// + /tmp/abc%def
> 
> splitURL in its current state cannot do that correctly either.
> However, I agree it would make sense to enhance it rather than
> creating a new function.

Attached is my attempt on doing that. Does that look right?

-- 
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 090cd1d811e98ce043f208bd3f23b1bcd0dd36aa Mon Sep 17 00:00:00 2001
From: Patrick Ohly 
Date: Tue, 6 Mar 2012 13:58:35 +
Subject: [PATCH] scripting: file URI decoding

Utility method for extracting the file path from an URI.  Needed for
inlining PHOTO data in scripts before sending to a peer.

Uses the existing splitURL() method and enhances it, because the
previous parser was a bit too simplistic. For example, in
file:///tmp/file it skipped all the leading slashes and interpreted
"tmp" as host name, instead of returning an empty host name.

Query and port number are split from the host and doc part directly by
splitURL() now because that seems more consistent.  The in-tree code
using syncURL() concatenates the separate parts to keep its own
semantic unchanged.

URL %xx decoding must be handled by the caller of syncURL(), using the
new urlDecode() utility code. This is not done in syncURL() to
simplify concatenating parts (would have to url encode reserved
characters otherwise).

The new code always returns an empty doc part. There's no longer
a special case for inserting a slash when only the query part is
set, because the caller is already expected to treat the doc name as
starting in the root of the document hierarchy.
---
 src/sysync/itemfield.cpp|2 +-
 src/sysync/scriptcontext.cpp|   21 
 src/sysync/syncagent.cpp|   12 ++-
 src/sysync/syncclientbase.cpp   |   18 +++-
 src/sysync_SDK/Sources/sysync_utils.cpp |  199 +++
 src/sysync_SDK/Sources/sysync_utils.h   |9 +-
 6 files changed, 228 insertions(+), 33 deletions(-)

diff --git a/src/sysync/itemfield.cpp b/src/sysync/itemfield.cpp
index ccb39a6..b7f68ed 100644
--- a/src/sysync/itemfield.cpp
+++ b/src/sysync/itemfield.cpp
@@ -1031,7 +1031,7 @@ void TURLField::stringWasAssigned(void)
   string proto;
   if (!fString.empty()) {
 // make sure we have a URL with protocol
-splitURL(fString.c_str() ,&proto, NULL, NULL, NULL, NULL);
+splitURL(fString.c_str() ,&proto, NULL, NULL, NULL, NULL, NULL, NULL);
 if (proto.empty()) {
   // no protocol set, but string not empty --> assume http
   fString.insert(0, "http://";);
diff --git a/src/sysync/scriptcontext.cpp b/src/sysync/scriptcontext.cpp
index b9ec512..f5a5af8 100755
--- a/src/sysync/scriptcontext.cpp
+++ b/src/sysync/scriptcontext.cpp
@@ -920,6 +920,26 @@ public:
 }
   } // func_Read
 
+  // string URIToPath(string uri)
+  // extracts the file path in a file:// uri; handles uri decoding
+  // Returns UNASSIGNED if not a file:// uri
+  static void func_URIToPath(TItemField *&aTermP, TScriptContext *aFuncContextP)
+  {
+// get params
+string uri;
+aFuncContextP->getLocalVar(0)->getAsString(uri);
+
+string protocol, doc;
+splitURL(uri.c_str(), &protocol, NULL, &doc, NULL, NULL, NULL, NULL);
+if (protocol == "file") {
+  string path;
+  path.reserve(doc.size() + 1);
+  path += "/"; // leading slash is never included by splitURL()
+  path += doc;
+  urlDecode(&path);
+  aTermP->setAsString(path);
+}
+  } // func_URIToPath
 
   // string REMOTERULENAME()
   // returns name of the LAST matched remote rule (or subrule), empty if none
@@ -2308,6 +2328,7 @@ const TBuiltInFuncDef BuiltInFuncDefs[] = {
   { "REQUESTMINTIME", TBuiltinStdFuncs::func_RequestMinTime, fty_none, 1, param_oneInteger },
   { "SHELLEXECUTE", TBuiltinStdFuncs::func_Shellexecute, fty_integer, 3, param_Shellexecute },
   { "READ",  TBuiltinStdFuncs::func_Read, fty_string, 1, param_oneString },
+  { "URITOPATH",  TBuiltinStdFuncs::func_URIToPath, fty_string, 1, param_oneString },
   { "SESSIONVAR", TBuiltinStdFuncs::func_SessionVar, fty_none, 1, param_oneString },
   { "SETSESSIONVAR", TBuiltinStdFuncs::func_SetSessionVar, fty_none, 2, param_SetSessionVar },
   { "ABORTSESSION", TB

[os-libsynthesis] URI decoding

2012-03-06 Thread Patrick Ohly
Hello!

Is there some utility code for decoding an URI into its individual
parts? Simple string operations don't work for an uri like this:

file:///tmp/abc%25def => file:// + /tmp/abc%def

I've recently come across this because Evolution Data Server somehow
puts such an encoded % into PHOTO file URIs, which the engine scripts
need to inline before sending to a peer.

I'm in the process of writing another script method which extracts the
file path and does URI decoding, but I don't want to reinvent the wheel
if there is already such code. A quick search hasn't brought up
anything, though.

-- 
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.



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


Re: [os-libsynthesis] 409 "item merged" in client + multiple sync cycles in a single session

2012-03-06 Thread Patrick Ohly
On Thu, 2012-02-16 at 18:14 +0100, Lukas Zeller wrote:
> On Feb 13, 2012, at 14:35 , Patrick Ohly wrote:
> 
> > Instead of repeating myself, let me quote the commit messages of the
> > relevant commits on that branch below. There are a few more commits in
> > that branch. Lukas, what do you think?
> 
> 
> Thanks for these patches (the one with firstReadNextItem not being set
> is an embarassing one - on the other hand it proves safe coding in all
> of the plugins in use so far in that they apparently all cleanly reset
> the iterator at initialisation without being told so).

I guess they would have failed in very obvious ways if they hadn't done
that. My main concern was that they now might now start to fail because
the engine works as described ;-)

> Regarding the solution for the 409 merge problem, I think the
> fake-ID-delete approach is fine. Numeric localids are not used in any
> libsynthesis build, this is legacy from the monolithic PalmOS clients
> which had severe memory constraints which ruled out anything as fat
> and resource wasting as string IDs :-) Still, I think the use of
> negative IDs would work here, the only thing I'm not sure is if
> RND_MAX can be safely assumed to be 31 bits only - in case it would
> extend into 32 bit we'd get positive IDs clashing with real ones
> occasionally. Maybe just AND rand() result with 0x7FFF to be on
> the safe side...

I haven't look into this yet, but still have it on my radar.

Instead I continued investigating multi-cycle syncs by writing more
tests and fixing some of the other TODO items. All of that works now, so
I merged the code into the master branched of the meego.gitorious.org
repos.

-- 
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.



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


Re: [os-libsynthesis] 409 "item merged" in client + multiple sync cycles in a single session

2012-02-13 Thread Patrick Ohly
On Tue, 2012-02-07 at 16:05 +0100, Patrick Ohly wrote:
> On Mon, 2012-02-06 at 21:29 +0100, Patrick Ohly wrote:
> > I'm currently experimenting with a different approach for handling the
> > 409 in the binfile client: when an Add fails with 409, catch it as it is
> > done at the moment, but then tell the server that it was mapped to a
> > dummy LUID. Mark that LUID as deleted in the change log. Then in the
> > next session, delete the redundant item on the server.
> > 
> > I'm combining that with running multiple SyncML sessions in the same
> > context.
> > 
> > Will post code soon...
> 
> The result is in the "internal-sync" branch in the meego.gitorious.org
> repo of libsynthesis. It's called "internal-sync" because I am working
> on an extension of the SyncML protocol that is only understand when
> SyncEvolution talks to SyncEvolution (either in the SyncEvolution local
> sync mode - see the
> http://syncevolution.org/blogs/pohly/2012/fosdem-2012 talk for a diagram
> illustrating that) or in SyncEvolution client to SyncEvolution server
> mode.
> 
> Instead of repeating myself, let me quote the commit messages of the
> relevant commits on that branch below. There are a few more commits in
> that branch. Lukas, what do you think?
> 
> This is not done yet, but if possible I'd like to get feedback before
> going down an entirely wrong path. Some tests for this new code are in
> SyncEvolution (not pushed yet) and pass as expected. I'll keep working
> on this, in particular the "temporary problem" part and "add more data
> in second cycle" cases.

I've also pushed the "internal-sync" branch for SyncEvolution. It has
tests for the "409 in client of dumb SyncML server" problem
(testAddBothSides when configured to use SyncEvolution server with file
backend), restarting a two-way sync with
SyncContext::requestAnotherSync() (testTwoWayRestart) and the extended
SyncSource semantic. All of that works fine.

It took a bit longer than expected because writing these tests required
quite a bit of code refactoring.

Speaking of the extended semantic that beginSync() may be called
multiple times: right now this is mandatory. My hope is that backend
implementers will adapt to that change before releasing 1.3, or that no
such changes are necessary. EDS and file backends worked fine without
changes. I prefer that approach because it avoids special cases.

Mikel, most relevant for you is the new
SyncSource/SyncContext::requestAnotherSync() API call. You might be able
to implement the two-phase transfer of contacts like this:
 1. In first beginSync() call:
  * only retrieve contacts without PHOTO data
  * ask for another cycle with requestAnotherSync()
 2. In the second beginSync():
  * also retrieve PHOTO data
  * mark all contacts with new or updated PHOTO data as
"updated"

I also implemented the "backend is read-only" part. The Synthesis engine
already had that, it was just a matter of making that available in
SyncEvolution.

You can rebase the PBAP branch onto "internal-sync". I'll refrain from
rebasing "internal-sync" from now on.


-- 
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.



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


Re: [os-libsynthesis] 409 "item merged" in client + multiple sync cycles in a single session

2012-02-07 Thread Patrick Ohly
On Mon, 2012-02-06 at 21:29 +0100, Patrick Ohly wrote:
> I'm currently experimenting with a different approach for handling the
> 409 in the binfile client: when an Add fails with 409, catch it as it is
> done at the moment, but then tell the server that it was mapped to a
> dummy LUID. Mark that LUID as deleted in the change log. Then in the
> next session, delete the redundant item on the server.
> 
> I'm combining that with running multiple SyncML sessions in the same
> context.
> 
> Will post code soon...

The result is in the "internal-sync" branch in the meego.gitorious.org
repo of libsynthesis. It's called "internal-sync" because I am working
on an extension of the SyncML protocol that is only understand when
SyncEvolution talks to SyncEvolution (either in the SyncEvolution local
sync mode - see the
http://syncevolution.org/blogs/pohly/2012/fosdem-2012 talk for a diagram
illustrating that) or in SyncEvolution client to SyncEvolution server
mode.

Instead of repeating myself, let me quote the commit messages of the
relevant commits on that branch below. There are a few more commits in
that branch. Lukas, what do you think?

This is not done yet, but if possible I'd like to get feedback before
going down an entirely wrong path. Some tests for this new code are in
SyncEvolution (not pushed yet) and pass as expected. I'll keep working
on this, in particular the "temporary problem" part and "add more data
in second cycle" cases.

commit 37910c119381eb9e8d64a3183b4f29f2fe5b246f
Author: Patrick Ohly 
Date:   Tue Feb 7 10:56:52 2012 +0100

DB_Conflict (409): different implementation in binfileimplds.cpp

The previous approach to handling a 409 status for an  (= item
conflicts with existing item) in the binfile client was to update that
existing item and then tell the server that its item maps to an
existing client item.

This leads to the situation where the server has two client IDs
mapping to the same item on the server and one server item with no
corresponding client item. Servers get confused by this. For example,
the Synthesis engine itself then sends a  with empty IDs to
the client in the next sync.

It also has the disadvantage that the client cannot ask the server to
delete the redundant item, because its  requests must include
a client ID, which the redundant item doesn't have.

Furthermore, the server was sent a redundant  in the case that
it already had an item that was in sync in the client (= item didn't
have to be written in local storage).

Therefore this commit implements a different approach:
- the new server item which triggers the 409 is used to update
  the existing item, as before
- if and only if the local item gets modified, it will be sent as
  update for the older server item already mapped to it (there must
  be such an item, because the client must have told the
  server about that local item, and that server item is now
  out-dated)
- the new server item is mapped to a fake client ID which is
  marked as deleted; in the next sync, that mapping is used
  to delete the new server item

Once a second sync is done, client and server are in sync again, with
the latest data as determined by the client stored in the servers
older item and the newer item deleted.

Generating a fake client ID is a bit hacky at the moment. The code for
numeric IDs is entirely untested, the code for string IDs doesn't
check for (unlikely?!) collisions.


commit 6504ae35543efe860feeda4ce498ec9824a74d3d
Author: Patrick Ohly 
Date:   Tue Feb 7 15:46:29 2012 +0100

SyncML extensions: multiple cycles in the same session

SyncML limits the data exchange to "client sends changes, server sends
changes back". If the client detects that it has further changes for
the server while already in the second phase, it will have to wait for
the next SyncML session before it can send these changes. This leads
to a bad user experience, because the user expectation is that client
and server are in sync after a sync session.

Examples when this becomes a problem:
- 409 conflict detected in binfile client (tested with
  SyncEvolution Client::Sync::eds_event::testAddBothSides[Refresh]
  and a SyncEvolution server with file backend which doesn't
  detect duplicates based on UID)
- temporary error prevents applying a change, needs to be
  retried (untested at the moment)
- PBAB use case: advanced client does a sync with a subset
  of the data (vCard without PHOTO, while retrieving PHOTO data
  in the background), then sends updates with more data (modified/new
  PHOTOs) later on (also untested)

This commit introduces a new mode where SyncML client and serve

Re: [os-libsynthesis] 409 "item merged" in client

2012-02-06 Thread Patrick Ohly
On Mon, 2012-01-30 at 18:04 +0100, Patrick Ohly wrote:
> Should it have removed one item while processing the ?
> 
> IMHO it can't, because it doesn't have enough information to determine
> which of its two items are up-to-date. It only knows that the client
> must have merged two items, but not yet which one won.
> 
> In the second session, the server gets an updated item with client ID
> "foo". Now it could update one copy and delete the other, but because it
> only kept a link between its item 0 and foo, it only does the update
> part.
> 
> Would something break if the server allowed the same client item to map
> to multiple items on the server and then did a remove in its database
> when receiving an update?
> 
> I suspect that the client is simply doing something not expected by
> typical SyncML servers. 

I'm currently experimenting with a different approach for handling the
409 in the binfile client: when an Add fails with 409, catch it as it is
done at the moment, but then tell the server that it was mapped to a
dummy LUID. Mark that LUID as deleted in the change log. Then in the
next session, delete the redundant item on the server.

I'm combining that with running multiple SyncML sessions in the same
context.

Will post code soon...

-- 
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.



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


Re: [os-libsynthesis] 409 "item merged" in client

2012-01-30 Thread Patrick Ohly
On Mi, 2011-12-21 at 17:17 +0100, Lukas Zeller wrote:
> Digging though the case for a while, I suddenly realized that in
> client case the implProcessItem() method used is the one in
> binfileimplds.cpp and not the one in customimplds.cpp. However, we
> added the 409 merge trick in customimplds.cpp only - no
> wonder it "goes south" in the client case.
> 
> I'll check if I can easily add the same functionality to binfileimpds.
> The reason why it is a separate method with seemingly duplicate
> functionality is that binfileimpl is integrated tightly with the
> binfile-based change log mechanism (even if you don't use it) - which
> makes the implementation differ quite a bit even if functionality
> provided is (i.e. in this case: *should be*) the same.

I've finally looked at this again, using the upstream gitorious repo's
master branch. That has your "DB_Conflict (409): added missing
implementation in binfileimplds.cpp" fix.

The sequence of events now is:
  * dumb server has one item with UID=foo
  * smart client has an updated version of that same item, created
there independently
  * client and server sync:
  * server stores two copies, then sends client the old copy
(ID 0) as "new item"
  * client detects duplicate, resolves this internally by
keeping its own copy and returning 201 to the server
(instead of 409 as before); the  sent to the server
maps the server's item 0 to the same LUID also sent to
the server as "new" from the client (stored there as
item 1)
  * the server stores a UID mapping where its own old item 0
maps to the existing item foo on the client, and its new
item 1 maps to no item on the client
  * client and server are now out of sync: the server still
has old and new copy of the item, the client only the
new one
  * client and server sync again:
  * client sends its copy as "updated"
  * server updates its old copy
  * client and server are still out of sync: one item too
many on the server

I'm not sure how to fix this. When processing the , the server must
have detected that the same client ID was used twice. What it logs at
the moment is this:

[2012-01-30 18:01:19.775] Mapping remoteID='foo' to localID='0'
[2012-01-30 18:01:19.775] file_calendar: testState=TRUE - expected 
state>='sync_mode_stable', found state=='sync_gen_done'
[2012-01-30 18:01:19.775] ModifyMap called: aEntryType=normal, aLocalID='0, 
aRemoteid='foo', aMapflags=0x0, aDelete=0
[2012-01-30 18:01:19.775] - found entry by entrytype/localID='0' - remoteid='', 
mapflags=0x0, changed=0, deleted=0, added=0, markforresume=0, savedmark=1
[2012-01-30 18:01:19.775] - matching entry found - re-activating deleted and/or 
updating contents if needed
[2012-01-30 18:01:19.775] - cleanup: removing same remoteID from other entry 
with localid='1', mapflags=0x0, changed=0, deleted=0, added=0, markforresume=0, 
savedmark=0
[2012-01-30 18:01:19.775] Map entry updated: LocalID='0', RemoteID='foo'

Should it have removed one item while processing the ?

IMHO it can't, because it doesn't have enough information to determine
which of its two items are up-to-date. It only knows that the client
must have merged two items, but not yet which one won.

In the second session, the server gets an updated item with client ID
"foo". Now it could update one copy and delete the other, but because it
only kept a link between its item 0 and foo, it only does the update
part.

Would something break if the server allowed the same client item to map
to multiple items on the server and then did a remove in its database
when receiving an update?

I suspect that the client is simply doing something not expected by
typical SyncML servers. Here's what other SyncML servers do:
  * Memotoo: doesn't send UID in its item data, thus the client does
not detect the duplicates
  * Funambol: doesn't let me run a sync at the moment, need to test
later

-- 
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.



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


Re: [os-libsynthesis] super data store: trying to read to check for existance

2011-10-28 Thread Patrick Ohly
On Tue, 2011-10-25 at 09:24 +0200, Lukas Zeller wrote:
> On Oct 24, 2011, at 16:55 , Patrick Ohly wrote:
> 
> > Why is it necessary to read before trying to delete? If the item exists,
> > then reading it is a fairly expensive test.
> 
> Unfortunately, with some some backends, this is the only test that reliably 
> works. For example, some SQL databases (or their ODBC driver) can't return a 
> correct "number of affected rows" for DELETE statements. So reading before 
> deleting was the only way to detect if any of the subdatastores really had 
> that item and correctly return 404 or 200.
> 
> > So far, my backends were written with the expectation that they have to
> > cope with delete requests for items which are already deleted. This
> > follows from the inherent race condition between syncing and some other
> > process which might delete items while a sync runs.
> > 
> > Are all backends expected to return 404 when an item doesn't exist?
> 
> Not all backends (see above, built-in ODBC can't), but yes, for plugin
> backends, returning proper error codes is specified, also for delete.
> Still, if a plugin would not conform to that in its implementation of
> delete, that would probably have gone unnoticed so far.

I checked the API docs and did not see it mentioned explicitly.

In SyncEvolution I added some remarks about 404 (will probably not be
noticed) and also added automated Client::Source tests for it (which is
harder to ignore if someone runs the tests ;-)

I also changed the error logging so that 404 is reported to users in
ReadItem and not logged in DeleteItem (while still returned to the
engine).

> Of course, the test is a bit expensive - if that's a concern, one
> could differentiate between plugin datastores and others (all the
> builtin ones, including SQL/ODBC), and use the expensive read test
> only for the latter. Like a virtual dsDeleteDetectsItemPresence()
> which returns false by default, but can be overridden in plugin
> datastores to return true.

The patch below implements that idea (from the for-master/bmc23744
branch). SyncEvolution relies on that patch to avoid the (harmless)
ERROR messages for "item not found" in ReadItem(), although everything
should work okay also without the patch.

Does that look right?

>From fb5cc0dc19fb60e40a4ca2dcfe44a52a75ec7354 Mon Sep 17 00:00:00 2001
From: Patrick Ohly 
Date: Fri, 28 Oct 2011 15:42:57 +0200
Subject: [PATCH] server engine: more efficient deletion in superdatastore

Attempting to read an item just to check whether it exists is expensive.
It also may trigger error messages when the item does not exist (done by
SyncEvolution).

Therefore, if a data store is able to properly report whether it found
the item which is to be deleted, a different logic is used for
deletion in the superdatastore:
- attempt to delete until it succeeds in a subdatastore
- recreate the sync item from the copy after a failed delete attempt
  (which deletes the sync item), if there is another loop iteration

Deleting the copy of the sync item was moved to the function exit code
to avoid code duplication.

By default all data stores are assumed to not support 404 in its
delete operation. The only exception is the plugin API. The (somewhat
undocumented) assumption is that all methods properly report 404 when
the requested item does not exist. The "attempt to read" code already
relied on that. Now the code relies on that for the "attempt to
delete".

Probably it even works when the plugin returns some other error
code (the "regular" value will be false in that case) or no error code
at all: the translation code between remote ID and local ID will
already detect that the item is not in the mapping table if it is not
in the subdatastore. So the actual plugin will not get called at all
in the case where its expected behavior would matter.
---
 src/DB_interfaces/api_db/pluginapids.h |4 ++
 src/sysync/superdatastore.cpp  |   80 ++--
 src/sysync/syncdatastore.h |2 +
 3 files changed, 62 insertions(+), 24 deletions(-)

diff --git a/src/DB_interfaces/api_db/pluginapids.h 
b/src/DB_interfaces/api_db/pluginapids.h
index 4fbfc79..f215094 100755
--- a/src/DB_interfaces/api_db/pluginapids.h
+++ b/src/DB_interfaces/api_db/pluginapids.h
@@ -172,6 +172,10 @@ public:
   virtual void dsResetDataStore(void) { InternalResetDataStore(); 
inherited::dsResetDataStore(); };
   virtual ~TPluginApiDS();
 
+  // override TSyncDataStore: the plugin must be able to return 404
+  // when an item is not found during delete
+  virtual bool dsDeleteDetectsItemPresence() const { return true; }
+
   #ifndef BINFILE_ALWAYS_ACTIVE
   /// @name api methods defining the interface from TCustomImplDS to 
TXXXApi actual API implementations
   /// @{
diff --git a/src/sys

Re: [os-libsynthesis] super data store: trying to read to check for existance

2011-10-25 Thread Patrick Ohly
On Tue, 2011-10-25 at 15:46 +0200, Lukas Zeller wrote:
> But if the superdatastore was changed from probing with ReadItem to
> directly probing/deleting with DeleteItem, the error reporting would
> look similar, as it would show failed deletes for those subdatastores
> not containing the object in question.

No, because I never consider attempts to delete a non-existent item as
an error and thus would always suppress the error message. After all,
the desired outcome was achieved.

This is different for reading where not getting the item data may or may
not be relevant.

-- 
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.



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


Re: [os-libsynthesis] super data store: trying to read to check for existance

2011-10-25 Thread Patrick Ohly
On Tue, 2011-10-25 at 10:16 +0200, Lukas Zeller wrote:
> > That sounds like the right compromise. I'm much more comfortable
> > returning 404 in a delete and not reporting that to the user as an
> > error, compared to not reporting a 404 in a ReadItem call (as I have to
> > do now).
> 
> Now I don't understand. Why do you have to *not* report a 404 in
> ReadItem now??

Reporting to the engine is fine, reporting to the user is the
problematic part. Let me explain.

The SyncEvolution output, the one that is visible to end-users, contains
INFO and ERROR messages about operations happening in the backend. Each
attempt by the Synthesis engine to read a non-existent item results in a
user-visible error. Or rather, two of them in the case of a super data
store combining "calendar" and "todo":

[ERROR] error code from SyncEvolution fatal error (local, status 10500): 
calendar: retrieving item: 20111023T082825Z-2322-1001-1969-0@lulu-rid: 
Objektpfad des Kalenders kann nicht abgerufen werden: Objekt nicht gefunden
[ERROR] error code from SyncEvolution fatal error (local, status 10500): todo: 
retrieving item: 20111023T082825Z-2322-1001-1969-0@lulu-rid: Objektpfad des 
Kalenders kann nicht abgerufen werden: Objekt nicht gefunden

At the point where that logging happens it is unknown whether the error
is really a problem or can be ignored. Only the Synthesis engine itself
knows. When I started to use the Synthesis engine, I tried to make the
logging happening inside the engine visible to users, but it simply
wasn't meant to be used for that and so I gave up on that approach. 

Admittedly the ERROR message above is not very informative either.

-- 
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.



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


Re: [os-libsynthesis] super data store: trying to read to check for existance

2011-10-25 Thread Patrick Ohly
On Tue, 2011-10-25 at 09:24 +0200, Lukas Zeller wrote:
> Hello Patrick,
> 
> On Oct 24, 2011, at 16:55 , Patrick Ohly wrote:
> 
> > Why is it necessary to read before trying to delete? If the item exists,
> > then reading it is a fairly expensive test.
> 
> Unfortunately, with some some backends, this is the only test that
> reliably works. For example, some SQL databases (or their ODBC driver)
> can't return a correct "number of affected rows" for DELETE
> statements. So reading before deleting was the only way to detect if
> any of the subdatastores really had that item and correctly return 404
> or 200.
> 
> > So far, my backends were written with the expectation that they have to
> > cope with delete requests for items which are already deleted. This
> > follows from the inherent race condition between syncing and some other
> > process which might delete items while a sync runs.
> > 
> > Are all backends expected to return 404 when an item doesn't exist?
> 
> Not all backends (see above, built-in ODBC can't), but yes, for plugin
> backends, returning proper error codes is specified, also for delete.

Are these expectations documented somewhere? I'm still a bit fuzzy about
which error codes are expected. For example, should a delete of a
non-existent item succeed (200) or fail (404)?

> Still, if a plugin would not conform to that in its implementation of
> delete, that would probably have gone unnoticed so far.

Right. SyncEvolution has hardly ever (never?) returned 404 and that has
not been an issue until now ;-)

> Of course, the test is a bit expensive - if that's a concern, one
> could differentiate between plugin datastores and others (all the
> builtin ones, including SQL/ODBC), and use the expensive read test
> only for the latter. Like a virtual dsDeleteDetectsItemPresence()
> which returns false by default, but can be overridden in plugin
> datastores to return true.

That sounds like the right compromise. I'm much more comfortable
returning 404 in a delete and not reporting that to the user as an
error, compared to not reporting a 404 in a ReadItem call (as I have to
do now).

-- 
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.



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


Re: [os-libsynthesis] super data store: trying to read to check for existance

2011-10-24 Thread Patrick Ohly
On Mon, 2011-10-24 at 16:55 +0200, Patrick Ohly wrote:
> Are all backends expected to return 404 when an item doesn't exist?

I have a patch ready for Evolution contacts and calendar ready which
does this. However, it is not very nice: in order to avoid confusing
ERROR messages to the user, the ReadItem callback now has to suppress
404 logging in all cases, regardless whether such an error is acceptable
or a real error.

-- 
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.



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


[os-libsynthesis] super data store: trying to read to check for existance

2011-10-24 Thread Patrick Ohly
Hello!

A SyncEvolution user found that SyncEvolution fails while synchronizing
against a mobile phone when an item was deleted on both the phone and
Evolution side. SyncEvolution is asked to read the already deleted item,
which then fails (currently) with an unspecified database error.

This is due to the following code:

#20 0x009b89de in sysync::TSuperDataStore::engProcessRemoteItem 
(this=0x1166900, syncitemP=0xe854e0, 
aStatusCommand=...) at 
/home/pohly/syncevolution/libsynthesis/src/sysync/superdatastore.cpp:637
637   
regular=linkP->fDatastoreLinkP->logicRetrieveItemByID(*itemcopyP,aStatusCommand);
(gdb) list
632   PDEBUGPRINTFX(DBG_DATA+DBG_DETAILS,(
633 "Trying to read item by remoteID='%s' from subdatastore 
'%s' to see if it is there",
634 itemcopyP->getRemoteID(),
635 linkP->fDatastoreLinkP->getName()
636   ));
637   
regular=linkP->fDatastoreLinkP->logicRetrieveItemByID(*itemcopyP,aStatusCommand);
638   // must be ok AND not 404 (item not found)
639   if (regular && aStatusCommand.getStatusCode()!=404) {
640 PDEBUGPRINTFX(DBG_DATA,(
641   "Item found in subdatastore '%s', deleting it there",

Why is it necessary to read before trying to delete? If the item exists,
then reading it is a fairly expensive test.

So far, my backends were written with the expectation that they have to
cope with delete requests for items which are already deleted. This
follows from the inherent race condition between syncing and some other
process which might delete items while a sync runs.

Are all backends expected to return 404 when an item doesn't exist?

-- 
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.



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


Re: [os-libsynthesis] StartDataRead/EndDataWrite + invalid anchor

2011-10-20 Thread Patrick Ohly
On Wed, 2011-10-12 at 10:14 +0200, Patrick Ohly wrote:
> On Thu, 2011-09-15 at 18:14 +0200, Lukas Zeller wrote:
> > On Sep 12, 2011, at 14:16 , Patrick Ohly wrote:
> > 
> > > On Sa, 2011-09-10 at 12:32 +0200, Lukas Zeller wrote:
> > >> Hello Patrick,
> > >> 
> > >> On Sep 7, 2011, at 14:53 , Patrick Ohly wrote:
> > >> But what we could do is allow StartDataRead to return 508 error code
> > >> to have the engine reset sync anchors before bailing out, such that
> > >> the next session would become a slow sync.
> > >> 
> > >> Would that solve the problem?
> > > 
> > > It is sub-optimal because it will be visible to users. Currently
> > > SyncEvolution doesn't automatically start a new sync. This will also
> > > break the automated testing, which will bail out with that error instead
> > > of doing a slow sync.
> > > 
> > > As one way of dealing with the problem adding such a special error code
> > > is worthwhile. But perhaps there is a different way?
> > > 
> > > For example, could the datastore API extended with a "verify anchor"
> > > call which is called soon enough to discard the anchor and fall back to
> > > a slow sync?
> > 
> > Officially adding API calls is very complicated due to the indirect
> > way the plugin ABI is designed with all the up/downwards compatibilty.
> > At least, I can't do that efficiently myself :-(
> > 
> > But I think I found a better way to do the same, by simply allowing
> > StartDataRead() to happen earlier - in time to cause a slow sync if
> > needed.
> 
> Your patched worked fine, after fixing one minor aspect. See attached
> patch.
> 
> This will allow me to close BMC #22881 after merging into the master
> branches.

More testing after merging into the master branch led to a "want to have
the cake and eat it" moment: invoking StartDataRead early is necessary
so that the datastore can force a slow sync. But SyncEvolution also
relies on seeing StartDataRead calls only after the first message
exchange with the peer was successful and the credentials were accepted.

Only then does it show a "sync started successful" message and does more
expensive operations like dumping the datastore's data into the
automatic backup.

Now both of this happens too early and sometimes (in those cases where
network is down or the login fails) unnecessarily.

I guess the only proper solution would be a new CheckAnchor callback
(invoked at the same time as StartDataRead with
) and then the real StartDataRead later (at
the traditional place).

How hard is it really to change the libsynthesis plugin API? It was
meant to be extensible, wasn't it?

As an intermediate solution I'll probably make SyncEvolution's use of
 conditional: as before for most backends,
early for ActiveSync (which is the only one which has the anchor
problem).

-- 
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.



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


Re: [os-libsynthesis] StartDataRead/EndDataWrite + invalid anchor

2011-10-12 Thread Patrick Ohly
On Thu, 2011-09-15 at 18:14 +0200, Lukas Zeller wrote:
> On Sep 12, 2011, at 14:16 , Patrick Ohly wrote:
> 
> > On Sa, 2011-09-10 at 12:32 +0200, Lukas Zeller wrote:
> >> Hello Patrick,
> >> 
> >> On Sep 7, 2011, at 14:53 , Patrick Ohly wrote:
> >> But what we could do is allow StartDataRead to return 508 error code
> >> to have the engine reset sync anchors before bailing out, such that
> >> the next session would become a slow sync.
> >> 
> >> Would that solve the problem?
> > 
> > It is sub-optimal because it will be visible to users. Currently
> > SyncEvolution doesn't automatically start a new sync. This will also
> > break the automated testing, which will bail out with that error instead
> > of doing a slow sync.
> > 
> > As one way of dealing with the problem adding such a special error code
> > is worthwhile. But perhaps there is a different way?
> > 
> > For example, could the datastore API extended with a "verify anchor"
> > call which is called soon enough to discard the anchor and fall back to
> > a slow sync?
> 
> Officially adding API calls is very complicated due to the indirect
> way the plugin ABI is designed with all the up/downwards compatibilty.
> At least, I can't do that efficiently myself :-(
> 
> But I think I found a better way to do the same, by simply allowing
> StartDataRead() to happen earlier - in time to cause a slow sync if
> needed.

Your patched worked fine, after fixing one minor aspect. See attached
patch.

This will allow me to close BMC #22881 after merging into the master
branches. In the case of libsynthesis I probably should catch up with
your latest published branch, too.

Right now I am still cherry-picking into the version used by the stable
SyncEvolution 1.2 branch, but this becomes harder because of some
whitespace cleanup that you must have done in your upstream branch.

-- 
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 900726043cca2b260308bedb15f4ed292763b81c Mon Sep 17 00:00:00 2001
From: Patrick Ohly 
Date: Wed, 12 Oct 2011 09:35:17 +0200
Subject: [PATCH] engine: fixed handling of 508

After detecting the special 508 "request slow sync" error, the
LOCERR_OK must be returned to the higher levels in the stack,
otherwise syncing aborts.
---
 src/sysync/customimplds.cpp |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/src/sysync/customimplds.cpp b/src/sysync/customimplds.cpp
index 6a44f47..2d18f69 100755
--- a/src/sysync/customimplds.cpp
+++ b/src/sysync/customimplds.cpp
@@ -1623,6 +1623,7 @@ localstatus TCustomImplDS::implMakeAdminReady(
   // special case: the database requests a slow sync for internal reasons (like change tracking disabled)
   // - force slow sync by removing last anchor
   fLastRemoteAnchor.erase();
+  sta = LOCERR_OK;
 }
   }
 } // if apiLoadAdminData successful
-- 
1.7.2.5

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


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

2011-09-30 Thread Patrick Ohly
On Fri, 2011-09-30 at 12:10 +0200, Patrick Ohly wrote:
> > > In the case of iCalendar 2.0 I wonder whether merge=lines is
> needed.
> > > For modify/modify conflicts perhaps, but not so much for add/add
> > > conflicts. Thoughts?
> > 
> > If you want to differentiate these cases, I'd say we'll need a
> > MERGEREASON() script function so the script can act differently
> > depending on why the merge is initiated. So far I see the following
> > different reasons: slowsync non-perfect match, syncml update
> conflict,
> > DB update conflict, DB add conflict.
> 
> I'm inclined to rather remove the merge=lines and thus keep properties
> from the winning side in all cases.
> 
> Except... what if a dumb phone truncates a property and then a slow
> sync
> is done? Will the default merge mode preserve the longer property or
> truncate if the version on the phone happens to be considered
> "winning"? 

I'm still curious about that.

Note that the previous age comparison patch had the -1/+1 logic
reversed. It worked correctly in my tests because the COMPARESCRIPT()
macro always returned 1 although it should have returned -1. Later it
failed for the inverse test.

Fix for that and updated patch attached.

-- 
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 b0252bdf02720193d1c638ec9ec32c1f4cedc6f3 Mon Sep 17 00:00:00 2001
From: Patrick Ohly 
Date: Fri, 30 Sep 2011 18:11:41 +0200
Subject: [PATCH 1/5] COMPAREFIELDS(): fix -1 case

The compare result was only 1 or 0, never -1, because of a teriary
operator which mapped all non-null values to 1. Cut-and-paste error?

The fix passes the underlying value through unchanged. Perhaps this
will return values like -999 to the caller, in contradiction to the
definition of the macro. However, it is unclear what should be
returned in that case.
---
 src/sysync/multifielditemtype.cpp |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/sysync/multifielditemtype.cpp b/src/sysync/multifielditemtype.cpp
index 6f1d810..794baad 100755
--- a/src/sysync/multifielditemtype.cpp
+++ b/src/sysync/multifielditemtype.cpp
@@ -215,7 +215,7 @@ public:
 if (!mfitP->fFirstItemP) aTermP->setAsInteger(SYSYNC_NOT_COMPARABLE);
 else {
   aTermP->setAsInteger(
-mfitP->fFirstItemP->standardCompareWith(*(mfitP->fSecondItemP),mfitP->fEqMode,OBJDEBUGTEST(mfitP,DBG_SCRIPTS+DBG_DATA+DBG_MATCH)) ? 1 : 0
+mfitP->fFirstItemP->standardCompareWith(*(mfitP->fSecondItemP),mfitP->fEqMode,OBJDEBUGTEST(mfitP,DBG_SCRIPTS+DBG_DATA+DBG_MATCH))
   );
 }
   }; // func_CompareFields
-- 
1.7.2.5

>From dbb5e1829e572b198694366a535e8b92ec0c07aa Mon Sep 17 00:00:00 2001
From: Patrick Ohly 
Date: Fri, 30 Sep 2011 11:13:56 +0200
Subject: [PATCH 3/5] 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.
---
 src/sysync/customimplds.cpp |   65 ++
 src/sysync/customimplds.h   |2 +-
 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, ch

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

2011-09-30 Thread Patrick Ohly
On Fri, 2011-09-30 at 12:10 +0200, Patrick Ohly wrote:
> This does not yet address the situation where the remote side also
> wasn't changed. A redundant Replace command will be sent in that case.
> Do you have a hint for me how this can be avoided? In the server case,
> is it as simple as avoiding the SendItemAsServer(augmentedItemP)
> call? 

That really seems to do the trick. Does the attached patch look right?
It works for me (TM).

-- 
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 92ca341dfd67b3cbc4178b57b66a2f24439fa178 Mon Sep 17 00:00:00 2001
From: Patrick Ohly 
Date: Fri, 30 Sep 2011 12:58:20 +0200
Subject: [PATCH] DB_Conflict (409): avoid sending unnecessary changes back to client

In server mode, when the merging after a 409 from the DB determines
that the client already has the latest data, sending it a Replace
command can be avoided.

This patch achieves that by introducing a new boolean
remoteHasLatestData which is false by default and only set when it is
certain that sending the update can be avoided. To minimize changes,
the DB_DataReplaced code path is left unchanged. It could be changed
to set the same boolean.

Not sure whether that will also avoid unnecessary changes sent when a
client detects a conflict.
---
 src/sysync/customimplds.cpp |   24 ++--
 src/sysync/customimplds.h   |2 +-
 2 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/src/sysync/customimplds.cpp b/src/sysync/customimplds.cpp
index 338f4d9..d3c32c8 100755
--- a/src/sysync/customimplds.cpp
+++ b/src/sysync/customimplds.cpp
@@ -2720,7 +2720,7 @@ localstatus TCustomImplDS::implProcessMap(cAppCharP aRemoteID, cAppCharP aLocalI
 
 /// 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)
+TMultiFieldItem *TCustomImplDS::mergeWithDatabaseVersion(TSyncItem *aSyncItemP, bool &aChangedDBVersion, bool &aChangedNewVersion)
 {
   aChangedDBVersion = false;
 
@@ -2754,16 +2754,15 @@ TMultiFieldItem *TCustomImplDS::mergeWithDatabaseVersion(TSyncItem *aSyncItemP,
 "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);
+aSyncItemP->mergeWith(*dbVersionItemP, aChangedNewVersion, aChangedDBVersion, this);
   } else {
-dbVersionItemP->mergeWith(*aSyncItemP, aChangedDBVersion, changedNewVersion, this);
+dbVersionItemP->mergeWith(*aSyncItemP, aChangedDBVersion, aChangedNewVersion, 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 ",
+  aChangedNewVersion ? "" : "NOT ",
   crstrategy==cr_server_wins ? "winning" : "loosing",
   aChangedDBVersion ? "to-be-replaced" : "to-be-left-unchanged",
   aChangedDBVersion ? "" : "NOT "
@@ -2839,6 +2838,7 @@ bool TCustomImplDS::implProcessItem(
 }
 // - now perform op
 aStatusCommand.setStatusCode(510); // default DB error
+bool remoteHasLatestData = false;
 switch (sop) {
   /// @todo sop_copy is now implemented by read/add sequence
   ///   in localEngineDS, but will be moved here later possibly
@@ -2863,8 +2863,8 @@ 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.
-  bool changedDBVersion;
-  augmentedItemP = mergeWithDatabaseVersion(myitemP, changedDBVersion);
+  bool changedDBVersion, changedNewVersion;
+  augmentedItemP = mergeWithDatabaseVersion(myitemP, changedDBVersion, changedNewVersion);
   if (augmentedItemP==NULL)
 sta = DB_Error; // no item found, DB error 
   else {
@@ -2886,6 +2886,10 @@ bool TCustomImplDS::implProcessItem(
 fLocalItemsUpdated++;
   sta = DB_DataMerged;
 }
+   

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

2011-09-30 Thread Patrick Ohly
On Thu, 2011-09-15 at 09:32 +0200, Patrick Ohly wrote:
> There's one more minor issue: in server mode, when an Add command
> results in modifying an existing item, the statistics say that the item
> was added. That means that the numbers do not add up, because "items
> before sync + added items != items after sync".
> 
> I'm seeing it with DB_Conflict, but I think it'll also occur with
> DB_DataReplaced/Merged.
> 
> The point where the statistics are changes is in
> TLocalEngineDS::engProcessRemoteItemAsServer():
> 
> // add allowed
> ===>fLocalItemsAdded++;
> #ifdef OBJECT_FILTERING
> // test if acceptable
> if (!isAcceptable(aSyncItemP,aStatusCommand)) { ok=false; break; } // 
> cannot be accepted
> // Note: making item to pass sync set filter is implemented in 
> derived DB implementation
> //   as criteria for passing might be in data that must first be read 
> from the DB
> #endif
> remainsvisible=true; // should remain visible
> ok=logicProcessRemoteItem(aSyncItemP,aStatusCommand,remainsvisible); 
> // add to local database NOW
> 
> What would be the right fix for this? Add another retval to
> logicProcessRemoteItem() which tells the caller whether an add was
> turned into an update? Or change the return value from bool to an enum?

None of these options seemed very attractive, because they are so
intrusive. How about the attached patch instead? Its purely local to the
409 handling code.

-- 
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 87b76b0098a884c7ab7a50d1910e928e83af12cc Mon Sep 17 00:00:00 2001
From: Patrick Ohly 
Date: Fri, 30 Sep 2011 11:56:28 +0200
Subject: [PATCH 3/3] DB_Conflict (409): correctly count updated and unmodified items

TLocalEngineDS::engProcessRemoteItemAsServer() in localengineds.cpp
counts Add commands as "added items", even if later on a 409 DB result
causes the item to be updated or be left unmodified.

Instead changing the whole statistics logic, this patch merely fixes
the statistics locally in the 409 handling code.
---
 src/sysync/customimplds.cpp |   13 -
 1 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/src/sysync/customimplds.cpp b/src/sysync/customimplds.cpp
index b30bc17..338f4d9 100755
--- a/src/sysync/customimplds.cpp
+++ b/src/sysync/customimplds.cpp
@@ -2874,7 +2874,18 @@ bool TCustomImplDS::implProcessItem(
 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; 
+if (IS_SERVER && sta==LOCERR_OK) {
+  // TLocalEngineDS::engProcessRemoteItemAsServer() in
+  // localengineds.cpp already counted the item as added
+  // because it didn't know that special handling would be
+  // needed. Instead of a complicated mechanism to report
+  // back the actual outcome, let's fix the statistics
+  // here.
+  fLocalItemsAdded--;
+  if (changedDBVersion)
+fLocalItemsUpdated++;
+  sta = DB_DataMerged;
+}
   }
 }
 if (IS_SERVER) {
-- 
1.7.2.5

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


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

2011-09-30 Thread Patrick Ohly
On Thu, 2011-09-15 at 15:40 +0200, Lukas Zeller wrote:
> Hi Patrick,
> 
> On Sep 12, 2011, at 20:33 , Patrick Ohly wrote:
> 
> > The only remaining question is: should the merge take full advantage of
> > the age information available in this case ("newer wins") or merge data
> > on a per-property basis for those which support it (multi-line +
> > merge=lines)?
> 
> What it exactly *should* do is probably depending on the reason why
> the plugin asks for merging, and would need to tweak the merge
> options / merge scripts such that it makes sense.
> 
> What it does so far is only a merge, with fixed roles: the incoming
> item is the "winning" side, the DB item is the "loosing" side, without
> checking age information. Without a merge script, what happens is what
> TMultiFieldItem::standardMergeWith() does, which is
> based on the "merge" option set in the field list.

I think that makes sense, except for the "fixed roles" - more on that
below.

> > What I see is right now is that for SUMMARY, the most recent value is
> > used (good!).
> 
> It's just the value coming from the remote, most recent or not.

Then it happened to work as expected accidentally, or I wasn't looking
closely enough.

> Because "SUMMARY" has no field-level merge option, which means that
> the winning side overwrites the loosing side.
> Of course, in the "most recently made known to the recipient", the
> incoming item is the most recent one by definition, but not
> necessarily in terms of when the user last edited it.

I think in this particular case, iCalendar 2.0 with UID, doing an age
comparison by default would be the right choice. I have implemented
that, see attached patch. Does that look right?

Note that it also avoids local DB updates which were found to be
unnecessary because the local side wasn't changed.

This does not yet address the situation where the remote side also
wasn't changed. A redundant Replace command will be sent in that case.
Do you have a hint for me how this can be avoided? In the server case,
is it as simple as avoiding the SendItemAsServer(augmentedItemP) call? 

> Would it make sense to add an option to configure 509-merge run in one
> of "DB wins", "incomin wins", "newer wins" modes?

SyncEvolution doesn't need it at the moment, if you agree that the age
comparison should be used by default.

> > In the case of iCalendar 2.0 I wonder whether merge=lines is needed.
> > For modify/modify conflicts perhaps, but not so much for add/add
> > conflicts. Thoughts?
> 
> If you want to differentiate these cases, I'd say we'll need a
> MERGEREASON() script function so the script can act differently
> depending on why the merge is initiated. So far I see the following
> different reasons: slowsync non-perfect match, syncml update conflict,
> DB update conflict, DB add conflict.

I'm inclined to rather remove the merge=lines and thus keep properties
from the winning side in all cases.

Except... what if a dumb phone truncates a property and then a slow sync
is done? Will the default merge mode preserve the longer property or
truncate if the version on the phone happens to be considered "winning"?

-- 
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 2fa26d7027686990bf89ace5fa0699e0dca8377f Mon Sep 17 00:00:00 2001
From: Patrick Ohly 
Date: Fri, 30 Sep 2011 11:13:56 +0200
Subject: [PATCH 1/3] 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.
---
 src/sysync/customimplds.cpp |   65 ++
 src/sysync/customimplds.h   |2 +-
 2 files changed, 53 insertions(+), 14 deletions(-)

diff --git a/src/sysync/customimplds.cpp b/src/sysync/customimplds.cpp
index f6ed50a..9445391 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 databas

  1   2   3   4   >