users@jax-rs-spec.java.net

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

From: Marek Potociar <marek.potociar_at_oracle.com>
Date: Tue, 18 Sep 2012 16:09:33 +0200

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?

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