Include json tags for the name of the field

[kddejong]

cloudformation-cli-go-plugin/python/rpdk/go/templates/types.go.tple

Line 15 in f7249d1

{{ name|uppercase_first_letter }} {{ type|translate_type }} `json:",omitempty"`

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

cloudformation-cli-go-plugin/cfn/handler/request.go

Lines 96 to 98 in f7249d1

	if err := encoding.Unmarshal(r.resourcePropertiesBody, v); err != nil {
	return cfnerr.New(marshalingError, "Unable to convert type", err)
	}

is called down in the guts of Unstringify

cloudformation-cli-go-plugin/cfn/encoding/unstringify.go

Line 212 in f7249d1

func Unstringify(data map[string]interface{}, v interface{}) error {

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:

cloudformation-cli-go-plugin/cfn/handler/request.go

Lines 37 to 38 in f7249d1

	previousResourcePropertiesBody []byte
	resourcePropertiesBody []byte

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.