felix-users mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Guillaume Nodet <gno...@apache.org>
Subject Re: Fileinstall, problems with variable substitution and write back
Date Wed, 04 Dec 2013 12:13:06 GMT
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