openstack/senlin
Code Issues Proposed changes

Revise profile sorting

Browse Source 
We are using a new schema for sorting profiles. This patch replaces the
'sort_keys' and the 'sort_dir' parameter with a single 'sort' parameter.
It also fixes multi-tenancy problem with profile listing.

Change-Id: If2bf9d60c056e8101b86fcaa48f85e2d60554adc
Closes-Bug: #1530748
This commit is contained in:
tengqm
2016-01-08 04:08:38 -05:00
parent 07f22506b4
commit aa9154a503
10 changed files with 61 additions and 82 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
senlin/api/openstack/v1/profiles.py
View File

@@ -75,8 +75,8 @@ class ProfileController(object):
param_whitelist = {
'limit': 'single',
'marker': 'single',
'sort_dir': 'single',
'sort_keys': 'multi',
'sort': 'single',
'global_project': 'single',
}
params = util.get_allowed_params(req.params, param_whitelist)
filters = util.get_allowed_params(req.params, filter_whitelist)
@@ -85,11 +85,16 @@ class ProfileController(object):
if key in params:
params[key] = utils.parse_int_param(key, params[key])
key = consts.PARAM_GLOBAL_PROJECT
if key in params:
project_safe = not utils.parse_bool_param(key, params[key])
del params[key]
params['project_safe'] = project_safe
if not filters:
filters = None
profiles = self.rpc_client.profile_list(req.context,
filters=filters,
profiles = self.rpc_client.profile_list(req.context, filters=filters,
**params)
return {'profiles': profiles}

8
senlin/db/api.py
View File

@@ -227,11 +227,11 @@ def profile_get_by_short_id(context, short_id, project_safe=True):
project_safe=project_safe)
def profile_get_all(context, limit=None, marker=None, sort_keys=None,
sort_dir=None, filters=None, project_safe=True):
def profile_get_all(context, limit=None, marker=None, sort=None, filters=None,
project_safe=True):
return IMPL.profile_get_all(context, limit=limit, marker=marker,
sort_keys=sort_keys, sort_dir=sort_dir,
filters=filters, project_safe=project_safe)
sort=sort, filters=filters,
project_safe=project_safe)
def profile_update(context, profile_id, values):

14
senlin/db/sqlalchemy/api.py
View File

@@ -709,8 +709,8 @@ def profile_get_by_short_id(context, short_id, project_safe=True):
project_safe=project_safe)
def profile_get_all(context, limit=None, marker=None, sort_keys=None,
sort_dir=None, filters=None, project_safe=True):
def profile_get_all(context, limit=None, marker=None, sort=None, filters=None,
project_safe=True):
query = model_query(context, models.Profile)
if project_safe:
@@ -719,12 +719,12 @@ def profile_get_all(context, limit=None, marker=None, sort_keys=None,
if filters is None:
filters = {}
keys = _get_sort_keys(sort_keys, consts.PROFILE_SORT_KEYS)
keys, dirs = _get_sort_params(sort, consts.PROFILE_SORT_KEYS, 'created_at')
query = db_filters.exact_filter(query, models.Profile, filters)
return _paginate_query(context, query, models.Profile,
limit=limit, marker=marker,
sort_keys=keys, sort_dir=sort_dir,
default_sort_keys=['created_at']).all()
if marker:
marker = model_query(context, models.Profile).get(marker)
return utils.paginate_query(query, models.Profile, limit, keys,
marker=marker, sort_dirs=dirs).all()
def profile_update(context, profile_id, values):

13
senlin/engine/service.py
View File

@@ -181,15 +181,14 @@ class EngineService(service.Service):
return profile
@request_context
def profile_list(self, context, limit=None, marker=None, sort_keys=None,
sort_dir=None, filters=None):
def profile_list(self, context, limit=None, marker=None, sort=None,
filters=None, project_safe=True):
if limit is not None:
limit = utils.parse_int_param('limit', limit)
profiles = profile_base.Profile.load_all(context, limit=limit,
marker=marker,
sort_keys=sort_keys,
sort_dir=sort_dir,
filters=filters)
profiles = profile_base.Profile.load_all(context,
limit=limit, marker=marker,
sort=sort, filters=filters,
project_safe=project_safe)
return [p.to_dict() for p in profiles]

9
senlin/profiles/base.py
View File

@@ -137,13 +137,12 @@ class Profile(object):
return cls.from_db_record(profile)
@classmethod
def load_all(cls, ctx, limit=None, sort_keys=None, marker=None,
sort_dir=None, filters=None, project_safe=True):
'''Retrieve all profiles from database.'''
def load_all(cls, ctx, limit=None, marker=None, sort=None, filters=None,
project_safe=True):
"""Retrieve all profiles from database."""
records = db_api.profile_get_all(ctx, limit=limit, marker=marker,
sort_keys=sort_keys,
sort_dir=sort_dir, filters=filters,
sort=sort, filters=filters,
project_safe=project_safe)
for record in records:

10
senlin/rpc/client.py
View File

@@ -76,12 +76,12 @@ class EngineClient(object):
return self.call(ctxt, self.make_msg('profile_type_get',
type_name=type_name))
def profile_list(self, ctxt, limit=None, marker=None, sort_keys=None,
sort_dir=None, filters=None):
def profile_list(self, ctxt, limit=None, marker=None, sort=None,
filters=None, project_safe=True):
return self.call(ctxt,
self.make_msg('profile_list', limit=limit,
marker=marker, sort_keys=sort_keys,
sort_dir=sort_dir, filters=filters))
self.make_msg('profile_list', filters=filters,
limit=limit, marker=marker, sort=sort,
project_safe=project_safe))
def profile_create(self, ctxt, name, spec, permission, metadata):
return self.call(ctxt,

11
senlin/tests/unit/apiv1/test_profiles.py
View File

@@ -101,7 +101,7 @@ class ProfileControllerTest(shared.ControllerTest, base.SenlinTestCase):
result = self.controller.index(req)
default_args = {'limit': None, 'marker': None, 'filters': None,
'sort_keys': None, 'sort_dir': None}
'sort': None, 'project_safe': True}
mock_call.assert_called_with(req.context,
('profile_list', default_args))
@@ -114,9 +114,9 @@ class ProfileControllerTest(shared.ControllerTest, base.SenlinTestCase):
params = {
'limit': 20,
'marker': 'fake marker',
'sort_keys': 'fake sort keys',
'sort_dir': 'fake sort dir',
'sort': 'fake sorting string',
'filters': None,
'global_project': False,
'balrog': 'you shall not pass!'
}
req = self._get('/profiles', params=params)
@@ -132,10 +132,9 @@ class ProfileControllerTest(shared.ControllerTest, base.SenlinTestCase):
self.assertEqual(5, len(engine_args))
self.assertIn('limit', engine_args)
self.assertIn('marker', engine_args)
self.assertIn('sort_keys', engine_args)
self.assertIn('sort_dir', engine_args)
self.assertIn('sort', engine_args)
self.assertIn('filters', engine_args)
self.assertNotIn('tenant_safe', engine_args)
self.assertIn('project_safe', engine_args)
self.assertNotIn('balrog', engine_args)
def test_profile_index_whitelist_filter_params(self, mock_enforce):

36
senlin/tests/unit/db/test_profile_api.py
View File

@@ -10,10 +10,10 @@
# License for the specific language governing permissions and limitations
# under the License.
from oslo_utils import timeutils as tu
import six
from oslo_utils import timeutils as tu
from senlin.common import consts
from senlin.common import exception
from senlin.db.sqlalchemy import api as db_api
from senlin.engine import parser
@@ -199,23 +199,14 @@ class DBAPIProfileTest(base.SenlinTestCase):
shared.create_profile(self.ctx, id=pid)
mock_paginate = self.patchobject(db_api.utils, 'paginate_query')
sort_keys = ['type', 'name', 'permission', 'metadata',
'created_at', 'updated_at']
db_api.profile_get_all(self.ctx, sort_keys=sort_keys)
sort_keys = consts.PROFILE_SORT_KEYS
db_api.profile_get_all(self.ctx, sort=','.join(sort_keys))
args = mock_paginate.call_args[0]
used_sort_keys = set(args[3])
expected_keys = set(['id', 'type', 'name', 'permission',
'created_at', 'updated_at'])
self.assertEqual(expected_keys, used_sort_keys)
sort_keys.append('id')
self.assertEqual(set(sort_keys), set(args[3]))
def test_profile_get_all_sort_keys_wont_change(self):
sort_keys = ['id']
db_api.profile_get_all(self.ctx, sort_keys=sort_keys)
self.assertEqual(['id'], sort_keys)
def test_profile_get_all_sort_keys_and_dir(self):
def test_profile_get_all_sorting(self):
values = [{'id': '001', 'name': 'profile1', 'type': 'C'},
{'id': '002', 'name': 'profile3', 'type': 'B'},
{'id': '003', 'name': 'profile2', 'type': 'A'}]
@@ -223,37 +214,34 @@ class DBAPIProfileTest(base.SenlinTestCase):
for v in values:
shared.create_profile(self.ctx, **v)
profiles = db_api.profile_get_all(self.ctx, sort_keys=['name', 'type'],
sort_dir='asc')
profiles = db_api.profile_get_all(self.ctx, sort='name,type')
self.assertEqual(3, len(profiles))
# Sorted by name
self.assertEqual('001', profiles[0].id)
self.assertEqual('003', profiles[1].id)
self.assertEqual('002', profiles[2].id)
profiles = db_api.profile_get_all(self.ctx, sort_keys=['type', 'name'],
sort_dir='asc')
profiles = db_api.profile_get_all(self.ctx, sort='type,name')
self.assertEqual(3, len(profiles))
# Sorted by levels (ascending)
self.assertEqual('003', profiles[0].id)
self.assertEqual('002', profiles[1].id)
self.assertEqual('001', profiles[2].id)
profiles = db_api.profile_get_all(self.ctx, sort_keys=['type', 'name'],
sort_dir='desc')
profiles = db_api.profile_get_all(self.ctx, sort='type:desc,name:desc')
self.assertEqual(3, len(profiles))
# Sorted by statuses (descending)
self.assertEqual('001', profiles[0].id)
self.assertEqual('002', profiles[1].id)
self.assertEqual('003', profiles[2].id)
def test_profile_get_all_default_sort_dir(self):
def test_profile_get_all_default_sorting(self):
profiles = []
for x in range(3):
profile = shared.create_profile(self.ctx, created_at=tu.utcnow())
profiles.append(profile)
results = db_api.profile_get_all(self.ctx, sort_dir='asc')
results = db_api.profile_get_all(self.ctx)
self.assertEqual(3, len(results))
self.assertEqual(profiles[0].id, results[0].id)
self.assertEqual(profiles[1].id, results[1].id)

25
senlin/tests/unit/engine/service/test_profiles.py
View File

@@ -166,26 +166,24 @@ class ProfileTest(base.SenlinTestCase):
self.assertEqual(p2['id'], result[1]['id'])
# use name for sorting
result = self.eng.profile_list(self.ctx, sort_keys=['name'])
result = self.eng.profile_list(self.ctx, sort='name')
self.assertEqual(p2['id'], result[0]['id'])
self.assertEqual(p1['id'], result[1]['id'])
# use permission for sorting
result = self.eng.profile_list(self.ctx, sort_keys=['permission'])
result = self.eng.profile_list(self.ctx, sort='permission')
self.assertEqual(p3['id'], result[0]['id'])
# use name and permission for sorting
result = self.eng.profile_list(self.ctx,
sort_keys=['permission', 'name'])
result = self.eng.profile_list(self.ctx, sort='permission,name')
self.assertEqual(p3['id'], result[0]['id'])
self.assertEqual(p2['id'], result[1]['id'])
self.assertEqual(p1['id'], result[2]['id'])
# unknown keys will be ignored
result = self.eng.profile_list(self.ctx, sort_keys=['duang'])
result = self.eng.profile_list(self.ctx, sort='duang')
self.assertIsNotNone(result)
def test_profile_list_with_sort_dir(self):
def test_profile_list_with_sorting(self):
p1 = self.eng.profile_create(self.ctx, 'p-B', self.spec,
permission='1111')
p2 = self.eng.profile_create(self.ctx, 'p-A', self.spec,
@@ -199,24 +197,15 @@ class ProfileTest(base.SenlinTestCase):
self.assertEqual(p2['id'], result[1]['id'])
# sort by created_at, descending
result = self.eng.profile_list(self.ctx, sort_dir='desc')
result = self.eng.profile_list(self.ctx, sort='created_at:desc')
self.assertEqual(p3['id'], result[0]['id'])
self.assertEqual(p2['id'], result[1]['id'])
# use name for sorting, descending
result = self.eng.profile_list(self.ctx, sort_keys=['name'],
sort_dir='desc')
result = self.eng.profile_list(self.ctx, sort='name:desc')
self.assertEqual(p3['id'], result[0]['id'])
self.assertEqual(p1['id'], result[1]['id'])
# use permission for sorting
ex = self.assertRaises(ValueError,
self.eng.profile_list, self.ctx,
sort_dir='Bogus')
self.assertEqual("Unknown sort direction, must be one of: "
"asc-nullsfirst, asc-nullslast, desc-nullsfirst, "
"desc-nullslast", six.text_type(ex))
def test_profile_list_with_filters(self):
self.eng.profile_create(self.ctx, 'p-B', self.spec,
permission='1111')

4
senlin/tests/unit/test_rpc_client.py
View File

@@ -162,9 +162,9 @@ class EngineRpcAPITestCase(base.SenlinTestCase):
default_args = {
'limit': mock.ANY,
'marker': mock.ANY,
'sort_keys': mock.ANY,
'sort_dir': mock.ANY,
'sort': mock.ANY,
'filters': mock.ANY,
'project_safe': mock.ANY,
}
self._test_engine_api('profile_list', 'call', **default_args)