diff --git a/app/serialization.py b/app/serialization.py index d66007022..915ac51c4 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"), validated_data.get("stale_timestamp"), validated_data.get("reporter"), ) @@ -76,7 +77,7 @@ def serialize_host(host, staleness_offset): 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): @@ -88,13 +89,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( @@ -104,7 +104,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): @@ -113,8 +118,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 81084b269..194b0a39e 100644 --- a/lib/host_repository.py +++ b/lib/host_repository.py @@ -60,11 +60,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 b6ccdfa65..b7a9dd54a 100755 --- a/test_unit.py +++ b/test_unit.py @@ -580,6 +580,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) @@ -794,7 +795,7 @@ def test_without_system_profile( input["account"], deserialize_facts.return_value, deserialize_tags.return_value, - {}, + None, input["stale_timestamp"], input["reporter"], ) @@ -942,7 +943,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()} @@ -953,15 +969,6 @@ def test_with_only_required_fields(self): actual = serialize_host(host, staleness_offset) 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"]), @@ -1075,7 +1082,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": {}} @@ -1128,19 +1134,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 = { @@ -1156,20 +1149,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): @@ -1179,14 +1158,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 = [ @@ -1232,13 +1211,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):