Fix URI template expansion

[andreaTP]

Fix #165

I have attempted to fix it using the original uritemplate library, but I failed having more and more issues and bugs.

I found this one: https://github.com/damnhandy/Handy-URI-Templates
The repo looks slightly better(more stars at least) and it passes the TCK.

The only "update" needed for this project is the usage of Java standard library DateTimes instead of joda

[baywet]

Thanks for the contribution.
As per the datetime comment, they have a PR pending for a while here
I've also noticed they haven't been merging dependabot pull requests for a while now.
And the lib author/only maintainer has had very few contribs in 2022
I'd be in favor of finding a better alternative, if one exists.

[baywet]

Suggested change

	implementation 'com.damnhandy:handy-uri-templates:2.1.7'
	implementation 'com.damnhandy:handy-uri-templates:2.1.8'

[baywet]

In addition, the latest version has multiple transitive vulnerabilities being reported.
If we take a dependency on it, we'll get a bunch of compliance issues on our end.
So unless we can get the maintainer to update their dependencies, I don't think we'll be able to merge this PR.
https://mvnrepository.com/artifact/com.damnhandy/handy-uri-templates/2.1.8

[andreaTP]

@baywet thanks a ton for looking into this.
I know is suboptimal, but this is my journey with hal4j:uritemplate:

• was not substituting queryParams and was discarding those
• despite the params being marked with * if any of them is not matched none is expanded
• subsequent expand are not producing the expected outcome
• building the URLBuilder and applying the query parameters works only in specific cases

The only "alternative" and self-contained library I found so far is this:
https://github.com/eclipse-vertx/vertx-uri-template

Another option would be to directly maintain in this codebase a copy of:
https://github.com/micronaut-projects/micronaut-core/blob/4.0.x/http/src/main/java/io/micronaut/http/uri/UriTemplate.java

which will bring the needed capabilities without the maintenance issues.
WDYT?

[baywet]

Copying code: I'm always warry of doing that, it introduced an informal fork, which is hard to keep in sync. Also depending on the license it might be "legally impossible", etc...

hal4j:uritemplate: happy to move away from this one if it's not properly implemented and if the community behind it is not reactive enough to support bugfixes.

vertx-uri-template: This one looks clean and active, and we could probably send a few PRs their way, e.g. a dependabot configuration.

[andreaTP]

@baywet , argh, I've just pushed another proposal, let's discuss it:

• License Apache -> keeping the original header and referring to the source of it should be enough
• the implementation is completely self-contained and mostly untouched for 4+ years
• the part we are missing is Beans expansion that, AFAIU this codebase, we are not using.

I'm worried about depending on vertx-uri-template as the bus factor there seems to be === 1 😞
No good options sadly ...

[baywet]

As another alternative, would Red Hat be open to stand-up a library and commit to maintaining it long term?

[andreaTP]

As another alternative, would Red Hat be open to stand-up a library and commit to maintaining it long term?

This is possible too ... let me do a couple of checks.

[andreaTP]

@baywet I checked again the possible alternatives:

• hal4j:uritemplate -> buggy implementation
• damnhandy:handy-uri-templates -> not maintained
• vertx-uri-template -> brings vertx-core on the classpath which is not desirable especially at the abstractions layer
• Add an implementation to something like: https://github.com/andreaTP/kiota-utils which will go "official" soon enough in the redhat-developer org -> this is doable, but it starts to create a cyclic dependency in between the 2 repos
• Spin a separate library under redhat-developer to maintain a standalone implementation like the one from micronaut -> at this point having a copy of the source code here is still better as it spares one dependency and dramatically decrease the burden in maintenance

I think that having a copy of one implementation in this repo is probably the least horrible alternative at this point, wdyt?

[baywet]

In that case the copy is probably the least bad solution.

[andreaTP]

@baywet happy to consider other alternatives, but, after having spent my day with this, I do believe that this is my preferred solution in between the alternatives 😅 .

I have added a little infrastructure to ease the testing, made built-in support for OffsetDateTime and tweaked things to be consistent.

If we decide to go ahead with this implementation, I need to ask if we can have a release after the merge.

[andreaTP]

ok, let me fix the Android linting issues before calling it a day ... 😅

[baywet]

yep we can have a minor, and you can update the changelog and the properties to prepare for that if you want (otherwise I'll do it as a separate PR).
We had a defect reported by the android lint workflow but I see you've just fixed that while I was writing my comment.

[baywet]

actually the defect is now reported at a different place.

[andreaTP]

should be good to go now, prepared the changelog, thanks for bearing with me!

[baywet]

Thanks for making the changes

[andreaTP]

Thanks a ton for the quick release @baywet ! I can confirm that now it works as expected 🙂