Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Interface to provide own implementation for data transport #72

Closed
xabolcs opened this issue Jun 9, 2017 · 8 comments
Closed

Interface to provide own implementation for data transport #72

xabolcs opened this issue Jun 9, 2017 · 8 comments

Comments

@xabolcs
Copy link

xabolcs commented Jun 9, 2017

As an aXMLRPC user I would like to use my own data transport implementation.

It would be nice if there will be an XMLRPCStream interface which provides the needed InputStream and OutputStream.
With this in mind, aXMLRPC's current functionality could be achieved by providing a default implementation of XMLRPCHTTPStream interface which extends XMLRPCStream.

With the help of XMLRPCStream interface and a unix socket library an user could use aXMLRPC through unix domain sockets.

The XMLRPCHTTPStream interface could help users to use their own HTTP implementation and also let changes to the default HTTP implementation happen easier, like the OkHttp change in #62 .

@gturri
Copy link
Owner

gturri commented Jun 11, 2017

Thanks for this detailed feature request.

I had a closer look at the code, and it appears that half of the code is dealing with the xmlrpc protocol, and the other half is dealing with http.

Rather than trying to decouple the code in XMLRPCClient (which appears to be basically "http glue"), I guess it would be much easier to use the xmlrpc code directly.

In practice, instead of

XMLRPCClient client = new XMLRPCClient(new URL("http://example.com/xmlrpc"));
Integer i = (Integer)client.call("add", 5, 10);

you could do something like:

boolean debug = false;
String payload = new Call("add", 5, 10).getXML(debug);
InputStream response = sendThePayloadWithTheTransportIWant(payload);
Integer i = (Integer) new ResponseParser().parse(response, debug);

(I haven't actually tested it, so don't hesitate to mention if it doesn't work out of the box).

It seems to me the overhead is minimal, and that way, I don't have to guess which interface would be the most future proof.
Does it seem it could fit your need?

(also: you mentioned that this feature could fit in axmlrpc V2. But actually this version has been started by timroes, but it's likely it will never be completed. See #49 for more infos)

@xabolcs
Copy link
Author

xabolcs commented Jun 12, 2017

Looks promising, but ResponseParser is package local. (Customizing privately aXMLRPC doesn't make sense for me, see README.md#license)

Continuing this minimal solution you proposed, there could be an XMLRPCSimpleClient class - optionallly containing proposed code - which would be extended by the current XMLRPCClient.
In this case the new XMLRPCSimpleClient would be really close to the proposed XMLRPCStream interface.

@xabolcs xabolcs changed the title XMLRPCStream interface to provide own implementation for data transport Interface to provide own implementation for data transport Jun 12, 2017
@gturri
Copy link
Owner

gturri commented Jun 12, 2017

Oh, indeed. I didn't pay attention to the visibility of this class.
So as you say, I could either:

  • make this class public
  • or create this kind of XMLRPCSimpleClient

It shouldn't be too hard, but I'm afraid I won't have time to work on it before at least the end of the month. (If you're in a hurry, you may push a pull request though)

@xabolcs
Copy link
Author

xabolcs commented Jun 14, 2017

If you vote for making that class public, please notice this minimal usage in the README.md!

@gturri gturri closed this as completed in 13c74d3 Jun 16, 2017
@gturri
Copy link
Owner

gturri commented Jun 16, 2017

I tried to create a class which would accept some kind of ITranport, and to have XMLRPCClient extends it, but it appears to be quite hard to keep retrocompatibility and to end up with an API that wouldn't be clumsy (in particular because of the handling of flags). Hence I made ResponseParser public, and updated the doc accordingly.

I didn't really tested it, so please let me know if it actually works, so that I can make a new release, and feel free to re-open this issue if it doesn't work as expected

@xabolcs
Copy link
Author

xabolcs commented Jun 19, 2017

Sadly, I was unable to try out with unix sockets, but it works flawlessly with OkHttp backed IXMLRPCClientTransport.

public interface IXMLRPCClientTransport {
	OutputStream getOutputStream();
	InputStream getInputStream();
}

Thanks! 👍

@gturri
Copy link
Owner

gturri commented Jun 23, 2017

I just pushed axmlrpc v1.11.0 on maven central. It contains this patch.

@xabolcs
Copy link
Author

xabolcs commented Jul 25, 2017

I was able to try out v1.11.0 with unix sockets and it works flawlessly with socat - UNIX-CONNECT:/tmp/socket.sock backed IXMLRPCClientTransport. 😀

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants