From d367d674e60510a6671a416bb530edb07c992bd7 Mon Sep 17 00:00:00 2001 From: Robert Myers Date: Thu, 5 Dec 2013 14:02:18 -0600 Subject: [PATCH] Paginate backup list api * Order list by updated at to show latest first * Add pagination support for backup list and instance backup list * Add tests for pagination and ordering Implements: blueprint paginate-backup-list Change-Id: Ie98bd2bc0500dfe4bbe0bb8b420ad80ad7f5a5df --- trove/backup/models.py | 40 ++++- trove/backup/service.py | 12 +- trove/common/cfg.py | 3 +- trove/instance/service.py | 9 +- .../unittests/backup/test_backup_models.py | 165 +++++++++++++++--- 5 files changed, 193 insertions(+), 36 deletions(-) diff --git a/trove/backup/models.py b/trove/backup/models.py index 74040fcae6..b7dfa6a6cc 100644 --- a/trove/backup/models.py +++ b/trove/backup/models.py @@ -14,11 +14,13 @@ """Model classes that form the core of snapshots functionality.""" +from sqlalchemy import desc +from swiftclient.client import ClientException + from trove.common import cfg from trove.common import exception from trove.db.models import DatabaseModelBase from trove.openstack.common import log as logging -from swiftclient.client import ClientException from trove.taskmanager import api from trove.common.remote import create_swift_client from trove.common import utils @@ -123,6 +125,26 @@ class Backup(object): except exception.NotFound: raise exception.NotFound(uuid=backup_id) + @classmethod + def _paginate(cls, context, query): + """Paginate the results of the base query. + We use limit/offset as the results need to be ordered by date + and not the primary key. + """ + marker = int(context.marker or 0) + limit = int(context.limit or CONF.backups_page_size) + # order by 'updated DESC' to show the most recent backups first + query = query.order_by(desc(DBBackup.updated)) + # Apply limit/offset + query = query.limit(limit) + query = query.offset(marker) + # check if we need to send a marker for the next page + if query.count() < limit: + marker = None + else: + marker += limit + return query.all(), marker + @classmethod def list(cls, context): """ @@ -131,21 +153,23 @@ class Backup(object): :param context: tenant_id included :return: """ - db_info = DBBackup.find_all(tenant_id=context.tenant, - deleted=False) - return db_info + query = DBBackup.query() + query = query.filter_by(tenant_id=context.tenant, + deleted=False) + return cls._paginate(context, query) @classmethod - def list_for_instance(cls, instance_id): + def list_for_instance(cls, context, instance_id): """ list all live Backups associated with given instance :param cls: :param instance_id: :return: """ - db_info = DBBackup.find_all(instance_id=instance_id, - deleted=False) - return db_info + query = DBBackup.query() + query = query.filter_by(instance_id=instance_id, + deleted=False) + return cls._paginate(context, query) @classmethod def fail_for_instance(cls, instance_id): diff --git a/trove/backup/service.py b/trove/backup/service.py index f26e3293b0..b6ea1b2d7d 100644 --- a/trove/backup/service.py +++ b/trove/backup/service.py @@ -15,13 +15,14 @@ # License for the specific language governing permissions and limitations # under the License. -from trove.common import wsgi from trove.backup import views from trove.backup.models import Backup +from trove.common import apischema from trove.common import cfg +from trove.common import pagination +from trove.common import wsgi from trove.openstack.common import log as logging from trove.openstack.common.gettextutils import _ -import trove.common.apischema as apischema CONF = cfg.CONF LOG = logging.getLogger(__name__) @@ -39,8 +40,11 @@ class BackupController(wsgi.Controller): """ LOG.debug("Listing Backups for tenant '%s'" % tenant_id) context = req.environ[wsgi.CONTEXT_KEY] - backups = Backup.list(context) - return wsgi.Result(views.BackupViews(backups).data(), 200) + backups, marker = Backup.list(context) + view = views.BackupViews(backups) + paged = pagination.SimplePaginatedDataView(req.url, 'backups', view, + marker) + return wsgi.Result(paged.data(), 200) def show(self, req, tenant_id, id): """Return a single backup.""" diff --git a/trove/common/cfg.py b/trove/common/cfg.py index d657b2e59f..6b2958b8c6 100644 --- a/trove/common/cfg.py +++ b/trove/common/cfg.py @@ -82,6 +82,7 @@ common_opts = [ cfg.IntOpt('users_page_size', default=20), cfg.IntOpt('databases_page_size', default=20), cfg.IntOpt('instances_page_size', default=20), + cfg.IntOpt('backups_page_size', default=20), cfg.ListOpt('ignore_users', default=['os_admin', 'root']), cfg.ListOpt('ignore_dbs', default=['lost+found', 'mysql', @@ -102,7 +103,7 @@ common_opts = [ help='default maximum volume size for an instance'), cfg.IntOpt('max_volumes_per_user', default=20, help='default maximum for total volume used by a tenant'), - cfg.IntOpt('max_backups_per_user', default=5, + cfg.IntOpt('max_backups_per_user', default=50, help='default maximum number of backups created by a tenant'), cfg.StrOpt('quota_driver', default='trove.quota.quota.DbQuotaDriver', diff --git a/trove/instance/service.py b/trove/instance/service.py index 30c0ddd400..fc2d4f7491 100644 --- a/trove/instance/service.py +++ b/trove/instance/service.py @@ -145,9 +145,12 @@ class InstanceController(wsgi.Controller): LOG.info(_("req : '%s'\n\n") % req) LOG.info(_("Indexing backups for instance '%s'") % id) - - backups = backup_model.list_for_instance(id) - return wsgi.Result(backup_views.BackupViews(backups).data(), 200) + context = req.environ[wsgi.CONTEXT_KEY] + backups, marker = backup_model.list_for_instance(context, id) + view = backup_views.BackupViews(backups) + paged = pagination.SimplePaginatedDataView(req.url, 'backups', view, + marker) + return wsgi.Result(paged.data(), 200) def show(self, req, tenant_id, id): """Return a single instance.""" diff --git a/trove/tests/unittests/backup/test_backup_models.py b/trove/tests/unittests/backup/test_backup_models.py index 7f536b453c..6e7dd5d237 100644 --- a/trove/tests/unittests/backup/test_backup_models.py +++ b/trove/tests/unittests/backup/test_backup_models.py @@ -12,21 +12,24 @@ #limitations under the License. -import testtools -from trove.backup import models -from trove.tests.unittests.util import util -from trove.common import utils, exception -from trove.common.context import TroveContext -from trove.instance.models import BuiltInstance, Instance +import datetime from mockito import mock, when, unstub, any +import testtools + +from trove.backup import models +from trove.common import context +from trove.common import exception +from trove.common import utils +from trove.instance import models as instance_models from trove.taskmanager import api +from trove.tests.unittests.util import util def _prep_conf(current_time): current_time = str(current_time) - context = TroveContext(tenant='TENANT-' + current_time) + _context = context.TroveContext(tenant='TENANT-' + current_time) instance_id = 'INSTANCE-' + current_time - return context, instance_id + return _context, instance_id BACKUP_NAME = 'WORKS' BACKUP_NAME_2 = 'IT-WORKS' @@ -51,8 +54,9 @@ class BackupCreateTest(testtools.TestCase): tenant_id=self.context.tenant).delete() def test_create(self): - instance = mock(Instance) - when(BuiltInstance).load(any(), any()).thenReturn(instance) + instance = mock(instance_models.Instance) + when(instance_models.BuiltInstance).load(any(), any()).thenReturn( + instance) when(instance).validate_can_perform_action().thenReturn(None) when(models.Backup).verify_swift_auth_token(any()).thenReturn( None) @@ -80,8 +84,9 @@ class BackupCreateTest(testtools.TestCase): BACKUP_NAME, BACKUP_DESC) def test_create_instance_not_active(self): - instance = mock(Instance) - when(BuiltInstance).load(any(), any()).thenReturn(instance) + instance = mock(instance_models.Instance) + when(instance_models.BuiltInstance).load(any(), any()).thenReturn( + instance) when(instance).validate_can_perform_action().thenRaise( exception.UnprocessableEntity) self.assertRaises(exception.UnprocessableEntity, models.Backup.create, @@ -89,8 +94,9 @@ class BackupCreateTest(testtools.TestCase): BACKUP_NAME, BACKUP_DESC) def test_create_backup_swift_token_invalid(self): - instance = mock(Instance) - when(BuiltInstance).load(any(), any()).thenReturn(instance) + instance = mock(instance_models.Instance) + when(instance_models.BuiltInstance).load(any(), any()).thenReturn( + instance) when(instance).validate_can_perform_action().thenReturn(None) when(models.Backup).verify_swift_auth_token(any()).thenRaise( exception.SwiftAuthError) @@ -151,8 +157,9 @@ class BackupORMTest(testtools.TestCase): models.DBBackup.find_by(tenant_id=self.context.tenant).delete() def test_list(self): - db_record = models.Backup.list(self.context) - self.assertEqual(1, db_record.count()) + backups, marker = models.Backup.list(self.context) + self.assertIsNone(marker) + self.assertEqual(1, len(backups)) def test_list_for_instance(self): models.DBBackup.create(tenant_id=self.context.tenant, @@ -161,8 +168,10 @@ class BackupORMTest(testtools.TestCase): instance_id=self.instance_id, size=2.0, deleted=False) - db_record = models.Backup.list_for_instance(self.instance_id) - self.assertEqual(2, db_record.count()) + backups, marker = models.Backup.list_for_instance(self.context, + self.instance_id) + self.assertIsNone(marker) + self.assertEqual(2, len(backups)) def test_running(self): running = models.Backup.running(instance_id=self.instance_id) @@ -200,8 +209,10 @@ class BackupORMTest(testtools.TestCase): def test_backup_delete(self): backup = models.DBBackup.find_by(id=self.backup.id) backup.delete() - query = models.Backup.list_for_instance(self.instance_id) - self.assertEqual(query.count(), 0) + backups, marker = models.Backup.list_for_instance(self.context, + self.instance_id) + self.assertIsNone(marker) + self.assertEqual(0, len(backups)) def test_delete(self): self.backup.delete() @@ -214,3 +225,117 @@ class BackupORMTest(testtools.TestCase): def test_filename(self): self.assertEqual(BACKUP_FILENAME, self.backup.filename) + + +class PaginationTests(testtools.TestCase): + + def setUp(self): + super(PaginationTests, self).setUp() + util.init_db() + self.context, self.instance_id = _prep_conf(utils.utcnow()) + # Create a bunch of backups + bkup_info = { + 'tenant_id': self.context.tenant, + 'state': BACKUP_STATE, + 'instance_id': self.instance_id, + 'size': 2.0, + 'deleted': False + } + for backup in xrange(50): + bkup_info.update({'name': 'Backup-%s' % backup}) + models.DBBackup.create(**bkup_info) + + def tearDown(self): + super(PaginationTests, self).tearDown() + unstub() + query = models.DBBackup.query() + query.filter_by(instance_id=self.instance_id).delete() + + def test_pagination_list(self): + # page one + backups, marker = models.Backup.list(self.context) + self.assertEqual(20, marker) + self.assertEqual(20, len(backups)) + # page two + self.context.marker = 20 + backups, marker = models.Backup.list(self.context) + self.assertEqual(40, marker) + self.assertEqual(20, len(backups)) + # page three + self.context.marker = 40 + backups, marker = models.Backup.list(self.context) + self.assertIsNone(marker) + self.assertEqual(10, len(backups)) + + def test_pagination_list_for_instance(self): + # page one + backups, marker = models.Backup.list_for_instance(self.context, + self.instance_id) + self.assertEqual(20, marker) + self.assertEqual(20, len(backups)) + # page two + self.context.marker = 20 + backups, marker = models.Backup.list(self.context) + self.assertEqual(40, marker) + self.assertEqual(20, len(backups)) + # page three + self.context.marker = 40 + backups, marker = models.Backup.list_for_instance(self.context, + self.instance_id) + self.assertIsNone(marker) + self.assertEqual(10, len(backups)) + + +class OrderingTests(testtools.TestCase): + + def setUp(self): + super(OrderingTests, self).setUp() + util.init_db() + now = utils.utcnow() + self.context, self.instance_id = _prep_conf(now) + info = { + 'tenant_id': self.context.tenant, + 'state': BACKUP_STATE, + 'instance_id': self.instance_id, + 'size': 2.0, + 'deleted': False + } + four = now - datetime.timedelta(days=4) + one = now - datetime.timedelta(days=1) + three = now - datetime.timedelta(days=3) + two = now - datetime.timedelta(days=2) + # Create backups out of order, save/create set the 'updated' field, + # so we need to use the db_api directly. + models.DBBackup().db_api.save( + models.DBBackup(name='four', updated=four, + id=utils.generate_uuid(), **info)) + models.DBBackup().db_api.save( + models.DBBackup(name='one', updated=one, + id=utils.generate_uuid(), **info)) + models.DBBackup().db_api.save( + models.DBBackup(name='three', updated=three, + id=utils.generate_uuid(), **info)) + models.DBBackup().db_api.save( + models.DBBackup(name='two', updated=two, + id=utils.generate_uuid(), **info)) + + def tearDown(self): + super(OrderingTests, self).tearDown() + unstub() + query = models.DBBackup.query() + query.filter_by(instance_id=self.instance_id).delete() + + def test_list(self): + backups, marker = models.Backup.list(self.context) + self.assertIsNone(marker) + actual = [b.name for b in backups] + expected = [u'one', u'two', u'three', u'four'] + self.assertEqual(expected, actual) + + def test_list_for_instance(self): + backups, marker = models.Backup.list_for_instance(self.context, + self.instance_id) + self.assertIsNone(marker) + actual = [b.name for b in backups] + expected = [u'one', u'two', u'three', u'four'] + self.assertEqual(expected, actual)