fix(csharp): push pagination to database level via ListAsync by davideme · Pull Request #406 · davideme/lamp-control-api-reference

davideme · 2026-02-22T17:04:04Z

Summary

Root cause: GetAllAsync fetched all rows from PostgreSQL (SELECT ... FROM lamps WHERE deleted_at IS NULL ORDER BY created_at) with no LIMIT/OFFSET, then LampControllerImplementation paginated the result in application memory using .Skip().Take() — O(n) in both memory and network on every list request
Fix: Added ILampRepository.ListAsync(int limit, int offset) implemented in PostgresLampRepository with EF Core's .Skip(offset).Take(limit), which translates directly to LIMIT/OFFSET in SQL
hasMore without COUNT: The controller now requests pageSize + 1 rows; if more than pageSize are returned, there is a next page — no extra COUNT(*) query needed
GetAllAsync is retained and used only by the parameterless ListLampsAsync() overload

SQL before

SELECT id, created_at, deleted_at, is_on, updated_at
FROM lamps WHERE deleted_at IS NULL ORDER BY created_at
-- (all rows, every request)

SQL after

SELECT id, created_at, deleted_at, is_on, updated_at
FROM lamps WHERE deleted_at IS NULL
ORDER BY created_at, id
LIMIT 26 OFFSET 0   -- (pageSize+1, only the page)

Test plan

ILampRepository interface extended with ListAsync
PostgresLampRepository.ListAsync implemented (EF Core Skip/Take → SQL LIMIT/OFFSET)
InMemoryLampRepository.ListAsync implemented (ordered Skip/Take in memory)
Unit tests added: paging, ordering, empty-offset-beyond-count (InMemoryLampRepositoryTests)
Integration tests added: paging, soft-delete exclusion, ordering (PostgresLampRepositoryTests)
All 94 non-integration tests pass (dotnet test green)
dotnet format --verify-no-changes passes, dotnet build 0 errors

🤖 Generated with Claude Code

Previously GetAllAsync loaded all rows into memory and pagination was applied in the controller with Skip/Take in C#. For large datasets this causes full-table fetches on every list request. Introduce ILampRepository.ListAsync(limit, offset) implemented with EF Core Skip/Take so the LIMIT/OFFSET is emitted in SQL. The controller now requests pageSize+1 rows to detect a next page without a separate COUNT(*) query. GetAllAsync is retained for internal use (parameterless ListLampsAsync). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

This PR improves the C# LampControl API list endpoint by pushing pagination down into the repository layer so PostgreSQL can apply LIMIT/OFFSET, reducing network transfer and in-memory processing for list requests.

Changes:

Add ILampRepository.ListAsync(limit, offset) and implement it for PostgreSQL (EF Core Skip/Take) and the in-memory repository.
Update LampControllerImplementation.ListLampsAsync(cursor, pageSize) to use repository paging and compute hasMore by fetching pageSize + 1 rows.
Add unit/integration tests covering repository paging, ordering, and soft-delete behavior.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
src/csharp/LampControlApi/Services/PostgresLampRepository.cs	Adds DB-level paginated listing via EF Core `Skip/Take`.
src/csharp/LampControlApi/Services/LampControllerImplementation.cs	Switches list endpoint to repository paging and `hasMore` via `pageSize+1`.
src/csharp/LampControlApi/Services/InMemoryLampRepository.cs	Adds in-memory implementation of `ListAsync` with ordering and paging.
src/csharp/LampControlApi/Domain/Repositories/ILampRepository.cs	Extends repository contract with `ListAsync(limit, offset)`.
src/csharp/LampControlApi.Tests/Infrastructure/PostgresLampRepositoryTests.cs	Adds integration tests for `ListAsync` paging/ordering/soft-delete.
src/csharp/LampControlApi.Tests/InMemoryLampRepositoryTests.cs	Adds unit tests for `ListAsync` paging/ordering/empty-offset.

Copilot · 2026-02-22T17:08:02Z

src/csharp/LampControlApi/Services/PostgresLampRepository.cs

+                .OrderBy(l => l.CreatedAt)
+                .ThenBy(l => l.Id)
+                .Skip(offset)
+                .Take(limit)
+                .AsNoTracking()


limit/offset are used directly in Skip/Take with no validation. Negative values (or overflowed values from callers) can cause EF Core to throw or produce unexpected SQL, and different implementations may behave inconsistently. Add guard clauses to enforce limit >= 0 and offset >= 0 (and consider capping limit), and consider aligning GetAllAsync ordering to CreatedAt + Id to keep list ordering deterministic across methods.

Copilot · 2026-02-22T17:08:02Z

src/csharp/LampControlApi/Services/InMemoryLampRepository.cs

+            var page = _lamps.Values
+                .OrderBy(l => l.CreatedAt)
+                .ThenBy(l => l.Id)
+                .Skip(offset)
+                .Take(limit)


limit/offset are not validated before applying Skip/Take. In LINQ-to-Objects, negative values can behave differently than EF Core (e.g., Skip(-1) effectively acts like Skip(0)), which can hide bugs and diverge from production behavior. Add guard clauses to enforce non-negative inputs (and optionally a max limit) for consistency with PostgresLampRepository.

Copilot · 2026-02-22T17:08:03Z

src/csharp/LampControlApi/Domain/Repositories/ILampRepository.cs

+        /// <param name="limit">Maximum number of lamps to return.</param>
+        /// <param name="offset">Number of lamps to skip.</param>


The contract for limit/offset doesn’t specify whether negative values are allowed; current implementations may throw (EF Core) or behave differently (LINQ-to-Objects). Document the expected constraints (e.g., limit >= 0, offset >= 0, and whether a max limit applies) so callers and implementations are consistent.

Suggested change

/// <param name="limit">Maximum number of lamps to return.</param>

/// <param name="offset">Number of lamps to skip.</param>

/// <param name="limit">

/// Maximum number of lamps to return. Must be greater than or equal to 0.

/// A value of 0 results in an empty collection being returned. Implementations

/// may enforce an upper bound on this value; callers should pass a value within

/// the allowed range for the specific implementation.

/// </param>

/// <param name="offset">

/// Number of lamps to skip before returning results. Must be greater than or equal to 0.

/// </param>

Copilot · 2026-02-22T17:08:03Z

src/csharp/LampControlApi/Services/LampControllerImplementation.cs

-                .ToList();
+            // Fetch one extra row to determine whether a next page exists,
+            // avoiding a separate COUNT(*) query.
+            var entities = await _lampRepository.ListAsync(pageSize + 1, offset);


pageSize + 1 can overflow (e.g., pageSize == int.MaxValue), resulting in a negative limit being passed to ListAsync and potentially causing runtime exceptions or incorrect paging. Clamp pageSize to a reasonable maximum and/or use checked arithmetic (or a wider type) before adding 1.

Copilot · 2026-02-22T17:08:03Z

src/csharp/LampControlApi/Services/LampControllerImplementation.cs

+            var entities = await _lampRepository.ListAsync(pageSize + 1, offset);

-            var page = lamps.Skip(start).Take(pageSize).ToList();
-            var hasMore = start + pageSize < lamps.Count;
-            var nextCursor = hasMore ? (start + pageSize).ToString() : string.Empty;
+            var hasMore = entities.Count > pageSize;
+            var page = entities.Take(pageSize).Select(LampMapper.ToApiModel).ToList();
+            var nextCursor = hasMore ? (offset + pageSize).ToString() : string.Empty;


The new paging/hasMore/nextCursor behavior in ListLampsAsync(cursor, pageSize) is untested. Add unit tests that verify (1) stable ordering across pages, (2) HasMore toggles correctly when requesting pageSize+1, and (3) NextCursor matches the returned page size/offset.

Copilot · 2026-02-22T17:08:03Z

src/csharp/LampControlApi/Services/LampControllerImplementation.cs

+            // Fetch all lamps (no pagination) via the repository, then map to API models.
+            var entities = await _lampRepository.GetAllAsync();


ListLampsAsync() now returns GetAllAsync() results without enforcing any ordering. This is nondeterministic for InMemoryLampRepository (dictionary value order) and can diverge from the paginated endpoint’s ordering; consider sorting here (e.g., by CreatedAt then Id) or reusing ListAsync with a deterministic order.

Suggested change

// Fetch all lamps (no pagination) via the repository, then map to API models.

var entities = await _lampRepository.GetAllAsync();

// Fetch all lamps (no pagination) using the same deterministic ordering as the paginated endpoint,

// then map to API models.

var entities = await _lampRepository.ListAsync(int.MaxValue, 0);

- Add ArgumentOutOfRangeException guards for limit < 0 and offset < 0 in both PostgresLampRepository and InMemoryLampRepository so behavior is consistent across implementations - Expand ILampRepository XML docs to document limit/offset constraints and the ArgumentOutOfRangeException contract - Cap pageSize at MaxPageSize (1000) in the controller before adding +1, eliminating any overflow risk; extract DefaultPageSize constant (25) - Fix parameterless ListLampsAsync() to use ListAsync(int.MaxValue, 0) so ordering matches the paginated endpoint (deterministic for in-memory) - Add controller unit tests: first page HasMore=true, last page HasMore=false, cursor offset propagation, default page size fallback, and MaxPageSize clamping; update existing mocks from GetAllAsync to ListAsync to match the new parameterless overload implementation Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot AI review requested due to automatic review settings February 22, 2026 17:04

Copilot started reviewing on behalf of davideme February 22, 2026 17:04 View session

Copilot AI reviewed Feb 22, 2026

View reviewed changes

davideme merged commit 06d8d9e into main Feb 22, 2026
7 checks passed

davideme deleted the claude/competent-leakey branch February 22, 2026 17:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

fix(csharp): push pagination to database level via ListAsync#406

fix(csharp): push pagination to database level via ListAsync#406
davideme merged 2 commits intomainfrom
claude/competent-leakey

davideme commented Feb 22, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 22, 2026

Uh oh!

Copilot AI Feb 22, 2026

Uh oh!

Copilot AI Feb 22, 2026

Uh oh!

Copilot AI Feb 22, 2026

Uh oh!

Copilot AI Feb 22, 2026

Uh oh!

Copilot AI Feb 22, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

		/// <param name="limit">Maximum number of lamps to return.</param>
		/// <param name="offset">Number of lamps to skip.</param>

-        /// <param name="limit">Maximum number of lamps to return.</param>
-        /// <param name="offset">Number of lamps to skip.</param>
+        /// <param name="limit">
+        /// Maximum number of lamps to return. Must be greater than or equal to 0.
+        /// A value of 0 results in an empty collection being returned. Implementations
+        /// may enforce an upper bound on this value; callers should pass a value within
+        /// the allowed range for the specific implementation.
+        /// </param>
+        /// <param name="offset">
+        /// Number of lamps to skip before returning results. Must be greater than or equal to 0.
+        /// </param>

		// Fetch all lamps (no pagination) via the repository, then map to API models.
		var entities = await _lampRepository.GetAllAsync();

-            // Fetch all lamps (no pagination) via the repository, then map to API models.
-            var entities = await _lampRepository.GetAllAsync();
+            // Fetch all lamps (no pagination) using the same deterministic ordering as the paginated endpoint,
+            // then map to API models.
+            var entities = await _lampRepository.ListAsync(int.MaxValue, 0);

Comments

Conversation

davideme commented Feb 22, 2026

Summary

SQL before

SQL after

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant