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: I5904b82d80d6011c1c3f220c357cc14e8d98da61
Closes-Bug: #1499369

heatclient/openstack/common/apiclient/base.py

@@ -39,14 +39,17 @@ Base utilities to build API operation managers and objects on top of.
 
 import abc
 import copy
+import logging
 
 from oslo_utils import strutils
 import six
 from six.moves.urllib import parse
 
-from heatclient.openstack.common._i18n import _
+from heatclient.openstack.common._i18n import _, _LW
 from heatclient.openstack.common.apiclient import exceptions
 
+LOG = logging.getLogger(__name__)
+
 
 def getid(obj):
     """Return id if argument is a Resource.
@@ -518,10 +521,19 @@ 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
+        LOG.warning(_LW("Two objects are equal when all of the attributes are "
+                        "equal, if you want to identify whether two objects "
+                        "are same one with same id, please use is_same_obj() "
+                        "function."))
         return self._info == other._info
 
+    def is_same_obj(self, other):
+        """Identify the two objects are same one with same id."""
+        if isinstance(other, self.__class__):
+            if hasattr(self, 'id') and hasattr(other, 'id'):
+                return self.id == other.id
+        return False
+
     def is_loaded(self):
         return self._loaded
 

heatclient/tests/unit/test_openstack_common.py

@@ -0,0 +1,75 @@
+# Copyright 2015 Huawei.
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+import testtools
+
+from heatclient.openstack.common.apiclient import base
+from heatclient.v1 import events
+from heatclient.v1 import stacks
+
+
+class BaseTest(testtools.TestCase):
+
+    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 = events.Event(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)
+
+    def test_is_same_object(self):
+        # Two resources with same type and same ID: is same object
+        r1 = base.Resource(None, {'id': 1, 'name': 'hi'})
+        r2 = base.Resource(None, {'id': 1, 'name': 'hello'})
+        self.assertTrue(r1.is_same_obj(r2))
+        self.assertTrue(r2.is_same_obj(r1))
+
+    def test_is_diff_object_with_diff_id(self):
+        # Two resources with same type and different ID: is different object
+        r1 = base.Resource(None, {'id': 1, 'name': 'hello'})
+        r2 = base.Resource(None, {'id': 2, 'name': 'hello'})
+        self.assertFalse(r1.is_same_obj(r2))
+        self.assertFalse(r2.is_same_obj(r1))
+
+    def test_is_diff_object_with_diff_type(self):
+        # Two resources with different types: is different object
+        r1 = events.Event(None, {'id': 1})
+        r2 = stacks.Stack(None, {'id': 1})
+        self.assertFalse(r1.is_same_obj(r2))
+        self.assertFalse(r2.is_same_obj(r1))
+
+    def test_is_diff_object_with_no_id(self):
+        # Two resources with no ID: is different object
+        r1 = base.Resource(None, {'name': 'joe', 'age': 12})
+        r2 = base.Resource(None, {'name': 'joe', 'age': 12})
+        self.assertFalse(r1.is_same_obj(r2))
+        self.assertFalse(r2.is_same_obj(r1))