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