Skip to content

Commit

Permalink
fix #15
Browse files Browse the repository at this point in the history
added fix when count query is disabled via meta info for internal paginator
now internal paginator is capable of producing page response without executing count
query. A naive implementation is used with query offset plan
Though the initial plan was to make an external strategy and switch between them but
there will come issue when user starts implementating their own paginator strategies
and decided to use the count flag along with their custom implementation as
it becomes hard to differentiate between custom paginator and core paginator

that's why we kept core service paginator selector as raw as possible
i.e., get and set (get the strategy and set the strategy)
  • Loading branch information
danielhasan1 committed Jan 30, 2024
1 parent a5c845e commit 22e952f
Show file tree
Hide file tree
Showing 8 changed files with 83 additions and 22 deletions.
2 changes: 1 addition & 1 deletion fastapi_listing/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
__version__ = "0.3.0"
__version__ = "0.3.1"

__all__ = [
"ListingService",
Expand Down
11 changes: 9 additions & 2 deletions fastapi_listing/ctyping.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
"AnySqlAlchemyColumn",
"SqlAlchemyModel",
"BasePage",
"Page"
"Page",
"PageWithoutCount",
]

from typing import TypeVar, List, Dict, Union, Sequence, Generic
Expand Down Expand Up @@ -46,7 +47,13 @@ class BasePage(TypedDict):


class Page(BasePage):
hasNext: Union[bool, None] # None tells that we've turned off means to calculate this result due to absence of totalCount
hasNext: bool
totalCount: int
currentPageSize: int
currentPageNumber: int


class PageWithoutCount(BasePage):
hasNext: bool
currentPageSize: int
currentPageNumber: int
4 changes: 2 additions & 2 deletions fastapi_listing/paginator/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
__all__ = ["ListingPage", "BaseListingPage"]
__all__ = ["ListingPage", "BaseListingPage", "PaginationStrategy", "ListingPageWithoutCount"]

from fastapi_listing.paginator.page_builder import PaginationStrategy
from fastapi_listing.paginator.default_page_format import ListingPage, BaseListingPage
from fastapi_listing.paginator.default_page_format import ListingPage, BaseListingPage, ListingPageWithoutCount
6 changes: 6 additions & 0 deletions fastapi_listing/paginator/default_page_format.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,9 @@ class ListingPage(BaseListingPage[T], Generic[T]):
currentPageSize: int = Field(alias="currentPageSize")
currentPageNumber: int = Field(alias="currentPageNumber")
totalCount: int = Field(alias="totalCount")


class ListingPageWithoutCount(BaseListingPage[T], Generic[T]):
hasNext: bool = Field(alias="hasNext")
currentPageSize: int = Field(alias="currentPageSize")
currentPageNumber: int = Field(alias="currentPageNumber")
46 changes: 33 additions & 13 deletions fastapi_listing/paginator/page_builder.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from typing import Optional, Union

from fastapi_listing.abstracts import AbsPaginatingStrategy
from fastapi_listing.ctyping import SqlAlchemyQuery, FastapiRequest, Page, BasePage
from fastapi_listing.ctyping import SqlAlchemyQuery, FastapiRequest, Page, BasePage, PageWithoutCount
from fastapi_listing.errors import ListingPaginatorError


Expand Down Expand Up @@ -32,11 +32,12 @@ def get_count(self, query: SqlAlchemyQuery) -> int:
"""
return query.count()

def is_next_page_exists(self) -> Union[bool, None]:
def is_next_page_exists(self) -> bool:
"""expression results in bool val if count query allowed else None"""
if self.fire_count_qry:
return True if self.count - (self.page_num * self.page_size) > self.page_size else False
return None
else:
return self.count > self.page_size

def validate_params(self, page_num, page_size):
"""validate given 1 based page number and pagesize"""
Expand Down Expand Up @@ -64,7 +65,7 @@ def set_count(self, count: int):
self.count = count

def paginate(self, query: SqlAlchemyQuery, pagination_params: dict, extra_context: dict) -> BasePage:
"""Retrun paginated response"""
"""Return paginated response"""
page_num = pagination_params.get('page')
page_size = pagination_params.get('pageSize')
try:
Expand All @@ -75,24 +76,24 @@ def paginate(self, query: SqlAlchemyQuery, pagination_params: dict, extra_contex
self.set_page_num(page_num)
self.set_page_size(page_size)
self.set_extra_context(extra_context)
if self.fire_count_qry:
self.set_count(self.get_count(query))
return self.page(query)

def page(self, query: SqlAlchemyQuery) -> BasePage:
"""Return a Page or BasePage for given 1-based page number."""
has_next: Union[bool, None] = self.is_next_page_exists()
query = self._slice_query(query)
return self._get_page(has_next, query)
if self.fire_count_qry:
self.set_count(self.get_count(query))
has_next: bool = self.is_next_page_exists()
query = self._slice_query(query)
return self._get_page(has_next, query)
else:
query = self._slice_query(query)
return self._get_page_without_count(query)

def _get_page(self, *args, **kwargs) -> Page:
"""
Return a single page of items
this hook can be used by subclasses if you want to
replace Page datastructure with your custom structure extending BasePage.
or
restricted count query execution and deduce has_next page on basis of current sequence length
for example: page size is 10 but len(page) < 10 so no next page exists.
"""
has_next, query = args
total_count = self.count
Expand All @@ -103,6 +104,20 @@ def _get_page(self, *args, **kwargs) -> Page:
currentPageNumber=self.page_num,
data=query.all())

def _get_page_without_count(self, *args, **kwargs) -> PageWithoutCount:
"""Get Page without total count for avoiding slow count query"""
query = args[0]
data = query.all()
self.set_count(len(data))
has_next = self.is_next_page_exists()
return PageWithoutCount(
hasNext=has_next,
currentPageSize=self.page_size,
currentPageNumber=self.page_num,
data=data[: self.count - 1] if has_next else data
)


def _slice_query(self, query: SqlAlchemyQuery) -> SqlAlchemyQuery:
"""
Return sliced query.
Expand All @@ -111,4 +126,9 @@ def _slice_query(self, query: SqlAlchemyQuery) -> SqlAlchemyQuery:
like using an id range with the help of shared extra_contex
or using a more advanced offset technique.
"""
return query.limit(self.page_size).offset(max(self.page_num - 1, 0) * self.page_size)
if self.fire_count_qry:
return query.limit(self.page_size).offset(max(self.page_num - 1, 0) * self.page_size)
else:
# get +1 than page size to see if next page exists
# a hotfix to avoid total count to determine next page existence
return query.limit(self.page_size + 1).offset(max(self.page_num - 1, 0) * self.page_size)
4 changes: 2 additions & 2 deletions fastapi_listing/strategies/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
__all__ = ['QueryStrategy', 'PaginationStrategy', 'SortingOrderStrategy']

from fastapi_listing.strategies.query_strategy import QueryStrategy
from fastapi_listing.paginator import PaginationStrategy # noqa: F401
from fastapi_listing.sorter import SortingOrderStrategy # noqa: F401
from fastapi_listing.paginator import PaginationStrategy
from fastapi_listing.sorter import SortingOrderStrategy


class ModuleInterface:
Expand Down
14 changes: 14 additions & 0 deletions tests/original_responses.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,3 +107,17 @@
'hrdt': '1987-03-24'}], 'currentPageSize': 10, 'currentPageNumber': 1, 'hasNext': False, 'totalCount': 2}

test_unix_between_filter = {'data': [], 'currentPageSize': 1, 'currentPageNumber': 1, 'hasNext': False, 'totalCount': 0}


test_default_employee_listing_without_count = {'data': [
{'empid': 499999, 'bdt': '1958-05-01', 'fnm': 'Sachin', 'lnm': 'Tsukuda', 'gdr': 'M', 'hdt': '1997-11-30'},
{'empid': 499998, 'bdt': '1956-09-05', 'fnm': 'Patricia', 'lnm': 'Breugel', 'gdr': 'M', 'hdt': '1993-10-13'},
{'empid': 499997, 'bdt': '1961-08-03', 'fnm': 'Berhard', 'lnm': 'Lenart', 'gdr': 'M', 'hdt': '1986-04-21'},
{'empid': 499996, 'bdt': '1953-03-07', 'fnm': 'Zito', 'lnm': 'Baaz', 'gdr': 'M', 'hdt': '1990-09-27'},
{'empid': 499995, 'bdt': '1958-09-24', 'fnm': 'Dekang', 'lnm': 'Lichtner', 'gdr': 'F', 'hdt': '1993-01-12'},
{'empid': 499994, 'bdt': '1952-02-26', 'fnm': 'Navin', 'lnm': 'Argence', 'gdr': 'F', 'hdt': '1990-04-24'},
{'empid': 499993, 'bdt': '1963-06-04', 'fnm': 'DeForest', 'lnm': 'Mullainathan', 'gdr': 'M', 'hdt': '1997-04-07'},
{'empid': 499992, 'bdt': '1960-10-12', 'fnm': 'Siamak', 'lnm': 'Salverda', 'gdr': 'F', 'hdt': '1987-05-10'},
{'empid': 499991, 'bdt': '1962-02-26', 'fnm': 'Pohua', 'lnm': 'Sichman', 'gdr': 'F', 'hdt': '1989-01-12'},
{'empid': 499990, 'bdt': '1963-11-03', 'fnm': 'Khaled', 'lnm': 'Kohling', 'gdr': 'M', 'hdt': '1985-10-10'}],
'currentPageSize': 10, 'currentPageNumber': 1, 'hasNext': True}
18 changes: 16 additions & 2 deletions tests/test_fast_listing_compact_version.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from sqlalchemy.orm import Session

from fastapi_listing import FastapiListing, MetaInfo
from fastapi_listing.paginator import ListingPage
from fastapi_listing.paginator import ListingPage, ListingPageWithoutCount

from tests.pydantic_setup import EmployeeListDetails
from tests.dao_setup import EmployeeDao
Expand Down Expand Up @@ -45,11 +45,25 @@ def read_main(request: Request):
return resp


@app.get("/v1/without-count/employees", response_model=ListingPageWithoutCount[EmployeeListDetails])
def read_main(request: Request):
dao = EmployeeDao(read_db=get_db())
resp = FastapiListing(dao=dao,
pydantic_serializer=EmployeeListDetails
).get_response(MetaInfo(default_srt_on="emp_no", allow_count_query_by_paginator=False))
return resp


client = TestClient(app)


def test_default_employee_listing():
response = client.get("/v1/employees")
print(response.json())
assert response.status_code == 200
assert response.json() == original_responses.test_default_employee_listing


def test_listing_without_total_count():
response = client.get("/v1/without-count/employees")
assert response.status_code == 200
assert response.json() == original_responses.test_default_employee_listing_without_count

0 comments on commit 22e952f

Please sign in to comment.