Merge pull request #305 from josephaxisa/fix/oas30-ignore-ref-siblings

Ignore $ref sibling keywords in OAS 3.0 validators

Author: p1c2u

openapi_schema_validator/_keywords.py

@@ -232,7 +232,7 @@ def required(
     for property in required:
         if property not in instance:
             prop_schema = schema.get("properties", {}).get(property)
-            if prop_schema:
+            if prop_schema and "$ref" not in prop_schema:
                 read_only = prop_schema.get("readOnly", False)
                 write_only = prop_schema.get("writeOnly", False)
                 if (
@@ -256,7 +256,7 @@ def read_required(
     for property in required:
         if property not in instance:
             prop_schema = schema.get("properties", {}).get(property)
-            if prop_schema:
+            if prop_schema and "$ref" not in prop_schema:
                 write_only = prop_schema.get("writeOnly", False)
                 if getattr(validator, "read", True) and write_only:
                     continue
@@ -274,7 +274,7 @@ def write_required(
     for property in required:
         if property not in instance:
             prop_schema = schema.get("properties", {}).get(property)
-            if prop_schema:
+            if prop_schema and "$ref" not in prop_schema:
                 read_only = prop_schema.get("readOnly", False)
                 if read_only:
                     continue

openapi_schema_validator/validators.py

@@ -111,6 +111,7 @@ def _build_oas30_validator() -> Any:
         # See https://github.com/python-openapi/openapi-schema-validator/pull/12
         # version="oas30",
         id_of=_oas30_id_of,
+        applicable_validators=_legacy_keywords.ignore_ref_siblings,
     )
 
 

tests/integration/test_validators.py

@@ -764,6 +764,65 @@ def test_nullable_schema_combos(
                 validator.validate({"testfield": None})
                 assert False
 
+    def test_ref_ignores_sibling_keywords(self, validator_class):
+        schema = {
+            "$ref": "#/components/schemas/ItemTarget",
+            "required": ["name"],
+            "components": {
+                "schemas": {
+                    "PlainId": {
+                        "type": "integer",
+                    },
+                    "NullableName": {
+                        "type": "string",
+                        "nullable": True,
+                        "minLength": 2,
+                    },
+                    "ItemTarget": {
+                        "type": "object",
+                        "required": ["id"],
+                        "properties": {
+                            "id": {
+                                "$ref": "#/components/schemas/PlainId",
+                                "type": "integer",
+                                "nullable": True,
+                            },
+                            "name": {
+                                "$ref": "#/components/schemas/NullableName",
+                                "type": "string",
+                                "nullable": False,
+                                "minLength": 10,
+                            },
+                            "code": {
+                                "$ref": "#/components/schemas/NullableName",
+                                "type": "integer",
+                            },
+                        },
+                    },
+                },
+            },
+        }
+        validator = validator_class(
+            schema,
+            format_checker=oas30_format_checker,
+        )
+
+        assert validator.validate({"id": 1}) is None
+        assert (
+            validator.validate({"id": 1, "name": None, "code": "abc"}) is None
+        )
+
+        with pytest.raises(
+            ValidationError, match="'id' is a required property"
+        ):
+            validator.validate({"name": "valid_name"})
+
+        with pytest.raises(ValidationError, match="None for not nullable"):
+            validator.validate({"id": None})
+
+        with pytest.raises(ValidationError, match="is too short"):
+            validator.validate({"id": 1, "name": "a"})
+
 
 class TestOAS30ReadWriteValidatorValidate:
     def test_read_only(self):
@@ -886,6 +945,90 @@ def test_write_only_false(self):
         )
         assert validator.validate({"some_prop": "hello"}) is None
 
+    def test_ref_ignores_sibling_read_write_keywords(self):
+        schema = {
+            "type": "object",
+            "required": ["id", "secret", "token"],
+            "properties": {
+                "id": {
+                    "$ref": "#/components/schemas/PlainInt",
+                    "readOnly": True,
+                },
+                "secret": {
+                    "$ref": "#/components/schemas/PlainString",
+                    "writeOnly": True,
+                },
+                "token": {
+                    "$ref": "#/components/schemas/ReadOnlyString",
+                    "readOnly": False,
+                },
+            },
+            "components": {
+                "schemas": {
+                    "PlainInt": {"type": "integer"},
+                    "PlainString": {"type": "string"},
+                    "ReadOnlyString": {"type": "string", "readOnly": True},
+                },
+            },
+        }
+
+        write_validator = OAS30WriteValidator(
+            schema,
+            format_checker=oas30_format_checker,
+        )
+        read_validator = OAS30ReadValidator(
+            schema,
+            format_checker=oas30_format_checker,
+        )
+        default_validator = OAS30Validator(
+            schema,
+            format_checker=oas30_format_checker,
+        )
+
+        # Sibling readOnly: True on `id` and writeOnly: True on `secret`
+        # are ignored
+        assert (
+            read_validator.validate({"id": 1, "secret": "abc", "token": "tok"})
+            is None
+        )
+        assert (
+            default_validator.validate(
+                {"id": 1, "secret": "abc", "token": "tok"}
+            )
+            is None
+        )
+
+        # Target ReadOnlyString still enforces readOnly: True on write
+        with pytest.raises(
+            ValidationError,
+            match="Tried to write read-only property with tok",
+        ):
+            write_validator.validate(
+                {"id": 1, "secret": "abc", "token": "tok"}
+            )
+
+        # Sibling readOnly: True on `id` does not exempt `id` from
+        # write_required or default required
+        with pytest.raises(
+            ValidationError, match="'id' is a required property"
+        ):
+            write_validator.validate({"secret": "abc"})
+        with pytest.raises(
+            ValidationError, match="'id' is a required property"
+        ):
+            default_validator.validate({"secret": "abc", "token": "tok"})
+
+        # Sibling writeOnly: True on `secret` does not exempt `secret` from
+        # read_required or default required
+        with pytest.raises(
+            ValidationError, match="'secret' is a required property"
+        ):
+            read_validator.validate({"id": 1, "token": "tok"})
+        with pytest.raises(
+            ValidationError, match="'secret' is a required property"
+        ):
+            default_validator.validate({"id": 1, "token": "tok"})
+
 
 class TestOAS31ValidatorFormatChecker:
     @pytest.fixture
@@ -1047,6 +1190,40 @@ def test_schema_ref(self, validator_class, format_checker):
         error = "'id' is a required property"
         assert error in str(excinfo.value)
 
+    def test_schema_ref_applies_sibling_keywords(
+        self, validator_class, format_checker
+    ):
+        schema = {
+            "$ref": "#/$defs/Pet",
+            "required": ["name"],
+            "$defs": {
+                "Pet": {
+                    "type": "object",
+                    "required": ["id"],
+                    "properties": {
+                        "id": {"type": "integer", "format": "int64"},
+                        "name": {"type": "string"},
+                    },
+                }
+            },
+        }
+        validator = validator_class(
+            schema,
+            format_checker=format_checker,
+        )
+
+        assert validator.validate({"id": 1, "name": "John"}) is None
+
+        with pytest.raises(
+            ValidationError, match="'name' is a required property"
+        ):
+            validator.validate({"id": 1})
+
+        with pytest.raises(
+            ValidationError, match="'id' is a required property"
+        ):
+            validator.validate({"name": "John"})
+
     @pytest.mark.parametrize(
         "value",
         [
@@ -1330,6 +1507,32 @@ def test_strict_binary_format_rejects_str(self):
         with pytest.raises(ValidationError, match="is not a 'binary'"):
             validator.validate("not base64")
 
+    def test_strict_validator_ref_ignores_sibling_keywords(self):
+        schema = {
+            "$ref": "#/components/schemas/NullableText",
+            "type": "string",
+            "nullable": False,
+            "minLength": 10,
+            "components": {
+                "schemas": {
+                    "NullableText": {
+                        "type": "string",
+                        "nullable": True,
+                        "minLength": 2,
+                    }
+                }
+            },
+        }
+        validator = OAS30StrictValidator(
+            schema, format_checker=oas30_strict_format_checker
+        )
+
+        assert validator.validate("abc") is None
+        assert validator.validate(None) is None
+
+        with pytest.raises(ValidationError, match="is too short"):
+            validator.validate("a")
+
 
 class TestValidatorForDiscovery:
     def test_oas31_base_dialect_resolves_to_oas31_validator(self):

tests/unit/test_shortcut.py

@@ -383,3 +383,53 @@ def test_enforce_properties_required(
         validate(
             instance, schema, cls=cls, enforce_properties_required=enforce
         )
+
+
+def test_validate_oas30_ignores_ref_siblings_while_oas31_oas32_apply_them():
+    schema = {
+        "$ref": "#/$defs/ItemTarget",
+        "required": ["name"],
+        "properties": {
+            "extra": {"type": "string"},
+        },
+        "$defs": {
+            "ItemTarget": {
+                "type": "object",
+                "required": ["id"],
+                "properties": {
+                    "id": {"type": "integer"},
+                    "name": {"type": "string"},
+                },
+            }
+        },
+    }
+
+    assert validate({"id": 1}, schema, cls=OAS30Validator) is None
+    assert (
+        validate(
+            {"id": 1, "name": "abc"},
+            schema,
+            cls=OAS30Validator,
+            enforce_properties_required=True,
+        )
+        is None
+    )
+
+    for validator_cls in (OAS31Validator, OAS32Validator):
+        with pytest.raises(
+            ValidationError, match="'name' is a required property"
+        ):
+            validate({"id": 1}, schema, cls=validator_cls)
+        assert (
+            validate({"id": 1, "name": "abc"}, schema, cls=validator_cls)
+            is None
+        )
+        with pytest.raises(
+            ValidationError, match="'extra' is a required property"
+        ):
+            validate(
+                {"id": 1, "name": "abc"},
+                schema,
+                cls=validator_cls,
+                enforce_properties_required=True,
+            )