From 932ac60dcbdc70c9f1ddf0968945cd43028081cc Mon Sep 17 00:00:00 2001
From: Rui Chen <chenrui.momo@gmail.com>
Date: Tue, 1 Dec 2015 20:25:18 +0800
Subject: [PATCH] Fix Resource.__eq__ mismatch semantics of object equal

The __eq__ of apiclient.base.Resource will return True,
if the two objects have same id, even if they have different
other attributes value. The behavior is weird and don't
match the semantics of object equal. The objects that have
different value should be different objects.
Fix this issue and add some test cases in this patch.

Change-Id: I1d072a900d07449b744f4e743e04a57e42109730
Closes-Bug: #1499369
---
 ironicclient/common/apiclient/base.py         |  2 --
 .../tests/unit/common/apiclient/test_base.py  | 24 +++++++++++++++++++
 2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/ironicclient/common/apiclient/base.py b/ironicclient/common/apiclient/base.py
index b8cb626a0..ef82d22fc 100644
--- a/ironicclient/common/apiclient/base.py
+++ b/ironicclient/common/apiclient/base.py
@@ -504,8 +504,6 @@ class Resource(object):
         # two resources of different types are not equal
         if not isinstance(other, self.__class__):
             return False
-        if hasattr(self, 'id') and hasattr(other, 'id'):
-            return self.id == other.id
         return self._info == other._info
 
     def is_loaded(self):
diff --git a/ironicclient/tests/unit/common/apiclient/test_base.py b/ironicclient/tests/unit/common/apiclient/test_base.py
index 7258ec3b2..9e2d6dfc0 100644
--- a/ironicclient/tests/unit/common/apiclient/test_base.py
+++ b/ironicclient/tests/unit/common/apiclient/test_base.py
@@ -79,6 +79,30 @@ class ResourceTest(test_base.BaseTestCase):
         r = HumanResource(None, {"name": None})
         self.assertIsNone(r.human_id)
 
+    def test_two_resources_with_same_id_are_not_equal(self):
+        # Two resources with same ID: never equal if their info is not equal
+        r1 = base.Resource(None, {'id': 1, 'name': 'hi'})
+        r2 = base.Resource(None, {'id': 1, 'name': 'hello'})
+        self.assertNotEqual(r1, r2)
+
+    def test_two_resources_with_same_id_and_info_are_equal(self):
+        # Two resources with same ID: equal if their info is equal
+        r1 = base.Resource(None, {'id': 1, 'name': 'hello'})
+        r2 = base.Resource(None, {'id': 1, 'name': 'hello'})
+        self.assertEqual(r1, r2)
+
+    def test_two_resources_with_diff_type_are_not_equal(self):
+        # Two resoruces of different types: never equal
+        r1 = base.Resource(None, {'id': 1})
+        r2 = HumanResource(None, {'id': 1})
+        self.assertNotEqual(r1, r2)
+
+    def test_two_resources_with_no_id_are_equal(self):
+        # Two resources with no ID: equal if their info is equal
+        r1 = base.Resource(None, {'name': 'joe', 'age': 12})
+        r2 = base.Resource(None, {'name': 'joe', 'age': 12})
+        self.assertEqual(r1, r2)
+
 
 class BaseManagerTestCase(test_base.BaseTestCase):