Skip to content

Commit 2553798

Browse files
azmeukazmeuk
committed
fix: compare_fields method when comparing two none values
1 parent 1d07b1a commit 2553798

File tree

6 files changed

+263
-88
lines changed

6 files changed

+263
-88
lines changed

‎scim2_tester/checkers/patch_add.py‎

Lines changed: 23 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
from ..utils import Status
1919
from ..utils import check_result
2020
from ..utils import checker
21-
from ..utils import compare_field
21+
from ..utils import fields_equality
2222

2323

2424
@checker("patch:add")
@@ -123,29 +123,28 @@ def check_add_attribute(
123123
continue
124124

125125
if modify_result is not None:
126-
if modify_actual_value := get_value_by_urn(modify_result, urn):
127-
if not (
128-
mutability == Mutability.write_only
129-
or compare_field(patch_value, modify_actual_value)
130-
):
131-
results.append(
132-
check_result(
133-
context,
134-
status=Status.ERROR,
135-
reason=(
136-
f"PATCH modify() returned unexpected value for '{urn}'.\n"
137-
f"Patched value: {patch_value}\n"
138-
f"Returned value: {modify_actual_value}"
139-
),
140-
resource_type=model.__name__,
141-
data={
142-
"urn": urn,
143-
"expected": patch_value,
144-
"modify_actual": modify_actual_value,
145-
},
146-
)
126+
modify_actual_value = get_value_by_urn(modify_result, urn)
127+
if mutability != Mutability.write_only and not fields_equality(
128+
patch_value, modify_actual_value
129+
):
130+
results.append(
131+
check_result(
132+
context,
133+
status=Status.ERROR,
134+
reason=(
135+
f"PATCH modify() returned unexpected value for '{urn}'.\n"
136+
f"Patched value: {patch_value}\n"
137+
f"Returned value: {modify_actual_value}"
138+
),
139+
resource_type=model.__name__,
140+
data={
141+
"urn": urn,
142+
"expected": patch_value,
143+
"modify_actual": modify_actual_value,
144+
},
147145
)
148-
continue
146+
)
147+
continue
149148

150149
try:
151150
updated_resource = context.client.query(
@@ -170,7 +169,7 @@ def check_add_attribute(
170169

171170
actual_value = get_value_by_urn(updated_resource, urn)
172171

173-
if mutability == Mutability.write_only or compare_field(
172+
if mutability == Mutability.write_only or fields_equality(
174173
patch_value, actual_value
175174
):
176175
results.append(

‎scim2_tester/checkers/patch_replace.py‎

Lines changed: 23 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
from ..utils import Status
1818
from ..utils import check_result
1919
from ..utils import checker
20-
from ..utils import compare_field
20+
from ..utils import fields_equality
2121

2222

2323
@checker("patch:replace")
@@ -117,29 +117,28 @@ def check_replace_attribute(
117117
continue
118118

119119
if modify_result is not None:
120-
if modify_actual_value := get_value_by_urn(modify_result, urn):
121-
if not (
122-
mutability == Mutability.write_only
123-
or compare_field(patch_value, modify_actual_value)
124-
):
125-
results.append(
126-
check_result(
127-
context,
128-
status=Status.ERROR,
129-
reason=(
130-
f"PATCH modify() returned unexpected value for '{urn}'.\n"
131-
f"Patched value: {patch_value}\n"
132-
f"Returned value: {modify_actual_value}"
133-
),
134-
resource_type=model.__name__,
135-
data={
136-
"urn": urn,
137-
"expected": patch_value,
138-
"modify_actual": modify_actual_value,
139-
},
140-
)
120+
modify_actual_value = get_value_by_urn(modify_result, urn)
121+
if mutability != Mutability.write_only and not fields_equality(
122+
patch_value, modify_actual_value
123+
):
124+
results.append(
125+
check_result(
126+
context,
127+
status=Status.ERROR,
128+
reason=(
129+
f"PATCH modify() returned unexpected value for '{urn}'.\n"
130+
f"Patched value: {patch_value}\n"
131+
f"Returned value: {modify_actual_value}"
132+
),
133+
resource_type=model.__name__,
134+
data={
135+
"urn": urn,
136+
"expected": patch_value,
137+
"modify_actual": modify_actual_value,
138+
},
141139
)
142-
continue
140+
)
141+
continue
143142

144143
try:
145144
updated_resource = context.client.query(
@@ -163,7 +162,7 @@ def check_replace_attribute(
163162
continue
164163

165164
actual_value = get_value_by_urn(updated_resource, urn)
166-
if mutability == Mutability.write_only or compare_field(
165+
if mutability == Mutability.write_only or fields_equality(
167166
patch_value, actual_value
168167
):
169168
results.append(

‎scim2_tester/utils.py‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -357,10 +357,7 @@ def wrapped(context: CheckContext, *args: Any, **kwargs: Any) -> Any:
357357
return decorator
358358

359359

360-
def compare_field(expected: Any, actual: Any) -> bool:
361-
if expected is None or actual is None:
362-
return False
363-
360+
def fields_equality(expected: Any, actual: Any) -> bool:
364361
expected = expected.model_dump() if isinstance(expected, BaseModel) else expected
365362
actual = actual.model_dump() if isinstance(actual, BaseModel) else actual
366363

‎tests/test_patch_add.py‎

Lines changed: 37 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,7 @@ def test_patch_add_modify_result_incorrect_value(testing_context):
371371

372372

373373
def test_patch_add_query_failure_after_patch(httpserver, testing_context):
374-
"""Test PATCH add when query fails after successful patch."""
374+
"""Test PATCH add when attribute appears in PATCH response but not in GET."""
375375
httpserver.expect_request(uri="/Users", method="POST").respond_with_json(
376376
{
377377
"schemas": ["urn:ietf:params:scim:schemas:core:2.0:User"],
@@ -381,24 +381,40 @@ def test_patch_add_query_failure_after_patch(httpserver, testing_context):
381381
status=201,
382382
)
383383

384-
httpserver.expect_request(uri="/Users/123", method="PATCH").respond_with_json(
385-
{
386-
"schemas": ["urn:ietf:params:scim:schemas:core:2.0:User"],
387-
"id": "123",
388-
"userName": "[email protected]",
389-
"displayName": "test_display",
390-
},
391-
status=200,
392-
)
384+
resource_state = {
385+
"schemas": ["urn:ietf:params:scim:schemas:core:2.0:User"],
386+
"id": "123",
387+
"userName": "[email protected]",
388+
}
393389

394-
httpserver.expect_request(uri="/Users/123", method="GET").respond_with_json(
395-
{
396-
"schemas": ["urn:ietf:params:scim:schemas:core:2.0:User"],
397-
"id": "123",
398-
"userName": "[email protected]",
399-
# displayName missing - attribute not actually added
400-
},
401-
status=200,
390+
def patch_handler(request):
391+
patch_data = json.loads(request.get_data(as_text=True))
392+
operation = patch_data["Operations"][0]
393+
path = operation["path"]
394+
value = operation["value"]
395+
396+
response_state = resource_state.copy()
397+
if path == "displayName":
398+
response_state["displayName"] = value
399+
400+
return Response(
401+
json.dumps(response_state),
402+
status=200,
403+
headers={"Content-Type": "application/scim+json"},
404+
)
405+
406+
def get_handler(request):
407+
return Response(
408+
json.dumps(resource_state),
409+
status=200,
410+
headers={"Content-Type": "application/scim+json"},
411+
)
412+
413+
httpserver.expect_request(uri="/Users/123", method="PATCH").respond_with_handler(
414+
patch_handler
415+
)
416+
httpserver.expect_request(uri="/Users/123", method="GET").respond_with_handler(
417+
get_handler
402418
)
403419

404420
results = check_add_attribute(testing_context, User)
@@ -408,6 +424,9 @@ def test_patch_add_query_failure_after_patch(httpserver, testing_context):
408424
for r in results
409425
if r.status == Status.ERROR
410426
and "was not added or has unexpected value" in r.reason
427+
and hasattr(r, "data")
428+
and r.data
429+
and r.data.get("urn") == "displayName"
411430
]
412431
assert len(error_results) > 0
413432

@@ -416,7 +435,6 @@ def test_patch_not_supported(testing_context):
416435
"""Test PATCH add returns SKIPPED when PATCH is not supported."""
417436
from unittest.mock import Mock
418437

419-
# Mock ServiceProviderConfig with patch.supported = False
420438
mock_service_provider_config = Mock()
421439
mock_service_provider_config.patch.supported = False
422440
testing_context.client.service_provider_config = mock_service_provider_config

0 commit comments

Comments
 (0)