Skip to content

Commit 871e05e

Browse files
ryyakoberyyakobe
committed
fix: improve parameter definition validation and error reporting
This commit includes several improvements to parameter validation: 1. Add explicit validation for None values in allowedValues across all parameter types 2. Fix error location reporting in validation error messages 3. Correct documentation comments for maxValue parameters 4. Fix type casting in JobPathParameterDefinition 5. Add test cases for both implicit and explicit None handling The changes ensure that explicitly set None values for allowedValues are properly rejected with clear error messages, while implicitly omitted allowedValues continue to work correctly. Signed-off-by: Roman Yakobenchuk <66849711+ryyakobe@users.noreply.github.com>
1 parent 067408c commit 871e05e

File tree

2 files changed

+87
-12
lines changed

2 files changed

+87
-12
lines changed

‎src/openjd/model/v2023_09/_model.py‎

Lines changed: 27 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1201,6 +1201,8 @@ def _validate_max_length(cls, value: Optional[int], info: ValidationInfo) -> Opt
12011201
def _validate_allowed_values_item(
12021202
cls, value: AllowedParameterStringValueList, info: ValidationInfo
12031203
) -> AllowedParameterStringValueList:
1204+
if value is None:
1205+
raise ValueError("allowedValues cannot be None. The field must contain at least one value or be omitted entirely.")
12041206
min_length = info.data.get("minLength")
12051207
max_length = info.data.get("maxLength")
12061208
errors = list[InitErrorDetails]()
@@ -1210,7 +1212,7 @@ def _validate_allowed_values_item(
12101212
errors.append(
12111213
InitErrorDetails(
12121214
type="value_error",
1213-
loc=(i,),
1215+
loc=("allowedValues", i),
12141216
ctx={"error": ValueError("Value is shorter than minLength.")},
12151217
input=item,
12161218
)
@@ -1220,7 +1222,7 @@ def _validate_allowed_values_item(
12201222
errors.append(
12211223
InitErrorDetails(
12221224
type="value_error",
1223-
loc=(i,),
1225+
loc=("allowedValues", i),
12241226
ctx={"error": ValueError("Value is longer than maxLength.")},
12251227
input=item,
12261228
)
@@ -1411,7 +1413,7 @@ class JobPathParameterDefinition(OpenJDModel_v2023_09, JobParameterInterface):
14111413
"default",
14121414
},
14131415
adds_fields=lambda key, this, symtab: {
1414-
"value": symtab[f"RawParam.{cast(JobStringParameterDefinition,this).name}"]
1416+
"value": symtab[f"RawParam.{cast(JobPathParameterDefinition,this).name}"]
14151417
},
14161418
)
14171419

@@ -1441,8 +1443,10 @@ def _validate_max_length(cls, value: Optional[int], info: ValidationInfo) -> Opt
14411443
@field_validator("allowedValues")
14421444
@classmethod
14431445
def _validate_allowed_values_item(
1444-
cls, value: ParameterStringValue, info: ValidationInfo
1445-
) -> ParameterStringValue:
1446+
cls, value: AllowedParameterStringValueList, info: ValidationInfo
1447+
) -> AllowedParameterStringValueList:
1448+
if value is None:
1449+
raise ValueError("allowedValues cannot be None. The field must contain at least one value or be omitted entirely.")
14461450
min_length = info.data.get("minLength")
14471451
max_length = info.data.get("maxLength")
14481452
errors = list[InitErrorDetails]()
@@ -1600,7 +1604,7 @@ class JobIntParameterDefinition(OpenJDModel_v2023_09):
16001604
allowedValues (Optional[AllowedIntParameterList]): Explicit list of values that the
16011605
parameter is allowed to take on.
16021606
minValue (Optional[int]): Minimum value that the parameter is allowed to be.
1603-
maxValue (Optional[int]): Minimum value that the parameter is allowed to be.
1607+
maxValue (Optional[int]): Maximum value that the parameter is allowed to be.
16041608
"""
16051609

16061610
name: Identifier
@@ -1661,7 +1665,12 @@ def _validate_max_value_type(cls, value: Optional[Any]) -> Optional[Any]:
16611665

16621666
@field_validator("allowedValues", mode="before")
16631667
@classmethod
1664-
def _validate_allowed_values_item_type(cls, value: Any) -> Any:
1668+
def _validate_allowed_values_item_type(
1669+
cls, value: AllowedIntParameterList
1670+
) -> AllowedIntParameterList:
1671+
if value is None:
1672+
raise ValueError("allowedValues cannot be None. The field must contain at least one value or be omitted entirely.")
1673+
16651674
errors = list[InitErrorDetails]()
16661675
for i, item in enumerate(value):
16671676
if isinstance(item, bool) or not isinstance(item, (int, str)):
@@ -1702,7 +1711,11 @@ def _validate_max_value(cls, value: Optional[int], info: ValidationInfo) -> Opti
17021711

17031712
@field_validator("allowedValues")
17041713
@classmethod
1705-
def _validate_allowed_values_item(cls, value: list[int], info: ValidationInfo) -> list[int]:
1714+
def _validate_allowed_values_item(
1715+
cls, value: AllowedIntParameterList, info: ValidationInfo
1716+
) -> AllowedIntParameterList:
1717+
if value is None:
1718+
raise ValueError("allowedValues cannot be None. The field must contain at least one value or be omitted entirely.")
17061719
min_value = info.data.get("minValue")
17071720
max_value = info.data.get("maxValue")
17081721
errors = list[InitErrorDetails]()
@@ -1723,7 +1736,7 @@ def _validate_allowed_values_item(cls, value: list[int], info: ValidationInfo) -
17231736
InitErrorDetails(
17241737
type="value_error",
17251738
loc=(i,),
1726-
ctx={"error": ValueError("Value larger than minValue.")},
1739+
ctx={"error": ValueError("Value larger than maxValue.")},
17271740
input=item,
17281741
)
17291742
)
@@ -1846,7 +1859,7 @@ class JobFloatParameterDefinition(OpenJDModel_v2023_09):
18461859
allowedValues (Optional[AllowedFloatParameterList]): Explicit list of values that the
18471860
parameter is allowed to take on.
18481861
minValue (Optional[Decimal]): Minimum value that the parameter is allowed to be.
1849-
maxValue (Optional[Decimal]): Minimum value that the parameter is allowed to be.
1862+
maxValue (Optional[Decimal]): Maximum value that the parameter is allowed to be.
18501863
"""
18511864

18521865
name: Identifier
@@ -1899,8 +1912,10 @@ def _validate_max_value(
18991912
@field_validator("allowedValues")
19001913
@classmethod
19011914
def _validate_allowed_values_item(
1902-
cls, value: list[Decimal], info: ValidationInfo
1903-
) -> list[Decimal]:
1915+
cls, value: AllowedFloatParameterList, info: ValidationInfo
1916+
) -> AllowedFloatParameterList:
1917+
if value is None:
1918+
raise ValueError("allowedValues cannot be None. The field must contain at least one value or be omitted entirely.")
19041919
min_value = info.data.get("minValue")
19051920
max_value = info.data.get("maxValue")
19061921
errors = list[InitErrorDetails]()

‎test/openjd/model/v2023_09/test_job_parameters.py‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,13 @@ class TestJobStringParameterDefinition:
152152
},
153153
id="all fields",
154154
),
155+
pytest.param(
156+
{
157+
"name": "Foo",
158+
"type": "STRING",
159+
},
160+
id="allowedValues is implicitly None",
161+
),
155162
),
156163
)
157164
def test_parse_success(self, data: dict[str, Any]) -> None:
@@ -195,6 +202,14 @@ def test_parse_success(self, data: dict[str, Any]) -> None:
195202
pytest.param(
196203
{"name": "Foo", "type": "STRING", "allowedValues": []}, id="allowedValues too small"
197204
),
205+
pytest.param(
206+
{
207+
"name": "Foo",
208+
"type": "STRING",
209+
"allowedValues": None,
210+
},
211+
id="allowedValues is explicitly None",
212+
),
198213
pytest.param(
199214
{"name": "Foo", "type": "STRING", "allowedValues": [12]},
200215
id="allowedValues item not string",
@@ -591,6 +606,13 @@ class TestJobPathParameterDefinition:
591606
},
592607
id="all fields",
593608
),
609+
pytest.param(
610+
{
611+
"name": "Foo",
612+
"type": "PATH",
613+
},
614+
id="allowedValues is implicitly None",
615+
),
594616
),
595617
)
596618
def test_parse_success(self, data: dict[str, Any]) -> None:
@@ -626,6 +648,14 @@ def test_parse_success(self, data: dict[str, Any]) -> None:
626648
pytest.param(
627649
{"name": "Foo", "type": "PATH", "allowedValues": []}, id="allowedValues too small"
628650
),
651+
pytest.param(
652+
{
653+
"name": "Foo",
654+
"type": "PATH",
655+
"allowedValues": None,
656+
},
657+
id="allowedValues is explicitly None",
658+
),
629659
pytest.param(
630660
{"name": "Foo", "type": "PATH", "allowedValues": [12]},
631661
id="allowedValues item not string",
@@ -1105,6 +1135,13 @@ class TestJobIntParameterDefinition:
11051135
},
11061136
id="all fields",
11071137
),
1138+
pytest.param(
1139+
{
1140+
"name": "Foo",
1141+
"type": "INT",
1142+
},
1143+
id="allowedValues is implicitly None",
1144+
),
11081145
),
11091146
)
11101147
def test_parse_success(self, data: dict[str, Any]) -> None:
@@ -1136,6 +1173,14 @@ def test_parse_success(self, data: dict[str, Any]) -> None:
11361173
pytest.param(
11371174
{"name": "Foo", "type": "INT", "allowedValues": []}, id="allowedValues too small"
11381175
),
1176+
pytest.param(
1177+
{
1178+
"name": "Foo",
1179+
"type": "INT",
1180+
"allowedValues": None,
1181+
},
1182+
id="allowedValues is explicitly None",
1183+
),
11391184
pytest.param(
11401185
{"name": "Foo", "type": "INT", "allowedValues": ["aa"]},
11411186
id="allowedValues item not number",
@@ -1500,6 +1545,13 @@ class TestJobFloatParameterDefinition:
15001545
},
15011546
id="all fields",
15021547
),
1548+
pytest.param(
1549+
{
1550+
"name": "Foo",
1551+
"type": "FLOAT",
1552+
},
1553+
id="allowedValues is implicitly None",
1554+
),
15031555
),
15041556
)
15051557
def test_parse_success(self, data: dict[str, Any]) -> None:
@@ -1530,6 +1582,14 @@ def test_parse_success(self, data: dict[str, Any]) -> None:
15301582
pytest.param(
15311583
{"name": "Foo", "type": "FLOAT", "allowedValues": []}, id="allowedValues too small"
15321584
),
1585+
pytest.param(
1586+
{
1587+
"name": "Foo",
1588+
"type": "FLOAT",
1589+
"allowedValues": None,
1590+
},
1591+
id="allowedValues is explicitly None",
1592+
),
15331593
pytest.param(
15341594
{"name": "Foo", "type": "FLOAT", "allowedValues": ["aa"]},
15351595
id="allowedValues item not number",

0 commit comments

Comments
 (0)