Include json tags for the name of the field

[kddejong]

https://github.com/aws-cloudformation/cloudformation-cli-go-plugin/blob/f7249d18d5feaa31e50b0bd108528510b4d72f6e/python/rpdk/go/templates/types.go.tple#L15

As the provided regex below supports lower case letters we should include the json name tags.
https://github.com/aws-cloudformation/cloudformation-cli/blob/04ead469a37db6d0efdb357d8fa66fd84e0f1c09/src/rpdk/core/data/schema/provider.definition.schema.v1.json#L316

Also it will help if we are going to rename it in the struct {{ name|uppercase_first_letter }}

[parsnips]

The generated models don't work with lower case schema names at all... Consider this schema:

{
  "typeName": "TX::Database::Schema",
  "description": "Schema resource for TxLayer database.",
  "sourceUrl": "https://github.com/txlayer/aws",
  "definitions": {
    "Table": {
      "type": "object",
      "description": "A table in TxLayer",
      "properties": {
        "name": {
          "type": "string"
        },
        "version": {
          "type": "string"
        },
        "schema": {
          "type": "object"
        },
        "policy": {
          "type": "object"
        }
      },
      "additionalProperties": false
    }
  },
  "properties": {
    "Table": {
      "$ref": "#/definitions/Table"
    },
    "TableId": {
      "type": "string"
    },
    "Tenant": {
      "description": "The tenant identifier to run as.",
      "type": "string",
      "format": "uuid"
    }
  },
  "additionalProperties": false,
  "required": [
    "Table",
    "Tenant"
  ],
  "readOnlyProperties": [
    "/properties/TableId"
  ],
  "primaryIdentifier": [
    "/properties/TableId"
  ],
  "handlers": {
    "create": {
      "permissions": [
      ]
    },
    "read": {
      "permissions": [
      ]
    },
    "update": {
      "permissions": [
      ]
    },
    "delete": {
      "permissions": [
      ]
    },
    "list": {
      "permissions": [
      ]
    }
  }
}

This generates the model:

// Code generated by 'cfn generate', changes will be undone by the next invocation. DO NOT EDIT.
// Updates to this type are made my editing the schema file and executing the 'generate' command.
package resource

// Model is autogenerated from the json schema
type Model struct {
    Table   *Table  `json:",omitempty"`
    TableId *string `json:",omitempty"`
    Tenant  *string `json:",omitempty"`
}

// Table is autogenerated from the json schema
type Table struct {
    Name    *string                `json:",omitempty"`
    Version *string                `json:",omitempty"`
    Schema  map[string]interface{} `json:",omitempty"`
    Policy  map[string]interface{} `json:",omitempty"`
}

When https://github.com/aws-cloudformation/cloudformation-cli-go-plugin/blob/f7249d18d5feaa31e50b0bd108528510b4d72f6e/cfn/handler/request.go#L96-L98 is called down in the guts of Unstringify

https://github.com/aws-cloudformation/cloudformation-cli-go-plugin/blob/f7249d18d5feaa31e50b0bd108528510b4d72f6e/cfn/encoding/unstringify.go#L212

None of the fields of will populate and you'll get empty instance of Model.

If you add the tag names, then you'll encounter this error when Unstringify is trying to populate child keys of Schema or Policy (ex payload https://gist.github.com/parsnips/c4297765281f4c787d9c00dd8a610b57):

Unable to complete request: Marshaling: Unable to convert type
caused by: Unsupported type interface{}

Looking at the implementation of Unstringify, I dont understand the purpose of unmarshaling to a map and then trying to reflect over the json tags. Why not just use json.Unmarshal on whatever type the user passes in? Another alternative is to make public:

https://github.com/aws-cloudformation/cloudformation-cli-go-plugin/blob/f7249d18d5feaa31e50b0bd108528510b4d72f6e/cfn/handler/request.go#L37-L38

So end users can do their own Unmarshal.

[parsnips]

Looking at the implementation of Unstringify, I dont understand the purpose of unmarshaling to a map and then trying to reflect over the json tags. Why not just use json.Unmarshal on whatever type the user passes in? Another alternative is to make public:

I now understand 😂 Using #185 I discover that the json you send in your template, is not the json the handler receives. That's unexpected!

Handler got:

https://gist.github.com/parsnips/c4297765281f4c787d9c00dd8a610b57#file-payload-json-L18

Whereas my template clearly had that as a boolean:

https://gist.github.com/parsnips/6730bba43c10947f2f335f8ec31ff46a#file-template-yaml-L56

[kddejong]

@parsnips still doing some testing but I've been able to get my model to be populated by using json tags. The Stringify and Unstringify is taking me a little time to figure out how to handle as well.

Name                       *string              `json:"name,omitempty"`

[parsnips]

@kddejong here's a patch with a failing unit test that illustrates the Unmarshal throwing that error on nested json objects.

https://gist.github.com/parsnips/65a4c91affe57f0a003c59258834adc8

[jamestelfer]

@parsnips I'm new to this, but I understood that the identifier names used in the schema correspond to identifiers used in the Cloudformation template. Since that seems to have a fairly strong PascalCase convention, does it make sense for custom types to be allowed to deviate from that?

If this is a rule however, it would be good if the CLI would reject the definition with a reason. The specification doesn't seem to enforce PascalCase for property names, and not even for type names, despite their being an obvious convention for both.