Mailing-List: contact dev-help@camel.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@camel.apache.org
Received-SPF: neutral (nike.apache.org: local policy)
User-Agent: Microsoft-MacOutlook/14.14.0.111121
Date: Sun, 11 Nov 2012 08:55:46 +0100
Subject: 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
From: Babak Vahdat <babak.vahdat@swissonline.ch>
To: <dev@camel.apache.org>
Message-ID: <CCC517A1.2BCA%babak.vahdat@swissonline.ch>
Thread-Topic: 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
In-Reply-To: <CCC40092.2B8A%babak.vahdat@swissonline.ch>
Mime-version: 1.0
Content-type: text/plain;
	charset="US-ASCII"
Content-transfer-encoding: 7bit


Am 10.11.12 13:12 schrieb "Babak Vahdat" unter
<babak.vahdat@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 <claus.ibsen@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,  <bvahdat@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
>
>