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