Skip to content

Commit

Permalink
feature/skip None values when merging (#373)
Browse files Browse the repository at this point in the history
# PR Context
- this broke the merging of optional fields, e.g. on ExtractedResources

# Fixed
- skip None values when merging extracted and rule items
  • Loading branch information
cutoffthetop authored Jan 24, 2025
1 parent e198e5f commit b9e8de1
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 6 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

- skip None values when merging extracted and rule items

### Security

## [0.47.0] - 2025-01-23
Expand Down
3 changes: 2 additions & 1 deletion mex/common/merged/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -153,4 +153,5 @@ def create_merged_item(
try:
return cls.model_validate(merged_dict)
except ValidationError as error:
raise MergingError from error
msg = "Could not validate merged model."
raise MergingError(msg) from error
16 changes: 12 additions & 4 deletions mex/common/merged/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,28 @@
T = TypeVar("T")


def extend_list_in_dict(dict_: dict[str, list[T]], key: str, item: list[T] | T) -> None:
def extend_list_in_dict(
dict_: dict[str, list[T]], key: str, item: list[T] | T | None
) -> None:
"""Extend a list in a dict for a given key with the given unique item(s)."""
list_ = dict_.setdefault(key, [])
if not isinstance(item, list):
if item is None:
item = []
elif not isinstance(item, list):
item = [item]
for mergeable in item:
if mergeable not in list_:
list_.append(mergeable)


def prune_list_in_dict(dict_: dict[str, list[T]], key: str, item: list[T] | T) -> None:
def prune_list_in_dict(
dict_: dict[str, list[T]], key: str, item: list[T] | T | None
) -> None:
"""Safely remove item(s) from a list in a dict for the given key."""
list_ = dict_.setdefault(key, [])
if not isinstance(item, list):
if item is None:
item = []
elif not isinstance(item, list):
item = [item]
for removable in item:
with contextlib.suppress(ValueError):
Expand Down
36 changes: 35 additions & 1 deletion tests/merged/test_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,22 +13,27 @@
ActivityRuleSetRequest,
AdditiveOrganizationalUnit,
AdditivePerson,
AdditiveResource,
AnyExtractedModel,
AnyRuleSetRequest,
ContactPointRuleSetRequest,
ExtractedActivity,
ExtractedContactPoint,
ExtractedPerson,
ExtractedResource,
PersonRuleSetRequest,
PreventiveContactPoint,
PreventivePerson,
PreventiveResource,
ResourceRuleSetRequest,
SubtractiveActivity,
SubtractiveContactPoint,
SubtractiveOrganizationalUnit,
SubtractivePerson,
SubtractiveResource,
)
from mex.common.testing import Joker
from mex.common.types import Identifier, Text, TextLanguage
from mex.common.types import AccessRestriction, Identifier, Text, TextLanguage, Theme


def test_merge_extracted_items_and_apply_preventive_rule() -> None:
Expand Down Expand Up @@ -121,6 +126,34 @@ def test_apply_subtractive_rule() -> None:
@pytest.mark.parametrize(
("extracted_items", "rule_set", "validate_cardinality", "expected"),
[
(
[
ExtractedResource(
identifierInPrimarySource="r1",
hadPrimarySource=Identifier.generate(seed=42),
accessRestriction=AccessRestriction["OPEN"],
contact=[Identifier.generate(seed=999)],
unitInCharge=[Identifier.generate(seed=999)],
theme=[Theme["PUBLIC_HEALTH"]],
title=[Text(value="Dummy resource")],
)
],
ResourceRuleSetRequest(
additive=AdditiveResource(),
subtractive=SubtractiveResource(),
preventive=PreventiveResource(),
),
True,
{
"accessRestriction": "https://mex.rki.de/item/access-restriction-1",
"contact": ["bFQoRhcVH5DIax"],
"theme": ["https://mex.rki.de/item/theme-1"],
"title": [{"value": "Dummy resource", "language": TextLanguage.EN}],
"unitInCharge": ["bFQoRhcVH5DIax"],
"entityType": "MergedResource",
"identifier": "bFQoRhcVH5DHU6",
},
),
(
[
ExtractedPerson(
Expand Down Expand Up @@ -295,6 +328,7 @@ def test_apply_subtractive_rule() -> None:
),
],
ids=(
"single extracted item",
"extracted items and rule set",
"only rule set",
"only extracted items",
Expand Down
25 changes: 25 additions & 0 deletions tests/merged/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,28 @@ def test_extend_list_in_dict() -> None:
"new-key": ["single-value"],
}

extend_list_in_dict(dict_, "existing-key", None)
assert dict_ == {
"existing-key": ["already-here", "another-value"],
"new-key": ["single-value"],
}

extend_list_in_dict(dict_, "another-key", None)
assert dict_ == {
"existing-key": ["already-here", "another-value"],
"new-key": ["single-value"],
"another-key": [],
}


def test_prune_list_in_dict() -> None:
dict_ = {
"existing-key": ["leave-me-alone"],
"prune-me": ["1", "2", "42", "foo"],
}

prune_list_in_dict(dict_, "prune-me", ["1", "2"])
assert dict_ == {
"existing-key": ["leave-me-alone"],
"prune-me": ["42", "foo"],
}
Expand All @@ -29,6 +48,12 @@ def test_prune_list_in_dict() -> None:
"prune-me": ["42"],
}

prune_list_in_dict(dict_, "prune-me", None)
assert dict_ == {
"existing-key": ["leave-me-alone"],
"prune-me": ["42"],
}

prune_list_in_dict(dict_, "does-not-exist", "42")
assert dict_ == {
"existing-key": ["leave-me-alone"],
Expand Down

0 comments on commit b9e8de1

Please sign in to comment.