openstack/horizon
Code Issues Proposed changes

Add "prev" link to instance page list pagination

Browse Source 
Currently there is no link to previous page at paginated instances
table. This patch resolves that issue by re-using the pagination
code for flavors.

It also supports Ying Zuo's scenario:
After I set only 1 item per page and deleted the instance on the first page,
the expected behavior is showing the next instance in the table after one is
deleted.

xxxIndexView uses PagedTableMixin's _get_marker() from now instead of GET()-
ing the markers

Closes-Bug: #1274427
Co-Authored-By: Dmitry Ratushnyy <dratushn@cisco.com>
Co-Authored-By: Akihiro Motoki <amotoki@gmail.com>
Change-Id: Id8eaae6bf1b5d6f42291291655e14b8715c08bc8
Signed-off-by: Ferenc Cserepkei <ferenc.cserepkei@ericsson.com>
This commit is contained in:
Ferenc Cserepkei 2017-08-25 19:53:42 +02:00 committed by Akihiro Motoki
parent 87e2cf67fa
commit 4676694179
7 changed files with 494 additions and 224 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

92
openstack_dashboard/api/nova.py
View File

@ -377,8 +377,7 @@ def flavor_list(request, is_public=True, get_extras=False):
@profiler.trace
def update_pagination(entities, page_size, marker, sort_dir, sort_key,
reversed_order):
def update_pagination(entities, page_size, marker, reversed_order=False):
has_more_data = has_prev_data = False
if len(entities) > page_size:
has_more_data = True
@ -394,9 +393,7 @@ def update_pagination(entities, page_size, marker, sort_dir, sort_key,
# restore the original ordering here
if reversed_order:
entities = sorted(entities, key=lambda entity:
(getattr(entity, sort_key) or '').lower(),
reverse=(sort_dir == 'asc'))
entities.reverse()
return entities, has_more_data, has_prev_data
@ -420,7 +417,7 @@ def flavor_list_paged(request, is_public=True, get_extras=False, marker=None,
sort_key=sort_key,
sort_dir=sort_dir)
flavors, has_more_data, has_prev_data = update_pagination(
flavors, page_size, marker, sort_dir, sort_key, reversed_order)
flavors, page_size, marker, reversed_order)
else:
flavors = novaclient(request).flavors.list(is_public=is_public)
@ -551,6 +548,14 @@ def server_create(request, name, image, flavor, key_name, user_data,
@profiler.trace
def server_delete(request, instance_id):
novaclient(request).servers.delete(instance_id)
# Session is available and consistent for the current view
# among Horizon django servers even in load-balancing setup,
# so only the view listing the servers will recognize it as
# own DeleteInstance action performed. Note that dict is passed
# by reference in python. Quote from django's developer manual:
# " You can read it and write to request.session at any point
# in your view. You can edit it multiple times."
request.session['server_deleted'] = instance_id
def get_novaclient_with_locked_status(request):
@ -570,33 +575,66 @@ def server_get(request, instance_id):
@profiler.trace
def server_list(request, search_opts=None, detailed=True):
def server_list_paged(request,
search_opts=None,
detailed=True,
sort_dir="desc"):
has_more_data = False
has_prev_data = False
nova_client = get_novaclient_with_locked_status(request)
page_size = utils.get_page_size(request)
paginate = False
if search_opts is None:
search_opts = {}
elif 'paginate' in search_opts:
paginate = search_opts.pop('paginate')
if paginate:
search_opts['limit'] = page_size + 1
search_opts = {} if search_opts is None else search_opts
marker = search_opts.get('marker', None)
all_tenants = search_opts.get('all_tenants', False)
if all_tenants:
search_opts['all_tenants'] = True
else:
if not search_opts.get('all_tenants', False):
search_opts['project_id'] = request.user.tenant_id
servers = [Server(s, request)
for s in nova_client.servers.list(detailed, search_opts)]
if search_opts.pop('paginate', False):
reversed_order = sort_dir is "asc"
LOG.debug("Notify received on deleted server: %r",
('server_deleted' in request.session))
deleted = request.session.pop('server_deleted',
None)
view_marker = 'possibly_deleted' if deleted and marker else 'ok'
search_opts['marker'] = deleted if deleted else marker
search_opts['limit'] = page_size + 1
search_opts['sort_dir'] = sort_dir
servers = [Server(s, request)
for s in nova_client.servers.list(detailed, search_opts)]
if view_marker == 'possibly_deleted':
if len(servers) == 0:
view_marker = 'head_deleted'
search_opts['sort_dir'] = 'desc'
reversed_order = False
servers = [Server(s, request)
for s in nova_client.servers.list(detailed,
search_opts)]
if len(servers) == 0:
view_marker = 'tail_deleted'
search_opts['sort_dir'] = 'asc'
reversed_order = True
servers = [Server(s, request)
for s in nova_client.servers.list(detailed,
search_opts)]
(servers, has_more_data, has_prev_data) = update_pagination(
servers, page_size, marker, reversed_order)
has_prev_data = (False
if view_marker == 'head_deleted'
else has_prev_data)
has_more_data = (False
if view_marker == 'tail_deleted'
else has_more_data)
else:
servers = [Server(s, request)
for s in nova_client.servers.list(detailed, search_opts)]
return (servers, has_more_data, has_prev_data)
has_more_data = False
if paginate and len(servers) > page_size:
servers.pop(-1)
has_more_data = True
elif paginate and len(servers) == getattr(settings, 'API_RESULT_LIMIT',
1000):
has_more_data = True
@profiler.trace
def server_list(request, search_opts=None, detailed=True):
(servers, has_more_data, _) = server_list_paged(request,
search_opts,
detailed)
return (servers, has_more_data)

184
openstack_dashboard/dashboards/admin/instances/tests.py
View File

@ -13,10 +13,13 @@
# under the License.
from collections import OrderedDict
import uuid
import mock
from django.conf import settings
from django.test import override_settings
from django.urls import reverse
from openstack_dashboard import api
@ -29,7 +32,7 @@ INDEX_TEMPLATE = 'horizon/common/_data_table_view.html'
class InstanceViewTest(test.BaseAdminViewTests):
@test.create_mocks({
api.nova: ['flavor_list', 'server_list', 'extension_supported'],
api.nova: ['flavor_list', 'server_list_paged', 'extension_supported'],
api.keystone: ['tenant_list'],
api.glance: ['image_list_detailed_by_ids'],
})
@ -42,7 +45,7 @@ class InstanceViewTest(test.BaseAdminViewTests):
self.mock_tenant_list.return_value = [self.tenants.list(), False]
self.mock_image_list_detailed_by_ids.return_value = self.images.list()
self.mock_flavor_list.return_value = self.flavors.list()
self.mock_server_list.return_value = [servers, False]
self.mock_server_list_paged.return_value = [servers, False, False]
res = self.client.get(INDEX_URL)
self.assertTemplateUsed(res, INDEX_TEMPLATE)
@ -59,11 +62,13 @@ class InstanceViewTest(test.BaseAdminViewTests):
test.IsHttpRequest(), instances_img_ids)
self.mock_flavor_list.assert_called_once_with(test.IsHttpRequest())
search_opts = {'marker': None, 'paginate': True, 'all_tenants': True}
self.mock_server_list.assert_called_once_with(
test.IsHttpRequest(), search_opts=search_opts)
self.mock_server_list_paged.assert_called_once_with(
test.IsHttpRequest(),
sort_dir='desc',
search_opts=search_opts)
@test.create_mocks({
api.nova: ['flavor_list', 'flavor_get', 'server_list',
api.nova: ['flavor_list', 'flavor_get', 'server_list_paged',
'extension_supported'],
api.keystone: ['tenant_list'],
api.glance: ['image_list_detailed_by_ids'],
@ -74,7 +79,7 @@ class InstanceViewTest(test.BaseAdminViewTests):
instances_img_ids = [instance.image.get('id') for instance in
servers if hasattr(instance, 'image')]
full_flavors = OrderedDict([(f.id, f) for f in flavors])
self.mock_server_list.return_value = [servers, False]
self.mock_server_list_paged.return_value = [servers, False, False]
self.mock_extension_supported.return_value = True
self.mock_flavor_list.side_effect = self.exceptions.nova
self.mock_tenant_list.return_value = [self.tenants.list(), False]
@ -92,8 +97,10 @@ class InstanceViewTest(test.BaseAdminViewTests):
self.assertItemsEqual(instances, servers)
search_opts = {'marker': None, 'paginate': True, 'all_tenants': True}
self.mock_server_list.assert_called_once_with(
test.IsHttpRequest(), search_opts=search_opts)
self.mock_server_list_paged.assert_called_once_with(
test.IsHttpRequest(),
sort_dir='desc',
search_opts=search_opts)
self.mock_extension_supported.assert_has_calls([
mock.call('AdminActions', test.IsHttpRequest()),
mock.call('AdminActions', test.IsHttpRequest()),
@ -108,7 +115,7 @@ class InstanceViewTest(test.BaseAdminViewTests):
test.IsHttpRequest(), instances_img_ids)
@test.create_mocks({
api.nova: ['flavor_list', 'flavor_get', 'server_list',
api.nova: ['flavor_list', 'flavor_get', 'server_list_paged',
'extension_supported'],
api.keystone: ['tenant_list'],
api.glance: ['image_list_detailed_by_ids'],
@ -124,7 +131,7 @@ class InstanceViewTest(test.BaseAdminViewTests):
self.mock_image_list_detailed_by_ids.return_value = self.images.list()
self.mock_flavor_list.return_value = self.flavors.list()
self.mock_server_list.return_value = [servers, False]
self.mock_server_list_paged.return_value = [servers, False, False]
self.mock_extension_supported.return_value = True
self.mock_tenant_list.return_value = [self.tenants.list(), False]
self.mock_flavor_get.side_effect = self.exceptions.nova
@ -142,8 +149,10 @@ class InstanceViewTest(test.BaseAdminViewTests):
test.IsHttpRequest(), instances_img_ids)
self.mock_flavor_list.assert_called_once_with(test.IsHttpRequest())
search_opts = {'marker': None, 'paginate': True, 'all_tenants': True}
self.mock_server_list.assert_called_once_with(
test.IsHttpRequest(), search_opts=search_opts)
self.mock_server_list_paged.assert_called_once_with(
test.IsHttpRequest(),
sort_dir='desc',
search_opts=search_opts)
self.mock_extension_supported.assert_has_calls([
mock.call('AdminActions', test.IsHttpRequest()),
mock.call('AdminActions', test.IsHttpRequest()),
@ -155,12 +164,12 @@ class InstanceViewTest(test.BaseAdminViewTests):
self.assertEqual(len(servers), self.mock_flavor_get.call_count)
@test.create_mocks({
api.nova: ['server_list', 'flavor_list'],
api.nova: ['server_list_paged', 'flavor_list'],
api.keystone: ['tenant_list'],
api.glance: ['image_list_detailed_by_ids'],
})
def test_index_server_list_exception(self):
self.mock_server_list.side_effect = self.exceptions.nova
self.mock_server_list_paged.side_effect = self.exceptions.nova
self.mock_flavor_list.return_value = self.flavors.list()
self.mock_tenant_list.return_value = [self.tenants.list(), False]
self.mock_image_list_detailed_by_ids.return_value = self.images.list()
@ -170,8 +179,9 @@ class InstanceViewTest(test.BaseAdminViewTests):
self.assertEqual(len(res.context['instances_table'].data), 0)
search_opts = {'marker': None, 'paginate': True, 'all_tenants': True}
self.mock_server_list.assert_called_once_with(
self.mock_server_list_paged.assert_called_once_with(
test.IsHttpRequest(),
sort_dir='desc',
search_opts=search_opts)
self.mock_tenant_list.assert_called_once_with(test.IsHttpRequest())
self.mock_image_list_detailed_by_ids.assert_called_once_with(
@ -223,7 +233,7 @@ class InstanceViewTest(test.BaseAdminViewTests):
test.IsHttpRequest(), [server])
@test.create_mocks({
api.nova: ['flavor_list', 'server_list', 'extension_supported'],
api.nova: ['flavor_list', 'server_list_paged', 'extension_supported'],
api.keystone: ['tenant_list'],
api.glance: ['image_list_detailed_by_ids'],
})
@ -234,9 +244,9 @@ class InstanceViewTest(test.BaseAdminViewTests):
self.mock_tenant_list.return_value = [self.tenants.list(), False]
self.mock_image_list_detailed_by_ids.return_value = self.images.list()
self.mock_flavor_list.return_value = self.flavors.list()
self.mock_server_list.return_value = [self.servers.list(), False]
self.mock_server_list_paged.return_value = [
self.servers.list(), False, False]
self.mock_extension_supported.return_value = True
res = self.client.get(INDEX_URL)
self.assertContains(res, "instances__migrate")
self.assertNotContains(res, "instances__confirm")
@ -247,8 +257,10 @@ class InstanceViewTest(test.BaseAdminViewTests):
test.IsHttpRequest(), instances_img_ids)
self.mock_flavor_list.assert_called_once_with(test.IsHttpRequest())
search_opts = {'marker': None, 'paginate': True, 'all_tenants': True}
self.mock_server_list.assert_called_once_with(
test.IsHttpRequest(), search_opts=search_opts)
self.mock_server_list_paged.assert_called_once_with(
test.IsHttpRequest(),
sort_dir='desc',
search_opts=search_opts)
self.mock_extension_supported.assert_has_calls([
mock.call('AdminActions', test.IsHttpRequest()),
mock.call('AdminActions', test.IsHttpRequest()),
@ -256,7 +268,7 @@ class InstanceViewTest(test.BaseAdminViewTests):
self.assertEqual(12, self.mock_extension_supported.call_count)
@test.create_mocks({
api.nova: ['flavor_list', 'server_list', 'extension_supported'],
api.nova: ['flavor_list', 'server_list_paged', 'extension_supported'],
api.keystone: ['tenant_list'],
api.glance: ['image_list_detailed_by_ids'],
})
@ -272,7 +284,7 @@ class InstanceViewTest(test.BaseAdminViewTests):
self.mock_image_list_detailed_by_ids.return_value = self.images.list()
self.mock_flavor_list.return_value = self.flavors.list()
self.mock_extension_supported.return_value = True
self.mock_server_list.return_value = [servers, False]
self.mock_server_list_paged.return_value = [servers, False, False]
res = self.client.get(INDEX_URL)
self.assertContains(res, "instances__confirm")
@ -289,8 +301,10 @@ class InstanceViewTest(test.BaseAdminViewTests):
mock.call('Shelve', test.IsHttpRequest())] * 4)
self.assertEqual(12, self.mock_extension_supported.call_count)
search_opts = {'marker': None, 'paginate': True, 'all_tenants': True}
self.mock_server_list.assert_called_once_with(
test.IsHttpRequest(), search_opts=search_opts)
self.mock_server_list_paged.assert_called_once_with(
test.IsHttpRequest(),
sort_dir='desc',
search_opts=search_opts)
@test.create_mocks({api.nova: ['service_list',
'server_get']})
@ -474,3 +488,125 @@ class InstanceViewTest(test.BaseAdminViewTests):
self.assertTemplateUsed(res, INDEX_TEMPLATE)
instances = res.context['table'].data
self.assertItemsEqual(instances, [])
@test.create_mocks({
api.nova: ['flavor_list',
'flavor_get',
'server_list_paged',
'extension_supported'],
api.keystone: ['tenant_list'],
api.glance: ['image_list_detailed_by_ids'],
})
def _test_servers_paginate_do(self,
marker,
servers,
has_more,
has_prev):
flavors = self.flavors.list()
tenants = self.tenants.list()
images = self.images.list()
# UUID indices are unique and are guaranteed being deterministic.
for i, server in enumerate(servers):
server.flavor['id'] = str(uuid.UUID(int=i))
self.mock_server_list_paged.return_value = [
servers, has_more, has_prev]
self.mock_extension_supported.return_value = True
self.mock_flavor_list.return_value = flavors
self.mock_image_list_detailed_by_ids.return_value = images
self.mock_tenant_list.return_value = [tenants, False]
self.mock_flavor_get.side_effect = self.exceptions.nova
if marker:
url = "?".join([INDEX_URL, "marker={}".format(marker)])
else:
url = INDEX_URL
res = self.client.get(url)
self.assertTemplateUsed(res, INDEX_TEMPLATE)
self.assertEqual(res.status_code, 200)
self.mock_extension_supported.assert_has_calls([
mock.call('AdminActions', test.IsHttpRequest()),
mock.call('AdminActions', test.IsHttpRequest()),
mock.call('Shelve', test.IsHttpRequest())])
self.assertEqual(3, self.mock_extension_supported.call_count)
self.mock_tenant_list.assert_called_once_with(test.IsHttpRequest())
self.mock_image_list_detailed_by_ids.assert_called_once_with(
test.IsHttpRequest(),
[server.image.id for server in servers])
self.mock_flavor_list.assert_called_once_with(test.IsHttpRequest())
search_opts = {'marker': marker, 'paginate': True, 'all_tenants': True}
self.mock_server_list_paged.assert_called_once_with(
test.IsHttpRequest(),
sort_dir='desc',
search_opts=search_opts)
self.mock_flavor_get.assert_has_calls(
[mock.call(test.IsHttpRequest(), s.flavor['id']) for s in servers])
self.assertEqual(len(servers), self.mock_flavor_get.call_count)
return res
@override_settings(API_RESULT_PAGE_SIZE=1)
def test_severs_index_paginated(self):
size = settings.API_RESULT_PAGE_SIZE
mox_servers = self.servers.list()
# get first page
expected_servers = mox_servers[:size]
res = self._test_servers_paginate_do(
marker=None,
servers=expected_servers,
has_more=True,
has_prev=False)
servers = res.context['table'].data
self.assertItemsEqual(servers, expected_servers)
# get second page
expected_servers = mox_servers[size:2 * size]
marker = expected_servers[0].id
res = self._test_servers_paginate_do(
marker=marker,
servers=expected_servers,
has_more=True,
has_prev=True)
servers = res.context['table'].data
self.assertItemsEqual(servers, expected_servers)
# get last page
expected_servers = mox_servers[-size:]
marker = expected_servers[0].id
res = self._test_servers_paginate_do(
marker=marker,
servers=expected_servers,
has_more=False,
has_prev=True)
servers = res.context['table'].data
self.assertItemsEqual(servers, expected_servers)
@override_settings(API_RESULT_PAGE_SIZE=1)
def test_servers_index_paginated_prev(self):
size = settings.API_RESULT_PAGE_SIZE
mox_servers = self.servers.list()
# prev from some page
expected_servers = mox_servers[size:2 * size]
marker = mox_servers[0].id
res = self._test_servers_paginate_do(
marker=marker,
servers=expected_servers,
has_more=False,
has_prev=True)
servers = res.context['table'].data
self.assertItemsEqual(servers, expected_servers)
# back to first page
expected_servers = mox_servers[:size]
marker = mox_servers[0].id
res = self._test_servers_paginate_do(
marker=marker,
servers=expected_servers,
has_more=True,
has_prev=False)
servers = res.context['table'].data
self.assertItemsEqual(servers, expected_servers)

19
openstack_dashboard/dashboards/admin/instances/views.py
View File

@ -70,10 +70,13 @@ class AdminUpdateView(views.UpdateView):
success_url = reverse_lazy("horizon:admin:instances:index")
class AdminIndexView(tables.DataTableView):
class AdminIndexView(tables.PagedTableMixin, tables.DataTableView):
table_class = project_tables.AdminInstancesTable
page_title = _("Instances")
def has_prev_data(self, table):
return getattr(self, "_prev", False)
def has_more_data(self, table):
return self._more
@ -115,21 +118,21 @@ class AdminIndexView(tables.DataTableView):
exceptions.handle(self.request, msg)
return {}
def _get_instances(self, search_opts):
def _get_instances(self, search_opts, sort_dir):
try:
instances, self._more = api.nova.server_list(
instances, self._more, self._prev = api.nova.server_list_paged(
self.request,
search_opts=search_opts)
search_opts=search_opts,
sort_dir=sort_dir)
except Exception:
self._more = False
self._more = self._prev = False
instances = []
exceptions.handle(self.request,
_('Unable to retrieve instance list.'))
return instances
def get_data(self):
marker = self.request.GET.get(
project_tables.AdminInstancesTable._meta.pagination_param, None)
marker, sort_dir = self._get_marker()
default_search_opts = {'marker': marker,
'paginate': True,
'all_tenants': True}
@ -148,7 +151,7 @@ class AdminIndexView(tables.DataTableView):
self._needs_filter_first = False
instances = self._get_instances(search_opts)
instances = self._get_instances(search_opts, sort_dir)
results = futurist_utils.call_functions_parallel(
(self._get_images, [tuple(instances)]),
self._get_flavors,

396
openstack_dashboard/dashboards/project/instances/tests.py
View File

File diff suppressed because it is too large Load Diff

19
openstack_dashboard/dashboards/project/instances/views.py
View File

@ -59,10 +59,13 @@ from openstack_dashboard.views import get_url_with_pagination
LOG = logging.getLogger(__name__)
class IndexView(tables.DataTableView):
class IndexView(tables.PagedTableMixin, tables.DataTableView):
table_class = project_tables.InstancesTable
page_title = _("Instances")
def has_prev_data(self, table):
return getattr(self, "_prev", False)
def has_more_data(self, table):
return self._more
@ -85,13 +88,14 @@ class IndexView(tables.DataTableView):
exceptions.handle(self.request, ignore=True)
return {}
def _get_instances(self, search_opts):
def _get_instances(self, search_opts, sort_dir):
try:
instances, self._more = api.nova.server_list(
instances, self._more, self._prev = api.nova.server_list_paged(
self.request,
search_opts=search_opts)
search_opts=search_opts,
sort_dir=sort_dir)
except Exception:
self._more = False
self._more = self._prev = False
instances = []
exceptions.handle(self.request,
_('Unable to retrieve instances.'))
@ -122,8 +126,7 @@ class IndexView(tables.DataTableView):
return instances
def get_data(self):
marker = self.request.GET.get(
project_tables.InstancesTable._meta.pagination_param, None)
marker, sort_dir = self._get_marker()
search_opts = self.get_filters({'marker': marker, 'paginate': True})
image_dict, flavor_dict = futurist_utils.call_functions_parallel(
@ -137,7 +140,7 @@ class IndexView(tables.DataTableView):
self._more = False
return []
instances = self._get_instances(search_opts)
instances = self._get_instances(search_opts, sort_dir)
# Loop through instances to get flavor info.
for instance in instances:

2
openstack_dashboard/test/integration_tests/tests/test_instances.py
View File

@ -73,7 +73,7 @@ class TestInstances(helpers.TestCase):
first_page_definition = {'Next': True, 'Prev': False,
'Count': items_per_page,
'Names': [instance_list[1]]}
second_page_definition = {'Next': False, 'Prev': False,
second_page_definition = {'Next': False, 'Prev': True,
'Count': items_per_page,
'Names': [instance_list[0]]}
settings_page = self.home_pg.go_to_settings_usersettingspage()

6
openstack_dashboard/test/unit/api/test_nova.py
View File

@ -204,7 +204,8 @@ class ComputeApiTests(test.APIMockTestCase):
True,
{'all_tenants': True,
'marker': None,
'limit': page_size + 1})
'limit': page_size + 1,
'sort_dir': 'desc'})
@override_settings(API_RESULT_PAGE_SIZE=1)
@mock.patch.object(api.nova, 'novaclient')
@ -229,7 +230,8 @@ class ComputeApiTests(test.APIMockTestCase):
True,
{'all_tenants': True,
'marker': None,
'limit': page_size + 1})
'limit': page_size + 1,
'sort_dir': 'desc'})
@mock.patch.object(api.nova, 'novaclient')
def test_usage_get(self, mock_novaclient):