Maillol / aiohttp-pydantic

Aiohttp View that validates request body and query sting regarding the annotations declared in the View method
MIT License
67 stars 21 forks source link

add support of Optional[Union[T]] #22

Closed ffkirill closed 3 years ago

ffkirill commented 3 years ago

Hello! I made this fix for case when query_param of type Optional[Union[T]] is reported as required

Code:

async def get(self, /, tags: Optional[Union[List[int],int]] = None) -> r200[List[Image]]: ...

codecov-commenter commented 3 years ago

Codecov Report

Merging #22 (c1a63e5) into main (43d2789) will decrease coverage by 0.06%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #22      +/-   ##
==========================================
- Coverage   92.60%   92.54%   -0.07%     
==========================================
  Files          11       11              
  Lines         690      684       -6     
==========================================
- Hits          639      633       -6     
  Misses         51       51              
Impacted Files Coverage Δ
aiohttp_pydantic/oas/view.py 94.49% <100.00%> (-0.29%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 43d2789...c1a63e5. Read the comment docs.

Maillol commented 3 years ago

The annotation Optional[Union[List[int],int]] does not make sens.

tag: List[T] means the tag can be used several time in the url:

"http://aiohttp-pydantic.com?tag=a&tag=b&tag=c"

So we cannot have Union[List[T], T]

Optional[T] is short hand for Union[T, None] but that does not mean optional.

In fact an argument shoul be reported as optional if it has not a defaut value.

https://pydantic-docs.helpmanual.io/usage/models/#required-fields

If tag can be used 0 or multiple time in the url, use:

async def get(self, /, tags:List[int]=Field(default_factory=list)) -> r200[List[Image]]: ... and tag should be reported as optional.

If tag can be used 0 or 1 time in the url, we can use:

async def get(self, /, tags: Optional[int]=None) -> r200[List[Image]]: ... Or

async def get(self, /, tags: int=0) -> r200[List[Image]]: ...

and the both should be reported as optional.

In fact _handle_optional should be removed only parameter without default value should be reported as required.

Maillol commented 3 years ago

Released in version v10.0.1