On Fri, 2014-05-02 at 22:08 +0200, Lukas Zeller wrote:
> Hello Patrick,
> 
> On 02.05.2014, at 13:44, Patrick Ohly <[email protected]> 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 <[email protected]>
Date: Tue, 6 May 2014 10:48:58 +0200
Subject: [PATCH 1/2] profile: <parameter sharedfield>

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=aShowNonEmpty;
   showInCTCap=aShowInCTCap;
+  sharedField=aSharedField;
   modeDependency=aModeDep;
   #ifndef NO_REMOTE_RULES
   ruleDependency=NULL;
@@ -1087,11 +1090,11 @@ void TPropertyDefinition::addNameExt(TProfileDefinition *aRootProfile, // for pr
 
 TParameterDefinition *TPropertyDefinition::addParam(
   const char *aName, bool aDefault, bool aExtendsName, bool aShowNonEmpty, bool aShowInCTCap, TMimeDirMode aModeDep
-)
+, bool aSharedField)
 {
   TParameterDefinition **paramPP = &parameterDefs;
   while(*paramPP!=NULL) paramPP=&((*paramPP)->next); // find last in chain
-  *paramPP = new TParameterDefinition(aName,aDefault,aExtendsName,aShowNonEmpty,aShowInCTCap, aModeDep);
+  *paramPP = new TParameterDefinition(aName,aDefault,aExtendsName,aShowNonEmpty,aShowInCTCap, aModeDep, aSharedField);
   return *paramPP;
 } // TPropertyDefinition::addParam
 
@@ -4154,6 +4157,37 @@ bool TMimeDirProfileHandler::parseProperty(
                   else
                     dostore = true; // at least one field exists, we might store
                 }
+                // - check if shared fields used for parameters are available;
+                //   must be enabled explicitly with <parameter ... sharedfield="yes">
+                const TParameterDefinition *paramP = aPropP->parameterDefs;
+                while (paramP) {
+                  if (paramP->sharedField &&
+                      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 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=paramP->next;
+                }
                 // check if we can test more repetitions
                 if (!dostore) {
                   if (aRepArray[repid]+1<maxrep || maxrep==REP_ARRAY) {
@@ -4290,6 +4324,21 @@ bool TMimeDirProfileHandler::parseProperty(
         aItem.getArrayFieldAdjusted(fid+baseoffset,repoffset,false);
       }
     }
+    const TParameterDefinition *paramP = aPropP->parameterDefs;
+    while (paramP) {
+      if (paramP->sharedField &&
+          mimeModeMatch(paramP->modeDependency)
+#ifndef NO_REMOTE_RULES
+          && (!paramP->ruleDependency || isActiveRule(paramP->ruleDependency))
+#endif
+          ) {
+        sInt16 fid=paramP->convdef.fieldid;
+        if (fid>=0) {
+          aItem.getArrayFieldAdjusted(fid+baseoffset,repoffset,false);
+        }
+      }
+      paramP=paramP->next;
+    }
   }
   if (!valuelist && repid>=0 && (notempty || !overwriteempty) && !repoffsByGroup) {
     // we have used this repetition and actually stored values, so count it now
diff --git a/src/sysync/mimedirprofile.h b/src/sysync/mimedirprofile.h
index f8453b5..b4b381f 100755
--- a/src/sysync/mimedirprofile.h
+++ b/src/sysync/mimedirprofile.h
@@ -157,7 +157,7 @@ public:
 class TParameterDefinition : noncopyable {
 public:
   // constructor/destructor
-  TParameterDefinition(const char *aName, bool aDefault, bool aExtendsName, bool aShowNonEmpty, bool aShowInCTCap, TMimeDirMode aModeDep);
+  TParameterDefinition(const char *aName, bool aDefault, bool aExtendsName, bool aShowNonEmpty, bool aShowInCTCap, TMimeDirMode aModeDep, bool aSharedField);
   ~TParameterDefinition();
   // tools
   TConversionDef *setConvDef(sInt16 aFieldId=FID_NOT_SUPPORTED,sInt16 aConvMode=0,char aCombSep=0)
@@ -177,6 +177,9 @@ public:
   bool shownonempty;
   // flag if parameter should be (not necessarily IS, depending on SyncML version) shown in CTCap
   bool showInCTCap;
+  // flag if parameter field is shared between different properties and thus needs to be checked
+  // when looking for the offset where repeating properties get stored
+  bool sharedField;
   // conversion information
   TConversionDef convdef;
   #ifndef NO_REMOTE_RULES
@@ -244,7 +247,7 @@ public:
   TPropertyDefinition(const char* aName, sInt16 aNumVals, bool aMandatory, bool aShowInCTCap, bool aSuppressEmpty, uInt16 aDelayedProcessing, char aValuesep, char aAltValuesep, uInt16 aPropertyGroupID, bool aCanFilter, TMimeDirMode aModeDep, sInt16 aGroupFieldID, bool aAllowFoldAtSep);
   ~TPropertyDefinition();
   // tools
-  TParameterDefinition *addParam(const char *aName, bool aDefault, bool aExtendsName, bool aShowNonEmpty=false, bool aShowInCTCap=false, TMimeDirMode aModeDep=numMimeModes);
+  TParameterDefinition *addParam(const char *aName, bool aDefault, bool aExtendsName, bool aShowNonEmpty=false, bool aShowInCTCap=false, TMimeDirMode aModeDep=numMimeModes, bool aSharedField=false);
   void addNameExt(TProfileDefinition *aRootProfile, // for profile-global RepID generation
     TNameExtIDMap aMusthave_ids, TNameExtIDMap aForbidden_ids, TNameExtIDMap aAddtlSend_ids,
     sInt16 aFieldidoffs, sInt16 aMaxRepeat=1, sInt16 aRepeatInc=1, sInt16 aMinShow=-1,
-- 
1.7.10.4

_______________________________________________
os-libsynthesis mailing list
[email protected]
http://lists.synthesis.ch/mailman/listinfo/os-libsynthesis

Reply via email to