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

Specification validation as part of shortcuts #686

Merged
merged 1 commit into from
Oct 13, 2023

Conversation

p1c2u
Copy link
Collaborator

@p1c2u p1c2u commented Oct 9, 2023

No description provided.

@p1c2u p1c2u force-pushed the feature/openapi-spec-validator-remove-deprecations branch 7 times, most recently from 18cace2 to 5140b73 Compare October 13, 2023 18:56
@p1c2u p1c2u changed the title openapi-spec-validator remove deprecations Specification validation as part of shortcuts Oct 13, 2023
@p1c2u p1c2u force-pushed the feature/openapi-spec-validator-remove-deprecations branch from 5140b73 to deb54f6 Compare October 13, 2023 19:23

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@p1c2u p1c2u force-pushed the feature/openapi-spec-validator-remove-deprecations branch from deb54f6 to 1752d35 Compare October 13, 2023 19:28
@codecov
Copy link

codecov bot commented Oct 13, 2023

Codecov Report

Merging #686 (1752d35) into master (0abae09) will decrease coverage by 0.08%.
The diff coverage is 86.66%.

@@            Coverage Diff             @@
##           master     #686      +/-   ##
==========================================
- Coverage   91.22%   91.15%   -0.08%     
==========================================
  Files         126      126              
  Lines        3580     3640      +60     
  Branches      429      430       +1     
==========================================
+ Hits         3266     3318      +52     
- Misses        265      273       +8     
  Partials       49       49              
Files Coverage Δ
openapi_core/shortcuts.py 100.00% <100.00%> (ø)
...penapi_core/unmarshalling/request/unmarshallers.py 79.67% <100.00%> (+0.10%) ⬆️
openapi_core/unmarshalling/unmarshallers.py 85.71% <100.00%> (+0.34%) ⬆️
openapi_core/validation/request/validators.py 87.50% <100.00%> (+0.70%) ⬆️
openapi_core/validation/response/validators.py 79.89% <100.00%> (+1.17%) ⬆️
openapi_core/validation/validators.py 95.27% <100.00%> (+0.27%) ⬆️
openapi_core/unmarshalling/request/protocols.py 73.91% <50.00%> (-5.04%) ⬇️
openapi_core/unmarshalling/response/protocols.py 76.92% <50.00%> (-4.90%) ⬇️
openapi_core/validation/request/protocols.py 70.37% <50.00%> (-3.55%) ⬇️
openapi_core/validation/response/protocols.py 71.42% <50.00%> (-3.58%) ⬇️

@p1c2u p1c2u merged commit 0f5ac8e into master Oct 13, 2023
@p1c2u p1c2u deleted the feature/openapi-spec-validator-remove-deprecations branch October 13, 2023 19:46
@andersk
Copy link
Contributor

andersk commented Oct 19, 2023

This seems wasteful since Spec.from_dict has already validated the spec. This now validates the spec three times, once in Spec.from_dict, once in validate_request, and once in validate_response:

from openapi_core import Spec, validate_request, validate_response
from openapi_core.testing import MockRequest, MockResponse

spec = Spec.from_dict(
    {
        "openapi": "3.1.0",
        "info": {"title": "Test", "version": "1.0.0"},
        "paths": {"/": {"get": {"responses": {"200": {"description": "OK"}}}}},
    },
)
request = MockRequest("http://localhost", "get", "/")
validate_request(request, spec=spec)
response = MockResponse("")
validate_response(request, response, spec=spec)

This adds a very expensive per-request and per-response overhead when the spec is large.

Moreover, the documented workaround doesn’t work; this still validates the spec (as can be seen by misspelling title, say):

from openapi_core import Spec, validate_request, validate_response
from openapi_core.testing import MockRequest, MockResponse

spec = Spec.from_dict(
    {
        "openapi": "3.1.0",
        "info": {"titel": "Test", "version": "1.0.0"},
        "paths": {"/": {"get": {"responses": {"200": {"description": "OK"}}}}},
    },
    validator=None,
)
request = MockRequest("http://localhost", "get", "/")
validate_request(request, spec=spec, spec_validator_cls=None)
response = MockResponse("")
validate_response(request, response, spec=spec, spec_validator_cls=None)

@p1c2u
Copy link
Collaborator Author

p1c2u commented Oct 20, 2023

Hi @andersk

Guess I need to rethink the change. Thanks for your feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants