Hi Saminda,
I'm glad you're looking at this. I noticed some of this a while back
and haven't had time to look into it in detail.
1. Refactoring the methods used for REST/SOAP so that they only
diverge when required is a good plan. +1
2. +1 to having the code in only one place. I don't mind where.
3. I'm in agreement on this if the intention is to keep this function
within the servlet.
4. Please be careful with this... I think the different fault handling
may be for different reasons. (REST/SOAP)
5. It's an implementation of RequestResponseTransport which we use to
support asynchonicity. There's a good class comment on
RequestResponseTransport for more info.
6. Another couple of things you could maybe look at?
a. getHeaders vs getTransportHeaders - I don't really understand the
difference
b. HTTPTransportReceiver does no such thing and is just a small
collection of utility methods. These could perhaps be rolled into one
of the other transport utils classes.
Cheers,
David
On 16/02/07, Saminda Abeyruwan <[EMAIL PROTECTED]> wrote:
Hi Devs,
AxisServlet has undergone many changes and apparently there is room for code
improvements. Looking at the code it is evident that some code has
repeated many times and some helper methods has only slight differences,
that could have been coupled into fewer helper methods. In fact some of the
helper methods in AxisServlet should have been in a util such that other
Listener developer folks could easily use with.
Following are the improvements that should have been done IMO.
1. protected MessageContext
createAndSetInitialParamsToMsgCtxt(HttpServletResponse
resp,
HttpServletRequest
req) throws AxisFault { ...}
and
protected MessageContext
createMessageContext(HttpServletRequest req,
HttpServletResponse resp) throws IOException {..}
apparently above methods will do the same thing; but prior use in POST
request and later use in GET request. Why? one method would be more than
enough for the most cases.
2. For a RESTFul invocation; it uses RESTUtil class. But to process a SOAP
request it uses inline code [line 268 - 341] [doPost()]. There also exist an
SOAPUtil (org.apache.axis2.transport.http.util.SOAPUtil)
for processing SOAP request. But no one uses it. The worst scenario is;
SOAPUtil has code for processing SOAP request but no one cares of updating
the code when ever they change the code in doPost(). IMHO this is really bad
and either we should move the logic to SOAPUtil and use it or just remove
the obsolete class. But IMHO we should move the code to SOAPUtil and use
that class instead. If some listener developer wants it; one could easily
use it.
3. It is apparent that most of the parts of the doPost(); doGet(); doPut()
and doDelete() are some. Most of the time code repeat itself. This is not
good either in code "close for modification" or "open for extension".
Should have moved to a util methods and reuse it.
4. Same goes to handleFault methods as well.
5. What is the purpose of inner class
ServletRequestResponseTransport. Does this handle all the
media types?
Should I've raised a JIRA on this. Before that please express you consensus
on the prior.
Thank you
Saminda
--
Saminda Abeyruwan
Software Engineer
WSO2 Inc. - www.wso2.org
--
David Illsley - IBM Web Services Development
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
