Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Serialization: Host serialization cleanup #529

Closed
wants to merge 2 commits into from
Closed

Serialization: Host serialization cleanup #529

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a7fb786
Trust the deserialized values
Glutexo Jul 3, 2019
0199550
Merge branch 'master' into host_serialization_cleanup
jharting Dec 6, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 13 additions & 13 deletions app/serialization.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from collections import defaultdict
from datetime import timezone

from marshmallow import ValidationError
Expand Down Expand Up @@ -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"),
)
Expand Down Expand Up @@ -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):
Expand All @@ -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(
Expand All @@ -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):
Expand All @@ -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))
5 changes: 3 additions & 2 deletions lib/host_repository.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
)
)

Expand Down
72 changes: 24 additions & 48 deletions test_unit.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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"],
)
Expand Down Expand Up @@ -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()}
Expand All @@ -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"]),
Expand Down Expand Up @@ -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": {}}
Expand Down Expand Up @@ -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 = {
Expand All @@ -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):
Expand All @@ -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 = [
Expand Down Expand Up @@ -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):
Expand Down