Make FakeSwift query param order agnostic

Apparently FakeSwift has always been persnickety about the order of
query params of registered responses and the app making the calls.
Since query params can often be converted to dictionaries the order of
the encoded params should be able to change without effecting the apps
dispatching of registered responses.

Change-Id: Ied68c9334201a7663e9c85f3bdaa5b0643d4b6db
This commit is contained in:
Clay Gerrard 2020-01-06 16:27:06 -06:00
parent fb538a9afe
commit 6140190791
3 changed files with 100 additions and 55 deletions

View File

@ -17,6 +17,7 @@
from collections import defaultdict, namedtuple
from hashlib import md5
from six.moves.urllib import parse
from swift.common import swob
from swift.common.header_key_dict import HeaderKeyDict
from swift.common.request_helpers import is_user_meta, \
@ -28,24 +29,38 @@ from test.unit import FakeLogger, FakeRing
class LeakTrackingIter(object):
def __init__(self, inner_iter, mark_closed, path):
def __init__(self, inner_iter, mark_closed, key):
if isinstance(inner_iter, bytes):
inner_iter = (inner_iter, )
self.inner_iter = inner_iter
self.mark_closed = mark_closed
self.path = path
self.key = key
def __iter__(self):
for x in self.inner_iter:
yield x
def close(self):
self.mark_closed(self.path)
self.mark_closed(self.key)
FakeSwiftCall = namedtuple('FakeSwiftCall', ['method', 'path', 'headers'])
def normalize_query_string(qs):
if qs.startswith('?'):
qs = qs[1:]
if not qs:
return ''
else:
return '?%s' % parse.urlencode(sorted(parse.parse_qsl(qs)))
def normalize_path(path):
parsed = parse.urlparse(path)
return parsed.path + normalize_query_string(parsed.query)
class FakeSwift(object):
"""
A good-enough fake Swift proxy server to use in testing middleware.
@ -55,7 +70,7 @@ class FakeSwift(object):
def __init__(self):
self._calls = []
self._unclosed_req_paths = defaultdict(int)
self._unclosed_req_keys = defaultdict(int)
self.req_method_paths = []
self.swift_sources = []
self.txn_ids = []
@ -68,6 +83,7 @@ class FakeSwift(object):
self.get_object_ring = lambda policy_index: FakeRing()
def _find_response(self, method, path):
path = normalize_path(path)
resp = self._responses[(method, path)]
if isinstance(resp, list):
try:
@ -88,6 +104,7 @@ class FakeSwift(object):
rest_with_last=True)
if env.get('QUERY_STRING'):
path += '?' + env['QUERY_STRING']
path = normalize_path(path)
if 'swift.authorize' in env:
resp = env['swift.authorize'](swob.Request(env))
@ -171,19 +188,19 @@ class FakeSwift(object):
conditional_response=req.method in ('GET', 'HEAD'),
conditional_etag=conditional_etag)
wsgi_iter = resp(env, start_response)
self.mark_opened(path)
return LeakTrackingIter(wsgi_iter, self.mark_closed, path)
self.mark_opened((method, path))
return LeakTrackingIter(wsgi_iter, self.mark_closed, (method, path))
def mark_opened(self, path):
self._unclosed_req_paths[path] += 1
def mark_opened(self, key):
self._unclosed_req_keys[key] += 1
def mark_closed(self, path):
self._unclosed_req_paths[path] -= 1
def mark_closed(self, key):
self._unclosed_req_keys[key] -= 1
@property
def unclosed_requests(self):
return {path: count
for path, count in self._unclosed_req_paths.items()
return {key: count
for key, count in self._unclosed_req_keys.items()
if count > 0}
@property
@ -203,9 +220,11 @@ class FakeSwift(object):
return len(self._calls)
def register(self, method, path, response_class, headers, body=b''):
path = normalize_path(path)
self._responses[(method, path)] = (response_class, headers, body)
def register_responses(self, method, path, responses):
path = normalize_path(path)
self._responses[(method, path)] = list(responses)

View File

@ -22,7 +22,7 @@ import unittest
from swift.common import swob, utils
from swift.common.middleware import versioned_writes, copy
from swift.common.swob import Request
from test.unit.common.middleware.helpers import FakeSwift
from test.unit.common.middleware import helpers
class FakeCache(object):
@ -58,7 +58,7 @@ def local_tz(func):
class VersionedWritesBaseTestCase(unittest.TestCase):
def setUp(self):
self.app = FakeSwift()
self.app = helpers.FakeSwift()
conf = {'allow_versioned_writes': 'true'}
self.vw = versioned_writes.filter_factory(conf)(self.app)
@ -590,8 +590,8 @@ class VersionedWritesTestCase(VersionedWritesBaseTestCase):
self.app.register(
'DELETE', '/v1/a/c/o', swob.HTTPOk, {}, 'passed')
self.app.register(
'GET',
'/v1/a/ver_cont?prefix=001o/&marker=&reverse=on',
'GET', helpers.normalize_path(
'/v1/a/ver_cont?prefix=001o/&marker=&reverse=on'),
swob.HTTPNotFound, {}, None)
cache = FakeCache({'sysmeta': {'versions-location': 'ver_cont'}})
@ -609,7 +609,8 @@ class VersionedWritesTestCase(VersionedWritesBaseTestCase):
prefix_listing_prefix = '/v1/a/ver_cont?prefix=001o/&'
self.assertEqual(self.app.calls, [
('GET', prefix_listing_prefix + 'marker=&reverse=on'),
('GET', helpers.normalize_path(
prefix_listing_prefix + 'marker=&reverse=on')),
('DELETE', '/v1/a/c/o'),
])
@ -617,8 +618,7 @@ class VersionedWritesTestCase(VersionedWritesBaseTestCase):
self.app.register(
'DELETE', '/v1/a/c/o', swob.HTTPOk, {}, 'passed')
self.app.register(
'GET',
'/v1/a/ver_cont?prefix=001o/&marker=&reverse=on',
'GET', '/v1/a/ver_cont?prefix=001o/&marker=&reverse=on',
swob.HTTPOk, {}, '[]')
cache = FakeCache({'sysmeta': {'versions-location': 'ver_cont'}})
@ -633,7 +633,8 @@ class VersionedWritesTestCase(VersionedWritesBaseTestCase):
prefix_listing_prefix = '/v1/a/ver_cont?prefix=001o/&'
self.assertEqual(self.app.calls, [
('GET', prefix_listing_prefix + 'marker=&reverse=on'),
('GET', helpers.normalize_path(
prefix_listing_prefix + 'marker=&reverse=on')),
('DELETE', '/v1/a/c/o'),
])
@ -681,7 +682,8 @@ class VersionedWritesTestCase(VersionedWritesBaseTestCase):
prefix_listing_prefix = '/v1/a/ver_cont?prefix=001o/&'
self.assertEqual(self.app.calls, [
('GET', prefix_listing_prefix + 'marker=&reverse=on'),
('GET', helpers.normalize_path(
prefix_listing_prefix + 'marker=&reverse=on')),
('GET', '/v1/a/ver_cont/001o/2?symlink=get'),
('PUT', '/v1/a/c/o'),
('DELETE', '/v1/a/ver_cont/001o/2'),
@ -738,7 +740,8 @@ class VersionedWritesTestCase(VersionedWritesBaseTestCase):
# in the base versioned container.
self.app.register(
'GET',
'/v1/a/ver_cont?prefix=001o/&marker=&reverse=on',
helpers.normalize_path(
'/v1/a/ver_cont?prefix=001o/&marker=&reverse=on'),
swob.HTTPOk, {},
'[{"hash": "y", '
'"last_modified": "2014-11-21T14:23:02.206740", '
@ -775,7 +778,8 @@ class VersionedWritesTestCase(VersionedWritesBaseTestCase):
prefix_listing_prefix = '/v1/a/ver_cont?prefix=001o/&'
self.assertEqual(self.app.calls, [
('GET', prefix_listing_prefix + 'marker=&reverse=on'),
('GET', helpers.normalize_path(
prefix_listing_prefix + 'marker=&reverse=on')),
('HEAD', '/v1/a/c/o'),
('GET', '/v1/a/ver_cont/001o/1?symlink=get'),
('PUT', '/v1/a/c/o'),
@ -940,7 +944,8 @@ class VersionedWritesTestCase(VersionedWritesBaseTestCase):
prefix_listing_prefix = '/v1/a/ver_cont?prefix=001o/&'
self.assertEqual(self.app.calls, [
('GET', prefix_listing_prefix + 'marker=&reverse=on'),
('GET', helpers.normalize_path(
prefix_listing_prefix + 'marker=&reverse=on')),
('GET', '/v1/a/ver_cont/001o/1?symlink=get'),
('PUT', '/v1/a/c/o'),
('DELETE', '/v1/a/ver_cont/001o/1'),
@ -988,11 +993,14 @@ class VersionedWritesTestCase(VersionedWritesBaseTestCase):
prefix_listing_prefix = '/v1/a/ver_cont?prefix=001o/&'
self.assertEqual(self.app.calls, [
('GET', prefix_listing_prefix + 'marker=&reverse=on'),
('GET', '/v1/a/ver_cont/001o/2?symlink=get'),
('GET', '/v1/a/ver_cont/001o/1?symlink=get'),
('PUT', '/v1/a/c/o'),
('DELETE', '/v1/a/ver_cont/001o/1'),
('GET', helpers.normalize_path(
prefix_listing_prefix + 'marker=&reverse=on')),
('GET', helpers.normalize_path(
'/v1/a/ver_cont/001o/2?symlink=get')),
('GET', helpers.normalize_path(
'/v1/a/ver_cont/001o/1?symlink=get')),
('PUT', helpers.normalize_path('/v1/a/c/o')),
('DELETE', helpers.normalize_path('/v1/a/ver_cont/001o/1')),
])
def test_denied_DELETE_of_versioned_object(self):
@ -1030,7 +1038,8 @@ class VersionedWritesTestCase(VersionedWritesBaseTestCase):
prefix_listing_prefix = '/v1/a/ver_cont?prefix=001o/&'
self.assertEqual(self.app.calls, [
('GET', prefix_listing_prefix + 'marker=&reverse=on'),
('GET', helpers.normalize_path(
prefix_listing_prefix + 'marker=&reverse=on')),
])
def test_denied_PUT_of_versioned_object(self):
@ -1112,8 +1121,10 @@ class VersionedWritesOldContainersTestCase(VersionedWritesBaseTestCase):
prefix_listing_prefix = '/v1/a/ver_cont?prefix=001o/&'
self.assertEqual(self.app.calls, [
('GET', prefix_listing_prefix + 'marker=&reverse=on'),
('GET', prefix_listing_prefix + 'marker=001o/2'),
('GET', helpers.normalize_path(
prefix_listing_prefix + 'marker=&reverse=on')),
('GET', helpers.normalize_path(
prefix_listing_prefix + 'marker=001o/2')),
('GET', '/v1/a/ver_cont/001o/2?symlink=get'),
('PUT', '/v1/a/c/o'),
('DELETE', '/v1/a/ver_cont/001o/2'),
@ -1165,8 +1176,10 @@ class VersionedWritesOldContainersTestCase(VersionedWritesBaseTestCase):
prefix_listing_prefix = '/v1/a/ver_cont?prefix=001o/&'
self.assertEqual(self.app.calls, [
('GET', prefix_listing_prefix + 'marker=&reverse=on'),
('GET', prefix_listing_prefix + 'marker=001o/2'),
('GET', helpers.normalize_path(
prefix_listing_prefix + 'marker=&reverse=on')),
('GET', helpers.normalize_path(
prefix_listing_prefix + 'marker=001o/2')),
('GET', '/v1/a/ver_cont/001o/2?symlink=get'),
('GET', '/v1/a/ver_cont/001o/1?symlink=get'),
('PUT', '/v1/a/c/o'),
@ -1215,8 +1228,10 @@ class VersionedWritesOldContainersTestCase(VersionedWritesBaseTestCase):
self.assertRequestEqual(req, authorize_call[0])
prefix_listing_prefix = '/v1/a/ver_cont?prefix=001o/&'
self.assertEqual(self.app.calls, [
('GET', prefix_listing_prefix + 'marker=&reverse=on'),
('GET', prefix_listing_prefix + 'marker=001o/2'),
('GET', helpers.normalize_path(
prefix_listing_prefix + 'marker=&reverse=on')),
('GET', helpers.normalize_path(
prefix_listing_prefix + 'marker=001o/2')),
])
def test_partially_upgraded_cluster(self):
@ -1281,14 +1296,19 @@ class VersionedWritesOldContainersTestCase(VersionedWritesBaseTestCase):
self.assertEqual(status, '204 No Content')
prefix_listing_prefix = '/v1/a/ver_cont?prefix=001o/&'
self.assertEqual(self.app.calls, [
('GET', prefix_listing_prefix + 'marker=&reverse=on'),
('GET', helpers.normalize_path(
prefix_listing_prefix + 'marker=&reverse=on')),
('GET', '/v1/a/ver_cont/001o/4?symlink=get'),
('GET', '/v1/a/ver_cont/001o/3?symlink=get'),
('GET', '/v1/a/ver_cont/001o/2?symlink=get'),
('GET', prefix_listing_prefix + 'marker=001o/2&reverse=on'),
('GET', prefix_listing_prefix + 'marker=&end_marker=001o/2'),
('GET', prefix_listing_prefix + 'marker=001o/0&end_marker=001o/2'),
('GET', prefix_listing_prefix + 'marker=001o/1&end_marker=001o/2'),
('GET', helpers.normalize_path(
prefix_listing_prefix + 'marker=001o/2&reverse=on')),
('GET', helpers.normalize_path(
prefix_listing_prefix + 'marker=&end_marker=001o/2')),
('GET', helpers.normalize_path(
prefix_listing_prefix + 'marker=001o/0&end_marker=001o/2')),
('GET', helpers.normalize_path(
prefix_listing_prefix + 'marker=001o/1&end_marker=001o/2')),
('GET', '/v1/a/ver_cont/001o/1?symlink=get'),
('PUT', '/v1/a/c/o'),
('DELETE', '/v1/a/ver_cont/001o/1'),
@ -1353,13 +1373,18 @@ class VersionedWritesOldContainersTestCase(VersionedWritesBaseTestCase):
self.assertEqual(status, '204 No Content')
prefix_listing_prefix = '/v1/a/ver_cont?prefix=001o/&'
self.assertEqual(self.app.calls, [
('GET', prefix_listing_prefix + 'marker=&reverse=on'),
('GET', helpers.normalize_path(
prefix_listing_prefix + 'marker=&reverse=on')),
('GET', '/v1/a/ver_cont/001o/4?symlink=get'),
('GET', '/v1/a/ver_cont/001o/3?symlink=get'),
('GET', prefix_listing_prefix + 'marker=001o/3&reverse=on'),
('GET', prefix_listing_prefix + 'marker=&end_marker=001o/3'),
('GET', prefix_listing_prefix + 'marker=001o/1&end_marker=001o/3'),
('GET', prefix_listing_prefix + 'marker=001o/2&end_marker=001o/3'),
('GET', helpers.normalize_path(
prefix_listing_prefix + 'marker=001o/3&reverse=on')),
('GET', helpers.normalize_path(
prefix_listing_prefix + 'marker=&end_marker=001o/3')),
('GET', helpers.normalize_path(
prefix_listing_prefix + 'marker=001o/1&end_marker=001o/3')),
('GET', helpers.normalize_path(
prefix_listing_prefix + 'marker=001o/2&end_marker=001o/3')),
('GET', '/v1/a/ver_cont/001o/2?symlink=get'),
('PUT', '/v1/a/c/o'),
('DELETE', '/v1/a/ver_cont/001o/2'),
@ -1370,7 +1395,7 @@ class VersionedWritesCopyingTestCase(VersionedWritesBaseTestCase):
# verify interaction of copy and versioned_writes middlewares
def setUp(self):
self.app = FakeSwift()
self.app = helpers.FakeSwift()
conf = {'allow_versioned_writes': 'true'}
self.vw = versioned_writes.filter_factory(conf)(self.app)
self.filter = copy.filter_factory({})(self.vw)

View File

@ -36,7 +36,7 @@ from swift.common.header_key_dict import HeaderKeyDict
from swift.common.utils import split_path, Timestamp, encode_timestamps
from test.unit import debug_logger, FakeRing, fake_http_connect
from test.unit.common.middleware.helpers import FakeSwift
from test.unit.common.middleware import helpers
def timestamp_to_last_modified(timestamp):
@ -51,7 +51,7 @@ def container_resp_headers(**kwargs):
class FakeStoragePolicySwift(object):
def __init__(self):
self.storage_policy = defaultdict(FakeSwift)
self.storage_policy = defaultdict(helpers.FakeSwift)
self._mock_oldest_spi_map = {}
def __getattribute__(self, name):
@ -158,17 +158,17 @@ class FakeInternalClient(reconciler.InternalClient):
container_listing_data.sort(key=operator.itemgetter('name'))
# register container listing response
container_headers = {}
container_qry_string = \
'?format=json&marker=&end_marker=&prefix='
container_qry_string = helpers.normalize_query_string(
'?format=json&marker=&end_marker=&prefix=')
self.app.register('GET', container_path + container_qry_string,
swob.HTTPOk, container_headers,
json.dumps(container_listing_data))
if container_listing_data:
obj_name = container_listing_data[-1]['name']
# client should quote and encode marker
end_qry_string = \
end_qry_string = helpers.normalize_query_string(
'?format=json&marker=%s&end_marker=&prefix=' % (
urllib.parse.quote(obj_name.encode('utf-8')))
urllib.parse.quote(obj_name.encode('utf-8'))))
self.app.register('GET', container_path + end_qry_string,
swob.HTTPOk, container_headers,
json.dumps([]))
@ -713,8 +713,9 @@ class TestReconcilerUtils(unittest.TestCase):
def listing_qs(marker):
return "?format=json&marker=%s&end_marker=&prefix=" % \
urllib.parse.quote(marker.encode('utf-8'))
return helpers.normalize_query_string(
"?format=json&marker=%s&end_marker=&prefix=" %
urllib.parse.quote(marker.encode('utf-8')))
class TestReconciler(unittest.TestCase):