accordproject / apap

Accord Project Agreement Protocol
2 stars 1 forks source link

JSON Schema generated code accepts different payload to Ergo #10

Closed martinhalford closed 1 year ago

martinhalford commented 1 year ago

Hi @mttrbrts

The JSON was hand-crafted by yours truly in order to match the Rust code's expectation.

This is my logic.

The schema.json file generated by Cicero contains the snippet for the MyRequest object is as follows:

    "org.accordproject.helloworld.MyRequest": {
      "title": "MyRequest",
      "description": "An instance of org.accordproject.helloworld.MyRequest",
      "type": "object",
      "properties": {
        "$class": {
          "type": "string",
          "default": "org.accordproject.helloworld.MyRequest",
          "pattern": "^org\\.accordproject\\.helloworld\\.MyRequest$",
          "description": "The class identifier for org.accordproject.helloworld.MyRequest"
        },
        "input": {
          "type": "string"
        }
      },
      "required": [
        "$class",
        "input"
      ]
    },

The bit that we really only care about is the "properties" object above.

The Rust that was generated from this snippet is;

#[derive(Serialize, Deserialize)]
pub struct OrgAccordprojectHelloworldMyRequestProperties {
    #[serde(rename = "$class")]
    class: Class,
    input: Name,
}

#[derive(Serialize, Deserialize)]
pub struct Class {
    #[serde(rename = "type")]
    class_type: String,
    #[serde(rename = "default")]
    class_default: String,
    pattern: String,
    description: String,
}

#[derive(Serialize, Deserialize)]
pub struct Name {
    #[serde(rename = "type")]
    name_type: String,
}

This Rust code looks sensible to me when compared to the the JSON Schema above.

I hand-crafted the JSON Request object, based on the JSON Schema and Rust code.

When I pass in the below JSON Request object, the code works fine. :-)

{
    "$class": {
        "type": "string",
        "default": "org.accordproject.helloworld.MyRequest",
        "pattern": "^org\\.accordproject\\.helloworld\\.MyRequest$",
        "description": "The class identifier for org.accordproject.helloworld.MyRequest"
    },
    "input": {
        "type": "Accord Project"
    }
}

When I pass the JSON object that is used by Ergo then it fails.

{
    "$class": "org.accordproject.helloworld.MyRequest",
    "input": "Accord Project"
}

The only way I can get the original JSON MyRequest object to work is if I change the Rust code to be:

#[derive(Serialize, Deserialize)]
pub struct OrgAccordprojectHelloworldMyRequestProperties2 {
    #[serde(rename = "$class")]
    class: String,
    input: String,
}

That is, ditch the Class and Name objects and use simple String types instead.

Everything above looks fine to me. It's just that I can't see how Ergo is executing this model, unless Ergo is interpreting the Concerto CTO file differently to JSON Schema.

What am I missing?

martinhalford commented 1 year ago

For completeness, Here is the original message, prior to the message above:


I have generated a JSON Schema from the HelloWorld template....

schema.json

...then used this to generate the Rust library. (Note: I used an online tool to create this rust lib https://app.quicktype.io/. In the fullness of time, I'll incorporate a Rust crate to do this.)

lib.rs

I then wrote a simple Rust app to parse a Request response using the Rust Lib.

main.rs

The challenge is that, according to the Schema.json, the valid HelloWorld MyRequest object is:

{
    "$class": {
        "type": "string",
        "default": "org.accordproject.helloworld.MyRequest",
        "pattern": "^org\\.accordproject\\.helloworld\\.MyRequest$",
        "description": "The class identifier for org.accordproject.helloworld.MyRequest"
    },
    "input": {
        "type": "Accord Project"
    }
}

whereas the one we use in TemplateStudio is:

{
    "$class": "org.accordproject.helloworld.MyRequest",
    "input": "Accord Project"
}

Any ideas why the Schema.json specifies a different MyRequest payload to the one we use with current Ergo?

(FYI... Full repo of this playpen app at: source code)

Am I generating the code from Schema.json incorrectly?

mttrbrts commented 1 year ago

I think that I see the problem.

The JSON Schema file that Concerto generates contains definitions but is not a helpful schema for validating on its own. To use the definitions, you need to include a $ref to the definition that you want to use.

For example, here's an abbreviated JSON Schema definition that matches MyRequest type.

{
    "definitions": {
     "org.accordproject.helloworld.MyRequest": {
      "title": "MyRequest",
      "description": "An instance of org.accordproject.helloworld.MyRequest",
      "type": "object",
      "properties": {
        "$class": {
          "type": "string",
          "default": "org.accordproject.helloworld.MyRequest",
          "pattern": "^org\\.accordproject\\.helloworld\\.MyRequest$",
          "description": "The class identifier for org.accordproject.helloworld.MyRequest"
        },
        "input": {
          "type": "string"
        }
      },
      "required": [
        "$class",
        "input"
      ]
    }
    },
    "$ref": "#/definitions/org.accordproject.helloworld.MyRequest"
}

Then using the "JSON Schema" source type at https://app.quicktype.io/, I produce a Rust file that looks like this....

use serde::{Serialize, Deserialize};

/// An instance of org.accordproject.helloworld.MyRequest
#[derive(Serialize, Deserialize)]
pub struct MyRequest {
    /// The class identifier for org.accordproject.helloworld.MyRequest
    #[serde(rename = "$class")]
    class: String,
    input: String,
}

This better matches what I would expect. My guess is that you generated Rust code using the "JSON" source type rather than "JSON Schema"?

martinhalford commented 1 year ago

Thanks @mttrbrts - That did it!

The "$ref" is the secret ingredient.

I was using the "JSON" source type in https://app.quicktype.io/ because, without the "$ref", no code was being generated when using the "JSON Schema" source type.

SO... I've expanded the "$ref" into a "properties" section with references to all the definitions.

This now allows QuickType to generate Rust structs for all definitions - with the MyRequest and MyResponse payloads as expected.

As a thought... It would be nice if we could get concerto compile to include these properties in the JSONSchema output, if possible - rather than having to hand modify it. What do you think? (Note: I'm happy to have a crack at this, if that helps?)

Source repo.

{
  "$schema": "http://json-schema.org/draft-07/schema#",
  "type": "object",
  "properties": {
    "MyRequest": {
      "$ref": "#/definitions/org.accordproject.helloworld.MyRequest"
    },
    "myResponse": {
      "$ref": "#/definitions/org.accordproject.helloworld.MyResponse"
    },
    "helloWorldClause": {
      "$ref": "#/definitions/org.accordproject.helloworld.HelloWorldClause"
    },
    "request": {
      "$ref": "#/definitions/org.accordproject.runtime.Request"
    },
    "response": {
      "$ref": "#/definitions/org.accordproject.runtime.Response"
    },
    "obligation": {
      "$ref": "#/definitions/org.accordproject.runtime.Obligation"
    },
    "state": {
      "$ref": "#/definitions/org.accordproject.runtime.State"
    },
    "contract": {
      "$ref": "#/definitions/org.accordproject.runtime.Contract"
    },
    "clause": {
      "$ref": "#/definitions/org.accordproject.runtime.Clause"
    }
  },
  "definitions": {
    "org.accordproject.helloworld.MyRequest": {
      "title": "MyRequest",
      "description": "An instance of org.accordproject.helloworld.MyRequest",
      "type": "object",
      "properties": {
        "$class": {
          "type": "string",
          "default": "org.accordproject.helloworld.MyRequest",
          "pattern": "^org\\.accordproject\\.helloworld\\.MyRequest$",
          "description": "The class identifier for org.accordproject.helloworld.MyRequest"
        },
        "input": {
          "type": "string"
        }
      },
      "required": [
        "$class",
        "input"
      ]
    },
   ...etc....etc...
mttrbrts commented 1 year ago

I don't believe that the JSON Schema output should always contain an object as you describe it, because it is unlikely to validate against a JSON object directly.

Alternatively, I suggest one (or both) of the following options:

  1. The JSON Schema code gen should have a parameter to allow a user to optionally specify the concept that they want to want to validate against. Aadrika called this the root type in her work over the summer.

  2. We add a roof definition in the JSON Schema that matches against anyOf the types in the model. A little like the way that we generate union types for typescript. This should also be optional and controlled by a command line flag.

Otherwise, it's not unreasonable to ask your Rust library to generate structs for all definitions, not just the root?

martinhalford commented 1 year ago

@mttrbrts - I can see your point.

However, I wasn't just trying to generate Rust for just a couple of specific concepts but for all concepts; and I'm not sure I like the idea of the user having to figure out which concepts that they want to validate against.

My thinking was that, by default, the JSON Schema and Rust code generator should generate structs for all the object definitions, not just one or two.

This is what I (manually) did above.

By creating "properties" within the JSON Schema, with different "$ref" values pointing to each, individual definition then this forced QuickType to generate Rust structs for the all JSON Schema definitions.

(Note: this means it is still a valid JSON Schema Definition as QuickType only accepts valid JSON Schema Defs.)

In this way, the contract author doesn't have to decide which concepts that they want to validate against, as all are available in the Rust library to be used at compile time.

Given it's simple to parse the JSON Schema and create the "properties" object containing all the "$ref" values then the suggestion is that we modify the JSONSchema Visitor to include these "properties".

Is this unreasonable?

Would having "properties" in the JSON Schema upset other use cases - even if it is still a valid JSON Schema definition?

Is there another, better, more elegant way?

mttrbrts commented 1 year ago

My option two should satisfy what you need, for example, we could generate JSON schema such as...

{
  "$schema": "http://json-schema.org/draft-07/schema#",
  "oneOf": [
    {
      "$ref": "#/definitions/org.accordproject.helloworld.MyRequest"
    },
    {
      "$ref": "#/definitions/org.accordproject.helloworld.MyResponse"
    },
    ...
  ],
  "definitions": {
    "org.accordproject.helloworld.MyRequest": {
      "title": "MyRequest",
      "description": "An instance of org.accordproject.helloworld.MyRequest",
      "type": "object",
      "properties": {
        "$class": {
          "type": "string",
          "default": "org.accordproject.helloworld.MyRequest",
          "pattern": "^org\\.accordproject\\.helloworld\\.MyRequest$",
          "description": "The class identifier for org.accordproject.helloworld.MyRequest"
        },
        "input": {
          "type": "string"
        }
      },
      "required": [
        "$class",
        "input"
      ]
    },
    "org.accordproject.helloworld.MyResponse": {
      "title": "MyRequest",
      "description": "An instance of org.accordproject.helloworld.MyResponse",
      "type": "object",
      "properties": {
        "$class": {
          "type": "string",
          "default": "org.accordproject.helloworld.MyResponse",
          "pattern": "^org\\.accordproject\\.helloworld\\.MyResponse$",
          "description": "The class identifier for org.accordproject.helloworld.MyResponse"
        },
        "input": {
          "type": "string"
        }
      },
      "required": [
        "$class",
        "input"
      ]
    }
  },
  ...
}
martinhalford commented 1 year ago

Hi @mttrbrts - I tried using the above and it doesn't seem to generate the Rust code correctly - at least, not with QuickType. It just generates one big struct "HelloWorld" containing a set of optional strings.

schema-oneOf.json.txt

use serde::{Serialize, Deserialize};

#[derive(Debug, Serialize, Deserialize)]
pub struct HelloWorld {

    #[serde(rename = "$class")]
    pub class: String,

    #[serde(rename = "input")]
    pub input: Option<String>,

    /// The instance identifier for this type
    #[serde(rename = "clauseId")]
    pub clause_id: Option<String>,

    /// The instance identifier for this type
    #[serde(rename = "contractId")]
    pub contract_id: Option<String>,

    #[serde(rename = "name")]
    pub name: Option<String>,

    #[serde(rename = "output")]
    pub output: Option<String>,

    #[serde(rename = "contract")]
    pub contract: Option<String>,

    #[serde(rename = "deadline")]
    pub deadline: Option<String>,

    #[serde(rename = "promisee")]
    pub promisee: Option<String>,

    #[serde(rename = "promisor")]
    pub promisor: Option<String>,
}
martinhalford commented 1 year ago

@mttrbrts - Of course, the other option is that we forget about generating Rust from JSON Schema and just handcraft a Visitor in Concerto and add --target rust as an option?

However, I was, kinda, thinking to (lazily) use QuickType to generate the Rust from JSONSchema, instead of handcrafting Rust code gen logic from scratch. This would, obviously, introduce a dependency which, I appreciate, may not be desirable.

What do you think?

mttrbrts commented 1 year ago

I say that if you're willing to crack open the code gen code, let's just go direct to rust.

martinhalford commented 1 year ago

@mttrbrts - OK. <<< Insert cracking sound >>>