Any chance of trimming returned values? #120

Closed
a1730 opened this Issue Mar 31, 2015 · 8 comments

Comments

Projects
None yet
2 participants
@a1730

a1730 commented Mar 31, 2015

First thank you for a great library - succinct and just right for the job.
We've been using the library a while and we LOVE it! However, QA was getting a funky error over the weekend and it turns out that one of the integer properties was throwing a conversion error.
The line in the properties file is

pollingPeriod = 300 # secs

The intent is for a polling period of 5 minutes. We caught the first exception because of the #secs comment but it was hard to see why '300 ' was throwing an error.

By the way, it would be nice if in line comment is supported but not a must have.

Thanks again for a job well done.

@lviggiano

This comment has been minimized.

Show comment
Hide comment
@lviggiano

lviggiano Apr 14, 2015

Owner

I'll look into it asap.

Owner

lviggiano commented Apr 14, 2015

I'll look into it asap.

@lviggiano

This comment has been minimized.

Show comment
Hide comment
@lviggiano

lviggiano Apr 14, 2015

Owner

Hi.

The properties file format does not support inline comments:

Lines that start with the comment characters ! or # are ignored. Blank lines are also ignored.

In fact the "# secs" is taken as part of the property value, and I get this error:

java.lang.UnsupportedOperationException: Cannot convert '300 # secs' to java.lang.Integer
    at org.aeonbits.owner.Util.unsupported(Util.java:132)
    at org.aeonbits.owner.Converters.unsupportedConversion(Converters.java:274)
    at org.aeonbits.owner.Converters.access$200(Converters.java:40)
    at org.aeonbits.owner.Converters$11.tryConvert(Converters.java:254)
    at org.aeonbits.owner.Converters.doConvert(Converters.java:266)
    at org.aeonbits.owner.Converters.convert(Converters.java:261)
    at org.aeonbits.owner.PropertiesInvocationHandler.resolveProperty(PropertiesInvocationHandler.java:82)
    at org.aeonbits.owner.PropertiesInvocationHandler.invoke(PropertiesInvocationHandler.java:63)
    at com.sun.proxy.$Proxy7.pollingPeriod(Unknown Source)
    at org.aeonbits.owner.issues.Issue120.shouldReturnProperValue(Issue120.java:32)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
    at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
    at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
    at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
    at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
    at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
    at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
    at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
    at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
    at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
    at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
    at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
    at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
    at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:78)
    at com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:212)
    at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:68)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
    at com.intellij.rt.execution.application.AppMain.main(AppMain.java:140)

Also my IDE (IntelliJ IDEA 14) shows the trailing "#" in green as value of the property:

screen shot 2015-04-14 at 18 26 18

So I suggest to specify the information in a different way:

# polling period must be specified in seconds
pollingPeriod = 300

pollingPeriodInSeconds = 300
polling.period.seconds = 300

If I implement the inline comment I break the compatibility with properties and I also need to provide an escaping mechanism for the "#" character, that is not needed in standard java properties.

That said, I'll look into the possibility to trim values for primitives and numeric wrapper classes.

Owner

lviggiano commented Apr 14, 2015

Hi.

The properties file format does not support inline comments:

Lines that start with the comment characters ! or # are ignored. Blank lines are also ignored.

In fact the "# secs" is taken as part of the property value, and I get this error:

java.lang.UnsupportedOperationException: Cannot convert '300 # secs' to java.lang.Integer
    at org.aeonbits.owner.Util.unsupported(Util.java:132)
    at org.aeonbits.owner.Converters.unsupportedConversion(Converters.java:274)
    at org.aeonbits.owner.Converters.access$200(Converters.java:40)
    at org.aeonbits.owner.Converters$11.tryConvert(Converters.java:254)
    at org.aeonbits.owner.Converters.doConvert(Converters.java:266)
    at org.aeonbits.owner.Converters.convert(Converters.java:261)
    at org.aeonbits.owner.PropertiesInvocationHandler.resolveProperty(PropertiesInvocationHandler.java:82)
    at org.aeonbits.owner.PropertiesInvocationHandler.invoke(PropertiesInvocationHandler.java:63)
    at com.sun.proxy.$Proxy7.pollingPeriod(Unknown Source)
    at org.aeonbits.owner.issues.Issue120.shouldReturnProperValue(Issue120.java:32)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
    at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
    at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
    at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
    at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
    at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
    at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
    at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
    at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
    at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
    at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
    at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
    at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
    at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:78)
    at com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:212)
    at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:68)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
    at com.intellij.rt.execution.application.AppMain.main(AppMain.java:140)

Also my IDE (IntelliJ IDEA 14) shows the trailing "#" in green as value of the property:

screen shot 2015-04-14 at 18 26 18

So I suggest to specify the information in a different way:

# polling period must be specified in seconds
pollingPeriod = 300

pollingPeriodInSeconds = 300
polling.period.seconds = 300

If I implement the inline comment I break the compatibility with properties and I also need to provide an escaping mechanism for the "#" character, that is not needed in standard java properties.

That said, I'll look into the possibility to trim values for primitives and numeric wrapper classes.

@lviggiano lviggiano closed this in 4d39966 Apr 16, 2015

@lviggiano

This comment has been minimized.

Show comment
Hide comment
@lviggiano

lviggiano Apr 16, 2015

Owner

Hi.

I committed on master branch a new 'Preprocessor' feature. Here is an example on how to use it:

public class PreprocessorTest {

    @PreprocessorClasses({ SkipInlineComments.class, Trim.class })
    public interface ConfigWithPreprocessors extends Config {
        @DefaultValue("  300  ")
        Integer pollingPeriod();

        @PreprocessorClasses(ToLowerCase.class)
        @DefaultValue("  HelloWorld  ")
        String helloWorld();

        @DefaultValue("  This is the value  ! this is an inline comment # this is the inline comment too")
        String skipsInlineComments();

        @PreprocessorClasses(ToLowerCase.class)
        String nullValue();
    }

    @Test
    public void shouldReturnTrimmedValue() {
        ConfigWithPreprocessors cfg = ConfigFactory.create(ConfigWithPreprocessors.class);
        assertEquals(new Integer(300), cfg.pollingPeriod());
    }

    @Test
    public void shouldReturnLowercasedTrimmedHelloWorld() {
        ConfigWithPreprocessors cfg = ConfigFactory.create(ConfigWithPreprocessors.class);
        assertEquals("helloworld", cfg.helloWorld());
    }

    @Test
    public void shouldSkipInlineComments() {
        ConfigWithPreprocessors cfg = ConfigFactory.create(ConfigWithPreprocessors.class);
        assertEquals("This is the value", cfg.skipsInlineComments());
    }

    @Test
    public void shouldNotThrowExceptions() {
        ConfigWithPreprocessors cfg = ConfigFactory.create(ConfigWithPreprocessors.class);
        assertNull(cfg.nullValue());
    }


    // preprocessors implementation

    public static class Trim implements Preprocessor {
        public String process(String input) {
            return input.trim();
        }
    }

    public static class ToLowerCase implements Preprocessor {
        public String process(String input) {
            return input.toLowerCase();
        }
    }

    public static class SkipInlineComments implements Preprocessor {
        public String process(String input) {
            int hashTagIndex = input.indexOf('#');
            int exclamationMarkIndex = input.indexOf("!");
            if (hashTagIndex == -1 && exclamationMarkIndex == -1)
                return input; // comments not present.

            int commentIndex = min(hashTagIndex, exclamationMarkIndex);
            if (commentIndex == -1)
                commentIndex = max(hashTagIndex, exclamationMarkIndex);

            return input.substring(0, commentIndex);
        }
    }

}

full source code of this example is available here.

Preprocessors are applied all together: first the ones specified on the method level, then the ones specified on the class level. Order specified in the @PreprocessorClasses annotation is honoured.

Owner

lviggiano commented Apr 16, 2015

Hi.

I committed on master branch a new 'Preprocessor' feature. Here is an example on how to use it:

public class PreprocessorTest {

    @PreprocessorClasses({ SkipInlineComments.class, Trim.class })
    public interface ConfigWithPreprocessors extends Config {
        @DefaultValue("  300  ")
        Integer pollingPeriod();

        @PreprocessorClasses(ToLowerCase.class)
        @DefaultValue("  HelloWorld  ")
        String helloWorld();

        @DefaultValue("  This is the value  ! this is an inline comment # this is the inline comment too")
        String skipsInlineComments();

        @PreprocessorClasses(ToLowerCase.class)
        String nullValue();
    }

    @Test
    public void shouldReturnTrimmedValue() {
        ConfigWithPreprocessors cfg = ConfigFactory.create(ConfigWithPreprocessors.class);
        assertEquals(new Integer(300), cfg.pollingPeriod());
    }

    @Test
    public void shouldReturnLowercasedTrimmedHelloWorld() {
        ConfigWithPreprocessors cfg = ConfigFactory.create(ConfigWithPreprocessors.class);
        assertEquals("helloworld", cfg.helloWorld());
    }

    @Test
    public void shouldSkipInlineComments() {
        ConfigWithPreprocessors cfg = ConfigFactory.create(ConfigWithPreprocessors.class);
        assertEquals("This is the value", cfg.skipsInlineComments());
    }

    @Test
    public void shouldNotThrowExceptions() {
        ConfigWithPreprocessors cfg = ConfigFactory.create(ConfigWithPreprocessors.class);
        assertNull(cfg.nullValue());
    }


    // preprocessors implementation

    public static class Trim implements Preprocessor {
        public String process(String input) {
            return input.trim();
        }
    }

    public static class ToLowerCase implements Preprocessor {
        public String process(String input) {
            return input.toLowerCase();
        }
    }

    public static class SkipInlineComments implements Preprocessor {
        public String process(String input) {
            int hashTagIndex = input.indexOf('#');
            int exclamationMarkIndex = input.indexOf("!");
            if (hashTagIndex == -1 && exclamationMarkIndex == -1)
                return input; // comments not present.

            int commentIndex = min(hashTagIndex, exclamationMarkIndex);
            if (commentIndex == -1)
                commentIndex = max(hashTagIndex, exclamationMarkIndex);

            return input.substring(0, commentIndex);
        }
    }

}

full source code of this example is available here.

Preprocessors are applied all together: first the ones specified on the method level, then the ones specified on the class level. Order specified in the @PreprocessorClasses annotation is honoured.

@lviggiano

This comment has been minimized.

Show comment
Hide comment
@lviggiano

lviggiano Apr 16, 2015

Owner

The interface of Preprocessor may change to address also #118, I could include parameter values passed to the method, and eventually additional context objects (i.e. the internal properties representation for other properties.)

Owner

lviggiano commented Apr 16, 2015

The interface of Preprocessor may change to address also #118, I could include parameter values passed to the method, and eventually additional context objects (i.e. the internal properties representation for other properties.)

@a1730

This comment has been minimized.

Show comment
Hide comment
@a1730

a1730 Apr 28, 2015

Thanks Iviggiano!
We are aware that Java properties api does not support inline comment and that was why it was a 'would be nice' request. Also, we would not be looking for alternative solution if that Java built-in solution had worked.
Thank you for your elegant solution to our requests. We will test your solution asap.

More importantly, thank you for a job well done on the Owner library.

a1730 commented Apr 28, 2015

Thanks Iviggiano!
We are aware that Java properties api does not support inline comment and that was why it was a 'would be nice' request. Also, we would not be looking for alternative solution if that Java built-in solution had worked.
Thank you for your elegant solution to our requests. We will test your solution asap.

More importantly, thank you for a job well done on the Owner library.

@a1730

This comment has been minimized.

Show comment
Hide comment
@a1730

a1730 May 6, 2015

Hi!
I just wanted to say that the pre-processor functionality is awesome.
The solution solved our problems - we are happy.
Thank you.

a1730 commented May 6, 2015

Hi!
I just wanted to say that the pre-processor functionality is awesome.
The solution solved our problems - we are happy.
Thank you.

@lviggiano

This comment has been minimized.

Show comment
Hide comment
@lviggiano

lviggiano May 6, 2015

Owner

Thanks for your feedback.
The feature will be available with the next released jar in maven central repository, I hope soon. (I am planning to get back on more active development on this library soon)

Owner

lviggiano commented May 6, 2015

Thanks for your feedback.
The feature will be available with the next released jar in maven central repository, I hope soon. (I am planning to get back on more active development on this library soon)

@lviggiano

This comment has been minimized.

Show comment
Hide comment
@lviggiano

lviggiano Jul 22, 2015

Owner

Today I release OWNER 1.0.9. Sorry for the long delay.
Hope everything is fine; let me know if there is any problem with it.

Owner

lviggiano commented Jul 22, 2015

Today I release OWNER 1.0.9. Sorry for the long delay.
Hope everything is fine; let me know if there is any problem with it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment