From 6fc085e78dcf20fc647f7b66ae47cfe2f9d613a9 Mon Sep 17 00:00:00 2001 From: Rui Chen Date: Wed, 2 Dec 2015 15:02:22 +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: If31659c503007e08baa0a23485a49a5e5dde8e8b Closes-Bug: #1499369 --- muranoclient/common/base.py | 2 - .../openstack/common/apiclient/base.py | 2 - muranoclient/tests/unit/test_base.py | 46 +++++++++++++++++++ 3 files changed, 46 insertions(+), 4 deletions(-) create mode 100644 muranoclient/tests/unit/test_base.py diff --git a/muranoclient/common/base.py b/muranoclient/common/base.py index 1decb49c..14791730 100644 --- a/muranoclient/common/base.py +++ b/muranoclient/common/base.py @@ -202,8 +202,6 @@ class Resource(object): def __eq__(self, other): 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/muranoclient/openstack/common/apiclient/base.py b/muranoclient/openstack/common/apiclient/base.py index 1dc3397d..bc56106e 100644 --- a/muranoclient/openstack/common/apiclient/base.py +++ b/muranoclient/openstack/common/apiclient/base.py @@ -511,8 +511,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/muranoclient/tests/unit/test_base.py b/muranoclient/tests/unit/test_base.py new file mode 100644 index 00000000..e438c622 --- /dev/null +++ b/muranoclient/tests/unit/test_base.py @@ -0,0 +1,46 @@ +# 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 muranoclient.common import base +from muranoclient.v1 import packages + + +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 = packages.Package(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)