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