openstack
/
designate
Code Issues Proposed changes

Fix #1494799 handle limit=max on v2 and Admin APIs 

Handle mixed case "max", raise ValueError on unexpected strings
on v2 and Admin APIs.
Add unit tests.

Change-Id: I2cf8d11bd2f4bc4bdff9d67812f6eb7437eb59ae
Closes-bug: 1494799
This commit is contained in:
Federico Ceratto 2015-09-22 11:56:45 +01:00
parent 78a83cca4a
commit 9d00e50e9d
6 changed files with 179 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
designate/api/admin/__init__.py
View File

@ -21,6 +21,11 @@ LOG = logging.getLogger(__name__)
OPTS = [
cfg.ListOpt('enabled-extensions-admin', default=[],
help='Enabled Admin API Extensions'),
cfg.IntOpt('default-limit-admin', default=20,
help='Default per-page limit for the Admin API, a value of None'
' means show all results by default'),
cfg.IntOpt('max-limit-admin', default=1000,
help='Max per-page limit for the Admin API'),
]
cfg.CONF.register_opts(OPTS, group='service:api')

25
designate/api/admin/views/base.py
View File

@ -128,21 +128,36 @@ class BaseView(object):
# when there are more/previous items.. This is what nova
# does.. But I think we can do better.
params = request.GET
# Duplicated code, see bug 1498432
result = {
links = {
"self": self._get_collection_href(request, parents),
}
params = request.GET
# See above
# if 'marker' in params:
# result['previous'] = self._get_previous_href(request, items,
# parents)
if 'limit' in params and int(params['limit']) == len(items):
result['next'] = self._get_next_href(request, items, parents)
# defined in etc/designate/designate.conf.sample
limit = cfg.CONF['service:api'].default_limit_admin
return result
if 'limit' in params:
limit = params['limit']
if limit.lower() == 'max':
limit = cfg.CONF['service:api'].max_limit_admin
else:
try:
limit = int(limit)
except ValueError:
raise exceptions.ValueError(
"'limit' should be an integer or 'max'")
if limit is not None and limit == len(items):
links['next'] = self._get_next_href(request, items)
return links
def _get_base_href(self, parents=None):
href = "%s/v2/%s" % (self.base_uri, self._collection_name)

25
designate/objects/adapters/api_v2/base.py
View File

@ -18,6 +18,7 @@ from oslo.config import cfg
from designate.objects.adapters import base
from designate.objects import base as obj_base
from designate import exceptions
LOG = logging.getLogger(__name__)
@ -92,23 +93,31 @@ class APIv2Adapter(base.DesignateAdapter):
item_path += '/' + part
@classmethod
def _get_collection_links(cls, list, request):
def _get_collection_links(cls, item_list, request):
links = {
'self': cls._get_collection_href(request)
}
params = request.GET
# defined in etc/designate/designate.conf.sample
limit = cfg.CONF['service:api'].default_limit_v2
if 'limit' in params and params['limit'] == 'max':
limit = cfg.CONF['service:api'].max_limit_v2
if 'limit' in params:
limit = params['limit']
if limit.lower() == 'max':
limit = cfg.CONF['service:api'].max_limit_v2
else:
try:
limit = int(limit)
except ValueError:
raise exceptions.ValueError(
"'limit' should be an integer or 'max'")
elif 'limit' in params:
limit = int(params['limit'])
if limit is not None and limit == len(list):
links['next'] = cls._get_next_href(request, list)
# Bug: this creates a link to "next" even on the last page if
# len(item_list) happens to be == limit
if limit is not None and limit == len(item_list):
links['next'] = cls._get_next_href(request, item_list)
return links

67
designate/tests/unit/test_api/test_admin_api.py Normal file
View File

@ -0,0 +1,67 @@
# Copyright 2015 Hewlett-Packard Development Company, L.P.
#
# Author: Federico Ceratto <federico.ceratto@hpe.com>
#
# 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.
import mock
from designate.tests import TestCase
from designate import exceptions
from oslo_config import cfg
from oslo_config import fixture as cfg_fixture
from designate.api.admin.views import base
class MockRequest(object):
def __init__(self, GET=None):
self.GET = GET
class TestAdminAPI(TestCase):
def setUp(self):
super(TestCase, self).setUp()
self.CONF = self.useFixture(cfg_fixture.Config(cfg.CONF)).conf
@mock.patch.object(base.BaseView, '_get_collection_href')
@mock.patch.object(base.BaseView, '_get_next_href')
def test_limit_max(self, mock_coll_href, mock_next_href):
# Bug 1494799
# The code being tested should be deduplicated, see bug 1498432
mock_coll_href.return_value = None
mock_next_href.return_value = None
item_list = range(200)
bv = base.BaseView()
request = MockRequest(GET=dict(limit="max"))
links = bv._get_collection_links(request, item_list)
self.assertDictEqual(links, dict(self=None))
request = MockRequest(GET=dict(limit="MAX"))
links = bv._get_collection_links(request, item_list)
self.assertDictEqual(links, dict(self=None))
request = MockRequest(GET=dict(limit="200"))
links = bv._get_collection_links(request, item_list)
self.assertDictEqual(links, dict(self=None, next=None))
request = MockRequest(GET=dict(limit="BOGUS_STRING"))
self.assertRaises(
exceptions.ValueError,
bv._get_collection_links, request, item_list
)

63
designate/tests/unit/test_api/test_api_v2.py Normal file
View File

@ -0,0 +1,63 @@
# Copyright 2015 Hewlett-Packard Development Company, L.P.
#
# Author: Federico Ceratto <federico.ceratto@hpe.com>
#
# 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.
import mock
from designate.tests import TestCase
from designate import exceptions
from oslo_config import cfg
from oslo_config import fixture as cfg_fixture
from designate.objects.adapters.api_v2 import base
class MockRequest(object):
def __init__(self, GET=None):
self.GET = GET
class TestAPIv2(TestCase):
def setUp(self):
super(TestCase, self).setUp()
self.CONF = self.useFixture(cfg_fixture.Config(cfg.CONF)).conf
@mock.patch.object(base.APIv2Adapter, '_get_collection_href')
@mock.patch.object(base.APIv2Adapter, '_get_next_href')
def test_limit_max(self, mock_coll_href, mock_next_href):
# Bug 1494799 bug:1494799
mock_coll_href.return_value = None
mock_next_href.return_value = None
item_list = range(200)
request = MockRequest(GET=dict(limit="max"))
links = base.APIv2Adapter._get_collection_links(item_list, request)
self.assertDictEqual(links, dict(self=None))
request = MockRequest(GET=dict(limit="MAX"))
links = base.APIv2Adapter._get_collection_links(item_list, request)
self.assertDictEqual(links, dict(self=None))
request = MockRequest(GET=dict(limit="200"))
links = base.APIv2Adapter._get_collection_links(item_list, request)
self.assertDictEqual(links, dict(self=None, next=None))
request = MockRequest(GET=dict(limit="BOGUS_STRING"))
self.assertRaises(
exceptions.ValueError,
base.APIv2Adapter._get_collection_links, item_list, request
)

7
etc/designate/designate.conf.sample
View File

@ -105,6 +105,13 @@ debug = False
# zone import / export is in zones extension
#enabled_extensions_admin =
# Default per-page limit for the Admin API, a value of None means show all results
# by default
#default_limit_admin = 20
# Max page size in the Admin API
#max_limit_admin = 1000
# Show the pecan HTML based debug interface (v2 only)
# This is only useful for development, and WILL break python-designateclient
# if an error occurs