Validate JSON document during auto-rules creation

[jan-law]

Fixes #530

@andrewazores I found a JsonDeserializer that can reuse requireNonNegative() and I have some questions about how to implement it.

private final Gson gson needs to be modified in order to register the Type Adapter for IntegerDeserializer. Why is gson final?

IntegerDeserializer gets passed JSON values without their keys, which means I'm not sure how to pass the correct Attribute to requireNonNegative().

IntegerDeserializer only works if our numeric values are of type Integer, not int. How will this affect performance?

Here's the output as of now:

$ vi testRule.json

{
   "name": "test",
   "description": "AutoRulesIT automated rule",
   "eventSpecifier": "template=Continuous,type=TARGET",
   "targetAlias": "es.andrewazor.demo.Main",
   "archivalPeriodSeconds": 60,
   "preservedArchives": -10,
   "maxAgeSeconds": 60
}

$ curl -v --insecure -H "Content-Type: application/json"  -d @testRule.json https://0.0.0.0:8181/api/v2/rules/
...
< HTTP/1.1 400 Bad Request
< content-type: application/json
< content-length: 131
< 
* Connection #0 to host 0.0.0.0 left intact
{"meta":{"type":"text/plain","status":"Bad Request"},"data":{"reason":"\"archivalPeriodSeconds\" cannot be negative, was \"-10\""}}

Some of the other approaches I've thought of include parsing params.getBody() before calling gson.fromJson(...) or using a Rule.Builder similar to the JSON form pathway. I'd like to avoid parsing the JSON more than once though, and am not sure how to efficiently map key/value pairs from the params.getBody() string to the Rule.Builder.

[andrewazores]

private final Gson gson needs to be modified in order to register the Type Adapter for IntegerDeserializer. Why is gson final?

The gson instance, and generally all other constructor-assigned fields that are provided by dependency injection, is final because the field reference should never need to be overwritten. Changing the field reference implies that the injected dependency is no longer being used, which violates the purpose of injecting the common dependency. In the case of the Gson instance, it's because the MainModule provides a single instance so that we have uniform behaviour of (de)serialization across the application.

IntegerDeserializer gets passed JSON values without their keys, which means I'm not sure how to pass the correct Attribute to requireNonNegative().

This makes me think this may not be the correct deserializer to use, then. If I'm understanding correctly, this is what Gson will use when deserializing something in JSON that looks like an integer value, so that you could instead map that to a different type than the Java Integer.

IntegerDeserializer only works if our numeric values are of type Integer, not int. How will this affect performance?

It's probably not of much concern.

I'd like to avoid parsing the JSON more than once though, and am not sure how to efficiently map key/value pairs from the params.getBody() string to the Rule.Builder.

Another possible path along this line of thought is to extract the validation that currently occurs in the Rule constructor (which is called by the Builder.build()) into a method like Rule.validate(). The Rule constructor can then call validate() at the end, after it has assigned all of its own fields. In the JSON parsing pathway of the handler, the handler can call rule.validate() on the Rule instance that is created by Gson.

However, the cleanest approach may be to use a deserializer/adapter, so that Gson itself knows how to deal with invalid Rule JSON definitions, and we don't have to ensure we call rule.validate() everywhere one may be deserialized. Our existing GsonJmxServiceUrlAdapter could be a good reference for this.

[andrewazores]

Another possible path along this line of thought is to extract the validation that currently occurs in the Rule constructor (which is called by the Builder.build()) into a method like Rule.validate(). The Rule constructor can then call validate() at the end, after it has assigned all of its own fields. In the JSON parsing pathway of the handler, the handler can call rule.validate() on the Rule instance that is created by Gson.

However, the cleanest approach may be to use a deserializer/adapter, so that Gson itself knows how to deal with invalid Rule JSON definitions, and we don't have to ensure we call rule.validate() everywhere one may be deserialized. Our existing GsonJmxServiceUrlAdapter could be a good reference for this.

You know, now that I've said this, maybe the correct approach is to combine them. There should be a Rule#validate() method which the Rule constructor can call to ensure its own field validity, and this can also be called by the Gson adapter after deserializing the Rule JSON.

[andrewazores]

This is probably for later enhancements in this same area:

https://stackoverflow.com/questions/21626690/gson-optional-and-required-fields

We could use an approach like this to create annotations and register adapters/deserializers to perform validations, like ensuring deserialized reference-type fields are non-null, or numeric types are positive, etc.

[jan-law]

gson will successfully deserialize -0 as 0. Should we leave this as is?

[andrewazores]

That's fine.

$ jshell
|  Welcome to JShell -- Version 11.0.11
|  For an introduction type: /help intro

jshell> -0 == 0
$1 ==> true

[jan-law]

I'm done making changes. PR is ready to merge