Merge "add mandatory limit value to complex query list"5.0.0.0b3

5 files changed, 29 insertions, 17 deletions

diff --git a/ceilometer/api/app.py b/ceilometer/api/app.py
index 020e5563..468d6d13 100644
--- a/ceilometer/api/app.py
+++ b/ceilometer/api/app.py
@@ -44,7 +44,8 @@ API_OPTS = [
                 default=False,
                 help='Toggle Pecan Debug Middleware.'),
     cfg.IntOpt('default_api_return_limit',
-               default=1000,
+               min=1,
+               default=100,
                help='Default maximum number of items returned by API request.'
                ),
 ]
diff --git a/ceilometer/api/controllers/v2/query.py b/ceilometer/api/controllers/v2/query.py
index c9f283bd..9b2b42a9 100644
--- a/ceilometer/api/controllers/v2/query.py
+++ b/ceilometer/api/controllers/v2/query.py
@@ -32,6 +32,7 @@ from ceilometer.alarm.storage import models as alarm_models
 from ceilometer.api.controllers.v2 import alarms
 from ceilometer.api.controllers.v2 import base
 from ceilometer.api.controllers.v2 import samples
+from ceilometer.api.controllers.v2 import utils as v2_utils
 from ceilometer.api import rbac
 from ceilometer.i18n import _
 from ceilometer import storage
@@ -220,14 +221,10 @@ class ValidatedComplexQuery(object):
             self._convert_orderby_to_lower_case(self.orderby)
             self._normalize_field_names_in_orderby(self.orderby)
 
-        if self.original_query.limit is wtypes.Unset:
-            self.limit = None
-        else:
-            self.limit = self.original_query.limit
+        self.limit = (None if self.original_query.limit is wtypes.Unset
+                      else self.original_query.limit)
 
-        if self.limit is not None and self.limit <= 0:
-            msg = _('Limit should be positive')
-            raise base.ClientSideError(msg)
+        self.limit = v2_utils.enforce_limit(self.limit)
 
     @staticmethod
     def _convert_orderby_to_lower_case(orderby):
diff --git a/ceilometer/api/controllers/v2/utils.py b/ceilometer/api/controllers/v2/utils.py
index 33f84f34..0e2ae8ee 100644
--- a/ceilometer/api/controllers/v2/utils.py
+++ b/ceilometer/api/controllers/v2/utils.py
@@ -46,7 +46,7 @@ def enforce_limit(limit):
             limit = cfg.CONF.api.default_api_return_limit
             LOG.info(_LI('No limit value provided, result set will be'
                          'limited to %(limit)d.'), {'limit': limit})
-        if limit and limit < 0:
+        if not limit or limit <= 0:
             raise base.ClientSideError(_("Limit must be positive"))
         return limit
 
diff --git a/ceilometer/tests/functional/api/v2/test_complex_query_scenarios.py b/ceilometer/tests/functional/api/v2/test_complex_query_scenarios.py
index 95c512cb..bc4fb737 100644
--- a/ceilometer/tests/functional/api/v2/test_complex_query_scenarios.py
+++ b/ceilometer/tests/functional/api/v2/test_complex_query_scenarios.py
@@ -304,13 +304,18 @@ class TestQueryMetersController(tests_api.FunctionalTest,
         for sample_item in data.json:
             self.assertTrue(float(sample_item["metadata"]["util"]) < 0.5)
 
-    def test_limit_should_be_positive(self):
+    def test_limit_must_be_positive(self):
         data = self.post_json(self.url,
                               params={"limit": 0},
                               expect_errors=True)
 
         self.assertEqual(400, data.status_int)
-        self.assertIn(b"Limit should be positive", data.body)
+        self.assertIn(b"Limit must be positive", data.body)
+
+    def test_default_limit(self):
+        self.CONF.set_override('default_api_return_limit', 1, group='api')
+        data = self.post_json(self.url, params={})
+        self.assertEqual(1, len(data.json))
 
 
 class TestQueryAlarmsController(tests_api.FunctionalTest,
@@ -471,13 +476,18 @@ class TestQueryAlarmsController(tests_api.FunctionalTest,
         for alarm in data.json:
             self.assertEqual("alarm", alarm["state"])
 
-    def test_limit_should_be_positive(self):
+    def test_limit_must_be_positive(self):
         data = self.post_json(self.alarm_url,
                               params={"limit": 0},
                               expect_errors=True)
 
         self.assertEqual(400, data.status_int)
-        self.assertIn(b"Limit should be positive", data.body)
+        self.assertIn(b"Limit must be positive", data.body)
+
+    def test_default_limit(self):
+        self.CONF.set_override('default_api_return_limit', 1, group='api')
+        data = self.post_json(self.alarm_url, params={})
+        self.assertEqual(1, len(data.json))
 
 
 class TestQueryAlarmsHistoryController(
@@ -591,10 +601,15 @@ class TestQueryAlarmsHistoryController(
         for history in data.json:
             self.assertEqual("creation", history['type'])
 
-    def test_limit_should_be_positive(self):
+    def test_limit_must_be_positive(self):
         data = self.post_json(self.url,
                               params={"limit": 0},
                               expect_errors=True)
 
         self.assertEqual(400, data.status_int)
-        self.assertIn(b"Limit should be positive", data.body)
+        self.assertIn(b"Limit must be positive", data.body)
+
+    def test_default_limit(self):
+        self.CONF.set_override('default_api_return_limit', 1, group='api')
+        data = self.post_json(self.url, params={})
+        self.assertEqual(1, len(data.json))
diff --git a/ceilometer/tests/functional/gabbi/gabbits/samples.yaml b/ceilometer/tests/functional/gabbi/gabbits/samples.yaml
index 11ab5fa8..be568c32 100644
--- a/ceilometer/tests/functional/gabbi/gabbits/samples.yaml
+++ b/ceilometer/tests/functional/gabbi/gabbits/samples.yaml
@@ -38,8 +38,7 @@ tests:
 
     - name: list zero samples with zero limit
       url: /v2/samples?limit=0
-      response_strings:
-          - '[]'
+      status: 400
 
     - name: list samples with query
       url: /v2/samples?q.field=resource_metadata.breed&q.value=cow&q.op=eq