Fix negative or 0 limit parameter in pagination
Octavia replace "limit" with None when it is less 1. (for example 0, -1) However the further code failed to compare None and int values. This patch fixes it by validation, that limit is None. Co-Authored-By: Roman Goncharov <gadzhet007@gmail.com> Closes-Bug: #2060917 Change-Id: I9bb45a1aca6b7b18644752a3dccc3ebfb7c106ef
This commit is contained in:
@@ -169,7 +169,7 @@ class PaginationHelper:
|
|||||||
# TODO(rm_work) Do we need to know when there are more vs exact?
|
# TODO(rm_work) Do we need to know when there are more vs exact?
|
||||||
# We safely know if we have a full page, but it might include the
|
# We safely know if we have a full page, but it might include the
|
||||||
# last element or it might not, it is unclear
|
# last element or it might not, it is unclear
|
||||||
if len(model_list) >= self.limit:
|
if self.limit is None or len(model_list) >= self.limit:
|
||||||
next_attr.append("marker={}".format(model_list[-1].get('id')))
|
next_attr.append("marker={}".format(model_list[-1].get('id')))
|
||||||
next_link = {
|
next_link = {
|
||||||
"rel": "next",
|
"rel": "next",
|
||||||
|
@@ -228,8 +228,9 @@ class TestPaginationHelper(base.TestCase):
|
|||||||
links = helper._make_links(model_list)
|
links = helper._make_links(model_list)
|
||||||
self.assertEqual(links[0].rel, "previous")
|
self.assertEqual(links[0].rel, "previous")
|
||||||
self.assertEqual(
|
self.assertEqual(
|
||||||
links[1].href,
|
links[0].href,
|
||||||
"{base_uri}{path}?limit={limit}&marker={marker}".format(
|
("{base_uri}{path}?limit={limit}&marker={marker}"
|
||||||
|
"&page_reverse=True").format(
|
||||||
base_uri=api_base_uri,
|
base_uri=api_base_uri,
|
||||||
path=request_mock.path,
|
path=request_mock.path,
|
||||||
limit=params['limit'],
|
limit=params['limit'],
|
||||||
@@ -243,3 +244,69 @@ class TestPaginationHelper(base.TestCase):
|
|||||||
path=request_mock.path,
|
path=request_mock.path,
|
||||||
limit=params['limit'],
|
limit=params['limit'],
|
||||||
marker=member1.id))
|
marker=member1.id))
|
||||||
|
|
||||||
|
@mock.patch('octavia.api.common.pagination.request')
|
||||||
|
def test_make_links_with_zero_limit(self, request_mock):
|
||||||
|
request_mock.path = "/lbaas/v2/pools/1/members"
|
||||||
|
request_mock.path_url = "http://localhost" + request_mock.path
|
||||||
|
api_base_uri = "https://127.0.0.1"
|
||||||
|
conf = self.useFixture(oslo_fixture.Config(cfg.CONF))
|
||||||
|
conf.config(group='api_settings', api_base_uri=api_base_uri)
|
||||||
|
member1 = models.Member()
|
||||||
|
member1.id = uuidutils.generate_uuid()
|
||||||
|
model_list = [member1]
|
||||||
|
|
||||||
|
params = {'limit': 0, 'marker': member1.id}
|
||||||
|
helper = pagination.PaginationHelper(params)
|
||||||
|
links = helper._make_links(model_list)
|
||||||
|
self.assertEqual(links[0].rel, "previous")
|
||||||
|
self.assertEqual(
|
||||||
|
links[0].href,
|
||||||
|
("{base_uri}{path}?limit={limit}&marker={marker}"
|
||||||
|
"&page_reverse=True").format(
|
||||||
|
base_uri=api_base_uri,
|
||||||
|
path=request_mock.path,
|
||||||
|
limit=None,
|
||||||
|
marker=member1.id
|
||||||
|
))
|
||||||
|
self.assertEqual(links[1].rel, "next")
|
||||||
|
self.assertEqual(
|
||||||
|
links[1].href,
|
||||||
|
"{base_uri}{path}?limit={limit}&marker={marker}".format(
|
||||||
|
base_uri=api_base_uri,
|
||||||
|
path=request_mock.path,
|
||||||
|
limit=None,
|
||||||
|
marker=member1.id))
|
||||||
|
|
||||||
|
@mock.patch('octavia.api.common.pagination.request')
|
||||||
|
def test_make_links_with_negative_limit(self, request_mock):
|
||||||
|
request_mock.path = "/lbaas/v2/pools/1/members"
|
||||||
|
request_mock.path_url = "http://localhost" + request_mock.path
|
||||||
|
api_base_uri = "https://127.0.0.1"
|
||||||
|
conf = self.useFixture(oslo_fixture.Config(cfg.CONF))
|
||||||
|
conf.config(group='api_settings', api_base_uri=api_base_uri)
|
||||||
|
member1 = models.Member()
|
||||||
|
member1.id = uuidutils.generate_uuid()
|
||||||
|
model_list = [member1]
|
||||||
|
|
||||||
|
params = {'limit': -1, 'marker': member1.id}
|
||||||
|
helper = pagination.PaginationHelper(params)
|
||||||
|
links = helper._make_links(model_list)
|
||||||
|
self.assertEqual(links[0].rel, "previous")
|
||||||
|
self.assertEqual(
|
||||||
|
links[0].href,
|
||||||
|
("{base_uri}{path}?limit={limit}&marker={marker}"
|
||||||
|
"&page_reverse=True").format(
|
||||||
|
base_uri=api_base_uri,
|
||||||
|
path=request_mock.path,
|
||||||
|
limit=None,
|
||||||
|
marker=member1.id
|
||||||
|
))
|
||||||
|
self.assertEqual(links[1].rel, "next")
|
||||||
|
self.assertEqual(
|
||||||
|
links[1].href,
|
||||||
|
"{base_uri}{path}?limit={limit}&marker={marker}".format(
|
||||||
|
base_uri=api_base_uri,
|
||||||
|
path=request_mock.path,
|
||||||
|
limit=None,
|
||||||
|
marker=member1.id))
|
||||||
|
@@ -0,0 +1,6 @@
|
|||||||
|
---
|
||||||
|
fixes:
|
||||||
|
- |
|
||||||
|
Fix the issue, when "limit" parameter in request less or equal 0.
|
||||||
|
Now it returns resources according pagination_max_limit as expected,
|
||||||
|
instead of error.
|
Reference in New Issue
Block a user