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
