Skip to content

Commit

Permalink
Merge branch 'master' into sameen/ENT-8351
Browse files Browse the repository at this point in the history
  • Loading branch information
zamanafzal authored Feb 12, 2024
2 parents cc9e558 + 9f8052a commit 56dd2df
Show file tree
Hide file tree
Showing 18 changed files with 535 additions and 192 deletions.
2 changes: 0 additions & 2 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,6 @@ private.py

docs/_build/

scripts/

.vscode/

.dev/
Expand Down
4 changes: 2 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,8 @@ upgrade: piptools $(COMMON_CONSTRAINTS_TXT) ## update the requirements/*.txt fil
# This is a temporary solution to override the real common_constraints.txt
# In edx-lint, until the pyjwt constraint in edx-lint has been removed.
# See BOM-271 for more details.
sed -i.'' 's/Django<4.0//g' requirements/common_constraints.txt
sed -i.'' 's/django-simple-history==3.0.0//g' requirements/common_constraints.txt
sed -i 's/Django<4.0//g' requirements/common_constraints.txt
sed -i 's/django-simple-history==3.0.0//g' requirements/common_constraints.txt
# Make sure to compile files after any other files they include!
pip-compile --upgrade --rebuild --allow-unsafe -o requirements/pip.txt requirements/pip.in
pip-compile --upgrade -o requirements/pip-tools.txt requirements/pip-tools.in
Expand Down
72 changes: 72 additions & 0 deletions license_manager/apps/api/pagination.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
"""
Defines custom paginators used by subscription viewsets.
"""
from django.core.paginator import Paginator as DjangoPaginator
from django.utils.functional import cached_property
from rest_framework.pagination import PageNumberPagination


class PageNumberPaginationWithCount(PageNumberPagination):
"""
A PageNumber paginator that adds the total number of pages to the paginated response.
"""

def get_paginated_response(self, data):
""" Adds a ``num_pages`` field into the paginated response. """
response = super().get_paginated_response(data)
response.data['num_pages'] = self.page.paginator.num_pages
return response


class LicensePagination(PageNumberPaginationWithCount):
"""
A PageNumber paginator that allows the client to specify the page size, up to some maximum.
"""
page_size_query_param = 'page_size'
max_page_size = 500


class EstimatedCountDjangoPaginator(DjangoPaginator):
"""
A lazy paginator that determines it's count from
the upstream `estimated_count`
"""
def __init__(self, *args, estimated_count=None, **kwargs):
self.estimated_count = estimated_count
super().__init__(*args, **kwargs)

@cached_property
def count(self):
if self.estimated_count is None:
return super().count
return self.estimated_count


class EstimatedCountLicensePagination(LicensePagination):
"""
Allows the caller (probably the `paginator()` property
of an upstream Viewset) to provided an `estimated_count`,
which means the downstream django paginator does *not*
perform an additional query to get the count of the queryset.
"""
def __init__(self, *args, estimated_count=None, **kwargs):
"""
Optionally stores an `estimated_count` to pass along
to `EstimatedCountDjangoPaginator`.
"""
self.estimated_count = estimated_count
super().__init__(*args, **kwargs)

def django_paginator_class(self, queryset, page_size):
"""
This only works because the implementation of `paginate_queryset`
treats `self.django_paginator_class` as if it is simply a callable,
and not necessarily a class, that returns a Django Paginator instance.
It also (safely) relies on `self` having an instance variable called `estimated_count`.
"""
if self.estimated_count is not None:
return EstimatedCountDjangoPaginator(
queryset, page_size, estimated_count=self.estimated_count,
)
return DjangoPaginator(queryset, page_size)
29 changes: 29 additions & 0 deletions license_manager/apps/api/v1/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@
LEARNER_ROLES,
SUBSCRIPTION_RENEWAL_DAYS_OFFSET,
)
from license_manager.apps.api.v1.views import (
ESTIMATED_COUNT_PAGINATOR_THRESHOLD,
)
from license_manager.apps.core.models import User
from license_manager.apps.subscriptions import constants
from license_manager.apps.subscriptions.exceptions import LicenseRevocationError
Expand Down Expand Up @@ -788,6 +791,32 @@ def test_license_list_staff_user_200_custom_page_size(api_client, staff_user):
assert response.data['next'] is not None


@pytest.mark.django_db
def test_license_list_staff_user_200_estimated_license_count(api_client, staff_user):
subscription, _, _, _, _ = _subscription_and_licenses()
_assign_role_via_jwt_or_db(
api_client,
staff_user,
subscription.enterprise_customer_uuid,
True,
)
subscription.desired_num_licenses = ESTIMATED_COUNT_PAGINATOR_THRESHOLD + 1
subscription.save()

response = _licenses_list_request(
api_client, subscription.uuid, page_size=1,
status=','.join([constants.UNASSIGNED, constants.ASSIGNED, constants.ACTIVATED]),
)

assert status.HTTP_200_OK == response.status_code
results_by_uuid = {item['uuid']: item for item in response.data['results']}
# We test for content in the test above,
# we're only worried about the response count matching `desired_num_licenses` here.
assert len(results_by_uuid) == 1
assert response.data['count'] == ESTIMATED_COUNT_PAGINATOR_THRESHOLD + 1
assert response.data['next'] is not None


@pytest.mark.django_db
def test_license_list_ignore_null_emails_query_param(api_client, staff_user, boolean_toggle):
"""
Expand Down
59 changes: 38 additions & 21 deletions license_manager/apps/api/v1/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
from rest_framework.decorators import action
from rest_framework.exceptions import ParseError
from rest_framework.mixins import ListModelMixin
from rest_framework.pagination import PageNumberPagination
from rest_framework.response import Response
from rest_framework.views import APIView
from rest_framework_csv.renderers import CSVRenderer
Expand Down Expand Up @@ -68,11 +67,15 @@
localized_utcnow,
)

from ..pagination import EstimatedCountLicensePagination, LicensePagination


logger = logging.getLogger(__name__)

ASSIGNMENT_LOCK_TIMEOUT_SECONDS = 300

ESTIMATED_COUNT_PAGINATOR_THRESHOLD = 10000


class CustomerAgreementViewSet(
PermissionRequiredForListingMixin,
Expand Down Expand Up @@ -485,26 +488,6 @@ def base_queryset(self):
return licenses


class PageNumberPaginationWithCount(PageNumberPagination):
"""
A PageNumber paginator that adds the total number of pages to the paginated response.
"""

def get_paginated_response(self, data):
""" Adds a ``num_pages`` field into the paginated response. """
response = super().get_paginated_response(data)
response.data['num_pages'] = self.page.paginator.num_pages
return response


class LicensePagination(PageNumberPaginationWithCount):
"""
A PageNumber paginator that allows the client to specify the page size, up to some maximum.
"""
page_size_query_param = 'page_size'
max_page_size = 500


class BaseLicenseViewSet(PermissionRequiredForListingMixin, viewsets.ReadOnlyModelViewSet):
"""
Base Viewset for read operations on individual licenses in a given subscription plan.
Expand Down Expand Up @@ -609,6 +592,40 @@ class LicenseAdminViewSet(BaseLicenseViewSet):

pagination_class = LicensePagination

@property
def paginator(self):
# pylint: disable=line-too-long
"""
If the caller has requested all usable licenses, we want to fall
back to grabbing the paginator's count from ``SubscriptionPlan.desired_num_licenses``
for large plans, as determining the count dynamically is an expensive query.
This is the only way to dynamically select a pagination class in DRF.
https://github.com/encode/django-rest-framework/issues/6397
Underlying implementation of the paginator() property:
https://github.com/encode/django-rest-framework/blob/7749e4e3bed56e0f3e7775b0b1158d300964f6c0/rest_framework/generics.py#L156
"""
if hasattr(self, '_paginator'):
return self._paginator

# If we don't have a subscription plan, or the requested
# status values aren't for all usable licenses, fall back to
# the normal LicensePagination class
self._paginator = super().paginator # pylint: disable=attribute-defined-outside-init

usable_license_states = {constants.UNASSIGNED, constants.ASSIGNED, constants.ACTIVATED}
if value := self.request.query_params.get('status'):
status_values = value.strip().split(',')
if set(status_values) == usable_license_states:
if subscription_plan := self._get_subscription_plan():
estimated_count = subscription_plan.desired_num_licenses
if estimated_count is not None and estimated_count > ESTIMATED_COUNT_PAGINATOR_THRESHOLD:
# pylint: disable=attribute-defined-outside-init
self._paginator = EstimatedCountLicensePagination(estimated_count=estimated_count)

return self._paginator

@property
def active_only(self):
return int(self.request.query_params.get('active_only', 0))
Expand Down
3 changes: 3 additions & 0 deletions license_manager/apps/subscriptions/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,9 @@ def get_fields(self, request, obj=None):
'start_date',
'expiration_date',
)

autocomplete_fields = ['customer_agreement']

actions = ['process_unused_licenses_post_freeze']

def get_queryset(self, request):
Expand Down
2 changes: 1 addition & 1 deletion license_manager/apps/subscriptions/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ class SegmentEvents:

# Subscription validation constants
MIN_NUM_LICENSES = 0
MAX_NUM_LICENSES = 11000
MAX_NUM_LICENSES = 1000000 # Set a reasonably high max to prevent us from crashing.

# Number of license uuids enrollments are expired for in each batch
LICENSE_EXPIRATION_BATCH_SIZE = 100
Expand Down
30 changes: 15 additions & 15 deletions requirements/base.txt
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ backports-zoneinfo[tzdata]==0.2.1
# kombu
billiard==4.2.0
# via celery
boto3==1.34.21
boto3==1.34.32
# via django-ses
botocore==1.34.21
botocore==1.34.32
# via
# boto3
# s3transfer
Expand Down Expand Up @@ -62,9 +62,9 @@ click-plugins==1.1.1
# via celery
click-repl==0.3.0
# via celery
code-annotations==1.5.0
code-annotations==1.6.0
# via edx-toggles
cryptography==41.0.7
cryptography==42.0.2
# via
# pyjwt
# social-auth-core
Expand Down Expand Up @@ -149,7 +149,7 @@ drf-jwt==1.19.2
# via edx-drf-extensions
drf-nested-routers==0.93.5
# via -r requirements/base.in
drf-spectacular==0.27.0
drf-spectacular==0.27.1
# via -r requirements/base.in
edx-auth-backends==4.2.0
# via -r requirements/base.in
Expand All @@ -163,7 +163,7 @@ edx-django-utils==5.10.1
# edx-drf-extensions
# edx-rest-api-client
# edx-toggles
edx-drf-extensions==9.1.2
edx-drf-extensions==10.1.0
# via
# -r requirements/base.in
# edx-rbac
Expand All @@ -173,7 +173,7 @@ edx-rbac==1.8.0
# via -r requirements/base.in
edx-rest-api-client==5.6.1
# via -r requirements/base.in
edx-toggles==5.1.0
edx-toggles==5.1.1
# via -r requirements/base.in
idna==3.6
# via requests
Expand All @@ -191,19 +191,19 @@ jmespath==1.0.1
# botocore
jsonfield==3.1.0
# via edx-celeryutils
jsonschema==4.21.0
jsonschema==4.21.1
# via drf-spectacular
jsonschema-specifications==2023.12.1
# via jsonschema
kombu==5.3.5
# via celery
markupsafe==2.1.3
markupsafe==2.1.4
# via jinja2
monotonic==1.6
# via analytics-python
mysqlclient==2.2.1
# via -r requirements/base.in
newrelic==9.5.0
newrelic==9.6.0
# via edx-django-utils
oauthlib==3.2.2
# via
Expand All @@ -217,7 +217,7 @@ ply==3.11
# via djangoql
prompt-toolkit==3.0.43
# via click-repl
psutil==5.9.7
psutil==5.9.8
# via edx-django-utils
pycparser==2.21
# via cffi
Expand All @@ -238,11 +238,11 @@ python-dateutil==2.8.2
# analytics-python
# botocore
# celery
python-slugify==8.0.1
python-slugify==8.0.3
# via code-annotations
python3-openid==3.2.0
# via social-auth-core
pytz==2023.3.post1
pytz==2023.4
# via
# -r requirements/base.in
# django-ses
Expand All @@ -253,7 +253,7 @@ pyyaml==6.0.1
# drf-spectacular
redis==5.0.1
# via -r requirements/base.in
referencing==0.32.1
referencing==0.33.0
# via
# jsonschema
# jsonschema-specifications
Expand Down Expand Up @@ -290,7 +290,7 @@ slumber==0.7.1
# via edx-rest-api-client
social-auth-app-django==5.4.0
# via edx-auth-backends
social-auth-core==4.5.1
social-auth-core==4.5.2
# via
# edx-auth-backends
# social-auth-app-django
Expand Down
28 changes: 0 additions & 28 deletions requirements/common_constraints.txt.

This file was deleted.

Loading

0 comments on commit 56dd2df

Please sign in to comment.