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

[BUG] (type-safe-api) openapi arrays in responses generate faulty python runtime code #904

Closed
jstrunk opened this issue Feb 12, 2025 · 2 comments · Fixed by #905
Closed
Labels
bug Something isn't working needs-triage

Comments

@jstrunk
Copy link
Contributor

jstrunk commented Feb 12, 2025

Describe the bug

When an API response includes an array, the python generated handler wrapper calls to_dict() on the list causing an AttributeError.

Expected Behavior

The Lambda function successfully returns the response.

Current Behavior

When the response includes an array, Python raises the following exception and the Lambda fails:

[ERROR] AttributeError: 'list' object has no attribute 'to_dict'
Traceback (most recent call last):
  File "/var/task/amzn_smart_product_onboarding_api_python_runtime/api/operation_config.py", line 1269, in wrapper
    response_body = response.body.to_json()
  File "/var/task/amzn_smart_product_onboarding_api_python_runtime/models/metaclass_response_content.py", line 52, in to_json
    return json.dumps(self.to_dict())
  File "/var/task/amzn_smart_product_onboarding_api_python_runtime/models/metaclass_response_content.py", line 77, in to_dict
    _dict['possibleCategories'] = self.possible_categories.to_dict()

Reproduction Steps

Here is a sample project with a few modifications to the default code. The handler unit test passes for test_say_hello_should_return_list, but fails for test_response_to_json and test_handler.

.projenrc.ts

import { monorepo } from "@aws/pdk";
import {
  Language,
  ModelLanguage,
  TypeSafeApiProject,
} from "@aws/pdk/type-safe-api";
import { javascript } from "projen";

const project = new monorepo.MonorepoTsProject({
  devDeps: ["@aws/pdk"],
  name: "pdk-python-api-sample",
  packageManager: javascript.NodePackageManager.PNPM,
  projenrcTs: true,
  prettier: true,
});

new TypeSafeApiProject({
  parent: project,
  outdir: "packages/api",
  name: "myapi",
  model: {
    language: ModelLanguage.OPENAPI,
    options: {
      openapi: {
        title: "sample api",
      },
    },
  },
  infrastructure: {
    language: Language.TYPESCRIPT,
  },
  handlers: {
    languages: [Language.PYTHON],
  },
});

project.synth();

packages/api/model/src/main/openapi/main.yaml

openapi: 3.0.3
info:
  version: 1.0.0
  title: sample api
paths:
  /hello:
    get:
      operationId: sayHello
      x-handler:
        language: python
      responses:
        200:
          description: Successful response
          content:
            'application/json':
              schema:
                $ref: '#/components/schemas/SayHelloResponseContent'
        500:
          description: An internal failure at the fault of the server
          content:
            'application/json':
              schema:
                $ref: '#/components/schemas/InternalFailureErrorResponseContent'
        400:
          description: An error at the fault of the client sending invalid input
          content:
            'application/json':
              schema:
                $ref: '#/components/schemas/BadRequestErrorResponseContent'
        403:
          description: An error due to the client not being authorized to access the resource
          content:
            'application/json':
              schema:
                $ref: '#/components/schemas/NotAuthorizedErrorResponseContent'
components:
  schemas:
    SayHelloResponseContent:
      type: object
      properties:
        message:
          type: string
        testList:
          type: array
          items:
            type: string
      required:
        - message
        - testList
    InternalFailureErrorResponseContent:
      type: object
      properties:
        message:
          type: string
      required:
        - message
    BadRequestErrorResponseContent:
      type: object
      properties:
        message:
          type: string
      required:
        - message
    NotAuthorizedErrorResponseContent:
      type: object
      properties:
        message:
          type: string
      required:
        - message

packages/api/handlers/python/myapi_python_handlers/say_hello.py

from myapi_python_runtime.models import *
from myapi_python_runtime.response import Response
from myapi_python_runtime.interceptors import INTERCEPTORS
from myapi_python_runtime.interceptors.powertools.logger import LoggingInterceptor
from myapi_python_runtime.api.operation_config import (
    say_hello_handler, SayHelloRequest, SayHelloOperationResponses
)


def say_hello(input: SayHelloRequest, **kwargs) -> SayHelloOperationResponses:
    """
    Type-safe handler for the SayHello operation
    """
    LoggingInterceptor.get_logger(input).info("Start SayHello Operation")

    return Response.success(SayHelloResponseContent(message="hello", testList=["foo", "bar", "baz"]))


# Entry point for the AWS Lambda handler for the SayHello operation.
# The say_hello_handler method wraps the type-safe handler and manages marshalling inputs and outputs
handler = say_hello_handler(interceptors=INTERCEPTORS)(say_hello)

packages/api/handlers/python/test/test_say_hello.py

from unittest.mock import Mock

import pytest
from aws_lambda_powertools import Logger

from myapi_python_handlers.say_hello import say_hello, handler
from myapi_python_runtime.api.operation_config import (
    SayHelloRequest, SayHelloResponseContent
)


@pytest.fixture
def request_arguments():
    """
    Fixture for constructing common request arguments
    """
    return {
        "event": {},
        "context": None,
        "interceptor_context": {
            "logger": Logger(),
        },
    }


def test_say_hello_should_return_list(request_arguments):
    response = say_hello(SayHelloRequest(
        **request_arguments,
        request_parameters=None,
        body=None,
    ))

    assert response.status_code == 200
    assert response.body.message == "hello"
    assert response.body.test_list == ["foo", "bar", "baz"]


def test_response_to_json():
    response = SayHelloResponseContent(
        message="hello",
        testList=["foo", "bar", "baz"],
    )
    assert response.to_json() == '{"message":"hello","test_list":["foo","bar","baz"]}'


def test_handler():
    context = Mock()
    response = handler({},  context)
    assert response["statusCode"] == 200
    assert response["body"] == '{"message":"hello","testList":["foo","bar","baz"]}'

Possible Solution

I tried using model_dump_json() instead of to_json() in the handler wrapper, and my tests passed.

From packages/api/generated/runtime/python/myapi_python_runtime/api/operation_config.py

def say_hello_handler(_handler: SayHelloHandlerFunction = None, interceptors: List[SayHelloInterceptor] = []):
    """
    Decorator for an api handler for the say_hello operation, providing a typed interface for inputs and outputs
    """
    def _handler_wrapper(handler: SayHelloHandlerFunction):
        @wraps(handler)
        def wrapper(event, context, additional_interceptors = [], **kwargs):
...
            response_body = ''
            if response.body is None:
                pass
            elif response.status_code == 200:
                response_body = response.body.model_dump_json(by_alias=True, exclude_unset=True)
            elif response.status_code == 400:
                response_body = response.body.model_dump_json(by_alias=True, exclude_unset=True)
            elif response.status_code == 403:
                response_body = response.body.model_dump_json(by_alias=True, exclude_unset=True)
            elif response.status_code == 500:
                response_body = response.body.model_dump_json(by_alias=True, exclude_unset=True)
...

Additional Information/Context

This did not occur in previous versions using OpenAPI generator.

PDK version used

0.25.16

What languages are you seeing this issue on?

Python

Environment details (OS name and version, etc.)

MacOS 15.3

@jstrunk jstrunk added bug Something isn't working needs-triage labels Feb 12, 2025
@jstrunk
Copy link
Contributor Author

jstrunk commented Feb 12, 2025

My plan is to replace the call to to_json() in operationConfig.ejs to call model_dump_json(by_alias=True, exclude_unset=True) as suggested in the TODO in the models.ejs and update the implementation of to_json() in models.ejs to call model_dump_json instead in case anyone is calling to_json() directly.

I also think I should fix to_dict() in case anyone calls it directly. @cogwirrel, Why do we need the extra code to override the behavior of model_dump?

    This has the following differences from calling pydantic's
   `self.model_dump(by_alias=True)`:
   
   * `None` is only added to the output dict for nullable fields that
     were set at model initialization. Other fields with value `None`
     are ignored.

While exploring models.ejs , it looks like it isn't properly handling the case where the field is an array of primitives.

In this sample OpenAPI spec, this template generates the following code:
packages/api/generated/runtime/python/myapi_python_runtime/models/say_hello_response_content.py

...
    def to_dict(self) -> Dict[str, Any]:
        """Return the dictionary representation of the model using alias.

        This has the following differences from calling pydantic's
        `self.model_dump(by_alias=True)`:

        * `None` is only added to the output dict for nullable fields that
          were set at model initialization. Other fields with value `None`
          are ignored.
        """
        _dict = self.model_dump(
            by_alias=True,
            exclude={
            },
            exclude_none=True,
        )
        # override the default output from pydantic by calling `to_dict()` of test_list
        if self.test_list:
            _dict['testList'] = self.test_list.to_dict()
        return _dict
...

The only usage of this to_dict() method other than to_json() appears to be in packages/api/generated/runtime/python/myapi_python_runtime/api_client.py in the sanitize_for_serialization() method.

This bug appears to come from the complexity of this template. I think the from and to methods could be simplified, and therefore eliminate bugs, by only using Pydantic's built in methods, but that would be another issue and PR.

jstrunk added a commit to jstrunk/aws-pdk that referenced this issue Feb 12, 2025
The template wasn't handling models with arrays of primitives properly, and it was getting handled
by the last else if statement for non-primitives. This change excludes arrays from the case.

fix aws#904
@jstrunk
Copy link
Contributor Author

jstrunk commented Feb 12, 2025

The PR makes the minimal change to fix the current bug. My sample code's tests pass after whitespace corrections.

cogwirrel pushed a commit that referenced this issue Feb 12, 2025
…905)

The template wasn't handling models with arrays of primitives properly, and it was getting handled
by the last else if statement for non-primitives. This change excludes arrays from the case.

fix #904
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs-triage
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant