|
I've deliberately left the API for enabling automatic intropection rather obtuse as of yet so that the community can decide how best to make it accessible from outside the library. However, here is how you currently have to do it, for those that want to use the patch as-is.
I haven't had the time to carefully read through your patch. I do not know "Aaron Hamid's Introspection patch" not do I now the one from xmlrpc-c.sf.net.
To me, it currently looks like yet another handler mapping. I wouldn't refuse the idea to add a different handler mapping to the source tree. The handler mapping API was written specifically with different mappings in mind. Otoh, not every mapping must be a port of the XML-RPC core distribution. What I currently would like to see, would be some sentences, that describe what the differences between the default handler mapping and your suggestions are. What advantages, what disadvantages, and so on. Don't worry about the original patches then, as they're not terrribly relevant -- they're both around 3-4 years old at this point, thus this new attempt. However, this is not just "yet another handler mapping." It adds a new interface that custom handlers can implement to support introspection, as well as a new derived Invoker that implements this interface using Java reflection. It also adds a new ContextXmlRpcHandler which makes use of this interface to actually implement the method calls specified by the introspection API, and a function in the SystemHandler to enable those methods if it is so desired.
First, I will argue that this is a useful addition to the library. Second, I will argue that this code belongs in the core distribution. For the first point: While introspection is not part of the XML-RPC specification, it is a very useful feature for automatic discovery of services and proxy interface generation, among other things. This particular introspection interface is supported both by the PHP XML-RPC extension here: http://us3.php.net/manual/en/ref.xmlrpc.php and the standard Python XML-RPC server library (although the latter doesn't yet support methodSignature queries automatically). Supporting it here would mean that this library would behave more similarly to other XML-RPC implementations that are available; whether this is desirable in and of itself is debatable, but I think it is if there is no downside to the proposition. For the second: First of all, it's harmless for those who do not wish to take advantage of it. None of the code for actually calling a particular RPC method is changed, so the only possible performance change is (very) slightly larger code size. Second, as I mentioned above, having it bundled increases similarity with other XML-RPC implementations. Third, some changes, that make the introspection API easier to use, such as the new introspection-related methods on the SystemHandler and DefaultHandlerMapping class, and the addition of introspection support to the SystemHandler, could not be implemented as elegantly were the introspection code to be published and maintained separately. Finally, if introspection is not merged into the main source tree, this patch will likely end up out-of-date and useless, just as the previous one did, resulting in people who need introspection support sticking with older versions of the library and being unable to contribute to the project's current code. On the other hand, if it is merged, then it should be fairly easy to see when it breaks and fix it as the changes that require it to be updated are made. From your comment, I conclude that
a) there are implementations in other languages, which do similar things and that b) it is based on introspection As for a), I still do miss a description what does happen. The links you mention above are much more generic and no specifications or something similar. As for b), the existing DefaultHandler is based on reflection itself. In other words, I do not have the feeling to know more. I am sorry if my descriptions are insufficient. I am not sure what you are saying in your last sentence, but in case you would like more information I will try to answer the issues you raise.
a) This is correct. Other languages to implement this sub-protocol. I did not link to the pages specifically concerned with this before; here are those links (in alphabetical order by language): C/C++: http://xmlrpc-c.sourceforge.net/doc/libxmlrpc_server.html#systemmethod .NET/C#: http://www.xml-rpc.net/faq/xmlrpcnetfaq.html#3.5 PHP: http://www.php.net/manual/en/function.xmlrpc-server-add-introspection-data.php Python: http://docs.python.org/lib/simple-xmlrpc-servers.html (see register_introspection_functions) and http://docs.python.org/lib/serverproxy-objects.html The second Python link provides a very good description of the three functions this patch is concerned with implementing. b) Also correct. However, what it does with introspection is quite different; rather than using it to implement a simple named method call operation, it essentially translates remote introspection requests into local Java reflection API calls. I do now understand, that the "Introspection" stuff is more than a new handler mapping. Basically, it is providing metadata to the client.
I'll look through the code in the next week. Walter, are you interested in getting this into the 3.0 branch?
If so, let's do it in a few steps. - First of all, you should propose necessary extensions for the interfaces handlermapping and handler. (Recommended: Create an interface like XmlRpcMetaDataHandlerMapping, which extends XmlRpcHandlerMapping.) - Second: Make the existing classes implement your new interfaces. - Third: Provide special handlers, which provide the requested metadata. How's this patch? Note that this assumes the patch from
Oops! My earlier comment is incorrect. The attached patch _includes_ the changes from
Walter, I have added the interfaces XmlRpcListableHandlerMapping, and XmlRpcMetaDataHandler. Please note:
- I have added some comments. They represent my thoughts. As such, they are questionable. Please read them carefully and verify, whether they meet your expectations. - I have added methods getMethodSignatures() and getMethodHelp() to the handler mapping. Obviously, the will usually be implemented by simply calling the respective handlers methods. However, I find it thinkable, that a handler mapping will not do this. If the above is fine for you, then I suggest the following steps to follow: - Modify your XmlRpcSystemImpl to use the handler mapping methods. (Nice idea, btw, to implement it like this. Did you note, that it can easily be added to the property mappings by simply adding a property "system"? Dynamic addition isn't required.) - Make the default handler mappings implement listable handler mapping. - Same for the default handlers, which should implement metadada handler. Okay, here is an updated patch which uses your interfaces. It also adds support in PropertyHandlerMapping for automatically passing the mapping to the constructor for any object specified in the property file. This could be used to subclass PHM and store additional information used by the various XML-RPC services.
Oh, the patch also adds method overloading support to AbstractReflectiveHandlerMapping, so that, for example, a class that implements both foo(String) and foo(String[]) for some operation can make both versions available via XML-RPC.
Walter,
I apologize, but I am unhappy with the overloading. While I accept, that it might be suitable in certain situations, I do not think that it should be the default. In particular, I do not think that the selection of the right method via reflection should occur with any method invocation. That's something we had in version 2 and have just successfully managed to get rid of. IMO, the AbstractReflectiveHandlerMapping should support a single method per name only. There is no reason for not adding more complicated subclasses, if you like that. Jochen Alright, if you insist, here's a version that makes overloading support an optional (and disabled-by-default) feature.
Oops! Minor fix, forgot to adjust a couple of references to the constructors for PropertyHandlerMapping that I changed to support overloading as an optional feature.
Walter, I have committed some changes, that make AbstractReflectiveXmlRpcHandlerMapping implement the XmlRpcMetaDataHandlerMapping interface. Please, note the following:
- The implementation ensures, that the various "system.*" methods return consistent result. In particular, a method returned by "system.listMethods" is always able to provide a meaningful result for "system.methodSignature" and "system.methodHelp". - I did not implement overloading. My goal is to keep the base classes simple and clean. I am still open for subclasses, which allow overloading. - The code is not checked. It is quite likely, that you'll find bugs. - Keep documentation in mind. If you need a pointer to the apt format, see http://maven.apache.org/guides/mini/guide-apt-format.html Returning to the discussions on xmlrpc-user: Considering the fact, that overloading was possible with version 2, I find that I should drop my position. Consequently I have today added support for method overloading.
Walter, I'd really hope that you finish your good work by adding some docs. Closing, user has lost interest.
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
http://xmlrpc-epi.sourceforge.net/specs/rfc.fault_codes.php
If you don't like using the codes from that RFC, feel free to change them back to zeros; I don't much care either way.