diff --git a/manila/api/common.py b/manila/api/common.py index 7e26c3e4fa..8484d1c9fb 100644 --- a/manila/api/common.py +++ b/manila/api/common.py @@ -29,6 +29,7 @@ import webob from manila.api.openstack import api_version_request as api_version from manila.api.openstack import versioned_method +from manila.common import constants from manila import exception from manila.i18n import _ from manila import policy @@ -66,7 +67,7 @@ def validate_key_names(key_names_list): def get_pagination_params(request): - """Return marker, limit tuple from request. + """Return marker, limit, offset tuple from request. :param request: `wsgi.Request` possibly containing 'marker' and 'limit' GET variables. 'marker' is the id of the last element @@ -82,11 +83,18 @@ def get_pagination_params(request): params['limit'] = _get_limit_param(request) if 'marker' in request.GET: params['marker'] = _get_marker_param(request) + if 'offset' in request.GET: + params['offset'] = _get_offset_param(request) return params def _get_limit_param(request): - """Extract integer limit from request or fail.""" + """Extract integer limit from request or fail. + + Defaults to max_limit if not present and returns max_limit if present + 'limit' is greater than max_limit. + """ + max_limit = CONF.osapi_max_limit try: limit = int(request.GET['limit']) except ValueError: @@ -95,6 +103,7 @@ def _get_limit_param(request): if limit < 0: msg = _('limit param must be positive') raise webob.exc.HTTPBadRequest(explanation=msg) + limit = min(limit, max_limit) return limit @@ -103,6 +112,31 @@ def _get_marker_param(request): return request.GET['marker'] +def _get_offset_param(request): + """Extract offset id from request's dictionary (defaults to 0) or fail.""" + offset = request.GET['offset'] + return _validate_integer(offset, + 'offset', + 0, + constants.DB_MAX_INT) + + +def _validate_integer(value, name, min_value=None, max_value=None): + """Make sure that value is a valid integer, potentially within range. + + :param value: the value of the integer + :param name: the name of the integer + :param min_value: the min_length of the integer + :param max_value: the max_length of the integer + :return: integer + """ + try: + value = strutils.validate_integer(value, name, min_value, max_value) + return value + except ValueError as e: + raise webob.exc.HTTPBadRequest(explanation=e) + + def _validate_pagination_query(request, max_limit=CONF.osapi_max_limit): """Validate the given request query and return limit and offset.""" @@ -148,6 +182,25 @@ def limited(items, request, max_limit=CONF.osapi_max_limit): return items[offset:range_end] +def get_sort_params(params, default_key='created_at', default_dir='desc'): + """Retrieves sort key/direction parameters. + + Processes the parameters to get the 'sort_key' and 'sort_dir' parameter + values. + + :param params: webob.multidict of request parameters (from + manila.api.openstack.wsgi.Request.params) + :param default_key: default sort key value, will return if no + sort key are supplied + :param default_dir: default sort dir value, will return if no + sort dir are supplied + :returns: value of sort key, value of sort dir + """ + sort_key = params.pop('sort_key', default_key) + sort_dir = params.pop('sort_dir', default_dir) + return sort_key, sort_dir + + def remove_version_from_href(href): """Removes the first api version from the href. diff --git a/manila/api/openstack/api_version_request.py b/manila/api/openstack/api_version_request.py index 009fb721ab..99887bbab2 100644 --- a/manila/api/openstack/api_version_request.py +++ b/manila/api/openstack/api_version_request.py @@ -140,13 +140,16 @@ REST_API_VERSION_HISTORY = """ * 2.51 - Added Share Network with multiple Subnets. Updated Share Networks to handle with one or more subnets in different availability zones. + * 2.52 - Added 'created_before' and 'created_since' field to list messages + filters, support querying user messages within the specified time + period. """ # The minimum and maximum versions of the API supported # The default api version request is defined to be the # minimum version of the API supported. _MIN_API_VERSION = "2.0" -_MAX_API_VERSION = "2.51" +_MAX_API_VERSION = "2.52" DEFAULT_API_VERSION = _MIN_API_VERSION diff --git a/manila/api/openstack/rest_api_version_history.rst b/manila/api/openstack/rest_api_version_history.rst index 7eb92db1c8..8e7eb56331 100644 --- a/manila/api/openstack/rest_api_version_history.rst +++ b/manila/api/openstack/rest_api_version_history.rst @@ -287,3 +287,8 @@ user documentation. Added to the service the possibility to have multiple subnets per share network, each of them associated to a different AZ. It is also possible to configure a default subnet that spans all availability zones. + +2.52 +---- + Added 'created_before' and 'created_since' field to list messages api, + support querying user messages within the specified time period. diff --git a/manila/api/v2/messages.py b/manila/api/v2/messages.py index 0158c7656a..95ab14a65a 100644 --- a/manila/api/v2/messages.py +++ b/manila/api/v2/messages.py @@ -18,6 +18,7 @@ GET /messages/ DELETE /messages/ """ +from oslo_utils import timeutils from six.moves import http_client import webob from webob import exc @@ -26,9 +27,11 @@ from manila.api import common from manila.api.openstack import wsgi from manila.api.views import messages as messages_view from manila import exception +from manila.i18n import _ from manila.message import api as message_api MESSAGES_BASE_MICRO_VERSION = '2.37' +MESSAGES_QUERY_BY_TIMESTAMP = '2.52' class MessagesController(wsgi.Controller): @@ -69,27 +72,57 @@ class MessagesController(wsgi.Controller): return webob.Response(status_int=http_client.NO_CONTENT) - @wsgi.Controller.api_version(MESSAGES_BASE_MICRO_VERSION) + @wsgi.Controller.api_version(MESSAGES_BASE_MICRO_VERSION, '2.51') @wsgi.Controller.authorize('get_all') def index(self, req): """Returns a list of messages, transformed through view builder.""" context = req.environ['manila.context'] + filters = req.params.copy() - search_opts = {} - search_opts.update(req.GET) + params = common.get_pagination_params(req) + limit, offset = [params.get('limit'), params.get('offset')] + sort_key, sort_dir = common.get_sort_params(filters) + filters.pop('created_since', None) + filters.pop('created_before', None) - # Remove keys that are not related to message attrs - search_opts.pop('limit', None) - search_opts.pop('marker', None) - sort_key = search_opts.pop('sort_key', 'created_at') - sort_dir = search_opts.pop('sort_dir', 'desc') + messages = self.message_api.get_all(context, search_opts=filters, + limit=limit, + offset=offset, + sort_key=sort_key, + sort_dir=sort_dir) - messages = self.message_api.get_all( - context, search_opts=search_opts, sort_dir=sort_dir, - sort_key=sort_key) - limited_list = common.limited(messages, req) + return self._view_builder.index(req, messages) - return self._view_builder.index(req, limited_list) + @wsgi.Controller.api_version(MESSAGES_QUERY_BY_TIMESTAMP) # noqa: F811 + @wsgi.Controller.authorize('get_all') + def index(self, req): # pylint: disable=function-redefined + """Returns a list of messages, transformed through view builder.""" + context = req.environ['manila.context'] + filters = req.params.copy() + + params = common.get_pagination_params(req) + limit, offset = [params.get('limit'), params.get('offset')] + sort_key, sort_dir = common.get_sort_params(filters) + + for time_comparison_filter in ['created_since', 'created_before']: + if time_comparison_filter in filters: + time_str = filters.get(time_comparison_filter) + try: + parsed_time = timeutils.parse_isotime(time_str) + except ValueError: + msg = _('Invalid value specified for the query ' + 'key: %s') % time_comparison_filter + raise exc.HTTPBadRequest(explanation=msg) + + filters[time_comparison_filter] = parsed_time + + messages = self.message_api.get_all(context, search_opts=filters, + limit=limit, + offset=offset, + sort_key=sort_key, + sort_dir=sort_dir) + + return self._view_builder.index(req, messages) def create_resource(): diff --git a/manila/common/constants.py b/manila/common/constants.py index ec331f54e2..fab8797772 100644 --- a/manila/common/constants.py +++ b/manila/common/constants.py @@ -13,6 +13,9 @@ # License for the specific language governing permissions and limitations # under the License. +# The maximum value a signed INT type may have +DB_MAX_INT = 0x7FFFFFFF + # SHARE AND GENERAL STATUSES STATUS_CREATING = 'creating' STATUS_DELETING = 'deleting' diff --git a/manila/db/api.py b/manila/db/api.py index 318bf35e6a..fc1e3a9809 100644 --- a/manila/db/api.py +++ b/manila/db/api.py @@ -1411,9 +1411,11 @@ def message_get(context, message_id): return IMPL.message_get(context, message_id) -def message_get_all(context, filters=None, sort_key=None, sort_dir=None): +def message_get_all(context, filters=None, limit=None, offset=None, + sort_key=None, sort_dir=None): """Returns all messages with the project of the specified context.""" - return IMPL.message_get_all(context, filters=filters, sort_key=sort_key, + return IMPL.message_get_all(context, filters=filters, limit=limit, + offset=offset, sort_key=sort_key, sort_dir=sort_dir) diff --git a/manila/db/sqlalchemy/api.py b/manila/db/sqlalchemy/api.py index 62e323bfff..14bd331ab9 100644 --- a/manila/db/sqlalchemy/api.py +++ b/manila/db/sqlalchemy/api.py @@ -50,6 +50,7 @@ from sqlalchemy.sql import func from manila.common import constants from manila.db.sqlalchemy import models +from manila.db.sqlalchemy import utils from manila import exception from manila.i18n import _ from manila import quota @@ -249,7 +250,8 @@ def model_query(context, model, *args, **kwargs): model=model, session=session, args=args, **kwargs) -def exact_filter(query, model, filters, legal_keys): +def exact_filter(query, model, filters, legal_keys, + created_at_key='created_at'): """Applies exact match filtering to a query. Returns the updated query. Modifies filters argument to remove @@ -266,7 +268,7 @@ def exact_filter(query, model, filters, legal_keys): """ filter_dict = {} - + created_at_attr = getattr(model, created_at_key, None) # Walk through all the keys for key in legal_keys: # Skip ones we're not filtering on @@ -276,7 +278,17 @@ def exact_filter(query, model, filters, legal_keys): # OK, filtering on this key; what value do we search for? value = filters.pop(key) - if isinstance(value, (list, tuple, set, frozenset)): + if key == 'created_since' and created_at_attr: + # This is a reserved query parameter to indicate resources created + # after a particular datetime + value = timeutils.normalize_time(value) + query = query.filter(created_at_attr.op('>=')(value)) + elif key == 'created_before' and created_at_attr: + # This is a reserved query parameter to indicate resources created + # before a particular datetime + value = timeutils.normalize_time(value) + query = query.filter(created_at_attr.op('<=')(value)) + elif isinstance(value, (list, tuple, set, frozenset)): # Looking for values in a list; apply to query directly column_attr = getattr(model, key) query = query.filter(column_attr.in_(value)) @@ -5221,28 +5233,49 @@ def message_get(context, message_id): @require_context -def message_get_all(context, filters=None, sort_key='created_at', - sort_dir='asc'): +def message_get_all(context, filters=None, limit=None, offset=None, + sort_key='created_at', sort_dir='desc'): + """Retrieves all messages. + + If no sort parameters are specified then the returned messages are + sorted by the 'created_at' key in descending order. + + :param context: context to query under + :param limit: maximum number of items to return + :param offset: the number of items to skip from the marker or from the + first element. + :param sort_key: attributes by which results should be sorted. + :param sort_dir: directions in which results should be sorted. + :param filters: dictionary of filters; values that are in lists, tuples, + or sets cause an 'IN' operation, while exact matching + is used for other values, see exact_filter function for + more information + :returns: list of matching messages + """ messages = models.Message - query = model_query(context, - messages, - read_deleted="no", - project_only="yes") - legal_filter_keys = ('request_id', 'resource_type', 'resource_id', - 'action_id', 'detail_id', 'message_level') + session = get_session() + with session.begin(): + query = model_query(context, + messages, + read_deleted="no", + project_only="yes") - if not filters: - filters = {} + legal_filter_keys = ('request_id', 'resource_type', 'resource_id', + 'action_id', 'detail_id', 'message_level', + 'created_since', 'created_before') - query = exact_filter(query, messages, filters, legal_filter_keys) - try: - query = apply_sorting(messages, query, sort_key, sort_dir) - except AttributeError: - msg = _("Wrong sorting key provided - '%s'.") % sort_key - raise exception.InvalidInput(reason=msg) + if not filters: + filters = {} - return query.all() + query = exact_filter(query, messages, filters, legal_filter_keys) + + query = utils.paginate_query(query, messages, limit, + sort_key=sort_key, + sort_dir=sort_dir, + offset=offset) + + return query.all() @require_context diff --git a/manila/db/sqlalchemy/utils.py b/manila/db/sqlalchemy/utils.py new file mode 100644 index 0000000000..98fabe132a --- /dev/null +++ b/manila/db/sqlalchemy/utils.py @@ -0,0 +1,51 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +"""Implementation of paginate query.""" + +from manila import exception +import sqlalchemy + + +def paginate_query(query, model, limit, sort_key='created_at', + sort_dir='desc', offset=None): + """Returns a query with sorting / pagination criteria added. + + :param query: the query object to which we should add paging/sorting + :param model: the ORM model class + :param limit: maximum number of items to return + :param sort_key: attributes by which results should be sorted, default is + created_at + :param sort_dir: direction in which results should be sorted (asc, desc) + :param offset: the number of items to skip from the marker or from the + first element. + + :rtype: sqlalchemy.orm.query.Query + :return: The query with sorting/pagination added. + """ + + try: + sort_key_attr = getattr(model, sort_key) + except AttributeError: + raise exception.InvalidInput(reason='Invalid sort key %s' % sort_key) + if sort_dir == 'desc': + query = query.order_by(sqlalchemy.desc(sort_key_attr)) + else: + query = query.order_by(sqlalchemy.asc(sort_key_attr)) + + if limit is not None: + query = query.limit(limit) + + if offset: + query = query.offset(offset) + + return query diff --git a/manila/message/api.py b/manila/message/api.py index 1fe44ac06e..1b7e63c96e 100644 --- a/manila/message/api.py +++ b/manila/message/api.py @@ -73,13 +73,17 @@ class API(base.Base): """Return message with the specified message id.""" return self.db.message_get(context, id) - def get_all(self, context, search_opts={}, sort_key=None, sort_dir=None): + def get_all(self, context, search_opts=None, limit=None, + offset=None, sort_key=None, sort_dir=None): """Return messages for the given context.""" LOG.debug("Searching for messages by: %s", six.text_type(search_opts)) - messages = self.db.message_get_all( - context, filters=search_opts, sort_key=sort_key, sort_dir=sort_dir) + search_opts = search_opts or {} + messages = self.db.message_get_all(context, filters=search_opts, + limit=limit, offset=offset, + sort_key=sort_key, + sort_dir=sort_dir) return messages diff --git a/manila/tests/api/v2/test_messages.py b/manila/tests/api/v2/test_messages.py index 09b505546f..d5c5b8e8a5 100644 --- a/manila/tests/api/v2/test_messages.py +++ b/manila/tests/api/v2/test_messages.py @@ -10,6 +10,8 @@ # License for the specific language governing permissions and limitations # under the License. +import datetime +import iso8601 import mock from oslo_config import cfg import webob @@ -37,8 +39,8 @@ class MessageApiTest(test.TestCase): self.mock_object(policy, 'check_policy', mock.Mock(return_value=True)) - def _expected_message_from_controller(self, id): - message = stubs.stub_message(id) + def _expected_message_from_controller(self, id, **kwargs): + message = stubs.stub_message(id, **kwargs) links = [ {'href': 'http://localhost/v2/fake/messages/%s' % id, 'rel': 'self'}, @@ -170,10 +172,9 @@ class MessageApiTest(test.TestCase): self.assertDictMatch(expected, res_dict) def test_index_with_limit_and_offset(self): - msg1 = stubs.stub_message(fakes.get_fake_uuid()) msg2 = stubs.stub_message(fakes.get_fake_uuid()) self.mock_object(message_api.API, 'get_all', mock.Mock( - return_value=[msg1, msg2])) + return_value=[msg2])) req = fakes.HTTPRequest.blank( '/messages?limit=1&offset=1', version=messages.MESSAGES_BASE_MICRO_VERSION, @@ -184,3 +185,34 @@ class MessageApiTest(test.TestCase): ex2 = self._expected_message_from_controller(msg2['id'])['message'] self.assertEqual([ex2], res_dict['messages']) + + def test_index_with_created_since_and_created_before(self): + msg = stubs.stub_message( + fakes.get_fake_uuid(), + created_at=datetime.datetime(1900, 2, 1, 1, 1, 1, + tzinfo=iso8601.UTC)) + self.mock_object(message_api.API, 'get_all', mock.Mock( + return_value=[msg])) + req = fakes.HTTPRequest.blank( + '/messages?created_since=1900-01-01T01:01:01&' + 'created_before=1900-03-01T01:01:01', + version=messages.MESSAGES_QUERY_BY_TIMESTAMP, + base_url='http://localhost/v2') + req.environ['manila.context'] = self.ctxt + + res_dict = self.controller.index(req) + + ex2 = self._expected_message_from_controller( + msg['id'], + created_at=datetime.datetime(1900, 2, 1, 1, 1, 1, + tzinfo=iso8601.UTC))['message'] + self.assertEqual([ex2], res_dict['messages']) + + def test_index_with_invalid_time_format(self): + req = fakes.HTTPRequest.blank( + '/messages?created_since=invalid_time_str', + version=messages.MESSAGES_QUERY_BY_TIMESTAMP, + base_url='http://localhost/v2') + req.environ['manila.context'] = self.ctxt + self.assertRaises(webob.exc.HTTPBadRequest, + self.controller.index, req) diff --git a/manila/tests/db/sqlalchemy/test_api.py b/manila/tests/db/sqlalchemy/test_api.py index 8568a7adb2..594646d73a 100644 --- a/manila/tests/db/sqlalchemy/test_api.py +++ b/manila/tests/db/sqlalchemy/test_api.py @@ -3627,13 +3627,57 @@ class MessagesDatabaseAPITestCase(test.TestCase): self.assertEqual(2, len(result)) - def test_message_get_all_sorted(self): + def test_message_get_all_with_created_since_or_before_filter(self): + now = timeutils.utcnow() + db_utils.create_message(project_id=self.project_id, + action_id='001', + created_at=now - datetime.timedelta(seconds=1)) + db_utils.create_message(project_id=self.project_id, + action_id='001', + created_at=now + datetime.timedelta(seconds=1)) + db_utils.create_message(project_id=self.project_id, + action_id='001', + created_at=now + datetime.timedelta(seconds=2)) + result1 = db_api.message_get_all(self.ctxt, + filters={'created_before': now}) + result2 = db_api.message_get_all(self.ctxt, + filters={'created_since': now}) + self.assertEqual(1, len(result1)) + self.assertEqual(2, len(result2)) + + def test_message_get_all_with_invalid_sort_key(self): + self.assertRaises(exception.InvalidInput, db_api.message_get_all, + self.ctxt, sort_key='invalid_key') + + def test_message_get_all_sorted_asc(self): ids = [] for i in ['001', '002', '003']: msg = db_utils.create_message(project_id=self.project_id, action_id=i) ids.append(msg.id) + result = db_api.message_get_all(self.ctxt, + sort_key='action_id', + sort_dir='asc') + result_ids = [r.id for r in result] + self.assertEqual(result_ids, ids) + + def test_message_get_all_with_limit_and_offset(self): + for i in ['001', '002']: + db_utils.create_message(project_id=self.project_id, + action_id=i) + + result = db_api.message_get_all(self.ctxt, limit=1, offset=1) + self.assertEqual(1, len(result)) + + def test_message_get_all_sorted(self): + ids = [] + for i in ['003', '002', '001']: + msg = db_utils.create_message(project_id=self.project_id, + action_id=i) + ids.append(msg.id) + + # Default the sort direction to descending result = db_api.message_get_all(self.ctxt, sort_key='action_id') result_ids = [r.id for r in result] self.assertEqual(result_ids, ids) diff --git a/manila/tests/message/test_api.py b/manila/tests/message/test_api.py index 08a0874c6a..75275bcade 100644 --- a/manila/tests/message/test_api.py +++ b/manila/tests/message/test_api.py @@ -85,7 +85,8 @@ class MessageApiTest(test.TestCase): self.message_api.get_all(self.ctxt) self.message_api.db.message_get_all.assert_called_once_with( - self.ctxt, filters={}, sort_dir=None, sort_key=None) + self.ctxt, filters={}, limit=None, offset=None, + sort_dir=None, sort_key=None) def test_delete(self): self.message_api.delete(self.ctxt, 'fake_id') diff --git a/releasenotes/notes/bp-support-query-user-message-by-timestamp-c0a02b3b3e337e12.yaml b/releasenotes/notes/bp-support-query-user-message-by-timestamp-c0a02b3b3e337e12.yaml new file mode 100644 index 0000000000..3c6806bc93 --- /dev/null +++ b/releasenotes/notes/bp-support-query-user-message-by-timestamp-c0a02b3b3e337e12.yaml @@ -0,0 +1,5 @@ +--- +features: + - User messages can be queried by timestamp with query keys + ``created_since`` and ``created_before`` starting with API + version ``2.52``.