diff --git a/app/serialization.py b/app/serialization.py index 74e689c966..a45d061217 100644 --- a/app/serialization.py +++ b/app/serialization.py @@ -1,3 +1,4 @@ +from collections import defaultdict from datetime import timezone from marshmallow import ValidationError @@ -41,7 +42,7 @@ def deserialize_host(raw_data): validated_data.get("account"), facts, tags, - validated_data.get("system_profile", {}), + validated_data.get("system_profile"), ) @@ -61,7 +62,7 @@ def serialize_host(host): def serialize_host_system_profile(host): - return {"id": _serialize_uuid(host.id), "system_profile": host.system_profile_facts or {}} + return {"id": _serialize_uuid(host.id), "system_profile": host.system_profile_facts} def _deserialize_canonical_facts(data): @@ -73,13 +74,12 @@ def serialize_canonical_facts(canonical_facts): def _deserialize_facts(data): - facts = {} + facts = defaultdict(lambda: {}) for fact in [] if data is None else data: try: - if fact["namespace"] in facts: - facts[fact["namespace"]].update(fact["facts"]) - else: - facts[fact["namespace"]] = fact["facts"] + old_facts = facts[fact["namespace"]] + new_facts = fact["facts"] or {} + facts[fact["namespace"]] = {**old_facts, **new_facts} except KeyError: # The facts from the request are formatted incorrectly raise InputFormatException( @@ -89,7 +89,12 @@ def _deserialize_facts(data): def _serialize_facts(facts): - return [{"namespace": namespace, "facts": facts or {}} for namespace, facts in facts.items()] + return [{"namespace": namespace, "facts": facts} for namespace, facts in facts.items()] + + +def _deserialize_tags(tags): + # TODO: Move the deserialization logic to this method. + return Tag.create_nested_from_tags(Tag.create_structered_tags_from_tag_data_list(tags)) def _serialize_datetime(dt): @@ -98,8 +103,3 @@ def _serialize_datetime(dt): def _serialize_uuid(u): return str(u) - - -def _deserialize_tags(tags): - # TODO: Move the deserialization logic to this method. - return Tag.create_nested_from_tags(Tag.create_structered_tags_from_tag_data_list(tags)) diff --git a/lib/host_repository.py b/lib/host_repository.py index 31975a7298..f43f981d8a 100644 --- a/lib/host_repository.py +++ b/lib/host_repository.py @@ -61,11 +61,12 @@ def _find_host_by_elevated_ids(account_number, canonical_facts): def _canonical_facts_host_query(account_number, canonical_facts): + cf_values = dict(filter(lambda item: item[1] is not None, canonical_facts.items())) return Host.query.filter( (Host.account == account_number) & ( - Host.canonical_facts.comparator.contains(canonical_facts) - | Host.canonical_facts.comparator.contained_by(canonical_facts) + Host.canonical_facts.comparator.contains(cf_values) + | Host.canonical_facts.comparator.contained_by(cf_values) ) ) diff --git a/test_unit.py b/test_unit.py index af03e574d9..39200bd69e 100755 --- a/test_unit.py +++ b/test_unit.py @@ -561,6 +561,7 @@ def test_with_only_required_fields(self): host = deserialize_host({"account": account, **canonical_facts}) self.assertIs(Host, type(host)) + self.assertEqual(canonical_facts, host.canonical_facts) self.assertIsNone(host.display_name) self.assertIsNone(host.ansible_host) @@ -761,7 +762,7 @@ def test_without_system_profile( input["account"], deserialize_facts.return_value, deserialize_tags.return_value, - {}, + None, ) def test_host_validation( @@ -849,7 +850,22 @@ def test_with_all_fields(self): def test_with_only_required_fields(self): unchanged_data = {"display_name": None, "account": None} - host_init_data = {"canonical_facts": {"fqdn": "some fqdn"}, **unchanged_data, "facts": {}} + host_init_data = { + "canonical_facts": { + "insights_id": None, + "rhel_machine_id": None, + "subscription_manager_id": None, + "satellite_id": None, + "bios_uuid": None, + "ip_addresses": None, + "fqdn": "some fqdn", + "mac_addresses": None, + "external_id": None, + "ansible_host": None, + }, + **unchanged_data, + "facts": {}, + } host = Host(**host_init_data) host_attr_data = {"id": uuid4(), "created_on": datetime.utcnow(), "modified_on": datetime.utcnow()} @@ -859,15 +875,6 @@ def test_with_only_required_fields(self): actual = serialize_host(host) expected = { **host_init_data["canonical_facts"], - "insights_id": None, - "rhel_machine_id": None, - "subscription_manager_id": None, - "satellite_id": None, - "bios_uuid": None, - "ip_addresses": None, - "mac_addresses": None, - "external_id": None, - "ansible_host": None, **unchanged_data, "facts": [], "id": str(host_attr_data["id"]), @@ -938,7 +945,6 @@ def test_non_empty_profile_is_not_changed(self): def test_empty_profile_is_empty_dict(self): host = Host(canonical_facts={"fqdn": "some fqdn"}, display_name="some display name") host.id = uuid4() - host.system_profile_facts = None actual = serialize_host_system_profile(host) expected = {"id": str(host.id), "system_profile": {}} @@ -991,19 +997,6 @@ def test_unknown_fields_are_rejected(self): result = _deserialize_canonical_facts(input) self.assertEqual(result, canonical_facts) - def test_empty_fields_are_rejected(self): - canonical_facts = {"fqdn": "some fqdn"} - input = { - **canonical_facts, - "insights_id": "", - "rhel_machine_id": None, - "ip_addresses": [], - "mac_addresses": tuple(), - } - result = _deserialize_canonical_facts(input) - self.assertEqual(result, canonical_facts) - - class SerializationSerializeCanonicalFactsTestCase(TestCase): def test_contains_all_values_unchanged(self): canonical_facts = { @@ -1019,20 +1012,6 @@ def test_contains_all_values_unchanged(self): } self.assertEqual(canonical_facts, serialize_canonical_facts(canonical_facts)) - def test_missing_fields_are_filled_with_none(self): - canonical_fact_fields = ( - "insights_id", - "rhel_machine_id", - "subscription_manager_id", - "satellite_id", - "bios_uuid", - "ip_addresses", - "fqdn", - "mac_addresses", - "external_id", - ) - self.assertEqual({field: None for field in canonical_fact_fields}, serialize_canonical_facts({})) - class SerializationDeserializeFactsTestCase(TestCase): def test_non_empty_namespaces_become_dict_items(self): @@ -1042,14 +1021,14 @@ def test_non_empty_namespaces_become_dict_items(self): ] self.assertEqual({item["namespace"]: item["facts"] for item in input}, _deserialize_facts(input)) - def test_empty_namespaces_remain_unchanged(self): + def test_empty_namespaces_become_empty_dict(self): for empty_facts in ({}, None): with self.subTest(empty_facts=empty_facts): input = [ {"namespace": "first namespace", "facts": {"first key": "first value"}}, {"namespace": "second namespace", "facts": empty_facts}, ] - self.assertEqual({item["namespace"]: item["facts"] for item in input}, _deserialize_facts(input)) + self.assertEqual({item["namespace"]: item["facts"] or {} for item in input}, _deserialize_facts(input)) def test_duplicate_namespaces_are_merged(self): input = [ @@ -1095,13 +1074,10 @@ def test_non_empty_namespaces_become_list_of_dicts(self): ) def test_empty_namespaces_have_facts_as_empty_dicts(self): - for empty_value in {}, None: - with self.subTest(empty_value=empty_value): - facts = {"first namespace": empty_value, "second namespace": {"first key": "first value"}} - self.assertEqual( - [{"namespace": namespace, "facts": facts or {}} for namespace, facts in facts.items()], - _serialize_facts(facts), - ) + facts = {"first namespace": {}, "second namespace": {"first key": "first value"}} + self.assertEqual( + [{"namespace": namespace, "facts": facts} for namespace, facts in facts.items()], _serialize_facts(facts) + ) class SerializationSerializeDatetime(TestCase):