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: Mon, 19 Dec 2011 22:05:54 +0000

For whatever it's worth:
http://java.net/jira/browse/JAX_RS_SPEC-153
http://java.net/jira/browse/JAX_RS_SPEC-154
http://java.net/jira/browse/JAX_RS_SPEC-155


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