smithy icon indicating copy to clipboard operation
smithy copied to clipboard

Prelude shapes are not available in JSON AST models built via `smithy-build`

Open crossleydominic opened this issue 2 years ago • 4 comments

Context

We’re starting to integrate smithy into our toolchain as a means to define interfaces and generate both clients and services. We’re authoring interfaces using the smithy IDL, building the models to the JSON AST using smithy-build/smithy-cli and passing the resulting model.json as input to a code-generator we’re developing in-house. This code-generator is written in TypeScript and does not depend on any java base classes provided in the smithy repo.

The issue

The built model.json that smithy-build emits (when running a “source” build: https://awslabs.github.io/smithy/1.0/guides/building-models/gradle-plugin.html#building-a-source-model) does not contain any of the shapes defined in the prelude (smithy.api namespace). Prelude shapes are explicitly filtered out without an ability to override this behaviour. See:https://github.com/awslabs/smithy/blob/3a6a9a6ef43be5bc8768a7d8daef85f1a6df9a60/smithy-model/src/main/java/software/amazon/smithy/model/shapes/ModelSerializer.java#L57 Without the prelude shapes being present in the model.json it’s not possible to determine certain properties of the model.

This isn’t an issue for any tooling that is built on-top of the java base classes provided because model assembly (via e.g. ModelAssembler and Prelude classes) ensures that the prelude shapes are merged into the in-memory model which is then supplied to plugins; plugins get to see a fully realised model. This is an issue for consumers that only depend on the model.json when using source builds.

Example

Given the interface:

namespace test.auth

@httpBasicAuth
service Example { version: "1.0" }

A source build emits model.json containing

{
    "smithy": "1.0",
    "shapes": {
        "test.auth#Example": {
            "type": "service",
            "traits": {
                "smithy.api#httpBasicAuth": {}
            },
            "version": "1.0"
        }
    }
}

This JSON AST model contains a reference to smithy.api#httpBasicAuth but not its definintion. Without the definition of this prelude shape in the model then it can’t be determined whether or not httpBasicAuth is an authentication trait or not (by the presence of the authDefinintion trait on it). This is one part of determining what the effective authentication traits exist on a service. See: https://github.com/awslabs/smithy/blob/3a6a9a6ef43be5bc8768a7d8daef85f1a6df9a60/smithy-model/src/main/java/software/amazon/smithy/model/knowledge/ServiceIndex.java#L206 This functionality cannot currently be implemented in our code-generator without the definition of the prelude shapes in the model.json.

I don’t think any type of projection we could supply to smithy-build would emit the prelude in JSON AST format, prelude shapes are always filtered out.

Proposals

We’re looking for some advice before we prep a PR on the best way to modify the code such that the prelude is available in JSON AST form so that we can make it available to our code-generator. There’s a few ways I can think to update the codebase to do this:

  1. Add a command-line flag to smithy-cli’s build command, e.g. --emitPreludeShapes to write the prelude shapes into the model.json. All existing consumers will not be supplying this flag so existing behvaviour will be preserved by default. This flag will effectively be threading some configuration down to ModelSerializer to allow the overriding of this line: https://github.com/awslabs/smithy/blob/3a6a9a6ef43be5bc8768a7d8daef85f1a6df9a60/smithy-model/src/main/java/software/amazon/smithy/model/shapes/ModelSerializer.java#L57
    The call stack from smithy-cli down to this layer of the code looks like this:

    software.amazon.smithy.model.shapes.ModelSerializer.serialize(ModelSerializer.java:65)
    software.amazon.smithy.build.plugins.ModelPlugin.serializeModel(ModelPlugin.java:41)
    software.amazon.smithy.build.plugins.ModelPlugin.execute(ModelPlugin.java:37)
    software.amazon.smithy.build.SmithyBuildImpl.applyPlugin(SmithyBuildImpl.java:387)
    software.amazon.smithy.build.SmithyBuildImpl.applyProjection(SmithyBuildImpl.java:319)
    software.amazon.smithy.build.SmithyBuildImpl.executeSerialProjection(SmithyBuildImpl.java:222)
    software.amazon.smithy.build.SmithyBuildImpl.lambda$applyAllProjections$5(SmithyBuildImpl.java:187)
    software.amazon.smithy.build.SmithyBuildImpl.applyAllProjections(SmithyBuildImpl.java:197)
    software.amazon.smithy.build.SmithyBuild.build(SmithyBuild.java:160)
    software.amazon.smithy.cli.commands.BuildCommand.execute(BuildCommand.java:130)
    software.amazon.smithy.cli.Cli.run(Cli.java:144)
    software.amazon.smithy.cli.SmithyCli.run(SmithyCli.java:91)
    software.amazon.smithy.cli.SmithyCli.main(SmithyCli.java:57)
    

    The configuration parameter would have to be threaded through SmithyBuild, SmithyBuildImpl, ModelPlugin down to ModelSerializer. Thoughts on the cleanest way to do this?

  2. Add a whole new command to smithy-cli, something like prelude-ast, that simply emits the prelude as a JSON AST to standard-out (or a file maybe). If the prelude model JSON AST is available, even as a separate JSON structure/file, we can merge the prelude JSON into the existing model.json at runtime within our code-generator.

  3. Any other suggestions welcome!

crossleydominic avatar Oct 20 '21 14:10 crossleydominic

The prelude is, per the spec, inherently part of every model. So any parsers need to build in an understanding of them. Having the model exposed better would be a good thing to do still, though I'm not sure how we go about it. I'll chat with the team about this.

In the meantime, is there any particular reason you're not using our typescript generators?

  • https://github.com/awslabs/smithy-typescript
  • https://github.com/aws/aws-sdk-js-v3

JordonPhillips avatar Oct 21 '21 10:10 JordonPhillips

Thanks for the response!

This makes sense. JSON AST models shouldn't have the prelude shapes in them because the prelude is implicit, which kind of rules out option 1 I presented.

It seems that the smithy library itself builds in it's knowledge of the prelude via loading/parsing the prelude.smithy file located here: https://github.com/awslabs/smithy/blob/main/smithy-model/src/main/resources/software/amazon/smithy/model/loader/prelude.smithy Because our tooling is based upon the JSON AST and not the smithy language itself we can't follow this same pattern of loading the prelude.smithy and merging this into our in-memory models. As it currently stands we'd have to re-implement the prelude as-is in JSON AST form such that our tooling can load it and merge it into the model in a similar manner. My concern is that this essentially duplicate version of the prelude in JSON AST form could get out of date with the prelude.smithy when/if it changes. Option 2 as presented above (with a potentially better command name, or a flag to the existing ast command) would allow the prelude to be available in JSON AST form but be derived from the authoritative prelude.smithy such that for a given version of the smithy tooling the JSON AST and IDL representations of the prelude would never diverge.

On your final point: Our code-generators are written in TypeScript and target Haskell as the language to generate for, which is why we're not using the typescript libraries you linked to, apologies for not being clear.

Thanks

crossleydominic avatar Oct 21 '21 12:10 crossleydominic

I think something like a cli command could work. If we wanted to make it look like a normal smithy model package, we could create a separate project within the smithy repo that extracts the model from smithy-model and then builds it or otherwise produces output that mimics a normal smithy build output. Either way, when the rest of the team wakes up I'll talk to them about it.

Our code-generators are written in TypeScript and target Haskell

Oh now that's interesting! What made you decide to go that route over using the Java tooling?

JordonPhillips avatar Oct 21 '21 13:10 JordonPhillips

It was a difficult choice to be honest. The primary motivating factors were:

  • A lot of the heavy lifting of adopting smithy (e.g. parsing/validating/projecting/transforming of models) is already done for us by just using the smithy-cli tooling you provide. We don't need to re-implement everything, just the key parts of of smithy-model and parts of the knowledge indices in order to generate code.
  • Our stack is composed primarily of Haskell and Typescript code, we don't have any Java code in the codebase at the moment. It made more sense from a maintainability point of view to develop our tooling using expertise we already have, rather than introduce another language.
  • It's likely that the code generators we develop will be heavily extended to handle domain specific/cross-cutting concerns unique to our organization. Although we can't gain the benefit of using the base code generation Java base classes it does mean that our in-house code-generators give us greatest flexibility in the future to extend/modify for our use-cases.

crossleydominic avatar Oct 22 '21 08:10 crossleydominic