Thanks Guillaume!
2013/12/2 Guillaume Nodet <[email protected]> > I'll try to have a look at those today or tomorrow. > > > 2013/12/2 Bengt Rodehav <[email protected]> > > > 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 <[email protected]> > > > > > 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 <[email protected]> > > > > > >> 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 <[email protected]> > > >> > > >>> JIRA created: > > >>> > > >>> https://issues.apache.org/jira/browse/FELIX-4332 > > >>> > > >>> /Bengt > > >>> > > >>> > > >>> 2013/11/28 Bengt Rodehav <[email protected]> > > >>> > > >>>> 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 <[email protected]> > > >>>> > > >>>>> 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: [email protected] > Web: http://fusesource.com > Blog: http://gnodet.blogspot.com/ >
