Paginator broken for one element and allow_empty_first_page=False

[Fidel Ramos]

I'm using the object_list generic view like this:

def galleryitem_list(request, shop, page):
    queryset = GalleryItem.objects.filter(shop__slug=shop)
    kwargs = {
        'paginate_by': 10,
        'page': page,
        'template_object_name': 'galleryitem',
        'allow_empty': False,
    }
    return list_detail.object_list(request, queryset, **kwargs)

I want to get an Http404 when the queryset is empty, but I'm getting it also when the queryset has exactly one object.

This fails in all my object_list views in the project, not just this one.

I've tracked down the problem to the [source:django/core/paginator.py:46 line 46 of paginator.py], where hits is 0 instead of 1.

[Fidel Ramos]

I made an error when linking to the paginator.py file. I intended to link to line 46 inside the file, but ended up linking to revision 46, so the link is broken. ​This is the right link.

[siudesign]

The problem is that the _get_num_pages method in Paginator currently returns 0 if your query returns 1 result and has allow_empty=False set. This returns the number to the validate_number method, which then throws an "Invalid Page" exception resulting in the 404 that you see, regardless of whether 0 or 1 results have been returned.

This is caused by the following line of code, which takes a number so in this case '1' and sets hits to '0'

 hits = self.count - 1 - self.orphans

The offending code however is this, which sees that hits are equal to 0 (even when there is 1 result) and that allow_empty=False and then goes ahead and sets num_pages to 0, which is fine if there truly is no matching query, but not if the query returns one result.

if hits == 0 and not self.allow_empty_first_page:
    self._num_pages = 0

You would not get the problem if you had allow_empty set to true, as there is code in the validate_number method which takes that variation of the setting into account.

My solution, in order to have minimal impact is to set self.num_pages to self.count, which by the point it enters the code above, can only be 1 or 0.

It works on my machine for all cases (0 results, 1 result, more than 1 result, allow_empty=True, allow_empty=False) etc.

[Fidel Ramos]

siudesign, the patch works for me too, at least in the tests I've done. Good work!

I think more unit tests for the Paginator may be needed if a bug like this has been undetected until now.

[Marc Fargas]

Could you add a TestCase on the patch that reproduces the bug (and demostrates it's fixed)?
If not, please unassign the ticket.

[simeon]

Added tests which fail against svn head but suceed with paginator2.diff applied. The patch also needed to handle the orphan parameter so I rewrote the method... In the first patch, consider the case where orphans = 2 and self.count = 1. self.num_pages would be set to 1 even though based on the orphans setting we should have an empty page...

[simeon]

Updated patch to hande orphans param, match svn head and add tests

[Malcolm Tredinnick]

I might be missing something, but isn't this replacing one problem with another one? If self.count is 1 and self.orphans is 2, it looks like the page count will be 0, when it should be 1.

[Gary Wilson]

(In [8129]) Fixed #6997 -- Corrected num_pages calculation when one item is in the object list and allow_empty_first_page=False, thanks to framos for the report. Also, made Page's start_index() return 0 if there are no items in the object list (previously it was returning 1, but now it is consistent with end_index()). As an added bonus, I threw in quite a few pagination tests.

[Jacob]

Milestone 1.0 deleted