chemistry-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Florian Müller <f...@apache.org>
Subject Re: Chunked transfer encoding
Date Fri, 10 May 2013 15:45:23 GMT
Got it. I thought you were heading in a different direction.

How is that different from making the current Dispatch object protected 
or adding a protected method that wraps the Dispatch.addResource() method?

Extensibility-wise they would be equal. Your version only adds a bit 
more syntactic sugar. It would also need a bit more memory because we 
would have to add about 100 additional classes (AtomPub + Browser) and 
runtime objects.

- Florian


> Is it next week already? :-)
>
> Jokes apart, I had one hour to spare for this topic now that it's still
> hot, and produced the patch you can see attached to
> CMIS-656<https://issues.apache.org/jira/browse/CMIS-656>.
> That's a translation of the current Chemistry dispatch logic to abandon
> reflection, static methods, private constructors, and final classes.
>
> The flow is quite the same as before, with the servlet initialization logic
> registering the default bindings by adding concrete instances of a
> ServiceCall interface into a local registry. At request serving time the
> registered ServiceCall is retrieved and its service() method invoked. The
> invocation logic is now a mere INVOKEINTERFACE JVM instruction instead of
> the previous reflection trick (tiny performance improvement here).
>
> Please note that the ServiceCall interface was already a hidden contract
> that all service calls had to implement to be addressable by the Method
> created by the Dispatcher.
>
> As a last note, CmisAtomPubServlet also exposes a protected
> registerDispatch(resource, method, ServiceCall) to allow implementors to
> override the default logic.
>
> That's it for now. Please note that as the Dispatcher is shared across the
> AtomPub and Browser bindings, and as only the AtomPub was properly
> refactored, that patch will break your build.
>
> Looking forward to hear your comments,
> c.
>
>
> 2013/5/10 Carlo Sciolla<carlo.sciolla@gmail.com>
>
>> Hi Florian,
>>
>> will do, I'll hopefully have a patch for you to review next week.
>>
>> c.
>>
>>
>> 2013/5/10 Florian Müller<fmui@apache.org>
>>
>>> Hi Carlo,
>>>
>>> Please go ahead and make a proposal. Submit a patch that sketches your
>>> ideas. It doesn't have to be complete and running.
>>> It's easier to talk about something concrete rather than discussing about
>>> generic topics.
>>>
>>> - Florian
>>>
>>>
>>>
>>>   Hi Florian,
>>>> it's not my intent to start a flame war, and we can continue offline if
>>>> you
>>>> want. To the interest of the list, I'll just write here some of the
>>>> reasons
>>>> behind my previous comment, and wait until I have some patch ready to
>>>> prove
>>>> my points before bothering you guys again with my ramblings.
>>>>
>>>> I understand your remark on the spec compliance, and that interop is at
>>>> stake when you allow the protocol details to be modified. BUT...
>>>>
>>>> - There's much more to HTTP than what CMIS specifies, and Chemistry is
>>>> currently in charge of handling the transport protocol on top of which
>>>> CMIS
>>>> expresses itself.
>>>>
>>>> - No extensibility means that backporting of fixes might be a no-go. For
>>>> example, if I now upgrade Chemistry to the latest trunk my code doesn't
>>>> compile anymore. Subclassing would be the best way to integrate your
>>>> changes in the version I use, but that's not available to me.
>>>>
>>>> - The CMIS spec is (hopefully!) not fixed and will evolve with time. OO
>>>> could help greatly e.g. to support different versions of the protocol
>>>> with
>>>> a single serve instance, which is painful to implement cleanly following
>>>> the current code approach.
>>>>
>>>> - Spec extensions are not evil per se. While they might indeed break
>>>> interoperability, they're also a way to open the protocol itself to
>>>> experiments and eventually drive a community based evolution of it.
>>>>
>>>> - Interop is not always the top priority. CMIS and Chemistry provide a
>>>> lot
>>>> of value OOTB, and I don't see why proprietary extensions should be
>>>> denied
>>>> by principle. Why shouldn't I have the possibility to write a custom
>>>> extension to improve e.g. content delivery capabilities
>>>> (getContentStreamByPath, anyone?), while still retaining full protocol
>>>> compliance for B2B data exchange?
>>>>
>>>> If you made it to here, a big thank you for your patience.
>>>>
>>>> Hope this helps,
>>>> c.
>>>>
>>>>
>>>>
>>>>
>>>> 2013/5/8 Florian Müller<fmui@apache.org>
>>>>
>>>>   Hi Carlo,
>>>>> This code is not meant to be extensible. The CMIS standard is fixed.
>>>>> There
>>>>> will be no additional services or operations. Also, the semantics of
the
>>>>> parameters will not change or have to be changed. Any extensions would
>>>>> bypass the specification, which would be a pain for clients and doesn't
>>>>> help interoperability.
>>>>> All methods that handle requests are stateless, isolated pieces of code.
>>>>> OO doesn't help here. And the alternative to reflections would be a 120
>>>>> lines if-statement. I can't see the advantage of that.
>>>>>
>>>>> Also, this part is considered internal code and may change at any time.
>>>>> OpenCMIS provides a lot of extensibility on the client side and keeping
>>>>> the
>>>>> interfaces compatible does hurt sometimes. I don't see a real use case
>>>>> to
>>>>> have that pain also on the server side. If there is something missing
or
>>>>> wrong, we usually fix it pretty quick - as you have seen.
>>>>>
>>>>> Having said that, if you have a great idea how to refactor that code,
>>>>> feel
>>>>> free to provide a patch.
>>>>>
>>>>>
>>>>> - Florian
>>>>>
>>>>>
>>>>> Am Mittwoch, den 08.05.2013, 16:05 +0200 schrieb Carlo Sciolla<
>>>>> carlo.sciolla@gmail.com>:
>>>>>
>>>>>   Hi Florian,
>>>>>> thanks for your quick commit, I will experiment a bit with it and
let
>>>>>> you
>>>>>> know what comes out of it. I do already have some initial comments
>>>>>> anyway:
>>>>>>
>>>>>> - I see you only addressed the browser bindings implementation. While
I
>>>>>> can
>>>>>> see the reason behind it, I think it won't hurt to also apply a similar
>>>>>> logic to the AtomPub binding
>>>>>>
>>>>>> - as I'm stuck with v0.6.0, I'm looking into ways to backport or
>>>>>> integrate
>>>>>> your code in my app. The current logic for method dispatch in AtomPub
>>>>>> goes
>>>>>> against extensibility (and some object oriented design principles,
IMO)
>>>>>> and
>>>>>> while for the time being I can work around it, would you guys consider
>>>>>> refactoring the dispatch logic to make use of non-final classes /
no
>>>>>> reflection / public constructors?
>>>>>>
>>>>>> Thanks,
>>>>>> c.
>>>>>>
>>>>>>
>>>>>> 2013/5/8 Florian Müller<fmui@apache.org>
>>>>>>
>>>>>>   Hi Carlo,
>>>>>>
>>>>>>> I've added some new code. There are now three interfaces that
let you
>>>>>>> control the server headers.
>>>>>>> The ContentStream object that is returned by getContentStream()
must
>>>>>>> implement the interface(s) to trigger the behavior:
>>>>>>>
>>>>>>> ContentLengthContentStream - Sets the Content-Length header and
turns
>>>>>>> chunking off.
>>>>>>>
>>>>>>> LastModifiedContentStream - Sets the Last-Modified header and
respects
>>>>>>> the
>>>>>>> If-Modified-Since header.
>>>>>>>
>>>>>>> CacheHeaderContentStream - Sets the Cache-Control header, the
Expires
>>>>>>> header, and the ETag header and respects the If-None-Match header.
>>>>>>>
>>>>>>>
>>>>>>> Please let me know if that works for you.
>>>>>>>
>>>>>>>
>>>>>>> - Florian
>>>>>>>
>>>>>>>
>>>>>>>   Hi there, sorry for the late reply.
>>>>>>>
>>>>>>>
>>>>>>>> 2013/5/7 Florian Müller<fmui@apache.org>
>>>>>>>>
>>>>>>>>   That is surprising. Chunked encoding isn't really exotic.
>>>>>>>>
>>>>>>>>
>>>>>>>>>   Definitely, but browsers are always there to remind
us that world
>>>>>>>>> class
>>>>>>>>>
>>>>>>>>>   standards are nothing different from regional social
conventions,
>>>>>>>> are
>>>>>>>> they?
>>>>>>>>
>>>>>>>>
>>>>>>>>   Could you please open an Improvement issue and add a few
details.
>>>>>>>> I'll
>>>>>>>>
>>>>>>>>   look into it.
>>>>>>>>>   Thanks, here it is<https://issues.apache.org/******<https://issues.apache.org/****>
>>>>>>>>>
>>>>>>>>> jira/browse/CMIS-655<https://**issues.apache.org/**jira/**
>>>>>>>>> browse/CMIS-655<https://issues.apache.org/**jira/browse/CMIS-655>>
>>>>>>>>>
>>>>>>>>> <https://**issues.apache.org/**jira/browse/**CMIS-655<http://issues.apache.org/jira/browse/**CMIS-655>
>>>>>>>>> <https:/**/issues.apache.org/jira/**browse/CMIS-655<https://issues.apache.org/jira/browse/CMIS-655>
>>>>>>>>>>
>>>>>>>>>   >.
>>>>>>>>
>>>>>>>>
>>>>>>>
>>
>> --
>> Carlo Sciolla
>>
>> --==(A)==--
>> Linux User #372086
>> My personal blog: http://www.skuro.tk
>> Follow me on twitter: http://twitter.com/skuro
>>   <http://twitter.com/skuro>Fork me on Github: http://github.com/skuro
>> <http://github.com/skuro>My LinkedIn profile:
>> http://nl.linkedin.com/in/carlosciolla
>> --==(A)==--
>>
>> Product Lead at Backbase - Next Generation Portal Software for Financials
>> &  Large Enterprises (http://www.backbase.com)
>>
>
>
>


Mime
View raw message