set default limit to meter/sample queries

this patch makes existing limit options in meter and samples apis
mandatory. if not set explicitly, a default limit will be applied.

DocImpact
ApiImpact

Change-Id: Ie354e8e64a6f8677a0be7e75c34e8901cdffbc6a
Implements: blueprint mandatory-limit

ceilometer/api/app.py

@@ -46,6 +46,10 @@ API_OPTS = [
     cfg.BoolOpt('pecan_debug',
                 default=False,
                 help='Toggle Pecan Debug Middleware.'),
+    cfg.IntOpt('default_api_return_limit',
+               default=1000,
+               help='Default maximum number of items returned by API request.'
+               ),
 ]
 
 CONF.register_opts(OPTS)

ceilometer/api/controllers/v2/meters.py

@@ -285,8 +285,7 @@ class MeterController(rest.RestController):
         rbac.enforce('get_samples', pecan.request)
 
         q = q or []
-        if limit and limit < 0:
+        limit = v2_utils.enforce_limit(limit)
-            raise base.ClientSideError(_("Limit must be positive"))
         kwargs = v2_utils.query_to_kwargs(q, storage.SampleFilter.__init__)
         kwargs['meter'] = self.meter_name
         f = storage.SampleFilter(**kwargs)

ceilometer/api/controllers/v2/samples.py

@@ -122,8 +122,7 @@ class SamplesController(rest.RestController):
 
         q = q or []
 
-        if limit and limit < 0:
+        limit = utils.enforce_limit(limit)
-            raise base.ClientSideError(_("Limit must be positive"))
         kwargs = utils.query_to_kwargs(q, storage.SampleFilter.__init__)
         f = storage.SampleFilter(**kwargs)
         return map(Sample.from_db_model,

ceilometer/api/controllers/v2/utils.py

@@ -23,6 +23,8 @@ import datetime
 import functools
 import inspect
 
+from oslo_config import cfg
+from oslo_log import log
 from oslo_utils import timeutils
 import pecan
 import six
@@ -30,8 +32,24 @@ import wsme
 
 from ceilometer.api.controllers.v2 import base
 from ceilometer.api import rbac
+from ceilometer.i18n import _, _LI
 from ceilometer import utils
 
+LOG = log.getLogger(__name__)
+cfg.CONF.import_opt('default_api_return_limit', 'ceilometer.api.app',
+                    group='api')
+
+
+def enforce_limit(limit):
+        """Ensure limit is defined and is valid. if not, set a default."""
+        if limit is None:
+            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:
+            raise base.ClientSideError(_("Limit must be positive"))
+        return limit
+
 
 def get_auth_project(on_behalf_of=None):
     # when an alarm is created by an admin on behalf of another tenant

ceilometer/tests/api/v2/test_list_events_scenarios.py

@@ -20,7 +20,6 @@ import datetime
 import mock
 from oslo_utils import timeutils
 import six
-import webtest.app
 
 from ceilometer.publisher import utils
 from ceilometer import sample
@@ -87,19 +86,6 @@ class TestListEvents(v2.FunctionalTest,
         data = self.get_json('/meters/instance/')
         self.assertEqual(2, len(data))
 
-    def test_all_limit(self):
-        data = self.get_json('/meters/instance?limit=1')
-        self.assertEqual(1, len(data))
-
-    def test_all_limit_negative(self):
-        self.assertRaises(webtest.app.AppError,
-                          self.get_json,
-                          '/meters/instance?limit=-2')
-
-    def test_all_limit_bigger(self):
-        data = self.get_json('/meters/instance?limit=42')
-        self.assertEqual(2, len(data))
-
     def test_empty_project(self):
         data = self.get_json('/meters/instance',
                              q=[{'field': 'project_id',

ceilometer/tests/api/v2/test_list_meters_scenarios.py

@@ -61,6 +61,68 @@ class TestValidateUserInput(v2.FunctionalTest,
                               'type': 'integer'}])
 
 
+class TestListMetersRestriction(v2.FunctionalTest,
+                                tests_db.MixinTestsWithBackendScenarios):
+
+    def setUp(self):
+        super(TestListMetersRestriction, self).setUp()
+        self.CONF.set_override('default_api_return_limit', 10, group='api')
+        for i in range(20):
+            s = sample.Sample(
+                'volume.size',
+                'gauge',
+                'GiB',
+                5 + i,
+                'user-id',
+                'project1',
+                'resource-id',
+                timestamp=(datetime.datetime(2012, 9, 25, 10, 30) +
+                           datetime.timedelta(seconds=i)),
+                resource_metadata={'display_name': 'test-volume',
+                                   'tag': 'self.sample',
+                                   },
+                source='source1',
+            )
+            msg = utils.meter_message_from_counter(
+                s, self.CONF.publisher.telemetry_secret,
+            )
+            self.conn.record_metering_data(msg)
+
+    def test_meter_limit(self):
+        data = self.get_json('/meters/volume.size?limit=1')
+        self.assertEqual(1, len(data))
+
+    def test_meter_limit_negative(self):
+        self.assertRaises(webtest.app.AppError,
+                          self.get_json,
+                          '/meters/volume.size?limit=-2')
+
+    def test_meter_limit_bigger(self):
+        data = self.get_json('/meters/volume.size?limit=42')
+        self.assertEqual(20, len(data))
+
+    def test_meter_default_limit(self):
+        data = self.get_json('/meters/volume.size')
+        self.assertEqual(10, len(data))
+
+    def test_sample_limit(self):
+        data = self.get_json('/samples?limit=1')
+        self.assertEqual(1, len(data))
+
+    def test_sample_limit_negative(self):
+        self.assertRaises(webtest.app.AppError,
+                          self.get_json,
+                          '/samples?limit=-2')
+
+    def test_sample_limit_bigger(self):
+        data = self.get_json('/samples?limit=42')
+        self.assertEqual(20, len(data))
+
+    def test_sample_default_limit(self):
+        data = self.get_json('/samples')
+        self.assertEqual(10, len(data))
+
+
 class TestListMeters(v2.FunctionalTest,
                      tests_db.MixinTestsWithBackendScenarios):