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", {})