From 892cba1651a8f437607c85a0a3fa3eb52619d05a Mon Sep 17 00:00:00 2001 From: Ian Cordasco Date: Thu, 2 Feb 2017 14:43:37 -0600 Subject: [PATCH] Add support to the client for autopagination This adds pagination support to the underlying client as well as autopagination support on the CLI. This adds --all and --marker to the associated {cells,hosts,projects,regions}-list commands. Depends-On: I11c5c6053e5f0873ee53df5b7dcb9b2ed25a0b64 Change-Id: Ic1f8f01e052ff7870f731d6c8a5d47d58d9dcfe9 --- cratonclient/crud.py | 17 ++-- cratonclient/session.py | 46 ++++++++++ cratonclient/shell/v1/cells_shell.py | 22 ++++- cratonclient/shell/v1/hosts_shell.py | 22 ++++- cratonclient/shell/v1/projects_shell.py | 22 ++++- cratonclient/shell/v1/regions_shell.py | 22 ++++- .../tests/integration/test_cells_shell.py | 14 ++++ cratonclient/tests/integration/test_crud.py | 7 +- .../tests/integration/test_hosts_shell.py | 18 ++++ .../tests/integration/test_projects_shell.py | 18 +++- .../tests/unit/shell/v1/test_cells_shell.py | 40 +++++++++ .../tests/unit/shell/v1/test_hosts_shell.py | 55 ++++++++++++ .../unit/shell/v1/test_projects_shell.py | 59 ++++++++++++- .../tests/unit/shell/v1/test_regions_shell.py | 39 +++++++++ cratonclient/tests/unit/test_crud.py | 11 ++- cratonclient/tests/unit/test_session.py | 84 +++++++++++++++++++ 16 files changed, 460 insertions(+), 36 deletions(-) diff --git a/cratonclient/crud.py b/cratonclient/crud.py index ae7cacd..39b48a5 100644 --- a/cratonclient/crud.py +++ b/cratonclient/crud.py @@ -103,14 +103,19 @@ class CRUDClient(object): return self.resource_class(self, response.json(), loaded=True) def list(self, skip_merge=False, **kwargs): - """List the items from this endpoint.""" + """Generate the items from this endpoint.""" + autopaginate = kwargs.pop('autopaginate', True) self.merge_request_arguments(kwargs, skip_merge) url = self.build_url(path_arguments=kwargs) - response = self.session.get(url, params=kwargs) - return [ - self.resource_class(self, item, loaded=True) - for item in response.json() - ] + response_generator = self.session.paginate( + url, + autopaginate=autopaginate, + items_key=(self.key + 's'), + params=kwargs, + ) + for response, items in response_generator: + for item in items: + yield self.resource_class(self, item, loaded=True) def update(self, item_id=None, skip_merge=True, **kwargs): """Update the item based on the keyword arguments provided.""" diff --git a/cratonclient/session.py b/cratonclient/session.py index 41e2958..d01a79b 100644 --- a/cratonclient/session.py +++ b/cratonclient/session.py @@ -232,3 +232,49 @@ class Session(object): raise exc.error_from(response) return response + + def paginate(self, url, items_key, autopaginate=True, **kwargs): + """Make a GET request to a paginated resource. + + If :param:`autopaginate` is set to ``True``, this will automatically + handle finding and retrieving the next page of items. + + .. code-block:: python + + >>> from cratonclient import session as craton + >>> session = craton.Session( + ... username='demo', + ... token='p@##w0rd', + ... project_id='84363597-721c-4068-9731-8824692b51bb', + ... ) + >>> url = 'https://example.com/v1/hosts' + >>> for response in session.paginate(url, items_key='hosts'): + ... print("Received status {}".format(response.status_code)) + ... print("Received {} items".format(len(items))) + + :param bool autopaginate: + Determines whether or not this method continues requesting items + automatically after the first page. + """ + response = self.get(url, **kwargs) + json_body = response.json() + items = json_body[items_key] + links = json_body['links'] + yield response, items + while autopaginate and len(items) > 0: + url = _find_next_link(links) + if url is None: + break + response = self.get(url) + json_body = response.json() + items = json_body[items_key] + links = json_body['links'] + yield response, items + + +def _find_next_link(links): + for link in links: + if link['rel'] == 'next': + return link['href'] + + return None diff --git a/cratonclient/shell/v1/cells_shell.py b/cratonclient/shell/v1/cells_shell.py index 9a75310..1acec3c 100644 --- a/cratonclient/shell/v1/cells_shell.py +++ b/cratonclient/shell/v1/cells_shell.py @@ -39,10 +39,6 @@ def do_cell_show(cc, args): action='store_true', default=False, help='Show detailed information about the cells.') -@cliutils.arg('--limit', - metavar='', - type=int, - help='Maximum number of cells to return.') @cliutils.arg('--sort-key', metavar='', help='Cell field that will be used for sorting.') @@ -58,6 +54,20 @@ def do_cell_show(cc, args): help='Comma-separated list of fields to display. ' 'Only these fields will be fetched from the server. ' 'Can not be used when "--detail" is specified') +@cliutils.arg('--all', + action='store_true', + default=False, + help='Retrieve and show all cells. This will override ' + 'the provided value for --limit and automatically ' + 'retrieve each page of results.') +@cliutils.arg('--limit', + metavar='', + type=int, + help='Maximum number of cells to return.') +@cliutils.arg('--marker', + metavar='', + default=None, + help='ID of the cell to use to resume listing cells.') def do_cell_list(cc, args): """Print list of cells which are registered with the Craton service.""" params = {} @@ -68,6 +78,8 @@ def do_cell_list(cc, args): 'non-negative limit, got {0}' .format(args.limit)) params['limit'] = args.limit + if args.all is True: + params['limit'] = 100 if args.fields and args.detail: raise exc.CommandError('Cannot specify both --fields and --detail.') @@ -95,6 +107,8 @@ def do_cell_list(cc, args): params['sort_key'] = sort_key params['sort_dir'] = args.sort_dir params['region_id'] = args.region + params['marker'] = args.marker + params['autopaginate'] = args.all listed_cells = cc.cells.list(**params) cliutils.print_list(listed_cells, list(fields)) diff --git a/cratonclient/shell/v1/hosts_shell.py b/cratonclient/shell/v1/hosts_shell.py index d9b9bc6..c6edd89 100644 --- a/cratonclient/shell/v1/hosts_shell.py +++ b/cratonclient/shell/v1/hosts_shell.py @@ -44,10 +44,6 @@ def do_host_show(cc, args): action='store_true', default=False, help='Show detailed information about the hosts.') -@cliutils.arg('--limit', - metavar='', - type=int, - help='Maximum number of hosts to return.') @cliutils.arg('--sort-key', metavar='', help='Host field that will be used for sorting.') @@ -63,6 +59,20 @@ def do_host_show(cc, args): help='Comma-separated list of fields to display. ' 'Only these fields will be fetched from the server. ' 'Can not be used when "--detail" is specified') +@cliutils.arg('--all', + action='store_true', + default=False, + help='Retrieve and show all hosts. This will override ' + 'the provided value for --limit and automatically ' + 'retrieve each page of results.') +@cliutils.arg('--limit', + metavar='', + type=int, + help='Maximum number of hosts to return.') +@cliutils.arg('--marker', + metavar='', + default=None, + help='ID of the cell to use to resume listing hosts.') def do_host_list(cc, args): """Print list of hosts which are registered with the Craton service.""" params = {} @@ -75,6 +85,8 @@ def do_host_list(cc, args): 'non-negative limit, got {0}' .format(args.limit)) params['limit'] = args.limit + if args.all is True: + params['limit'] = 100 if args.fields and args.detail: raise exc.CommandError('Cannot specify both --fields and --detail.') @@ -101,6 +113,8 @@ def do_host_list(cc, args): params['sort_key'] = sort_key params['sort_dir'] = args.sort_dir params['region_id'] = args.region + params['marker'] = args.marker + params['autopaginate'] = args.all host_list = cc.hosts.list(**params) cliutils.print_list(host_list, list(fields)) diff --git a/cratonclient/shell/v1/projects_shell.py b/cratonclient/shell/v1/projects_shell.py index 6bda22e..972128b 100644 --- a/cratonclient/shell/v1/projects_shell.py +++ b/cratonclient/shell/v1/projects_shell.py @@ -36,10 +36,6 @@ def do_project_show(cc, args): action='store_true', default=False, help='Show detailed information about the projects.') -@cliutils.arg('--limit', - metavar='', - type=int, - help='Maximum number of projects to return.') @cliutils.arg('--fields', nargs='+', metavar='', @@ -47,6 +43,20 @@ def do_project_show(cc, args): help='Comma-separated list of fields to display. ' 'Only these fields will be fetched from the server. ' 'Can not be used when "--detail" is specified') +@cliutils.arg('--all', + action='store_true', + default=False, + help='Retrieve and show all projects. This will override ' + 'the provided value for --limit and automatically ' + 'retrieve each page of results.') +@cliutils.arg('--limit', + metavar='', + type=int, + help='Maximum number of projects to return.') +@cliutils.arg('--marker', + metavar='', + default=None, + help='ID of the cell to use to resume listing projects.') def do_project_list(cc, args): """Print list of projects which are registered with the Craton service.""" params = {} @@ -57,6 +67,8 @@ def do_project_list(cc, args): 'non-negative limit, got {0}' .format(args.limit)) params['limit'] = args.limit + if args.all is True: + params['limit'] = 100 if args.fields and args.detail: raise exc.CommandError('Cannot specify both --fields and --detail.') @@ -72,6 +84,8 @@ def do_project_list(cc, args): raise exc.CommandError('Invalid field "{}"'.format(keyerr.args[0])) else: fields = {x: projects.PROJECT_FIELDS[x] for x in default_fields} + params['marker'] = args.marker + params['autopaginate'] = args.all listed_projects = cc.projects.list(**params) cliutils.print_list(listed_projects, list(fields)) diff --git a/cratonclient/shell/v1/regions_shell.py b/cratonclient/shell/v1/regions_shell.py index c112185..c537de3 100644 --- a/cratonclient/shell/v1/regions_shell.py +++ b/cratonclient/shell/v1/regions_shell.py @@ -33,10 +33,6 @@ def do_region_create(cc, args): cliutils.print_dict(data, wrap=72) -@cliutils.arg('--limit', - metavar='', - type=int, - help='Maximum number of regions to return.') @cliutils.arg('--fields', nargs='+', metavar='', @@ -44,6 +40,20 @@ def do_region_create(cc, args): help='Comma-separated list of fields to display. ' 'Only these fields will be fetched from the server. ' 'Can not be used when "--detail" is specified') +@cliutils.arg('--all', + action='store_true', + default=False, + help='Retrieve and show all regions. This will override ' + 'the provided value for --limit and automatically ' + 'retrieve each page of results.') +@cliutils.arg('--limit', + metavar='', + type=int, + help='Maximum number of regions to return.') +@cliutils.arg('--marker', + metavar='', + default=None, + help='ID of the cell to use to resume listing regions.') def do_region_list(cc, args): """List all regions.""" params = {} @@ -54,6 +64,8 @@ def do_region_list(cc, args): 'non-negative limit, got {0}' .format(args.limit)) params['limit'] = args.limit + if args.all is True: + params['limit'] = 100 if args.fields: try: @@ -62,6 +74,8 @@ def do_region_list(cc, args): raise exc.CommandError('Invalid field "{}"'.format(err.args[0])) else: fields = default_fields + params['marker'] = args.marker + params['autopaginate'] = args.all regions_list = cc.regions.list(**params) cliutils.print_list(regions_list, list(fields)) diff --git a/cratonclient/tests/integration/test_cells_shell.py b/cratonclient/tests/integration/test_cells_shell.py index e583840..9bf10f4 100644 --- a/cratonclient/tests/integration/test_cells_shell.py +++ b/cratonclient/tests/integration/test_cells_shell.py @@ -62,6 +62,8 @@ class TestCellsShell(base.ShellTestCase): limit=0, sort_dir='asc', region_id=1, + autopaginate=False, + marker=None, ) @mock.patch('cratonclient.v1.cells.CellManager.list') @@ -75,6 +77,8 @@ class TestCellsShell(base.ShellTestCase): limit=1, sort_dir='asc', region_id=1, + autopaginate=False, + marker=None, ) def test_cell_list_limit_negative_num_failure(self): @@ -94,6 +98,8 @@ class TestCellsShell(base.ShellTestCase): detail=True, region_id=1, sort_dir='asc', + autopaginate=False, + marker=None, ) @mock.patch('cratonclient.v1.cells.CellManager.list') @@ -104,6 +110,8 @@ class TestCellsShell(base.ShellTestCase): mock_list.assert_called_once_with( sort_dir='asc', region_id=1, + autopaginate=False, + marker=None, ) mock_printlist.assert_called_once_with(mock.ANY, list({'id': 'ID', @@ -117,6 +125,8 @@ class TestCellsShell(base.ShellTestCase): sort_key='name', sort_dir='asc', region_id=1, + autopaginate=False, + marker=None, ) def test_cell_list_sort_key_invalid(self): @@ -133,6 +143,8 @@ class TestCellsShell(base.ShellTestCase): sort_key='name', sort_dir='asc', region_id=1, + autopaginate=False, + marker=None, ) @mock.patch('cratonclient.v1.cells.CellManager.list') @@ -143,6 +155,8 @@ class TestCellsShell(base.ShellTestCase): sort_key='name', sort_dir='desc', region_id=1, + autopaginate=False, + marker=None, ) def test_cell_list_sort_dir_invalid_value(self): diff --git a/cratonclient/tests/integration/test_crud.py b/cratonclient/tests/integration/test_crud.py index 3d0c88c..491f5f2 100644 --- a/cratonclient/tests/integration/test_crud.py +++ b/cratonclient/tests/integration/test_crud.py @@ -134,10 +134,13 @@ class TestCrudIntegration(base.TestCase): """Verify the request to list a resource.""" self.session.request.return_value = self.create_response( status_code=200, - json_return_value=[{'name': 'Test', 'id': 1234}], + json_return_value={ + 'test_keys': [{'name': 'Test', 'id': 1234}], + 'links': [], + }, ) - resources = self.client.list(filter_by='some-attribute') + resources = list(self.client.list(filter_by='some-attribute')) self.session.request.assert_called_once_with( method='GET', diff --git a/cratonclient/tests/integration/test_hosts_shell.py b/cratonclient/tests/integration/test_hosts_shell.py index 2106e84..64a9587 100644 --- a/cratonclient/tests/integration/test_hosts_shell.py +++ b/cratonclient/tests/integration/test_hosts_shell.py @@ -66,6 +66,8 @@ class TestHostsShell(base.ShellTestCase): limit=0, sort_dir='asc', region_id=1, + marker=None, + autopaginate=False, ) @mock.patch('cratonclient.v1.hosts.HostManager.list') @@ -79,6 +81,8 @@ class TestHostsShell(base.ShellTestCase): limit=1, sort_dir='asc', region_id=1, + marker=None, + autopaginate=False, ) def test_host_list_limit_negative_num_failure(self): @@ -99,6 +103,8 @@ class TestHostsShell(base.ShellTestCase): cell_id=1, sort_dir='asc', region_id=1, + marker=None, + autopaginate=False, ) mock_list.reset_mock() @@ -110,6 +116,8 @@ class TestHostsShell(base.ShellTestCase): detail=True, sort_dir='asc', region_id=1, + marker=None, + autopaginate=False, ) @mock.patch('cratonclient.v1.hosts.HostManager.list') @@ -120,6 +128,8 @@ class TestHostsShell(base.ShellTestCase): mock_list.assert_called_once_with( sort_dir='asc', region_id=1, + marker=None, + autopaginate=False, ) mock_printlist.assert_called_once_with(mock.ANY, list({'id': 'ID', @@ -133,6 +143,8 @@ class TestHostsShell(base.ShellTestCase): sort_key='cell_id', sort_dir='asc', region_id=1, + marker=None, + autopaginate=False, ) def test_host_list_sort_key_invalid(self): @@ -148,6 +160,8 @@ class TestHostsShell(base.ShellTestCase): mock_list.assert_called_once_with( sort_dir='desc', region_id=1, + marker=None, + autopaginate=False, ) @mock.patch('cratonclient.v1.hosts.HostManager.list') @@ -158,6 +172,8 @@ class TestHostsShell(base.ShellTestCase): sort_key='name', sort_dir='asc', region_id=1, + marker=None, + autopaginate=False, ) @mock.patch('cratonclient.v1.hosts.HostManager.list') @@ -168,6 +184,8 @@ class TestHostsShell(base.ShellTestCase): sort_key='name', sort_dir='desc', region_id=1, + marker=None, + autopaginate=False, ) def test_host_list_sort_dir_invalid_value(self): diff --git a/cratonclient/tests/integration/test_projects_shell.py b/cratonclient/tests/integration/test_projects_shell.py index 0a1334f..0470969 100644 --- a/cratonclient/tests/integration/test_projects_shell.py +++ b/cratonclient/tests/integration/test_projects_shell.py @@ -55,7 +55,9 @@ class TestProjectsShell(base.ShellTestCase): """Verify that --limit 0 prints out all project projects.""" self.shell('project-list --limit 0') mock_list.assert_called_once_with( - limit=0 + limit=0, + marker=None, + autopaginate=False, ) @mock.patch('cratonclient.v1.projects.ProjectManager.list') @@ -66,7 +68,9 @@ class TestProjectsShell(base.ShellTestCase): """ self.shell('project-list --limit 1') mock_list.assert_called_once_with( - limit=1 + limit=1, + marker=None, + autopaginate=False, ) def test_project_list_limit_negative_num_failure(self): @@ -82,14 +86,20 @@ class TestProjectsShell(base.ShellTestCase): def test_project_list_detail_success(self, mock_list): """Verify --detail argument successfully pass detail to Client.""" self.shell('project-list --detail') - mock_list.assert_called_once_with() + mock_list.assert_called_once_with( + marker=None, + autopaginate=False, + ) @mock.patch('cratonclient.v1.projects.ProjectManager.list') @mock.patch('cratonclient.common.cliutils.print_list') def test_project_list_fields_success(self, mock_printlist, mock_list): """Verify --fields argument successfully passed to Client.""" self.shell('project-list --fields id name') - mock_list.assert_called_once_with() + mock_list.assert_called_once_with( + marker=None, + autopaginate=False, + ) mock_printlist.assert_called_once_with(mock.ANY, list({'id': 'ID', 'name': 'Name'})) diff --git a/cratonclient/tests/unit/shell/v1/test_cells_shell.py b/cratonclient/tests/unit/shell/v1/test_cells_shell.py index ee2ba27..4d447f7 100644 --- a/cratonclient/tests/unit/shell/v1/test_cells_shell.py +++ b/cratonclient/tests/unit/shell/v1/test_cells_shell.py @@ -55,6 +55,8 @@ class TestDoCellList(base.TestShellCommandUsingPrintList): kwargs.setdefault('sort_key', None) kwargs.setdefault('sort_dir', 'asc') kwargs.setdefault('fields', []) + kwargs.setdefault('marker', None) + kwargs.setdefault('all', False) return super(TestDoCellList, self).args_for(**kwargs) def test_with_defaults(self): @@ -66,6 +68,8 @@ class TestDoCellList(base.TestShellCommandUsingPrintList): self.craton_client.cells.list.assert_called_once_with( sort_dir='asc', region_id=123, + autopaginate=False, + marker=None, ) self.assertTrue(self.print_list.called) self.assertEqual(['id', 'name'], @@ -88,6 +92,8 @@ class TestDoCellList(base.TestShellCommandUsingPrintList): limit=5, sort_dir='asc', region_id=123, + autopaginate=False, + marker=None, ) self.assertTrue(self.print_list.called) self.assertEqual(['id', 'name'], @@ -103,6 +109,8 @@ class TestDoCellList(base.TestShellCommandUsingPrintList): sort_dir='asc', sort_key='name', region_id=123, + autopaginate=False, + marker=None, ) self.assertTrue(self.print_list.called) self.assertEqual(['id', 'name'], @@ -125,6 +133,8 @@ class TestDoCellList(base.TestShellCommandUsingPrintList): sort_dir='asc', detail=True, region_id=123, + autopaginate=False, + marker=None, ) self.assertEqual(sorted(list(cells.CELL_FIELDS)), sorted(self.print_list.call_args[0][-1])) @@ -148,6 +158,8 @@ class TestDoCellList(base.TestShellCommandUsingPrintList): self.craton_client.cells.list.assert_called_once_with( sort_dir='asc', region_id=123, + autopaginate=False, + marker=None, ) self.assertEqual(['id', 'name', 'note'], sorted(self.print_list.call_args[0][-1])) @@ -159,6 +171,34 @@ class TestDoCellList(base.TestShellCommandUsingPrintList): self.assertRaisesCommandErrorWith(cells_shell.do_cell_list, args) self.assertNothingWasCalled() + def test_autopaginate(self): + """Verify that autopagination works.""" + args = self.args_for(all=True) + + cells_shell.do_cell_list(self.craton_client, args) + + self.craton_client.cells.list.assert_called_once_with( + sort_dir='asc', + region_id=123, + limit=100, + autopaginate=True, + marker=None, + ) + + def test_autopagination_overrides_limit(self): + """Verify that --all overrides --limit.""" + args = self.args_for(all=True, limit=10) + + cells_shell.do_cell_list(self.craton_client, args) + + self.craton_client.cells.list.assert_called_once_with( + sort_dir='asc', + region_id=123, + limit=100, + autopaginate=True, + marker=None, + ) + class TestDoCellCreate(base.TestShellCommandUsingPrintDict): """Unit tests for the cell create command.""" diff --git a/cratonclient/tests/unit/shell/v1/test_hosts_shell.py b/cratonclient/tests/unit/shell/v1/test_hosts_shell.py index 2793d3a..61e3159 100644 --- a/cratonclient/tests/unit/shell/v1/test_hosts_shell.py +++ b/cratonclient/tests/unit/shell/v1/test_hosts_shell.py @@ -51,6 +51,8 @@ class TestDoHostList(base.TestShellCommandUsingPrintList): kwargs.setdefault('sort_key', None) kwargs.setdefault('sort_dir', 'asc') kwargs.setdefault('fields', []) + kwargs.setdefault('marker', None) + kwargs.setdefault('all', False) return super(TestDoHostList, self).args_for(**kwargs) def test_only_required_parameters(self): @@ -62,6 +64,8 @@ class TestDoHostList(base.TestShellCommandUsingPrintList): self.craton_client.hosts.list.assert_called_once_with( sort_dir='asc', region_id=246, + autopaginate=False, + marker=None, ) self.assertSortedPrintListFieldsEqualTo([ 'active', 'cell_id', 'device_type', 'id', 'name' @@ -77,6 +81,8 @@ class TestDoHostList(base.TestShellCommandUsingPrintList): cell_id=789, sort_dir='asc', region_id=246, + autopaginate=False, + marker=None, ) self.assertSortedPrintListFieldsEqualTo([ 'active', 'cell_id', 'device_type', 'id', 'name', @@ -92,6 +98,8 @@ class TestDoHostList(base.TestShellCommandUsingPrintList): detail=True, sort_dir='asc', region_id=246, + autopaginate=False, + marker=None, ) self.assertSortedPrintListFieldsEqualTo([ 'active', @@ -118,6 +126,8 @@ class TestDoHostList(base.TestShellCommandUsingPrintList): limit=20, sort_dir='asc', region_id=246, + autopaginate=False, + marker=None, ) self.assertSortedPrintListFieldsEqualTo([ 'active', 'cell_id', 'device_type', 'id', 'name' @@ -139,6 +149,8 @@ class TestDoHostList(base.TestShellCommandUsingPrintList): self.craton_client.hosts.list.assert_called_once_with( sort_dir='asc', region_id=246, + autopaginate=False, + marker=None, ) self.assertSortedPrintListFieldsEqualTo([ 'cell_id', 'id', 'name', @@ -163,6 +175,8 @@ class TestDoHostList(base.TestShellCommandUsingPrintList): sort_key='ip_address', sort_dir='asc', region_id=246, + autopaginate=False, + marker=None, ) def test_fields_and_detail_raise_command_error(self): @@ -183,6 +197,47 @@ class TestDoHostList(base.TestShellCommandUsingPrintList): ) self.assertNothingWasCalled() + def test_autopagination(self): + """Verify autopagination is controlled by --all.""" + args = self.args_for(all=True) + + hosts_shell.do_host_list(self.craton_client, args) + + self.craton_client.hosts.list.assert_called_once_with( + region_id=246, + sort_dir='asc', + limit=100, + marker=None, + autopaginate=True, + ) + + def test_autopagination_overrides_limit(self): + """Verify --all overrides --limit.""" + args = self.args_for(all=True, limit=30) + + hosts_shell.do_host_list(self.craton_client, args) + + self.craton_client.hosts.list.assert_called_once_with( + region_id=246, + sort_dir='asc', + limit=100, + marker=None, + autopaginate=True, + ) + + def test_marker_pass_through(self): + """Verify we pass our marker through to the client.""" + args = self.args_for(marker=42) + + hosts_shell.do_host_list(self.craton_client, args) + + self.craton_client.hosts.list.assert_called_once_with( + region_id=246, + sort_dir='asc', + marker=42, + autopaginate=False, + ) + class TestDoHostCreate(base.TestShellCommandUsingPrintDict): """Tests for the do_host_create shell command.""" diff --git a/cratonclient/tests/unit/shell/v1/test_projects_shell.py b/cratonclient/tests/unit/shell/v1/test_projects_shell.py index f9728de..8c73057 100644 --- a/cratonclient/tests/unit/shell/v1/test_projects_shell.py +++ b/cratonclient/tests/unit/shell/v1/test_projects_shell.py @@ -55,6 +55,8 @@ class TestDoProjectList(base.TestShellCommandUsingPrintList): kwargs.setdefault('limit', None) kwargs.setdefault('detail', False) kwargs.setdefault('fields', []) + kwargs.setdefault('marker', None) + kwargs.setdefault('all', False) return super(TestDoProjectList, self).args_for(**kwargs) def test_with_defaults(self): @@ -63,7 +65,10 @@ class TestDoProjectList(base.TestShellCommandUsingPrintList): projects_shell.do_project_list(self.craton_client, args) - self.craton_client.projects.list.assert_called_once_with() + self.craton_client.projects.list.assert_called_once_with( + marker=None, + autopaginate=False, + ) self.assertTrue(self.print_list.called) self.assertEqual(['id', 'name'], sorted(self.print_list.call_args[0][-1])) @@ -84,6 +89,8 @@ class TestDoProjectList(base.TestShellCommandUsingPrintList): self.craton_client.projects.list.assert_called_once_with( limit=5, + autopaginate=False, + marker=None, ) self.assertTrue(self.print_list.called) self.assertEqual(['id', 'name'], @@ -95,7 +102,10 @@ class TestDoProjectList(base.TestShellCommandUsingPrintList): projects_shell.do_project_list(self.craton_client, args) - self.craton_client.projects.list.assert_called_once_with() + self.craton_client.projects.list.assert_called_once_with( + marker=None, + autopaginate=False, + ) self.assertEqual(sorted(list(projects.PROJECT_FIELDS)), sorted(self.print_list.call_args[0][-1])) @@ -106,7 +116,9 @@ class TestDoProjectList(base.TestShellCommandUsingPrintList): projects_shell.do_project_list(self.craton_client, args) self.craton_client.projects.list.assert_called_once_with( - name='project_1' + name='project_1', + autopaginate=False, + marker=None, ) self.assertEqual(['id', 'name'], sorted(self.print_list.call_args[0][-1])) @@ -127,7 +139,10 @@ class TestDoProjectList(base.TestShellCommandUsingPrintList): projects_shell.do_project_list(self.craton_client, args) - self.craton_client.projects.list.assert_called_once_with() + self.craton_client.projects.list.assert_called_once_with( + autopaginate=False, + marker=None, + ) self.assertEqual(['id', 'name'], sorted(self.print_list.call_args[0][-1])) @@ -138,6 +153,42 @@ class TestDoProjectList(base.TestShellCommandUsingPrintList): self.assertRaisesCommandErrorWith(projects_shell.do_project_list, args) self.assertNothingWasCalled() + def test_autopagination(self): + """Verify autopagination is controlled by --all.""" + args = self.args_for(all=True) + + projects_shell.do_project_list(self.craton_client, args) + + self.craton_client.projects.list.assert_called_once_with( + limit=100, + autopaginate=True, + marker=None, + ) + + def test_autopagination_overrides_limit(self): + """Verify --all overrides --limit.""" + args = self.args_for(all=True, limit=25) + + projects_shell.do_project_list(self.craton_client, args) + + self.craton_client.projects.list.assert_called_once_with( + limit=100, + autopaginate=True, + marker=None, + ) + + def test_marker_support(self): + """Verify we pass through the marker.""" + project_id = uuid.uuid4().hex + args = self.args_for(marker=project_id) + + projects_shell.do_project_list(self.craton_client, args) + + self.craton_client.projects.list.assert_called_once_with( + autopaginate=False, + marker=project_id, + ) + class TestDoProjectCreate(base.TestShellCommandUsingPrintDict): """Unit tests for the project create command.""" diff --git a/cratonclient/tests/unit/shell/v1/test_regions_shell.py b/cratonclient/tests/unit/shell/v1/test_regions_shell.py index 77f0ad8..e45371e 100644 --- a/cratonclient/tests/unit/shell/v1/test_regions_shell.py +++ b/cratonclient/tests/unit/shell/v1/test_regions_shell.py @@ -210,6 +210,8 @@ class TestDoRegionList(base.TestShellCommandUsingPrintList): kwargs.setdefault('detail', False) kwargs.setdefault('limit', None) kwargs.setdefault('fields', []) + kwargs.setdefault('marker', None) + kwargs.setdefault('all', False) return super(TestDoRegionList, self).args_for(**kwargs) def test_with_defaults(self): @@ -232,6 +234,8 @@ class TestDoRegionList(base.TestShellCommandUsingPrintList): regions_shell.do_region_list(self.craton_client, args) self.craton_client.regions.list.assert_called_once_with( limit=5, + marker=None, + autopaginate=False, ) self.assertTrue(self.print_list.called) self.assertEqual(['id', 'name'], @@ -249,3 +253,38 @@ class TestDoRegionList(base.TestShellCommandUsingPrintList): args = self.args_for(fields=['uuid', 'not-name', 'nate']) self.assertRaisesCommandErrorWith(regions_shell.do_region_list, args) self.assertNothingWasCalled() + + def test_autopagination(self): + """Verify autopagination is controlled by --all.""" + args = self.args_for(all=True) + + regions_shell.do_region_list(self.craton_client, args) + + self.craton_client.regions.list.assert_called_once_with( + limit=100, + marker=None, + autopaginate=True, + ) + + def test_autopagination_overrides_limit(self): + """Verify --all overrides --limit.""" + args = self.args_for(all=True, limit=35) + + regions_shell.do_region_list(self.craton_client, args) + + self.craton_client.regions.list.assert_called_once_with( + limit=100, + marker=None, + autopaginate=True, + ) + + def test_marker_pass_through(self): + """Verify we pass our marker through to the client.""" + args = self.args_for(marker=31) + + regions_shell.do_region_list(self.craton_client, args) + + self.craton_client.regions.list.assert_called_once_with( + marker=31, + autopaginate=False, + ) diff --git a/cratonclient/tests/unit/test_crud.py b/cratonclient/tests/unit/test_crud.py index ec9c31b..9d1dc4f 100644 --- a/cratonclient/tests/unit/test_crud.py +++ b/cratonclient/tests/unit/test_crud.py @@ -172,13 +172,16 @@ class TestCRUDClient(base.TestCase): def test_list_generates_a_get_request(self): """Verify that using our list method will GET from our service.""" - response = self.session.get.return_value = mock.Mock() - response.json.return_value = [{'name': 'fake-name', 'id': 1}] + response = mock.Mock() + items = [{'name': 'fake-name', 'id': 1}] + self.session.paginate.return_value = iter([(response, items)]) - self.client.list(sort='asc') + next(self.client.list(sort='asc')) - self.session.get.assert_called_once_with( + self.session.paginate.assert_called_once_with( 'http://example.com/v1/test', + autopaginate=True, + items_key='test_keys', params={'sort': 'asc'}, ) self.resource_spec.assert_called_once_with( diff --git a/cratonclient/tests/unit/test_session.py b/cratonclient/tests/unit/test_session.py index 6055d7c..3e92cea 100644 --- a/cratonclient/tests/unit/test_session.py +++ b/cratonclient/tests/unit/test_session.py @@ -13,6 +13,7 @@ # under the License. """Session specific unit tests.""" from keystoneauth1 import session as ksa_session +import mock from cratonclient import auth from cratonclient import session @@ -64,6 +65,20 @@ class TestCratonAuth(base.TestCase): class TestSession(base.TestCase): """Unit tests for cratonclient's Session abstraction.""" + @staticmethod + def create_response(items, next_link): + """Create a mock mimicing requests.Response.""" + response = mock.Mock() + response.status_code = 200 + response.json.return_value = { + 'items': items, + 'links': [{ + 'rel': 'next', + 'href': next_link, + }], + } + return response + def test_creates_keystoneauth_session(self): """Verify we default to keystoneauth sessions and semantics.""" craton_session = session.Session(username=TEST_USERNAME_0, @@ -78,3 +93,72 @@ class TestSession(base.TestCase): craton_session = session.Session(session=ksa_session_obj) self.assertIs(ksa_session_obj, craton_session._session) + + def test_paginate_stops_with_first_empty_list(self): + """Verify the behaviour of Session#paginate.""" + response = self.create_response( + [], 'http://example.com/v1/items?limit=30&marker=foo' + ) + mock_session = mock.Mock() + mock_session.request.return_value = response + + craton_session = session.Session(session=mock_session) + paginated_items = list(craton_session.paginate( + url='http://example.com/v1/items', + items_key='items', + autopaginate=True, + )) + + self.assertListEqual([(response, [])], paginated_items) + mock_session.request.assert_called_once_with( + method='GET', + url='http://example.com/v1/items', + endpoint_filter={'service_type': 'fleet_management'}, + ) + + def test_paginate_follows_until_an_empty_list(self): + """Verify that Session#paginate follows links.""" + responses = [ + self.create_response( + [{'id': _id}], + 'http://example.com/v1/items?limit=30&marker={}'.format(_id), + ) for _id in ['foo', 'bar', 'bogus'] + ] + responses.append(self.create_response([], '')) + mock_session = mock.Mock() + mock_session.request.side_effect = responses + + craton_session = session.Session(session=mock_session) + paginated_items = list(craton_session.paginate( + url='http://example.com/v1/items', + items_key='items', + autopaginate=True, + )) + + self.assertEqual(4, len(paginated_items)) + + self.assertListEqual( + mock_session.request.call_args_list, + [ + mock.call( + method='GET', + url='http://example.com/v1/items', + endpoint_filter={'service_type': 'fleet_management'}, + ), + mock.call( + method='GET', + url='http://example.com/v1/items?limit=30&marker=foo', + endpoint_filter={'service_type': 'fleet_management'}, + ), + mock.call( + method='GET', + url='http://example.com/v1/items?limit=30&marker=bar', + endpoint_filter={'service_type': 'fleet_management'}, + ), + mock.call( + method='GET', + url='http://example.com/v1/items?limit=30&marker=bogus', + endpoint_filter={'service_type': 'fleet_management'}, + ), + ], + )