chemistry-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Carlo Sciolla <carlo.scio...@gmail.com>
Subject Re: Chunked transfer encoding
Date Fri, 10 May 2013 14:57:53 GMT
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)
>



-- 
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
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message