feat!: change limit=0 to return no items and enforce maximum limit#2351
Conversation
There was a problem hiding this comment.
Sorry @ctron, your pull request is larger than the review limit of 150000 diff characters
a2b57fc to
1d57549
Compare
1d57549 to
3abcca5
Compare
|
@sourcery-ai review |
Reviewer's GuideIntroduces configurable maximum pagination limits with HTTP 400 responses when exceeded, changes limit=0 semantics to return no items while still allowing total computation, and refactors limiter APIs and services to enforce the new behavior consistently across queries, OpenAPI docs, and tests. Sequence diagram for exceeding maximum pagination limitsequenceDiagram
actor Client
participant ApiHandler
participant Service
participant Cache as PaginationCache
participant Lim as Limiter
Client->>ApiHandler: HTTP GET /items?offset=0&limit=2000&total=true
ApiHandler->>Service: get_items(paginated)
Service->>Cache: check_limit(limit=2000)
alt limit_exceeds_max_limit
Cache-->>Service: LimitError(max_limit)
Service-->>ApiHandler: Error_Limit(LimitError)
ApiHandler-->>Client: 400 BadRequest\nX-Pagination-Max-Limit: max_limit\nbody: ErrorInformation LimitExceeded
else limit_within_max
Cache-->>Service: Ok(limit)
Service->>Lim: construct_limiter(db, page, cache)
Lim->>Lim: fetch()
Lim-->>Service: LimitedResult(items, TotalCount)
Service->>Cache: TotalCount.requested(total=true)
Cache-->>Service: total_count
Service-->>ApiHandler: PaginatedResults
ApiHandler-->>Client: 200 OK with items and total
end
Class diagram for updated pagination and limiter typesclassDiagram
class Paginated {
u64 offset
u64 limit
bool total
+paginate_array(vec: VecT): PaginatedResultsT
}
class PaginatedResultsT {
VecT items
Option_u64 total
}
class PaginationConfig {
humantime_Duration cache_ttl
u64 max_limit
+into_cache(): PaginationCache
}
class PaginationCache {
CacheStringU64 cache
CounterU64 total
CounterU64 misses
u64 max_limit
+new(ttl: Duration, max_limit: u64): PaginationCache
+for_test(): PaginationCache
+check_limit(limit: u64): Result_u64_LimitError
+max_limit(): u64
}
class LimitError {
u64 max_limit
+error_response(): HttpResponseBoxBody
}
class LimiterError {
Db(DbErr)
Limit(LimitError)
}
class LimiterDBFetchCount {
C db
PaginatorCCount paginator
FetchSelector selector
String cache_key
PaginationCache cache
u64 limit
+fetch(): Result_LimitedResult_DbErr
}
class TotalCount {
C db
PaginatorCCount paginator
String cache_key
PaginationCache cache
+requested(total: bool): Result_Option_u64_DbErr
}
class Resulting {
<<trait>>
+resulting(db: C, query: SelectE, cache: PaginationCache): Future_Output
}
class PaginatedResultingImpl {
+resulting(db: C, query: SelectE, cache: PaginationCache): Future_PaginatedResultsM
}
class UnitResultingImpl {
+resulting(db: C, query: SelectE, cache: PaginationCache): Future_VecM
}
class limit_selector_fn {
+limit_selector(db: C, select: SelectE, page: Page, cache: PaginationCache): Result_LimiterDBFetchCount_LimitError
}
Paginated --> PaginatedResultsT : produces
PaginationConfig --> PaginationCache : into_cache
PaginationCache --> LimiterDBFetchCount : configures
PaginationCache --> TotalCount : caches_totals
LimiterDBFetchCount *-- TotalCount : builds
LimitError <|-- LimiterError : Limit
LimiterError --> LimiterDBFetchCount : error_from_construction
Resulting <|.. PaginatedResultingImpl : impl
Resulting <|.. UnitResultingImpl : impl
PaginatedResultingImpl --> limit_selector_fn : uses
UnitResultingImpl --> limit_selector_fn : uses
LimiterDBFetchCount --> PaginationCache : uses
TotalCount --> PaginationCache : uses
Paginated --> PaginationCache : used_via_services
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
8d74faf to
a1a587f
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2351 +/- ##
==========================================
- Coverage 70.92% 70.89% -0.04%
==========================================
Files 442 442
Lines 25265 25357 +92
Branches 25265 25357 +92
==========================================
+ Hits 17920 17976 +56
- Misses 6367 6392 +25
- Partials 978 989 +11 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
helio-frota
left a comment
There was a problem hiding this comment.
Approving from the performance part. I'm not sure if I'm missing other extra details
Co-authored-by: Claude <noreply@anthropic.com>
Change limit=0 semantics from "no limit" to "return no items", skipping the database query entirely while still computing the total when requested. Introduce a configurable maximum pagination limit (default 1000) that rejects requests exceeding it with HTTP 400 and an X-Pagination-Max-Limit header via a dedicated LimitError. Add Page struct to replace loose offset/limit parameters in limiter traits. Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Claude <noreply@anthropic.com>
a1a587f to
c94ed9a
Compare
|
/scale-test |
|
🛠️ Scale test has started! Follow the progress here: Workflow Run |
Goose ReportGoose Attack ReportPlan Overview
Request Metrics
Response Time Metrics
Status Code Metrics
Transaction Metrics
Scenario Metrics
Error Metrics
📄 Full Report (Go to "Artifacts" and download report) |

This is based on PR #2338 … only take a look at the most recent commits.Summary by Sourcery
Enforce configurable maximum pagination limits and change limit=0 semantics while propagating limit validation and error handling across services and APIs.
New Features:
Enhancements:
Documentation:
Tests: