jsr339-experts@jax-rs-spec.java.net

[jsr339-experts] Re: [jax-rs-spec users] Re: PLEASE READ: Resolving UriBuilder / WebTarget templates

From: Sergey Beryozkin <sberyozkin_at_talend.com>
Date: Thu, 6 Sep 2012 21:05:58 +0100

On 06/09/12 18:24, Marek Potociar wrote:
>
> On Sep 6, 2012, at 6:10 PM, Sergey Beryozkin<sberyozkin_at_talend.com> wrote:
>
>> On 06/09/12 15:09, Marek Potociar wrote:
>>> Hello experts,
>>>
>>> I know it's a long email, but please bear with me. I'd like to propose
>>> some changes to the WebTarget and UriBuilder API. Following email
>>> provides the rationale as well as the proposal.
>>>
>>> Currently we have following methods in the WebTarget:
>>>
>>> pathParam(String name, Object value)
>>> pathParams(Map<String, Object> parameters)
>>>
>>> Their purpose is to set values to path template variables. E.g.:
>>>
>>> client.target("{a}/{b}").pathParam("a", "x") --> "x/{b}"
>>>
>>> Also, we have a couple of requests in Jira to provide similar
>>> functionality in UriBuilder as well as in general resolve the
>>> inconsistencies between WebTarget and UriBuilder URI building API:
>>>
>>> http://java.net/jira/browse/JAX_RS_SPEC-211
>>> http://java.net/jira/browse/JAX_RS_SPEC-163
>>>
>>> Also there is a Jersey issue filed around inconsistencies/confusion of
>>> what WebTarget.getUriBuilder() should return:
>>>
>>> http://java.net/jira/browse/JERSEY-1329
>>>
>>> So while looking at the WebTarget/UriBuilder consolidation from the
>>> larger perspective provided by all these issues, I'd like to propose
>>> some changes in these APIs.
>>>
>>> First of all,WebTarget.pathParam(s) are misleading method names, since
>>> the actual intended purpose of the methods was to resolve a template
>>> variable to a value. IOW the expected behavior is:
>>>
>>> client.target("{a}").path("{b}").queryParam("a", "{a}").pathParam("a",
>>> "x") --> "x/{b}?a=x"
>>>
>>> Now I think you're with me, when I say that the name of the pathParam
>>> method is misleading. Another issue, perhaps even more interesting is
>>> finding the right answer for a question on what should
>>> WebTarget.getUriBuilder() return - how should the returned UriBuilder be
>>> initialized. For example:
>>>
>>> client.target("{a}/{b}").pathParam("a", "x").getUriBuilder()
>>>
>>> In this case a few answers are possible for the internal state of the
>>> returned UriBuilder:
>>>
>>> 1. URI template: "{a}/{b}", internal template values: N/A
>>> 2. URI template: "{a}/{b}", internal template values: "a" -> "x"
>>> 3. URI template: "x/{b}", internal template values: N/A
>>>
>>>
>>> Now after a careful consideration, I'd argue that we should take
>>> approach that corresponds to the answer #3 (even though it may not seem
>>> to be the most intuitive answer). IOW, I could not find any practical
>>> use case that would support the other 2 options and could not be
>>> solvable by 3, but I could find use cases that would only be solvable by
>>> #3. One other indication might be that it's hard to find a use case
>>> where following makes sense and could be considered as a good practice:
>>>
>>> client.target("{a}/{b}").pathParam("a", "x")...pathParam("a", "y")
>>>
>>> So my conclusion is that typically you only set a template value only
>>> once and once you set it you do want to overwrite it later. E.g.:
>>>
>>> authors = client.target("bookstore/authors/{name}");
>>> author = authors.pathParam("name", "Dan Ariely");
>>>
>>> ... /* somewhere in a different part of app */ ...
>>>
>>> books = author.path("books/{name}");
>>> book = books.pathParam("name", "Predictably Irrational");
>>>
>>> In the code above the issue with overwriting template values set before
>>> is obvious. You certainly do not want to overwrite the author's name
>>> with the name of the book.
>>>
>>> So finally, here comes my proposal:
>>>
>>> * remove WebTarget.pathParam(s) methods
>>> * introduce new WebTarget.resolveTemplate(s) methods, including method
>>> versions that provide flag for "/" encoding in path
>>> * introduce new UriBuilder.resolveTemplate(s) methods, including
>>> method versions that provide flag for "/" encoding in path
>>>
>>>
>>> The new resolveTemplate(s) methods would *irreversibly* resolve the
>>> template values in WebTarget or UriBuilder for given template variables,
>>> e.g.:
>>>
>>> client.target("{a}/{b}").resolveTemplate("a", "x").resolveTemplate("a",
>>> "y") --> "x/{b}"
>>> // resolveTemplate("a", "y") above is either ignored or throws
>>> IllegalStateException
>>>
>>
>> Can having 'irreversible' effect help users who work directly with UriBuilder ?
>
> Yes. Similar example as above could apply to UriBuilder as well.
>
>> Can that be introduced only at WebTarget level ?
>
> That would be inconsistent. What would be the benefit of not introducing the methods in UriBuilder? See the issues filed in Jira that ask for them.
>

I do not see the benefit of introducing it in UriBuilder. It has so many
methods and they can do all the replacing as needed.

By the way,

http://java.net/jira/browse/JERSEY-1329

has nothing to do with these proposed methods - this is an issue of the
wrongly initialized UriBuilder.

You are asking what would be the benefit of not introducing them in
UriBuilder ? Keeping the complex UriBuilder interface to the existing
'minimum', avoiding adding 4+ methods which are assumed without serious
justification, are required.

By the way, agree with Markus. 2 days before PR, no time to properly
discuss or have any reasonable analysis...

Sergey

> Marek
>
>>
>> Thanks, Sergey
>>
>>> This proposal should make things consistent. Please provide your
>>> feedback ASAP. I'd like to make this change before the PR release
>>> planned for next Monday.
>>>
>>> Thank you,
>>> Marek
>>>
>