From 09b903ed5e3306cf200328f41ce6df371923d03d Mon Sep 17 00:00:00 2001 From: Mark Washenberger Date: Wed, 19 Sep 2012 18:41:03 +0000 Subject: Fix marker pagination for /servers Fixes bug 1053028. Recent changes to pagination required passing in full sqlalchemy objects into paginate_query. However, calls to paginate_query were passing in strings. Change-Id: Ib1396a78a12aef1a9cdc31f3624af30f19da44b5 --- nova/api/openstack/compute/servers.py | 11 ++++++++--- nova/db/sqlalchemy/api.py | 5 +++++ nova/exception.py | 4 ++++ nova/tests/api/openstack/fakes.py | 2 +- nova/tests/test_db_api.py | 11 ++++++++--- 5 files changed, 26 insertions(+), 7 deletions(-) diff --git a/nova/api/openstack/compute/servers.py b/nova/api/openstack/compute/servers.py index 18161b830..53fb6517d 100644 --- a/nova/api/openstack/compute/servers.py +++ b/nova/api/openstack/compute/servers.py @@ -496,9 +496,14 @@ class Controller(wsgi.Controller): search_opts['user_id'] = context.user_id limit, marker = common.get_limit_and_marker(req) - instance_list = self.compute_api.get_all(context, - search_opts=search_opts, - limit=limit, marker=marker) + try: + instance_list = self.compute_api.get_all(context, + search_opts=search_opts, + limit=limit, + marker=marker) + except exception.MarkerNotFound as e: + msg = _('marker [%s] not found') % marker + raise webob.exc.HTTPBadRequest(explanation=msg) if is_detail: self._add_instance_faults(context, instance_list) diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index ad013b877..f87899df7 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -1596,6 +1596,11 @@ def instance_get_all_by_filters(context, filters, sort_key, sort_dir, query_prefix = regex_filter(query_prefix, models.Instance, filters) # paginate query + if marker is not None: + try: + marker = instance_get_by_uuid(context, marker, session=session) + except exception.InstanceNotFound as e: + raise exception.MarkerNotFound(marker) query_prefix = paginate_query(query_prefix, models.Instance, limit, [sort_key, 'created_at', 'id'], marker=marker, diff --git a/nova/exception.py b/nova/exception.py index c9f339e6c..4de43c366 100644 --- a/nova/exception.py +++ b/nova/exception.py @@ -1067,6 +1067,10 @@ class InstanceNotFound(NotFound): message = _("Instance %(instance_id)s could not be found.") +class MarkerNotFound(NotFound): + message = _("Marker %(marker)s could not be found.") + + class InvalidInstanceIDMalformed(Invalid): message = _("Invalid id: %(val)s (expecting \"i-...\").") diff --git a/nova/tests/api/openstack/fakes.py b/nova/tests/api/openstack/fakes.py index 18407b6a7..dc9ccc5f3 100644 --- a/nova/tests/api/openstack/fakes.py +++ b/nova/tests/api/openstack/fakes.py @@ -402,7 +402,7 @@ def fake_instance_get_all_by_filters(num_servers=5, **kwargs): found_marker = True servers_list = [] if not marker is None and not found_marker: - raise webob.exc.HTTPBadRequest + raise exc.MarkerNotFound(marker) if not limit is None: servers_list = servers_list[:limit] return servers_list diff --git a/nova/tests/test_db_api.py b/nova/tests/test_db_api.py index fbc5908df..7a295b4e1 100644 --- a/nova/tests/test_db_api.py +++ b/nova/tests/test_db_api.py @@ -126,19 +126,24 @@ class DbApiTestCase(test.TestCase): result = db.instance_get_all_by_filters(self.context, {'display_name': '%test%'}, sort_dir="asc", - marker=test1) + marker=test1['uuid']) self.assertEqual(2, len(result)) result = db.instance_get_all_by_filters(self.context, {'display_name': '%test%'}, sort_dir="asc", - marker=test2) + marker=test2['uuid']) self.assertEqual(1, len(result)) result = db.instance_get_all_by_filters(self.context, {'display_name': '%test%'}, sort_dir="asc", - marker=test3) + marker=test3['uuid']) self.assertEqual(0, len(result)) + self.assertRaises(exception.MarkerNotFound, + db.instance_get_all_by_filters, + self.context, {'display_name': '%test%'}, + marker=str(utils.gen_uuid())) + def test_migration_get_unconfirmed_by_dest_compute(self): ctxt = context.get_admin_context() -- cgit