camel-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Claus Ibsen <claus.ib...@gmail.com>
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
Date Sun, 11 Nov 2012 10:25:10 GMT
On Sat, Nov 10, 2012 at 1:12 PM, Babak Vahdat
<babak.vahdat@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 <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/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

Mime
View raw message