diff --git a/keystoneclient/base.py b/keystoneclient/base.py index 025362bf9..eabbdc4fb 100644 --- a/keystoneclient/base.py +++ b/keystoneclient/base.py @@ -356,6 +356,17 @@ class CrudManager(Manager): @filter_kwargs def list(self, fallback_to_auth=False, **kwargs): + if 'id' in kwargs.keys(): + # Ensure that users are not trying to call things like + # ``domains.list(id='default')`` when they should have used + # ``[domains.get(domain_id='default')]`` instead. Keystone supports + # ``GET /v3/domains/{domain_id}``, not ``GET + # /v3/domains?id={domain_id}``. + raise TypeError( + _("list() got an unexpected keyword argument 'id'. To " + "retrieve a single object using a globally unique " + "identifier, try using get() instead.")) + url = self.build_url(dict_args_in_out=kwargs) try: diff --git a/keystoneclient/tests/unit/v3/test_domains.py b/keystoneclient/tests/unit/v3/test_domains.py index 9cc23e7eb..4dbfd73d7 100644 --- a/keystoneclient/tests/unit/v3/test_domains.py +++ b/keystoneclient/tests/unit/v3/test_domains.py @@ -30,6 +30,12 @@ class DomainTests(utils.TestCase, utils.CrudTests): kwargs.setdefault('name', uuid.uuid4().hex) return kwargs + def test_filter_for_default_domain_by_id(self): + ref = self.new_ref(id='default') + super(DomainTests, self).test_list_by_id( + ref=ref, + id=ref['id']) + def test_list_filter_name(self): super(DomainTests, self).test_list(name='adomain123') diff --git a/keystoneclient/tests/unit/v3/test_federation.py b/keystoneclient/tests/unit/v3/test_federation.py index 4cbae8dc5..5782aa648 100644 --- a/keystoneclient/tests/unit/v3/test_federation.py +++ b/keystoneclient/tests/unit/v3/test_federation.py @@ -278,6 +278,16 @@ class ProtocolTests(utils.TestCase, utils.CrudTests): for obj, ref_obj in zip(returned, expected): self.assertEqual(obj.to_dict(), ref_obj) + def test_list_by_id(self): + # The test in the parent class needs to be overridden because it + # assumes globally unique IDs, which is not the case with protocol IDs + # (which are contextualized per identity provider). + ref = self.new_ref() + super(ProtocolTests, self).test_list_by_id( + ref=ref, + identity_provider=ref['identity_provider'], + id=ref['id']) + def test_list_params(self): request_args = self.new_ref() filter_kwargs = {uuid.uuid4().hex: uuid.uuid4().hex} diff --git a/keystoneclient/tests/unit/v3/test_role_assignments.py b/keystoneclient/tests/unit/v3/test_role_assignments.py index 79d2585df..e77bdccce 100644 --- a/keystoneclient/tests/unit/v3/test_role_assignments.py +++ b/keystoneclient/tests/unit/v3/test_role_assignments.py @@ -71,6 +71,15 @@ class RoleAssignmentsTests(utils.TestCase, utils.CrudTests): self.assertEqual(len(ref_list), len(returned_list)) [self.assertIsInstance(r, self.model) for r in returned_list] + def test_list_by_id(self): + # It doesn't make sense to "list role assignments by ID" at all, given + # that they don't have globally unique IDs in the first place. But + # calling RoleAssignmentsManager.list(id=...) should still raise a + # TypeError when given an unexpected keyword argument 'id', so we don't + # actually have to modify the test in the superclass... I just wanted + # to make a note here in case the superclass changes. + super(RoleAssignmentsTests, self).test_list_by_id() + def test_list_params(self): ref_list = self.TEST_USER_PROJECT_LIST self.stub_entity('GET', diff --git a/keystoneclient/tests/unit/v3/utils.py b/keystoneclient/tests/unit/v3/utils.py index 7f2d633d7..b5fb7ce54 100644 --- a/keystoneclient/tests/unit/v3/utils.py +++ b/keystoneclient/tests/unit/v3/utils.py @@ -245,6 +245,20 @@ class CrudTests(object): return expected_path + def test_list_by_id(self, ref=None, **filter_kwargs): + """Test ``entities.list(id=x)`` being rewritten as ``GET /v3/entities/x``. + + This tests an edge case of each manager's list() implementation, to + ensure that it "does the right thing" when users call ``.list()`` + when they should have used ``.get()``. + + """ + if 'id' not in filter_kwargs: + ref = ref or self.new_ref() + filter_kwargs['id'] = ref['id'] + + self.assertRaises(TypeError, self.manager.list, **filter_kwargs) + def test_list(self, ref_list=None, expected_path=None, expected_query=None, **filter_kwargs): ref_list = ref_list or [self.new_ref(), self.new_ref()]