-
Notifications
You must be signed in to change notification settings - Fork 28
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Create new array pagination, and apply it to path contents #1009
Merged
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
f76dba5
first round of tests
RulaKhaled 4cc54fd
Update path contents to reflect path content connection, create new a…
RulaKhaled c561b77
Update connection.py
RulaKhaled 5512b33
flip the condition
RulaKhaled 713dee1
Fix lint issues
RulaKhaled 90cf132
resolve Jerry's comments
RulaKhaled 7c433bf
update with one more test
RulaKhaled 5b18f31
Merge branch 'main' into array-pagination
RulaKhaled 02a5361
more tests
RulaKhaled File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,12 @@ | ||
import enum | ||
from dataclasses import dataclass | ||
from functools import cached_property | ||
from typing import Any, Dict, List, Optional | ||
|
||
from cursor_pagination import CursorPage, CursorPaginator | ||
from django.db.models import QuerySet | ||
|
||
from codecov.commands.exceptions import ValidationError | ||
from codecov.db import sync_to_async | ||
from graphql_api.types.enums import OrderingDirection | ||
|
||
|
@@ -68,6 +70,116 @@ def page_info(self, *args, **kwargs): | |
} | ||
|
||
|
||
class ArrayPaginator: | ||
"""Cursor-based paginator for in-memory arrays.""" | ||
|
||
def __init__( | ||
self, | ||
data: List[Any], | ||
first: Optional[int] = None, | ||
last: Optional[int] = None, | ||
after: Optional[str] = None, | ||
before: Optional[str] = None, | ||
): | ||
self.data = data | ||
self.start_index = 0 | ||
self.end_index = len(data) | ||
|
||
if first and last: | ||
raise ValidationError("Cannot provide both 'first' and 'last'") | ||
|
||
if after is not None: | ||
try: | ||
self.start_index = int(after) + 1 | ||
except ValueError: | ||
raise ValidationError("'after' cursor must be an integer") | ||
|
||
if before is not None: | ||
try: | ||
self.end_index = min(self.end_index, int(before)) | ||
except ValueError: | ||
raise ValidationError("'before' cursor must be an integer") | ||
|
||
# Ensure valid bounds after 'after' and 'before' | ||
self.start_index = max(self.start_index, 0) | ||
self.end_index = min(self.end_index, len(data)) | ||
|
||
if first is not None: | ||
self.end_index = min(self.start_index + first, len(data)) | ||
|
||
if last is not None: | ||
range_length = self.end_index - self.start_index | ||
if range_length > last: | ||
self.start_index = self.end_index - last | ||
|
||
# Ensure bounds remain valid | ||
self.start_index = max(self.start_index, 0) | ||
self.end_index = min(self.end_index, len(data)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This final safe guard is great! |
||
|
||
def cursor(self, position: int) -> str: | ||
"""Generate a cursor based on the position (index).""" | ||
return str(position) | ||
|
||
@property | ||
def page(self) -> List[Any]: | ||
"""Returns the sliced page of data.""" | ||
return self.data[self.start_index : self.end_index] | ||
|
||
@property | ||
def has_next(self) -> bool: | ||
"""Check if there's a next page.""" | ||
return self.end_index < len(self.data) | ||
|
||
@property | ||
def has_previous(self) -> bool: | ||
"""Check if there's a previous page.""" | ||
return self.start_index > 0 | ||
|
||
|
||
class ArrayConnection: | ||
"""Connection wrapper for array pagination.""" | ||
|
||
def __init__(self, paginator: ArrayPaginator): | ||
self.data = paginator.data | ||
self.paginator = paginator | ||
self.page = paginator.page | ||
|
||
@property | ||
def edges(self) -> List[Dict[str, Any]]: | ||
"""Generate edges with cursor and node information""" | ||
return [ | ||
{"cursor": self.paginator.cursor(pos), "node": node} | ||
for pos, node in enumerate(self.page) | ||
] | ||
|
||
@property | ||
def total_count(self) -> int: | ||
"""Total number of items in the original data""" | ||
return len(self.data) | ||
|
||
@property | ||
def start_cursor(self) -> Optional[str]: | ||
"""Cursor for the first item in the page""" | ||
return self.paginator.cursor(self.paginator.start_index) if self.page else None | ||
|
||
@property | ||
def end_cursor(self) -> Optional[str]: | ||
"""Cursor for the last item in the page""" | ||
return ( | ||
self.paginator.cursor(self.paginator.end_index - 1) if self.page else None | ||
) | ||
|
||
@property | ||
def page_info(self) -> Dict[str, Any]: | ||
"""Pagination information""" | ||
return { | ||
"has_next_page": self.paginator.has_next, | ||
"has_previous_page": self.paginator.has_previous, | ||
"start_cursor": self.start_cursor, | ||
"end_cursor": self.end_cursor, | ||
} | ||
|
||
|
||
class DictCursorPaginator(CursorPaginator): | ||
""" | ||
WARNING: DictCursorPaginator does not work for dict objects where a key contains the following string: "__" | ||
|
@@ -112,26 +224,33 @@ def position_from_instance(self, instance): | |
|
||
|
||
def queryset_to_connection_sync( | ||
queryset, | ||
data: QuerySet | list, | ||
*, | ||
ordering, | ||
ordering_direction, | ||
ordering=None, | ||
ordering_direction=None, | ||
first=None, | ||
after=None, | ||
last=None, | ||
before=None, | ||
): | ||
""" | ||
A method to take a queryset and return it in paginated order based on the cursor pattern. | ||
A method to take a queryset or an array and return it in paginated order based on the cursor pattern. | ||
Handles both QuerySets (database queries) and arrays (in-memory data). | ||
""" | ||
if not first and not last: | ||
first = 25 | ||
|
||
ordering = tuple(field_order(field, ordering_direction) for field in ordering) | ||
paginator = DictCursorPaginator(queryset, ordering=ordering) | ||
page = paginator.page(first=first, after=after, last=last, before=before) | ||
if isinstance(data, list): | ||
array_paginator = ArrayPaginator( | ||
data, first=first, last=last, after=after, before=before | ||
) | ||
return ArrayConnection(array_paginator) | ||
|
||
return Connection(queryset, paginator, page) | ||
else: | ||
ordering = tuple(field_order(field, ordering_direction) for field in ordering) | ||
paginator = DictCursorPaginator(data, ordering=ordering) | ||
page = paginator.page(first=first, after=after, last=last, before=before) | ||
return Connection(data, paginator, page) | ||
|
||
|
||
@sync_to_async | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was debating between using stringified random cursors and numeric ones for pagination. I decided to go with numeric cursors because they are simple, efficient, and directly map to the array indices (we have cases where customers have over 1,000 files), making pagination faster and easier to debug. Since the dataset is static, there’s no need for the added complexity or security of random strings. If you have other perspectives, lmk