Community
Participate
Working Groups
Build Identifier: I had a play with the REST based provider the other day. Unfortunately, it's too limited in a number of ways. The first is that it's terribly based to XML transports. There's a deserializer which stamps on an XML transport parser automatically, even when you're not using XML. It's pretty unintuitive to have something automatically be XML, particularly when most transports are not XML these days (e.g. JSON). Reproducible: Always
The ECF REST impl is not bound to XML-based (de) serialization. All that's necessary is to define define an impl of this interface: org.eclipse.ecf.remoteservice.client.IRemoteResponseDeserializer See, for example, this test class for an example of using the json parser. org.eclipse.ecf.tests.remoteservice.rest.twitter.TwitterRemoteServiceTest
Ok so (a) that's the one I'm referring to in the other bug - it should be public - but this one is about the constructor for the restservice stamping on an XML deserislizer without making it obvious that's what it's doing.
See: http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.ecf/framework/bundles/org.eclipse.ecf.remoteservice.rest/src/org/eclipse/ecf/remoteservice/rest/client/RestClientContainer.java?root=RT_Project&view=markup public RestClientContainer(RestID id) { super(id); // Set serializers setParameterSerializer(new StringParameterSerializer()); setResponseDeserializer(new XMLRemoteResponseDeserializer()); } It should not be the case that you get an XMLRemoteResponseDeserializer on any generic rest call. In fact, it shouldn't be assuming text at all. The fact that some can be text (and of that, a small subset is XML) should not mean that the default is XML, even if it can be specified in the setup elsewhere.
(In reply to comment #3) > See: > > http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.ecf/framework/bundles/org.eclipse.ecf.remoteservice.rest/src/org/eclipse/ecf/remoteservice/rest/client/RestClientContainer.java?root=RT_Project&view=markup > > public RestClientContainer(RestID id) { > super(id); > // Set serializers > setParameterSerializer(new StringParameterSerializer()); > setResponseDeserializer(new XMLRemoteResponseDeserializer()); > } > > It should not be the case that you get an XMLRemoteResponseDeserializer on any > generic rest call. In fact, it shouldn't be assuming text at all. > > The fact that some can be text (and of that, a small subset is XML) should not > mean that the default is XML, even if it can be specified in the setup > elsewhere. What should the default be then, if not xml?
The problem with assuming the default is XML is (a) it assumes everything is XML (when there's a lot that's JSON, for example) and (b) it assumes it's a String. Neither of those two assumptions hold in the real world. Many web-based applications will use JSON as their API rather than XML, and there's plenty of other examples that are non-textual (e.g. Google Chart API will serve back images, iTunes preview gives you an MP3). Others might return (non-well-formed) HTML content. The problem is that the header that you get back from the remote HTTP tells you what the information is, but that's lost before it hits the client. So the client has no say in what to get (other than by defining the services ahead of time such that one always expects an Image, for example). But the default constructor should not make those assumptions, and just pass data back as-is. For text/* responses, I'd suggest just returning that String. For other data types, we've really lost the information by then - perhaps a callback on the REST client could perform a transformation - given a stream, a mime type, return an object.
(In reply to comment #5) > The problem with assuming the default is XML is (a) it assumes everything is > XML (when there's a lot that's JSON, for example) and (b) it assumes it's a > String. Neither of those two assumptions hold in the real world. Many web-based > applications will use JSON as their API rather than XML, and there's plenty of > other examples that are non-textual (e.g. Google Chart API will serve back > images, iTunes preview gives you an MP3). Others might return (non-well-formed) > HTML content. Right. So that's why the current API has several options for handling these various other (non-default) use cases (i.e. not using xml and/or String): 1) Define your own impl of org.eclipse.ecf.remoteservice.client.IRemoteResponseDeserializer and set it (via IRemoteServiceClientContainerAdapter.setResponseDeserializer). This deals with 'a' successfully (probably a fairly large number of additional cases...e.g. all json-based services). Mainly for simplicity's sake, it does not deal with 'b', however...which leads us to 2) Override one/several methods on RestClientService (or AbstractClientService). Note the following methods can be overridden by subclasses (protected): createAndPrepareHttpMethod getResponseAsString processResponse or even invokeRemoteCall which allows control of every part of the actual remote invocation...from the handling of request/response headers, to the reading of the data, to the handling/processing/deserialization of the data (i.e. as String, byte[], or whatever you want)...and the calling (or not) of the IRemoteResponseDeserializer...which is actually done in AbstractClientService.processResponse. So overriding some/several of these methods allows one to customize the behavior of the remote invocation for more exotic/unusual use cases. > > The problem is that the header that you get back from the remote HTTP tells you > what the information is, but that's lost before it hits the client. So the > client has no say in what to get (other than by defining the services ahead of > time such that one always expects an Image, for example). But the default > constructor should not make those assumptions, and just pass data back as-is. > For text/* responses, I'd suggest just returning that String. For other data > types, we've really lost the information by then - perhaps a callback on the > REST client could perform a transformation - given a stream, a mime type, > return an object. See the methods above to do this customization. Any/all of the ones you list can be done with the current API through subclassing RestClientService or AbstractClientService (and creating/constructing your type via the associated AbstractClientContainer.createRemoteService) 3) If you would like to propose new/other API to simplify further some or all of these use cases then that would be great...please propose specifics via code contributions/patches to this bug and the API can most likely be added or the existing API generalized where needed.
I'm resolving this bug as worksforme, as we a) already have support for json (and other) serialization/deserialization (through the mechanisms described in comment b), and changing the default from xml to json adds an extra plugin dependency (as org.json is then required to be present, whereas the xml parser comes with the framework). Alex if you still think the default should be json, then I'm completely open to contributions that make it so...and use/provide a system property (for example) as a way to set/change the default (from/to xml/json). Unfortunately, none of the present committers have time for this addition at the moment.
