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
372 stars 74 forks source link

[BUG] type-safe-api appending "operation" to some operationIds #789

Closed jstrunk closed 1 month ago

jstrunk commented 4 months ago

Describe the bug

For some of the operations with Python Lambda handlers defined in my OpenAPI spec, the operationId gets changed and has "operation" appended. When this happens, a handler file is created without the extra string, but the CDK infra has it in the handler path string.

Expected Behavior

All of the operations defined in the spec should be named consistently, and the infrastructure should be consistent with module names.

Current Behavior

Given a main.yaml including the following:

  /generateSpec:
    post:
      operationId: generateSpec
      x-handler:
        language: python
      requestBody:
...
  /listSessions:
    get:
      description: List all sessions started in a time range.
      operationId: listSessions
      x-handler:
        language: python
      requestBody:
...

generates packages/api/generated/infrastructure/typescript/src/functions.ts

/**
 * Options for the GenerateSpecFunction construct
 */
export interface GenerateSpecFunctionProps extends Omit<FunctionProps, 'code' | 'handler' | 'runtime'> {}

/**
 * Lambda function construct which points to the python implementation of GenerateSpec
 */
export class GenerateSpecFunction extends Function {
  constructor(scope: Construct, id: string, props?: GenerateSpecFunctionProps) {
    super(scope, id, {
      runtime: Runtime.PYTHON_3_12,
      handler: "api_detective_api_python_handlers.generate_spec.handler",
      code: Code.fromAsset(path.resolve(__dirname, "..",
        "../../../handlers/python/dist/lambda",
      )),
      tracing: Tracing.ACTIVE,
      timeout: Duration.seconds(30),
      ...props,
    });
  }
}
...
/**
 * Options for the ListSessionsOperationFunction construct
 */
export interface ListSessionsOperationFunctionProps extends Omit<FunctionProps, 'code' | 'handler' | 'runtime'> {}

/**
 * Lambda function construct which points to the python implementation of ListSessionsOperation
 */
export class ListSessionsOperationFunction extends Function {
  constructor(scope: Construct, id: string, props?: ListSessionsOperationFunctionProps) {
    super(scope, id, {
      runtime: Runtime.PYTHON_3_12,
      handler: "api_detective_api_python_handlers.list_sessions_operation.handler",
      code: Code.fromAsset(path.resolve(__dirname, "..",
        "../../../handlers/python/dist/lambda",
      )),
      tracing: Tracing.ACTIVE,
      timeout: Duration.seconds(30),
      ...props,
    });
  }
}

The handler file created for listSessions is inconsistent with the handler property. packages/api/handlers/python/api_detective_api_python_handlers/generate_spec.py packages/api/handlers/python/api_detective_api_python_handlers/list_sessions.py

Reproduction Steps

I have no idea what's triggering this.

Possible Solution

No response

Additional Information/Context

I've created two new resources on this API in the last week. Both had this weird behavior. The first was named commentSpec. It was able to generate everything without "operation" appended when I changed the name to iterateSpec, but only after I deleted the nx cache, the packages/api directory, ran pdk, and restored the non-generated files.

PDK version used

0.23.38 and 0.23.40

What languages are you seeing this issue on?

Typescript, Python

Environment details (OS name and version, etc.)

MacOS 14.4.1 intel

jstrunk commented 4 months ago

After renaming the operationId to getSessionList, it generated correct output. There was no need to delete the nx cache, the packages/api directory, ran pdk, and restore the non-generated files, after all.

Perhaps it's because I named the schemas for the request and response ListSessionsRequest and ListSessionsResponse?

Here are snippets from the two operations that caused me problems:

  /generateSpec/{session_id}/comment:
    post:
      operationId: iterateSpec
      x-handler:
        language: python
      parameters:
        - name: session_id
          in: path
          required: true
          schema:
            type: string
      requestBody:
        content:
          'application/json':
            schema:
              $ref: '#/components/schemas/CommentSpecRequest'
      responses:
        200:
          description: Successful response
          content:
            'application/json':
              schema:
                $ref: '#/components/schemas/InitGenSpecResponse'
...
  /listSessions:
    get:
      description: List all sessions started in a time range.
      operationId: getSessionList
      x-handler:
        language: python
      requestBody:
        content:
          'application/json':
            schema:
              $ref: '#/components/schemas/ListSessionsRequest'
      responses:
        200:
          description: Successful response
          content:
            'application/json':
              schema:
                $ref: '#/components/schemas/ListSessionsResponse'
...
github-actions[bot] commented 2 months ago

This issue is now marked as stale because it hasn't seen activity for a while. Add a comment or it will be closed soon. If you wish to exclude this issue from being marked as stale, add the "backlog" label.

github-actions[bot] commented 1 month ago

Closing this issue as it hasn't seen activity for a while. Please add a comment @mentioning a maintainer to reopen. If you wish to exclude this issue from being marked as stale, add the "backlog" label.

cogwirrel commented 2 weeks ago

Hi Jeff,

Sorry this got auto-closed, I'm just back from leave!

So I think what's causing this is indeed naming your input schema {{operationId}}Request! You can also reproduce it by defining an inline requestBody in your OpenAPI specification which will implicitly create a structure named as such, eg:

paths:
  /hello:
    post:
      operationId: sayHello
      x-handler:
        language: typescript
      requestBody:
        content:
          application/json:
            schema:
              type: object
              properties:
                message:
                  type: string
              required:
                - message

This causes us to hit this special case built into the TypeScript OpenAPI generator, which appends Operation to the operation ID:

https://github.com/OpenAPITools/openapi-generator/blob/ff1fe256d8a98aad3a1104d11716996dcc2e8788/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/TypeScriptFetchClientCodegen.java#L858-L863

I think the special case is there because the TypeScript OpenAPI generator creates an {{operationId}}Request structure itself for the whole HTTP request including the headers, url etc.

Unfortunately we don't have control over that part of the generator, but we can work around it by not using inline requestBody schemas, or not naming structures {{operationId}}Request.

Note that we always avoid this when using Smithy as the model language as it names the request body: {{operationId}}RequestContent.

I'm going to leave this closed as I don't think there's much we can do here unfortunately, but I hope this clarifies things at least! :)

Cheers, Jack