users@jax-rs-spec.java.net

[jax-rs-spec users] [jsr339-experts] Re: Re: HEADS-UP: Encoding values of UriBuilder template parameters

From: Sergey Beryozkin <sberyozkin_at_talend.com>
Date: Fri, 23 Dec 2011 16:14:29 +0000

On 23/12/11 16:09, Markus KARG wrote:
> Given the fact that existing code has a method annotated with
> @Path("foo/{id}") and I want to get "foo/a%2Fb%2Fc" when calling
> .path(Class, String).build("a/b/c"), what would be your solution? I mean,
> you just criticize Marek's proposal, but what would you propose instead?

I'm always trying my best to offer something in alternative, and never
just keep saying does not work :-)

> Adding segment(Class, String)? Using a different synax like
"foo/{+id}" as
> Julian pointed to?

"foo/{+id}" seems elegant, but segment(Class, String) can be good too;
Overload ? May be, but I'd review then what should segment() and query()
report when they accept template vars with '/' and build() initiating
the final step with 'false'.

Let Marek decide :-)

Sergey


>
>> -----Original Message-----
>> From: Sergey Beryozkin [mailto:sberyozkin_at_talend.com]
>> Sent: Freitag, 23. Dezember 2011 17:02
>> To: jsr339-experts_at_jax-rs-spec.java.net
>> Subject: [jsr339-experts] Re: [jax-rs-spec users] Re: HEADS-UP:
>> Encoding values of UriBuilder template parameters
>>
>> I was nearly ready to suggest that may be we were coming to the
>> conclusion that segment() can become deprecated because
>>
>> path() + new build overload will just cover this case.
>>
>> Or we'd say that segment("{id}") and the false means an illegal state
>> exception or something because that would effectively break the
>> segment() contract otherwise
>>
>> But we also can get template vars in query(...) so...
>> Sergey
>>
>> On 23/12/11 12:59, Sergey Beryozkin wrote:
>>> I'm wondering if the only way not to get 'injured' in this thread is
>>> to just say "yes, yes, yes". Come on guys, show the patience, please
>> !
>>> We all have to be able to repeat the same thing easily without going
>>> to capital letters or other pokes. Are you going to EXPLAIN it like
>>> this to the users too ?
>>>
>>> More comments below
>>>
>>> On 23/12/11 12:43, Markus KARG wrote:
>>>>> The situation with pm,entath(Class) is completely different ? The
>>>>> case where we are reading from the legacy DB is about submitting
>> the
>>>>> values we do not control to UriBuilder. We are in control when
>>>>> annotating the resource classes.
>>>>
>>>> No that exactly is the case we talk about: The annotation is given
>> by
>>>> the programmer and it provides the path template given to .path().
>>>> But the value provided to .build() is taken form a database and must
>>>> get encoded ALWAYS.
>>>> That is what we discuss in this thread: The fact that slashes taken
>>>> from databases and given to .build() are NOT encoded currently but
>>>> MUST get encoded ALWAYS as those NEVER are intended to be path
>>>> separators but ALWAYS are intended to be DATA.
>>>
>>> And I'm saying that passing such values indirectly via templates
>>> associated with path() is wrong in the first place and they should be
>>> given to segment() and segment() java docs need to be fixed to make
>>> sure the user understands that irrespectivelt of the way you pass the
>>> data to
>>> segment() you get the same consistent result, same way you get it
>> with
>>> path().
>>>
>>>>
>>>>>> (c) As described earlier in depth, segment is defined in the RFC
>> as
>>>>>> a part of the path element (not as something different to path),
>> so
>>>>>> without improved JavaDocs people will not assume to find different
>>>>>> encoding rules for .path() and .segment(). It is counterintuitive
>>>>> that
>>>>>> it is up to the
>>>>>> *declaration* method how the encoding works like, as people are
>>>>>> used to other templating mechanisms like JDBC where this plays no
>>>>>> role (it ALWAYS encodes data, even if not necessary, as people
>>>>>> could fill in unencoded values by .path(variable) instead of
>>>>>> .path("{x}").build(variable) -- which is much more
>> straightforward).
>>>>>>
>>>>>>> 2. What happens with builder.segment("{bar}").path("{foo}") and a
>>>>>>> value 'true' passed to a new overload, where a "{bar}"
>>>>>>> substitution will also contain "/" characters. I feel at the
>>>>>>> moment that given that both
>>>>>>> segment() and path() operate on the same URI component this extra
>>>>>>> flag will confuse people like me at least a lot
>>>>>>
>>>>>> As 'true' means 'always encode all values' it will enforce
>> complete
>>>>>> encoding. You will get %2F for each slash in both, the values for
>>>>>> bar and foo.
>>>>>>
>>>>>> Rule: As long as you pass 'true' to the new overload, you can
>>>>>> simply pass variables to .segment() and .path() to get slashes
>>>>>> ALWAYS, and simply pass variables to .build() to get %2F ALWAYS.
>>>>>> This rule is
>>>>> much
>>>>>> simpler to understand than the current (default, 'false')
>> behaviour.
>>>>>> Nothing to think really. Just pass 'true' and decide whether you
>>>>>> want slashs (--> .segment(x), .path(x)) or your want %2F -->
>> .build(x)).
>>>>>>
>>>>> I guess I'm really slow,
>>>>>
>>>>> What happens if I do segment("a/x/c") and pass 'false' ? Guess we
>>>>> should get "a/x/c" as this boolean applies to template vars, right
>> ?
>>>>
>>>> YES, as the new boolean only decides what to do with VALUES provided
>>>> to .BUILD(). LITERALS provided to .SEGMENT() or .PATH() are OUT OF
>>>> SCOPE of this thread.
>>>>
>>>>> How about
>>>>> segment("{x}") and false, should not we get "a%2Fx%2Fc" ?
>>>>
>>>> Marek, your new parameter, your business, your turn. ;-)
>>>
>>> What is your take on it ?
>>>
>>>
>>>>
>>>>>> Hope that you see the simplicity in this new overload?
>>>>>>
>>>>>
>>>>> I see better why you and Marek see it improving the situation. I
>>>>> won't rush with my final conclusion this time, but so far what I
>> see
>>>>> is that when we have a case where we do path("{a}") and seeing the
>>>>> substitution value (say a key read from the legacy data source and
>>>>> similar) still containing "/" then we are simply doing a wrong
>> code,
>>>>> we should be doing
>>>>>
>>>>> segment("{a}")
>>>>
>>>> BUT YOU CANNOT IF THE TEMPLATE IS TAKEN FROM SOMEWHERE ELESE AS ONE
>>>> COMPLETE PATH STRING, E. G. FROM EXTERNAL CONFIGURATION OR FROM
>>>> @PATH, AS THERE IS NEITHER SEGMENT(STRING) WHICH WILL SPLIT INTO
>>>> SEGMENTS (AS IT THINKS IT IS A SINGLE SEGMENT), NOR THERE IS
>>>> SEGMENT(CLASS) NOR SEGMENT(CLASS, STRING). GOT THE PROBLEM NOW?
>>>>
>>>
>>> But that is what you want, right ? You want '/' in "a/b/c" encoded
>> and
>>> this is what segment() needs to be able to do
>>>
>>>> Don't know how often I explained already on this list... ;-(
>>>>
>>>> Regards
>>>> Markus
>>>>
>>>
>>>
>>
>>
>> --
>> Sergey Beryozkin
>>
>> Talend Community Coders
>> http://coders.talend.com/
>>
>> Blog: http://sberyozkin.blogspot.com
>


-- 
Sergey Beryozkin
Talend Community Coders
http://coders.talend.com/
Blog: http://sberyozkin.blogspot.com