Merge "Refactor api.catalog.search() method to provide 'next_marker' value"

etc/murano/murano.conf.sample

@@ -17,6 +17,14 @@ bind_port = 8082
 # Maximum application package size, Mb
 package_size_limit = 5
 
+# If a `limit` query param is not provided in an api request, it will
+# default to `limit_param_default`
+limit_param_default = 20
+
+# Limit the api to return `api_limit_max` items in a call to a container. If
+# a larger `limit` query param is provided, it will be reduced to this value.
+api_limit_max = 100
+
 # Set up logging. Make sure the user has permissions to write to this file! To use syslog just set use_syslog parameter value to 'True'.
 log_file = /tmp/murano-api.log
 

murano/api/v1/catalog.py

@@ -82,14 +82,14 @@ def _validate_body(body):
     reset the file position to the beginning
     """
     def check_file_size(f):
-        pkg_size_limit = CONF.package_size_limit * 1024 * 1024
+        mb_limit = CONF.packages_opts.package_size_limit
+        pkg_size_limit = mb_limit * 1024 * 1024
         f.seek(0, 2)
         size = f.tell()
         f.seek(0)
         if size > pkg_size_limit:
             raise exc.HTTPBadRequest('Uploading file is too large.'
-                                     ' The limit is {0}'
+                                     ' The limit is {0} Mb'.format(mb_limit))
-                                     ' Mb'.format(CONF.package_size_limit))
 
     if len(body.keys()) != 2:
         msg = _("'multipart/form-data' request body should contain "
@@ -148,9 +148,35 @@ class Controller(object):
         return package.to_dict()
 
     def search(self, req):
+        def _validate_limit(value):
+            if value is None:
+                return
+            try:
+                value = int(value)
+            except ValueError:
+                msg = _("limit param must be an integer")
+                LOG.error(msg)
+                raise exc.HTTPBadRequest(explanation=msg)
+
+            if value <= 0:
+                msg = _("limit param must be positive")
+                LOG.error(msg)
+                raise exc.HTTPBadRequest(explanation=msg)
+
+            return value
+
         filters = _get_filters(req.GET.items())
-        packages = db_api.package_search(filters, req.context)
+        limit = _validate_limit(filters.get('limit'))
-        return {"packages": [package.to_dict() for package in packages]}
+        if limit is None:
+            limit = CONF.packages_opts.limit_param_default
+        limit = min(CONF.packages_opts.api_limit_max, limit)
+
+        result = {}
+        packages = db_api.package_search(filters, req.context, limit)
+        if len(packages) == limit:
+            result['next_marker'] = packages[-1].id
+        result['packages'] = [package.to_dict() for package in packages]
+        return result
 
     def upload(self, req, body=None):
         """

murano/common/config.py

@@ -103,9 +103,13 @@ stats_opt = [
 metadata_dir = cfg.StrOpt('metadata-dir', default='./meta')
 
 temp_pkg_cache = os.path.join(tempfile.gettempdir(), 'murano-packages-cache')
-packages_cache = cfg.StrOpt('packages-cache', default=temp_pkg_cache)
 
-package_size_limit = cfg.IntOpt('package_size_limit', default=5)
+packages_opts = [
+    cfg.StrOpt('packages_cache', default=temp_pkg_cache),
+    cfg.IntOpt('package_size_limit', default=5),
+    cfg.IntOpt('limit_param_default', default=20),
+    cfg.IntOpt('api_limit_max', default=100)
+]
 
 CONF = cfg.CONF
 CONF.register_opts(paste_deploy_opts, group='paste_deploy')
@@ -119,8 +123,7 @@ CONF.register_opts(murano_opts, group='murano')
 CONF.register_opt(cfg.StrOpt('file_server'))
 CONF.register_cli_opt(cfg.StrOpt('murano_metadata_url'))
 CONF.register_cli_opt(metadata_dir)
-CONF.register_cli_opt(packages_cache)
+CONF.register_opts(packages_opts, group='packages_opts')
-CONF.register_cli_opt(package_size_limit)
 CONF.register_opts(stats_opt, group='stats')
 CONF.register_opts(networking_opts, group='networking')
 

murano/db/catalog/api.py

@@ -12,16 +12,19 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+from oslo.config import cfg
 from sqlalchemy import or_
 from sqlalchemy.orm import attributes
-from sqlalchemy import sql
 from webob import exc
 
 from murano.db import models
 from murano.db import session as db_session
+from murano.openstack.common.db.sqlalchemy import utils
 from murano.openstack.common.gettextutils import _  # noqa
 from murano.openstack.common import log as logging
 
+CONF = cfg.CONF
+
 SEARCH_MAPPING = {'fqn': 'fully_qualified_name',
                   'name': 'name',
                   'created': 'created'
@@ -209,41 +212,18 @@ def package_update(pkg_id, changes, context):
     return pkg
 
 
-def package_search(filters, context):
+def package_search(filters, context, limit=None):
     """
     Search packages with different filters
       * Admin is allowed to browse all the packages
       * Regular user is allowed to browse all packages belongs to user tenant
         and all other packages marked is_public.
         Also all packages should be enabled.
-      * Use marker and limit for pagination:
+      * Use marker (inside filters param) and limit for pagination:
         The typical pattern of limit and marker is to make an initial limited
         request and then to use the ID of the last package from the response
         as the marker parameter in a subsequent limited request.
     """
-
-    def _validate_limit(value):
-        if value is None:
-            return
-        try:
-            value = int(value)
-        except ValueError:
-            msg = _("limit param must be an integer")
-            LOG.error(msg)
-            raise exc.HTTPBadRequest(explanation=msg)
-
-        if value < 0:
-            msg = _("limit param must be positive")
-            LOG.error(msg)
-            raise exc.HTTPBadRequest(explanation=msg)
-
-        return value
-
-    def get_pkg_attr(package_obj, search_attr_name):
-        return getattr(package_obj, SEARCH_MAPPING[search_attr_name])
-
-    limit = _validate_limit(filters.get('limit'))
-
     session = db_session.get_session()
     pkg = models.Package
 
@@ -323,47 +303,12 @@ def package_search(filters, context):
                         conditions.append(getattr(pkg, attr).like(_word))
         query = query.filter(or_(*conditions))
 
-    sort_keys = filters.get('order_by', []) or ['created']
+    sort_keys = [SEARCH_MAPPING[sort_key] for sort_key in
-    for key in sort_keys:
+                 filters.get('order_by', []) or ['created']]
-        query = query.order_by(get_pkg_attr(pkg, key))
-
     marker = filters.get('marker')
-    if marker is not None:
+    if marker is not None:  # set marker to real object instead of its id
-        # Note(efedorova): Copied from Glance
+        marker = _package_get(marker, session)
-        #   Pagination works by requiring a unique sort_key, specified by sort_
+    query = utils.paginate_query(query, pkg, limit, sort_keys, marker)
-        #   keys. (If sort_keys is not unique, then we risk looping through
-        #   values.) We use the last row in the previous page as the 'marker'
-        #   for pagination. So we must return values that follow the passed
-        #   marker in the order. With a single-valued sort_key, this would
-        #   be easy: sort_key > X. With a compound-values sort_key,
-        #   (k1, k2, k3) we must do this to repeat the lexicographical
-        #   ordering: (k1 > X1) or (k1 == X1 && k2 > X2) or
-        #   (k1 == X1 && k2 == X2 && k3 > X3)
-
-        model_marker = _package_get(marker, session)
-        marker_values = []
-        for sort_key in sort_keys:
-            v = getattr(model_marker, sort_key)
-            marker_values.append(v)
-
-        # Build up an array of sort criteria as in the docstring
-        criteria_list = []
-        for i in range(len(sort_keys)):
-            crit_attrs = []
-            for j in range(i):
-                model_attr = get_pkg_attr(pkg, sort_keys[j])
-                crit_attrs.append((model_attr == marker_values[j]))
-
-            model_attr = get_pkg_attr(pkg, sort_keys[i])
-            crit_attrs.append((model_attr > marker_values[i]))
-
-            criteria = sql.and_(*crit_attrs)
-            criteria_list.append(criteria)
-            f = sql.or_(*criteria_list)
-            query = query.filter(f)
-
-    if limit is not None:
-        query = query.limit(limit)
 
     return query.all()
 

murano/engine/package_loader.py

@@ -83,7 +83,8 @@ class ApiPackageLoader(PackageLoader):
 
     @staticmethod
     def _get_cache_directory():
-        directory = os.path.join(config.CONF.packages_cache, str(uuid.uuid4()))
+        directory = os.path.join(config.CONF.packages_opts.packages_cache,
+                                 str(uuid.uuid4()))
         directory = os.path.abspath(directory)
         os.makedirs(directory)