From 1772a9116d127294354addd830e16877658bbee3 Mon Sep 17 00:00:00 2001 From: Jia Dong Date: Fri, 20 Dec 2013 15:37:25 +0800 Subject: [PATCH] Remove return stmt of add,save and remove method Remove the add and save method's return statement in the ImageMemberRepo class, as the same as ImageRepo class. Also modify authorization.py and policy.py and the related unittest. Closes-Bug: #1254210 Change-Id: I472cd15af8648beea10abc595e905618091f3dab --- glance/api/authorization.py | 5 ++--- glance/api/policy.py | 6 +++--- glance/api/v2/image_members.py | 6 +++--- glance/db/__init__.py | 2 -- glance/store/__init__.py | 6 ++---- glance/tests/unit/test_db.py | 13 ++++++++----- glance/tests/unit/test_policy.py | 32 ++++++++++++++++++++------------ 7 files changed, 38 insertions(+), 32 deletions(-) diff --git a/glance/api/authorization.py b/glance/api/authorization.py index 354be25d8e..f1279ece40 100644 --- a/glance/api/authorization.py +++ b/glance/api/authorization.py @@ -132,7 +132,7 @@ class ImageMemberRepoProxy(glance.domain.proxy.Repo): def add(self, image_member): if (self.image.owner == self.context.owner or self.context.is_admin): - return self.member_repo.add(image_member) + self.member_repo.add(image_member) else: message = _("You cannot add image member for %s") raise exception.Forbidden(message @@ -141,8 +141,7 @@ class ImageMemberRepoProxy(glance.domain.proxy.Repo): def save(self, image_member): if (self.context.is_admin or self.context.owner == image_member.member_id): - updated_member = self.member_repo.save(image_member) - return proxy_member(self.context, updated_member) + self.member_repo.save(image_member) else: message = _("You cannot update image member %s") raise exception.Forbidden(message % image_member.member_id) diff --git a/glance/api/policy.py b/glance/api/policy.py index 90e0e90873..d182fdee3c 100644 --- a/glance/api/policy.py +++ b/glance/api/policy.py @@ -278,7 +278,7 @@ class ImageMemberRepoProxy(glance.domain.proxy.Repo): def add(self, member): self.policy.enforce(self.context, 'add_member', {}) - return self.member_repo.add(member) + self.member_repo.add(member) def get(self, member_id): self.policy.enforce(self.context, 'get_member', {}) @@ -286,7 +286,7 @@ class ImageMemberRepoProxy(glance.domain.proxy.Repo): def save(self, member): self.policy.enforce(self.context, 'modify_member', {}) - return self.member_repo.save(member) + self.member_repo.save(member) def list(self, *args, **kwargs): self.policy.enforce(self.context, 'get_members', {}) @@ -294,7 +294,7 @@ class ImageMemberRepoProxy(glance.domain.proxy.Repo): def remove(self, member): self.policy.enforce(self.context, 'delete_member', {}) - return self.member_repo.remove(member) + self.member_repo.remove(member) class ImageLocationsProxy(object): diff --git a/glance/api/v2/image_members.py b/glance/api/v2/image_members.py index 973ca77612..abfd4257f5 100644 --- a/glance/api/v2/image_members.py +++ b/glance/api/v2/image_members.py @@ -64,9 +64,9 @@ class ImageMembersController(object): member_repo = image.get_member_repo() new_member = image_member_factory.new_image_member(image, member_id) - member = member_repo.add(new_member) + member_repo.add(new_member) - return member + return new_member except exception.NotFound as e: raise webob.exc.HTTPNotFound(explanation=unicode(e)) except exception.Forbidden as e: @@ -98,7 +98,7 @@ class ImageMembersController(object): member_repo = image.get_member_repo() member = member_repo.get(member_id) member.status = status - member = member_repo.save(member) + member_repo.save(member) return member except exception.NotFound as e: raise webob.exc.HTTPNotFound(explanation=unicode(e)) diff --git a/glance/db/__init__.py b/glance/db/__init__.py index 466cc72b2a..d7d9940e89 100644 --- a/glance/db/__init__.py +++ b/glance/db/__init__.py @@ -246,7 +246,6 @@ class ImageMemberRepo(object): image_member.created_at = new_values['created_at'] image_member.updated_at = new_values['updated_at'] image_member.id = new_values['id'] - return self._format_image_member_from_db(new_values) def remove(self, image_member): try: @@ -264,7 +263,6 @@ class ImageMemberRepo(object): except (exception.NotFound, exception.Forbidden): raise exception.NotFound() image_member.updated_at = new_values['updated_at'] - return self._format_image_member_from_db(new_values) def get(self, member_id): try: diff --git a/glance/store/__init__.py b/glance/store/__init__.py index 3edaae69d5..fb51d1d483 100644 --- a/glance/store/__init__.py +++ b/glance/store/__init__.py @@ -691,11 +691,9 @@ class ImageMemberRepoProxy(glance.domain.proxy.Repo): read_tenants=member_ids) def add(self, member): - result = super(ImageMemberRepoProxy, self).add(member) + super(ImageMemberRepoProxy, self).add(member) self._set_acls() - return result def remove(self, member): - result = super(ImageMemberRepoProxy, self).remove(member) + super(ImageMemberRepoProxy, self).remove(member) self._set_acls() - return result diff --git a/glance/tests/unit/test_db.py b/glance/tests/unit/test_db.py index cf60c03e27..8a9ef354f1 100644 --- a/glance/tests/unit/test_db.py +++ b/glance/tests/unit/test_db.py @@ -441,7 +441,8 @@ class TestImageMemberRepo(test_utils.BaseTestCase): def test_save_image_member(self): image_member = self.image_member_repo.get(TENANT2) image_member.status = 'accepted' - image_member_updated = self.image_member_repo.save(image_member) + self.image_member_repo.save(image_member) + image_member_updated = self.image_member_repo.get(TENANT2) self.assertEqual(image_member.id, image_member_updated.id) self.assertEqual(image_member_updated.status, 'accepted') @@ -450,8 +451,9 @@ class TestImageMemberRepo(test_utils.BaseTestCase): image_member = self.image_member_factory.new_image_member(image, TENANT4) self.assertTrue(image_member.id is None) - retreived_image_member = self.image_member_repo.add(image_member) - self.assertEqual(retreived_image_member.id, image_member.id) + self.image_member_repo.add(image_member) + retreived_image_member = self.image_member_repo.get(TENANT4) + self.assertIsNotNone(retreived_image_member.id) self.assertEqual(retreived_image_member.image_id, image_member.image_id) self.assertEqual(retreived_image_member.member_id, @@ -464,8 +466,9 @@ class TestImageMemberRepo(test_utils.BaseTestCase): image_member = self.image_member_factory.new_image_member(image, TENANT4) self.assertTrue(image_member.id is None) - retreived_image_member = self.image_member_repo.add(image_member) - self.assertEqual(retreived_image_member.id, image_member.id) + self.image_member_repo.add(image_member) + retreived_image_member = self.image_member_repo.get(TENANT4) + self.assertIsNotNone(retreived_image_member.id) self.assertEqual(retreived_image_member.image_id, image_member.image_id) self.assertEqual(retreived_image_member.member_id, diff --git a/glance/tests/unit/test_policy.py b/glance/tests/unit/test_policy.py index 5dfd4079ac..387273dd00 100644 --- a/glance/tests/unit/test_policy.py +++ b/glance/tests/unit/test_policy.py @@ -63,20 +63,25 @@ class ImageFactoryStub(object): class MemberRepoStub(object): - def add(self, *args, **kwargs): - return 'member_repo_add' + def add(self, image_member): + image_member.output = 'member_repo_add' def get(self, *args, **kwargs): return 'member_repo_get' - def save(self, *args, **kwargs): - return 'member_repo_save' + def save(self, image_member): + image_member.output = 'member_repo_save' def list(self, *args, **kwargs): return 'member_repo_list' - def remove(self, *args, **kwargs): - return 'member_repo_remove' + def remove(self, image_member): + image_member.output = 'member_repo_remove' + + +class ImageMembershipStub(object): + def __init__(self, output=None): + self.output = output class TaskRepoStub(object): @@ -316,8 +321,9 @@ class TestMemberPolicy(test_utils.BaseTestCase): self.policy.enforce.assert_called_once_with({}, "add_member", {}) def test_add_member_allowed(self): - output = self.member_repo.add('') - self.assertEqual(output, 'member_repo_add') + image_member = ImageMembershipStub() + self.member_repo.add(image_member) + self.assertEqual(image_member.output, 'member_repo_add') self.policy.enforce.assert_called_once_with({}, "add_member", {}) def test_get_member_not_allowed(self): @@ -336,8 +342,9 @@ class TestMemberPolicy(test_utils.BaseTestCase): self.policy.enforce.assert_called_once_with({}, "modify_member", {}) def test_modify_member_allowed(self): - output = self.member_repo.save('') - self.assertEqual(output, 'member_repo_save') + image_member = ImageMembershipStub() + self.member_repo.save(image_member) + self.assertEqual(image_member.output, 'member_repo_save') self.policy.enforce.assert_called_once_with({}, "modify_member", {}) def test_get_members_not_allowed(self): @@ -356,8 +363,9 @@ class TestMemberPolicy(test_utils.BaseTestCase): self.policy.enforce.assert_called_once_with({}, "delete_member", {}) def test_delete_member_allowed(self): - output = self.member_repo.remove('') - self.assertEqual(output, 'member_repo_remove') + image_member = ImageMembershipStub() + self.member_repo.remove(image_member) + self.assertEqual(image_member.output, 'member_repo_remove') self.policy.enforce.assert_called_once_with({}, "delete_member", {})