Skip to content

Commit

Permalink
Trust the deserialized values
Browse files Browse the repository at this point in the history 
Change the (de)serialization logic, so the serialization expects that
the object is valid. All default values and validation happens now upon
deserialization. That concentrates the logic into a single place.
  • Loading branch information
@Glutexo
Glutexo committed Jul 29, 2019
1 parent 7dc333a commit 1e01204
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 65 deletions.
19 changes: 10 additions & 9 deletions app/serialization.py
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from collections import defaultdict

from app.models import Host as ModelsHost
from app.exceptions import InputFormatException

Expand All @@ -22,7 +24,7 @@ def from_json(cls, data):
data.get("ansible_host"),
data.get("account"),
Facts.from_json(data.get("facts")),
data.get("system_profile", {}),
data.get("system_profile")
)

@classmethod
Expand All @@ -42,7 +44,7 @@ def to_json(cls, host):
def to_system_profile_json(cls, host):
return {
"id": _serialize_uuid(host.id),
"system_profile": host.system_profile_facts or {}
"system_profile": host.system_profile_facts
}


Expand Down Expand Up @@ -70,7 +72,7 @@ class CanonicalFacts:

@classmethod
def from_json(cls, data):
return {field: data[field] for field in cls.fields if data.get(field)}
return {field: data[field] or None for field in cls.fields if data.get(field)}

@classmethod
def to_json(cls, canonical_facts):
Expand All @@ -89,13 +91,12 @@ class Facts:

@classmethod
def from_json(cls, data):
facts = {}
facts = defaultdict(lambda: {})
for item in data or []:
try:
if item["namespace"] in facts:
facts[item["namespace"]].update(item["facts"])
else:
facts[item["namespace"]] = item["facts"]
old_facts = facts[item["namespace"]]
new_facts = item["facts"] or {}
facts[item["namespace"]] = {**old_facts, **new_facts}
except KeyError:
# The facts from the request are formatted incorrectly
raise InputFormatException("Invalid format of Fact object. Fact "
Expand All @@ -105,6 +106,6 @@ def from_json(cls, data):
@classmethod
def to_json(cls, facts):
return [
{"namespace": namespace, "facts": facts or {}}
{"namespace": namespace, "facts": facts}
for namespace, facts in facts.items()
]
5 changes: 3 additions & 2 deletions lib/host_repository.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,12 @@ def find_host_by_insights_id(account_number, insights_id):


def _canonical_facts_host_query(account_number, canonical_facts):
cf_values = dict(filter(lambda item: item[1] is not None, canonical_facts.items()))
return ModelsHost.query.filter(
(ModelsHost.account == account_number)
& (
ModelsHost.canonical_facts.comparator.contains(canonical_facts)
| ModelsHost.canonical_facts.comparator.contained_by(canonical_facts)
ModelsHost.canonical_facts.comparator.contains(cf_values)
| ModelsHost.canonical_facts.comparator.contained_by(cf_values)
)
)

Expand Down
5 changes: 5 additions & 0 deletions test_api.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,11 @@ def _validate_host(self, received_host, expected_host,

class CreateHostsTestCase(DBAPITestCase):
def test_create_and_update(self):
from logging import DEBUG, getLogger, INFO

logger = getLogger("sqlalchemy.engine")
logger.setLevel(INFO)

facts = None

host_data = HostWrapper(test_data(facts=facts))
Expand Down
117 changes: 63 additions & 54 deletions test_unit.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -398,11 +398,25 @@ def test_with_all_fields(self):
self.assertEqual(value, getattr(actual, key))

def test_with_only_required_fields(self):
canonical_facts = {"fqdn": "some fqdn"}
host = SerializationHost.from_json(canonical_facts)
create_canonical_facts = {"fqdn": "some fqdn"}
host = SerializationHost.from_json(create_canonical_facts)

self.assertIs(ModelsHost, type(host))
self.assertEqual(canonical_facts, host.canonical_facts)

expected_canonical_facts = {
**create_canonical_facts,
**{field: None for field in (
"insights_id",
"rhel_machine_id",
"subscription_manager_id",
"satellite_id",
"bios_uuid",
"ip_addresses",
"mac_addresses",
"external_id",
)}
}
self.assertEqual(expected_canonical_facts, host.canonical_facts)
self.assertIsNone(host.display_name)
self.assertIsNone(host.ansible_host)
self.assertIsNone(host.account)
Expand Down Expand Up @@ -601,7 +615,18 @@ def test_with_only_required_fields(self):
"account": None
}
host_init_data = {
"canonical_facts": {"fqdn": "some fqdn"},
"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": {}
}
Expand All @@ -618,15 +643,6 @@ def test_with_only_required_fields(self):
actual = SerializationHost.to_json(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"]),
Expand Down Expand Up @@ -721,7 +737,6 @@ def test_empty_profile_is_empty_dict(self):
display_name="some display name"
)
host.id = uuid4()
host.system_profile_facts = None

actual = SerializationHost.to_system_profile_json(host)
expected = {"id": str(host.id), "system_profile": {}}
Expand Down Expand Up @@ -778,17 +793,29 @@ def test_unknown_fields_are_rejected(self):
result = CanonicalFacts.from_json(input)
self.assertEqual(result, canonical_facts)

def test_empty_fields_are_rejected(self):
canonical_facts = {"fqdn": "some fqdn"}
input = {
def test_empty_fields_become_none(self):
canonical_facts = {
"insights_id": str(uuid4()),
"rhel_machine_id": str(uuid4()),
"subscription_manager_id": str(uuid4()),
"satellite_id": str(uuid4()),
"bios_uuid": str(uuid4()),
"ip_addresses": (),
"fqdn": "",
"mac_addresses": [],
"external_id": None
}
actual = CanonicalFacts.from_json(canonical_facts)
expected = {
**canonical_facts,
"insights_id": "",
"rhel_machine_id": None,
"ip_addresses": [],
"mac_addresses": tuple()
**{
"ip_addresses": None,
"fqdn": None,
"mac_addresses": None,
"external_id": None
}
}
result = CanonicalFacts.from_json(input)
self.assertEqual(result, canonical_facts)
self.assertEqual(actual, expected)


class SerializationCanonicalFactsToJsonTestCase(TestCase):
Expand All @@ -807,22 +834,6 @@ def test_contains_all_values_unchanged(self):
}
self.assertEqual(canonical_facts, CanonicalFacts.to_json(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}, CanonicalFacts.to_json({})
)


class SerializationFactsFromJsonTestCase(TestCase):

Expand All @@ -841,7 +852,7 @@ def test_non_empty_namespaces_become_dict_items(self):
{item["namespace"]: item["facts"] for item in input}, Facts.from_json(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 = [
Expand All @@ -855,7 +866,7 @@ def test_empty_namespaces_remain_unchanged(self):
}
]
self.assertEqual(
{item["namespace"]: item["facts"] for item in input},
{item["namespace"]: item["facts"] or {} for item in input},
Facts.from_json(input)
)

Expand Down Expand Up @@ -932,19 +943,17 @@ 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()
],
Facts.to_json(facts)
)
facts = {
"first namespace": {},
"second namespace": {"first key": "first value"}
}
self.assertEqual(
[
{"namespace": namespace, "facts": facts}
for namespace, facts in facts.items()
],
Facts.to_json(facts)
)


if __name__ == "__main__":
Expand Down

0 comments on commit 1e01204

Please sign in to comment.