Don't return marker item when paginating backwards

In emulated pagination mode, we should not return the marker when
paginating backwards (page_reverse), same as we do when we paginate
forward, or when we paginate backwards with a plugin that supports
native pagination.

Closes-Bug: #1591981
Change-Id: I8b553ab22846122dde22372f9901e46a5276ee8e

neutron/api/api_common.py

@@ -222,15 +222,31 @@ class PaginationEmulatedHelper(PaginationHelper):
     def paginate(self, items):
         if not self.limit:
             return items
-        i = -1
+
+        if not items:
+            return []
+
+        # first, calculate the base index for pagination
         if self.marker:
+            i = 0
             for item in items:
-                i = i + 1
                 if item[self.primary_key] == self.marker:
                     break
+                i += 1
+            else:
+                # if marker is not found, return nothing
+                return []
+        else:
+            i = len(items) if self.page_reverse else 0
+
         if self.page_reverse:
-            return items[i - self.limit:i]
-        return items[i + 1:i + self.limit + 1]
+            # don't wrap
+            return items[max(i - self.limit, 0):i]
+        else:
+            if self.marker:
+                # skip the matched marker
+                i += 1
+            return items[i:i + self.limit]
 
     def get_links(self, items):
         return get_pagination_links(

neutron/tests/unit/api/test_api_common.py

@@ -121,3 +121,100 @@ class APICommonTestCase(base.BaseTestCase):
         e = n_exc.MultipleExceptions([])
         conv = common.convert_exception_to_http_exc(e, base_v2.FAULT_MAP, None)
         self.assertIsInstance(conv, exc.HTTPInternalServerError)
+
+
+class FakeRequest(object):
+    def __init__(self, **kwargs):
+        if 'page_reverse' in kwargs:
+            kwargs['page_reverse'] = str(kwargs['page_reverse'])
+
+        self.kwargs = kwargs
+
+    def __getattribute__(self, name):
+        if name == 'GET':
+            return self.kwargs
+        return super(FakeRequest, self).__getattribute__(name)
+
+
+class _PaginationEmulatedHelperTest(object):
+
+    def test_paginate_no_limit_no_items(self):
+        req = FakeRequest()
+        helper = common.PaginationEmulatedHelper(req)
+        self.assertEqual([], helper.paginate([]))
+
+    def test_paginate_no_limit_several_items(self):
+        req = FakeRequest()
+        helper = common.PaginationEmulatedHelper(req)
+        self.assertEqual(self.items, helper.paginate(self.items))
+
+    def test_paginate_limit_1(self):
+        req = FakeRequest(limit=1)
+        helper = common.PaginationEmulatedHelper(req)
+        self.assertEqual(self.items[:1], helper.paginate(self.items))
+
+    def test_paginate_limit_1_page_reverse(self):
+        req = FakeRequest(limit=1, page_reverse=True)
+        helper = common.PaginationEmulatedHelper(req)
+        self.assertEqual(self.items[-1:], helper.paginate(self.items))
+
+    def test_paginate_high_limit_page_reverse(self):
+        req = FakeRequest(limit=len(self.items) + 100, page_reverse=True)
+        helper = common.PaginationEmulatedHelper(req)
+        self.assertEqual(self.items, helper.paginate(self.items))
+
+    def test_paginate_limit_higher_than_nitems(self):
+        req = FakeRequest(limit=len(self.items) + 100)
+        helper = common.PaginationEmulatedHelper(req)
+        self.assertEqual(self.items, helper.paginate(self.items))
+
+    def test_paginate_bad_marker(self):
+        req = FakeRequest(limit=1, marker='unknown-id')
+        helper = common.PaginationEmulatedHelper(req)
+        self.assertEqual([], helper.paginate(self.items))
+
+
+class PaginationEmulatedHelperTest(_PaginationEmulatedHelperTest,
+                                   base.BaseTestCase):
+
+    items = [
+        {'id': id_}
+        for id_ in ('', 'id1', '#12', 'fake', 'foo', 'bar')
+    ]
+
+    def test_paginate_marker(self):
+        req = FakeRequest(limit=1, marker=self.items[1]['id'])
+        helper = common.PaginationEmulatedHelper(req)
+        self.assertEqual(self.items[2:3], helper.paginate(self.items))
+
+    def test_paginate_marker_page_reverse(self):
+        req = FakeRequest(
+            limit=1, marker=self.items[1]['id'], page_reverse=True)
+        helper = common.PaginationEmulatedHelper(req)
+        self.assertEqual(self.items[:1], helper.paginate(self.items))
+
+    def test_paginate_dont_wrap(self):
+        req = FakeRequest(limit=100, marker=self.items[1]['id'])
+        helper = common.PaginationEmulatedHelper(req)
+        self.assertEqual(self.items[2:], helper.paginate(self.items))
+
+    def test_paginate_dont_wrap_page_reverse(self):
+        req = FakeRequest(
+            limit=100, marker=self.items[1]['id'], page_reverse=True)
+        helper = common.PaginationEmulatedHelper(req)
+        self.assertEqual(self.items[:1], helper.paginate(self.items))
+
+    def test_custom_primary_key(self):
+        items = [
+            {'fake_id': item['id']}
+            for item in self.items
+        ]
+        req = FakeRequest(limit=2, marker=self.items[1]['id'])
+        helper = common.PaginationEmulatedHelper(req, primary_key='fake_id')
+        self.assertEqual(items[2:4], helper.paginate(items))
+
+
+class PaginationEmulatedHelperEmptyItemsTest(_PaginationEmulatedHelperTest,
+                                             base.BaseTestCase):
+
+    items = []