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

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

From: Sergey Beryozkin <sberyozkin_at_talend.com>
Date: Tue, 18 Sep 2012 15:25:07 +0100

On 18/09/12 15:09, Marek Potociar wrote:
>
> On Sep 17, 2012, at 6:36 PM, Sergey Beryozkin<sberyozkin_at_talend.com> wrote:
>
>> On 17/09/12 17:04, Marek Potociar wrote:
>>> FYI, after allowing more time for a discussion, I've implemented the change:
>>>
>>> http://java.net/projects/jax-rs-spec/sources/git/revision/31cad61236c5dfadefcdd070c5c81ed55306cc4a
>>
>> What should happen in build()/etc (where template var values are provided) after resolveTemplates has been called ? IllegalStateException ?
>
> resolveTemplate does an irreversible change. IOW, the resolved template is no longer a template. So the build() should behave the same way it would behave if you passed in a value for a non-existent template.
>
> IOW:
>
> UriBuilder.fromPath("{a}/{b}").resolveTemplate("a", "x") == UriBuilder.fromPath("x/{b}");
>
> Makes sense?

It does. I'm not thinking this is what users want (well, some may do but
not sure about users at large), having a capability to do a partial
resolution as it makes build()/etc more error-prone to use.

On the positive side, from the diff, I can see only two methods added -
that looks reasonable. Extending UriBuilder interface even further may
not be ideal IMHO

Cheers, Sergey

>
> I may want to add the example to javadoc.
>
> Marek
>
>>
>> Cheers. Sergey
>>
>>>
>>> Thanks,
>>> Marek
>>>
>>> On Sep 7, 2012, at 5:51 PM, Markus KARG<markus_at_headcrashing.eu
>>> <mailto:markus_at_headcrashing.eu>> wrote:
>>>
>>>> Interesting use case, but never had the need for it. So I'd abstain from a
>>>> vote on this. As it seems, the count is 2+ and 1- and 1 abstain.
>>>>
>>>>> -----Original Message-----
>>>>> From: Bill Burke [mailto:bburke@redhat.com<http://redhat.com>]
>>>>> Sent: Freitag, 7. September 2012 15:11
>>>>> To: jsr339-experts_at_jax-rs-spec.java.net
>>>>> <mailto:jsr339-experts_at_jax-rs-spec.java.net>
>>>>> Subject: [jsr339-experts] Re: [jax-rs-spec users] Re: PLEASE READ:
>>>>> Resolving UriBuilder / WebTarget templates
>>>>>
>>>>>
>>>>>
>>>>> 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
>>>>>>> <mailto: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
>>>>
>>>
>