-
Notifications
You must be signed in to change notification settings - Fork 1
feat(server): enforce pagination limits for Prisma findMany queries #101
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| # Changelog | ||
|
|
||
| All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). | ||
|
|
||
| ## [Unreleased] | ||
| ### Added | ||
| - Shared pagination utility `applyPaginationDefaults` with configurable `MAX_PAGE_SIZE` (default **100**). | ||
| - Unit tests covering pagination helper edge cases. | ||
| - Integration tests validating enforced pagination across list endpoints. | ||
|
|
||
| ### Changed | ||
| - List endpoints for Hotels, Customers, Rooms, and Reservations now automatically enforce a maximum page size of 100 records when the client omits or exceeds this limit. Negative `skip` values are clamped to `0`. (**Performance fix**, closes #84) | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,18 @@ | ||
| import { Injectable } from "@nestjs/common"; | ||
| import { PrismaService } from "../prisma/prisma.service"; | ||
| import { CustomerServiceBase } from "./base/customer.service.base"; | ||
| import { applyPaginationDefaults } from "../pagination"; | ||
| import { Prisma, Customer as PrismaCustomer } from "@prisma/client"; | ||
|
|
||
| @Injectable() | ||
| export class CustomerService extends CustomerServiceBase { | ||
| constructor(protected readonly prisma: PrismaService) { | ||
| super(prisma); | ||
| } | ||
|
|
||
| async customers<T extends Prisma.CustomerFindManyArgs>( | ||
| args: T | ||
| ): Promise<PrismaCustomer[]> { | ||
| return super.customers(applyPaginationDefaults(args)); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,18 @@ | ||
| import { Injectable } from "@nestjs/common"; | ||
| import { PrismaService } from "../prisma/prisma.service"; | ||
| import { HotelServiceBase } from "./base/hotel.service.base"; | ||
| import { applyPaginationDefaults } from "../pagination"; | ||
| import { Prisma, Hotel as PrismaHotel } from "@prisma/client"; | ||
|
|
||
| @Injectable() | ||
| export class HotelService extends HotelServiceBase { | ||
| constructor(protected readonly prisma: PrismaService) { | ||
| super(prisma); | ||
| } | ||
|
|
||
| async hotels<T extends Prisma.HotelFindManyArgs>( | ||
| args: T | ||
| ): Promise<PrismaHotel[]> { | ||
| return super.hotels(applyPaginationDefaults(args)); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| /** | ||
| * Shared pagination utilities. | ||
| */ | ||
|
|
||
| export const MAX_PAGE_SIZE = 100; | ||
|
|
||
| /** | ||
| * Ensures `take` and `skip` arguments are within sensible defaults. | ||
| * | ||
| * - If `take` is `undefined` or greater than `MAX_PAGE_SIZE`, it will be set to `MAX_PAGE_SIZE`. | ||
| * - If `skip` is provided and is less than `0`, it will be clamped to `0`. | ||
| * | ||
| * The function returns a shallow–copied object preserving all original properties | ||
| * while applying the sanitized `take` and `skip` values. The return type is kept | ||
| * generic so that TypeScript maintains type information from the original args. | ||
| */ | ||
| export function applyPaginationDefaults< | ||
| T extends { | ||
| take?: number; | ||
| skip?: number; | ||
| [key: string]: unknown; | ||
| } | ||
| >(args: T): T { | ||
| const sanitized: T = { | ||
| ...args, | ||
| take: | ||
| args.take === undefined || args.take === null | ||
|
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. Potential correctness issue – take:
args.take === undefined || args.take === null
? MAX_PAGE_SIZE
: Math.min(args.take, MAX_PAGE_SIZE),Consider additionally clamping the lower bound, e.g. |
||
| ? (MAX_PAGE_SIZE as unknown as T["take"]) | ||
| : (Math.min(args.take as number, MAX_PAGE_SIZE) as unknown as T["take"]), | ||
| skip: | ||
| args.skip !== undefined && (args.skip as number) < 0 | ||
| ? (0 as unknown as T["skip"]) | ||
| : args.skip, | ||
| }; | ||
|
|
||
| return sanitized; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,18 @@ | ||
| import { Injectable } from "@nestjs/common"; | ||
| import { PrismaService } from "../prisma/prisma.service"; | ||
| import { ReservationServiceBase } from "./base/reservation.service.base"; | ||
| import { applyPaginationDefaults } from "../pagination"; | ||
| import { Prisma, Reservation as PrismaReservation } from "@prisma/client"; | ||
|
|
||
| @Injectable() | ||
| export class ReservationService extends ReservationServiceBase { | ||
| constructor(protected readonly prisma: PrismaService) { | ||
| super(prisma); | ||
| } | ||
|
|
||
| async reservations<T extends Prisma.ReservationFindManyArgs>( | ||
| args: T | ||
| ): Promise<PrismaReservation[]> { | ||
| return super.reservations(applyPaginationDefaults(args)); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,18 @@ | ||
| import { Injectable } from "@nestjs/common"; | ||
| import { PrismaService } from "../prisma/prisma.service"; | ||
| import { RoomServiceBase } from "./base/room.service.base"; | ||
| import { applyPaginationDefaults } from "../pagination"; | ||
| import { Prisma, Room as PrismaRoom } from "@prisma/client"; | ||
|
|
||
| @Injectable() | ||
| export class RoomService extends RoomServiceBase { | ||
| constructor(protected readonly prisma: PrismaService) { | ||
| super(prisma); | ||
| } | ||
|
|
||
| async rooms<T extends Prisma.RoomFindManyArgs>( | ||
| args: T | ||
| ): Promise<PrismaRoom[]> { | ||
| return super.rooms(applyPaginationDefaults(args)); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,104 @@ | ||
| import request from "supertest"; | ||
| import { INestApplication } from "@nestjs/common"; | ||
| import { Test } from "@nestjs/testing"; | ||
| import { AppModule } from "../../../src/app.module"; | ||
| import { PrismaService } from "../../../src/prisma/prisma.service"; | ||
| import { applyPaginationDefaults, MAX_PAGE_SIZE } from "../../../src/pagination"; | ||
|
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. Import path appears to be off by one directory: |
||
| import { PrismaClient } from "@prisma/client"; | ||
|
|
||
| // NOTE: These e2e tests spin up the NestJS app with an in-memory SQLite DB (via Prisma) | ||
| // for isolation. Adjust the datasource provider/environment if needed. | ||
|
|
||
| describe("Pagination limits (e2e)", () => { | ||
| let app: INestApplication; | ||
| let prisma: PrismaClient; | ||
|
|
||
| beforeAll(async () => { | ||
|
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. Persisting data to |
||
| // Override DB connection to sqlite for tests | ||
| process.env.DB_URL = "file:./test.db?connection_limit=1"; | ||
|
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.
|
||
|
|
||
| const moduleRef = await Test.createTestingModule({ | ||
| imports: [AppModule], | ||
| }) | ||
| .overrideProvider(PrismaService) | ||
| .useValue(new PrismaService()) | ||
| .compile(); | ||
|
|
||
| app = moduleRef.createNestApplication(); | ||
| await app.init(); | ||
|
|
||
| prisma = new PrismaClient({ datasources: { db: { url: process.env.DB_URL } } }); | ||
|
|
||
| // Seed with > MAX_PAGE_SIZE records for each model to test limits | ||
| await seedData(MAX_PAGE_SIZE + 20); | ||
| }); | ||
|
|
||
| afterAll(async () => { | ||
| await prisma.$executeRawUnsafe(`PRAGMA wal_checkpoint(FULL);`); | ||
| await prisma.$disconnect(); | ||
| await app.close(); | ||
| }); | ||
|
|
||
| async function seedData(count: number) { | ||
| // Hotels | ||
| const hotelsData = Array.from({ length: count }).map((_, i) => ({ | ||
| name: `Hotel ${i}`, | ||
| address: `Street ${i}`, | ||
| description: `Desc ${i}`, | ||
| })); | ||
|
|
||
| await prisma.hotel.createMany({ data: hotelsData }); | ||
|
|
||
| // Customers | ||
| const customersData = Array.from({ length: count }).map((_, i) => ({ | ||
| firstName: `First${i}`, | ||
| lastName: `Last${i}`, | ||
| email: `email${i}@example.com`, | ||
| })); | ||
| await prisma.customer.createMany({ data: customersData }); | ||
|
|
||
| // Rooms | ||
| const roomsData = Array.from({ length: count }).map((_, i) => ({ | ||
| floor: Math.floor(i / 10), | ||
| numberField: `${100 + i}`, | ||
| typeField: "STANDARD", | ||
| })); | ||
| await prisma.room.createMany({ data: roomsData }); | ||
|
|
||
| // Reservations | ||
| const reservationData = Array.from({ length: count }).map((_, i) => ({ | ||
| checkIn: new Date(), | ||
| checkOut: new Date(), | ||
| })); | ||
| await prisma.reservation.createMany({ data: reservationData }); | ||
| } | ||
|
|
||
| const endpoints = [ | ||
| { path: "/hotels", key: "hotels" }, | ||
| { path: "/customers", key: "customers" }, | ||
| { path: "/rooms", key: "rooms" }, | ||
| { path: "/reservations", key: "reservations" }, | ||
| ]; | ||
|
|
||
| endpoints.forEach(({ path, key }) => { | ||
| describe(`${key} list`, () => { | ||
| it(`GET ${path} with no take`, async () => { | ||
| const res = await request(app.getHttpServer()).get(path); | ||
| expect(res.status).toBe(200); | ||
| expect(res.body.length).toBeLessThanOrEqual(MAX_PAGE_SIZE); | ||
| }); | ||
|
|
||
| it(`GET ${path}?take=150 limited`, async () => { | ||
| const res = await request(app.getHttpServer()).get(path).query({ take: 150 }); | ||
| expect(res.status).toBe(200); | ||
| expect(res.body.length).toBe(MAX_PAGE_SIZE); | ||
| }); | ||
|
|
||
| it(`GET ${path}?take=20 returns 20`, async () => { | ||
| const res = await request(app.getHttpServer()).get(path).query({ take: 20 }); | ||
| expect(res.status).toBe(200); | ||
| expect(res.body.length).toBe(20); | ||
| }); | ||
| }); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| import { applyPaginationDefaults, MAX_PAGE_SIZE } from "../src/pagination"; | ||
|
|
||
| describe("applyPaginationDefaults", () => { | ||
| it("should set take to MAX_PAGE_SIZE and leave skip undefined when not provided", () => { | ||
| const args = {}; | ||
| const result = applyPaginationDefaults(args); | ||
| expect(result.take).toBe(MAX_PAGE_SIZE); | ||
| expect(result.skip).toBeUndefined(); | ||
| }); | ||
|
|
||
| it("should keep take unchanged when within limits", () => { | ||
| const args = { take: 20 }; | ||
| const result = applyPaginationDefaults(args); | ||
| expect(result.take).toBe(20); | ||
| }); | ||
|
|
||
| it("should cap take at MAX_PAGE_SIZE when it exceeds the limit", () => { | ||
| const args = { take: MAX_PAGE_SIZE + 50 }; | ||
| const result = applyPaginationDefaults(args); | ||
| expect(result.take).toBe(MAX_PAGE_SIZE); | ||
| }); | ||
|
|
||
| it("should clamp negative skip values to 0", () => { | ||
| const args = { skip: -5 }; | ||
| const result = applyPaginationDefaults(args); | ||
| expect(result.skip).toBe(0); | ||
| }); | ||
| }); |
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.
HotelService.hotels()correctly appliesapplyPaginationDefaults, but the nested resolverfindRoomsinhotel.controller.base.tsstill forwards user-supplied args directly toprisma.room.findMany. That bypasses the new pagination guard and allows clients to request unbounded data. Please ensure nested relation resolvers wrap incoming args withapplyPaginationDefaults(or delegate toRoomService) before calling Prisma.