Skip to content

Commit

Permalink
container_hierarchy: fix paginated listing
Browse files Browse the repository at this point in the history
This commit fix paginated listing:
- the marker parameter was not used
- the limit was not used, even using pagesize of 1000,
  10000 entries was extracted.
- the result of listing was not sorted and a previous entries
  were reused instead of last one
  • Loading branch information
murlock authored and fvennetier committed Apr 9, 2018
1 parent dc5b072 commit 585674f
Showing 1 changed file with 34 additions and 9 deletions.
43 changes: 34 additions & 9 deletions oioswift/common/middleware/container_hierarchy.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

LOG = None
MIDDLEWARE_NAME = 'container_hierarchy'
DEFAULT_LIMIT = 10000


class ContainerHierarchyMiddleware(AutoContainerBase):
Expand Down Expand Up @@ -151,7 +152,8 @@ def _build_empty_response(self, start_response, status='200 OK'):

def _build_object_listing(self, start_response, env,
account, container, obj,
recursive=False):
limit=None,
recursive=False, marker=None):
oheaders = dict()

def header_cb(header_dict):
Expand All @@ -160,8 +162,12 @@ def header_cb(header_dict):
all_objs = [x for x in self._list_objects(
env, account,
tuple(container.split(self.ENCODED_DELIMITER)),
header_cb, prefix=obj or '',
recursive=recursive)]
header_cb, prefix=obj or '', marker=marker,
limit=limit, recursive=recursive)]
# results must be sorted or paginated listing are broken
all_objs = sorted(all_objs,
key=lambda entry: entry.get('name',
entry.get('subdir')))
body = json.dumps(all_objs)
oheaders['X-Container-Object-Count'] = len(all_objs)
# FIXME: aggregate X-Container-Bytes-Used
Expand All @@ -188,7 +194,8 @@ def _fake_container_and_obj(self, container, obj_parts, is_listing=False):
return container, obj

def _list_objects(self, env, account, ct_parts, header_cb,
prefix='', recursive=True, limit=10000):
prefix='', recursive=True, limit=DEFAULT_LIMIT,
marker=None):
"""
If `recursive` is set (the default), for each subdirectory marker
encountered, make a listing subrequest, and yield object list.
Expand All @@ -198,7 +205,9 @@ def _list_objects(self, env, account, ct_parts, header_cb,
"""
sub_path = quote_plus(self.DELIMITER.join(
('', 'v1', account, self.ENCODED_DELIMITER.join(ct_parts))))
LOG.debug("%s: listing objects from '%s'", self.SWIFT_SOURCE, sub_path)
LOG.debug("%s: listing objects from '%s' "
"(limit=%d, prefix=%s, marker=%s)",
self.SWIFT_SOURCE, sub_path, limit, prefix, marker)
sub_req = make_subrequest(env.copy(), method='GET', path=sub_path,
body='',
swift_source=self.SWIFT_SOURCE)
Expand All @@ -207,6 +216,8 @@ def _list_objects(self, env, account, ct_parts, header_cb,
params['limit'] = str(limit) # FIXME: why is it str?
params['prefix'] = prefix
params['format'] = 'json'
if marker:
params['marker'] = marker
sub_req.params = params
resp = sub_req.get_response(self.app)
obj_prefix = ''
Expand Down Expand Up @@ -264,8 +275,12 @@ def __call__(self, env, start_response):
env2 = env.copy()
qs = parse_qs(req.query_string or '')
prefix = qs.get('prefix') # returns a list or None
LOG.debug("%s: Got %s request for container=%s, obj=%s, prefix=%s",
self.SWIFT_SOURCE, req.method, container, obj, prefix)
marker = qs.get('marker')
limit = qs.get('limit')
LOG.debug("%s: Got %s request for container=%s, "
"obj=%s, prefix=%s marker=%s",
self.SWIFT_SOURCE, req.method, container, obj, prefix,
marker)
must_recurse = False

# Rework Oio-Copy-From to use correct source (container, obj)
Expand All @@ -290,6 +305,14 @@ def __call__(self, env, start_response):
env2['QUERY_STRING'] = urlencode(qs, True)
container, obj = self._fake_container_and_obj(
container, obj_parts, is_listing=True)
if not marker:
marker = None
else:
marker = marker[0].strip('/').split(self.DELIMITER)[-1]
if not limit:
limit = self.DEFAULT_LIMIT
else:
limit = int(limit[0])
else:
LOG.debug("%s: -> is NOT listing request", self.SWIFT_SOURCE)
obj_parts = obj.split(self.DELIMITER)
Expand All @@ -310,7 +333,8 @@ def __call__(self, env, start_response):
if must_recurse:
res = self._build_object_listing(start_response, env,
account, container, obj,
recursive=True)
limit=limit, recursive=True,
marker=marker)
elif not (qs.get('prefix') or qs.get('delimiter')):
# should be other operation that listing
if obj:
Expand All @@ -321,7 +345,8 @@ def __call__(self, env, start_response):
else:
res = self._build_object_listing(start_response, env,
account, container, obj,
recursive=False)
limit=limit,
recursive=False, marker=marker)

return res

Expand Down

0 comments on commit 585674f

Please sign in to comment.