You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
#920 fixed unconditional constraint instantiation in Draft06Constraint, Draft07Constraint, and Draft2019Constraint by adding a KEYWORD_SCHEMA_PROPERTIES map in each dispatcher class, gating Factory::createInstanceFor() on schema-property presence before instantiating a constraint. That fix does not account for whether the keyword is even applicable to the value's type.
Opportunity
Most individual constraint classes have a second internal guard beyond property_exists($schema, ...), checking the value's type (is_object, is_array, is_string, is_numeric, etc.) before doing real work. For Draft2019 specifically, 26 of 34 constraint classes (~76%) carry this second guard.
A schema that legally declares keywords applicable to multiple instance types (e.g. both properties and pattern on the same subschema) will, under the #920 fix, still instantiate a constraint that's guaranteed to no-op based on the value's type alone.
Example: validating a bare string against a schema declaring both properties (object-applicable) and pattern (string-applicable) still instantiates PropertiesConstraint needlessly.
Proposed approach
Expose a static method on each constraint class (e.g. public static function isApplicable($value, $schema): bool) that encapsulates both the schema-property and value-type applicability checks, colocated with the class that actually has that domain knowledge. Draft*Constraint::checkForKeyword() would call this statically — avoiding instantiation entirely rather than just deferring it — before ever calling Factory::createInstanceFor().
Why this is a separate, larger change (not part of #920)
Blast radius: ~98 constraint classes across Draft06/Draft07/Draft2019 would each need this method added. For ~90% of them (where the keyword name already equals the schema property name), it's a one-line restatement of the guard that already exists in check().
Backward-compatibility / breaking change: ConstraintInterface is public, and consumers can substitute their own constraint classes via Factory::setConstraintClass(). Adding a required static method to the interface would break any consumer-authored constraint that doesn't implement it. This needs to be either an opt-in contract (checked via method_exists(), falling back to today's always-instantiate behavior) or shipped as an explicit breaking change with a major version bump.
Factory needs a new accessor: checkForKeyword() currently only learns a keyword's class via createInstanceFor(), which also instantiates it. A non-instantiating lookup (e.g. getConstraintClassFor(string $keyword): string) would be needed.
This should be scoped and justified by a demonstrated need (e.g. profiling schemas that genuinely mix multiple instance-type keywords) rather than pursued reflexively — the #920 fix already resolves the primary unconditional-instantiation cost for the common case of schemas with 3-5 keywords.
Description
#920 fixed unconditional constraint instantiation in
Draft06Constraint,Draft07Constraint, andDraft2019Constraintby adding aKEYWORD_SCHEMA_PROPERTIESmap in each dispatcher class, gatingFactory::createInstanceFor()on schema-property presence before instantiating a constraint. That fix does not account for whether the keyword is even applicable to the value's type.Opportunity
Most individual constraint classes have a second internal guard beyond
property_exists($schema, ...), checking the value's type (is_object,is_array,is_string,is_numeric, etc.) before doing real work. ForDraft2019specifically, 26 of 34 constraint classes (~76%) carry this second guard.A schema that legally declares keywords applicable to multiple instance types (e.g. both
propertiesandpatternon the same subschema) will, under the #920 fix, still instantiate a constraint that's guaranteed to no-op based on the value's type alone.Example: validating a bare string against a schema declaring both
properties(object-applicable) andpattern(string-applicable) still instantiatesPropertiesConstraintneedlessly.Proposed approach
Expose a static method on each constraint class (e.g.
public static function isApplicable($value, $schema): bool) that encapsulates both the schema-property and value-type applicability checks, colocated with the class that actually has that domain knowledge.Draft*Constraint::checkForKeyword()would call this statically — avoiding instantiation entirely rather than just deferring it — before ever callingFactory::createInstanceFor().Why this is a separate, larger change (not part of #920)
Draft06/Draft07/Draft2019would each need this method added. For ~90% of them (where the keyword name already equals the schema property name), it's a one-line restatement of the guard that already exists incheck().ConstraintInterfaceis public, and consumers can substitute their own constraint classes viaFactory::setConstraintClass(). Adding a required static method to the interface would break any consumer-authored constraint that doesn't implement it. This needs to be either an opt-in contract (checked viamethod_exists(), falling back to today's always-instantiate behavior) or shipped as an explicit breaking change with a major version bump.Factoryneeds a new accessor:checkForKeyword()currently only learns a keyword's class viacreateInstanceFor(), which also instantiates it. A non-instantiating lookup (e.g.getConstraintClassFor(string $keyword): string) would be needed.Factorymocking used by the tests added in Draft06Constraint, Draft07Constraint, and Draft2019Constraint instantiate all constraint objects unconditionally per schema node #920 (PHPUnit 8.5 has no native static-method mocking).Scope
This should be scoped and justified by a demonstrated need (e.g. profiling schemas that genuinely mix multiple instance-type keywords) rather than pursued reflexively — the #920 fix already resolves the primary unconditional-instantiation cost for the common case of schemas with 3-5 keywords.