# HG changeset patch # User Daniel Bentley # Date 1239527205 0 # Node ID c39a81bce1bd27b1b5d42ce2b928c1371f994ce8 # Parent 71a5bc0f0398b746738a1bf3e27836acd0f644a6 Use offset_linkid instead of offset to scan >1000 entities. this is a first-cut. It works in all the ways I could make earlier versions fail. It passes link_id as URL parameters. It also has a new class LinkCreator which makes the main body of getListContents even easier to write. I wasn't sure if link_id's could have non alphanumeric characters; if so, they need to be URL encoded/decoded. I also need to go and remove any mention of raw offsets now, because we don't use them. I believe I've talked about this approach with a few of you and it sounded reasonable. Feel free to roll-back/fix/amend/comment-for-me-to-fix. This is my first big-logic-change to Melange. Patch by: Dan Bentley diff -r 71a5bc0f0398 -r c39a81bce1bd app/soc/views/helper/lists.py --- a/app/soc/views/helper/lists.py Fri May 15 12:37:01 2009 +0200 +++ b/app/soc/views/helper/lists.py Sun Apr 12 09:06:45 2009 +0000 @@ -22,7 +22,6 @@ '"Pawel Solyga" ', ] -import logging from soc.logic import dicts from soc.logic.models.user import logic as user_logic @@ -61,6 +60,8 @@ OFFSET_KEY = 'offset_%d' LIMIT_KEY = 'limit_%d' +OFFSET_LINKID_KEY = 'offset_linkid_%d' +REVERSE_DIRECTION_KEY = 'reverse_sort_direction_%d' def makeOffsetKey(limit_idx): @@ -71,6 +72,14 @@ return LIMIT_KEY % limit_idx +def makeOffsetLinkidKey(limit_idx): + return OFFSET_LINKID_KEY % limit_idx + + +def makeReverseDirectionKey(limit_idx): + return REVERSE_DIRECTION_KEY % limit_idx + + def getListParameters(request, list_index): """Retrieves, converts and validates values for one list @@ -110,30 +119,44 @@ else: limit = min(DEF_MAX_PAGINATION, limit) - return dict(limit=limit, offset=offset) + result = dict(limit=limit, offset=offset) + offset_linkid = request.GET.get(makeOffsetLinkidKey(list_index), + '') + # TODO(dbentley): URL unescape + result['offset_linkid'] = offset_linkid + + reverse_direction = makeReverseDirectionKey(list_index) in request.GET + result['reverse_direction'] = reverse_direction + + return result -def generateLinkFromGetArgs(request, offset_and_limits): - """Constructs the get args for the url. +class LinkCreator(object): + """A way to create links for a page. """ - - args = ["%s=%s" % (k, v) for k, v in offset_and_limits.iteritems()] - link_suffix = '?' + '&'.join(args) - - return request.path + link_suffix - + def __init__(self, request, list_idx, limit): + self.path = request.path + self.base_params = dict( + i for i in request.GET.iteritems() if + i[0].startswith('offset_') or i[0].startswith('limit_')) + self.idx = list_idx + self.base_params[makeLimitKey(self.idx)] = limit -def generateLinkForRequest(request, base_params, updated_params): - """Create a link to the same page as request but with different params - - Params: - request: the request for the page - base_params: the base parameters - updated_params: the parameters to update - """ - params = base_params.copy() - params.update(updated_params) - return generateLinkFromGetArgs(request, params) + def create(self, offset_linkid=None, export=False, reverse_direction=False): + params = self.base_params.copy() + if offset_linkid is not None: + # TODO(dbentley): URL encode + if offset_linkid == '': + try: + del params[makeOffsetLinkidKey(self.idx)] + except KeyError: + pass + else: + params[makeOffsetLinkidKey(self.idx)]=offset_linkid + if reverse_direction: + params[makeReverseDirectionKey(self.idx)]=True + link_suffix = '&'.join('%s=%s' % (k, v) for k, v in params.iteritems()) + return '%s?%s' % (self.path, link_suffix) def getListContent(request, params, filter=None, order=None, @@ -174,13 +197,36 @@ # as we only use this logic for getForFields, which is never overridden logic = params['logic'] - limit_key, offset_key = makeLimitKey(idx), makeOffsetKey(idx) + limit_key = makeLimitKey(idx) + # offset_key = makeOffsetKey(idx) + # offset_linkid_key = makeOffsetLinkidKey(idx) + # reverse_direction_key = makeReverseDirectionKey(idx) list_params = getListParameters(request, idx) limit, offset = list_params['limit'], list_params['offset'] + offset_linkid = list_params['offset_linkid'] + reverse_direction = list_params['reverse_direction'] pagination_form = makePaginationForm(request, list_params['limit'], limit_key) + if offset_linkid: + if filter is None: + filter = {} + + if reverse_direction: + filter['link_id <'] = offset_linkid + else: + filter['link_id >'] = offset_linkid + + if order is None: + order = [] + if reverse_direction: + order.append('-link_id') + else: + order.append('link_id') + + + # Fetch one more to see if there should be a 'next' link data = logic.getForFields(filter=filter, limit=limit+1, offset=offset, order=order) @@ -189,46 +235,61 @@ return None more = len(data) > limit - - if more: - del data[limit:] - - newest = next = prev = export_link = '' - - base_params = dict(i for i in request.GET.iteritems() if - i[0].startswith('offset_') or i[0].startswith('limit_')) - - if params.get('list_key_order'): - export_link = generateLinkForRequest(request, base_params, {'export' : idx}) + if reverse_direction: + data.reverse() if more: - # TODO(dbentley): here we need to implement a new field "last_key" - next = generateLinkForRequest(request, base_params, {offset_key : offset+limit, - limit_key : limit}) + if reverse_direction: + data = data[1:] + else: + data = data[:limit] + + should_have_next_link = True + if not reverse_direction and not more: + should_have_next_link = False - if offset > 0: - # TODO(dbentley): here we need to implement previous in the good way. - prev = generateLinkForRequest(request, base_params, - { offset_key : max(0, offset-limit), - limit_key : limit }) + # Calculating should_have_previous_link is tricky. It's possible we could + # be creating a previous link to a page that would have 0 entities. + # That would be suboptimal; what's a better way? + should_have_previous_link = False + if offset_linkid: + should_have_previous_link = True + if reverse_direction and not more: + should_have_previous_link = False - if offset > limit: - # Having a link to the first doesn't make sense on the first page (we're on - # it). It also doesn't make sense on the second page (because the first - # page is the previous page). + if data: + first_displayed_item = data[0] + last_displayed_item = data[-1] + else: + class Dummy(object): + pass + first_displayed_item = last_displayed_item = Dummy() + first_displayed_item.link_id = None + newest = next = prev = export_link = '' + + link_creator = LinkCreator(request, idx, limit) - # NOTE(dbentley): I personally disagree that it's simpler to do that way, - # because sometimes you want to go to the first page without having to - # consider what page you're on now. - newest = generateLinkForGetArgs(request, base_params, {offset_key : 0, - limit_key : limit}) + if params.get('list_key_order'): + export_link = link_creator.create(export=True) + + if should_have_next_link: + next = link_creator.create(offset_linkid=last_displayed_item.link_id) + + if should_have_previous_link: + prev = link_creator.create(offset_linkid=first_displayed_item.link_id, + reverse_direction=True) + + newest = link_creator.create(offset_linkid='') + + # TODO(dbentley): add a "last" link (which is now possible because we can + # query with a reverse linkid sorting content = { 'idx': idx, 'data': data, 'export': export_link, - 'first': offset+1, - 'last': len(data) > 1 and offset+len(data) or None, + 'first': first_displayed_item.link_id, + 'last': last_displayed_item.link_id, 'logic': logic, 'limit': limit, 'newest': newest,