On 9/6/2012 4:05 PM, Sergey Beryozkin wrote:
> 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...
>
You don't want to add resolveTemplate() methods in UriBuilder? I
disagree. I've needed those methods for a long time. Sometimes you
want to partially complete a template and pass on the resulting UriBuilder.
--
Bill Burke
JBoss, a division of Red Hat
http://bill.burkecentral.com