Hi Mathieu,

Great ideas, thank you for your contributions so far.

Before we dig into the details, may I first understand _why_ are you
adding that functionality in the first place? In other words, you can
decide on the HTTP protocol from the calling page right? If it is a
form, it's a POST, if it is a URL, it's a GET, or you can override
that in certain areas using the OFBiz DSL. So what is the purpose here
of adding this functionality in the controller? If we implement a
RESTFUL API, wouldn't we still decide on the HTTP protocol from the
client side? Or am I missing something here?

On Tue, Jul 3, 2018 at 7:05 PM, Mathieu Lirzin
<[email protected]> wrote:
> Hello,
>
> Following revert of rev 1834917, I want to “properly” propose adding a
> ‘method’ attribute to ‘request-map’ elements.
>
> I am currently working on adding REST based Web APIs to OFBiz [1].  In
> order to do that, it is important for the HTTP controller to handle
> various HTTP methods differently, meaning allowing a different request
> handler to be called depending on the method of the HTTP request.
> Syntactically I am proposing the following:
>
>    <request-map uri="examples" method="get">
>       <security https="true" auth="true"/>
>       <event type="java" path="ExamplesHandlers" invoke="getExamples"/>
>       <response name="success" type="view" value="..."/>
>       <response name="error" type="view" value="..."/>
>    </request-map>
>
>    <request-map uri="examples" method="post">
>       <security https="true" auth="true"/>
>       <event type="java" path="ExamplesHandlers" invoke="postExamples"/>
>       <response name="success" type="view" value="..."/>
>       <response name="error" type="view" value="..."/>
>    </request-map>
>
> Here ‘method’ is an optional attribute which defaults to "all" and/or
> "".  Currently the patch supports both but I would be in favour of
> handling only one for the sake of simplicity.  The semantic of that
> default is that it mimics the current behavior, meaning it handles every
> methods.
>
> Attached you will find a serie of 3 patches (meant to be applied in
> order) implementing the described change:
>
> - The first one handles the parsing of the XML (the change in the xsd
>   file has not been reverted).
>
> - The second is a simple refactoring to agglomerate the read of the XML
>   file in one place.  This is done to ease the implementation and tests
>   in the third patch.
>
> - The third patch implements the segregation of ‘request-map’ elements
>   based on the actual request method.  In order to associate a request
>   to an event handler we now need to dispatch both on the URI and on the
>   method.  To achieve that, I have decided to keep using the URI as a
>   key but associate it to a List of ‘request-map’ (one for each method)
>   instead of a single one.  In term of implementation a ‘MultivaluedMap’
>   has been used.  The list of candidates for a particular URI is then
>   filtered to keep only the best matching one.  The ‘request-map’ Map
>   concrete type was a MapContext which is a stack of Maps (to handle
>   includes), The ‘MultivaluedMapContext’ implementation is an adaptation
>   of MapContext to MultivaluedMaps.  Since
>   ‘ControllerConfig::getRequestMapMap’ is used at multiple places I have
>   kept it temporarily and replaced its code with an adapter to the new
>   ‘ControllerConfig::getRequestMapMultiMap’ method.
>
>
>
> Comments, questions and/or reviews welcome.
>
> [1] https://issues.apache.org/jira/browse/OFBIZ-4274
>
> --
> Mathieu Lirzin
> GPG: F2A3 8D7E EB2B 6640 5761  070D 0ADE E100 9460 4D37
>

Reply via email to