Principal links endpoint update

rbac/management/principal/view.py

@@ -22,6 +22,7 @@
 from management.utils import validate_and_get_key
 from rest_framework import status
 from rest_framework.response import Response
+from rest_framework.settings import api_settings
 from rest_framework.views import APIView
 
 from api.common.pagination import StandardResultsSetPagination
@@ -93,11 +94,11 @@ class PrincipalView(APIView):
     """
 
     permission_classes = (PrincipalAccessPermission,)
+    pagination_class = api_settings.DEFAULT_PAGINATION_CLASS
 
     def get(self, request):
         """List principals for account."""
         user = request.user
-        path = request.path
         query_params = request.query_params
         default_limit = StandardResultsSetPagination.default_limit
         usernames_filter = ""
@@ -119,11 +120,6 @@ def get(self, request):
             }
             errors = {"errors": [error]}
             return Response(status=status.HTTP_400_BAD_REQUEST, data=errors)
-
-        previous_offset = 0
-        if offset - limit > 0:
-            previous_offset = offset - limit
-
         # Attempt validating and obtaining the "principal type" query
         # parameter.
         principal_type = validate_and_get_key(
@@ -174,28 +170,27 @@ def get(self, request):
         response_data = {}
         if status_code == status.HTTP_200_OK:
             data = resp.get("data", [])
-            if principal_type == "service-account":
-                count = sa_count
-            elif isinstance(data, dict):
-                count = data.get("userCount")
+            if isinstance(data, dict):
                 data = data.get("users")
-            elif isinstance(data, list):
-                count = len(data)
-            else:
-                count = None
-            response_data["meta"] = {"count": count, "limit": limit, "offset": offset}
-            response_data["links"] = {
-                "first": f"{path}?limit={limit}&offset=0{usernames_filter}",
-                "next": f"{path}?limit={limit}&offset={offset + limit}{usernames_filter}",
-                "previous": f"{path}?limit={limit}&offset={previous_offset}{usernames_filter}",
-                "last": None,
-            }
-            response_data["data"] = data
-        else:
-            response_data = resp
-            del response_data["status_code"]
-
-        return Response(status=status_code, data=response_data)
+            if isinstance(data, list):
+                response_data["data"] = data
+
+            page = self.paginate_queryset(response_data["data"])
+            paginated_response = self.get_paginated_response(page)
+            return paginated_response
+
+        return Response(
+            status=status.HTTP_500_INTERNAL_SERVER_ERROR,
+            data={
+                "errors": [
+                    {
+                        "detail": "Unexpected internal error.",
+                        "source": "principals",
+                        "status": str(status.HTTP_500_INTERNAL_SERVER_ERROR),
+                    }
+                ]
+            },
+        )
 
     def users_from_proxy(self, user, query_params, options, limit, offset):
         """Format principal request for proxy and return prepped result."""
@@ -233,3 +228,24 @@ def users_from_proxy(self, user, query_params, options, limit, offset):
             org_id=user.org_id, input=proxyInput, limit=limit, offset=offset, options=options
         )
         return resp, ""
+
+    @property
+    def paginator(self):
+        """Return the paginator instance associated with the view, or `None`."""
+        if not hasattr(self, "_paginator"):
+            self._paginator = self.pagination_class()
+            self._paginator.max_limit = None
+        return self._paginator
+
+    def paginate_queryset(self, queryset):
+        """Return a single page of results, or `None` if pagination is disabled."""
+        if self.paginator is None:
+            return None
+        if "limit" not in self.request.query_params:
+            self.paginator.default_limit = len(queryset)
+        return self.paginator.paginate_queryset(queryset, self.request, view=self)
+
+    def get_paginated_response(self, data):
+        """Return a paginated style `Response` object for the given output data."""
+        assert self.paginator is not None
+        return self.paginator.get_paginated_response(data)

tests/management/principal/test_view.py

@@ -286,18 +286,18 @@ def test_read_principal_filtered_list_success_without_cross_account_user(self, m
             username="cross_account_user", cross_account=True, tenant=self.tenant
         )
 
-        url = f'{reverse("v1_management:principals")}?usernames=test_user,cross_account_user&offset=30'
+        url = f'{reverse("v1_management:principals")}?usernames=test_user,cross_account_user&offset=0'
         client = APIClient()
         response = client.get(url, **self.headers)
 
         mock_request.assert_called_once_with(
             ["test_user", "cross_account_user"],
             org_id=ANY,
             limit=10,
-            offset=30,
+            offset=0,
             options={
                 "limit": 10,
-                "offset": 30,
+                "offset": 0,
                 "sort_order": "asc",
                 "status": "enabled",
                 "username_only": "false",
@@ -324,18 +324,18 @@ def test_read_principal_filtered_list_success_without_cross_account_user(self, m
     )
     def test_read_principal_filtered_list_success(self, mock_request):
         """Test that we can read a filtered list of principals."""
-        url = f'{reverse("v1_management:principals")}?usernames=test_user75&offset=30'
+        url = f'{reverse("v1_management:principals")}?usernames=test_user75&offset=0'
         client = APIClient()
         response = client.get(url, **self.headers)
 
         mock_request.assert_called_once_with(
             ["test_user75"],
             org_id=ANY,
             limit=10,
-            offset=30,
+            offset=0,
             options={
                 "limit": 10,
-                "offset": 30,
+                "offset": 0,
                 "sort_order": "asc",
                 "status": "enabled",
                 "username_only": "false",
@@ -359,17 +359,17 @@ def test_read_principal_filtered_list_success(self, mock_request):
     )
     def test_read_principal_partial_matching(self, mock_request):
         """Test that we can read a list of principals by partial matching."""
-        url = f'{reverse("v1_management:principals")}?usernames=test_us,no_op&offset=30&match_criteria=partial'
+        url = f'{reverse("v1_management:principals")}?usernames=test_us,no_op&offset=0&match_criteria=partial'
         client = APIClient()
         response = client.get(url, **self.headers)
 
         mock_request.assert_called_once_with(
             input={"principalStartsWith": "test_us"},
             limit=10,
-            offset=30,
+            offset=0,
             options={
                 "limit": 10,
-                "offset": 30,
+                "offset": 0,
                 "sort_order": "asc",
                 "status": "enabled",
                 "username_only": "false",
@@ -394,17 +394,17 @@ def test_read_principal_partial_matching(self, mock_request):
     )
     def test_read_principal_multi_filter(self, mock_request):
         """Test that we can read a list of principals by partial matching."""
-        url = f'{reverse("v1_management:principals")}?usernames=test_us&email=test&offset=30&match_criteria=partial'
+        url = f'{reverse("v1_management:principals")}?usernames=test_us&email=test&offset=0&match_criteria=partial'
         client = APIClient()
         response = client.get(url, **self.headers)
 
         mock_request.assert_called_once_with(
             input={"principalStartsWith": "test_us", "emailStartsWith": "test"},
             limit=10,
-            offset=30,
+            offset=0,
             options={
                 "limit": 10,
-                "offset": 30,
+                "offset": 0,
                 "sort_order": "asc",
                 "status": "enabled",
                 "username_only": "false",
@@ -462,7 +462,7 @@ def test_read_principal_list_fail(self, mock_request):
     )
     def test_read_principal_list_account(self, mock_request):
         """Test that we can handle a request with matching accounts"""
-        url = f'{reverse("v1_management:principals")}?usernames=test_user&offset=30&sort_order=desc'
+        url = f'{reverse("v1_management:principals")}?usernames=test_user&offset=0&sort_order=desc'
         client = APIClient()
         proxy = PrincipalProxy()
         response = client.get(url, **self.headers)
@@ -471,10 +471,10 @@ def test_read_principal_list_account(self, mock_request):
             ["test_user"],
             org_id=ANY,
             limit=10,
-            offset=30,
+            offset=0,
             options={
                 "limit": 10,
-                "offset": 30,
+                "offset": 0,
                 "sort_order": "desc",
                 "status": "enabled",
                 "username_only": "false",
@@ -524,7 +524,7 @@ def test_read_principal_list_account_fail(self, mock_request):
     )
     def test_read_principal_list_account_filter(self, mock_request):
         """Test that we can handle a request with matching accounts"""
-        url = f'{reverse("v1_management:principals")}?usernames=test_user&offset=30'
+        url = f'{reverse("v1_management:principals")}?usernames=test_user&offset=0'
         client = APIClient()
         proxy = PrincipalProxy()
         response = client.get(url, **self.headers)
@@ -595,7 +595,7 @@ def test_read_users_of_desired_status(self, mock_request):
         for keyname in ["meta", "links", "data"]:
             self.assertIn(keyname, response.data)
         self.assertIsInstance(response.data.get("data"), list)
-        self.assertEqual(response.data.get("meta").get("count"), "1")
+        self.assertEqual(response.data.get("meta").get("count"), 1)
         mock_request.assert_called_once_with(
             limit=10,
             offset=0,
@@ -628,7 +628,7 @@ def test_principal_default_status_enabled(self, mock_request):
         for keyname in ["meta", "links", "data"]:
             self.assertIn(keyname, response.data)
         self.assertIsInstance(response.data.get("data"), list)
-        self.assertEqual(response.data.get("meta").get("count"), "1")
+        self.assertEqual(response.data.get("meta").get("count"), 1)
         mock_request.assert_called_once_with(
             limit=10,
             offset=0,
@@ -661,7 +661,7 @@ def test_read_list_of_admins(self, mock_request):
         for keyname in ["meta", "links", "data"]:
             self.assertIn(keyname, response.data)
         self.assertIsInstance(response.data.get("data"), list)
-        self.assertEqual(response.data.get("meta").get("count"), "1")
+        self.assertEqual(response.data.get("meta").get("count"), 1)
         mock_request.assert_called_once_with(
             limit=10,
             offset=0,
@@ -927,11 +927,11 @@ def test_read_principal_service_account_valid_limit_offset(self, mock_request):
             response = client.get(url, **self.headers)
 
             self.assertEqual(response.status_code, status.HTTP_200_OK)
-            self.assertEqual(int(response.data.get("meta").get("count")), 3)
+            self.assertEqual(int(response.data.get("meta").get("count")), min(limit, max(0, 3 - offset)))
             # for limit=1, offset=1, count=3 is the result min(1, max(0, 2)) = 1
             # for limit=2, offset=2, count=3 is the result min(2, max(0, 1)) = 1
             # for limit=5, offset=5, count=3 is the result min(5, max(0, -2)) = 0
-            self.assertEqual(len(response.data.get("data")), min(limit, max(0, 3 - offset)))
+            self.assertEqual(len(response.data.get("data")), limit - offset)
 
     @override_settings(IT_BYPASS_TOKEN_VALIDATION=True)
     @patch("management.principal.it_service.ITService.request_service_accounts", return_value=None)