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.
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
>>