New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Can't convert 'file:${my.properties}' to a valid URI #134

Closed
ant1g opened this Issue Jul 29, 2015 · 9 comments

Comments

Projects
None yet
5 participants
@ant1g

ant1g commented Jul 29, 2015

Hi,

In the latest 1.0.9, I am seeing an exception being thrown when creating the config from the ConfigFactory.
Here is the exception:

Caused by: java.lang.UnsupportedOperationException: Can't convert 'file:${my.properties}' to a valid URI
    at org.aeonbits.owner.Util.unsupported(Util.java:128)
    at org.aeonbits.owner.PropertiesManager.toURIs(PropertiesManager.java:134)
    at org.aeonbits.owner.PropertiesManager.<init>(PropertiesManager.java:105)
    at org.aeonbits.owner.DefaultFactory.create(DefaultFactory.java:43)
    at org.aeonbits.owner.ConfigFactory.create(ConfigFactory.java:66)
    ... 66 more
Caused by: java.net.URISyntaxException: Expected scheme-specific part at index 5: file:
    at java.net.URI$Parser.fail(URI.java:2829)
    at java.net.URI$Parser.failExpecting(URI.java:2835)
    at java.net.URI$Parser.parse(URI.java:3038)
    at java.net.URI.<init>(URI.java:595)
    at org.aeonbits.owner.ConfigURIFactory.newURI(ConfigURIFactory.java:41)
    at org.aeonbits.owner.PropertiesManager.toURIs(PropertiesManager.java:130)

Here is my @sources annotation:

@Sources({"file:${my.properties}", "classpath:conf/${my.env}.my.properties", "classpath:conf/my.properties"})
public interface MyProperties extends Config {
...
}

This used to work fine in previous versions (1.0.5.1), as follows:

  • If a system property "my.properties" was defined, it would use it first
  • If not, it would try to look for another system property called "my.env", and then load the matching file from the classpath
  • finally, if the above was not found, it would try to look for a file at "conf/my.properties" on the classpath

It seems that the system properties used in the @sources cannot be optional... And that's truly annoying!

Please let me know!

Cheers

@lviggiano

This comment has been minimized.

Owner

lviggiano commented Jul 31, 2015

Hello @ant1g

I had a look at it, and I am sorry you are experiencing this problem, since my goal is to make sure that everything that works with a prior version still works with the updates. But this is not always easy to predict and sometimes this promise has to be broken in order to let things evolve without too many chains.

In version 1.0.7 and 1.0.8 internal implementation of owner has been changed in order to use URI instead of URL.

Why we changed this? Because URI are more flexible than URL in order to have user specific resource identifiers.
For intance you can create an URI with new URI("myprotocol:myresource") and Java will not try to find a protocol handler and just accept that, while if you do the same with URL you'll get a MalformedURLException. This difference between URI and URL implementation in Java made me opt to change internals of OWNER to switch from URL to URI: see pull request #70.
So, for instance, a user can define a loader who accepts an URI like "myprotocol:myresource" and implement a loader that parses the URI and load the properties without specifying an URLStreamHandler and having to configure it inside the jdk to automatically handle all URL specified with that protocol (possibly impacting the whole running jvm and interfering with other running code).
So this was a big advantage and simplification and URL->URI change is an advantage that I am not thinking to rollback.

The problem with URI is that it looks like it doesn't accept chars { and } while URL does accept that. Why these two classes have this difference is unknown to me, anyway a reason why URI is not accepting the curly braces can be found here.

This is demonstrated if you try to run this code (committed here):

String spec = "file:${my.properties}";

System.out.println("The parsed URL is: " + new URL(spec)); // this will run fine
System.out.println("The parsed URI is: " + new URI(spec)); // this will run throw the URISyntaxException

So when you don't specify the my.properties the string "file:${my.properties}" is not expanded and remain the same, so OWNER 1.0.5.1 would use an URL internally to try to load a file called ${my.properties} and not finding it would try to load subsequent files. While instead OWNER 1.0.9 would fail to parse the URI since java.net.URI parsing is apparently more strict about safe chars than java.net.URL.

So there are two possible solutions to solve your problem.

  • I may need to implement a change in owner to allow variable expansion using some different chars, other than "${" and "}" as placeholder for variables. But this will require a new release, and this my require some time depending on my available spare time that I can dedicate to the development of this library. And I am not able to give you some time estimation on when this would be ready. But this will still need you to change the annotation with the new placeholders for the variables.
  • You can implement a work around, something like this:
if (System.getProperty("my.properties") == null) System.setProperty("my.properties", "/dev/dummy/unexistent");

if (System.getenv().get("my.env") == null) System.setProperty("my.env", "/dev/dummy/unexistent");

MyProperties props = ConfigFactory.create(MyProperties.class);

this should solve your problem since /dev/dummy/unexistent will likely not exist on any machine your code would run on. This is also a safer solution, since I am thinking that the fact that owner does not expands the unset variables may not have been the best choice, and in future this may change to expanding to empty strings as it works for unix variable expannsion in bash shell for instance. But even in this case, the workaround I exposed above would produce the best results, since a value will always be specified as a default value.

So, I would close this issue since a work around is possible and from my point of view, it is the best solution available for the problem.

Let me know what do you think or if you have any suggestion.

Thanks and have great summer holidays.

Luigi.

@ant1g

This comment has been minimized.

ant1g commented Aug 14, 2015

Hi Luigi,

Thanks for your reply! I was away for some time.
I am gonna have a deeper look at the issue. If I find an elegant way to fix or work around the problem, I'll submit a pull request so that you can have a look!

I understand your proposed solution, but I am always against adding unnecessary code in the client...

Please keep the issue open for a little while, I'll come back to you soon.

Best regards!

@lviggiano

This comment has been minimized.

Owner

lviggiano commented Aug 14, 2015

Ok Thanks.

@ant1g

This comment has been minimized.

ant1g commented Sep 9, 2015

Hi Luigi,
Would it be an option to simply not re-throw the URISyntaxException and just ignore it?
It would then get on to the next @source and try to load it.
I really believe that this overloading mechanism (in order) is nice, and I must definitely not be the only one wanting that!
This is actually a core feature to me!
I can imagine that it would be annoying for someone to have to supply a system property pointing to a config file, when you could just not provide it at all and use all default values instead (for unit testing for instance!)
An addition could be to log the @sources that were not successfully loaded so that the user can know what is actually being loaded.

Let me know what you think!

Cheers!

@ant1g

This comment has been minimized.

ant1g commented Sep 28, 2015

By the way, I found something else a bit annoying with the URIs, if you use a windows path as a system property when starting your app, it will fail with the same exception.
For instance you give a valid quoted windows path like this:

-Dmy.properties="C:\ProgramData\My Program\my.properties"

I will throw a "java.net.URISyntaxException: Illegal character in opaque part at index..." (where the space is...)

Reverting to 1.0.5.1 (with URLs) makes it work again.
To me this is another blocker, since I don't want to perform a windows specific escaping routine to update the system property...!

Let me know what do you think of this, perhaps you already have a solution for that.
Note: I can't change the way the system property is given, so I can't escape the spaces beforehand.

@averst69

This comment has been minimized.

averst69 commented Oct 2, 2015

I reverted to version 1.0.6 and it worked again

@QuantumGestalt

This comment has been minimized.

QuantumGestalt commented Nov 24, 2015

Just investigating OWNER for use in my own project, when I came across this thread.

Thought I'd chime in to point out that URIs are Encoded. See the introductory message in the API doc https://docs.oracle.com/javase/8/docs/api/java/net/URL.html or even http://docs.oracle.com/javase/1.5.0/docs/api/java/net/URL.html).

@lviggiano

This comment has been minimized.

Owner

lviggiano commented Nov 24, 2015

Hi. If someone wants to improve this part, it would be welcome. Just note that we need URIs, not just URLs.
And please note that at the moment I am closed into a hospital for important health issues, so I cannot really look into the code for some time to merge the improvements.

@smgonzales

This comment has been minimized.

smgonzales commented Mar 30, 2016

I also ran across the problem of having spaces in the file path. The fix from icirellik should resolved it when it's available. To get around it until a fix is available I replaced spaces with %20. The URI will correctly decode the file path and load the properties as expected.

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