aws / aws-pdk

The AWS PDK provides building blocks for common patterns together with development tools to manage and build your projects.
https://aws.github.io/aws-pdk/
Apache License 2.0
343 stars 71 forks source link

[FEATURE] Smithy mixin input and de-nested API call #460

Closed zetashift closed 5 months ago

zetashift commented 1 year ago

Describe the feature

If I have the following Smithy shapes:

resource Customer {
    identifiers: {
        /// Unique customer ID.
        customerId: CustomerId
    }
    properties: {
        /// Name of the customer.
        name: String
        createdAt: Timestamp
    }
    create: CreateCustomer
}

/// The data needed to create a new customer.
@mixin
structure CustomerData for Customer {
    @required
    $name
}

/// A single record from all the known customers.
structure CustomerRecord for Customer with [CustomerData] {
    @required
    $customerId
    @required
    $createdAt
    @required
    $name
}

/// Create a new customer.
@http(method: "POST", uri: "/customers")
operation CreateCustomer {
    input := with [CustomerData] {}
    output: CustomerRecord
    errors: [AlreadyExistsError]
}

@pattern("^[A-Za-z0-9 ]+$")
string CustomerId

And then generate the client, I have to call the CreateCustomer method as follows:

const response = await apiClient.createCustomer({
  createCustomerRequestContent: { name: customerName },
});

The nesting is slightly annoying, is there anyway to get it flattened, so that the { name: ... } could just be defined as the first parameter:

const response = await apiClient.createCustomer({ name: customerName });

The current case for is that I get the following error:

Argument of type '{ name: string; }' is not assignable to parameter of type 'CreateCustomerRequest'.
  Object literal may only specify known properties, and 'name' does not exist in type

Use Case

I don't need it, it's just a nice to have :P.

Proposed Solution

No response

Other Information

Just do it the current way!

Acknowledgements

PDK version used

0.19.15

What languages will this feature affect?

Typescript

Environment details (OS name and version, etc.)

MacOS ARM64, Ventura

cogwirrel commented 1 year ago

Hey! I agree this would be a nicer interface! 😄 It might be a bit tricky to support however, as the client is generated by the typescript-fetch OpenAPI Generator. We can customise it through templates (which is how we generate the lambda handler wrappers etc) but the capabilities are still a bit limited.

The interface each client method takes accepts all the request parameters (ie path and query params), plus an argument for the request body, eg. an operation like POST /customers/{pathParam}?queryParam=someValue might end up with an interface like: { pathParam: string; queryParam: string; createCustomerRequestContent: { ... } }.

"Unpacking" the body parameter might lead to naming conflicts for users using ModelLanguage.OPENAPI as request parameters and the body are defined separately rather than in the same input structure in Smithy. I'm also not 100% sure if we can easily access each member of the request body parameter to create that kind of input - could be possible but would be a bit of work I think!

Since you're using Smithy, generating code directly from Smithy could be the way to create a more "user friendly" client however, as the generated code can align more closely to the Smithy model rather than the generated OpenAPI specification. There's a plugin called typescript-codegen which can generate a client directly from Smithy, with the docs here: https://smithy.io/2.0/guides/using-code-generation/generating-a-client.html - I haven't used this before so I'm not sure how easy it is to customise things like middleware for auth etc though!

Perhaps this could be an option? You can customise model.options.smithy.smithyBuildOptions to add the dependency and configure the plugin. You might need to add some additional commands to your preCompile task for the model project to copy the generated code out of the build directory and into the src directory of another TypeScriptProject you set up as part of your monorepo to get everything playing nicely together...

Perhaps you could give that a try and see if it works nicely? If it's an improvement we could look at adding a developer guide to the docs which explains how to do it in more detail, or even look at building an option in to TypeSafeApi for Smithy users :)

zetashift commented 1 year ago

Perhaps this could be an option? You can customise model.options.smithy.smithyBuildOptions to add the dependency and configure the plugin. You might need to add some additional commands to your preCompile task for the model project to copy the generated code out of the build directory and into the src directory of another TypeScriptProject you set up as part of your monorepo to get everything playing nicely together...

I'll try it out this weekend! Can't make any guarentees I'll succeed tho :P

cogwirrel commented 1 year ago

Keen to hear how you go! 😄 I might also give it a try over the next few days if I get a spare moment :)

cogwirrel commented 1 year ago

Had a bit of time this evening and had a go! Example repo here! https://github.com/cogwirrel/tsapi-smithy-client-example

It seems to work alright! Note that the auth is built in to the generated client based on what you specify in your Smithy model, so you need eg @sigv4(name: "execute-api") on your service shape for sigv4 auth.

I also extended that example out of curiosity to use the Smithy TypeScript Server SDK for implementing the lambda handlers :) Error handling in the Smithy generated client seems to be a little bit nicer when you use it in combination with the Smithy server sdk, as you can use instanceof checks.

Hope that helps :)

cogwirrel commented 1 year ago

https://github.com/aws/aws-prototyping-sdk/pull/468 adds the error header expected by the Smithy generated client so that you don't lose out on better error handling when using the native type-safe-api handler wrappers.

I think the final step for this issue would be to either:

  1. Add documentation to explain how to generate and use the Smithy typescript client and/or server sdk in your monorepo (ie the steps to reproduce this example repo)
  2. Add "native" support for the Smithy typescript client and server sdk as "libraries", (eg libraries: [Library.SMITHY_TYPESCRIPT_CLIENT, Library.SMITHY_TYPESCRIPT_SERVER_SDK]) to abstract away the changes to smithy-build.json and TypeScriptProject setup, plus documentation on how to set things up.

I'm leaning towards (1), as (2) will add the challenge of keeping dependency versions in sync since we can't use the Smithy generated list of deps/devDeps in a projen TypeScriptProject, which means things may break if users pick a different Smithy version. This is the same for the OpenAPI generated clients though so perhaps it's not too much of a stretch :)

zetashift commented 1 year ago
  1. Add documentation to explain how to generate and use the Smithy typescript client and/or server sdk in your monorepo (ie the steps to reproduce this example repo)

This is a fine solution for me! It doesn't need to be fancy and the nesting isn't that big of a problem.

zetashift commented 11 months ago

Ah this doesn't only happen with mixins, but also with the shorthand operation syntax:

@tags(["Journeys", "Qrd"])
@idempotent
@http(method: "POST", uri: "/qrds/safeqrd/activate")
operation ValidateTag {
    input := {
        @required
        activationCode: String
    }
    output := {
        @required
        isValid: Boolean
    }
}

I have to call this the following way:

const isValid = await journeyClient.validateTag({
  validateTagRequestContent: {
    activationCode,
  },
});
cogwirrel commented 11 months ago

That key like validateTagRequestContent will always be there in the generated client if the operation accepts a request body unfortunately, regardless of how it’s defined in the Smithy spec. Smithy generates an object schema with that name in the OpenApi spec it creates, which is then used as the key name in the generated clients.

It’s still on my todo list to document generating a client directly from Smithy (without going via OpenApi), which results in a client which has types that align much closer to the Smithy model :)

cogwirrel commented 5 months ago

I'm going to close this issue in favour of https://github.com/aws/aws-pdk/issues/687 - as it'd be great if we could natively generate Smithy clients/servers etc