Mailing-List: contact users-help@felix.apache.org; run by ezmlm
Precedence: bulk
Reply-To: users@felix.apache.org
Received-SPF: pass (nike.apache.org: domain of bengt.rodehav@gmail.com
 designates 209.85.192.172 as permitted sender)
MIME-Version: 1.0
Sender: bengt.rodehav@gmail.com
In-Reply-To: 
 <CAJ0TPGKzv4pyCe=R4UKvaqjqypCmefgAti5svHXYv_WJy9Pe7w@mail.gmail.com>
References: 
 <CAJ0TPGK1+xa+SNCXG1BehcriOwMnPP+3Qp2ip=Qt=qcYwkAFZA@mail.gmail.com>
	<CAJ0TPGKqrvGgiG4cL8ZydnfbD8RyxvGSAzwJkoG11koWfmPwag@mail.gmail.com>
	<CAJ0TPGKNg3gUTUJG9JL_zKXXM1FFtCX1CV49McmA1_Z0_92yhQ@mail.gmail.com>
	<CAJ0TPGK4rca1vRfVuMWyncKn20_RJLQKhHXHS-mLSB+XsSoz6A@mail.gmail.com>
	<CAJ0TPGKzv4pyCe=R4UKvaqjqypCmefgAti5svHXYv_WJy9Pe7w@mail.gmail.com>
Date: Mon, 2 Dec 2013 11:23:06 +0100
Message-ID: 
 <CAJ0TPGKF4=QRSjO2gV24AFhFB4axZ1Chf6TO8JOC1=SDA7stYQ@mail.gmail.com>
Subject: Re: Fileinstall, problems with variable substitution and write back
From: Bengt Rodehav <bengt@rodehav.com>
To: "users@felix.apache.org" <users@felix.apache.org>
Content-Type: multipart/alternative; boundary=047d7b86e3a8d8834404ec8a90ef

--047d7b86e3a8d8834404ec8a90ef
Content-Type: text/plain; charset=ISO-8859-1

I've replaced FELIX-4332 with FELIX-4338 and FELIX-4339.

I have attached a patch for FELIX-4338 and hope that someone can have a
look at it and possibly commit it.

FELIX-4339 is trickier but I would appreciate a discussion about how this
should be handled.

/Bengt


2013/11/29 Bengt Rodehav <bengt@rodehav.com>

> I've tested more with the proposed change in order to stop FileInstall to
> incorrectly change the contents of the configuration file (problem b) from
> my previous post). It seems to work fine. I would really like that to be
> fixed. Would you like me to create a patch atttached to the JIRA?
>
> Problem a) is probably not trivial to fix. I've experimented a lot and
> it's very hard for me to foresee how many escape characters I need in
> different circumstances. One real life example for me is how I configure an
> integration service that uses a Camel route underneath. If I put the
> followiing contents in a test.cfg file:
>
> *mydir=C:/temp*
>
> *timestampedfile=$\\\\{file:onlyname\\\\}-$\\\\{date:now:yyyyMMddHHmmssSSS\\\\}.$\\\\{file:ext\\\\}*
> *move=${mydir}/archive/$\\{date:now:yyyyMMdd\\}/${timestampedfile}*
> *moveFailed=${mydir}/failed/${timestampedfile}*
> *fromUri=file:${mydir}?move=${move}&moveFailed=${moveFailed}*
>
> And execute the following command:
>
>
> *config:list "(service.pid=test)"*
>
> I get the following output:
>
> *----------------------------------------------------------------*
> *Pid:            test*
> *BundleLocation: null*
> *Properties:*
> *   moveFailed =
> C:/temp/failed/${file:onlyname}-${date:now:yyyyMMddHHmmssSSS}.${file:ext}*
> *   mydir = C:/temp*
> *   timestampedfile =
> $\{file:onlyname\}-$\{date:now:yyyyMMddHHmmssSSS\}.$\{file:ext\}*
> *   service.pid = test*
> *   fromUri =
> file:C:/temp?move=C:/temp/archive//-.&moveFailed=C:/temp/failed/-.*
> *   move =
> C:/temp/archive/${date:now:yyyyMMdd}/${file:onlyname}-${date:now:yyyyMMddHHmmssSSS}.${file:ext}*
> *   felix.fileinstall.filename =
> file:/C:/dev/karaf/connect/common/etc/test.cfg*
>
> Thus, the variables "move" and "moveFailed" looks the way I want but the
> final variable "fromUri" is messed up because of an extra variable
> substitution.
>
> I haven't managed to come up with any number of backslashes that will
> produce the correct result for me.
>
> The only workaround I have right now is to not use variables at all. It
> does, however, make the configuration files extremely verbose and it's easy
> to introduce errors that way.
>
> Presently, variable substitution is very unpredictable since it's being
> done in a recursive way. I would prefer doing it in an iterative manner to
> make it predictable. E g "${a}" should always evaluate to the same value no
> matter where in the configuration file it is referenced.
>
> /Bengt
>
>
>
>
>
>
>
>
> 2013/11/28 Bengt Rodehav <bengt@rodehav.com>
>
>> I've investigated this a bit more. There are actually two different
>> problems:
>>
>> a) The number of escape characters I need depends on from where I
>> reference the variable. For every indirection I need to double the number
>> of backslashes. This also means that all uses of a variable containing
>> escape characters must be used from the same level of indirection. A bit
>> complicated but it's due to the fact that all variables are evaluated
>> dynamically. This means that unescaping can occur several times.
>>
>> b) FileInstall incorrectly thinks that a configuration property is
>> changed and therefore overwrites the property with the evaluated value.
>>
>> I think I've found the reason (and possibly a solution) to b).
>>
>> In the ConfigInstaller.setConfig() method the properties are read from a
>> configuration file and propagated as a configuration. Here is an excerpt
>> from that method:
>>
>> *                final Properties p = new Properties();*
>> *                in.mark(1);*
>> *                boolean isXml = in.read() == '<';*
>> *                in.reset();*
>> *                if (isXml) {*
>> *                    p.loadFromXML(in);*
>> *                } else {*
>> *                    p.load(in);*
>> *                }*
>> *                InterpolationHelper.performSubstitution((Map) p,
>> context);*
>> *                ht.putAll(p);*
>>
>> Note that the file is read using Java's standard Properties class. The
>> unescaping is also done by that class. Then, at the end, the variable
>> substitution is done as a separate call.
>>
>> Then look at the ConfigInstaller.configurationEvent() method:
>>
>> *        if (configurationEvent.getType() ==
>> ConfigurationEvent.CM_UPDATED)*
>> *        {*
>> *            try*
>> *            {*
>> *                Configuration config =
>> getConfigurationAdmin().getConfiguration(*
>> *                                            configurationEvent.getPid(),*
>> *
>> configurationEvent.getFactoryPid());*
>> *                Dictionary dict = config.getProperties();*
>> *                String fileName = (String) dict.get(
>> DirectoryWatcher.FILENAME );*
>> *                File file = fileName != null ? fromConfigKey(fileName) :
>> null;*
>> *                if( file != null && file.isFile()   ) {*
>> *                    if( fileName.endsWith( ".cfg" ) )*
>> *                    {*
>> *                        org.apache.felix.utils.properties.Properties
>> props = new org.apache.felix.utils.properties.Properties( file, context );*
>>
>> Note that now the configuration file is read using
>> org.apache.felix.utils.properties.Properties class. It turns out that they
>> don't produce identical results. I haven't investigated exactly how they
>> differ but they do.
>>
>> A simple test:
>>
>> 1. Create a configuration file with the following content:
>>
>> a=$\\\\{var}
>> ab=${a}b
>> abc=${ab}c
>>
>> 2. Add the following line at the end:
>>
>> d=foo
>>
>> 3. FileInstall will now incorrectly change the contents of the
>> configuration file to:
>>
>>  a=$\\\\{var}
>> ab=${a}b
>> abc = ${var}bc
>> d=foo
>>
>> Now if I change the ConfigInstaller.setConfig() method to the following:
>>
>> *org.apache.felix.utils.properties.Properties p = new
>> org.apache.felix.utils.properties.Properties( f, context );*
>> *InterpolationHelper.performSubstitution((Map) p, context);*
>>
>> Then FileInstall will not incorrectly change the contents of the
>> configuration file.
>>
>> I propose to do this change in order to solve problem b) above. I
>> appreciate if you have any thoughts on this.
>>
>> I realize that problem a) is trickier due to the dynamic nature of
>> variable substitution. I haven't yet determined how I think the escape
>> characters should be handled but the current situation is not ideal.
>>
>> /Bengt
>>
>>
>>
>>
>> 2013/11/28 Bengt Rodehav <bengt@rodehav.com>
>>
>>> JIRA created:
>>>
>>> https://issues.apache.org/jira/browse/FELIX-4332
>>>
>>> /Bengt
>>>
>>>
>>> 2013/11/28 Bengt Rodehav <bengt@rodehav.com>
>>>
>>>> I've come up with easily reproducable errors using Karaf 2.3.3:
>>>>
>>>> - Install a fresh Karaf 2.3.3
>>>> - Add the following line to etc/custom.properties:
>>>>   felix.fileinstall.enableConfigSave = true
>>>>
>>>> Create a file etc/test.cfg with the following contents:
>>>>
>>>> a=$\\{var}
>>>> ab=${a}b
>>>> abc=${ab}c
>>>>
>>>> I expect this to be evaluated to:
>>>> a=$\{var}
>>>> ab=$\{var}b
>>>> abc=$\{var}bc
>>>>
>>>> But if I execute the Karaf command:
>>>>
>>>>   config:list "(service.pid=test)"
>>>>
>>>> I get:
>>>>
>>>> ----------------------------------------------------------------
>>>> Pid:            test
>>>> BundleLocation: null
>>>> Properties:
>>>>    service.pid = test
>>>>    a = ${var}
>>>>    abc = bc
>>>>    felix.fileinstall.filename =
>>>> file:/C:/dev/Karaf/apache-karaf-2.3.3/etc/test.cfg
>>>>    ab = b
>>>>
>>>> My interpretation of this is that the variable "a" has been correctly
>>>> evaluated. But, when evalutating the variable "ab" it seems that the
>>>> variable "a" is evaluated again despite the fact that it has already been
>>>> evaluated. FileInstall now looks for the value of a variable called "var"
>>>> which evalutes to an empty string because there is no such variable.
>>>>
>>>> The variable "abc" consequently evaluates to "bc" since the variable
>>>> "ab" has been evaluated to "b".
>>>>
>>>> To make it even worse, now change the first row in test.cfg to:
>>>>
>>>> a=$\\\\{var}
>>>>
>>>> We now get:
>>>>
>>>> ----------------------------------------------------------------
>>>> Pid:            test
>>>> BundleLocation: null
>>>> Properties:
>>>>    service.pid = test
>>>>    a = $\{var}
>>>>    abc = ${var}bc
>>>>    felix.fileinstall.filename =
>>>> file:/C:/dev/Karaf/apache-karaf-2.3.3/etc/test.cfg
>>>>    ab = ${var}b
>>>>
>>>> Thus we get the same phenomenom. The variable "a" is evaluated
>>>> differently if it is evaluated on its own or as part of another expression.
>>>> But, due to having configured FileInstall to write back changes, the
>>>> contents of the test.cfg is now changed by FileInstall despite the fact
>>>> that the configuration has not changed at all. The contents of test.cfg is
>>>> now:
>>>>
>>>> a=$\\\\{var}
>>>> ab=${a}b
>>>> abc = ${var}bc
>>>>
>>>> The "abc" variable has been altered. FileInstall has incorrectly
>>>> determined that its value has changed.
>>>>
>>>> This is clearly a bug. I will create a JIRA.
>>>>
>>>> /Bengt
>>>>
>>>>
>>>>
>>>> 2013/11/26 Bengt Rodehav <bengt@rodehav.com>
>>>>
>>>>> I'm using Apache Karaf 2.3.3 which comes with FileInstall 3.2.6. I
>>>>> have set the felix.fileinstall.enableConfigSave property to true in order
>>>>> to have FileInstall write back configuration changes to the file. Normally
>>>>> all configuration changes are done by editing the configuration file but
>>>>> there is one property that I change programmatically using ConfigAdmin (an
>>>>> "enable" property to start/stop my service). I am dependent on that
>>>>> property being persisted in the configuration file which is why I set the
>>>>> enableConfigSave property to true.
>>>>>
>>>>> When configuring FileInstall to write back configuration changes to
>>>>> the configuration file, it is important that variables are not substituted
>>>>> for the evaluated value. This normally works since FileInstall evalutates
>>>>> the property in the configuration file and compares it with the
>>>>> configuration admin's value. If they are the same, the value in the
>>>>> configuration file is kept unchanged.
>>>>>
>>>>> However, when using the escape character this is broken. In my case
>>>>> I'm using Apache Camel underneath. When configuring routes via the config
>>>>> admin, I sometimes need to set a value to
>>>>> "${expression-to-be-evaluated-by-camel}". I therefore escape the "{" and
>>>>> "}" to stop FileInstall from trying to evaluate the expression. Like this:
>>>>>
>>>>> $\\{expression-to-be-evaluated-by-camel\\}
>>>>>
>>>>> This also normally works but not when I have an indirection. E g when
>>>>> specifying the following:
>>>>>
>>>>> a=$\\{var}
>>>>> ab=${a}b
>>>>>
>>>>> FileInstall will change the configuration file to:
>>>>>
>>>>> a=$\\{var}
>>>>> ab = ${var}b
>>>>>
>>>>> Note that the variable "ab" has now been expanded and written back to
>>>>> the configuration file even if neither of the variables "a" and "ab" have
>>>>> been changed.
>>>>>
>>>>> I think this is because FileInstall does the following:
>>>>>
>>>>> 1. Calculates the value of "a" to "$\{var}
>>>>> 2. Calculates the value of "b" to "${var}b
>>>>>
>>>>> Note that every evaluation will perform "unescaping". This means that
>>>>> an extra "unescaping" will be done for every indirection which fools
>>>>> FileInstall into thinking that the property has been changed.
>>>>>
>>>>> I'm not exactly sure how this should be fixed in FileInstall. One idea
>>>>> is to never "unescape" already evaluated variables. Actually I think this
>>>>> is probably what would fix this...
>>>>>
>>>>> Does anybody have any ideas about this? Should I create a JIRA?
>>>>>
>>>>> /Bengt
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>
>>>
>>
>

--047d7b86e3a8d8834404ec8a90ef--