Request CTO validation is not working

[dselman]

Describe the bug
It is possible to send an invalid request (with respect to the CTO model) and have the Ergo execute.

To Reproduce

1. Download the attached template
2. Using the request and state within the archive
3. cicero execute --request monthlysales-invalid.json --state state.json
4. Note that execution succeeds, when it should fail

Expected behavior
The execution should fail because sales[0].price.doubleValue is a string.

Version

Cicero 0.13.5

[jeromesimeon]

Validation seems to be triggered, so it might be down to Concerto. If you can attach the archive, it would be helpful for reproducing with the same context.

Below is a run on the promissory notes template. The last example is a String value instead of a Double which does not raise the expected validation error.

bash-3.2$ cicero --version
0.13.5
bash-3.2$ cat request-invalid1.json 
{
  "$class": "org.accordproject.promissorynote.Payment",
  "amountPaid": {
    "$class": "org.accordproject.money.MonetaryAmount",
    "doubleValue": null,
    "currencyCode": "USD"
  }
}
bash-3.2$ cicero execute --request request-invalid1.json
8:03:37 AM - info: Using current directory as template folder
8:03:37 AM - info: Loading a default sample.txt file.
8:03:39 AM - warn: A state file was not provided, initializing state. Try the --state flag or create a state.json in the root folder of your template.
8:03:39 AM - error: Instance org.accordproject.promissorynote.Payment#9f9a6eff-1cb7-4be3-8cf7-cdcbc2cc1947 missing required field doubleValue
bash-3.2$ cat request-invalid2.json 
{
  "$class": "org.accordproject.promissorynote.Payment",
  "amountPaid": {
    "$class": "org.accordproject.money.MonetaryAmount",
    "doubleValue": 100,
    "currencyCode": 20
  }
}
bash-3.2$ cicero execute --request request-invalid2.json
8:03:45 AM - info: Using current directory as template folder
8:03:45 AM - info: Loading a default sample.txt file.
8:03:47 AM - warn: A state file was not provided, initializing state. Try the --state flag or create a state.json in the root folder of your template.
8:03:47 AM - error: Instance org.accordproject.promissorynote.Payment#83c232aa-7870-498d-b298-e978eefb8149 invalid enum value 20 for field CurrencyCode
bash-3.2$ cat request-invalid3.json 
{
  "$class": "org.accordproject.promissorynote.Payment",
  "amountPaid": {
    "$class": "org.accordproject.money.MonetaryAmount",
    "doubleValue": 100,
    "currencyCode": "USD",
    "foo": 1
  }
}
bash-3.2$ cicero execute --request request-invalid3.json
8:04:08 AM - info: Using current directory as template folder
8:04:08 AM - info: Loading a default sample.txt file.
8:04:10 AM - warn: A state file was not provided, initializing state. Try the --state flag or create a state.json in the root folder of your template.
8:04:10 AM - error: Unexpected properties for type org.accordproject.money.MonetaryAmount: foo
bash-3.2$ cat request-invalid4.json 
{
  "$class": "org.accordproject.promissorynote.Payment",
  "amountPaid": {
    "$class": "org.accordproject.money.MonetaryAmount",
    "doubleValue": "FOO",
    "currencyCode": "USD"
  }
}
bash-3.2$ cicero execute --request request-invalid4.json
8:04:16 AM - info: Using current directory as template folder
8:04:16 AM - info: Loading a default sample.txt file.
8:04:17 AM - warn: A state file was not provided, initializing state. Try the --state flag or create a state.json in the root folder of your template.
8:04:17 AM - error: [Ergo] Enforce Error at 29:4-37:5 ''
bash-3.2$ 

[jeromesimeon]

Same behaviour in Concerto:

bash-3.2$ concerto --version
0.82.6
bash-3.2$ concerto validate --ctoFiles models/*.cto --sample request-invalid1.json 
8:01:19 AM - error: Instance org.accordproject.promissorynote.Payment#60a72979-cbdc-4199-b6ba-9dab46269fc3 missing required field doubleValue
bash-3.2$ concerto validate --ctoFiles models/*.cto --sample request-invalid2.json 
8:01:24 AM - error: Instance org.accordproject.promissorynote.Payment#85f42a68-9729-4a39-a438-8a2a907b77a6 invalid enum value 20 for field CurrencyCode
bash-3.2$ concerto validate --ctoFiles models/*.cto --sample request-invalid3.json 
8:01:28 AM - error: Unexpected properties for type org.accordproject.money.MonetaryAmount: foo
bash-3.2$ concerto validate --ctoFiles models/*.cto --sample request-invalid4.json 
8:01:32 AM - info:
{
  "$class": "org.accordproject.promissorynote.Payment",
  "amountPaid": {
    "$class": "org.accordproject.money.MonetaryAmount",
    "doubleValue": null,
    "currencyCode": "USD"
  },
  "transactionId": "cfd92767-c7f9-4287-b4b6-495cdd52c6c6",
  "timestamp": "2019-12-06T13:01:32.070Z"
}

[jeromesimeon]

Cicero 0.13.5 was based on Composer Concerto 0.71.6 before it was moved to the Accord Project.

[jeromesimeon]

Concerto uses parseFloat which returns NaN on something that is not a double (e.g., "Foo").

concerto/packages/concerto-core/lib/serializer/jsonpopulator.js

Line 229 in 2c0b62a

result = parseFloat(json);

bash-3.2$ cat ~/test.js
console.log(parseFloat("FOO"));
// expected output: NaN
console.log(JSON.stringify(parseFloat("FOO")));
// expected output: "null"
bash-3.2$ node ~/test.js
NaN
null

[jeromesimeon]

I'm moving this issue to Concerto.

[jeromesimeon]

Concerto is generally quite lose with validation. Bug or feature?

bash-3.2$ cat models/model.cto 
namespace org.accordproject.promissorynote

import org.accordproject.money.MonetaryAmount from https://models.accordproject.org/money.cto

transaction Payment extends Request {
  o MonetaryAmount amountPaid
  o Boolean isValid
  o Integer someNumber
}
bash-3.2$ cat request.json 
{
  "$class": "org.accordproject.promissorynote.Payment",
  "amountPaid": {
    "doubleValue": 1.0,
    "currencyCode": "USD"
  },
  "isValid" : "something",
  "someNumber" : "something else"
}
bash-3.2$ concerto validate --ctoFiles models/*.cto --sample request.json 
8:45:45 AM - info:
{
  "$class": "org.accordproject.promissorynote.Payment",
  "amountPaid": {
    "$class": "org.accordproject.money.MonetaryAmount",
    "doubleValue": 1,
    "currencyCode": "USD"
  },
  "isValid": false,
  "someNumber": null,
  "transactionId": "7a521051-6af6-4aca-8a26-199f66b09753",
  "timestamp": "2019-12-06T13:45:45.698Z"
}

[jeromesimeon]

A proposed resolution is to change the validation semantics to be strict. This means atomic values in the JSON will have to be proper instances of the corresponding atomic type.

New Validation Semantics

1. Boolean values have to be either true or false, or a validation error is thrown
2. Integer and Long values have to be a number without a decimal point or a validation error is thrown
3. Double has to be a number or a validation error is thrown
4. String has to be a string or a validation error is thrown
5. DateTime has to be a string in ISO 8601 format or a validation error is thrown

Breaking changes

Boolean

"true" validate Boolean -> true now an ERROR
3.14 validate Boolean -> false now an ERROR
null validate Boolean -> false now an ERROR
undefined validate Boolean -> false now an ERROR
{} validate Boolean -> false now an ERROR
[] validate Boolean -> false now an ERROR

String

3.14 validate String -> "3.14" now an ERROR
true validate String -> "true" now an ERROR

Double

"3.14" validate Double -> 3.14 now an ERROR
"foo" validate Double -> NaN now an ERROR   // From issue
null validate Double -> NaN now an ERROR

Integer / Long

3.14 validate Integer -> 3 now an ERROR
"3.14" validate Double -> 3.14 now an ERROR
"foo" validate Double -> NaN now an ERROR   // From issue
null validate Double -> NaN now an ERROR

DateTime

"foo" validate DateTime -> Moment.invalid now an ERROR
"December 17, 1995 03:24:00" validate DateTime -> moment("1995-12-17T03:24:00.000") now an ERROR

[jeromesimeon]

Initial PR implementing the proposal is in #161

[jeromesimeon]

Initial PR implementing the proposal is in #161

The change in this PR is great at identifying potential issues in tests. E.g., in Ergo:

accordproject/ergo#724

[jeromesimeon]

Fixed based on proposal in #161 in the 1.0 branch