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.

Sign up for GitHub

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

Fix missing Field.default when it's value is None in openapi spec #189

Merged
merged 5 commits into from
Nov 10, 2024
Merged

Fix missing Field.default when it's value is None in openapi spec #189

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ed3fffa
Fix missing `Field.default` when it's value is `None` in openapi spec
ddorian Oct 24, 2024
916cd1e
Faster generation of spec_json
ddorian Oct 26, 2024
2331a26
Assignment follows the `unset` rule
Oct 27, 2024
78484b6
Add test case with "null" parameters.
ddorian Oct 28, 2024
6707a81
Add `test_deprecated_none`
ddorian Oct 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions flask_openapi3/openapi.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,9 @@ def api_doc(self) -> Dict:

# Convert spec to JSON
self.spec_json = json.loads(spec.model_dump_json(by_alias=True, exclude_none=True, warnings=False))
# fix to include `default=None` in fields
components_only = json.loads(spec.model_dump_json(by_alias=True, exclude_unset=True, warnings=False))["components"]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why components only?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other models have many fields with default None that show up in the output (example=None, security=None, deprecated=None etc).

This worked for me on a 5K lines json schema.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree to replace exclude_none with exclude_unset, but I don't think it's perfect just for components.
Can you write an example of an error other than the action components?

Copy link
Contributor Author

@ddorian ddorian Oct 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using this:

self.spec_json = spec.model_dump(mode="json", by_alias=True, exclude_unset=True, warnings=False)

These are the fields that get removed:

RequestBody.required = True

These are the extra fields that show up in json. I guess they are being set from functions.

Operation.deprecated = None
Operation.externalDocs = None
Opeation.example = None
Operation.examples = None
Operation.servers = None
Operation.security = None
Parameter.deprecated = None
Parameter.example = None
Parameter.examples = None

So we can do my way or fix functions that set the values above to None.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some models end up in parameters, so just using the unset rule in components is not a good solution.

I committed a commit with all assignments using the unset rule.

Hopefully we can agree.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works great for me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After my 2 commits my tests are fine and the json is valid openapi spec.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@luolingchun can you merge my 2 PRs?

self.spec_json["components"] = components_only

# Update with OpenAPI extensions
self.spec_json.update(**self.openapi_extensions)
Expand Down
24 changes: 23 additions & 1 deletion tests/test_openapi.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from __future__ import annotations

from typing import Generic, TypeVar, List
from typing import Generic, TypeVar, List, Optional

from pydantic import BaseModel, Field

Expand Down Expand Up @@ -495,3 +495,25 @@ def endpoint_test(header: HeaderParam, cookie: CookieParam):
"required": False,
"schema": {"description": "app name", "example": "aaa", "title": "App Name", "type": "string"}
}



class Model(BaseModel):
one: Optional[int] = Field(default=None)
two: Optional[int] = Field(default=2)

def test_default_none(request):
test_app = OpenAPI(request.node.name)
test_app.config["TESTING"] = True

@test_app.post("/test")
def endpoint_test(body:Model):
print([]) # pragma: no cover


works = Model.model_json_schema()["properties"]
assert works["one"]["default"] is None
assert works["two"]["default"] ==2
breaks = test_app.api_doc["components"]["schemas"]["Model"]["properties"]
assert breaks["two"]["default"] == 2
assert breaks["one"]["default"] is None