xml-rpc-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Andrew Evers" <aev...@redwood.nl>
Subject PATCH: Refactor XmlRpcServer and friends for flexible threading.
Date Fri, 23 Aug 2002 08:00:07 GMT
> "Andrew Evers" <aevers@redwood.nl> writes:
>
> Looks right on, and worth adding to its header documentation.  I'd do
> it myself, but don't want to interfere with your changes; would you
> include this list of responsibilities in your patch?

Shall we wait for the discussion to die down? ;)

>>      XmlRpcRequest parseRequest(InputStream, String, String)
>>      XmlRpcRequest parseRequest(InputStream)
>
> Excellent suggestion!  This follows the pattern set by the servlet API,
> Apache's httpd, and many other request/response app framework I've
> worked with.  Were you thinking protected or public for the
> method scoping?
>
> I would be +1 on deprecating the XmlRpc class, moving its content into
> a new XmlRpcProcessor class, and having the original XmlRpc class
> extend the new one.
>
>> b. XmlRpcProcessor also provides an executeRequest method that
>>    will use a callback interface to map handlers to objects.
>>      void executeRequest(InputStream, String, String, HandlerMapping)
>>      void executeRequest(InputStream, HandlerMapping)
>
> I like it.  Why do to suggest passing the HandlerMapping into the
> method call rather than having the XmlRpcProcessor maintain that
> information internally?
>
>> c. The HandlerMapping interface is a single method
>>      Object getHandler(String methodName)
>>    this will return null for no mapping.
>
> Why not use the Map interface instead of a custom HandlerMapping?
> Though HandlerMapping might have a narrower interface (which is nice),
> is it really worth the trade-off of introducing a new object type? The
> less custom object types involved in a package, the easier I find it to
> understand (especially as a new developer/user).  It seems that
> alternate mapping types could be just as easily supplied through
> custom Map implementations.

Two reasons (other than the narrower interface):
1. The current patch specifies that the HandlerMapping will throw an
   Exception if the handler is not found. This allows better error
   propagation than simply returning null.
2. It works with Java 1.1.

I think calling through an XmlRpcHandlerMapping is clearer than
through a generic Map, but that could just be me.

>> 3. The changes above allow external management of threads
>
> Yay!
>
>> 4. Splitting out the Invoker has recently been suggested by Kevin
>>    Hester (my formatting):
> Yup.

OK, the patch is attached in "cvs diff -u" format, with extra files
attached independently. Everything is in a JAR file, since I hope
you all have jar ;) Commentary for the patch follows.

New classes effectively already described:
+ XmlRpcHandlerMapping - the getHandler(String) throws Exception interface.
+ DefaultHandlerMapping - an implementation of HandlerMapping based
  on the existing XmlRpcServer implementation.
+ ParseFailed - A runtime exception a'la AuthenticationFailed.
+ Invoker - the Invoker class previously in XmlRpcServer.java, now public.
+ XmlRpcRequest - encapsulates an XML-RPC request.

Completely new classes:
+ XmlRpcRequestProcessor - decode a request
  Produce an XmlRpcRequest from an InputStream (optional user/pass).
    public XmlRpcRequest processRequest(InputStream, String, String)
    public XmlRpcRequest processRequest(InputStream)
+ XmlRpcResponseProcessor - encode a response/exception
  Produce a byte [] from either an Object - representing a return value
  or an Exception - representing an error.
    public byte [] processException(Exception x, String encoding)
    public byte [] processResponse(Object outParam, String encoding)
+ XmlRpcWorker - decode, process and encode a request/response.
  Ties everything together, but only communicates with the XmlRpcServer
  via XmlRpcServer -> XmlRpcWorker execute() and XmlRpcWorker calls via
  the XmlRpcHandlerMapping.

Current classes with new roles:
+ XmlRpcServer - handle a thread pool and a default handler mapping.

Things changed a little from the patch I originally proposed. The
whole thing is probably best explained by four lines in XmlRpcWorker:

  request = requestProcessor.processRequest(is, user, password);
  handler = handlerMapping.getHandler(request.getMethodName());
  response = invokeHandler(handler, request);
  return responseProcessor.processResponse(response,
  requestProcessor.getEncoding());
The *Processor classes are public and have public entry points so that
it is possible to build your own XmlRpcWorker-like classes, by assembly
(ie. delegation) rather than by inheritance, which can be trickier.

I've copied in licenses, copyright dates and redistributed authors
as best I could, please don't be offended if I've made a mistake here ;)

Otherwise, let me know what you think.

Andrew.



Mime
View raw message