Using JacksonRepresentation for RIAP results in java.lang.IllegalArgumentException: argument type mismatch

[d4nirod]

Restlet 2.3.12 with org.restlet.ext.jackson

I've got this method on a Resource that is used to dispatch some calls internally via RIAP:

protected Response dispatch(Method method, String url, Map body) {
	Request req = new Request(method, "riap://component" + url);
	JacksonRepresentation<Map> rep = new JacksonRepresentation<>(body);
	rep.setMediaType(MediaType.APPLICATION_JSON);
	rep.setCharacterSet(CharacterSet.UTF_8);
	req.setEntity(rep);
	Response res = getContext().getClientDispatcher().handle(req);
	getResponse().setStatus(res.getStatus());
	return res;
}

where args are Method.PUT, "/tasks/59706be", and a simple Map such as:

{ "id":"59706be0162c3fed9911315f",
  "name":"Task ONE d",
  "modifiedTimestamp":"2018-01-23T15:28:18.773Z"
}

Since moving to Jackson instead of the default json.org it throws this exception:

Caused by: java.lang.IllegalArgumentException: argument type mismatch
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[?:1.8.0_152]
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) ~[?:1.8.0_152]
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[?:1.8.0_152]
	at java.lang.reflect.Method.invoke(Method.java:498) ~[?:1.8.0_152]
	at org.restlet.resource.ServerResource.doHandle(ServerResource.java:508) ~[classes/:?]

when trying to invoke through reflection the target resource method annotated with @Put
java.lang.reflect.Method.invoke(Object, Object...)
specifically in that method the line 498:
return ma.invoke(obj, args);
where obj is the target resource and args is an array whose only member is the map above.

If I use JsonRepresentation again instead of JacksonRepresentation works fine as before.

[d4nirod]

Same breakpoint on Method.invoke(..) as mentioned before.
It seems it's expecting a Representation but with Jackson is getting the Map instead.
With JsonRepresentation

With JacksonRepresentation

[Tembrel]

What does the target resource method declaration look like?

…

On Tue, Jan 23, 2018 at 1:52 PM, Daniel Rodríguez ***@***.***> wrote: Same breakpoint on Method.invoke(..) as mentioned before. It seems it's expecting a Representation but with Jackson is getting the Map instead. With JsonRepresentation [image: screen shot 2018-01-23 at 19 39 27] <https://user-images.githubusercontent.com/440496/35294413-cde0384a-0076-11e8-8a14-05d0a57e6907.png> With JacksonRepresentation [image: screen shot 2018-01-23 at 19 47 56] <https://user-images.githubusercontent.com/440496/35294346-9d648086-0076-11e8-8ddd-d7a38321cb36.png> — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#1312 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAOeykH1qkys-MDTNL9SpXZKlO-dKSTGks5tNiqJgaJpZM4RqD9n> .

[d4nirod]

@RestResource(path = "/tasks/{task_id}", access = Access.PROTECTED)
public class TaskResource extends SingleEntityResource<CWTask> {
  //...
  // Update task 
  @Put("json")
  @RequestData(params = {IODocs.NAME, IODocs.PROJECT_ID, IODocs.TaskParams.CANCELED, IODocs.FAVORITE, IODocs.MODIFIED_TIMESTAMP})
  public Representation updateTask(Representation entity) {

SingleEntityResource extends our BaseResource which in turn extends org.restlet.resource.ServerResource

[d4nirod]

A little more digging seems to point to the JacksonConverter?
Stack just before the exception is thrown when invoking the target resource through reflection:

JacksonConverter.toObject(Representation, Class<T>, Resource) line: 208	
	> returns HashMap from JacksonRepresentation<T>.getObject() line: 294	
ConverterService.toObject(Representation, Class<T>, Resource) line: 229	
TaskResource(Resource).toObject(Representation, Class<T>) line: 889	
TaskResource(ServerResource).doHandle(MethodAnnotationInfo, Variant) line: 493	
TaskResource(ServerResource).put(Representation, Variant) line: 1395	
TaskResource(ServerResource).doHandle(Variant) line: 595	
TaskResource(ServerResource).doNegotiatedHandle() line: 662	
TaskResource(ServerResource).doConditionalHandle() line: 348	
TaskResource(ServerResource).handle() line: 1020	
GuiceFinder(Finder).handle(Request, Response) line: 236	

Back to
ConverterService.toObject(Representation, Class<T>, Resource) line: 231
checks result is instance of Representation and since it isn't, it returns the result HashMap as is which is what's is passed in as parameter in TaskResource(ServerResource).doHandle(MethodAnnotationInfo, Variant) line: 508 on invocation
resultObject = annotationInfo.getJavaMethod().invoke(this, parameters.toArray());
Since it expects a Representation throws the mentioned exception.
Any suggestions for a workaround?

[d4nirod]

Comparing DefaultConverter.toObject(Representation, Class<T>, Resource) which is used for JsonRepresentation with
JacksonConverter.toObject(Representation, Class<T>, Resource) which is used for JacksonRepresentation

DefaultConverter.toObject(..) line 248 does

if (target.isAssignableFrom(source.getClass())) {
  result = source;

so result is returned as JsonRepresentation

Whereas in JacksonConverter.toObject(..) line 204 does

if ((target != null) && JacksonRepresentation.class.isAssignableFrom(target)) {
  result = jacksonSource;
} else {
   result = jacksonSource.getObject();
}

and jacksonRepresentation.class.isAssignableFrom(target) evaluates to false so jacksonSource.getObject(); is returned.

This looks like a bug, shouldn't it be target.isAssignableFrom(jacksonSource.getClass()) ?

[d4nirod]

Ok, fixed it with a custom JacksonConverter that just overrides toObject(..) and does target.isAssignableFrom(jacksonSource.getClass()) seems to do the trick.

[Tembrel]

Don't you want JacksonConverter to convert into the data type you are actually PUTting? You have this:

@Put("json")
...
public Representation updateTask(Representation entity) ...

but I would have expected something like this:

@Put("json")
public MyMap updateTask(MyMap updatedMap) ...

where MyMap is some domain type of yours.

[d4nirod]

Nope, sorry I should have completed the code excerpt with the return statement, it's like this:

@RestResource(path = "/tasks/{task_id}", access = Access.PROTECTED)
public class TaskResource extends SingleEntityResource<CWTask> {
  //...
  // Update task 
  @Put("json")
  @RequestData(params = {IODocs.NAME, IODocs.PROJECT_ID, IODocs.TaskParams.CANCELED, IODocs.FAVORITE, IODocs.MODIFIED_TIMESTAMP})
  public Representation updateTask(Representation entity) {
    // do update stuff and return updated entity as a `HashMap`
    return new JacksonRepresentation(taskSerializer.serialize(task));
  }

The serialize(..): we basically do the serialization ourselves converting the DB entity into a HashMap<String, Object> plus adding some more contextual data that clients need for a particular view so they don't need to do more requests to get expanded details on some bits of data of the principal entity (Task).

[Tembrel]

My understanding is that the whole point of the converters is to allow you to use domain types in the resource method signatures rather than working in terms of Representation. Is there any reason you particularly want to use Representation here rather than working with the underlying domain types directly?

Here's an example. (It uses unparameterized type StringObjMap instead of Map<...>, because there are tricky issues with using parameterized types with converters.)

// Your domain type, adjust to fit your needs.
public class StringObjMap extends Map<String, Object> { ... }
...
@Put("json")
@RequestData(...)
public StringObjMap updateTask(StringObjMap updates) {
  // do update stuff with updates and return new value
  StringObjMap newValue = ...
  return newValue;
}

No need to talk about representations or mention Jackson at the resource level; it's all done behind the scenes by the converter.

[d4nirod]

Hi Trembel,
Thanks for looking into this. I had to ask around since this code is 5 years old, way before I joined the team. Seems to be a historical reason, someone decided to go this way perhaps because then it was the recommended or obvious thing to do 🤷‍♂️ Additionally we've implemented ETags and it's been built on top of the Representation... anyway changing to returning domain objects is a doable change but it would be subject to lots of conditions and non negligible risks and in the end doesn't seem to solve any current big problem for us.
So

1. Is this Using JacksonRepresentation for RIAP results in java.lang.IllegalArgumentException: argument type mismatch #1312 (comment) a valid workaround for now in your opinion or do you think it could cause problems that I'm not aware of?
2. Is this whole issue an actual bug?

[Tembrel]

I think the original code was doing the right thing, and that your change is in general incorrect. I'm going to go over some of the same ground you covered above, but in more detail.

Before the PUT method, updateTask, is invoked, the server resource handling tries to convert the request entity into an object of the same type as the method parameter, in your case, Representation.

That call to Resource.toObject calls ConverterService.toObject, which eventually ends up at JacksonConverter.toObject.

The first paragraph tries to get a JacksonRepresentation out of the source representation, which in your case presumably contains the raw JSON representing the updates. It has a test to shortcut if some upstream filter, say, has already turned it into a JacksonRepresentation, otherwise it explicitly constructs a new JacksonRepresentation instance with knowledge of the target type.

restlet-framework-java/modules/org.restlet.ext.jackson/src/org/restlet/ext/jackson/JacksonConverter.java

Lines 206 to 212 in 5716362

	// The source for the Jackson conversion
	JacksonRepresentation<?> jacksonSource = null;
	if (source instanceof JacksonRepresentation) {
	jacksonSource = (JacksonRepresentation<?>) source;
	} else if (isCompatible(source)) {
	jacksonSource = create(source, target);
	}

The next paragraph, which is the one you changed, is trying to get the actual object of target type from the JacksonRepresentation. It, too, has a special case for when the target type is JacksonRepresentation (or a subclass), in which case it can use the jacksonSource value directly.

restlet-framework-java/modules/org.restlet.ext.jackson/src/org/restlet/ext/jackson/JacksonConverter.java

Lines 214 to 222 in 5716362

	if (jacksonSource != null) {
	// Handle the conversion
	if ((target != null)
	&& JacksonRepresentation.class.isAssignableFrom(target)) {
	result = jacksonSource;
	} else {
	result = jacksonSource.getObject();
	}
	}

It's the jacksonSource.getObject() call in line 220 that was failing for you. Normally, target is a domain type, like DomainUpdates, for which Jackson has been configured through annotations or some other means. But Jackson doesn't know how to construct a vanilla Representation, which is ultimately why you got the exception.

Your change happened to work because instead of testing whether the target type was a subclass of JacksonRepresentation, you tested whether JacksonRepresentation is a subclass of Representation, which it always is. (jacksonSource.getClass() is JacksonRepresentation or a subclass; it's not the type of the contents.) So even though your method call wasn't asking for a JacksonRepresentation, it happened to work, anyway.

Have you tried simply using JacksonRepresentation as the parameter type (undo your change)?

[d4nirod]

Thank you for the detailed explanation, I do appreciate all the handholding here.
Yes, if I change it like you suggest, it works for RIAPs but then normal (external/HTTP) requests fail with the same kind of error. In any case it'd be an ugly fix since it'd require changing the method signatures for all of our 125+ Resources! and tie them up to a particular implementation of Representation so if tomorrow we want to change from Jackson to whatever we have to redoit again.

There is a piece of information missing here. This problem was originally occurring with RIAP requests only, the external ones worked fine.
For performance reasons we use RIAP to batch several requests at once. We have a BatchResource mapped to path /batch that receives POST requests like this:

{
  "requests": [
    {  "path":"tasks/59706be0162c3fed9911315f",
       "method":"PUT",
       "body":{
         "id":"59706be0162c3fed9911315f",
         "name":"Task ONE zzzz",
         "modifiedTimestamp":"2018-01-23T15:28:18.773Z"
       }
    },
    {   "path": "feeds/59706be0162c3fed9911315f",
        "method": "GET"
    }
  ]
}

Converts the payload to a Map and then generates n subsequent RIAP requests, one for each requests element which is dispatched to the appropriate resource which is "protocol-agnostic", ie. we can call them either through HTTP or RIAP (shown in the first code excerpt on my first post #1312 (comment) see the dispatch method signature which has url ("path"), method and body matching the elements in JSON requests array). The responses for each request are then assembled and returned to client.

---

In the ServerResource.doHandle()... etc code you just mentioned

restlet-framework-java/modules/org.restlet/src/org/restlet/resource/ServerResource.java

Lines 502 to 507 in 5716362

	// Assume there is content to be read.
	// NB: it does not handle the case where the size is
	// unknown, but there is no content.
	parameter = toObject(getRequestEntity(),
	parameterType);

then

restlet-framework-java/modules/org.restlet/src/org/restlet/resource/Resource.java

Lines 894 to 903 in 5716362

	public <T> T toObject(Representation source, Class<T> target)
	throws ResourceException {
	T result = null;
	if (source != null) {
	try {
	org.restlet.service.ConverterService cs = getConverterService();
	result = cs.toObject(source, target, this);
	} catch (ResourceException e) {
	throw e;

With a direct, non-RIAP request the entity coming in is of type InputRepresentation:
source is an InputRepresentation and target is Representation, it uses the DefaultConverter so in DefaultConverter.toObject(..):

• if (target.isAssignableFrom(source.getClass())) is true

With RIAP requests (from our BatchResource.dispatch(..)):

• Using JsonRepresentation: source is an InputRepresentation (!!?) and target is Representation, it uses again the DefaultConverter so in DefaultConverter.toObject(..):
  • if (target.isAssignableFrom(source.getClass())) is true
• Using JacksonRepresentation: source is an JacksonRepresentation and target is Representation, it uses the JacksonConverter so in JacksonConverter.toObject(..):
  • JacksonRepresentation.class.isAssignableFrom(target) is false --> ERROR

---

The only way I see it works for both cases is by using JsonRepresentation in our dispatch(..) since we needed to replace json.org for Jackson in the first place is a kind of inconsistent.

[Tembrel]

As you noted, you have the fix for your immediate problem: Replace the original JacksonConverter in the engine with your modified JacksonConverter that just returns the jacksonSource object if the target type is a supertype of JacksonRepresentation. (I shared code for replaceConverter some time ago.)

Again, I don't think the code to JacksonConverter is wrong. It's meant for the case when you want Jackson to convert a representation into some non-representation type. Should it be more lenient, like DefaultConverter? Maybe, but I think it's worth waiting to see if other people have this need.

[d4nirod]

Ok, I just needed confirmation that the custom Jackson converter workaround I implemented wasn't something completely off the mark. My converter replacement code is very similar to the one you posted.
It would be nice to have this behaviour implemented in the the library without the need for workarounds but for our situation this settles the issue for now.

Many thanks for your help,
Daniel