Hi Jan,
When you say the overall design is broken, do you specifically mean the MBW.getSize() not fitting well into the picture or is there anything else?
Also, when you say that the problem cannot be fixed by the developers, do you think that (given most MBWs already return -1 anyway) a workaround where a user would need to override a particular MBW with it's own (perhaps wrapper or extension) implementation that returns -1 from getSize() is not possible? Again, most MBWs already are "compatible", so we're talking about marginal use cases here...
FWIW, I agree that we need to fix the getSize() issue in the spec. It just seems to me we need a longer discussion about this and I'm not sure if we can agree on a solution in time for 2.0.
Marek
On Oct 22, 2012, at 12:45 PM, Jan Algermissen <jan.algermissen_at_nordsc.com> wrote:
>
> On Oct 22, 2012, at 12:43 PM, Jan Algermissen wrote:
>>>
>>
>> I think the overall design is now broken because the original MBW API does not fit with the filter concept.
>>
>> The problem must be taken care of by the runtime because a developer alone cannot ensure correct Content-Length settings.
>
> Sorry, should have added '..taken care of until getSize() is removed from the API ...'
>
> Jan
>
>
>>
>> I am not sure though the runtime can do that either.
>>
>> Jan
>>
>>>
>>> Marek
>>>
>>>>
>>>> or
>>>>
>>>> 2) depricate getSize()
>>>>
>>>> I guess 1) needs to be added and implemented by every container in any case given that we cannot deprecate getSize immediately.
>>>>
>>>>
>>>> It never struck me as an issue, because I never use built-in MBWs and mine allways return -1 from getSize() but looking at this, the original design of MBWs deciding upon Content-Length vs. Chunked-Encoding as simply wrong. This is an HTTP-connector issue and should be left to that layer.
>>>>
>>>> Bottom line: +1 to Markus saying that 2.0 cannot be shipped without that being fixed.
>>>>
>>>> Jan
>>>>
>>>>
>>>>
>>>> On Oct 20, 2012, at 6:09 PM, Markus KARG wrote:
>>>>
>>>>> Marek,
>>>>>
>>>>>>> Sorry for repeating myself, but, whether to use chunked encoding or
>>>>>> set a content-length header should be left up to the underlying HTTP
>>>>>> engine (i.e. the servlet container). I know for a fact that Jetty and
>>>>>> Tomcat use a buffered output stream. If you finish a request within
>>>>>> the limits of the buffer (without calling flush), then Content-length
>>>>>> is set, otherwise chunk encoding is used.
>>>>>
>>>>> ...not just "should" but MUST! And that underlying "engine" for a JAX-RS
>>>>> filter (hence the layer that the filter relies on to manage this) is the
>>>>> JAX-RS implementation (independend of whether it does it on its own or
>>>>> instead relies on a deeper layer to do it), as that "engine" is defined
>>>>> solely by means of an API -- JAX RS 2.0! (I think we all agree that a filter
>>>>> has to work in Java SE environments, too, as in the opposite case we do not
>>>>> need to define JAX-RS filter API at all: If we enforce Servlets, we can just
>>>>> use Servlet Filters!).
>>>>>
>>>>>> Ok, sorry for confusion. What I meant to say is that if you remove
>>>>>> manually set Content-Lenght header as well as return -1 from
>>>>>> MBW.getSize(), the IO container will take care of it. Since the
>>>>>> solution is IO container dependent it would be hard to enforce a single
>>>>>> standard behavior in JAX-RS IMO.
>>>>>
>>>>> This does not work as the Content-Length header is not manually set, but is
>>>>> set by Jersey. See below.
>>>>>
>>>>>>> getSize() should also be deprecated and ignored. With the ability to
>>>>>> wrap streams to do things like GZip encoding, getSize() is often wrong
>>>>>> and should never be relied upon.
>>>>>
>>>>> Exactly!
>>>>>
>>>>> BTW, agreed, even for a simple log recorder *both*, getEntityStream *and*
>>>>> setEntityStream are needed, otherwise the recorded content will never be
>>>>> transferred. You are right; I missed this side effect.
>>>>>
>>>>> But, frankly spoken, I still think (basing on what I experienced in the past
>>>>> with Servlet Filters I came across, and basing on the filters I contributed
>>>>> to) many of the future JAX-RS filters will be representational
>>>>> *transformers* (hence: change Content-Length) in some way (not Java entity
>>>>> modifiers or MBR/MBW caches, which surely are good for *different*
>>>>> purposes), hence will enforce a "correctly working" underlying *automatic*
>>>>> processing of Content-Length / Chunking in the JAX-RS provider (which may
>>>>> itself offload this burden to the Servlet Container or implement it directly
>>>>> in a Java SE case, in its sole discretion), adjusting the Content-Length
>>>>> information according to the transformation *result* (not *source*) in its
>>>>> representational form (hence: modify JSON or XML without knowledge or care
>>>>> of neither the used MBR / MBW, nor Java Entity Class, nor JAX-RS Resource).
>>>>>
>>>>> In the case of ISVs -like me- the problem is getting rather worse: The
>>>>> filters we provide to the open market not only are purely representational
>>>>> transformers, but are provided in binary form to anybody that has the need
>>>>> to enable the particular functionality (hence "feature") of that filter to
>>>>> any representational form produced by any completely unknown combination of
>>>>> MBR / MBW, Java Entity Class, JAX-RS Resource *and JAX-RS implementation*.
>>>>> Due to that, any proposed solution to the mentioned problem will "not exist
>>>>> by definition" (i. e. are to be ignored by the designers), as that solution
>>>>> is *Jersey* and / or *Servlet* specific! Neither does an ISV know whether or
>>>>> not the filter will be used in a Servlet container or on a Java SE
>>>>> environment, nor would it be wise to rely on one particular JAX-RS product.
>>>>> Hence, ISVs can and will *only* rely on solutions guaranteed to work by pure
>>>>> JAX-RS 2.0 API. So a Jersey-only solution is "not existing by definition" in
>>>>> the context of a *spec* discussion.
>>>>>
>>>>> Moreover I think you maybe missed the fact the a filter *cannot* remove the
>>>>> false Content-Length header, as at time of filtering that header is actually
>>>>> *not set* --- because it is *not* the JAX-RS Resource nor an earlier filter
>>>>> that has set that header, but it is the JAX-RS 2.0 implementation (here:
>>>>> Jersey 2.0 M08-1) itself which had set that header *automatically* basing on
>>>>> the *original* representation (maybe it is a MBW inside of Jersey, I don't
>>>>> know, it just is not there at .filter() time, but it is there in the
>>>>> representation received by the client)! THAT DOES SIMPLY NOT WORK AND LEADS
>>>>> TO SEVERE ERROR IN PRODUCTION USE!
>>>>>
>>>>> As a result, there is *no* solution to the problem I have so far, which
>>>>> unfortunately *enforces* a vote for "NO GO!" for the current draft of the
>>>>> spec!
>>>>>
>>>>> Potentially (and even worse: such easily) opening the doors to
>>>>> *unexpectedly* induce such severe problems is a definitive no-go situation
>>>>> for the complete specification, unfortunately! And the sole existing
>>>>> workaround (using a filter to inspect request headers to enable / disable
>>>>> the transformer, store them as a context property, check this in an entity
>>>>> listener and then apply the transformation *there* as this runs *before* the
>>>>> JAX-RS implementation auto-adds the Content-Length header), is such overly
>>>>> complicated and error-prone that it simply looks as a design fault of the
>>>>> specification to the average reader (as one sees the getEntityStream /
>>>>> setEntityStream combination and will think that he can simply inline a
>>>>> FilterStream, which, again, will result in the wrong Content-Length). Also
>>>>> it will induce the next problem: What if that transformer must be the *last*
>>>>> step in the filter chain (to ensure that another filter does not ruin the
>>>>> transformed result again) - which currently simply is impossible as by
>>>>> definition entity listeners MUST run *before* the very first filter will
>>>>> run? As you see, all currently proposed "solutions" are just NOT COMPLETE OR
>>>>> WORKING STABLE in any way!
>>>>>
>>>>> Maybe you see a working solution with the current draft. I do not. The only
>>>>> solution I see is that the spec has to be changed.
>>>>>
>>>>> My solution proposal is: The spec must mandate that if a compliant JAX-RS
>>>>> 2.0 implementation is setting the Content-Length header, then it MUST
>>>>> calcuate its value from the *transformed* representation, but not from the
>>>>> *original* representation provided by the MBW. Moreover, I would even
>>>>> mandate that a compliant implementation MUST OVERWRITE the Content-Length
>>>>> (to the correct value, certainly) always, as neither the JAX-RS resource,
>>>>> nor a MBW, nor another filter, will know whether the deployer might add a
>>>>> filter that changes the length of the representation! At last, JAX-RS must
>>>>> not allow that a JAX-RS 2.0 implementation is providing a false value to
>>>>> Content-Length just like Jersey does it at the moment (better to have NO
>>>>> Content-Length set by Jersey than to have it set a wrong one)!
>>>>>
>>>>> This is a very severe issue, and we MUST have a solution for that before
>>>>> final release. I see absolutely no chance for a "GO!" to a draft that
>>>>> provides the possibility to modify the representation using an inlined
>>>>> FilterStream while the JAX-RS implementation is allowed to add a *false*
>>>>> Content-Length header at the same time.
>>>>>
>>>>> Regards
>>>>> Markus
>>>>>
>>>>>
>>>>
>>>
>>
>