felix-users mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Bengt Rodehav <be...@rodehav.com>
Subject Re: Fileinstall, problems with variable substitution and write back
Date Wed, 04 Dec 2013 14:53:22 GMT
BTW. I did some experimenting with declaring the properties in different
order in the configuration file. It did not seem to matter. I was under the
impression that the recursive variable substitution is what makes it
non-deterministic.

If a property has been evaluated already it should not be evaluated again
because another layer of the escape characters will then be removed.

/Bengt


2013/12/4 Bengt Rodehav <bengt@rodehav.com>

> OK.
>
>
> 2013/12/4 Guillaume Nodet <gnodet@apache.org>
>
>> Unfortunately, it does not seem to be sufficient, I'm investigating
>> further
>>
>>
>> 2013/12/4 Bengt Rodehav <bengt@rodehav.com>
>>
>> > I noticed that you seem to have fixed the issues I had reported
>> Guillaume.
>> > Thanks a lot! Looking forward to the next release.
>> >
>> > /Bengt
>> >
>> >
>> > 2013/12/2 Bengt Rodehav <bengt@rodehav.com>
>> >
>> > > Thanks Guillaume!
>> > >
>> > >
>> > > 2013/12/2 Guillaume Nodet <gnodet.apache@gmail.com>
>> > >
>> > >> I'll try to have a look at those today or tomorrow.
>> > >>
>> > >>
>> > >> 2013/12/2 Bengt Rodehav <bengt@rodehav.com>
>> > >>
>> > >> > 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
>> > >> > >>>>>
>> > >> > >>>>>
>> > >> > >>>>>
>> > >> > >>>>>
>> > >> > >>>>>
>> > >> > >>>>>
>> > >> > >>>>>
>> > >> > >>>>
>> > >> > >>>
>> > >> > >>
>> > >> > >
>> > >> >
>> > >>
>> > >>
>> > >>
>> > >> --
>> > >> -----------------------
>> > >> Guillaume Nodet
>> > >> ------------------------
>> > >> Red Hat, Open Source Integration
>> > >>
>> > >> Email: gnodet@redhat.com
>> > >> Web: http://fusesource.com
>> > >> Blog: http://gnodet.blogspot.com/
>> > >>
>> > >
>> > >
>> >
>>
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message