Support non-Object varargs; fix primitive handling.

[mslipper]

Fixes #198.

[mslipper]

@brettwooldridge please review.

@gaborbernat @kostapc - please review as well if possible.

[brettwooldridge]

@mslipper Thanks for the fast work!

Running with your branch, most the initial exceptions we were seeing are now resolved. These methods were mostly of the form:

List<String> getAllHardwareVendors(@WebParam(name = "network") String... network)

Unfortunately, I did run into an issue:

09:59:26,204 [JsonRpcBasicServer] [Jetty-60] WARN  - Error in JSON-RPC Service
 com.fasterxml.jackson.databind.exc.MismatchedInputException: Cannot deserialize instance of java.lang.String[] out of VALUE_STRING token
 at [Source: UNKNOWN; line: -1, column: -1]
	at com.fasterxml.jackson.databind.exc.MismatchedInputException.from(MismatchedInputException.java:63) ~[jackson-databind-2.9.1.jar:2.9.1]
	at com.fasterxml.jackson.databind.DeserializationContext.reportInputMismatch(DeserializationContext.java:1329) ~[jackson-databind-2.9.1.jar:2.9.1]
	at com.fasterxml.jackson.databind.DeserializationContext.handleUnexpectedToken(DeserializationContext.java:1138) ~[jackson-databind-2.9.1.jar:2.9.1]
	at com.fasterxml.jackson.databind.DeserializationContext.handleUnexpectedToken(DeserializationContext.java:1092) ~[jackson-databind-2.9.1.jar:2.9.1]
	at com.fasterxml.jackson.databind.deser.std.StringArrayDeserializer.handleNonArray(StringArrayDeserializer.java:314) ~[jackson-databind-2.9.1.jar:2.9.1]
	at com.fasterxml.jackson.databind.deser.std.StringArrayDeserializer.deserialize(StringArrayDeserializer.java:132) ~[jackson-databind-2.9.1.jar:2.9.1]
	at com.fasterxml.jackson.databind.deser.std.StringArrayDeserializer.deserialize(StringArrayDeserializer.java:21) ~[jackson-databind-2.9.1.jar:2.9.1]
	at com.fasterxml.jackson.databind.ObjectMapper._readValue(ObjectMapper.java:3972) ~[jackson-databind-2.9.1.jar:2.9.1]
	at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:2330) ~[jackson-databind-2.9.1.jar:2.9.1]
	at com.googlecode.jsonrpc4j.JsonRpcBasicServer.convertJsonToParameters(JsonRpcBasicServer.java:528) ~[jsonrpc4j-1.5.1.jar:1.5.1]
	at com.googlecode.jsonrpc4j.JsonRpcBasicServer.invoke(JsonRpcBasicServer.java:476) [jsonrpc4j-1.5.1.jar:1.5.1]
	at com.googlecode.jsonrpc4j.JsonRpcBasicServer.handleObject(JsonRpcBasicServer.java:351) [jsonrpc4j-1.5.1.jar:1.5.1]
	at com.googlecode.jsonrpc4j.JsonRpcBasicServer.handleJsonNodeRequest(JsonRpcBasicServer.java:276) [jsonrpc4j-1.5.1.jar:1.5.1]
	at com.googlecode.jsonrpc4j.JsonRpcBasicServer.handleRequest(JsonRpcBasicServer.java:244) [jsonrpc4j-1.5.1.jar:1.5.1]
	at com.googlecode.jsonrpc4j.JsonRpcServer.handleRequest0(JsonRpcServer.java:185) [jsonrpc4j-1.5.1.jar:1.5.1]
	at com.googlecode.jsonrpc4j.JsonRpcServer.handle(JsonRpcServer.java:144) [jsonrpc4j-1.5.1.jar:1.5.1]
	at com.dancer.zap.rest.RestDispatcherServlet.doPost(RestDispatcherServlet.java:84) [com-dancer-zap-rest.jar:?]
	at javax.servlet.http.HttpServlet.service(HttpServlet.java:707) [javax.servlet-api-3.1.0.jar:3.1.0]
	at javax.servlet.http.HttpServlet.service(HttpServlet.java:790) [javax.servlet-api-3.1.0.jar:3.1.0]
	at org.eclipse.jetty.servlet.ServletHolder.handle(ServletHolder.java:833) [jetty-servlet-9.4.7.v20170914.jar:9.4.7.v20170914]
	... [Jetty stack truncated]

This exception is encountered against a method with the following signature:

PageData retrieveHistory(@WebParam(name = "pageData") PageData pageData, @WebParam(name = "networks") String... networks)

When replacing your revised 1.5.1 jar with the 1.5.0 jar, the exception is not encountered.

EDIT: Not that it matters in this case, but just for completeness, this is what the PageData parameter looks like that precedes the String... vararg:

public class PageData {
    private int offset;
    private int pageSize;
    private int total;
    private List<HistoryItem> historyItems;
    ...
   [getters and setters]
}

[mslipper]

Got it. I'll update this PR with fixes for the above issue as well. Since there were some fairly significant regressions between 1.5.0 and 1.5.1 I'll push a release as soon as this PR gets merged.

[brettwooldridge]

@mslipper Awesomeness.

[mslipper]

@brettwooldridge PR updated. Give it a shot now.

[brettwooldridge]

@mslipper Thanks. Unfortunately, there is an "edge case" not covered ... though I'm not sure it's very "edgy".

Using the aforementioned retrieveHistory() method...

retrieveHistory(@WebParam(name = "pageData") PageData pageData, @WebParam(name = "networks") String... networks)

And calling it with a single String in JSON (not an array) for the networks parameter, we get this exception:

14:29:12,774 [JsonRpcBasicServer       ] [Jetty-60                 ] WARN  - Error in JSON-RPC Service
 com.fasterxml.jackson.databind.exc.MismatchedInputException: Cannot deserialize instance of java.lang.String[] out of VALUE_STRING token
 at [Source: UNKNOWN; line: -1, column: -1]
	at com.fasterxml.jackson.databind.exc.MismatchedInputException.from(MismatchedInputException.java:63) ~[jackson-databind-2.9.1.jar:2.9.1]
	at com.fasterxml.jackson.databind.DeserializationContext.reportInputMismatch(DeserializationContext.java:1329) ~[jackson-databind-2.9.1.jar:2.9.1]
	at com.fasterxml.jackson.databind.DeserializationContext.handleUnexpectedToken(DeserializationContext.java:1138) ~[jackson-databind-2.9.1.jar:2.9.1]
	at com.fasterxml.jackson.databind.DeserializationContext.handleUnexpectedToken(DeserializationContext.java:1092) ~[jackson-databind-2.9.1.jar:2.9.1]
	at com.fasterxml.jackson.databind.deser.std.StringArrayDeserializer.handleNonArray(StringArrayDeserializer.java:314) ~[jackson-databind-2.9.1.jar:2.9.1]
	at com.fasterxml.jackson.databind.deser.std.StringArrayDeserializer.deserialize(StringArrayDeserializer.java:132) ~[jackson-databind-2.9.1.jar:2.9.1]
	at com.fasterxml.jackson.databind.deser.std.StringArrayDeserializer.deserialize(StringArrayDeserializer.java:21) ~[jackson-databind-2.9.1.jar:2.9.1]
	at com.fasterxml.jackson.databind.ObjectMapper._readValue(ObjectMapper.java:3972) ~[jackson-databind-2.9.1.jar:2.9.1]
	at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:2330) ~[jackson-databind-2.9.1.jar:2.9.1]
	at com.googlecode.jsonrpc4j.JsonRpcBasicServer.convertJsonToParameters(JsonRpcBasicServer.java:528) ~[jsonrpc4j-1.5.1.jar:1.5.1]
	at com.googlecode.jsonrpc4j.JsonRpcBasicServer.invoke(JsonRpcBasicServer.java:476) [jsonrpc4j-1.5.1.jar:1.5.1]
	at com.googlecode.jsonrpc4j.JsonRpcBasicServer.handleObject(JsonRpcBasicServer.java:351) [jsonrpc4j-1.5.1.jar:1.5.1]
	at com.googlecode.jsonrpc4j.JsonRpcBasicServer.handleJsonNodeRequest(JsonRpcBasicServer.java:276) [jsonrpc4j-1.5.1.jar:1.5.1]
	at com.googlecode.jsonrpc4j.JsonRpcBasicServer.handleRequest(JsonRpcBasicServer.java:244) [jsonrpc4j-1.5.1.jar:1.5.1]
	at com.googlecode.jsonrpc4j.JsonRpcServer.handleRequest0(JsonRpcServer.java:185) [jsonrpc4j-1.5.1.jar:1.5.1]
	at com.googlecode.jsonrpc4j.JsonRpcServer.handle(JsonRpcServer.java:144) [jsonrpc4j-1.5.1.jar:1.5.1]
...

Apparently, "implicit wrapping" of single values used to work in v1.5.0, but now does not.

When debugging into that jackson method, I see that canWrap is false. It seems that the ObjectMapper/Deserializer used by jsonrpc4j needs to be configured to enable that behavior.

It looks like jackson was revved from v2.7.2 in jsonrpc4j v1.5.0 up to version v2.8.5 in jsonrpc4j v1.5.1. I don't know if this was a change in jackson default behavior, or whether there was another jsonrpc4j change that altered this behavior...

[mslipper]

Ok, understood. I'll keep digging. Apologies for the back-and-forth.

[brettwooldridge]

@mslipper It looks like we are already employing jackson v2.9.1 in our stack. So, jsonrpc4j v1.5.0 + jackson v2.9.1 was a working combination, whereas jsonrpc4j v1.5.1 (incl. your branch) + jackson v2.9.1 fails due to the disablement of single parameter implicit wrapping.

This implies to me that there was not a change in jackson behavior per se when jsonrpc4j revved from v2.7.1 -> v2.8.5, but more like a change in how jsonrpc4j configures/constructs jackson.

[brettwooldridge]

@mslipper No apologies needed. First, it wasn't your regression. Second, this is open source and it's awesome that you are pitching in to help at all.

jsonrpc4j has lacked a solid test suite, which would have uncovered this before v1.5.1 got out the door, so all the tests you have added will definitely help going forward.

[mslipper]

Thanks @brettwooldridge, I really appreciate it. I reproduced the case you just described; setting ACCEPT_SINGLE_VALUE_AS_ARRAY fixes it. I'm going to see if I can identify the root cause prior to committing any changes. I'll need a few hours.

[mslipper]

I added the ACCEPT_SINGLE_VALUE_AS_ARRAY setting to the ObjectReader instance used in convertJsonToParameters. It doesn't look like ObjectMapper's instantiation changed from 1.5.0 to 1.5.1 (unless you're passing in an instance from your code), so I'm not sure yet how the issue was introduced originally. I'll continue to dig. In any case, this should fix the issue.

To make sure, I understood the following to be the type of broken JSON payload described above:

{"method":"mixedObjectStringVarargMethodWithJsonRpcParam","id":1,"jsonrpc":"2.0","params":{"strings":"foo","object":{"test":"value"}}}

Where strings is your varargs @WebParam. Is this correct? If so, then the issue should be resolved. I added tests for both cases (array wrapped/single value) for good measure. Let me know if there are any other issues!

[brettwooldridge]

@mslipper Yes, the JSON payload you describe is correct.

Interestingly, we do pass an ObjectMapper into jsonrpc4j, however, mysteriously simply swapping the jsonrpc4j v1.5.0 jar with the v1.5.1 jar surfaces the failure, and swapping back fixes it -- all without recompiling our code.

Our code simply does this:

@Override
public void init() throws ServletException {
   ObjectMapper mapper = new ObjectMapper();
   mapper.registerModule(new JaxbAnnotationModule());

   jsonServer = new JsonRpcMultiServer(mapper);
   jsonServer.setErrorResolver(new StackTraceErrorResolver());
}

As I said above, that code remains constant, and simply swapping out the jsonrpc4j jar in the deployment either fails (v1.5.1) or doesn't fail (v1.5.0).

[brettwooldridge]

@mslipper I can confirm that your latest change resolved the issue.

[mslipper]

Great. Will merge and release now.