You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by bv...@apache.org on 2012/11/10 12:48:44 UTC

svn commit: r1407776 - in /camel/trunk/camel-core/src: main/java/org/apache/camel/component/properties/DefaultPropertiesResolver.java test/java/org/apache/camel/component/properties/PropertiesComponentLoadPropertiesFromFileTrimValuesTest.java

Author: bvahdat
Date: Sat Nov 10 11:48:43 2012
New Revision: 1407776

URL: http://svn.apache.org/viewvc?rev=1407776&view=rev
Log:
CAMEL-5784: Preparing the loaded properties should only trim any potential whitespace characters.

Modified:
    camel/trunk/camel-core/src/main/java/org/apache/camel/component/properties/DefaultPropertiesResolver.java
    camel/trunk/camel-core/src/test/java/org/apache/camel/component/properties/PropertiesComponentLoadPropertiesFromFileTrimValuesTest.java

Modified: camel/trunk/camel-core/src/main/java/org/apache/camel/component/properties/DefaultPropertiesResolver.java
URL: http://svn.apache.org/viewvc/camel/trunk/camel-core/src/main/java/org/apache/camel/component/properties/DefaultPropertiesResolver.java?rev=1407776&r1=1407775&r2=1407776&view=diff
==============================================================================
--- camel/trunk/camel-core/src/main/java/org/apache/camel/component/properties/DefaultPropertiesResolver.java (original)
+++ camel/trunk/camel-core/src/main/java/org/apache/camel/component/properties/DefaultPropertiesResolver.java Sat Nov 10 11:48:43 2012
@@ -131,11 +131,20 @@ public class DefaultPropertiesResolver i
         for (Map.Entry<Object, Object> entry : properties.entrySet()) {
             Object key = entry.getKey();
             Object value = entry.getValue();
-            // trim string values which can be a problem when loading from a properties file and there
-            // is leading or trailing spaces in the value
+            // trim any trailing spaces which can be a problem when loading from
+            // a properties file, note that java.util.Properties does already this
+            // for any potential leading spaces so there's nothing to do there
             if (value instanceof String) {
                 String s = (String) value;
-                s = s.trim();
+                int endIndex = s.length();
+                for (int index = s.length() - 1; index >= 0; index--) {
+                    if (s.charAt(index) == ' ') {
+                        endIndex = index;
+                    } else {
+                        break;
+                    }
+                }
+                s = s.substring(0, endIndex);
                 value = s;
             }
             answer.put(key, value);

Modified: camel/trunk/camel-core/src/test/java/org/apache/camel/component/properties/PropertiesComponentLoadPropertiesFromFileTrimValuesTest.java
URL: http://svn.apache.org/viewvc/camel/trunk/camel-core/src/test/java/org/apache/camel/component/properties/PropertiesComponentLoadPropertiesFromFileTrimValuesTest.java?rev=1407776&r1=1407775&r2=1407776&view=diff
==============================================================================
--- camel/trunk/camel-core/src/test/java/org/apache/camel/component/properties/PropertiesComponentLoadPropertiesFromFileTrimValuesTest.java (original)
+++ camel/trunk/camel-core/src/test/java/org/apache/camel/component/properties/PropertiesComponentLoadPropertiesFromFileTrimValuesTest.java Sat Nov 10 11:48:43 2012
@@ -16,7 +16,6 @@
  */
 package org.apache.camel.component.properties;
 
-import java.io.File;
 import java.io.FileOutputStream;
 
 import org.apache.camel.CamelContext;
@@ -39,15 +38,27 @@ public class PropertiesComponentLoadProp
         CamelContext context = super.createCamelContext();
 
         // create space.properties file
-        File file = new File("target/space/space.properties");
-        file.createNewFile();
-        FileOutputStream fos = new FileOutputStream(file);
-        fos.write("cool.leading= Leading space\ncool.trailing=Trailing space \ncool.both= Both leading and trailing space ".getBytes());
+        FileOutputStream fos = new FileOutputStream("target/space/space.properties");
+        String cool = "cool.leading= Leading space" + LS + "cool.trailing=Trailing space " + LS + "cool.both= Both leading and trailing space ";
+        fos.write(cool.getBytes());
+        fos.write(LS.getBytes());
+
+        String space = "space.leading=   \\r\\n" + LS + "space.trailing=\\t   " + LS + "space.both=  \\r   \\t  \\n   ";
+        fos.write(space.getBytes());
+        fos.write(LS.getBytes());
+
+        String mixed = "mixed.leading=   Leading space\\r\\n" + LS + "mixed.trailing=Trailing space\\t   " + LS + "mixed.both=  Both leading and trailing space\\r   \\t  \\n   ";
+        fos.write(mixed.getBytes());
+        fos.write(LS.getBytes());
+
+        String empty = "empty.line=                               ";
+        fos.write(empty.getBytes());
+
         fos.close();
 
         PropertiesComponent pc = new PropertiesComponent();
         pc.setCamelContext(context);
-        pc.setLocations(new String[]{"file:target/space/space.properties"});
+        pc.setLocation("file:target/space/space.properties");
         context.addComponent("properties", pc);
 
         return context;
@@ -57,6 +68,16 @@ public class PropertiesComponentLoadProp
         assertEquals("Leading space", context.resolvePropertyPlaceholders("{{cool.leading}}"));
         assertEquals("Trailing space", context.resolvePropertyPlaceholders("{{cool.trailing}}"));
         assertEquals("Both leading and trailing space", context.resolvePropertyPlaceholders("{{cool.both}}"));
+
+        assertEquals("\r\n", context.resolvePropertyPlaceholders("{{space.leading}}"));
+        assertEquals("\t", context.resolvePropertyPlaceholders("{{space.trailing}}"));
+        assertEquals("\r   \t  \n", context.resolvePropertyPlaceholders("{{space.both}}"));
+
+        assertEquals("Leading space\r\n", context.resolvePropertyPlaceholders("{{mixed.leading}}"));
+        assertEquals("Trailing space\t", context.resolvePropertyPlaceholders("{{mixed.trailing}}"));
+        assertEquals("Both leading and trailing space\r   \t  \n", context.resolvePropertyPlaceholders("{{mixed.both}}"));
+
+        assertEquals("", context.resolvePropertyPlaceholders("{{empty.line}}"));
     }
 
 }
\ No newline at end of file



Re: svn commit: r1407776 - in /camel/trunk/camel-core/src: main/java/org/apache/camel/component/properties/DefaultPropertiesResolver.java test/java/org/apache/camel/component/properties/PropertiesComponentLoadPropertiesFromFileTrimValuesTest.java

Posted by Babak Vahdat <ba...@swissonline.ch>.

Am 11.11.12 13:58 schrieb "Claus Ibsen" unter <cl...@gmail.com>:

>On Sun, Nov 11, 2012 at 12:29 PM, Babak Vahdat
><ba...@swissonline.ch> wrote:
>>
>>
>> Am 11.11.12 11:25 schrieb "Claus Ibsen" unter <cl...@gmail.com>:
>>
>>>On Sat, Nov 10, 2012 at 1:12 PM, Babak Vahdat
>>><ba...@swissonline.ch> wrote:
>>>> Hi
>>>>
>>>> There is no need to remove the leading whitespaces (that's the space
>>>> character ' ') as java.uti.Properties does already that for free:
>>>>
>>>>
>>>>http://docs.oracle.com/javase/6/docs/api/java/util/Properties.html#load
>>>>(j
>>>>av
>>>> a.io.Reader)
>>>>
>>>> So that we should only take care of the tailing spaces (see also the
>>>> comment inside the commit).
>>>>
>>>> As this's a special case for "trimming logic" I don't see any good
>>>>reason
>>>> why we should pop it up into a XXXHelper utility class because (as
>>>>already
>>>> said) we just touch the tail of the String but *not* it's head.
>>>>
>>>> Thoughts?
>>>>
>>>
>>>I haven't looked but Camel's property placeholder supports loading
>>>properties from different sources.
>>>For example people can load from a database etc.
>>
>> Yes this for sure, but as far as I see there's no such a out-of-box
>> PropertiesResolver so that ppl will have to implement their own logic
>>for
>> this and then plug it in into their PropertiesComponent. Then they will
>> decide by their own if they *want* to cut the tailing spaces or not.
>>Just
>> to be clear neither JDK nor Spring does this. And in the meanwhile, by
>>my
>> concrete use case I've switched to Bridging which of course made the
>> problem to completely disappear as Spring doesn't cut the tailing spaces
>> inside the property files (so no need anymore for me to inject my own
>> PropertiesResolver into the PropertiesComponent).
>>
>>>
>>>Just wonder if the same code logic is used in such use-cases. If not
>>>then IMHO we should support the leading/trailing trims.
>>
>> As already said there's NO need to trim the leading spaces as there will
>> be no such spaces at all, the *contract* of java.util.Properties is to
>> trim them all already before Camel comes into the play (see the
>>Javadoc).
>>
>
>
>
>>>
>>>And you may at least consider moving the logic to a private method in
>>>the class so the code is nicely separated.
>>
>> We've got already that separation given through the strategy method
>> prepareLoadedProperties() being introduced by CAMEL-3565. So don't see
>>why
>> we should still define another level of the separation inside this
>> separation/strategy by prepareLoadedProperties() method, as trimming is
>> *exactly* what *this* level of separation is supposed to do. This would
>>be
>> an over-engineering for no good gain. So IMHO "lets keep things as
>>simple
>> as possible but not simpler".
>>
>
>Well adding 12 lines of low level String manipulation code inside an
>existing for loop (the 1st loop), makes it harder
>to read the code.
>
>Instead refactorting that code to a private method, makes it easier to
>read the overall intend, of the for looping
>of the properties (eg the 1st for loop).

Indeed this last reasoning of yours convinced me to "do refactor to a
separate method". And the corresponding tests/asserts were already added
by my original first commit of this ticket so that nothing changed there.

Babak

>
>And there is often bugs in String manipulation code, as its hard to
>get the indexing right, and whatnot.
>So having an util method with unit test helps ensure this properly, to
>have it test throughly.
>
>Code like this is easer to read (= only 1 for loop, and logic at higher
>level)
>
>    /**
>     * Strategy to prepare loaded properties before being used by Camel.
>     * <p/>
>     * This implementation will ensure values are trimmed, as loading
>properties from
>     * a file with values having trailing spaces is not automatic
>trimmed by the Properties API
>     * from the JDK.
>     *
>     * @param properties  the properties
>     * @return the prepared properties
>     */
>    protected Properties prepareLoadedProperties(Properties properties) {
>        Properties answer = new Properties();
>        for (Map.Entry<Object, Object> entry : properties.entrySet()) {
>            Object key = entry.getKey();
>            Object value = entry.getValue();
>            if (value instanceof String) {
>                String s = (String) value;
>                value = trimTrailingWhitespaces(s);
>            }
>            answer.put(key, value);
>        }
>        return answer;
>    }
>
>
>
>
>> Babak
>>
>>>If it should not be moved to a StringHelper in the util package.
>>>
>>>> Babak
>>>>
>>>> Am 10.11.12 12:59 schrieb "Claus Ibsen" unter <cl...@gmail.com>:
>>>>
>>>>>Hi
>>>>>
>>>>>I suggest to add an util method to StringHelper (or what we call it).
>>>>>And then add unit test for the trim method.
>>>>>
>>>>>Then it's easier to verify, and also re-use this method in other
>>>>>places.
>>>>>And mind that it should also remove leading whitespaces.
>>>>>
>>>>>
>>>>>
>>>>>On Sat, Nov 10, 2012 at 12:48 PM,  <bv...@apache.org> wrote:
>>>>>> Author: bvahdat
>>>>>> Date: Sat Nov 10 11:48:43 2012
>>>>>> New Revision: 1407776
>>>>>>
>>>>>> URL: http://svn.apache.org/viewvc?rev=1407776&view=rev
>>>>>> Log:
>>>>>> CAMEL-5784: Preparing the loaded properties should only trim any
>>>>>>potential whitespace characters.
>>>>>>
>>>>>> Modified:
>>>>>>
>>>>>>camel/trunk/camel-core/src/main/java/org/apache/camel/component/prope
>>>>>>rt
>>>>>>ie
>>>>>>s/DefaultPropertiesResolver.java
>>>>>>
>>>>>>camel/trunk/camel-core/src/test/java/org/apache/camel/component/prope
>>>>>>rt
>>>>>>ie
>>>>>>s/PropertiesComponentLoadPropertiesFromFileTrimValuesTest.java
>>>>>>
>>>>>> Modified:
>>>>>>camel/trunk/camel-core/src/main/java/org/apache/camel/component/prope
>>>>>>rt
>>>>>>ie
>>>>>>s/DefaultPropertiesResolver.java
>>>>>> URL:
>>>>>>http://svn.apache.org/viewvc/camel/trunk/camel-core/src/main/java/org
>>>>>>/a
>>>>>>pa
>>>>>>che/camel/component/properties/DefaultPropertiesResolver.java?rev=140
>>>>>>77
>>>>>>76
>>>>>>&r1=1407775&r2=1407776&view=diff
>>>>>>
>>>>>>=====================================================================
>>>>>>==
>>>>>>==
>>>>>>=====
>>>>>> ---
>>>>>>camel/trunk/camel-core/src/main/java/org/apache/camel/component/prope
>>>>>>rt
>>>>>>ie
>>>>>>s/DefaultPropertiesResolver.java (original)
>>>>>> +++
>>>>>>camel/trunk/camel-core/src/main/java/org/apache/camel/component/prope
>>>>>>rt
>>>>>>ie
>>>>>>s/DefaultPropertiesResolver.java Sat Nov 10 11:48:43 2012
>>>>>> @@ -131,11 +131,20 @@ public class DefaultPropertiesResolver i
>>>>>>          for (Map.Entry<Object, Object> entry :
>>>>>>properties.entrySet()) {
>>>>>>              Object key = entry.getKey();
>>>>>>              Object value = entry.getValue();
>>>>>> -            // trim string values which can be a problem when
>>>>>>loading
>>>>>>from a properties file and there
>>>>>> -            // is leading or trailing spaces in the value
>>>>>> +            // trim any trailing spaces which can be a problem when
>>>>>>loading from
>>>>>> +            // a properties file, note that java.util.Properties
>>>>>>does
>>>>>>already this
>>>>>> +            // for any potential leading spaces so there's nothing
>>>>>>to
>>>>>>do there
>>>>>>              if (value instanceof String) {
>>>>>>                  String s = (String) value;
>>>>>> -                s = s.trim();
>>>>>> +                int endIndex = s.length();
>>>>>> +                for (int index = s.length() - 1; index >= 0;
>>>>>>index--) {
>>>>>> +                    if (s.charAt(index) == ' ') {
>>>>>> +                        endIndex = index;
>>>>>> +                    } else {
>>>>>> +                        break;
>>>>>> +                    }
>>>>>> +                }
>>>>>> +                s = s.substring(0, endIndex);
>>>>>>                  value = s;
>>>>>>              }
>>>>>>              answer.put(key, value);
>>>>>>
>>>>>> Modified:
>>>>>>camel/trunk/camel-core/src/test/java/org/apache/camel/component/prope
>>>>>>rt
>>>>>>ie
>>>>>>s/PropertiesComponentLoadPropertiesFromFileTrimValuesTest.java
>>>>>> URL:
>>>>>>http://svn.apache.org/viewvc/camel/trunk/camel-core/src/test/java/org
>>>>>>/a
>>>>>>pa
>>>>>>che/camel/component/properties/PropertiesComponentLoadPropertiesFromF
>>>>>>il
>>>>>>eT
>>>>>>rimValuesTest.java?rev=1407776&r1=1407775&r2=1407776&view=diff
>>>>>>
>>>>>>=====================================================================
>>>>>>==
>>>>>>==
>>>>>>=====
>>>>>> ---
>>>>>>camel/trunk/camel-core/src/test/java/org/apache/camel/component/prope
>>>>>>rt
>>>>>>ie
>>>>>>s/PropertiesComponentLoadPropertiesFromFileTrimValuesTest.java
>>>>>>(original)
>>>>>> +++
>>>>>>camel/trunk/camel-core/src/test/java/org/apache/camel/component/prope
>>>>>>rt
>>>>>>ie
>>>>>>s/PropertiesComponentLoadPropertiesFromFileTrimValuesTest.java Sat
>>>>>>Nov
>>>>>>10 11:48:43 2012
>>>>>> @@ -16,7 +16,6 @@
>>>>>>   */
>>>>>>  package org.apache.camel.component.properties;
>>>>>>
>>>>>> -import java.io.File;
>>>>>>  import java.io.FileOutputStream;
>>>>>>
>>>>>>  import org.apache.camel.CamelContext;
>>>>>> @@ -39,15 +38,27 @@ public class PropertiesComponentLoadProp
>>>>>>          CamelContext context = super.createCamelContext();
>>>>>>
>>>>>>          // create space.properties file
>>>>>> -        File file = new File("target/space/space.properties");
>>>>>> -        file.createNewFile();
>>>>>> -        FileOutputStream fos = new FileOutputStream(file);
>>>>>> -        fos.write("cool.leading= Leading
>>>>>>space\ncool.trailing=Trailing
>>>>>>space \ncool.both= Both leading and trailing space ".getBytes());
>>>>>> +        FileOutputStream fos = new
>>>>>>FileOutputStream("target/space/space.properties");
>>>>>> +        String cool = "cool.leading= Leading space" + LS +
>>>>>>"cool.trailing=Trailing space " + LS + "cool.both= Both leading and
>>>>>>trailing space ";
>>>>>> +        fos.write(cool.getBytes());
>>>>>> +        fos.write(LS.getBytes());
>>>>>> +
>>>>>> +        String space = "space.leading=   \\r\\n" + LS +
>>>>>>"space.trailing=\\t   " + LS + "space.both=  \\r   \\t  \\n   ";
>>>>>> +        fos.write(space.getBytes());
>>>>>> +        fos.write(LS.getBytes());
>>>>>> +
>>>>>> +        String mixed = "mixed.leading=   Leading space\\r\\n" + LS
>>>>>>+
>>>>>>"mixed.trailing=Trailing space\\t   " + LS + "mixed.both=  Both
>>>>>>leading
>>>>>>and trailing space\\r   \\t  \\n   ";
>>>>>> +        fos.write(mixed.getBytes());
>>>>>> +        fos.write(LS.getBytes());
>>>>>> +
>>>>>> +        String empty = "empty.line=
>>>>>>";
>>>>>> +        fos.write(empty.getBytes());
>>>>>> +
>>>>>>          fos.close();
>>>>>>
>>>>>>          PropertiesComponent pc = new PropertiesComponent();
>>>>>>          pc.setCamelContext(context);
>>>>>> -        pc.setLocations(new
>>>>>>String[]{"file:target/space/space.properties"});
>>>>>> +        pc.setLocation("file:target/space/space.properties");
>>>>>>          context.addComponent("properties", pc);
>>>>>>
>>>>>>          return context;
>>>>>> @@ -57,6 +68,16 @@ public class PropertiesComponentLoadProp
>>>>>>          assertEquals("Leading space",
>>>>>>context.resolvePropertyPlaceholders("{{cool.leading}}"));
>>>>>>          assertEquals("Trailing space",
>>>>>>context.resolvePropertyPlaceholders("{{cool.trailing}}"));
>>>>>>          assertEquals("Both leading and trailing space",
>>>>>>context.resolvePropertyPlaceholders("{{cool.both}}"));
>>>>>> +
>>>>>> +        assertEquals("\r\n",
>>>>>>context.resolvePropertyPlaceholders("{{space.leading}}"));
>>>>>> +        assertEquals("\t",
>>>>>>context.resolvePropertyPlaceholders("{{space.trailing}}"));
>>>>>> +        assertEquals("\r   \t  \n",
>>>>>>context.resolvePropertyPlaceholders("{{space.both}}"));
>>>>>> +
>>>>>> +        assertEquals("Leading space\r\n",
>>>>>>context.resolvePropertyPlaceholders("{{mixed.leading}}"));
>>>>>> +        assertEquals("Trailing space\t",
>>>>>>context.resolvePropertyPlaceholders("{{mixed.trailing}}"));
>>>>>> +        assertEquals("Both leading and trailing space\r   \t  \n",
>>>>>>context.resolvePropertyPlaceholders("{{mixed.both}}"));
>>>>>> +
>>>>>> +        assertEquals("",
>>>>>>context.resolvePropertyPlaceholders("{{empty.line}}"));
>>>>>>      }
>>>>>>
>>>>>>  }
>>>>>> \ No newline at end of file
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>>--
>>>>>Claus Ibsen
>>>>>-----------------
>>>>>Red Hat, Inc.
>>>>>FuseSource is now part of Red Hat
>>>>>Email: cibsen@redhat.com
>>>>>Web: http://fusesource.com
>>>>>Twitter: davsclaus
>>>>>Blog: http://davsclaus.com
>>>>>Author of Camel in Action: http://www.manning.com/ibsen
>>>>
>>>>
>>>
>>>
>>>
>>>--
>>>Claus Ibsen
>>>-----------------
>>>Red Hat, Inc.
>>>FuseSource is now part of Red Hat
>>>Email: cibsen@redhat.com
>>>Web: http://fusesource.com
>>>Twitter: davsclaus
>>>Blog: http://davsclaus.com
>>>Author of Camel in Action: http://www.manning.com/ibsen
>>
>>
>
>
>
>-- 
>Claus Ibsen
>-----------------
>Red Hat, Inc.
>FuseSource is now part of Red Hat
>Email: cibsen@redhat.com
>Web: http://fusesource.com
>Twitter: davsclaus
>Blog: http://davsclaus.com
>Author of Camel in Action: http://www.manning.com/ibsen



Re: svn commit: r1407776 - in /camel/trunk/camel-core/src: main/java/org/apache/camel/component/properties/DefaultPropertiesResolver.java test/java/org/apache/camel/component/properties/PropertiesComponentLoadPropertiesFromFileTrimValuesTest.java

Posted by Claus Ibsen <cl...@gmail.com>.
On Sun, Nov 11, 2012 at 12:29 PM, Babak Vahdat
<ba...@swissonline.ch> wrote:
>
>
> Am 11.11.12 11:25 schrieb "Claus Ibsen" unter <cl...@gmail.com>:
>
>>On Sat, Nov 10, 2012 at 1:12 PM, Babak Vahdat
>><ba...@swissonline.ch> wrote:
>>> Hi
>>>
>>> There is no need to remove the leading whitespaces (that's the space
>>> character ' ') as java.uti.Properties does already that for free:
>>>
>>>
>>>http://docs.oracle.com/javase/6/docs/api/java/util/Properties.html#load(j
>>>av
>>> a.io.Reader)
>>>
>>> So that we should only take care of the tailing spaces (see also the
>>> comment inside the commit).
>>>
>>> As this's a special case for "trimming logic" I don't see any good
>>>reason
>>> why we should pop it up into a XXXHelper utility class because (as
>>>already
>>> said) we just touch the tail of the String but *not* it's head.
>>>
>>> Thoughts?
>>>
>>
>>I haven't looked but Camel's property placeholder supports loading
>>properties from different sources.
>>For example people can load from a database etc.
>
> Yes this for sure, but as far as I see there's no such a out-of-box
> PropertiesResolver so that ppl will have to implement their own logic for
> this and then plug it in into their PropertiesComponent. Then they will
> decide by their own if they *want* to cut the tailing spaces or not. Just
> to be clear neither JDK nor Spring does this. And in the meanwhile, by my
> concrete use case I've switched to Bridging which of course made the
> problem to completely disappear as Spring doesn't cut the tailing spaces
> inside the property files (so no need anymore for me to inject my own
> PropertiesResolver into the PropertiesComponent).
>
>>
>>Just wonder if the same code logic is used in such use-cases. If not
>>then IMHO we should support the leading/trailing trims.
>
> As already said there's NO need to trim the leading spaces as there will
> be no such spaces at all, the *contract* of java.util.Properties is to
> trim them all already before Camel comes into the play (see the Javadoc).
>



>>
>>And you may at least consider moving the logic to a private method in
>>the class so the code is nicely separated.
>
> We've got already that separation given through the strategy method
> prepareLoadedProperties() being introduced by CAMEL-3565. So don't see why
> we should still define another level of the separation inside this
> separation/strategy by prepareLoadedProperties() method, as trimming is
> *exactly* what *this* level of separation is supposed to do. This would be
> an over-engineering for no good gain. So IMHO "lets keep things as simple
> as possible but not simpler".
>

Well adding 12 lines of low level String manipulation code inside an
existing for loop (the 1st loop), makes it harder
to read the code.

Instead refactorting that code to a private method, makes it easier to
read the overall intend, of the for looping
of the properties (eg the 1st for loop).

And there is often bugs in String manipulation code, as its hard to
get the indexing right, and whatnot.
So having an util method with unit test helps ensure this properly, to
have it test throughly.

Code like this is easer to read (= only 1 for loop, and logic at higher level)

    /**
     * Strategy to prepare loaded properties before being used by Camel.
     * <p/>
     * This implementation will ensure values are trimmed, as loading
properties from
     * a file with values having trailing spaces is not automatic
trimmed by the Properties API
     * from the JDK.
     *
     * @param properties  the properties
     * @return the prepared properties
     */
    protected Properties prepareLoadedProperties(Properties properties) {
        Properties answer = new Properties();
        for (Map.Entry<Object, Object> entry : properties.entrySet()) {
            Object key = entry.getKey();
            Object value = entry.getValue();
            if (value instanceof String) {
                String s = (String) value;
                value = trimTrailingWhitespaces(s);
            }
            answer.put(key, value);
        }
        return answer;
    }




> Babak
>
>>If it should not be moved to a StringHelper in the util package.
>>
>>> Babak
>>>
>>> Am 10.11.12 12:59 schrieb "Claus Ibsen" unter <cl...@gmail.com>:
>>>
>>>>Hi
>>>>
>>>>I suggest to add an util method to StringHelper (or what we call it).
>>>>And then add unit test for the trim method.
>>>>
>>>>Then it's easier to verify, and also re-use this method in other places.
>>>>And mind that it should also remove leading whitespaces.
>>>>
>>>>
>>>>
>>>>On Sat, Nov 10, 2012 at 12:48 PM,  <bv...@apache.org> wrote:
>>>>> Author: bvahdat
>>>>> Date: Sat Nov 10 11:48:43 2012
>>>>> New Revision: 1407776
>>>>>
>>>>> URL: http://svn.apache.org/viewvc?rev=1407776&view=rev
>>>>> Log:
>>>>> CAMEL-5784: Preparing the loaded properties should only trim any
>>>>>potential whitespace characters.
>>>>>
>>>>> Modified:
>>>>>
>>>>>camel/trunk/camel-core/src/main/java/org/apache/camel/component/propert
>>>>>ie
>>>>>s/DefaultPropertiesResolver.java
>>>>>
>>>>>camel/trunk/camel-core/src/test/java/org/apache/camel/component/propert
>>>>>ie
>>>>>s/PropertiesComponentLoadPropertiesFromFileTrimValuesTest.java
>>>>>
>>>>> Modified:
>>>>>camel/trunk/camel-core/src/main/java/org/apache/camel/component/propert
>>>>>ie
>>>>>s/DefaultPropertiesResolver.java
>>>>> URL:
>>>>>http://svn.apache.org/viewvc/camel/trunk/camel-core/src/main/java/org/a
>>>>>pa
>>>>>che/camel/component/properties/DefaultPropertiesResolver.java?rev=14077
>>>>>76
>>>>>&r1=1407775&r2=1407776&view=diff
>>>>>
>>>>>=======================================================================
>>>>>==
>>>>>=====
>>>>> ---
>>>>>camel/trunk/camel-core/src/main/java/org/apache/camel/component/propert
>>>>>ie
>>>>>s/DefaultPropertiesResolver.java (original)
>>>>> +++
>>>>>camel/trunk/camel-core/src/main/java/org/apache/camel/component/propert
>>>>>ie
>>>>>s/DefaultPropertiesResolver.java Sat Nov 10 11:48:43 2012
>>>>> @@ -131,11 +131,20 @@ public class DefaultPropertiesResolver i
>>>>>          for (Map.Entry<Object, Object> entry :
>>>>>properties.entrySet()) {
>>>>>              Object key = entry.getKey();
>>>>>              Object value = entry.getValue();
>>>>> -            // trim string values which can be a problem when loading
>>>>>from a properties file and there
>>>>> -            // is leading or trailing spaces in the value
>>>>> +            // trim any trailing spaces which can be a problem when
>>>>>loading from
>>>>> +            // a properties file, note that java.util.Properties does
>>>>>already this
>>>>> +            // for any potential leading spaces so there's nothing to
>>>>>do there
>>>>>              if (value instanceof String) {
>>>>>                  String s = (String) value;
>>>>> -                s = s.trim();
>>>>> +                int endIndex = s.length();
>>>>> +                for (int index = s.length() - 1; index >= 0;
>>>>>index--) {
>>>>> +                    if (s.charAt(index) == ' ') {
>>>>> +                        endIndex = index;
>>>>> +                    } else {
>>>>> +                        break;
>>>>> +                    }
>>>>> +                }
>>>>> +                s = s.substring(0, endIndex);
>>>>>                  value = s;
>>>>>              }
>>>>>              answer.put(key, value);
>>>>>
>>>>> Modified:
>>>>>camel/trunk/camel-core/src/test/java/org/apache/camel/component/propert
>>>>>ie
>>>>>s/PropertiesComponentLoadPropertiesFromFileTrimValuesTest.java
>>>>> URL:
>>>>>http://svn.apache.org/viewvc/camel/trunk/camel-core/src/test/java/org/a
>>>>>pa
>>>>>che/camel/component/properties/PropertiesComponentLoadPropertiesFromFil
>>>>>eT
>>>>>rimValuesTest.java?rev=1407776&r1=1407775&r2=1407776&view=diff
>>>>>
>>>>>=======================================================================
>>>>>==
>>>>>=====
>>>>> ---
>>>>>camel/trunk/camel-core/src/test/java/org/apache/camel/component/propert
>>>>>ie
>>>>>s/PropertiesComponentLoadPropertiesFromFileTrimValuesTest.java
>>>>>(original)
>>>>> +++
>>>>>camel/trunk/camel-core/src/test/java/org/apache/camel/component/propert
>>>>>ie
>>>>>s/PropertiesComponentLoadPropertiesFromFileTrimValuesTest.java Sat Nov
>>>>>10 11:48:43 2012
>>>>> @@ -16,7 +16,6 @@
>>>>>   */
>>>>>  package org.apache.camel.component.properties;
>>>>>
>>>>> -import java.io.File;
>>>>>  import java.io.FileOutputStream;
>>>>>
>>>>>  import org.apache.camel.CamelContext;
>>>>> @@ -39,15 +38,27 @@ public class PropertiesComponentLoadProp
>>>>>          CamelContext context = super.createCamelContext();
>>>>>
>>>>>          // create space.properties file
>>>>> -        File file = new File("target/space/space.properties");
>>>>> -        file.createNewFile();
>>>>> -        FileOutputStream fos = new FileOutputStream(file);
>>>>> -        fos.write("cool.leading= Leading
>>>>>space\ncool.trailing=Trailing
>>>>>space \ncool.both= Both leading and trailing space ".getBytes());
>>>>> +        FileOutputStream fos = new
>>>>>FileOutputStream("target/space/space.properties");
>>>>> +        String cool = "cool.leading= Leading space" + LS +
>>>>>"cool.trailing=Trailing space " + LS + "cool.both= Both leading and
>>>>>trailing space ";
>>>>> +        fos.write(cool.getBytes());
>>>>> +        fos.write(LS.getBytes());
>>>>> +
>>>>> +        String space = "space.leading=   \\r\\n" + LS +
>>>>>"space.trailing=\\t   " + LS + "space.both=  \\r   \\t  \\n   ";
>>>>> +        fos.write(space.getBytes());
>>>>> +        fos.write(LS.getBytes());
>>>>> +
>>>>> +        String mixed = "mixed.leading=   Leading space\\r\\n" + LS +
>>>>>"mixed.trailing=Trailing space\\t   " + LS + "mixed.both=  Both leading
>>>>>and trailing space\\r   \\t  \\n   ";
>>>>> +        fos.write(mixed.getBytes());
>>>>> +        fos.write(LS.getBytes());
>>>>> +
>>>>> +        String empty = "empty.line=                               ";
>>>>> +        fos.write(empty.getBytes());
>>>>> +
>>>>>          fos.close();
>>>>>
>>>>>          PropertiesComponent pc = new PropertiesComponent();
>>>>>          pc.setCamelContext(context);
>>>>> -        pc.setLocations(new
>>>>>String[]{"file:target/space/space.properties"});
>>>>> +        pc.setLocation("file:target/space/space.properties");
>>>>>          context.addComponent("properties", pc);
>>>>>
>>>>>          return context;
>>>>> @@ -57,6 +68,16 @@ public class PropertiesComponentLoadProp
>>>>>          assertEquals("Leading space",
>>>>>context.resolvePropertyPlaceholders("{{cool.leading}}"));
>>>>>          assertEquals("Trailing space",
>>>>>context.resolvePropertyPlaceholders("{{cool.trailing}}"));
>>>>>          assertEquals("Both leading and trailing space",
>>>>>context.resolvePropertyPlaceholders("{{cool.both}}"));
>>>>> +
>>>>> +        assertEquals("\r\n",
>>>>>context.resolvePropertyPlaceholders("{{space.leading}}"));
>>>>> +        assertEquals("\t",
>>>>>context.resolvePropertyPlaceholders("{{space.trailing}}"));
>>>>> +        assertEquals("\r   \t  \n",
>>>>>context.resolvePropertyPlaceholders("{{space.both}}"));
>>>>> +
>>>>> +        assertEquals("Leading space\r\n",
>>>>>context.resolvePropertyPlaceholders("{{mixed.leading}}"));
>>>>> +        assertEquals("Trailing space\t",
>>>>>context.resolvePropertyPlaceholders("{{mixed.trailing}}"));
>>>>> +        assertEquals("Both leading and trailing space\r   \t  \n",
>>>>>context.resolvePropertyPlaceholders("{{mixed.both}}"));
>>>>> +
>>>>> +        assertEquals("",
>>>>>context.resolvePropertyPlaceholders("{{empty.line}}"));
>>>>>      }
>>>>>
>>>>>  }
>>>>> \ No newline at end of file
>>>>>
>>>>>
>>>>
>>>>
>>>>
>>>>--
>>>>Claus Ibsen
>>>>-----------------
>>>>Red Hat, Inc.
>>>>FuseSource is now part of Red Hat
>>>>Email: cibsen@redhat.com
>>>>Web: http://fusesource.com
>>>>Twitter: davsclaus
>>>>Blog: http://davsclaus.com
>>>>Author of Camel in Action: http://www.manning.com/ibsen
>>>
>>>
>>
>>
>>
>>--
>>Claus Ibsen
>>-----------------
>>Red Hat, Inc.
>>FuseSource is now part of Red Hat
>>Email: cibsen@redhat.com
>>Web: http://fusesource.com
>>Twitter: davsclaus
>>Blog: http://davsclaus.com
>>Author of Camel in Action: http://www.manning.com/ibsen
>
>



-- 
Claus Ibsen
-----------------
Red Hat, Inc.
FuseSource is now part of Red Hat
Email: cibsen@redhat.com
Web: http://fusesource.com
Twitter: davsclaus
Blog: http://davsclaus.com
Author of Camel in Action: http://www.manning.com/ibsen

Re: svn commit: r1407776 - in /camel/trunk/camel-core/src: main/java/org/apache/camel/component/properties/DefaultPropertiesResolver.java test/java/org/apache/camel/component/properties/PropertiesComponentLoadPropertiesFromFileTrimValuesTest.java

Posted by Babak Vahdat <ba...@swissonline.ch>.

Am 11.11.12 11:25 schrieb "Claus Ibsen" unter <cl...@gmail.com>:

>On Sat, Nov 10, 2012 at 1:12 PM, Babak Vahdat
><ba...@swissonline.ch> wrote:
>> Hi
>>
>> There is no need to remove the leading whitespaces (that's the space
>> character ' ') as java.uti.Properties does already that for free:
>>
>> 
>>http://docs.oracle.com/javase/6/docs/api/java/util/Properties.html#load(j
>>av
>> a.io.Reader)
>>
>> So that we should only take care of the tailing spaces (see also the
>> comment inside the commit).
>>
>> As this's a special case for "trimming logic" I don't see any good
>>reason
>> why we should pop it up into a XXXHelper utility class because (as
>>already
>> said) we just touch the tail of the String but *not* it's head.
>>
>> Thoughts?
>>
>
>I haven't looked but Camel's property placeholder supports loading
>properties from different sources.
>For example people can load from a database etc.

Yes this for sure, but as far as I see there's no such a out-of-box
PropertiesResolver so that ppl will have to implement their own logic for
this and then plug it in into their PropertiesComponent. Then they will
decide by their own if they *want* to cut the tailing spaces or not. Just
to be clear neither JDK nor Spring does this. And in the meanwhile, by my
concrete use case I've switched to Bridging which of course made the
problem to completely disappear as Spring doesn't cut the tailing spaces
inside the property files (so no need anymore for me to inject my own
PropertiesResolver into the PropertiesComponent).

>
>Just wonder if the same code logic is used in such use-cases. If not
>then IMHO we should support the leading/trailing trims.

As already said there's NO need to trim the leading spaces as there will
be no such spaces at all, the *contract* of java.util.Properties is to
trim them all already before Camel comes into the play (see the Javadoc).

>
>And you may at least consider moving the logic to a private method in
>the class so the code is nicely separated.

We've got already that separation given through the strategy method
prepareLoadedProperties() being introduced by CAMEL-3565. So don't see why
we should still define another level of the separation inside this
separation/strategy by prepareLoadedProperties() method, as trimming is
*exactly* what *this* level of separation is supposed to do. This would be
an over-engineering for no good gain. So IMHO "lets keep things as simple
as possible but not simpler".

Babak

>If it should not be moved to a StringHelper in the util package.
>
>> Babak
>>
>> Am 10.11.12 12:59 schrieb "Claus Ibsen" unter <cl...@gmail.com>:
>>
>>>Hi
>>>
>>>I suggest to add an util method to StringHelper (or what we call it).
>>>And then add unit test for the trim method.
>>>
>>>Then it's easier to verify, and also re-use this method in other places.
>>>And mind that it should also remove leading whitespaces.
>>>
>>>
>>>
>>>On Sat, Nov 10, 2012 at 12:48 PM,  <bv...@apache.org> wrote:
>>>> Author: bvahdat
>>>> Date: Sat Nov 10 11:48:43 2012
>>>> New Revision: 1407776
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=1407776&view=rev
>>>> Log:
>>>> CAMEL-5784: Preparing the loaded properties should only trim any
>>>>potential whitespace characters.
>>>>
>>>> Modified:
>>>>
>>>>camel/trunk/camel-core/src/main/java/org/apache/camel/component/propert
>>>>ie
>>>>s/DefaultPropertiesResolver.java
>>>>
>>>>camel/trunk/camel-core/src/test/java/org/apache/camel/component/propert
>>>>ie
>>>>s/PropertiesComponentLoadPropertiesFromFileTrimValuesTest.java
>>>>
>>>> Modified:
>>>>camel/trunk/camel-core/src/main/java/org/apache/camel/component/propert
>>>>ie
>>>>s/DefaultPropertiesResolver.java
>>>> URL:
>>>>http://svn.apache.org/viewvc/camel/trunk/camel-core/src/main/java/org/a
>>>>pa
>>>>che/camel/component/properties/DefaultPropertiesResolver.java?rev=14077
>>>>76
>>>>&r1=1407775&r2=1407776&view=diff
>>>>
>>>>=======================================================================
>>>>==
>>>>=====
>>>> ---
>>>>camel/trunk/camel-core/src/main/java/org/apache/camel/component/propert
>>>>ie
>>>>s/DefaultPropertiesResolver.java (original)
>>>> +++
>>>>camel/trunk/camel-core/src/main/java/org/apache/camel/component/propert
>>>>ie
>>>>s/DefaultPropertiesResolver.java Sat Nov 10 11:48:43 2012
>>>> @@ -131,11 +131,20 @@ public class DefaultPropertiesResolver i
>>>>          for (Map.Entry<Object, Object> entry :
>>>>properties.entrySet()) {
>>>>              Object key = entry.getKey();
>>>>              Object value = entry.getValue();
>>>> -            // trim string values which can be a problem when loading
>>>>from a properties file and there
>>>> -            // is leading or trailing spaces in the value
>>>> +            // trim any trailing spaces which can be a problem when
>>>>loading from
>>>> +            // a properties file, note that java.util.Properties does
>>>>already this
>>>> +            // for any potential leading spaces so there's nothing to
>>>>do there
>>>>              if (value instanceof String) {
>>>>                  String s = (String) value;
>>>> -                s = s.trim();
>>>> +                int endIndex = s.length();
>>>> +                for (int index = s.length() - 1; index >= 0;
>>>>index--) {
>>>> +                    if (s.charAt(index) == ' ') {
>>>> +                        endIndex = index;
>>>> +                    } else {
>>>> +                        break;
>>>> +                    }
>>>> +                }
>>>> +                s = s.substring(0, endIndex);
>>>>                  value = s;
>>>>              }
>>>>              answer.put(key, value);
>>>>
>>>> Modified:
>>>>camel/trunk/camel-core/src/test/java/org/apache/camel/component/propert
>>>>ie
>>>>s/PropertiesComponentLoadPropertiesFromFileTrimValuesTest.java
>>>> URL:
>>>>http://svn.apache.org/viewvc/camel/trunk/camel-core/src/test/java/org/a
>>>>pa
>>>>che/camel/component/properties/PropertiesComponentLoadPropertiesFromFil
>>>>eT
>>>>rimValuesTest.java?rev=1407776&r1=1407775&r2=1407776&view=diff
>>>>
>>>>=======================================================================
>>>>==
>>>>=====
>>>> ---
>>>>camel/trunk/camel-core/src/test/java/org/apache/camel/component/propert
>>>>ie
>>>>s/PropertiesComponentLoadPropertiesFromFileTrimValuesTest.java
>>>>(original)
>>>> +++
>>>>camel/trunk/camel-core/src/test/java/org/apache/camel/component/propert
>>>>ie
>>>>s/PropertiesComponentLoadPropertiesFromFileTrimValuesTest.java Sat Nov
>>>>10 11:48:43 2012
>>>> @@ -16,7 +16,6 @@
>>>>   */
>>>>  package org.apache.camel.component.properties;
>>>>
>>>> -import java.io.File;
>>>>  import java.io.FileOutputStream;
>>>>
>>>>  import org.apache.camel.CamelContext;
>>>> @@ -39,15 +38,27 @@ public class PropertiesComponentLoadProp
>>>>          CamelContext context = super.createCamelContext();
>>>>
>>>>          // create space.properties file
>>>> -        File file = new File("target/space/space.properties");
>>>> -        file.createNewFile();
>>>> -        FileOutputStream fos = new FileOutputStream(file);
>>>> -        fos.write("cool.leading= Leading
>>>>space\ncool.trailing=Trailing
>>>>space \ncool.both= Both leading and trailing space ".getBytes());
>>>> +        FileOutputStream fos = new
>>>>FileOutputStream("target/space/space.properties");
>>>> +        String cool = "cool.leading= Leading space" + LS +
>>>>"cool.trailing=Trailing space " + LS + "cool.both= Both leading and
>>>>trailing space ";
>>>> +        fos.write(cool.getBytes());
>>>> +        fos.write(LS.getBytes());
>>>> +
>>>> +        String space = "space.leading=   \\r\\n" + LS +
>>>>"space.trailing=\\t   " + LS + "space.both=  \\r   \\t  \\n   ";
>>>> +        fos.write(space.getBytes());
>>>> +        fos.write(LS.getBytes());
>>>> +
>>>> +        String mixed = "mixed.leading=   Leading space\\r\\n" + LS +
>>>>"mixed.trailing=Trailing space\\t   " + LS + "mixed.both=  Both leading
>>>>and trailing space\\r   \\t  \\n   ";
>>>> +        fos.write(mixed.getBytes());
>>>> +        fos.write(LS.getBytes());
>>>> +
>>>> +        String empty = "empty.line=                               ";
>>>> +        fos.write(empty.getBytes());
>>>> +
>>>>          fos.close();
>>>>
>>>>          PropertiesComponent pc = new PropertiesComponent();
>>>>          pc.setCamelContext(context);
>>>> -        pc.setLocations(new
>>>>String[]{"file:target/space/space.properties"});
>>>> +        pc.setLocation("file:target/space/space.properties");
>>>>          context.addComponent("properties", pc);
>>>>
>>>>          return context;
>>>> @@ -57,6 +68,16 @@ public class PropertiesComponentLoadProp
>>>>          assertEquals("Leading space",
>>>>context.resolvePropertyPlaceholders("{{cool.leading}}"));
>>>>          assertEquals("Trailing space",
>>>>context.resolvePropertyPlaceholders("{{cool.trailing}}"));
>>>>          assertEquals("Both leading and trailing space",
>>>>context.resolvePropertyPlaceholders("{{cool.both}}"));
>>>> +
>>>> +        assertEquals("\r\n",
>>>>context.resolvePropertyPlaceholders("{{space.leading}}"));
>>>> +        assertEquals("\t",
>>>>context.resolvePropertyPlaceholders("{{space.trailing}}"));
>>>> +        assertEquals("\r   \t  \n",
>>>>context.resolvePropertyPlaceholders("{{space.both}}"));
>>>> +
>>>> +        assertEquals("Leading space\r\n",
>>>>context.resolvePropertyPlaceholders("{{mixed.leading}}"));
>>>> +        assertEquals("Trailing space\t",
>>>>context.resolvePropertyPlaceholders("{{mixed.trailing}}"));
>>>> +        assertEquals("Both leading and trailing space\r   \t  \n",
>>>>context.resolvePropertyPlaceholders("{{mixed.both}}"));
>>>> +
>>>> +        assertEquals("",
>>>>context.resolvePropertyPlaceholders("{{empty.line}}"));
>>>>      }
>>>>
>>>>  }
>>>> \ No newline at end of file
>>>>
>>>>
>>>
>>>
>>>
>>>--
>>>Claus Ibsen
>>>-----------------
>>>Red Hat, Inc.
>>>FuseSource is now part of Red Hat
>>>Email: cibsen@redhat.com
>>>Web: http://fusesource.com
>>>Twitter: davsclaus
>>>Blog: http://davsclaus.com
>>>Author of Camel in Action: http://www.manning.com/ibsen
>>
>>
>
>
>
>-- 
>Claus Ibsen
>-----------------
>Red Hat, Inc.
>FuseSource is now part of Red Hat
>Email: cibsen@redhat.com
>Web: http://fusesource.com
>Twitter: davsclaus
>Blog: http://davsclaus.com
>Author of Camel in Action: http://www.manning.com/ibsen



Re: svn commit: r1407776 - in /camel/trunk/camel-core/src: main/java/org/apache/camel/component/properties/DefaultPropertiesResolver.java test/java/org/apache/camel/component/properties/PropertiesComponentLoadPropertiesFromFileTrimValuesTest.java

Posted by Claus Ibsen <cl...@gmail.com>.
On Sat, Nov 10, 2012 at 1:12 PM, Babak Vahdat
<ba...@swissonline.ch> wrote:
> Hi
>
> There is no need to remove the leading whitespaces (that's the space
> character ' ') as java.uti.Properties does already that for free:
>
> http://docs.oracle.com/javase/6/docs/api/java/util/Properties.html#load(jav
> a.io.Reader)
>
> So that we should only take care of the tailing spaces (see also the
> comment inside the commit).
>
> As this's a special case for "trimming logic" I don't see any good reason
> why we should pop it up into a XXXHelper utility class because (as already
> said) we just touch the tail of the String but *not* it's head.
>
> Thoughts?
>

I haven't looked but Camel's property placeholder supports loading
properties from different sources.
For example people can load from a database etc.

Just wonder if the same code logic is used in such use-cases. If not
then IMHO we should support the leading/trailing trims.

And you may at least consider moving the logic to a private method in
the class so the code is nicely separated.
If it should not be moved to a StringHelper in the util package.

> Babak
>
> Am 10.11.12 12:59 schrieb "Claus Ibsen" unter <cl...@gmail.com>:
>
>>Hi
>>
>>I suggest to add an util method to StringHelper (or what we call it).
>>And then add unit test for the trim method.
>>
>>Then it's easier to verify, and also re-use this method in other places.
>>And mind that it should also remove leading whitespaces.
>>
>>
>>
>>On Sat, Nov 10, 2012 at 12:48 PM,  <bv...@apache.org> wrote:
>>> Author: bvahdat
>>> Date: Sat Nov 10 11:48:43 2012
>>> New Revision: 1407776
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1407776&view=rev
>>> Log:
>>> CAMEL-5784: Preparing the loaded properties should only trim any
>>>potential whitespace characters.
>>>
>>> Modified:
>>>
>>>camel/trunk/camel-core/src/main/java/org/apache/camel/component/propertie
>>>s/DefaultPropertiesResolver.java
>>>
>>>camel/trunk/camel-core/src/test/java/org/apache/camel/component/propertie
>>>s/PropertiesComponentLoadPropertiesFromFileTrimValuesTest.java
>>>
>>> Modified:
>>>camel/trunk/camel-core/src/main/java/org/apache/camel/component/propertie
>>>s/DefaultPropertiesResolver.java
>>> URL:
>>>http://svn.apache.org/viewvc/camel/trunk/camel-core/src/main/java/org/apa
>>>che/camel/component/properties/DefaultPropertiesResolver.java?rev=1407776
>>>&r1=1407775&r2=1407776&view=diff
>>>
>>>=========================================================================
>>>=====
>>> ---
>>>camel/trunk/camel-core/src/main/java/org/apache/camel/component/propertie
>>>s/DefaultPropertiesResolver.java (original)
>>> +++
>>>camel/trunk/camel-core/src/main/java/org/apache/camel/component/propertie
>>>s/DefaultPropertiesResolver.java Sat Nov 10 11:48:43 2012
>>> @@ -131,11 +131,20 @@ public class DefaultPropertiesResolver i
>>>          for (Map.Entry<Object, Object> entry : properties.entrySet()) {
>>>              Object key = entry.getKey();
>>>              Object value = entry.getValue();
>>> -            // trim string values which can be a problem when loading
>>>from a properties file and there
>>> -            // is leading or trailing spaces in the value
>>> +            // trim any trailing spaces which can be a problem when
>>>loading from
>>> +            // a properties file, note that java.util.Properties does
>>>already this
>>> +            // for any potential leading spaces so there's nothing to
>>>do there
>>>              if (value instanceof String) {
>>>                  String s = (String) value;
>>> -                s = s.trim();
>>> +                int endIndex = s.length();
>>> +                for (int index = s.length() - 1; index >= 0; index--) {
>>> +                    if (s.charAt(index) == ' ') {
>>> +                        endIndex = index;
>>> +                    } else {
>>> +                        break;
>>> +                    }
>>> +                }
>>> +                s = s.substring(0, endIndex);
>>>                  value = s;
>>>              }
>>>              answer.put(key, value);
>>>
>>> Modified:
>>>camel/trunk/camel-core/src/test/java/org/apache/camel/component/propertie
>>>s/PropertiesComponentLoadPropertiesFromFileTrimValuesTest.java
>>> URL:
>>>http://svn.apache.org/viewvc/camel/trunk/camel-core/src/test/java/org/apa
>>>che/camel/component/properties/PropertiesComponentLoadPropertiesFromFileT
>>>rimValuesTest.java?rev=1407776&r1=1407775&r2=1407776&view=diff
>>>
>>>=========================================================================
>>>=====
>>> ---
>>>camel/trunk/camel-core/src/test/java/org/apache/camel/component/propertie
>>>s/PropertiesComponentLoadPropertiesFromFileTrimValuesTest.java (original)
>>> +++
>>>camel/trunk/camel-core/src/test/java/org/apache/camel/component/propertie
>>>s/PropertiesComponentLoadPropertiesFromFileTrimValuesTest.java Sat Nov
>>>10 11:48:43 2012
>>> @@ -16,7 +16,6 @@
>>>   */
>>>  package org.apache.camel.component.properties;
>>>
>>> -import java.io.File;
>>>  import java.io.FileOutputStream;
>>>
>>>  import org.apache.camel.CamelContext;
>>> @@ -39,15 +38,27 @@ public class PropertiesComponentLoadProp
>>>          CamelContext context = super.createCamelContext();
>>>
>>>          // create space.properties file
>>> -        File file = new File("target/space/space.properties");
>>> -        file.createNewFile();
>>> -        FileOutputStream fos = new FileOutputStream(file);
>>> -        fos.write("cool.leading= Leading space\ncool.trailing=Trailing
>>>space \ncool.both= Both leading and trailing space ".getBytes());
>>> +        FileOutputStream fos = new
>>>FileOutputStream("target/space/space.properties");
>>> +        String cool = "cool.leading= Leading space" + LS +
>>>"cool.trailing=Trailing space " + LS + "cool.both= Both leading and
>>>trailing space ";
>>> +        fos.write(cool.getBytes());
>>> +        fos.write(LS.getBytes());
>>> +
>>> +        String space = "space.leading=   \\r\\n" + LS +
>>>"space.trailing=\\t   " + LS + "space.both=  \\r   \\t  \\n   ";
>>> +        fos.write(space.getBytes());
>>> +        fos.write(LS.getBytes());
>>> +
>>> +        String mixed = "mixed.leading=   Leading space\\r\\n" + LS +
>>>"mixed.trailing=Trailing space\\t   " + LS + "mixed.both=  Both leading
>>>and trailing space\\r   \\t  \\n   ";
>>> +        fos.write(mixed.getBytes());
>>> +        fos.write(LS.getBytes());
>>> +
>>> +        String empty = "empty.line=                               ";
>>> +        fos.write(empty.getBytes());
>>> +
>>>          fos.close();
>>>
>>>          PropertiesComponent pc = new PropertiesComponent();
>>>          pc.setCamelContext(context);
>>> -        pc.setLocations(new
>>>String[]{"file:target/space/space.properties"});
>>> +        pc.setLocation("file:target/space/space.properties");
>>>          context.addComponent("properties", pc);
>>>
>>>          return context;
>>> @@ -57,6 +68,16 @@ public class PropertiesComponentLoadProp
>>>          assertEquals("Leading space",
>>>context.resolvePropertyPlaceholders("{{cool.leading}}"));
>>>          assertEquals("Trailing space",
>>>context.resolvePropertyPlaceholders("{{cool.trailing}}"));
>>>          assertEquals("Both leading and trailing space",
>>>context.resolvePropertyPlaceholders("{{cool.both}}"));
>>> +
>>> +        assertEquals("\r\n",
>>>context.resolvePropertyPlaceholders("{{space.leading}}"));
>>> +        assertEquals("\t",
>>>context.resolvePropertyPlaceholders("{{space.trailing}}"));
>>> +        assertEquals("\r   \t  \n",
>>>context.resolvePropertyPlaceholders("{{space.both}}"));
>>> +
>>> +        assertEquals("Leading space\r\n",
>>>context.resolvePropertyPlaceholders("{{mixed.leading}}"));
>>> +        assertEquals("Trailing space\t",
>>>context.resolvePropertyPlaceholders("{{mixed.trailing}}"));
>>> +        assertEquals("Both leading and trailing space\r   \t  \n",
>>>context.resolvePropertyPlaceholders("{{mixed.both}}"));
>>> +
>>> +        assertEquals("",
>>>context.resolvePropertyPlaceholders("{{empty.line}}"));
>>>      }
>>>
>>>  }
>>> \ No newline at end of file
>>>
>>>
>>
>>
>>
>>--
>>Claus Ibsen
>>-----------------
>>Red Hat, Inc.
>>FuseSource is now part of Red Hat
>>Email: cibsen@redhat.com
>>Web: http://fusesource.com
>>Twitter: davsclaus
>>Blog: http://davsclaus.com
>>Author of Camel in Action: http://www.manning.com/ibsen
>
>



-- 
Claus Ibsen
-----------------
Red Hat, Inc.
FuseSource is now part of Red Hat
Email: cibsen@redhat.com
Web: http://fusesource.com
Twitter: davsclaus
Blog: http://davsclaus.com
Author of Camel in Action: http://www.manning.com/ibsen

Re: svn commit: r1407776 - in /camel/trunk/camel-core/src: main/java/org/apache/camel/component/properties/DefaultPropertiesResolver.java test/java/org/apache/camel/component/properties/PropertiesComponentLoadPropertiesFromFileTrimValuesTest.java

Posted by Babak Vahdat <ba...@swissonline.ch>.

Am 10.11.12 13:12 schrieb "Babak Vahdat" unter
<ba...@swissonline.ch>:

>Hi
>
>There is no need to remove the leading whitespaces (that's the space
>character ' ') as java.uti.Properties does already that for free:
>
>http://docs.oracle.com/javase/6/docs/api/java/util/Properties.html#load(ja
>v
>a.io.Reader)
>
>So that we should only take care of the tailing spaces (see also the
>comment inside the commit).
>
>As this's a special case for "trimming logic" I don't see any good reason
>why we should pop it up into a XXXHelper utility class because (as already
>said) we just touch the tail of the String but *not* it's head.
>
>Thoughts?

Good morning Claus

Can I assume that you do agree with my explanation?

Babak

>
>Babak
>
>Am 10.11.12 12:59 schrieb "Claus Ibsen" unter <cl...@gmail.com>:
>
>>Hi
>>
>>I suggest to add an util method to StringHelper (or what we call it).
>>And then add unit test for the trim method.
>>
>>Then it's easier to verify, and also re-use this method in other places.
>>And mind that it should also remove leading whitespaces.
>>
>>
>>
>>On Sat, Nov 10, 2012 at 12:48 PM,  <bv...@apache.org> wrote:
>>> Author: bvahdat
>>> Date: Sat Nov 10 11:48:43 2012
>>> New Revision: 1407776
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1407776&view=rev
>>> Log:
>>> CAMEL-5784: Preparing the loaded properties should only trim any
>>>potential whitespace characters.
>>>
>>> Modified:
>>>     
>>>camel/trunk/camel-core/src/main/java/org/apache/camel/component/properti
>>>e
>>>s/DefaultPropertiesResolver.java
>>>     
>>>camel/trunk/camel-core/src/test/java/org/apache/camel/component/properti
>>>e
>>>s/PropertiesComponentLoadPropertiesFromFileTrimValuesTest.java
>>>
>>> Modified: 
>>>camel/trunk/camel-core/src/main/java/org/apache/camel/component/properti
>>>e
>>>s/DefaultPropertiesResolver.java
>>> URL: 
>>>http://svn.apache.org/viewvc/camel/trunk/camel-core/src/main/java/org/ap
>>>a
>>>che/camel/component/properties/DefaultPropertiesResolver.java?rev=140777
>>>6
>>>&r1=1407775&r2=1407776&view=diff
>>> 
>>>========================================================================
>>>=
>>>=====
>>> --- 
>>>camel/trunk/camel-core/src/main/java/org/apache/camel/component/properti
>>>e
>>>s/DefaultPropertiesResolver.java (original)
>>> +++ 
>>>camel/trunk/camel-core/src/main/java/org/apache/camel/component/properti
>>>e
>>>s/DefaultPropertiesResolver.java Sat Nov 10 11:48:43 2012
>>> @@ -131,11 +131,20 @@ public class DefaultPropertiesResolver i
>>>          for (Map.Entry<Object, Object> entry : properties.entrySet())
>>>{
>>>              Object key = entry.getKey();
>>>              Object value = entry.getValue();
>>> -            // trim string values which can be a problem when loading
>>>from a properties file and there
>>> -            // is leading or trailing spaces in the value
>>> +            // trim any trailing spaces which can be a problem when
>>>loading from
>>> +            // a properties file, note that java.util.Properties does
>>>already this
>>> +            // for any potential leading spaces so there's nothing to
>>>do there
>>>              if (value instanceof String) {
>>>                  String s = (String) value;
>>> -                s = s.trim();
>>> +                int endIndex = s.length();
>>> +                for (int index = s.length() - 1; index >= 0; index--)
>>>{
>>> +                    if (s.charAt(index) == ' ') {
>>> +                        endIndex = index;
>>> +                    } else {
>>> +                        break;
>>> +                    }
>>> +                }
>>> +                s = s.substring(0, endIndex);
>>>                  value = s;
>>>              }
>>>              answer.put(key, value);
>>>
>>> Modified: 
>>>camel/trunk/camel-core/src/test/java/org/apache/camel/component/properti
>>>e
>>>s/PropertiesComponentLoadPropertiesFromFileTrimValuesTest.java
>>> URL: 
>>>http://svn.apache.org/viewvc/camel/trunk/camel-core/src/test/java/org/ap
>>>a
>>>che/camel/component/properties/PropertiesComponentLoadPropertiesFromFile
>>>T
>>>rimValuesTest.java?rev=1407776&r1=1407775&r2=1407776&view=diff
>>> 
>>>========================================================================
>>>=
>>>=====
>>> --- 
>>>camel/trunk/camel-core/src/test/java/org/apache/camel/component/properti
>>>e
>>>s/PropertiesComponentLoadPropertiesFromFileTrimValuesTest.java
>>>(original)
>>> +++ 
>>>camel/trunk/camel-core/src/test/java/org/apache/camel/component/properti
>>>e
>>>s/PropertiesComponentLoadPropertiesFromFileTrimValuesTest.java Sat Nov
>>>10 11:48:43 2012
>>> @@ -16,7 +16,6 @@
>>>   */
>>>  package org.apache.camel.component.properties;
>>>
>>> -import java.io.File;
>>>  import java.io.FileOutputStream;
>>>
>>>  import org.apache.camel.CamelContext;
>>> @@ -39,15 +38,27 @@ public class PropertiesComponentLoadProp
>>>          CamelContext context = super.createCamelContext();
>>>
>>>          // create space.properties file
>>> -        File file = new File("target/space/space.properties");
>>> -        file.createNewFile();
>>> -        FileOutputStream fos = new FileOutputStream(file);
>>> -        fos.write("cool.leading= Leading space\ncool.trailing=Trailing
>>>space \ncool.both= Both leading and trailing space ".getBytes());
>>> +        FileOutputStream fos = new
>>>FileOutputStream("target/space/space.properties");
>>> +        String cool = "cool.leading= Leading space" + LS +
>>>"cool.trailing=Trailing space " + LS + "cool.both= Both leading and
>>>trailing space ";
>>> +        fos.write(cool.getBytes());
>>> +        fos.write(LS.getBytes());
>>> +
>>> +        String space = "space.leading=   \\r\\n" + LS +
>>>"space.trailing=\\t   " + LS + "space.both=  \\r   \\t  \\n   ";
>>> +        fos.write(space.getBytes());
>>> +        fos.write(LS.getBytes());
>>> +
>>> +        String mixed = "mixed.leading=   Leading space\\r\\n" + LS +
>>>"mixed.trailing=Trailing space\\t   " + LS + "mixed.both=  Both leading
>>>and trailing space\\r   \\t  \\n   ";
>>> +        fos.write(mixed.getBytes());
>>> +        fos.write(LS.getBytes());
>>> +
>>> +        String empty = "empty.line=                               ";
>>> +        fos.write(empty.getBytes());
>>> +
>>>          fos.close();
>>>
>>>          PropertiesComponent pc = new PropertiesComponent();
>>>          pc.setCamelContext(context);
>>> -        pc.setLocations(new
>>>String[]{"file:target/space/space.properties"});
>>> +        pc.setLocation("file:target/space/space.properties");
>>>          context.addComponent("properties", pc);
>>>
>>>          return context;
>>> @@ -57,6 +68,16 @@ public class PropertiesComponentLoadProp
>>>          assertEquals("Leading space",
>>>context.resolvePropertyPlaceholders("{{cool.leading}}"));
>>>          assertEquals("Trailing space",
>>>context.resolvePropertyPlaceholders("{{cool.trailing}}"));
>>>          assertEquals("Both leading and trailing space",
>>>context.resolvePropertyPlaceholders("{{cool.both}}"));
>>> +
>>> +        assertEquals("\r\n",
>>>context.resolvePropertyPlaceholders("{{space.leading}}"));
>>> +        assertEquals("\t",
>>>context.resolvePropertyPlaceholders("{{space.trailing}}"));
>>> +        assertEquals("\r   \t  \n",
>>>context.resolvePropertyPlaceholders("{{space.both}}"));
>>> +
>>> +        assertEquals("Leading space\r\n",
>>>context.resolvePropertyPlaceholders("{{mixed.leading}}"));
>>> +        assertEquals("Trailing space\t",
>>>context.resolvePropertyPlaceholders("{{mixed.trailing}}"));
>>> +        assertEquals("Both leading and trailing space\r   \t  \n",
>>>context.resolvePropertyPlaceholders("{{mixed.both}}"));
>>> +
>>> +        assertEquals("",
>>>context.resolvePropertyPlaceholders("{{empty.line}}"));
>>>      }
>>>
>>>  }
>>> \ No newline at end of file
>>>
>>>
>>
>>
>>
>>-- 
>>Claus Ibsen
>>-----------------
>>Red Hat, Inc.
>>FuseSource is now part of Red Hat
>>Email: cibsen@redhat.com
>>Web: http://fusesource.com
>>Twitter: davsclaus
>>Blog: http://davsclaus.com
>>Author of Camel in Action: http://www.manning.com/ibsen
>
>



Re: svn commit: r1407776 - in /camel/trunk/camel-core/src: main/java/org/apache/camel/component/properties/DefaultPropertiesResolver.java test/java/org/apache/camel/component/properties/PropertiesComponentLoadPropertiesFromFileTrimValuesTest.java

Posted by Babak Vahdat <ba...@swissonline.ch>.
Hi

There is no need to remove the leading whitespaces (that's the space
character ' ') as java.uti.Properties does already that for free:

http://docs.oracle.com/javase/6/docs/api/java/util/Properties.html#load(jav
a.io.Reader)

So that we should only take care of the tailing spaces (see also the
comment inside the commit).

As this's a special case for "trimming logic" I don't see any good reason
why we should pop it up into a XXXHelper utility class because (as already
said) we just touch the tail of the String but *not* it's head.

Thoughts?

Babak

Am 10.11.12 12:59 schrieb "Claus Ibsen" unter <cl...@gmail.com>:

>Hi
>
>I suggest to add an util method to StringHelper (or what we call it).
>And then add unit test for the trim method.
>
>Then it's easier to verify, and also re-use this method in other places.
>And mind that it should also remove leading whitespaces.
>
>
>
>On Sat, Nov 10, 2012 at 12:48 PM,  <bv...@apache.org> wrote:
>> Author: bvahdat
>> Date: Sat Nov 10 11:48:43 2012
>> New Revision: 1407776
>>
>> URL: http://svn.apache.org/viewvc?rev=1407776&view=rev
>> Log:
>> CAMEL-5784: Preparing the loaded properties should only trim any
>>potential whitespace characters.
>>
>> Modified:
>>     
>>camel/trunk/camel-core/src/main/java/org/apache/camel/component/propertie
>>s/DefaultPropertiesResolver.java
>>     
>>camel/trunk/camel-core/src/test/java/org/apache/camel/component/propertie
>>s/PropertiesComponentLoadPropertiesFromFileTrimValuesTest.java
>>
>> Modified: 
>>camel/trunk/camel-core/src/main/java/org/apache/camel/component/propertie
>>s/DefaultPropertiesResolver.java
>> URL: 
>>http://svn.apache.org/viewvc/camel/trunk/camel-core/src/main/java/org/apa
>>che/camel/component/properties/DefaultPropertiesResolver.java?rev=1407776
>>&r1=1407775&r2=1407776&view=diff
>> 
>>=========================================================================
>>=====
>> --- 
>>camel/trunk/camel-core/src/main/java/org/apache/camel/component/propertie
>>s/DefaultPropertiesResolver.java (original)
>> +++ 
>>camel/trunk/camel-core/src/main/java/org/apache/camel/component/propertie
>>s/DefaultPropertiesResolver.java Sat Nov 10 11:48:43 2012
>> @@ -131,11 +131,20 @@ public class DefaultPropertiesResolver i
>>          for (Map.Entry<Object, Object> entry : properties.entrySet()) {
>>              Object key = entry.getKey();
>>              Object value = entry.getValue();
>> -            // trim string values which can be a problem when loading
>>from a properties file and there
>> -            // is leading or trailing spaces in the value
>> +            // trim any trailing spaces which can be a problem when
>>loading from
>> +            // a properties file, note that java.util.Properties does
>>already this
>> +            // for any potential leading spaces so there's nothing to
>>do there
>>              if (value instanceof String) {
>>                  String s = (String) value;
>> -                s = s.trim();
>> +                int endIndex = s.length();
>> +                for (int index = s.length() - 1; index >= 0; index--) {
>> +                    if (s.charAt(index) == ' ') {
>> +                        endIndex = index;
>> +                    } else {
>> +                        break;
>> +                    }
>> +                }
>> +                s = s.substring(0, endIndex);
>>                  value = s;
>>              }
>>              answer.put(key, value);
>>
>> Modified: 
>>camel/trunk/camel-core/src/test/java/org/apache/camel/component/propertie
>>s/PropertiesComponentLoadPropertiesFromFileTrimValuesTest.java
>> URL: 
>>http://svn.apache.org/viewvc/camel/trunk/camel-core/src/test/java/org/apa
>>che/camel/component/properties/PropertiesComponentLoadPropertiesFromFileT
>>rimValuesTest.java?rev=1407776&r1=1407775&r2=1407776&view=diff
>> 
>>=========================================================================
>>=====
>> --- 
>>camel/trunk/camel-core/src/test/java/org/apache/camel/component/propertie
>>s/PropertiesComponentLoadPropertiesFromFileTrimValuesTest.java (original)
>> +++ 
>>camel/trunk/camel-core/src/test/java/org/apache/camel/component/propertie
>>s/PropertiesComponentLoadPropertiesFromFileTrimValuesTest.java Sat Nov
>>10 11:48:43 2012
>> @@ -16,7 +16,6 @@
>>   */
>>  package org.apache.camel.component.properties;
>>
>> -import java.io.File;
>>  import java.io.FileOutputStream;
>>
>>  import org.apache.camel.CamelContext;
>> @@ -39,15 +38,27 @@ public class PropertiesComponentLoadProp
>>          CamelContext context = super.createCamelContext();
>>
>>          // create space.properties file
>> -        File file = new File("target/space/space.properties");
>> -        file.createNewFile();
>> -        FileOutputStream fos = new FileOutputStream(file);
>> -        fos.write("cool.leading= Leading space\ncool.trailing=Trailing
>>space \ncool.both= Both leading and trailing space ".getBytes());
>> +        FileOutputStream fos = new
>>FileOutputStream("target/space/space.properties");
>> +        String cool = "cool.leading= Leading space" + LS +
>>"cool.trailing=Trailing space " + LS + "cool.both= Both leading and
>>trailing space ";
>> +        fos.write(cool.getBytes());
>> +        fos.write(LS.getBytes());
>> +
>> +        String space = "space.leading=   \\r\\n" + LS +
>>"space.trailing=\\t   " + LS + "space.both=  \\r   \\t  \\n   ";
>> +        fos.write(space.getBytes());
>> +        fos.write(LS.getBytes());
>> +
>> +        String mixed = "mixed.leading=   Leading space\\r\\n" + LS +
>>"mixed.trailing=Trailing space\\t   " + LS + "mixed.both=  Both leading
>>and trailing space\\r   \\t  \\n   ";
>> +        fos.write(mixed.getBytes());
>> +        fos.write(LS.getBytes());
>> +
>> +        String empty = "empty.line=                               ";
>> +        fos.write(empty.getBytes());
>> +
>>          fos.close();
>>
>>          PropertiesComponent pc = new PropertiesComponent();
>>          pc.setCamelContext(context);
>> -        pc.setLocations(new
>>String[]{"file:target/space/space.properties"});
>> +        pc.setLocation("file:target/space/space.properties");
>>          context.addComponent("properties", pc);
>>
>>          return context;
>> @@ -57,6 +68,16 @@ public class PropertiesComponentLoadProp
>>          assertEquals("Leading space",
>>context.resolvePropertyPlaceholders("{{cool.leading}}"));
>>          assertEquals("Trailing space",
>>context.resolvePropertyPlaceholders("{{cool.trailing}}"));
>>          assertEquals("Both leading and trailing space",
>>context.resolvePropertyPlaceholders("{{cool.both}}"));
>> +
>> +        assertEquals("\r\n",
>>context.resolvePropertyPlaceholders("{{space.leading}}"));
>> +        assertEquals("\t",
>>context.resolvePropertyPlaceholders("{{space.trailing}}"));
>> +        assertEquals("\r   \t  \n",
>>context.resolvePropertyPlaceholders("{{space.both}}"));
>> +
>> +        assertEquals("Leading space\r\n",
>>context.resolvePropertyPlaceholders("{{mixed.leading}}"));
>> +        assertEquals("Trailing space\t",
>>context.resolvePropertyPlaceholders("{{mixed.trailing}}"));
>> +        assertEquals("Both leading and trailing space\r   \t  \n",
>>context.resolvePropertyPlaceholders("{{mixed.both}}"));
>> +
>> +        assertEquals("",
>>context.resolvePropertyPlaceholders("{{empty.line}}"));
>>      }
>>
>>  }
>> \ No newline at end of file
>>
>>
>
>
>
>-- 
>Claus Ibsen
>-----------------
>Red Hat, Inc.
>FuseSource is now part of Red Hat
>Email: cibsen@redhat.com
>Web: http://fusesource.com
>Twitter: davsclaus
>Blog: http://davsclaus.com
>Author of Camel in Action: http://www.manning.com/ibsen



Re: svn commit: r1407776 - in /camel/trunk/camel-core/src: main/java/org/apache/camel/component/properties/DefaultPropertiesResolver.java test/java/org/apache/camel/component/properties/PropertiesComponentLoadPropertiesFromFileTrimValuesTest.java

Posted by Claus Ibsen <cl...@gmail.com>.
Hi

I suggest to add an util method to StringHelper (or what we call it).
And then add unit test for the trim method.

Then it's easier to verify, and also re-use this method in other places.
And mind that it should also remove leading whitespaces.



On Sat, Nov 10, 2012 at 12:48 PM,  <bv...@apache.org> wrote:
> Author: bvahdat
> Date: Sat Nov 10 11:48:43 2012
> New Revision: 1407776
>
> URL: http://svn.apache.org/viewvc?rev=1407776&view=rev
> Log:
> CAMEL-5784: Preparing the loaded properties should only trim any potential whitespace characters.
>
> Modified:
>     camel/trunk/camel-core/src/main/java/org/apache/camel/component/properties/DefaultPropertiesResolver.java
>     camel/trunk/camel-core/src/test/java/org/apache/camel/component/properties/PropertiesComponentLoadPropertiesFromFileTrimValuesTest.java
>
> Modified: camel/trunk/camel-core/src/main/java/org/apache/camel/component/properties/DefaultPropertiesResolver.java
> URL: http://svn.apache.org/viewvc/camel/trunk/camel-core/src/main/java/org/apache/camel/component/properties/DefaultPropertiesResolver.java?rev=1407776&r1=1407775&r2=1407776&view=diff
> ==============================================================================
> --- camel/trunk/camel-core/src/main/java/org/apache/camel/component/properties/DefaultPropertiesResolver.java (original)
> +++ camel/trunk/camel-core/src/main/java/org/apache/camel/component/properties/DefaultPropertiesResolver.java Sat Nov 10 11:48:43 2012
> @@ -131,11 +131,20 @@ public class DefaultPropertiesResolver i
>          for (Map.Entry<Object, Object> entry : properties.entrySet()) {
>              Object key = entry.getKey();
>              Object value = entry.getValue();
> -            // trim string values which can be a problem when loading from a properties file and there
> -            // is leading or trailing spaces in the value
> +            // trim any trailing spaces which can be a problem when loading from
> +            // a properties file, note that java.util.Properties does already this
> +            // for any potential leading spaces so there's nothing to do there
>              if (value instanceof String) {
>                  String s = (String) value;
> -                s = s.trim();
> +                int endIndex = s.length();
> +                for (int index = s.length() - 1; index >= 0; index--) {
> +                    if (s.charAt(index) == ' ') {
> +                        endIndex = index;
> +                    } else {
> +                        break;
> +                    }
> +                }
> +                s = s.substring(0, endIndex);
>                  value = s;
>              }
>              answer.put(key, value);
>
> Modified: camel/trunk/camel-core/src/test/java/org/apache/camel/component/properties/PropertiesComponentLoadPropertiesFromFileTrimValuesTest.java
> URL: http://svn.apache.org/viewvc/camel/trunk/camel-core/src/test/java/org/apache/camel/component/properties/PropertiesComponentLoadPropertiesFromFileTrimValuesTest.java?rev=1407776&r1=1407775&r2=1407776&view=diff
> ==============================================================================
> --- camel/trunk/camel-core/src/test/java/org/apache/camel/component/properties/PropertiesComponentLoadPropertiesFromFileTrimValuesTest.java (original)
> +++ camel/trunk/camel-core/src/test/java/org/apache/camel/component/properties/PropertiesComponentLoadPropertiesFromFileTrimValuesTest.java Sat Nov 10 11:48:43 2012
> @@ -16,7 +16,6 @@
>   */
>  package org.apache.camel.component.properties;
>
> -import java.io.File;
>  import java.io.FileOutputStream;
>
>  import org.apache.camel.CamelContext;
> @@ -39,15 +38,27 @@ public class PropertiesComponentLoadProp
>          CamelContext context = super.createCamelContext();
>
>          // create space.properties file
> -        File file = new File("target/space/space.properties");
> -        file.createNewFile();
> -        FileOutputStream fos = new FileOutputStream(file);
> -        fos.write("cool.leading= Leading space\ncool.trailing=Trailing space \ncool.both= Both leading and trailing space ".getBytes());
> +        FileOutputStream fos = new FileOutputStream("target/space/space.properties");
> +        String cool = "cool.leading= Leading space" + LS + "cool.trailing=Trailing space " + LS + "cool.both= Both leading and trailing space ";
> +        fos.write(cool.getBytes());
> +        fos.write(LS.getBytes());
> +
> +        String space = "space.leading=   \\r\\n" + LS + "space.trailing=\\t   " + LS + "space.both=  \\r   \\t  \\n   ";
> +        fos.write(space.getBytes());
> +        fos.write(LS.getBytes());
> +
> +        String mixed = "mixed.leading=   Leading space\\r\\n" + LS + "mixed.trailing=Trailing space\\t   " + LS + "mixed.both=  Both leading and trailing space\\r   \\t  \\n   ";
> +        fos.write(mixed.getBytes());
> +        fos.write(LS.getBytes());
> +
> +        String empty = "empty.line=                               ";
> +        fos.write(empty.getBytes());
> +
>          fos.close();
>
>          PropertiesComponent pc = new PropertiesComponent();
>          pc.setCamelContext(context);
> -        pc.setLocations(new String[]{"file:target/space/space.properties"});
> +        pc.setLocation("file:target/space/space.properties");
>          context.addComponent("properties", pc);
>
>          return context;
> @@ -57,6 +68,16 @@ public class PropertiesComponentLoadProp
>          assertEquals("Leading space", context.resolvePropertyPlaceholders("{{cool.leading}}"));
>          assertEquals("Trailing space", context.resolvePropertyPlaceholders("{{cool.trailing}}"));
>          assertEquals("Both leading and trailing space", context.resolvePropertyPlaceholders("{{cool.both}}"));
> +
> +        assertEquals("\r\n", context.resolvePropertyPlaceholders("{{space.leading}}"));
> +        assertEquals("\t", context.resolvePropertyPlaceholders("{{space.trailing}}"));
> +        assertEquals("\r   \t  \n", context.resolvePropertyPlaceholders("{{space.both}}"));
> +
> +        assertEquals("Leading space\r\n", context.resolvePropertyPlaceholders("{{mixed.leading}}"));
> +        assertEquals("Trailing space\t", context.resolvePropertyPlaceholders("{{mixed.trailing}}"));
> +        assertEquals("Both leading and trailing space\r   \t  \n", context.resolvePropertyPlaceholders("{{mixed.both}}"));
> +
> +        assertEquals("", context.resolvePropertyPlaceholders("{{empty.line}}"));
>      }
>
>  }
> \ No newline at end of file
>
>



-- 
Claus Ibsen
-----------------
Red Hat, Inc.
FuseSource is now part of Red Hat
Email: cibsen@redhat.com
Web: http://fusesource.com
Twitter: davsclaus
Blog: http://davsclaus.com
Author of Camel in Action: http://www.manning.com/ibsen