Merge pull request #195 from coderbydesign/remove-default-limit-on-ac…

…cess Remove default `limit` on `/access/` requests
RedHatInsights · Feb 5, 2020 · c16707b · c16707b
2 parents debb5b0 + 199c1fb
commit c16707b
Show file tree

Hide file tree

Showing 3 changed files with 110 additions and 8 deletions.
diff --git a/docs/source/specs/openapi.json b/docs/source/specs/openapi.json
@@ -1417,19 +1417,34 @@
             }
           },
           {
-            "$ref": "#/components/parameters/QueryLimit"
+            "$ref": "#/components/parameters/QueryLimitNoDefault"
           },
           {
             "$ref": "#/components/parameters/QueryOffset"
           }
         ],
         "responses": {
           "200": {
-            "description": "A paginated list of access objects",
+            "description": "A list of access objects. By default, this request will not be paginated. To received a paginated response, include the `/?limit=` query parameter.",
             "content": {
               "application/json": {
                 "schema": {
-                  "$ref": "#/components/schemas/AccessPagination"
+                  "oneOf": [
+                    {
+                      "$ref": "#/components/schemas/AccessNoPagination"
+                    },
+                    {
+                      "$ref": "#/components/schemas/AccessPagination"
+                    }
+                  ]
+                },
+                "examples": {
+                  "AccessNoPagination": {
+                    "$ref": "#/components/examples/AccessNoPagination"
+                  },
+                  "AccessPagination": {
+                    "$ref": "#/components/examples/AccessPagination"
+                  }
                 }
               }
             }
@@ -1491,6 +1506,17 @@
           "maximum": 1000
         }
       },
+      "QueryLimitNoDefault": {
+        "in": "query",
+        "name": "limit",
+        "required": false,
+        "description": "Parameter for selecting the amount of data returned.",
+        "schema": {
+          "type": "integer",
+          "minimum": 1,
+          "maximum": 1000
+        }
+      },
       "NameFilter": {
         "in": "query",
         "name": "name",
@@ -2214,6 +2240,24 @@
           }
         ]
       },
+      "AccessNoPagination": {
+        "allOf": [
+          {
+            "type": "object",
+            "required": [
+              "data"
+            ],
+            "properties": {
+              "data": {
+                "type": "array",
+                "items": {
+                  "$ref": "#/components/schemas/Access"
+                }
+              }
+            }
+          }
+        ]
+      },
       "Status": {
         "required": [
           "api_version"
@@ -2265,6 +2309,39 @@
           }
         }
       }
+    },
+    "examples": {
+      "AccessNoPagination": {
+        "value": {
+          "data": [
+            {
+              "permission": "app-name:*:*",
+              "resourceDefinitions": []
+            }
+          ]
+        }
+      },
+      "AccessPagination": {
+        "value": {
+          "meta": {
+            "count": 1,
+            "limit": 10,
+            "offset": 0
+          },
+          "links": {
+            "first": "/access/?application=app-name&limit=10",
+            "next": null,
+            "previous": null,
+            "last": "/access/?application=app-name&limit=10"
+          },
+          "data": [
+            {
+              "permission": "app-name:*:*",
+              "resourceDefinitions": []
+            }
+          ]
+        }
+      }
     }
   }
 }
diff --git a/rbac/management/access/view.py b/rbac/management/access/view.py
@@ -18,7 +18,6 @@
 """View for principal access."""
 from management.querysets import get_access_queryset
 from management.role.serializer import AccessSerializer
-from rest_framework import status
 from rest_framework.permissions import AllowAny
 from rest_framework.response import Response
 from rest_framework.settings import api_settings
@@ -83,17 +82,19 @@ def get_queryset(self):
 
     def get(self, request):
         """Provide access data for prinicpal."""
-        page = self.paginate_queryset(self.get_queryset())
+        queryset = self.get_queryset()
+        page = self.paginate_queryset(queryset)
         if page is not None:
             serializer = self.serializer_class(page, many=True)
             return self.get_paginated_response(serializer.data)
-        return Response(status=status.HTTP_500_INTERNAL_SERVER_ERROR)
+
+        return Response({'data': self.serializer_class(queryset, many=True).data})
 
     @property
     def paginator(self):
         """Return the paginator instance associated with the view, or `None`."""
         if not hasattr(self, '_paginator'):
-            if self.pagination_class is None:
+            if self.pagination_class is None or 'limit' not in self.request.query_params:
                 self._paginator = None
             else:
                 self._paginator = self.pagination_class()

diff --git a/tests/management/access/test_view.py b/tests/management/access/test_view.py
@@ -105,7 +105,7 @@ def create_policy(self, policy_name, group, roles, status=status.HTTP_201_CREATE
         return response
 
     def test_get_access_success(self):
-        """Test that we can obtain the expected access."""
+        """Test that we can obtain the expected access without pagination."""
         role_name = 'roleA'
         response = self.create_role(role_name)
         self.assertEqual(response.status_code, status.HTTP_201_CREATED)
@@ -123,6 +123,30 @@ def test_get_access_success(self):
         self.assertIsNotNone(response.data.get('data'))
         self.assertIsInstance(response.data.get('data'), list)
         self.assertEqual(len(response.data.get('data')), 1)
+        self.assertIsNone(response.data.get('meta'))
+        self.assertEqual(self.access_data, response.data.get('data')[0])
+
+    def test_get_access_with_limit(self):
+        """Test that we can obtain the expected access with pagination."""
+        role_name = 'roleA'
+        response = self.create_role(role_name)
+        self.assertEqual(response.status_code, status.HTTP_201_CREATED)
+        role_uuid = response.data.get('uuid')
+        policy_name = 'policyA'
+        response = self.create_policy(policy_name, self.group.uuid, [role_uuid])
+
+        # test that we can retrieve the principal access
+        url = '{}?application={}&username={}&limit=1'.format(reverse('access'),
+                                                     'app',
+                                                     self.principal.username)
+        client = APIClient()
+        response = client.get(url, **self.headers)
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        self.assertIsNotNone(response.data.get('data'))
+        self.assertIsInstance(response.data.get('data'), list)
+        self.assertEqual(len(response.data.get('data')), 1)
+        self.assertEqual(response.data.get('meta').get('count'), 1)
+        self.assertEqual(response.data.get('meta').get('limit'), 1)
         self.assertEqual(self.access_data, response.data.get('data')[0])
 
     def test_missing_query_params(self):