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.

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

Implemented Entity __eq__ method comparing all fields #350

Merged
merged 3 commits into from Jan 10, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 18 additions & 4 deletions nailgun/entity_mixins.py
Expand Up @@ -83,6 +83,7 @@ def _poll_task(task_id, server_config, poll_rate=None, timeout=None):
def raise_task_timeout(): # pragma: no cover
"""Raise a KeyboardInterrupt exception in the main thread."""
thread.interrupt_main()

timer = threading.Timer(timeout, raise_task_timeout)

# Poll until the task finishes. The timeout prevents an infinite loop.
Expand Down Expand Up @@ -551,17 +552,30 @@ def to_json_dict(self):
json_dct = {}
for field_name, field in fields.items():
if field_name in values:
value = values[field_name]
if value is None:
json_dct[field_name] = None
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

coveralls says this line misses test coverage

if isinstance(field, OneToOneField):
json_dct[field_name] = values[field_name].to_json_dict()
json_dct[field_name] = value.to_json_dict()
elif isinstance(field, OneToManyField):
json_dct[field_name] = [
entity.to_json_dict() for entity in values[field_name]
entity.to_json_dict() for entity in value
]
else:
json_dct[field_name] = to_json_serializable(
values[field_name])
json_dct[field_name] = to_json_serializable(value)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did the following test:

In [1]: from nailgun.entity_mixins import to_json_serializable

In [2]: to_json_serializable(None)

In [3]: print(to_json_serializable(None))
None

There is no need to treat the value is None since the else clause will already take care of it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At first I was doing it. But discovered a case during test on robottelo where an OneToOne is None and. 'json_dct[field_name] = value.to_json_dict()' lead to error because to_json_dict() is been called on a None value.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Finnaly found the case I mentioned above:

hostgroup = entities.HostGroup(
            location=[self.loc],
            organization=[self.org],
        ).create()
        hostgroup.to_json_dict()

Stack trace:

    def to_json_dict(self):
        """Create a dct with Entity properties for json encoding.
            It can be overridden by subclasses for each standard serialization
            doesn't work. By default it call _to_json_dict on OneToOne fields
            and build a list calling the same method on each object on OneToMany
            fields.
    
            :return: dct
            """
        fields, values = self.get_fields(), self.get_values()
        json_dct = {}
        for field_name, field in fields.items():
            if field_name in values:
                if isinstance(field, OneToOneField):
>                   json_dct[field_name] = values[field_name].to_json_dict()
E                   AttributeError: 'NoneType' object has no attribute 'to_json_dict'

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, do you mind adding a comment on that if clause? It may happen that someone else realize that the json_dct[field_name] = to_json_serializable(value) works for None and is not aware of that specific case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@elyezer: done!

return json_dct

def __eq__(self, other):
"""Compare two entities based on their properties. Even nested
objects are considered for equality

:param other: entity to compare self to
:return: boolean indicating if entities are equal or not
"""
if other is None:
return False
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line also missing test coverage

return self.to_json_dict() == other.to_json_dict()


class EntityDeleteMixin(object):
"""This mixin provides the ability to delete an entity.
Expand Down
74 changes: 74 additions & 0 deletions tests/test_entity_mixins.py
Expand Up @@ -7,6 +7,7 @@
from nailgun import client, config, entity_mixins
from nailgun.entity_fields import (
IntegerField,
ListField,
OneToManyField,
OneToOneField,
StringField,
Expand Down Expand Up @@ -60,6 +61,25 @@ def __init__(self, server_config=None, **kwargs):
super(SampleEntityTwo, self).__init__(server_config, **kwargs)


class SampleEntityThree(entity_mixins.Entity):
"""An entity with foreign key fields as One to One and ListField.

This class has a :class:`nailgun.entity_fields.OneToOneField` called
"one_to_one" pointing to :class:`tests.test_entity_mixins.SampleEntityTwo`.

This class has a :class:`nailgun.entity_fields.ListField` called "list"
containing instances of :class:`tests.test_entity_mixins.SampleEntity`.

"""

def __init__(self, server_config=None, **kwargs):
self._fields = {
'one_to_one': OneToOneField(SampleEntityTwo),
'list': ListField()
}
super(SampleEntityThree, self).__init__(server_config, **kwargs)


class EntityWithCreate(entity_mixins.Entity, entity_mixins.EntityCreateMixin):
"""Inherits from :class:`nailgun.entity_mixins.EntityCreateMixin`."""

Expand Down Expand Up @@ -344,6 +364,59 @@ def test_bad_value_error(self):
with self.assertRaises(entity_mixins.BadValueError):
SampleEntityTwo(self.cfg, one_to_many=1)

def test_eq(self):
"""Test method ``nailgun.entity_mixins.Entity.__eq__``.

Assert that ``__eq__`` works comparing all attributes, even from
nested structures.
"""
# Testing simple properties
alice = SampleEntity(self.cfg, id=1, name='Alice')
alice_clone = SampleEntity(self.cfg, id=1, name='Alice')
self.assertEquals(alice, alice_clone)

alice_id_2 = SampleEntity(self.cfg, id=2, name='Alice')
self.assertNotEquals(alice, alice_id_2)

# Testing OneToMany nested objects

john = SampleEntityTwo(self.cfg, one_to_many=[alice, alice_id_2])
john_clone = SampleEntityTwo(self.cfg, one_to_many=[alice, alice_id_2])
self.assertEquals(john, john_clone)

john_different_order = SampleEntityTwo(self.cfg, one_to_many=[
alice_id_2, alice,
])
self.assertNotEquals(john, john_different_order)

john_missing_alice = SampleEntityTwo(self.cfg, one_to_many=[alice])
self.assertNotEquals(john, john_missing_alice)

john_without_alice = SampleEntityTwo(self.cfg)
self.assertNotEquals(john, john_without_alice)

# Testing OneToOne nested objects

mary = SampleEntityThree(self.cfg, one_to_one=john)
mary_clone = SampleEntityThree(
self.cfg, one_to_one=john_clone)
self.assertEquals(mary, mary_clone)

mary_different = SampleEntityThree(
self.cfg, one_to_one=john_different_order)
self.assertNotEquals(mary, mary_different)

mary_without_john = SampleEntityThree(self.cfg)
self.assertNotEquals(mary, mary_without_john)

# Testing List nested objects
# noqa pylint:disable=attribute-defined-outside-init
mary.list = [alice]
self.assertNotEquals(mary, mary_clone)
# noqa pylint:disable=attribute-defined-outside-init
mary_clone.list = [alice_clone]
self.assertEquals(mary, mary_clone)

def test_repr_v1(self):
"""Test method ``nailgun.entity_mixins.Entity.__repr__``.

Expand Down Expand Up @@ -546,6 +619,7 @@ def setUpClass(cls):
``test_entity`` is a class having one to one and one to many fields.

"""

class TestEntity(entity_mixins.Entity, entity_mixins.EntityReadMixin):
"""An entity with several different types of fields."""

Expand Down