Fix sort of *list command output
While sorting output of list command ("audittemplate list",
"strategy list", etc) by sort-key that is not belongs
to specific model, this sort-key was passed to db what
caused error (HTTP 500). We added check on such keys and now,
if got one of them, then we make sort on API side
instead of db side.
We removed excess sort and changed all sorting routines
to unified way.
Also added sort tests on every model.
Change-Id: I41faea1622605ee4fa8dc48cd572876d75be8383
Closes-Bug: 1662887
This commit is contained in:
@@ -205,7 +205,7 @@ class ActionCollection(collection.Collection):
|
||||
collection = ActionCollection()
|
||||
collection.actions = [Action.convert_with_links(p, expand)
|
||||
for p in actions]
|
||||
|
||||
collection.next = collection.get_next(limit, url=url, **kwargs)
|
||||
return collection
|
||||
|
||||
@classmethod
|
||||
@@ -232,6 +232,10 @@ class ActionsController(rest.RestController):
|
||||
sort_key, sort_dir, expand=False,
|
||||
resource_url=None,
|
||||
action_plan_uuid=None, audit_uuid=None):
|
||||
additional_fields = ['action_plan_uuid']
|
||||
|
||||
api_utils.validate_sort_key(sort_key, list(objects.Action.fields) +
|
||||
additional_fields)
|
||||
limit = api_utils.validate_limit(limit)
|
||||
api_utils.validate_sort_dir(sort_dir)
|
||||
|
||||
@@ -247,7 +251,10 @@ class ActionsController(rest.RestController):
|
||||
if audit_uuid:
|
||||
filters['audit_uuid'] = audit_uuid
|
||||
|
||||
sort_db_key = sort_key
|
||||
need_api_sort = api_utils.check_need_api_sort(sort_key,
|
||||
additional_fields)
|
||||
sort_db_key = (sort_key if not need_api_sort
|
||||
else None)
|
||||
|
||||
actions = objects.Action.list(pecan.request.context,
|
||||
limit,
|
||||
@@ -255,11 +262,15 @@ class ActionsController(rest.RestController):
|
||||
sort_dir=sort_dir,
|
||||
filters=filters)
|
||||
|
||||
return ActionCollection.convert_with_links(actions, limit,
|
||||
url=resource_url,
|
||||
expand=expand,
|
||||
sort_key=sort_key,
|
||||
sort_dir=sort_dir)
|
||||
actions_collection = ActionCollection.convert_with_links(
|
||||
actions, limit, url=resource_url, expand=expand,
|
||||
sort_key=sort_key, sort_dir=sort_dir)
|
||||
|
||||
if need_api_sort:
|
||||
api_utils.make_api_sort(actions_collection.actions,
|
||||
sort_key, sort_dir)
|
||||
|
||||
return actions_collection
|
||||
|
||||
@wsme_pecan.wsexpose(ActionCollection, types.uuid, int,
|
||||
wtypes.text, wtypes.text, types.uuid,
|
||||
|
||||
@@ -305,17 +305,6 @@ class ActionPlanCollection(collection.Collection):
|
||||
ap_collection = ActionPlanCollection()
|
||||
ap_collection.action_plans = [ActionPlan.convert_with_links(
|
||||
p, expand) for p in rpc_action_plans]
|
||||
|
||||
if 'sort_key' in kwargs:
|
||||
reverse = False
|
||||
if kwargs['sort_key'] == 'audit_uuid':
|
||||
if 'sort_dir' in kwargs:
|
||||
reverse = True if kwargs['sort_dir'] == 'desc' else False
|
||||
ap_collection.action_plans = sorted(
|
||||
ap_collection.action_plans,
|
||||
key=lambda action_plan: action_plan.audit_uuid,
|
||||
reverse=reverse)
|
||||
|
||||
ap_collection.next = ap_collection.get_next(limit, url=url, **kwargs)
|
||||
return ap_collection
|
||||
|
||||
@@ -344,7 +333,10 @@ class ActionPlansController(rest.RestController):
|
||||
sort_key, sort_dir, expand=False,
|
||||
resource_url=None, audit_uuid=None,
|
||||
strategy=None):
|
||||
additional_fields = ['audit_uuid', 'strategy_uuid', 'strategy_name']
|
||||
|
||||
api_utils.validate_sort_key(
|
||||
sort_key, list(objects.ActionPlan.fields) + additional_fields)
|
||||
limit = api_utils.validate_limit(limit)
|
||||
api_utils.validate_sort_dir(sort_dir)
|
||||
|
||||
@@ -363,10 +355,10 @@ class ActionPlansController(rest.RestController):
|
||||
else:
|
||||
filters['strategy_name'] = strategy
|
||||
|
||||
if sort_key == 'audit_uuid':
|
||||
sort_db_key = None
|
||||
else:
|
||||
sort_db_key = sort_key
|
||||
need_api_sort = api_utils.check_need_api_sort(sort_key,
|
||||
additional_fields)
|
||||
sort_db_key = (sort_key if not need_api_sort
|
||||
else None)
|
||||
|
||||
action_plans = objects.ActionPlan.list(
|
||||
pecan.request.context,
|
||||
@@ -374,12 +366,15 @@ class ActionPlansController(rest.RestController):
|
||||
marker_obj, sort_key=sort_db_key,
|
||||
sort_dir=sort_dir, filters=filters)
|
||||
|
||||
return ActionPlanCollection.convert_with_links(
|
||||
action_plans, limit,
|
||||
url=resource_url,
|
||||
expand=expand,
|
||||
sort_key=sort_key,
|
||||
sort_dir=sort_dir)
|
||||
action_plans_collection = ActionPlanCollection.convert_with_links(
|
||||
action_plans, limit, url=resource_url, expand=expand,
|
||||
sort_key=sort_key, sort_dir=sort_dir)
|
||||
|
||||
if need_api_sort:
|
||||
api_utils.make_api_sort(action_plans_collection.action_plans,
|
||||
sort_key, sort_dir)
|
||||
|
||||
return action_plans_collection
|
||||
|
||||
@wsme_pecan.wsexpose(ActionPlanCollection, types.uuid, int, wtypes.text,
|
||||
wtypes.text, types.uuid, wtypes.text)
|
||||
|
||||
@@ -389,17 +389,6 @@ class AuditCollection(collection.Collection):
|
||||
collection = AuditCollection()
|
||||
collection.audits = [Audit.convert_with_links(p, expand)
|
||||
for p in rpc_audits]
|
||||
|
||||
if 'sort_key' in kwargs:
|
||||
reverse = False
|
||||
if kwargs['sort_key'] == 'goal_uuid':
|
||||
if 'sort_dir' in kwargs:
|
||||
reverse = True if kwargs['sort_dir'] == 'desc' else False
|
||||
collection.audits = sorted(
|
||||
collection.audits,
|
||||
key=lambda audit: audit.goal_uuid,
|
||||
reverse=reverse)
|
||||
|
||||
collection.next = collection.get_next(limit, url=url, **kwargs)
|
||||
return collection
|
||||
|
||||
@@ -427,8 +416,14 @@ class AuditsController(rest.RestController):
|
||||
sort_key, sort_dir, expand=False,
|
||||
resource_url=None, goal=None,
|
||||
strategy=None):
|
||||
additional_fields = ["goal_uuid", "goal_name", "strategy_uuid",
|
||||
"strategy_name"]
|
||||
|
||||
api_utils.validate_sort_key(
|
||||
sort_key, list(objects.Audit.fields) + additional_fields)
|
||||
limit = api_utils.validate_limit(limit)
|
||||
api_utils.validate_sort_dir(sort_dir)
|
||||
|
||||
marker_obj = None
|
||||
if marker:
|
||||
marker_obj = objects.Audit.get_by_uuid(pecan.request.context,
|
||||
@@ -449,23 +444,25 @@ class AuditsController(rest.RestController):
|
||||
# TODO(michaelgugino): add method to get goal by name.
|
||||
filters['strategy_name'] = strategy
|
||||
|
||||
if sort_key == 'goal_uuid':
|
||||
sort_db_key = 'goal_id'
|
||||
elif sort_key == 'strategy_uuid':
|
||||
sort_db_key = 'strategy_id'
|
||||
else:
|
||||
sort_db_key = sort_key
|
||||
need_api_sort = api_utils.check_need_api_sort(sort_key,
|
||||
additional_fields)
|
||||
sort_db_key = (sort_key if not need_api_sort
|
||||
else None)
|
||||
|
||||
audits = objects.Audit.list(pecan.request.context,
|
||||
limit,
|
||||
marker_obj, sort_key=sort_db_key,
|
||||
sort_dir=sort_dir, filters=filters)
|
||||
|
||||
return AuditCollection.convert_with_links(audits, limit,
|
||||
url=resource_url,
|
||||
expand=expand,
|
||||
sort_key=sort_key,
|
||||
sort_dir=sort_dir)
|
||||
audits_collection = AuditCollection.convert_with_links(
|
||||
audits, limit, url=resource_url, expand=expand,
|
||||
sort_key=sort_key, sort_dir=sort_dir)
|
||||
|
||||
if need_api_sort:
|
||||
api_utils.make_api_sort(audits_collection.audits, sort_key,
|
||||
sort_dir)
|
||||
|
||||
return audits_collection
|
||||
|
||||
@wsme_pecan.wsexpose(AuditCollection, types.uuid, int, wtypes.text,
|
||||
wtypes.text, wtypes.text, wtypes.text, int)
|
||||
|
||||
@@ -474,9 +474,13 @@ class AuditTemplatesController(rest.RestController):
|
||||
def _get_audit_templates_collection(self, filters, marker, limit,
|
||||
sort_key, sort_dir, expand=False,
|
||||
resource_url=None):
|
||||
additional_fields = ["goal_uuid", "goal_name", "strategy_uuid",
|
||||
"strategy_name"]
|
||||
|
||||
api_utils.validate_sort_key(
|
||||
sort_key, list(objects.AuditTemplate.fields) + additional_fields)
|
||||
api_utils.validate_search_filters(
|
||||
filters, list(objects.audit_template.AuditTemplate.fields) +
|
||||
["goal_uuid", "goal_name", "strategy_uuid", "strategy_name"])
|
||||
filters, list(objects.AuditTemplate.fields) + additional_fields)
|
||||
limit = api_utils.validate_limit(limit)
|
||||
api_utils.validate_sort_dir(sort_dir)
|
||||
|
||||
@@ -486,19 +490,26 @@ class AuditTemplatesController(rest.RestController):
|
||||
pecan.request.context,
|
||||
marker)
|
||||
|
||||
audit_templates = objects.AuditTemplate.list(
|
||||
pecan.request.context,
|
||||
filters,
|
||||
limit,
|
||||
marker_obj, sort_key=sort_key,
|
||||
sort_dir=sort_dir)
|
||||
need_api_sort = api_utils.check_need_api_sort(sort_key,
|
||||
additional_fields)
|
||||
sort_db_key = (sort_key if not need_api_sort
|
||||
else None)
|
||||
|
||||
return AuditTemplateCollection.convert_with_links(audit_templates,
|
||||
limit,
|
||||
url=resource_url,
|
||||
expand=expand,
|
||||
sort_key=sort_key,
|
||||
sort_dir=sort_dir)
|
||||
audit_templates = objects.AuditTemplate.list(
|
||||
pecan.request.context, filters, limit, marker_obj,
|
||||
sort_key=sort_db_key, sort_dir=sort_dir)
|
||||
|
||||
audit_templates_collection = \
|
||||
AuditTemplateCollection.convert_with_links(
|
||||
audit_templates, limit, url=resource_url, expand=expand,
|
||||
sort_key=sort_key, sort_dir=sort_dir)
|
||||
|
||||
if need_api_sort:
|
||||
api_utils.make_api_sort(
|
||||
audit_templates_collection.audit_templates, sort_key,
|
||||
sort_dir)
|
||||
|
||||
return audit_templates_collection
|
||||
|
||||
@wsme_pecan.wsexpose(AuditTemplateCollection, wtypes.text, wtypes.text,
|
||||
types.uuid, int, wtypes.text, wtypes.text)
|
||||
|
||||
@@ -130,17 +130,6 @@ class GoalCollection(collection.Collection):
|
||||
goal_collection = GoalCollection()
|
||||
goal_collection.goals = [
|
||||
Goal.convert_with_links(g, expand) for g in goals]
|
||||
|
||||
if 'sort_key' in kwargs:
|
||||
reverse = False
|
||||
if kwargs['sort_key'] == 'strategy':
|
||||
if 'sort_dir' in kwargs:
|
||||
reverse = True if kwargs['sort_dir'] == 'desc' else False
|
||||
goal_collection.goals = sorted(
|
||||
goal_collection.goals,
|
||||
key=lambda goal: goal.uuid,
|
||||
reverse=reverse)
|
||||
|
||||
goal_collection.next = goal_collection.get_next(
|
||||
limit, url=url, **kwargs)
|
||||
return goal_collection
|
||||
@@ -167,17 +156,19 @@ class GoalsController(rest.RestController):
|
||||
|
||||
def _get_goals_collection(self, marker, limit, sort_key, sort_dir,
|
||||
expand=False, resource_url=None):
|
||||
api_utils.validate_sort_key(
|
||||
sort_key, list(objects.Goal.fields))
|
||||
limit = api_utils.validate_limit(limit)
|
||||
api_utils.validate_sort_dir(sort_dir)
|
||||
|
||||
sort_db_key = (sort_key if sort_key in objects.Goal.fields
|
||||
else None)
|
||||
|
||||
marker_obj = None
|
||||
if marker:
|
||||
marker_obj = objects.Goal.get_by_uuid(
|
||||
pecan.request.context, marker)
|
||||
|
||||
sort_db_key = (sort_key if sort_key in objects.Goal.fields
|
||||
else None)
|
||||
|
||||
goals = objects.Goal.list(pecan.request.context, limit, marker_obj,
|
||||
sort_key=sort_db_key, sort_dir=sort_dir)
|
||||
|
||||
|
||||
@@ -123,17 +123,6 @@ class ScoringEngineCollection(collection.Collection):
|
||||
collection = ScoringEngineCollection()
|
||||
collection.scoring_engines = [ScoringEngine.convert_with_links(
|
||||
se, expand) for se in scoring_engines]
|
||||
|
||||
if 'sort_key' in kwargs:
|
||||
reverse = False
|
||||
if kwargs['sort_key'] == 'name':
|
||||
if 'sort_dir' in kwargs:
|
||||
reverse = True if kwargs['sort_dir'] == 'desc' else False
|
||||
collection.goals = sorted(
|
||||
collection.scoring_engines,
|
||||
key=lambda se: se.name,
|
||||
reverse=reverse)
|
||||
|
||||
collection.next = collection.get_next(limit, url=url, **kwargs)
|
||||
return collection
|
||||
|
||||
@@ -160,7 +149,8 @@ class ScoringEngineController(rest.RestController):
|
||||
def _get_scoring_engines_collection(self, marker, limit,
|
||||
sort_key, sort_dir, expand=False,
|
||||
resource_url=None):
|
||||
|
||||
api_utils.validate_sort_key(
|
||||
sort_key, list(objects.ScoringEngine.fields))
|
||||
limit = api_utils.validate_limit(limit)
|
||||
api_utils.validate_sort_dir(sort_dir)
|
||||
|
||||
@@ -171,7 +161,8 @@ class ScoringEngineController(rest.RestController):
|
||||
|
||||
filters = {}
|
||||
|
||||
sort_db_key = sort_key
|
||||
sort_db_key = (sort_key if sort_key in objects.ScoringEngine.fields
|
||||
else None)
|
||||
|
||||
scoring_engines = objects.ScoringEngine.list(
|
||||
context=pecan.request.context,
|
||||
|
||||
@@ -154,17 +154,6 @@ class ServiceCollection(collection.Collection):
|
||||
service_collection = ServiceCollection()
|
||||
service_collection.services = [
|
||||
Service.convert_with_links(g, expand) for g in services]
|
||||
|
||||
if 'sort_key' in kwargs:
|
||||
reverse = False
|
||||
if kwargs['sort_key'] == 'service':
|
||||
if 'sort_dir' in kwargs:
|
||||
reverse = True if kwargs['sort_dir'] == 'desc' else False
|
||||
service_collection.services = sorted(
|
||||
service_collection.services,
|
||||
key=lambda service: service.id,
|
||||
reverse=reverse)
|
||||
|
||||
service_collection.next = service_collection.get_next(
|
||||
limit, url=url, marker_field='id', **kwargs)
|
||||
return service_collection
|
||||
@@ -191,17 +180,19 @@ class ServicesController(rest.RestController):
|
||||
|
||||
def _get_services_collection(self, marker, limit, sort_key, sort_dir,
|
||||
expand=False, resource_url=None):
|
||||
api_utils.validate_sort_key(
|
||||
sort_key, list(objects.Service.fields))
|
||||
limit = api_utils.validate_limit(limit)
|
||||
api_utils.validate_sort_dir(sort_dir)
|
||||
|
||||
sort_db_key = (sort_key if sort_key in objects.Service.fields
|
||||
else None)
|
||||
|
||||
marker_obj = None
|
||||
if marker:
|
||||
marker_obj = objects.Service.get(
|
||||
pecan.request.context, marker)
|
||||
|
||||
sort_db_key = (sort_key if sort_key in objects.Service.fields
|
||||
else None)
|
||||
|
||||
services = objects.Service.list(
|
||||
pecan.request.context, limit, marker_obj,
|
||||
sort_key=sort_db_key, sort_dir=sort_dir)
|
||||
|
||||
@@ -173,17 +173,6 @@ class StrategyCollection(collection.Collection):
|
||||
strategy_collection = StrategyCollection()
|
||||
strategy_collection.strategies = [
|
||||
Strategy.convert_with_links(g, expand) for g in strategies]
|
||||
|
||||
if 'sort_key' in kwargs:
|
||||
reverse = False
|
||||
if kwargs['sort_key'] == 'strategy':
|
||||
if 'sort_dir' in kwargs:
|
||||
reverse = True if kwargs['sort_dir'] == 'desc' else False
|
||||
strategy_collection.strategies = sorted(
|
||||
strategy_collection.strategies,
|
||||
key=lambda strategy: strategy.uuid,
|
||||
reverse=reverse)
|
||||
|
||||
strategy_collection.next = strategy_collection.get_next(
|
||||
limit, url=url, **kwargs)
|
||||
return strategy_collection
|
||||
@@ -211,28 +200,39 @@ class StrategiesController(rest.RestController):
|
||||
|
||||
def _get_strategies_collection(self, filters, marker, limit, sort_key,
|
||||
sort_dir, expand=False, resource_url=None):
|
||||
additional_fields = ["goal_uuid", "goal_name"]
|
||||
|
||||
api_utils.validate_sort_key(
|
||||
sort_key, list(objects.Strategy.fields) + additional_fields)
|
||||
api_utils.validate_search_filters(
|
||||
filters, list(objects.strategy.Strategy.fields) +
|
||||
["goal_uuid", "goal_name"])
|
||||
filters, list(objects.Strategy.fields) + additional_fields)
|
||||
limit = api_utils.validate_limit(limit)
|
||||
api_utils.validate_sort_dir(sort_dir)
|
||||
|
||||
sort_db_key = (sort_key if sort_key in objects.Strategy.fields
|
||||
else None)
|
||||
|
||||
marker_obj = None
|
||||
if marker:
|
||||
marker_obj = objects.Strategy.get_by_uuid(
|
||||
pecan.request.context, marker)
|
||||
|
||||
need_api_sort = api_utils.check_need_api_sort(sort_key,
|
||||
additional_fields)
|
||||
sort_db_key = (sort_key if not need_api_sort
|
||||
else None)
|
||||
|
||||
strategies = objects.Strategy.list(
|
||||
pecan.request.context, limit, marker_obj, filters=filters,
|
||||
sort_key=sort_db_key, sort_dir=sort_dir)
|
||||
|
||||
return StrategyCollection.convert_with_links(
|
||||
strategies_collection = StrategyCollection.convert_with_links(
|
||||
strategies, limit, url=resource_url, expand=expand,
|
||||
sort_key=sort_key, sort_dir=sort_dir)
|
||||
|
||||
if need_api_sort:
|
||||
api_utils.make_api_sort(strategies_collection.strategies,
|
||||
sort_key, sort_dir)
|
||||
|
||||
return strategies_collection
|
||||
|
||||
@wsme_pecan.wsexpose(StrategyCollection, wtypes.text, wtypes.text,
|
||||
int, wtypes.text, wtypes.text)
|
||||
def get_all(self, goal=None, marker=None, limit=None,
|
||||
|
||||
@@ -13,6 +13,8 @@
|
||||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
|
||||
from operator import attrgetter
|
||||
|
||||
import jsonpatch
|
||||
from oslo_config import cfg
|
||||
from oslo_utils import reflection
|
||||
@@ -54,6 +56,13 @@ def validate_sort_dir(sort_dir):
|
||||
"'asc' or 'desc'") % sort_dir)
|
||||
|
||||
|
||||
def validate_sort_key(sort_key, allowed_fields):
|
||||
# Very lightweight validation for now
|
||||
if sort_key not in allowed_fields:
|
||||
raise wsme.exc.ClientSideError(
|
||||
_("Invalid sort key: %s") % sort_key)
|
||||
|
||||
|
||||
def validate_search_filters(filters, allowed_fields):
|
||||
# Very lightweight validation for now
|
||||
# todo: improve this (e.g. https://www.parse.com/docs/rest/guide/#queries)
|
||||
@@ -63,6 +72,19 @@ def validate_search_filters(filters, allowed_fields):
|
||||
_("Invalid filter: %s") % filter_name)
|
||||
|
||||
|
||||
def check_need_api_sort(sort_key, additional_fields):
|
||||
return sort_key in additional_fields
|
||||
|
||||
|
||||
def make_api_sort(sorting_list, sort_key, sort_dir):
|
||||
# First sort by uuid field, than sort by sort_key
|
||||
# sort() ensures stable sorting, so we could
|
||||
# make lexicographical sort
|
||||
reverse_direction = (sort_dir == 'desc')
|
||||
sorting_list.sort(key=attrgetter('uuid'), reverse=reverse_direction)
|
||||
sorting_list.sort(key=attrgetter(sort_key), reverse=reverse_direction)
|
||||
|
||||
|
||||
def apply_jsonpatch(doc, patch):
|
||||
for p in patch:
|
||||
if p['op'] == 'add' and p['path'].count('/') == 1:
|
||||
|
||||
Reference in New Issue
Block a user