diff --git a/openapi_core/validation/schemas/validators.py b/openapi_core/validation/schemas/validators.py index c7a8f03b..87133119 100644 --- a/openapi_core/validation/schemas/validators.py +++ b/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 diff --git a/tests/integration/unmarshalling/test_unmarshallers.py b/tests/integration/unmarshalling/test_unmarshallers.py index ddb322be..a384e381 100644 --- a/tests/integration/unmarshalling/test_unmarshallers.py +++ b/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)} diff --git a/tests/unit/validation/test_schema_validators.py b/tests/unit/validation/test_schema_validators.py index 2dea1e10..6a290643 100644 --- a/tests/unit/validation/test_schema_validators.py +++ b/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