fix: pass examples given in the FieldDefinition to the OpenAPIMediaTy…

[robswc]

…pe, within request_body.py

Description

• Passes examples given within the FieldDefinition to the OpenAPIMediaType

@post(media_type=MediaType.TEXT)
def create_person(
    self, data: Annotated[DataclassPerson, Body(examples=[Example(summary="this is an example", value={"key": "value"})])], secret_header: str = Parameter(header="secret")
) -> DataclassPerson:
    return data

Fixes

Fixes #3076

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 98.27%. Comparing base (bfeb5e9) to head (bfb7758). Report is 120 commits behind head on develop.

:exclamation: Current head bfb7758 differs from pull request most recent head 2530adf. Consider uploading reports for the commit 2530adf to get more accurate results

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #3128      +/-   ##
===========================================
+ Coverage    98.23%   98.27%   +0.03%     
===========================================
  Files          312      320       +8     
  Lines        14121    14451     +330     
  Branches      2430     2321     -109     
===========================================
+ Hits         13872    14201     +329     
- Misses         107      109       +2     
+ Partials       142      141       -1     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[vkcku]

Thanks for this @robswc! I had a few points/doubts:

1. I think this logic could maybe be moved into __post_init__ of OpenAPIMediaType so that this will work for both request as well as responses. Basically if there are no examples and the provided schema does have examples, then do the required transformation.
2. From my understanding, the keys of the dictionaries are used as the name of the example, so it feels a bit confusing to take the Example.summary as the name of the example.
3. Was there a reason that only examples in the form of dictionaries are considered? What if the response is just a simple integer, or a string? I think we should be able to pass in any type there and the serialization of the generated schema into JSON will handle that properly.

EDIT: Moving the logic into __post_init__ might not work as easily as I expected since the given schema might be a Reference and we can't get the actual Schema instance without being able to access the schema registry. In this case, it's probably better to do it as you've done right now and handle these cases when the request/response bodies are made.

[robswc]

Thanks for the feedback! @guacs

From my understanding, the keys of the dictionaries are used as the name of the example, so it feels a bit confusing to take the Example.summary as the name of the example.

Truthfully, I wasn't sure the best way to handle it so I'm on board with whatever is the best (or least worst!) solution. Any ideas on how to pass the names through? I suppose I could do it by modifying the Example to give it a name but wanted to start simple as possible.

Was there a reason that only examples in the form of dictionaries are considered? What if the response is just a simple integer, or a string? I think we should be able to pass in any type there and the serialization of the generated schema into JSON will handle that properly.

It was a bit of a hail mary as I haven't worked with Sonar before. Had an unrelated issue with linux permissions which was making linting not work but I moved the project to a new drive so I should be able to generate coverage reports now and make sure that works properly! IIRC, value is an Any so that would work best?

Thanks again!

[vkcku]

I suppose I could do it by modifying the Example to give it a name but wanted to start simple as possible.

I don't know about this either. Since Example is part of the OpenAPI spec but it doesn't include a name field as far as I remember.

value is an Any so that would work best

Seems good to me.

[robswc]

I don't know about this either. Since Example is part of the OpenAPI spec but it doesn't include a name field as far as I remember.

Makes sense. I like the idea of making as little modifications as possible.

data: Annotated[SampleClass, Body(examples=[Example(summary="example", value={"name": "John", "age": 30})])]

What about Body in litestar/params.py ?

def Body(
    *,
    const: bool | None = None,
    content_encoding: str | None = None,
    default: Any = Empty,
    description: str | None = None,
    examples: list[Example] | None = None,

And have examples be dict[str, Example] ? Not sure if that would effect the way things currently work though.

I'm still getting familiar with this amazing framework, so not sure the in's and out's quite yet but feel very close to making this work! :)

Thanks again for the help!

[vkcku]

And have examples be dict[str, Example] ? Not sure if that would effect the way things currently work though.

We will have to allow both list[Example as well as dict[str, Example]. So if the user gives a dict[str, Example], then the keys should be used as the name and if it's a list[Example], we should generate the example names. I think you can refer get_formatted_examples on how to handle the automatic generation of the names.

[vkcku]

And have examples be dict[str, Example] ? Not sure if that would effect the way things currently work though.

We will have to allow both list[Example as well as dict[str, Example]. So if the user gives a dict[str, Example], then the keys should be used as the name and if it's a list[Example], we should generate the example names. I think you can refer get_formatted_examples on how to handle the automatic generation of the names.

Btw, this is for backwards compatibility.

[robswc]

And have examples be dict[str, Example] ? Not sure if that would effect the way things currently work though.

We will have to allow both list[Example as well as dict[str, Example]. So if the user gives a dict[str, Example], then the keys should be used as the name and if it's a list[Example], we should generate the example names. I think you can refer get_formatted_examples on how to handle the automatic generation of the names.

Makes sense!

When I attempt to change the types for "Example" in body, or rather, when I pass a dict instead of list for Examples in Body, I get this error:

>               meta_data = Meta(**meta_kwargs)
E               TypeError: `examples` must be a list, got dict

../../../litestar/_signature/model.py:249: TypeError

I think the issue is the name examples, there is an attempt to add it to Meta in the model.py file.

for k, v in kwarg_definition.items():
    if hasattr(Meta, k) and v is not None:
        meta_kwargs[k] = v
    else:
        meta_kwargs["extra"][k] = v

Perhaps it would be easier all around to ditch the idea of naming an example and have it generate using the get_formatted_examples you mentioned? Personally, naming the examples would be great but for my use case just having them there would be priceless!

[vkcku]

And have examples be dict[str, Example] ? Not sure if that would effect the way things currently work though.

We will have to allow both list[Example as well as dict[str, Example]. So if the user gives a dict[str, Example], then the keys should be used as the name and if it's a list[Example], we should generate the example names. I think you can refer get_formatted_examples on how to handle the automatic generation of the names.

Makes sense!

When I attempt to change the types for "Example" in body, or rather, when I pass a dict instead of list for Examples in Body, I get this error:

>               meta_data = Meta(**meta_kwargs)
E               TypeError: `examples` must be a list, got dict

../../../litestar/_signature/model.py:249: TypeError

I think the issue is the name examples, there is an attempt to add it to Meta in the model.py file.

for k, v in kwarg_definition.items():
    if hasattr(Meta, k) and v is not None:
        meta_kwargs[k] = v
    else:
        meta_kwargs["extra"][k] = v

Perhaps it would be easier all around to ditch the idea of naming an example and have it generate using the get_formatted_examples you mentioned? Personally, naming the examples would be great but for my use case just having them there would be priceless!

For now, I think we can stick to using get_formatted_examples like you said. Allowing the explicit naming of examples can be dealt with seperately if there's enough demand for it.

[robswc]

For now, I think we can stick to using get_formatted_examples like you said. Allowing the explicit naming of examples can be dealt with seperately if there's enough demand for it.

Sounds good to me! @guacs - the title was changed to feat (which makes sense, initially I thought it was a bug/fix), I got some CI errors, looked like it wanted me to point to "develop" branch, which I have done. Is there anything else needed on my end?

Thanks again!

[provinzkraut]

Sounds good to me! @guacs - the title was changed to feat (which makes sense, initially I thought it was a bug/fix), I got some CI errors, looked like it wanted me to point to "develop" branch, which I have done. Is there anything else needed on my end?

All good on that front, just needs a review to go in

[github-actions[bot]]

Documentation preview will be available shortly at https://litestar-org.github.io/litestar-docs-preview/3128

[JacobCoffee]

Are we able to rebase against the latest develop branch and get this merged? @robswc

[robswc]

Are we able to rebase against the latest develop branch and get this merged? @robswc

I can look into it! Been super busy lately and IIRC, changing some of the types was the only way I could get it to work with mypy. I'll give it another shot though!

[robswc]

@JacobCoffee I believe I've messed something up while trying to rebase :sweat_smile:

Could you let me know what went wrong if you get a chance?

I get these errors when running the tests:

FAILED tests/unit/test_cli/test_core_commands.py::test_routes_command_options[schema-disabled_no-exclude] - AssertionError: assert 12 == 13
FAILED tests/unit/test_cli/test_core_commands.py::test_routes_command_options[schema-disabled_exclude] - AssertionError: assert 11 == 12

Edit:

Actually, I could just restart this from scratch. I can make a new PR and use the latest develop branch.