openstack/nova
Code Issues Proposed changes

cleanup based on waldon's comments, also caught a few other issues

Browse Source
This commit is contained in:
William Wolf
2011-06-01 23:09:37 -04:00
parent b5cc7cb35c
commit 4fb46873ef
8 changed files with 119 additions and 158 deletions
Download Patch File Download Diff File Expand all files Collapse all files

18
nova/api/openstack/common.py
View File

@@ -50,7 +50,7 @@ def get_pagination_params(request):
try:
marker = int(request.GET.get('marker', 0))
except ValueError:
raise webob.exc.HTTPBadRequest(_('offset param must be an integer'))
raise webob.exc.HTTPBadRequest(_('marker param must be an integer'))
try:
limit = int(request.GET.get('limit', 0))
@@ -102,19 +102,11 @@ def limited(items, request, max_limit=FLAGS.osapi_max_limit):
def limited_by_marker(items, request, max_limit=FLAGS.osapi_max_limit):
"""Return a slice of items according to the requested marker and limit."""
print "TEST LIMIT"
(marker, limit) = get_pagination_params(request)
try:
marker = int(request.GET.get('marker', 0))
except ValueError:
raise webob.exc.HTTPBadRequest(_('marker param must be an integer'))
try:
limit = int(request.GET.get('limit', max_limit))
except ValueError:
raise webob.exc.HTTPBadRequest(_('limit param must be an integer'))
if limit < 0:
raise webob.exc.HTTPBadRequest(_('limit param must be positive'))
if limit == 0:
limit = max_limit
limit = min(max_limit, limit)
start_index = 0

8
nova/api/openstack/images.py
View File

@@ -46,9 +46,6 @@ class Controller(object):
self._compute_service = compute_service or compute.API()
self._image_service = image_service or _default_service
def _limit_items(self, items, req):
return common.limited(items, req)
def index(self, req):
"""Return an index listing of images available to the request.
@@ -162,13 +159,11 @@ class ControllerV11(Controller):
base_url = request.application_url
return images_view.ViewBuilderV11(base_url)
def get_default_xmlns(self, req):
return common.XML_NS_V11
def index(self, req):
"""Return an index listing of images available to the request.
:param req: `wsgi.Request` object
"""
context = req.environ['nova.context']
filters = self._get_filters(req)
@@ -182,6 +177,7 @@ class ControllerV11(Controller):
"""Return a detailed index listing of images available to the request.
:param req: `wsgi.Request` object.
"""
context = req.environ['nova.context']
filters = self._get_filters(req)

1
nova/api/openstack/servers.py
View File

@@ -55,6 +55,7 @@ class Controller(object):
def detail(self, req):
""" Returns a list of server details for a given user """
print "DETAIL"
return self._items(req, is_detail=True)
def _image_id_from_req_data(self, data):

33
nova/tests/api/openstack/fakes.py
View File

@@ -166,29 +166,7 @@ def stub_out_glance(stubs, initial_fixtures=None):
def __init__(self, initial_fixtures):
self.fixtures = initial_fixtures or []
def fake_get_images(self, filters=None, marker=None, limit=None):
found = True
if marker:
found = False
if limit == 0:
limit = None
fixtures = []
count = 0
for f in self.fixtures:
if limit and count >= limit:
break
if found:
fixtures.append(f)
count = count + 1
if f['id'] == marker:
found = True
return [dict(id=f['id'], name=f['name'])
for f in fixtures]
def fake_get_images_detailed(self, filters=None,
marker=None, limit=None):
def _filter_images(self, filters=None, marker=None, limit=None):
found = True
if marker:
found = False
@@ -208,6 +186,15 @@ def stub_out_glance(stubs, initial_fixtures=None):
return fixtures
def fake_get_images(self, filters=None, marker=None, limit=None):
fixtures = self._filter_images(filters, marker, limit)
return [dict(id=f['id'], name=f['name'])
for f in fixtures]
def fake_get_images_detailed(self, filters=None,
marker=None, limit=None):
return self._filter_images(filters, marker, limit)
def fake_get_image_meta(self, image_id):
image = self._find_image(image_id)
if image:

180
nova/tests/api/openstack/test_common.py
View File

@@ -24,7 +24,7 @@ import webob.exc
from webob import Request
from nova import test
from nova.api.openstack.common import limited, get_pagination_params
from nova.api.openstack import common
class LimiterTest(test.TestCase):
@@ -35,9 +35,7 @@ class LimiterTest(test.TestCase):
"""
def setUp(self):
"""
Run before each test.
"""
""" Run before each test. """
super(LimiterTest, self).setUp()
self.tiny = range(1)
self.small = range(10)
@@ -45,130 +43,112 @@ class LimiterTest(test.TestCase):
self.large = range(10000)
def test_limiter_offset_zero(self):
"""
Test offset key works with 0.
"""
""" Test offset key works with 0. """
req = Request.blank('/?offset=0')
self.assertEqual(limited(self.tiny, req), self.tiny)
self.assertEqual(limited(self.small, req), self.small)
self.assertEqual(limited(self.medium, req), self.medium)
self.assertEqual(limited(self.large, req), self.large[:1000])
self.assertEqual(common.limited(self.tiny, req), self.tiny)
self.assertEqual(common.limited(self.small, req), self.small)
self.assertEqual(common.limited(self.medium, req), self.medium)
self.assertEqual(common.limited(self.large, req), self.large[:1000])
def test_limiter_offset_medium(self):
"""
Test offset key works with a medium sized number.
"""
""" Test offset key works with a medium sized number. """
req = Request.blank('/?offset=10')
self.assertEqual(limited(self.tiny, req), [])
self.assertEqual(limited(self.small, req), self.small[10:])
self.assertEqual(limited(self.medium, req), self.medium[10:])
self.assertEqual(limited(self.large, req), self.large[10:1010])
self.assertEqual(common.limited(self.tiny, req), [])
self.assertEqual(common.limited(self.small, req), self.small[10:])
self.assertEqual(common.limited(self.medium, req), self.medium[10:])
self.assertEqual(common.limited(self.large, req), self.large[10:1010])
def test_limiter_offset_over_max(self):
"""
Test offset key works with a number over 1000 (max_limit).
"""
""" Test offset key works with a number over 1000 (max_limit). """
req = Request.blank('/?offset=1001')
self.assertEqual(limited(self.tiny, req), [])
self.assertEqual(limited(self.small, req), [])
self.assertEqual(limited(self.medium, req), [])
self.assertEqual(limited(self.large, req), self.large[1001:2001])
self.assertEqual(common.limited(self.tiny, req), [])
self.assertEqual(common.limited(self.small, req), [])
self.assertEqual(common.limited(self.medium, req), [])
self.assertEqual(
common.limited(self.large, req), self.large[1001:2001])
def test_limiter_offset_blank(self):
"""
Test offset key works with a blank offset.
"""
""" Test offset key works with a blank offset. """
req = Request.blank('/?offset=')
self.assertRaises(webob.exc.HTTPBadRequest, limited, self.tiny, req)
self.assertRaises(
webob.exc.HTTPBadRequest, common.limited, self.tiny, req)
def test_limiter_offset_bad(self):
"""
Test offset key works with a BAD offset.
"""
""" Test offset key works with a BAD offset. """
req = Request.blank(u'/?offset=\u0020aa')
self.assertRaises(webob.exc.HTTPBadRequest, limited, self.tiny, req)
self.assertRaises(
webob.exc.HTTPBadRequest, common.limited, self.tiny, req)
def test_limiter_nothing(self):
"""
Test request with no offset or limit
"""
""" Test request with no offset or limit """
req = Request.blank('/')
self.assertEqual(limited(self.tiny, req), self.tiny)
self.assertEqual(limited(self.small, req), self.small)
self.assertEqual(limited(self.medium, req), self.medium)
self.assertEqual(limited(self.large, req), self.large[:1000])
self.assertEqual(common.limited(self.tiny, req), self.tiny)
self.assertEqual(common.limited(self.small, req), self.small)
self.assertEqual(common.limited(self.medium, req), self.medium)
self.assertEqual(common.limited(self.large, req), self.large[:1000])
def test_limiter_limit_zero(self):
"""
Test limit of zero.
"""
""" Test limit of zero. """
req = Request.blank('/?limit=0')
self.assertEqual(limited(self.tiny, req), self.tiny)
self.assertEqual(limited(self.small, req), self.small)
self.assertEqual(limited(self.medium, req), self.medium)
self.assertEqual(limited(self.large, req), self.large[:1000])
self.assertEqual(common.limited(self.tiny, req), self.tiny)
self.assertEqual(common.limited(self.small, req), self.small)
self.assertEqual(common.limited(self.medium, req), self.medium)
self.assertEqual(common.limited(self.large, req), self.large[:1000])
def test_limiter_limit_medium(self):
"""
Test limit of 10.
"""
""" Test limit of 10. """
req = Request.blank('/?limit=10')
self.assertEqual(limited(self.tiny, req), self.tiny)
self.assertEqual(limited(self.small, req), self.small)
self.assertEqual(limited(self.medium, req), self.medium[:10])
self.assertEqual(limited(self.large, req), self.large[:10])
self.assertEqual(common.limited(self.tiny, req), self.tiny)
self.assertEqual(common.limited(self.small, req), self.small)
self.assertEqual(common.limited(self.medium, req), self.medium[:10])
self.assertEqual(common.limited(self.large, req), self.large[:10])
def test_limiter_limit_over_max(self):
"""
Test limit of 3000.
"""
""" Test limit of 3000. """
req = Request.blank('/?limit=3000')
self.assertEqual(limited(self.tiny, req), self.tiny)
self.assertEqual(limited(self.small, req), self.small)
self.assertEqual(limited(self.medium, req), self.medium)
self.assertEqual(limited(self.large, req), self.large[:1000])
self.assertEqual(common.limited(self.tiny, req), self.tiny)
self.assertEqual(common.limited(self.small, req), self.small)
self.assertEqual(common.limited(self.medium, req), self.medium)
self.assertEqual(common.limited(self.large, req), self.large[:1000])
def test_limiter_limit_and_offset(self):
"""
Test request with both limit and offset.
"""
""" Test request with both limit and offset. """
items = range(2000)
req = Request.blank('/?offset=1&limit=3')
self.assertEqual(limited(items, req), items[1:4])
self.assertEqual(common.limited(items, req), items[1:4])
req = Request.blank('/?offset=3&limit=0')
self.assertEqual(limited(items, req), items[3:1003])
self.assertEqual(common.limited(items, req), items[3:1003])
req = Request.blank('/?offset=3&limit=1500')
self.assertEqual(limited(items, req), items[3:1003])
self.assertEqual(common.limited(items, req), items[3:1003])
req = Request.blank('/?offset=3000&limit=10')
self.assertEqual(limited(items, req), [])
self.assertEqual(common.limited(items, req), [])
def test_limiter_custom_max_limit(self):
"""
Test a max_limit other than 1000.
"""
""" Test a max_limit other than 1000. """
items = range(2000)
req = Request.blank('/?offset=1&limit=3')
self.assertEqual(limited(items, req, max_limit=2000), items[1:4])
self.assertEqual(
common.limited(items, req, max_limit=2000), items[1:4])
req = Request.blank('/?offset=3&limit=0')
self.assertEqual(limited(items, req, max_limit=2000), items[3:])
self.assertEqual(
common.limited(items, req, max_limit=2000), items[3:])
req = Request.blank('/?offset=3&limit=2500')
self.assertEqual(limited(items, req, max_limit=2000), items[3:])
self.assertEqual(
common.limited(items, req, max_limit=2000), items[3:])
req = Request.blank('/?offset=3000&limit=10')
self.assertEqual(limited(items, req, max_limit=2000), [])
self.assertEqual(common.limited(items, req, max_limit=2000), [])
def test_limiter_negative_limit(self):
"""
Test a negative limit.
"""
""" Test a negative limit. """
req = Request.blank('/?limit=-3000')
self.assertRaises(webob.exc.HTTPBadRequest, limited, self.tiny, req)
self.assertRaises(
webob.exc.HTTPBadRequest, common.limited, self.tiny, req)
def test_limiter_negative_offset(self):
"""
Test a negative offset.
"""
""" Test a negative offset. """
req = Request.blank('/?offset=-30')
self.assertRaises(webob.exc.HTTPBadRequest, limited, self.tiny, req)
self.assertRaises(
webob.exc.HTTPBadRequest, common.limited, self.tiny, req)
class PaginationParamsTest(test.TestCase):
@@ -179,38 +159,28 @@ class PaginationParamsTest(test.TestCase):
"""
def test_no_params(self):
"""
Test no params.
"""
""" Test no params. """
req = Request.blank('/')
self.assertEqual(get_pagination_params(req), (0, 0))
self.assertEqual(common.get_pagination_params(req), (0, 0))
def test_valid_marker(self):
"""
Test valid marker param.
"""
""" Test valid marker param. """
req = Request.blank('/?marker=1')
self.assertEqual(get_pagination_params(req), (1, 0))
self.assertEqual(common.get_pagination_params(req), (1, 0))
def test_invalid_marker(self):
"""
Test invalid marker param.
"""
""" Test invalid marker param. """
req = Request.blank('/?marker=-2')
self.assertRaises(webob.exc.HTTPBadRequest,
get_pagination_params, req)
self.assertRaises(
webob.exc.HTTPBadRequest, common.get_pagination_params, req)
def test_valid_limit(self):
"""
Test valid limit param.
"""
""" Test valid limit param. """
req = Request.blank('/?limit=10')
self.assertEqual(get_pagination_params(req), (0, 10))
self.assertEqual(common.get_pagination_params(req), (0, 10))
def test_invalid_limit(self):
"""
Test invalid limit param.
"""
""" Test invalid limit param. """
req = Request.blank('/?limit=-2')
self.assertRaises(webob.exc.HTTPBadRequest,
get_pagination_params, req)
self.assertRaises(
webob.exc.HTTPBadRequest, common.get_pagination_params, req)

33
nova/tests/api/openstack/test_images.py
View File

@@ -239,7 +239,7 @@ class GlanceImageServiceTest(_BaseImageServiceTests):
i = 0
for meta in image_metas:
expected = {'id': 'DONTCARE',
'name': 'TestImage %d' % (i)}
'name': 'TestImage %d' % (i)}
self.assertDictMatch(meta, expected)
i = i + 1
@@ -256,7 +256,7 @@ class GlanceImageServiceTest(_BaseImageServiceTests):
i = 2
for meta in image_metas:
expected = {'id': 'DONTCARE',
'name': 'TestImage %d' % (i)}
'name': 'TestImage %d' % (i)}
self.assertDictMatch(meta, expected)
i = i + 1
@@ -284,7 +284,7 @@ class GlanceImageServiceTest(_BaseImageServiceTests):
i = 4
for meta in image_metas:
expected = {'id': 'DONTCARE',
'name': 'TestImage %d' % (i)}
'name': 'TestImage %d' % (i)}
self.assertDictMatch(meta, expected)
i = i + 1
@@ -300,10 +300,17 @@ class GlanceImageServiceTest(_BaseImageServiceTests):
self.assertEquals(len(image_metas), 8)
i = 2
for meta in image_metas:
expected = {'id': 'DONTCARE', 'status': None,
'is_public': True, 'properties': {
'updated': None, 'created': None},
'name': 'TestImage %d' % (i)}
expected = {
'id': 'DONTCARE',
'status': None,
'is_public': True,
'name': 'TestImage %d' % (i),
'properties': {
'updated': None,
'created': None,
},
}
self.assertDictMatch(meta, expected)
i = i + 1
@@ -330,10 +337,14 @@ class GlanceImageServiceTest(_BaseImageServiceTests):
self.assertEquals(len(image_metas), 3)
i = 4
for meta in image_metas:
expected = {'id': 'DONTCARE', 'status': None,
'is_public': True, 'properties': {
'updated': None, 'created': None},
'name': 'TestImage %d' % (i)}
expected = {
'id': 'DONTCARE',
'status': None,
'is_public': True,
'name': 'TestImage %d' % (i),
'properties': {
'updated': None, 'created': None},
}
self.assertDictMatch(meta, expected)
i = i + 1

1
nova/tests/api/openstack/test_servers.py
View File

@@ -448,6 +448,7 @@ class ServersTest(test.TestCase):
req = webob.Request.blank('/v1.1/servers?limit=2&marker=asdf')
res = req.get_response(fakes.wsgi_app())
self.assertEqual(res.status_int, 400)
print "BODY",res.body
self.assertTrue(res.body.find('marker param') > -1)
def _setup_for_create_instance(self):

3
nova/tests/integrated/test_servers.py
View File

@@ -88,12 +88,15 @@ class ServersTest(integrated_helpers._IntegratedTestBase):
# Check it's there
found_server = self.api.get_server(created_server_id)
print "FOUND_SERVER:", found_server
self.assertEqual(created_server_id, found_server['id'])
# It should also be in the all-servers list
servers = self.api.get_servers()
print "SERVERS:", servers
server_ids = [server['id'] for server in servers]
self.assertTrue(created_server_id in server_ids)
return
# Wait (briefly) for creation
retries = 0