On Thu, 2013-07-04 at 15:56 +0200, Lukas Zeller wrote: > On 04.07.2013, at 12:01, Patrick Ohly <patrick.o...@intel.com> wrote: > > > I just noticed one aspect of the example configs that I wasn't aware of: > > syncclient_sample_config.xml: <field name="PHOTO" type="blob" > > 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: '<BLOB size=0>' <> '<BLOB size=0>'" 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_51D53D1800000001_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 <comparescript> 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: <field name="PHOTO" type="string" compare="conflict" merge="fillempty"/> <field name="PHOTO_TYPE" type="string" compare="never" merge="fillempty"/> <field name="PHOTO_VALUE" type="string" compare="never" merge="fillempty"/> And here's the merge script: <!-- Special treatment of PHOTO data: keep a local file if it has the same content as the binary data in the winning item. Use in combination with a PHOTO field of type="string" (not blob, because we need to be able to compare the field in the MERGEFIELDS() call to detect when the data really changes) and compare="conflict" (not used to find matches, changes are relevant). --> <macro name="VCARD_MERGESCRIPT"><![CDATA[ integer mode; mode = MERGEMODE(); if (mode == 1 && WINNING.PHOTO == EMPTY) { // Removing photo from loosing item. if (LOOSING.PHOTO != EMPTY) { SETLOOSINGCHANGED(1); } LOOSING.PHOTO = EMPTY; LOOSING.PHOTO_TYPE = ""; LOOSING.PHOTO_VALUE = ""; } else if (LOOSING.PHOTO != EMPTY) { // Updating photo. Might actually be the same! if (LOOSING.PHOTO_VALUE == "uri" && (WINNING.PHOTO_VALUE == EMPTY || WINNING.PHOTO_VALUE == "binary")) { string path; path = URITOPATH(LOOSING.PHOTO); if (path) { if (READ(path) == WINNING.PHOTO) { // Reuse photo file from loosing item. // If we need to write back for some other reason, we'll inline the data again. WINNING.PHOTO = LOOSING.PHOTO; WINNING.PHOTO_TYPE = LOOSING.PHOTO_TYPE; WINNING.PHOTO_VALUE = LOOSING.PHOTO_VALUE; } } } } // Continue merge. MERGEFIELDS(mode); ]]></macro> 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 <patrick.o...@intel.com> 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/multifielditemtype.cpp b/src/sysync/multifielditemtype.cpp index 6dd32d9..da00af4 100755 --- a/src/sysync/multifielditemtype.cpp +++ b/src/sysync/multifielditemtype.cpp @@ -195,9 +195,11 @@ public: static void func_MergeFields(TItemField *&aTermP, TScriptContext *aFuncContextP) { TMultiFieldItemType *mfitP = static_cast<TMultiFieldItemType *>(aFuncContextP->getCallerContext()); - if (mfitP->fFirstItemP) + if (mfitP->fFirstItemP) { + TItemField *argP = aFuncContextP->getLocalVar(0); mfitP->fFirstItemP->standardMergeWith(*(mfitP->fSecondItemP),mfitP->fChangedFirst,mfitP->fChangedSecond, - aFuncContextP->getLocalVar(0)->getAsInteger()); + argP ? argP->getAsInteger() : 0); + } }; // func_MergeFields @@ -360,7 +362,7 @@ const TBuiltInFuncDef DataTypeFuncDefs[] = { { "DELETEWINS", TMFTypeFuncs::func_DeleteWins, fty_none, 0, NULL }, { "PREVENTADD", TMFTypeFuncs::func_PreventAdd, fty_none, 0, NULL }, { "IGNOREUPDATE", TMFTypeFuncs::func_IgnoreUpdate, fty_none, 0, NULL }, - { "MERGEFIELDS", TMFTypeFuncs::func_MergeFields, fty_none, 0, NULL }, + { "MERGEFIELDS", TMFTypeFuncs::func_MergeFields, fty_none, 1, param_IntArg }, { "WINNINGCHANGED", TMFTypeFuncs::func_WinningChanged, fty_integer, 0, NULL }, { "LOOSINGCHANGED", TMFTypeFuncs::func_LoosingChanged, fty_integer, 0, NULL }, { "SETWINNINGCHANGED", TMFTypeFuncs::func_SetWinningChanged, fty_none, 1, param_IntArg }, -- 1.7.10.4
>From 9aaf87500996b4c4837abe049d0332b73ae2aa5f Mon Sep 17 00:00:00 2001 From: Patrick Ohly <patrick.o...@intel.com> Date: Fri, 5 Jul 2013 09:09:35 +0200 Subject: [PATCH 4/4] engine: <mergescript> + caching mode When in caching mode, the purpose of merging is to make the server item identical to the client item and detecting when the server item was changed. Doing this in combination with a merge script was not possible. Now the extra mode parameter is passed through standardMergeWith() into the merge script, where it can be retrieved via the new MERGEMODE() method. 0 is the traditional mode which updates both items. 1 updates the loosing item (this is the mode used for caching) and 2 updates the winning item (was added for the sake of completeness). --- src/sysync/multifielditem.cpp | 4 ++-- src/sysync/multifielditemtype.cpp | 20 ++++++++++++++++---- src/sysync/multifielditemtype.h | 4 +++- 3 files changed, 21 insertions(+), 7 deletions(-) diff --git a/src/sysync/multifielditem.cpp b/src/sysync/multifielditem.cpp index 3d00435..c933fae 100755 --- a/src/sysync/multifielditem.cpp +++ b/src/sysync/multifielditem.cpp @@ -1361,8 +1361,8 @@ void TMultiFieldItem::mergeWith(TSyncItem &aItem, bool &aChangedThis, bool &aCha TMultiFieldItem *multifielditemP = castToSameTypeP(&aItem); if (!multifielditemP) return; // do the merge - if (fItemTypeP && mode == MERGE_OPTION_FROM_CONFIG) - fItemTypeP->mergeItems(*this,*multifielditemP,aChangedThis,aChangedOther,aDatastoreP); + if (fItemTypeP) + fItemTypeP->mergeItems(*this,*multifielditemP,aChangedThis,aChangedOther,aDatastoreP, mode); else standardMergeWith(*multifielditemP,aChangedThis,aChangedOther, mode); // show result diff --git a/src/sysync/multifielditemtype.cpp b/src/sysync/multifielditemtype.cpp index da00af4..7028174 100755 --- a/src/sysync/multifielditemtype.cpp +++ b/src/sysync/multifielditemtype.cpp @@ -260,7 +260,16 @@ public: } }; // func_CompareFields - + // int MERGEMODE() + // returns mode of merging, see MERGEFIELDS() + static void func_MergeMode(TItemField *&aTermP, TScriptContext *aFuncContextP) + { + TMultiFieldItemType *mfitP = static_cast<TMultiFieldItemType *>(aFuncContextP->getCallerContext()); + if (!mfitP->fFirstItemP) aTermP->unAssign(); // no merging + else { + aTermP->setAsInteger(mfitP->fMergeMode); + } + }; // func_MergeMode #endif @@ -369,6 +378,7 @@ const TBuiltInFuncDef DataTypeFuncDefs[] = { { "SETLOOSINGCHANGED", TMFTypeFuncs::func_SetLoosingChanged, fty_none, 1, param_IntArg }, { "COMPAREFIELDS", TMFTypeFuncs::func_CompareFields, fty_integer, 0, NULL }, { "COMPARISONMODE", TMFTypeFuncs::func_ComparisonMode, fty_string, 0, NULL }, + { "MERGEMODE", TMFTypeFuncs::func_MergeMode, fty_integer, 0, NULL }, #endif { "SYNCOP", TMFTypeFuncs::func_SyncOp, fty_string, 0, NULL }, { "REJECTITEM", TMFTypeFuncs::func_RejectItem, fty_none, 1, param_IntArg }, @@ -796,18 +806,19 @@ void TMultiFieldItemType::mergeItems( TMultiFieldItem &aLoosingItem, bool &aChangedWinning, bool &aChangedLoosing, - TLocalEngineDS *aDatastoreP + TLocalEngineDS *aDatastoreP, + int mode ) { #ifndef SCRIPT_SUPPORT // just do standard merge - aWinningItem.standardMergeWith(aLoosingItem,aChangedWinning,aChangedLoosing); + aWinningItem.standardMergeWith(aLoosingItem,aChangedWinning,aChangedLoosing,mode); return; #else // if no script use standard merging string &script = static_cast<TMultiFieldTypeConfig *>(fTypeConfigP)->fMergeScript; if (script.empty()) { - aWinningItem.standardMergeWith(aLoosingItem,aChangedWinning,aChangedLoosing); + aWinningItem.standardMergeWith(aLoosingItem,aChangedWinning,aChangedLoosing,mode); return; } // execute script to perform merge @@ -818,6 +829,7 @@ void TMultiFieldItemType::mergeItems( fChangedFirst = aChangedWinning; fChangedSecond = aChangedLoosing; fCurrentSyncOp = fDsP->fCurrentSyncOp; + fMergeMode = mode; TScriptContext::execute( aDatastoreP->fReceivingTypeScriptContextP ? aDatastoreP->fReceivingTypeScriptContextP : diff --git a/src/sysync/multifielditemtype.h b/src/sysync/multifielditemtype.h index 62c2691..b2b0b02 100755 --- a/src/sysync/multifielditemtype.h +++ b/src/sysync/multifielditemtype.h @@ -170,7 +170,8 @@ public: TMultiFieldItem &aLoosingItem, bool &aChangedWinning, bool &aChangedLoosing, - TLocalEngineDS *aDatastoreP + TLocalEngineDS *aDatastoreP, + int mode //< MERGE_OPTION_* ); // check item before processing it bool checkItem(TMultiFieldItem &aItem, TLocalEngineDS *aDatastoreP); @@ -209,6 +210,7 @@ public: bool fChangedSecond; // helper for script context funcs TEqualityMode fEqMode; // helper for script context funcs TSyncOperation fCurrentSyncOp; // helper for script context funcs + int fMergeMode; // helper for script context funcs #endif private: // array of field options -- 1.7.10.4
_______________________________________________ os-libsynthesis mailing list os-libsynthesis@synthesis.ch http://lists.synthesis.ch/mailman/listinfo/os-libsynthesis