Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve JSON AST model format #189

Closed
mtdowling opened this issue Oct 17, 2019 · 0 comments · Fixed by #213
Closed

Improve JSON AST model format #189

mtdowling opened this issue Oct 17, 2019 · 0 comments · Fixed by #213
Labels
feature-request A feature should be added or improved.

Comments

@mtdowling
Copy link
Member

mtdowling commented Oct 17, 2019

The JSON model format was originally intended to be authored through YAML or Ion files, and as such, we made tradeoffs to make it less verbose. For example, namespaces are just extra top-level keys in the model, traits are extra keys in shapes, etc. Now that we have an IDL format and recommend people author models with that, we could look at improving the JSON format to be easier to parse, easier to load into a semantic model, and easier to grep.

Version number

I recommend that we create a version 0.5.0.

Backward compatibility

We will continue to support loading 0.4.0 JSON AST models and emit deprecation warnings when they are encountered. We will remove support for 0.4.0 when 1.0.0 is released.

Explicit namespaces

Namespaces are currently defined as top-level additional properties.

{
    "smithy": "0.4.0",
    "smithy.example": {
        "shapes": {
            "MyString": {
                "type": "string",
                "documentation": "My documentation string"
            }
        }
    }
}

This is problematic because:

  1. It's generally harder to parse this kind of schema or interact with it in tools like jq.
  2. We might need to add other properties to Smithy models, and relying on additional properties could technically result in a breaking change if a namespace uses the same name as an added property.

I recommend that we add a top-level "shapes" property that contains shape definitions.

{
    "version": "0.5.0",
    "shapes": {
        "smithy.example#String": {
            "type": "string"
        }
    }
}

Absolute shape IDs

The new JSON format will support only absolute shape IDs both when defining and referencing shapes.

{
    "version": "0.5.0",
    "shapes": {
        "smithy.example#String": {
            "type": "string",
            "traits": {
                "smithy.api#documentation": "docs"
            }
        }
    }
}

Traits are stored in the traits property

In the 0.4.0 format, traits are applied to shapes using additional properties on shapes. This makes loading shapes harder than it needs to be since you need to keep a set of know shape properties and skip over key-value pairs for known properties to detect traits. Using additional properties also makes it harder to add new properties to shapes in the future since trait names could potentially conflict with these properties (for example, we're considering adding an "isa" property to structures).

As seen in the above example, traits in 0.5.0 are defined on shapes inside of the "traits" property of a shape. This property is a map of absolute shape IDs to the trait's value. Yes, even traits use absolute shape IDs.

Applying traits is done with a type of "apply"

In 0.4.0, traits applied externally to shapes using the "traits" property of a namespace:

{
    "smithy": "0.4.0",
    "smithy.example": {
        "traits": {
            "MyString": {
                "documentation": "This is my string!"
            }
        }
    }
}

However, this makes working with single-file models using tools like grep, jq, or jmespath harder. For example, imagine if you needed to find all shapes with a specific trait. You'd need to search through both the "shapes" property and "traits" property of every namespace.

There's also an issue with how a file stores models. A single JSON model should not both define a shape and externally apply traits to it. For example, this is a sub-optimal model:

{
    "smithy": "0.4.0",
    "smithy.example": {
        "shapes": {
            "type": "string"
        },
        "traits": {
            "MyString": {
                "documentation": "This is my string!"
            }
        }
    }
}

I propose that we reuse the "shapes" map and add a new "type" called "apply" that is used to apply traits to shapes.

{
    "version": "0.5.0",
    "shapes": {
        "smithy.example#MyString": {
            "type": "apply",
            "traits": {
                "smithy.api#documentation": "This is my string!"
            }
        }
    }
}

This now makes it much simpler to find all shapes with a given trait since every trait is defined in a uniform location in the model. It also makes it impossible to both define a shape and apply traits to a shape externally in the same file.

Normalized relationships

Relationships between shapes will use a normalized object format of a property named "target" that maps to an absolute shape ID. While things like service operations, operation input, operation output, and operation errors are not actually member references that allow traits, using an object form that matches the form of members makes it easier to detect shape references in tools like grep, jq, or jmespath.

UPDATE: Rethinking this, I realized that using $ for some shape relationships but not all would actually introduce an inconsistency in the format. For example, trait values with an idRef trait would not use this syntax, nor would applying traits to shapes since a trait would be the key of a map. A better approach for simpler tools to try to reason about references between shapes would be to look for strings that match the shape ID ABNF.

Example

The following example is a bit longer form that defines several relationships using the normalized format.

{
    "version": "0.5.0",
    "metadata": {
        "xy": "z"
    },
    "shapes": {
        "com.foo#Service": {
            "type": "service",
            "operations": [
                {"target": "com.foo#Operation"}
            ]
        },
        "com.foo#Operation": {
            "type": "operation"
        },
        "com.foo#Baz": {
            "type": "structure",
            "members": {
                "foo": {
                    "target": "smithy.api#String",
                    "traits": {
                        "smithy.api#documentation": "Hello"
                    }
                }
            },
            "traits": {
                "smithy.api#documentation": "Hello"
            }
        },
        "com.foo#Baz$foo": {
            "type": "apply",
            "traits": {
                "smithy.api#sensitive": true
            }
        }
    }
}
@mtdowling mtdowling added the feature-request A feature should be added or improved. label Oct 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant