openapi: Use openapi_core ResponseValidator to validate responses.

Signed-off-by: Anders Kaseorg <anders@zulip.com>

pyproject.toml

@@ -79,7 +79,6 @@ module = [
     "netifaces.*",
     "onelogin.*",
     "openapi_core.*",
-    "openapi_schema_validator.*",
     "premailer.*",
     "pyinotify.*",
     "pyoembed.*",

zerver/openapi/openapi.py

@@ -10,11 +10,13 @@ import os
 import re
 from typing import Any, Dict, List, Mapping, Optional, Set, Tuple, Union
 
+import orjson
 from jsonschema.exceptions import ValidationError as JsonSchemaValidationError
 from openapi_core import create_spec
-from openapi_core.testing import MockRequest
+from openapi_core.testing import MockRequest, MockResponse
+from openapi_core.unmarshalling.schemas.exceptions import InvalidSchemaValue
 from openapi_core.validation.request.validators import RequestValidator
-from openapi_schema_validator import OAS30Validator
+from openapi_core.validation.response.validators import ResponseValidator
 
 OPENAPI_SPEC_PATH = os.path.abspath(
     os.path.join(os.path.dirname(__file__), "../openapi/zulip.yaml")
@@ -78,6 +80,7 @@ class OpenAPISpec:
         self._openapi: Dict[str, Any] = {}
         self._endpoints_dict: Dict[str, str] = {}
         self._request_validator: Optional[RequestValidator] = None
+        self._response_validator: Optional[ResponseValidator] = None
 
     def check_reload(self) -> None:
         # Because importing yaml takes significant time, and we only
@@ -104,6 +107,7 @@ class OpenAPISpec:
 
         spec = create_spec(openapi)
         self._request_validator = RequestValidator(spec)
+        self._response_validator = ResponseValidator(spec)
         self._openapi = naively_merge_allOf_dict(JsonRef.replace_refs(openapi))
         self.create_endpoints_dict()
         self.mtime = mtime
@@ -170,6 +174,15 @@ class OpenAPISpec:
         assert self._request_validator is not None
         return self._request_validator
 
+    def response_validator(self) -> RequestValidator:
+        """Reload the OpenAPI file if it has been modified after the last time
+        it was read, and then return the openapi_core validator object. Similar
+        to preceding functions. Used for proper access to OpenAPI objects.
+        """
+        self.check_reload()
+        assert self._response_validator is not None
+        return self._response_validator
+
 
 class SchemaError(Exception):
     pass
@@ -420,22 +433,26 @@ def validate_against_openapi_schema(
         # been added as all 400 have the same schema.  When all 400
         # response have been defined this should be removed.
         return True
-    # The actual work of validating that the response matches the
-    # schema is done via the third-party OAS30Validator.
-    schema = get_schema(endpoint, method, status_code)
+
     if endpoint == "/events" and method == "get":
         # This a temporary function for checking only documented events
         # as all events haven't been documented yet.
         # TODO: Remove this after all events have been documented.
         fix_events(content)
 
-    validator = OAS30Validator(schema)
+    mock_request = MockRequest("http://localhost:9991/", method, "/api/v1" + path)
+    mock_response = MockResponse(
+        # TODO: Use original response content instead of re-serializing it.
+        orjson.dumps(content),
+        status_code=status_code,
+    )
+    result = openapi_spec.response_validator().validate(mock_request, mock_response)
     try:
-        validator.validate(content)
-    except JsonSchemaValidationError as error:
-        if not display_brief_error:
-            raise error
-
+        result.raise_for_errors()
+    except InvalidSchemaValue as isv:
+        message = f"{len(isv.schema_errors)} response validation error(s) at {method} /api/v1{path} ({status_code}):"
+        for error in isv.schema_errors:
+            if display_brief_error:
                 # display_brief_error is designed to avoid printing 1000 lines
                 # of output when the schema to validate is extremely large
                 # (E.g. the several dozen format variants for individual
@@ -455,7 +472,7 @@ def validate_against_openapi_schema(
                     ]
 
                 # Field list from https://python-jsonschema.readthedocs.io/en/stable/errors/
-        raise JsonSchemaValidationError(
+                error = JsonSchemaValidationError(
                     message=error.message,
                     validator=error.validator,
                     path=error.path,
@@ -465,6 +482,8 @@ def validate_against_openapi_schema(
                     validator_value=brief_error_validator_value,
                     cause=error.cause,
                 )
+            message += f"\n\n{type(error).__name__}: {error}"
+        raise SchemaError(message) from None
 
     return True
 

zerver/tests/test_openapi.py

@@ -8,7 +8,6 @@ from unittest.mock import MagicMock, patch
 import yaml
 from django.http import HttpResponse
 from django.utils import regex_helper
-from jsonschema.exceptions import ValidationError
 
 from zerver.lib.request import _REQ, arguments_map
 from zerver.lib.rest import rest_dispatch
@@ -86,7 +85,7 @@ class OpenAPIToolsTest(ZulipTestCase):
 
     def test_validate_against_openapi_schema(self) -> None:
         with self.assertRaisesRegex(
-            ValidationError, r"Additional properties are not allowed \('foo' was unexpected\)"
+            SchemaError, r"Additional properties are not allowed \('foo' was unexpected\)"
         ):
             bad_content: Dict[str, object] = {
                 "msg": "",
@@ -97,7 +96,7 @@ class OpenAPIToolsTest(ZulipTestCase):
                 bad_content, TEST_ENDPOINT, TEST_METHOD, TEST_RESPONSE_SUCCESS
             )
 
-        with self.assertRaisesRegex(ValidationError, r"42 is not of type string"):
+        with self.assertRaisesRegex(SchemaError, r"42 is not of type string"):
             bad_content = {
                 "msg": 42,
                 "result": "success",
@@ -106,7 +105,7 @@ class OpenAPIToolsTest(ZulipTestCase):
                 bad_content, TEST_ENDPOINT, TEST_METHOD, TEST_RESPONSE_SUCCESS
             )
 
-        with self.assertRaisesRegex(ValidationError, r"'msg' is a required property"):
+        with self.assertRaisesRegex(SchemaError, r"'msg' is a required property"):
             bad_content = {
                 "result": "success",
             }
@@ -148,7 +147,7 @@ class OpenAPIToolsTest(ZulipTestCase):
                 "200",
             )
             with self.assertRaisesRegex(
-                ValidationError,
+                SchemaError,
                 r"\{'obj': \{'str3': 'test', 'str4': 'extraneous'\}\} is not valid under any of the given schemas",
             ):
                 validate_against_openapi_schema(