Skip to content

Commit c357a50

Browse files
sethfitzsethfitz
authored
Treat None as absent in model constraint validation (#457)
* Treat None as absent in model constraint validation require_any_of, require_if, and forbid_if now treat None as "not present" for constraint purposes. Fields must be both in model_fields_set and non-null to satisfy (or violate) a constraint. - require_any_of: None no longer satisfies "at least one must be set" - require_if: None no longer satisfies "must be set when condition holds" - forbid_if: None no longer violates "must not be set when condition holds" JSON Schema generation updated to emit {"not": {"type": "null"}} property constraints alongside "required" assertions, via a shared required_non_null helper in _json_schema.py. Shared predicate _field_has_non_null_value extracted onto OptionalFieldGroupConstraint so the check lives in one place. Also fixes a stray backtick in the require_if error message. * Use Python None terminology and clarify constraint docs Use Python terminology (None) instead of null in docstrings, identifiers, and error messages. Rewrite error messages to describe what's checked ("set to a value other than None") rather than using jargon.
1 parent c4c0ebe commit c357a50

15 files changed

Lines changed: 562 additions & 74 deletions

File tree

‎packages/overture-schema-divisions-theme/tests/division_area_baseline_schema.json‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,13 @@
350350
}
351351
},
352352
"then": {
353+
"properties": {
354+
"admin_level": {
355+
"not": {
356+
"type": "null"
357+
}
358+
}
359+
},
353360
"required": [
354361
"admin_level"
355362
]
@@ -364,6 +371,13 @@
364371
}
365372
},
366373
"then": {
374+
"properties": {
375+
"admin_level": {
376+
"not": {
377+
"type": "null"
378+
}
379+
}
380+
},
367381
"required": [
368382
"admin_level"
369383
]
@@ -378,6 +392,13 @@
378392
}
379393
},
380394
"then": {
395+
"properties": {
396+
"admin_level": {
397+
"not": {
398+
"type": "null"
399+
}
400+
}
401+
},
381402
"required": [
382403
"admin_level"
383404
]
@@ -392,6 +413,13 @@
392413
}
393414
},
394415
"then": {
416+
"properties": {
417+
"admin_level": {
418+
"not": {
419+
"type": "null"
420+
}
421+
}
422+
},
395423
"required": [
396424
"admin_level"
397425
]
@@ -406,6 +434,13 @@
406434
}
407435
},
408436
"then": {
437+
"properties": {
438+
"admin_level": {
439+
"not": {
440+
"type": "null"
441+
}
442+
}
443+
},
409444
"required": [
410445
"admin_level"
411446
]
@@ -420,6 +455,13 @@
420455
}
421456
},
422457
"then": {
458+
"properties": {
459+
"admin_level": {
460+
"not": {
461+
"type": "null"
462+
}
463+
}
464+
},
423465
"required": [
424466
"admin_level"
425467
]

‎packages/overture-schema-divisions-theme/tests/division_baseline_schema.json‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -401,6 +401,13 @@
401401
}
402402
},
403403
"then": {
404+
"properties": {
405+
"admin_level": {
406+
"not": {
407+
"type": "null"
408+
}
409+
}
410+
},
404411
"required": [
405412
"admin_level"
406413
]
@@ -415,6 +422,13 @@
415422
}
416423
},
417424
"then": {
425+
"properties": {
426+
"admin_level": {
427+
"not": {
428+
"type": "null"
429+
}
430+
}
431+
},
418432
"required": [
419433
"admin_level"
420434
]
@@ -429,6 +443,13 @@
429443
}
430444
},
431445
"then": {
446+
"properties": {
447+
"admin_level": {
448+
"not": {
449+
"type": "null"
450+
}
451+
}
452+
},
432453
"required": [
433454
"admin_level"
434455
]
@@ -443,6 +464,13 @@
443464
}
444465
},
445466
"then": {
467+
"properties": {
468+
"admin_level": {
469+
"not": {
470+
"type": "null"
471+
}
472+
}
473+
},
446474
"required": [
447475
"admin_level"
448476
]
@@ -457,6 +485,13 @@
457485
}
458486
},
459487
"then": {
488+
"properties": {
489+
"admin_level": {
490+
"not": {
491+
"type": "null"
492+
}
493+
}
494+
},
460495
"required": [
461496
"admin_level"
462497
]
@@ -471,6 +506,13 @@
471506
}
472507
},
473508
"then": {
509+
"properties": {
510+
"admin_level": {
511+
"not": {
512+
"type": "null"
513+
}
514+
}
515+
},
474516
"required": [
475517
"admin_level"
476518
]
@@ -487,6 +529,13 @@
487529
}
488530
},
489531
"then": {
532+
"properties": {
533+
"parent_division_id": {
534+
"not": {
535+
"type": "null"
536+
}
537+
}
538+
},
490539
"required": [
491540
"parent_division_id"
492541
]
@@ -502,6 +551,13 @@
502551
},
503552
"then": {
504553
"not": {
554+
"properties": {
555+
"parent_division_id": {
556+
"not": {
557+
"type": "null"
558+
}
559+
}
560+
},
505561
"required": [
506562
"parent_division_id"
507563
]

‎packages/overture-schema-divisions-theme/tests/division_boundary_baseline_schema.json‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,13 @@
231231
}
232232
},
233233
"then": {
234+
"properties": {
235+
"admin_level": {
236+
"not": {
237+
"type": "null"
238+
}
239+
}
240+
},
234241
"required": [
235242
"admin_level"
236243
]
@@ -245,6 +252,13 @@
245252
}
246253
},
247254
"then": {
255+
"properties": {
256+
"admin_level": {
257+
"not": {
258+
"type": "null"
259+
}
260+
}
261+
},
248262
"required": [
249263
"admin_level"
250264
]
@@ -259,6 +273,13 @@
259273
}
260274
},
261275
"then": {
276+
"properties": {
277+
"admin_level": {
278+
"not": {
279+
"type": "null"
280+
}
281+
}
282+
},
262283
"required": [
263284
"admin_level"
264285
]
@@ -273,6 +294,13 @@
273294
}
274295
},
275296
"then": {
297+
"properties": {
298+
"admin_level": {
299+
"not": {
300+
"type": "null"
301+
}
302+
}
303+
},
276304
"required": [
277305
"admin_level"
278306
]
@@ -287,6 +315,13 @@
287315
}
288316
},
289317
"then": {
318+
"properties": {
319+
"admin_level": {
320+
"not": {
321+
"type": "null"
322+
}
323+
}
324+
},
290325
"required": [
291326
"admin_level"
292327
]
@@ -301,6 +336,13 @@
301336
}
302337
},
303338
"then": {
339+
"properties": {
340+
"admin_level": {
341+
"not": {
342+
"type": "null"
343+
}
344+
}
345+
},
304346
"required": [
305347
"admin_level"
306348
]
@@ -317,6 +359,13 @@
317359
}
318360
},
319361
"then": {
362+
"properties": {
363+
"country": {
364+
"not": {
365+
"type": "null"
366+
}
367+
}
368+
},
320369
"required": [
321370
"country"
322371
]
@@ -332,6 +381,13 @@
332381
},
333382
"then": {
334383
"not": {
384+
"properties": {
385+
"country": {
386+
"not": {
387+
"type": "null"
388+
}
389+
}
390+
},
335391
"required": [
336392
"country"
337393
]

‎packages/overture-schema-system/src/overture/schema/system/__init__.py‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -122,15 +122,19 @@
122122
MyModel(foo=42, bar=None)
123123
>>> MyModel(bar="hello") # validates OK
124124
MyModel(foo=None, bar='hello')
125-
>>> MyModel(foo=None, bar=None) # validates OK because foo and bar are explicitly set to `None`
126-
MyModel(foo=None, bar=None)
127125
>>>
128126
>>> try:
129127
... MyModel()
130128
... except ValidationError as e:
131-
... assert "at least one of these fields must be explicitly set, but none are: foo, bar" in str(e)
132-
... print("Validation failed")
133-
Validation failed
129+
... assert "at least one of these fields must be set to a value other than None, but none are: foo, bar" in str(e)
130+
... print("Validation failed (no fields set)")
131+
Validation failed (no fields set)
132+
>>> try:
133+
... MyModel(foo=None, bar=None)
134+
... except ValidationError as e:
135+
... assert "at least one of these fields must be set to a value other than None, but none are: foo, bar" in str(e)
136+
... print("Validation failed (all fields None)")
137+
Validation failed (all fields None)
134138
135139
Describe a foreign key relationship between two models where one model has a field that contains the
136140
unique identifier of another model.

‎packages/overture-schema-system/src/overture/schema/system/_json_schema.py‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,30 @@ def try_move(key: str, src: JsonSchemaValue, dst: JsonSchemaValue) -> None:
331331
pass
332332

333333

334+
def required_non_null(aliases: list[str]) -> JsonSchemaValue:
335+
"""
336+
Build a JSON Schema requiring listed properties to be present and non-null.
337+
338+
Combines `"required"` (property must exist) with a per-property
339+
constraint `{"not": {"type": "null"}}` (value must not be null).
340+
341+
Parameters
342+
----------
343+
aliases : list[str]
344+
Non-empty list of JSON Schema property names to constrain
345+
346+
Returns
347+
-------
348+
JsonSchemaValue
349+
Schema requiring each property to be present and non-null
350+
"""
351+
_verify_operands_not_empty(str, aliases)
352+
return {
353+
"required": aliases,
354+
"properties": {a: {"not": {"type": "null"}} for a in aliases},
355+
}
356+
357+
334358
T = TypeVar("T", JsonSchemaValue, str)
335359

336360

0 commit comments

Comments
 (0)