-
Notifications
You must be signed in to change notification settings - Fork 13
Description
Behavior
require_any_of('max_speed', 'min_speed') accepts max_speed=None as satisfying the constraint. The same applies to require_if and forbid_if — all three treat "field is in model_fields_set" as sufficient, regardless of value.
@require_any_of("max_speed", "min_speed")
class SpeedLimit(BaseModel):
max_speed: int | None = None
min_speed: int | None = None
SpeedLimit(max_speed=None) # passes validation — should it?Why it works this way
The docstrings document this as intentional, designed for JSON Schema parity (appropriate given the nature of the port). In JSON Schema, {"required": ["foo"]} checks key presence — {"foo": null} satisfies it. The Python validators align to that semantic: model_fields_set membership equals satisfaction.
Victor Schappert (@vcschapp) and I discussed this: the implementation focused on the presence/omitted distinction rather than Overture's domain semantics.
Proposed fix
These constraints encode domain invariants. require_any_of('max_speed', 'min_speed') means "a speed limit needs at least one bound." None is not a bound.
Update validate_instance in all three constraints to require model_fields_set membership AND a non-None value. Update JSON Schema generation to match:
{
"anyOf": [
{"required": ["max_speed"], "properties": {"max_speed": {"not": {"type": "null"}}}},
{"required": ["min_speed"], "properties": {"min_speed": {"not": {"type": "null"}}}}
]
}Affected constraints
require_any_of— at least one field must be set and non-nullrequire_if— conditionally required fields must be set and non-nullforbid_if— only non-null values violate the prohibition