Repository: ucfopen/canvasapi

Issue:
Negative index on PaginatedList causes unexpected results
# Describe the bug

It is expected that you cannot negative-index the PaginatedList due to its inability to know its length until it is full. However, it is still possible to use the negative index if at least one page of results has been loaded. When this happens, the list returns the last value it has loaded (which is not necessarily the last value it can load).

# To Reproduce

Steps to reproduce the behavior:

```python
users = canvas.get_course(1).get_users()
users[1]
users[-1]
```

# Expected behavior

I am not allowed to use negative indexing (wishlist, I *can* use negative indexing :D)

# Environment information (please complete the following information)

- Python 3.7.0
- CanvasAPI 0.14.0

# Additional context

I'll try my hand at fixing this.


Discussion:

Patch:
diff --git a/canvasapi/paginated_list.py b/canvasapi/paginated_list.py
index 21cceaa..90ceade 100644
--- a/canvasapi/paginated_list.py
+++ b/canvasapi/paginated_list.py
@@ -11,6 +11,8 @@ class PaginatedList(object):
     def __getitem__(self, index):
         assert isinstance(index, (int, slice))
         if isinstance(index, int):
+            if index < 0:
+                raise IndexError("Cannot negative index a PaginatedList")
             self._get_up_to_index(index)
             return self._elements[index]
         else:
@@ -105,6 +107,9 @@ class PaginatedList(object):
             self._stop = the_slice.stop
             self._step = the_slice.step or 1
 
+            if self._start < 0 or self._stop < 0:
+                raise IndexError("Cannot negative index a PaginatedList slice")
+
         def __iter__(self):
             index = self._start
             while not self._finished(index):


Test patch:
diff --git a/tests/test_paginated_list.py b/tests/test_paginated_list.py
index 570faa8..99fa473 100644
--- a/tests/test_paginated_list.py
+++ b/tests/test_paginated_list.py
@@ -170,3 +170,36 @@ class TestPaginatedList(unittest.TestCase):
         )
 
         self.assertIsInstance(pag_list[0], EnrollmentTerm)
+
+    def test_negative_index(self, m):
+        # Regression test for https://github.com/ucfopen/canvasapi/issues/305
+        # Ensure that we can't use negative indexing, even after loading a page
+
+        register_uris({"paginated_list": ["4_2_pages_p1", "4_2_pages_p2"]}, m)
+        pag_list = PaginatedList(User, self.requester, "GET", "four_objects_two_pages")
+        pag_list[0]
+
+        with self.assertRaises(IndexError):
+            pag_list[-1]
+
+    def test_negative_index_for_slice_start(self, m):
+        # Regression test for https://github.com/ucfopen/canvasapi/issues/305
+        # Ensure that we can't slice using a negative index as the start item
+
+        register_uris({"paginated_list": ["4_2_pages_p1", "4_2_pages_p2"]}, m)
+        pag_list = PaginatedList(User, self.requester, "GET", "four_objects_two_pages")
+        pag_list[0]
+
+        with self.assertRaises(IndexError):
+            pag_list[-1:1]
+
+    def test_negative_index_for_slice_end(self, m):
+        # Regression test for https://github.com/ucfopen/canvasapi/issues/305
+        # Ensure that we can't slice using a negative index as the end item
+
+        register_uris({"paginated_list": ["4_2_pages_p1", "4_2_pages_p2"]}, m)
+        pag_list = PaginatedList(User, self.requester, "GET", "four_objects_two_pages")
+        pag_list[0]
+
+        with self.assertRaises(IndexError):
+            pag_list[:-1]