Merge pull request #1191 from python-openapi/feature/schema-validator-per-resolver-clarification

SchemaValidator per-resolver cache clarification

Author: p1c2u

openapi_core/validation/schemas/validators.py

@@ -48,10 +48,15 @@ def validate(self, value: Any) -> None:
             raise InvalidSchemaValue(value, schema_type, schema_errors=errors)
 
     # Cache the recursive "does this schema benefit from a ValidationState?"
-    # check, keyed on the SchemaPath. SchemaPath is hashed by content, so
-    # two SchemaPaths pointing at the same spec location share a cache
-    # slot regardless of identity -- safe across GC, bounded by the number
-    # of distinct schema shapes in the spec rather than by input volume.
+    # check, keyed on the SchemaPath. Under jsonschema-path 0.5 (pathable
+    # 0.6) SchemaPath is an AccessorPath whose identity is
+    # (parts, accessor), and SchemaAccessor in turn hashes/compares on
+    # id(node) and id(path_resolver). The key is therefore effectively
+    # per-resolver: two SchemaPaths share a cache slot only when they
+    # address the same location *within the same loaded spec*, never
+    # across distinct specs that merely share a JSON-pointer path.
+    # Entries are bounded by the number of distinct schema shapes per
+    # spec and become collectable once the owning resolver is GC'd.
     _needs_state_cache: dict[SchemaPath, bool] = {}
 
     @classmethod

tests/integration/unmarshalling/test_unmarshallers.py

@@ -2174,3 +2174,66 @@ def test_subschema_null(self, spec, unmarshallers_factory):
         result = unmarshaller.unmarshal(value)
 
         assert result is None
+
+
+class TestCrossSpecUnmarshalling:
+    """Unmarshalling two separately-loaded specs in the same process must
+    produce independent, correct results even when the specs share
+    JSON-pointer paths.
+    """
+
+    @pytest.fixture
+    def root(self):
+        return SchemaPath.from_dict({})
+
+    def test_composed_and_plain_specs_unmarshal_independently(self, root):
+        # Composed: oneOf selects the object branch, whose date format
+        # must be applied -> created_at becomes a date object.
+        composed = SchemaPath.from_dict(
+            {
+                "oneOf": [
+                    {
+                        "type": "object",
+                        "properties": {
+                            "created_at": {
+                                "type": "string",
+                                "format": "date",
+                            }
+                        },
+                    },
+                    {"type": "integer"},
+                ]
+            }
+        )
+        # Plain: same JSON-pointer shape but no composition and no
+        # format -> created_at stays a string.
+        plain = SchemaPath.from_dict(
+            {
+                "type": "object",
+                "properties": {"created_at": {"type": "string"}},
+            }
+        )
+
+        value = {"created_at": "2020-01-02"}
+
+        composed_result = oas31_schema_unmarshallers_factory.create(
+            root, composed
+        ).unmarshal(value)
+        plain_result = oas31_schema_unmarshallers_factory.create(
+            root, plain
+        ).unmarshal(value)
+
+        assert composed_result == {"created_at": date(2020, 1, 2)}
+        assert plain_result == {"created_at": "2020-01-02"}
+
+        # Order independence: re-run in reverse to ensure neither spec's
+        # cached answer poisoned the other.
+        plain_again = oas31_schema_unmarshallers_factory.create(
+            root, plain
+        ).unmarshal(value)
+        composed_again = oas31_schema_unmarshallers_factory.create(
+            root, composed
+        ).unmarshal(value)
+
+        assert plain_again == {"created_at": "2020-01-02"}
+        assert composed_again == {"created_at": date(2020, 1, 2)}

tests/unit/validation/test_schema_validators.py

@@ -5,6 +5,7 @@
     oas30_write_schema_validators_factory,
 )
 from openapi_core.validation.schemas.exceptions import InvalidSchemaValue
+from openapi_core.validation.schemas.validators import SchemaValidator
 
 
 class TestSchemaValidate:
@@ -356,3 +357,144 @@ def test_enforce_properties_required_applies_to_nested_composed_schemas(
                 schema,
                 enforce_properties_required=True,
             ).validate({"name": "openapi-core", "meta": {}})
+
+
+class TestSchemaValidateState:
+    SCHEMA_DICT = {
+        "type": "object",
+        "properties": {
+            "x": {"oneOf": [{"type": "string"}, {"type": "integer"}]}
+        },
+    }
+    VALUE = {"x": "hi"}
+
+    @pytest.fixture(autouse=True)
+    def clear_cache(self):
+        # Keep this class's cache observations isolated from other tests.
+        SchemaValidator._needs_state_cache.clear()
+        yield
+        SchemaValidator._needs_state_cache.clear()
+
+    @pytest.fixture
+    def cache(self):
+        return SchemaValidator._needs_state_cache
+
+    @pytest.fixture
+    def validator_and_prop_factory(self):
+        # Build a validator over a freshly loaded spec and return it
+        # alongside the SchemaPath the cache keys on for property "x".
+        root = SchemaPath.from_dict({})
+
+        def _build(schema_dict):
+            spec = SchemaPath.from_dict(schema_dict)
+            validator = oas30_write_schema_validators_factory.create(
+                root, spec
+            )
+            prop = spec / "properties" / "x"
+            return validator, prop
+
+        return _build
+
+    def test_cold_pass_populates_cache(
+        self, cache, validator_and_prop_factory
+    ):
+        validator, prop = validator_and_prop_factory(self.SCHEMA_DICT)
+        assert prop not in cache
+
+        validator.validate_state(self.VALUE)
+
+        # oneOf under "x" -> a ValidationState is worthwhile.
+        assert cache[prop] is True
+
+    def test_warm_pass_reads_cached_answer(
+        self, cache, validator_and_prop_factory
+    ):
+        validator, prop = validator_and_prop_factory(self.SCHEMA_DICT)
+        validator.validate_state(self.VALUE)  # prime
+        # Poison the entry: a genuine cache hit returns this value
+        # unchanged, whereas a recompute would overwrite it back to True.
+        cache[prop] = False
+
+        validator.validate_state(self.VALUE)
+
+        assert cache[prop] is False
+
+    def test_distinct_spec_does_not_collide(
+        self, cache, validator_and_prop_factory
+    ):
+        # Two separately loaded specs with identical contents have
+        # distinct identity, so their equally-pathed property schemas
+        # occupy separate cache slots instead of colliding.
+        validator_a, prop_a = validator_and_prop_factory(self.SCHEMA_DICT)
+        validator_b, prop_b = validator_and_prop_factory(self.SCHEMA_DICT)
+
+        validator_a.validate_state(self.VALUE)
+        assert prop_a in cache
+        assert prop_b not in cache
+
+        validator_b.validate_state(self.VALUE)
+        assert cache[prop_a] is True
+        assert cache[prop_b] is True
+
+
+class TestSchemaValidateStateRefDedup:
+    # A single composed schema reached through two different $ref aliases.
+    SCHEMA_DICT = {
+        "type": "object",
+        "properties": {
+            "a": {"$ref": "#/$defs/Composed"},
+            "b": {"$ref": "#/$defs/Composed"},
+        },
+        "$defs": {
+            "Composed": {"oneOf": [{"type": "string"}, {"type": "integer"}]},
+        },
+    }
+    VALUE = {"a": "hi", "b": 1}
+
+    @pytest.fixture(autouse=True)
+    def clear_cache(self):
+        SchemaValidator._needs_state_cache.clear()
+        yield
+        SchemaValidator._needs_state_cache.clear()
+
+    @pytest.fixture
+    def cache(self):
+        return SchemaValidator._needs_state_cache
+
+    @pytest.fixture
+    def validator_and_props_factory(self):
+        root = SchemaPath.from_dict({})
+
+        def _build(schema_dict):
+            spec = SchemaPath.from_dict(schema_dict)
+            validator = oas30_write_schema_validators_factory.create(
+                root, spec
+            )
+            prop_a = spec / "properties" / "a"
+            prop_b = spec / "properties" / "b"
+            canonical = spec / "$defs" / "Composed"
+            return validator, prop_a, prop_b, canonical
+
+        return _build
+
+    @pytest.mark.xfail(
+        strict=True,
+        reason=(
+            "The cache keys on the navigation path, so each $ref "
+            "alias gets its own slot. Once the cache keys on canonical "
+            "the aliases collapse to a single entry."
+        ),
+    )
+    def test_aliases_to_same_node_share_one_cache_slot(
+        self, cache, validator_and_props_factory
+    ):
+        validator, prop_a, prop_b, canonical = validator_and_props_factory(
+            self.SCHEMA_DICT
+        )
+
+        validator.validate_state(self.VALUE)
+
+        assert len(cache) == 1
+        assert prop_a not in cache
+        assert prop_b not in cache
+        assert cache[canonical] is True