Skip to content

Commit 433d2ed

Browse files
azmeukazmeuk
authored
fix: patch checks handle empty responses (#40)
204 is a valid response for PATCH operations. Perform an additional GET request to check the edited value.
1 parent f2787ee commit 433d2ed

File tree

10 files changed

+400
-142
lines changed

10 files changed

+400
-142
lines changed

‎doc/changelog.rst‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ Fixed
88
^^^^^
99
- Sort results in get_all_available_tags, so it can be used with pytest-xdist.
1010
- `generate_random_value` generate coherent `ref` and `value` values for complex attributes. :pr:`30` :pr:`37`
11+
- Include PATCH operations in get_all_available_tags.
1112

1213
[0.2.0] - 2025-08-12
1314
--------------------

‎scim2_tester/checkers/__init__.py‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@
1111
"""
1212

1313
from .misc import random_url
14+
from .patch_add import check_add_attribute
15+
from .patch_remove import check_remove_attribute
16+
from .patch_replace import check_replace_attribute
1417
from .resource import resource_type_tests
1518
from .resource_delete import object_deletion
1619
from .resource_get import object_query
@@ -49,5 +52,8 @@
4952
"object_replacement",
5053
"object_deletion",
5154
"resource_type_tests",
55+
"check_add_attribute",
56+
"check_remove_attribute",
57+
"check_replace_attribute",
5258
"random_url",
5359
]

‎scim2_tester/checkers/patch_add.py‎

Lines changed: 45 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ def check_add_attribute(
6969

7070
for urn, source_model in all_urns:
7171
patch_value = generate_random_value(context, urn=urn, model=source_model)
72+
mutability = get_annotation_by_urn(Mutability, urn, source_model)
7273

7374
patch_op = PatchOp[type(base_resource)](
7475
operations=[
@@ -81,7 +82,7 @@ def check_add_attribute(
8182
)
8283

8384
try:
84-
updated_resource = context.client.modify(
85+
modify_result = context.client.modify(
8586
resource_model=type(base_resource),
8687
id=base_resource.id,
8788
patch_op=patch_op,
@@ -101,11 +102,51 @@ def check_add_attribute(
101102
)
102103
continue
103104

105+
if modify_result is not None:
106+
if modify_actual_value := get_value_by_urn(modify_result, urn):
107+
if not (
108+
mutability == Mutability.write_only
109+
or compare_field(patch_value, modify_actual_value)
110+
):
111+
results.append(
112+
CheckResult(
113+
status=Status.ERROR,
114+
reason=f"PATCH modify() returned incorrect value for '{urn}'",
115+
resource_type=model.__name__,
116+
data={
117+
"urn": urn,
118+
"expected": patch_value,
119+
"modify_actual": modify_actual_value,
120+
},
121+
)
122+
)
123+
continue
124+
125+
try:
126+
updated_resource = context.client.query(
127+
type(base_resource),
128+
base_resource.id,
129+
)
130+
except SCIMClientError as exc:
131+
results.append(
132+
CheckResult(
133+
status=Status.ERROR,
134+
reason=f"Failed to query resource after add on '{urn}': {exc}",
135+
resource_type=model.__name__,
136+
data={
137+
"urn": urn,
138+
"error": exc,
139+
"patch_value": patch_value,
140+
},
141+
)
142+
)
143+
continue
144+
104145
actual_value = get_value_by_urn(updated_resource, urn)
105146

106-
if get_annotation_by_urn(
107-
Mutability, urn, source_model
108-
) == Mutability.write_only or compare_field(patch_value, actual_value):
147+
if mutability == Mutability.write_only or compare_field(
148+
patch_value, actual_value
149+
):
109150
results.append(
110151
CheckResult(
111152
status=Status.SUCCESS,

‎scim2_tester/checkers/patch_remove.py‎

Lines changed: 43 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ def check_remove_attribute(
6868

6969
for urn, source_model in all_urns:
7070
initial_value = get_value_by_urn(full_resource, urn)
71+
mutability = get_annotation_by_urn(Mutability, urn, source_model)
7172
if initial_value is None:
7273
continue
7374

@@ -81,7 +82,7 @@ def check_remove_attribute(
8182
)
8283

8384
try:
84-
updated_resource = context.client.modify(
85+
modify_result = context.client.modify(
8586
resource_model=type(full_resource),
8687
id=full_resource.id,
8788
patch_op=remove_op,
@@ -101,13 +102,48 @@ def check_remove_attribute(
101102
)
102103
continue
103104

104-
actual_value = get_value_by_urn(updated_resource, urn)
105+
if modify_result is not None:
106+
if modify_actual_value := get_value_by_urn(modify_result, urn):
107+
if (
108+
mutability != Mutability.write_only
109+
and modify_actual_value is not None
110+
):
111+
results.append(
112+
CheckResult(
113+
status=Status.ERROR,
114+
reason=f"PATCH modify() did not remove attribute '{urn}'",
115+
resource_type=model.__name__,
116+
data={
117+
"urn": urn,
118+
"initial_value": initial_value,
119+
"modify_actual": modify_actual_value,
120+
},
121+
)
122+
)
123+
continue
124+
125+
try:
126+
updated_resource = context.client.query(
127+
type(full_resource),
128+
full_resource.id,
129+
)
130+
except SCIMClientError as exc:
131+
results.append(
132+
CheckResult(
133+
status=Status.ERROR,
134+
reason=f"Failed to query resource after remove on '{urn}': {exc}",
135+
resource_type=model.__name__,
136+
data={
137+
"urn": urn,
138+
"error": exc,
139+
"initial_value": initial_value,
140+
},
141+
)
142+
)
143+
continue
105144

106-
if (
107-
get_annotation_by_urn(Mutability, urn, source_model)
108-
== Mutability.write_only
109-
or actual_value is None
110-
):
145+
actual_value = get_value_by_urn(updated_resource, urn)
146+
if mutability == Mutability.write_only or actual_value is None:
111147
results.append(
112148
CheckResult(
113149
status=Status.SUCCESS,

‎scim2_tester/checkers/patch_replace.py‎

Lines changed: 45 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ def check_replace_attribute(
6767

6868
for urn, source_model in all_urns:
6969
patch_value = generate_random_value(context, urn=urn, model=source_model)
70+
mutability = get_annotation_by_urn(Mutability, urn, source_model)
7071

7172
patch_op = PatchOp[type(base_resource)](
7273
operations=[
@@ -79,8 +80,7 @@ def check_replace_attribute(
7980
)
8081

8182
try:
82-
# Perform the PATCH replace operation
83-
updated_resource = context.client.modify(
83+
modify_result = context.client.modify(
8484
resource_model=type(base_resource),
8585
id=base_resource.id,
8686
patch_op=patch_op,
@@ -100,11 +100,50 @@ def check_replace_attribute(
100100
)
101101
continue
102102

103-
actual_value = get_value_by_urn(updated_resource, urn)
103+
if modify_result is not None:
104+
if modify_actual_value := get_value_by_urn(modify_result, urn):
105+
if not (
106+
mutability == Mutability.write_only
107+
or compare_field(patch_value, modify_actual_value)
108+
):
109+
results.append(
110+
CheckResult(
111+
status=Status.ERROR,
112+
reason=f"PATCH modify() returned incorrect value for '{urn}'",
113+
resource_type=model.__name__,
114+
data={
115+
"urn": urn,
116+
"expected": patch_value,
117+
"modify_actual": modify_actual_value,
118+
},
119+
)
120+
)
121+
continue
122+
123+
try:
124+
updated_resource = context.client.query(
125+
type(base_resource),
126+
base_resource.id,
127+
)
128+
except SCIMClientError as exc:
129+
results.append(
130+
CheckResult(
131+
status=Status.ERROR,
132+
reason=f"Failed to query resource after replace on '{urn}': {exc}",
133+
resource_type=model.__name__,
134+
data={
135+
"urn": urn,
136+
"error": exc,
137+
"patch_value": patch_value,
138+
},
139+
)
140+
)
141+
continue
104142

105-
if get_annotation_by_urn(
106-
Mutability, urn, source_model
107-
) == Mutability.write_only or compare_field(patch_value, actual_value):
143+
actual_value = get_value_by_urn(updated_resource, urn)
144+
if mutability == Mutability.write_only or compare_field(
145+
patch_value, actual_value
146+
):
108147
results.append(
109148
CheckResult(
110149
status=Status.SUCCESS,

‎scim2_tester/checkers/resource.py‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@
33
from ..utils import CheckContext
44
from ..utils import CheckResult
55
from ..utils import Status
6+
from .patch_add import check_add_attribute
7+
from .patch_remove import check_remove_attribute
8+
from .patch_replace import check_replace_attribute
69
from .resource_delete import object_deletion
710
from .resource_get import _model_from_resource_type
811
from .resource_get import object_query
@@ -49,4 +52,9 @@ def resource_type_tests(
4952
results.extend(object_replacement(context, model))
5053
results.extend(object_deletion(context, model))
5154

55+
# PATCH operations
56+
results.extend(check_add_attribute(context, model))
57+
results.extend(check_remove_attribute(context, model))
58+
results.extend(check_replace_attribute(context, model))
59+
5260
return results

0 commit comments

Comments
 (0)