Merge "Support query user message by timestamp"

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.
 

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
 
 

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.

manila/api/v2/messages.py

@@ -18,6 +18,7 @@ GET /messages/<message_id>
 DELETE /messages/<message_id>
 """
 
+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 = {}
+        params = common.get_pagination_params(req)
-        search_opts.update(req.GET)
+        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
+        messages = self.message_api.get_all(context, search_opts=filters,
-        search_opts.pop('limit', None)
+                                            limit=limit,
-        search_opts.pop('marker', None)
+                                            offset=offset,
-        sort_key = search_opts.pop('sort_key', 'created_at')
+                                            sort_key=sort_key,
-        sort_dir = search_opts.pop('sort_dir', 'desc')
+                                            sort_dir=sort_dir)
 
-        messages = self.message_api.get_all(
+        return self._view_builder.index(req, messages)
-            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, 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():

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'

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)
 
 

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,26 +5233,47 @@ def message_get(context, message_id):
 
 
 @require_context
-def message_get_all(context, filters=None, sort_key='created_at',
+def message_get_all(context, filters=None, limit=None, offset=None,
-                    sort_dir='asc'):
+                    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
+
+    session = get_session()
+    with session.begin():
         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')
+                             'action_id', 'detail_id', 'message_level',
+                             'created_since', 'created_before')
 
         if not filters:
             filters = {}
 
         query = exact_filter(query, messages, filters, legal_filter_keys)
-    try:
+
-        query = apply_sorting(messages, query, sort_key, sort_dir)
+        query = utils.paginate_query(query, messages, limit,
-    except AttributeError:
+                                     sort_key=sort_key,
-        msg = _("Wrong sorting key provided - '%s'.") % sort_key
+                                     sort_dir=sort_dir,
-        raise exception.InvalidInput(reason=msg)
+                                     offset=offset)
 
         return query.all()
 

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

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(
+        search_opts = search_opts or {}
-            context, filters=search_opts, sort_key=sort_key, sort_dir=sort_dir)
+        messages = self.db.message_get_all(context, filters=search_opts,
+                                           limit=limit, offset=offset,
+                                           sort_key=sort_key,
+                                           sort_dir=sort_dir)
 
         return messages
 

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):
+    def _expected_message_from_controller(self, id, **kwargs):
-        message = stubs.stub_message(id)
+        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)

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)

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')

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``.