users@jax-rs-spec.java.net

[jax-rs-spec users] [jsr339-experts] Re: Request interface duplicates MBRs, HttpHeaders and UriInfo

From: Sergey Beryozkin <sberyozkin_at_talend.com>
Date: Tue, 20 Dec 2011 14:27:16 +0000

Comments on the JIRA actions:

On 19/12/11 22:05, Sergey Beryozkin wrote:
> For whatever it's worth:
> http://java.net/jira/browse/JAX_RS_SPEC-153

OK

> http://java.net/jira/browse/JAX_RS_SPEC-154

Agreed

> http://java.net/jira/browse/JAX_RS_SPEC-155
>

Sigh...Does anyone else in this group has an opinion re JAX_RS_SPEC-155
(duplicate interfaces representing the request headers) ?

Sergey


>
> On 15/12/11 22:17, Sergey Beryozkin wrote:
>> Marek,
>>
>> this has been one of those deeply-threaded and a bit emotional threads
>> which I'm quite happy to acknowledge we've already had a few of, well
>> done us :-)...
>>
>> Now let me step back a bit and provide my final recommendation which
>> I've thought about during the short break.
>>
>> 1. Headers
>>
>> I strongly encourage you to reconsider the introduction of
>> RequestHeaders for the following reasons
>> 1.1 Your argument about possibly breaking the user code which extends
>> HttpHeaders while passing it to subresources is not strong enough to
>> 'protect' RequestHeaders. The same can be said about users extending
>> Request, Response (to deal with them in custom response filters), and
>> UriInfo.
>> 1.2 The strategy of introducing a new interface and deprecating the old
>> one limits a lot our scope for adding further interesting methods to
>> (Request)HttpHeaders, UriInfo, Request itself. Surely we won't do
>> RequestHeaders2 in JAX-RS 3.0 because of the concern that someone may've
>> extended RequestHeaders. I recommend to document in JAX-RS 2.0 API that
>> Request, HttpHeadequers, UriInfo and Response are not to be extended.
>> This will give us a nice window to add more interesting methods, this is
>> not JDK so we can afford it; and the worst that will happen to users who
>> ignore this advice is that they will need to recomile when moving to a
>> new major release.
>> 1.3 Having two mostly duplicate interfaces without one of them even
>> being deprecated is not a good idea I'm afraid
>>
>> I think you should have
>> Request.getRequestHeaders():HttpHeaders
>>
>> 2. UriInfo methods duplication
>>
>> Please 'partially' get rid of them, meaning that you only have
>>
>> Request.getUriIno():UriInfo
>>
>> 3. Input Stream/entity getters:
>>
>> 3.1 Keep them in; if possible - consider introducing RequestBody
>> interface (optional - yes 1 extra interface) which encapsulates all
>> those getters
>> 3.2. Offer an example in the spec showing how Request can be used on its
>> own to get to headers, uri info and the body, for example when having a
>> method handling diff xml payloads:
>> @POST
>> public void process(@Context Request req) {
>> req.getHeaders();
>> req.getUriInfo();
>> if (something) {
>> req.getEntity(Book.class)
>> } else {
>> req.getEntity(Book2.class)
>>
>> }
>> }
>>
>> So:
>>
>> interface Request {
>> HttpHeaders getRequestHeaders();
>> UriInfo getUriInfo();
>> // IO getters or
>> RequestBody getBody();
>> }
>>
>> So I support your approach more now but I encourage to take some more
>> refactoring on it
>>
>> Think about it
>> Sergey
>>
>> On 15/12/11 18:21, Marek Potociar wrote:
>>>
>>>
>>> On Thu 15 Dec 2011 06:18:39 PM CET, Sergey Beryozkin wrote:
>>>> On 15/12/11 16:45, Marek Potociar wrote:
>>>>>
>>>>>
>>>>> On 12/15/2011 01:42 PM, Sergey Beryozkin wrote:
>>>>>> Hi
>>>>>> On 14/12/11 17:58, Marek Potociar wrote:
>>>>>>>
>>>>>>>
>>>>>>> On Wed 14 Dec 2011 05:25:58 PM CET, Sergey Beryozkin wrote:
>>>>>>>> On 14/12/11 13:37, Marek Potociar wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Wed 14 Dec 2011 11:47:47 AM CET, Sergey Beryozkin wrote:
>>>>>>>>>> Hi
>>>>>>>>>>
>>>>>>>>>> I've had a closer look at the updated Request interface and
>>>>>>>>>> IMHO it's grown into a uber-interface that exposes
>>>>>>>>>> much of
>>>>>>>>>> the imformation which is already available in other interfaces
>>>>>>>>>> which can be injected into the application code and
>>>>>>>>>> also exposes the methods that the application code should not
>>>>>>>>>> even see.
>>>>>>>>>>
>>>>>>>>>> 1. HttpHeaders is there to provide the info about the current
>>>>>>>>>> headers;
>>>>>>>>>> Request offers RequestHeaders which mainly duplicates what
>>>>>>>>>> HttpHeaders can offer; would it make sense to get
>>>>>>>>>> Request
>>>>>>>>>> updated to return HttpHeaders instead and may be just drop
>>>>>>>>>> RequestHeaders;
>>>>>>>>>
>>>>>>>>> HttpHeaders are incomplete in terms of type-safe support of
>>>>>>>>> request headers. However, they are too request
>>>>>>>>> specific to
>>>>>>>>> be reusable in the response. Thus the new separate set of
>>>>>>>>> interfaces was designed with the intent to make
>>>>>>>>> HttpHeaders
>>>>>>>>> deprecated (via javadoc wording) over time.
>>>>>>>>>
>>>>>>>>
>>>>>>>> Why did you decide to avoid adding extra methods to HttpHeaders
>>>>>>>> and retaining them for the purpose of representing
>>>>>>>> the
>>>>>>>> request headers ? Adding new methods should not break the legacy
>>>>>>>> client code and we'll have one less interface,
>>>>>>>> instead of keeping 2 mostly duplicate interfaces around for years ?
>>>>>>>
>>>>>>> Please understand that we want to keep the amount of potential BW
>>>>>>> incompatibilities to an absolute minimum.
>>>>>>
>>>>>> This reminds me of the similar discussion we had about FilterContext.
>>>>>> Users do not implement HttpHeaders, right ? What BW compatibility
>>>>>> issues do you see users hitting in case of
>>>>>> HttpHeaders
>>>>>> being updated further ?
>>>>>
>>>>> That's what we are not fully sure. We have some indications that
>>>>> users may have actually extended HttpHeaders to provide
>>>>> additional type-safe header accessors.
>>>>>
>>>>>>
>>>>>>> In case of
>>>>>>> Request/Response the extension greatly outweighed the potential
>>>>>>> drawbacks of the interface extension. In case of
>>>>>>> HttpHeaders though, extending the interface does not bring enough
>>>>>>> value to be justified, esp. as the extension would
>>>>>>> result in an asymmetric API (HttpHeaders on request side and
>>>>>>> ResponseHeaders on the response side)
>>>>>>
>>>>>> is this highly cosmetic drawback outweighed the decision in favor
>>>>>> of RequestHeaders ? JAX-RS users understand that
>>>>>> HttpHeaders represent the request headers. Having ResponseHeaders&
>>>>>> HttpHeaders does not read the pure way but
>>>>>> ResponseHttpHeaders& HttpHeaders would be better and again it would
>>>>>> save us one extra interface.
>>>>>
>>>>> The "highly cosmetic" drawback does matter to me, but it's just one
>>>>> piece of the puzzle.
>>>>>
>>>>
>>>> You used the word 'especially' in the previous comment didn't you ?
>>>> Now you say it does not matter to you, difficult
>>>> to follow
>>>>
>>>>>>
>>>>>> You are thinking of RequestHeaders replacing HttpHeaders and we
>>>>>> know what it means in the Java land; besides as a
>>>>>> replacement it will have to be injectable too alongside
>>>>>> HttpHeaders; the idea of answering to CXF users why we have
>>>>>> HttpHeaders& RequestHeaders and when we should use either of those
>>>>>> does not encourage me at all.
>>>>>
>>>>> Yes, HttpHeaders will still be supported and injectable. The
>>>>> provided javadoc will also clearly state that HttpHeaders
>>>>> is a stale artefact from JAX-RS 1.x days and new users should avoid
>>>>> using it in favor of RequestHeaders.
>>>>
>>>> Will you mark it as deprecated
>>>
>>> No.
>>>
>>>> and update the spec to make sure RequestHeaders are to be supported
>>>> as @Context ?
>>>
>>> Yes.
>>>
>>>>
>>>>> That should
>>>>> hopefully limit the number of CXF users asking these types of
>>>>> questions (at least the ones that occasionally read
>>>>> javadoc).
>>>>>
>>>> I'm disappointed to hear it from a spec lead - I don't understand why
>>>> you think you can even afford such comments.
>>>>
>>>>>>
>>>>>> What exactly does this
>>>>>> http://jax-rs-spec.java.net/nonav/2.0/apidocs/javax/ws/rs/core/RequestHeaders.html
>>>>>>
>>>>>>
>>>>>>
>>>>>> adds ?
>>>>>>
>>>>>> getContentLength() is useful, just add to HttpHeaders;
>>>>>
>>>>> I hope with my previous explanation it is now clear why we don't
>>>>> want to break BW for HttpHeaders. What we could do is
>>>>> to make RequestHeaders extend HttpHeaders, but I don't see much
>>>>> value in it.
>>>>>
>>>>
>>>> There's nothing in your previous comments. The example with
>>>> subresources that follows is the only one which can
>>>> justify this contribution to the interfaces bloat.
>>>>
>>>>>> by the way, getAllowMethods() - is is supposed to be on the
>>>>>> ResponseHeaders instead ?
>>>>>
>>>>> As an expert homework, kindly search through the HTTP spec or browse
>>>>> through this EG mailing list archive to find the
>>>>> answer and come back with it ;)
>>>>
>>>> Yes I checked. Are you using this "MAY" bit: "The Allow header field
>>>> MAY be provided with a PUT request to recommend
>>>> the methods to be supported by the new or modified resource.". Does
>>>> it deserve a dedicated method ? What is your
>>>> target audience ? What do you expect the JAX-RS server implementation
>>>> do ? We have 4 major HTTP verbs, that MAY thing
>>>> talks about 1 verb, see what I mean ?
>>>
>>> No I don't. Do you want me to remove the method, because the HTTP spec
>>> does say the Allow is a mandatory request header?
>>> Allow is a general-purpose header. It surely is more frequent in the
>>> response, but if we have it in the response, we
>>> should also have it on the request, as it is a general-purpose header.
>>>
>>>>>>
>>>>>> IMHO we should try really hard now to get the number of interfaces
>>>>>> to the absolute bare minimum...
>>>>>
>>>>> I can assure you that I already noted your (and Bills) position down
>>>>> and whenever I am to propose any new
>>>>> interface/class I do very carefully consider if I am truly convinced
>>>>> that the new addition is really-really useful or
>>>>> needed as I am perfectly aware of the reaction I would receive from
>>>>> you guys if it was not the case. In fact - have a
>>>>> look at latest changes. For example, the
>>>>> request/response+headers+builder API was recently flattened and 4
>>>>> interfaces
>>>>> have been removed (Headers, Headers.Builder, RequestHeaders.Builder,
>>>>> ResponseHeaders.Builder).
>>>>>
>>>>
>>>> I know you are putting a lot of effort into it, I honestly do; but
>>>> I'd appreciate getting some convincing arguments
>>>> rather than what I'm reading now
>>>>
>>>>>>
>>>>>>> as well as we are far
>>>>>>> less sure that people are not extending this interface (e.g. to
>>>>>>> provide extra type-safe http getters).
>>>>>>>
>>>>>> this is a speculation; I've never ever heard of anyone attempting
>>>>>> to implement HttpHeaders - and it would simply would
>>>>>> not work anyway because it is the runtime that drives the injection
>>>>>> of HttpHeaders impls with JAX-RS 1.1 saying nothing
>>>>>> about checking for custom HttpHeaders impls.
>>>>>
>>>>> You can pass your own extended implementation wrapping the injected
>>>>> instance into sub resources.
>>>>>
>>>>
>>>> Can you give me a favor and provide a simple example, what is missing
>>>> from HttpHeaders, String is returned in case
>>>> headers shipping int values, just curious
>>>
>>> I am sure you can do such a diff on your own. I am already spending
>>> quite some time writing all these replies.
>>>
>>>>>>
>>>>>>>>>>
>>>>>>>>>> 2. UriInfo offers all the methods needed to get to the current
>>>>>>>>>> request/base/absolute uri, Request duplicates 5-8
>>>>>>>>>> methods from UriInfo
>>>>>>>>>
>>>>>>>>> I assume you are referring to something that's being already
>>>>>>>>> discussed in a different thread.
>>>>>>>>>
>>>>>>>> I was simple summarizing here;
>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> 3. JAX-RS is about MessageBodyReaders making it possible for
>>>>>>>>>> users to write the code like this:
>>>>>>>>>>
>>>>>>>>>> public void post(Book book);
>>>>>>>>>>
>>>>>>>>>> Having Request all of those methods that users are supposedly
>>>>>>>>>> can/should (?) call instead does not make sense to
>>>>>>>>>> me.
>>>>>>>>>> Request takes upon the MBR and in fact the Providers
>>>>>>>>>> functionality here.
>>>>>>>>>
>>>>>>>>> No it doesn't. Request encapsulates the logic provided by
>>>>>>>>> handlers as well as mbr/mbw providers. In some cases it
>>>>>>>>> may
>>>>>>>>> make sense to use request in other cases it doesn't. It's just a
>>>>>>>>> tool for a job. It's no different from any other
>>>>>>>>> component in (any) API.
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> For example, now instead of composite MBRs injecting Providers
>>>>>>>>>> they will/should inject Request instead ?
>>>>>>>>>
>>>>>>>>> Most certainly not.
>>>>>>>>
>>>>>>>> Why does that mean ?
>>>>>>>
>>>>>>> Perhaps I do not fully understand, what are you trying to point
>>>>>>> out, but I really don't see a reasonable connection
>>>>>>> between Request and Providers. MBR is a lower level concept than
>>>>>>> request. So I assume that most certainly people will
>>>>>>> still want to stick to Providers in their MBRs.
>>>>>>>
>>>>>>>>
>>>>>>>>> But obviously, it is a very broad question so a correct answer
>>>>>>>>> would be "It depends."...
>>>>>>>>> Yet, I can only repeat that it is not our job to absolutely
>>>>>>>>> protect people from doing stupid things with the
>>>>>>>>> tools we
>>>>>>>>> provide.
>>>>>>>>>
>>>>>>>>
>>>>>>>> I find this argument being bizarre; first we offer 5 options to
>>>>>>>> users to get to the input stream from their
>>>>>>>> application code and then we say, or lets tell not be stupid and
>>>>>>>> don't try to use Request IS getters when you get
>>>>>>>> Book
>>>>>>>> in the signature.
>>>>>>>
>>>>>>> I never said that. I said that injecting Request in MBR would be
>>>>>>> most likely not something you would want to do (i.e.
>>>>>>> stupid). Obviously, injecting a request into a resource or a
>>>>>>> resource method is a valid thing to do. Similarly,
>>>>>>> injecting entity input stream to a resource method is also a valid
>>>>>>> thing to do.
>>>>>>>
>>>>>>> I am sorry, but I fail to understand your point here. From what I
>>>>>>> understand so far, you seem to be mixing message
>>>>>>> body
>>>>>>> readers, injection of Providers as well as application-level
>>>>>>> resource method signatures into a single question that
>>>>>>> seems to evolve around using injected Request in some ambiguous
>>>>>>> use case.
>>>>>>>
>>>>>>> Perhaps it would help if you could share a specific example of the
>>>>>>> issue you see?
>>>>>>>
>>>>>>
>>>>>> JAX-RS offers few options to the users to get access to the
>>>>>> InputStream:
>>>>>> - indirectly - MBRs help with serializing InputStream into custom
>>>>>> Java types
>>>>>> - directly: InputStream MBR offers this option to users
>>>>>>
>>>>>> As far as the users writing the application code is concerned, they
>>>>>> are completely covered as far as the access to
>>>>>> InputStream is concerned. And if some users really need it then
>>>>>> JAX-RS can leak HttpServletRequest stream by getting it
>>>>>> injected.
>>>>>>
>>>>>> 'Request' is visible to the application code. Making all the input
>>>>>> stream getters there visible to the code is
>>>>>> 'redundant'. There is only one case I can think of: users type
>>>>>> resource methods with void in signatures and just use
>>>>>> Request to get the actual data they need; however this has never
>>>>>> been requested and is or more interest in context of
>>>>>> working with multi-parts. I recommend to minimize the exposure of
>>>>>> input streams at the application level; no user will
>>>>>> ever implement Request so we can push it back to Request if the
>>>>>> demands get in
>>>>>>
>>>>>> Re your remark about injecting Request into MBRs... You can't
>>>>>> assume that. Custom MBRs will want to check a verb, MBWs
>>>>>> can check if it's a conditional GET and ignore the response, they
>>>>>> can do something we can't of think of now at the
>>>>>> moment. I've had a user checking for a list of matched URIs in MBR
>>>>>> - users have a lot of custom cases to cover.
>>>>>>
>>>>>> Besides, consider a user writing a composite MBR and checking the
>>>>>> JAX-RS 2.0 dev archives and seeing the comment that
>>>>>> Request encapsulates the logic of MBRs. as you know the composite
>>>>>> MBR does not do any reads, just delegates. So I don't
>>>>>> see what will prevent a user from injecting Request and doing
>>>>>>
>>>>>> public class CompositeMBR implements MessageBodyReader<Book> {
>>>>>> public Book readFrom(Book.class, ...)
>>>>>> return request.getEntity(Book.class) {
>>>>>> }
>>>>>> }
>>>>>> why mess with checking Providers, the above is plain simpler ?
>>>>>
>>>>> Umm... maybe because it is a correct thing to do considering the
>>>>> role MBRs/MBWs play in the framework??
>>>>>
>>>>
>>>> Are you saying composite MBR should use Providers ? I agree;
>>>> You are still avoiding to give the answer I'm after : what benefit
>>>> does making getEntity.etc visible to the
>>>> application code users bring ?
>>>>
>>>>>> What is our position here ? Will the above call recurse to this
>>>>>> very CompositeMBR ?
>>>>>
>>>>> Spec-wise, the behavior should remain undefined IMO.
>>>>> Implementation-wise, most likely yes, unless the implementation
>>>>> provider decides to do something about it.
>>>>>
>>>>>>
>>>>>> Consider moving Request data getters to where they are actually
>>>>>> needed - at the handler/filter level rather than saying
>>>>>> we should tell users just be careful out there
>>>>>
>>>>> Forgive me, but the example above is rather far-fetched. All it
>>>>> shows is that someone does not understand the basic
>>>>> concepts behind MBR/MBW and their role in JAX-RS framework. Request
>>>>> is a concept that, under the hood encapsulates logic
>>>>> of marshalling and un-marshalling the request data, including
>>>>> potential invocation of MBR/MBW.
>>>>>
>>>> The example above shows how many hurdles JAX-RS 2.0 users will have
>>>> to jump over;
>>>>
>>>>> To illustrate how absurd the above is, please consider whether we
>>>>> also need to make sure that people writing their own
>>>>> MBRs are not using Providers to lookup the same MBRs to prevent
>>>>> recursion? Or do we need to make sure that people are
>>>>> not trying to actually marshal the data via
>>>>> "Response.ok(book).build();" in a message body writer on the server
>>>>> side??
>>>>>
>>>>
>>>> This is just some non-nonsense I can't follow; I'm talking about the
>>>> basic refactoring here
>>>
>>> That was the purpose - it's the same non-sense as the example you
>>> provided.
>>>
>>> Marek
>>>
>>>>
>>>> Sergey
>>>>
>>>>> Marek
>>>>>
>>>>>>
>>>>>> Sergey
>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> I know that filters do need such methods - hence I propose to
>>>>>>>>>> move these input stream/etc getters away from Request
>>>>>>>>>> and make them visible on relevant filter/handler interfaces
>>>>>>>>>
>>>>>>>>> Consuming streams is already allowed by JAX-RS 1.x spec also in
>>>>>>>>> the resource methods. It is quite useful in fact
>>>>>>>>> if you
>>>>>>>>> need to process large entities.
>>>>>>>>>
>>>>>>>> I'm getting annoyed, I'm sorry.
>>>>>>>> Yes, that is exactly what I'm saying, we let people do
>>>>>>>> InputStream or Book in the signature. Please justify why we
>>>>>>>> also should let them do the same via Request in the application
>>>>>>>> code
>>>>>>>
>>>>>>> For the very same reasons we let them inject Request into
>>>>>>> resources or resource methods already now - e.g. to be
>>>>>>> able to
>>>>>>> produce a response or work with conditional tags etc.
>>>>>>>
>>>>>>> Marek
>>>>>>>
>>>>>>>>
>>>>>>>> Thanks, Sergey
>>>>>>>>
>>>>>>>>> Marek
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Sergey
>>>>>>>>
>>>>>>>>
>>>>>>
>>>>>>
>>>>
>>>>
>>
>>
>
>


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