woowabros / nestjs-library-crud

Automatically generate CRUD Rest API based on NestJS and TypeOrm
https://www.npmjs.com/package/@nestjs-library/crud
MIT License
175 stars 32 forks source link

[Issue] cursor pagination skips the duplicate values with search request #593

Closed JadenKim-dev closed 2 weeks ago

JadenKim-dev commented 1 month ago

Hello! I found that if sort key is not unique, cursor pagination skips the duplicate items in search request

In the test code below, half of the data is set with type as 0 and the other half as 1. Then, a search request is made using cursor pagination to sort the data by type.

In the first request not all data with type 0 had been retrieved. So I expect that the next request would continue to fetch the remaining data. However, the data with type 0 was skipped, and the retrieval started from the data with type 1.

import { Controller, forwardRef, INestApplication } from '@nestjs/common';
import { Test } from '@nestjs/testing';
import { TypeOrmModule } from '@nestjs/typeorm';
import request from 'supertest';

import { Crud, CrudController } from '../../src';
import { BaseEntity } from '../base/base.entity';
import { BaseService } from '../base/base.service';
import { TestHelper } from '../test.helper';

describe('Cursor Pagination', () => {
    let app: INestApplication;
    let service: BaseService;

    beforeAll(async () => {
        @Crud({
            entity: BaseEntity,
            routes: {
                search: {
                    paginationType: 'cursor',
                },
            },
            logging: true,
        })
        @Controller('cursor')
        class CursorController implements CrudController<BaseEntity> {
            constructor(public readonly crudService: BaseService) {}
        }

        const moduleFixture = await Test.createTestingModule({
            imports: [forwardRef(() => TestHelper.getTypeOrmMysqlModule([BaseEntity])), TypeOrmModule.forFeature([BaseEntity])],
            controllers: [CursorController],
            providers: [BaseService],
        }).compile();

        app = moduleFixture.createNestApplication();
        service = moduleFixture.get<BaseService>(BaseService);
        await app.init();
    });

    beforeEach(async () => {
        await service.repository.delete({});
        await service.repository.save(
            // Odd id items have a type of 1. (total: 50)
            // Even id items have a type of 0. (total: 50)
            Array.from({ length: 100 }, (_, index) => index).map((number) => ({
                name: `name-${number}`,
                type: number % 2,
            })),
        );
    });

    it('search cursor pagination', async () => {
        // search data using cursor pagination with ascending sorting by “type” column
        const firstResponse = await request(app.getHttpServer())
            .post('/cursor/search')
            .send({ take: 5, order: { type: 'ASC' } });
        expect(firstResponse.status).toBe(200);
        expect(firstResponse.body.data.length).toBe(5);

        const secondResponse = await request(app.getHttpServer()).post('/cursor/search').send({
            nextCursor: firstResponse.body.metadata.nextCursor,
        });
        expect(secondResponse.status).toBe(200);
        expect(secondResponse.body.data.length).toBe(5);

        // expect next 5 items to have type 0
        for (const item of secondResponse.body.data) {
            expect(item.type).toBe(0);
        }
    });
});
Error: expect(received).toBe(expected) // Object.is equality

Expected: 0
Received: 1
JadenKim-dev commented 1 month ago

It seems that only order keys are extracted and stored in the cursor. (code) In the next request pagination receives the cursor value, so that query filter condition is created using the order key list. And they are combined as AND operation.(code)

A possible solution seems to include the pagination keys in the sort keys even if user does not specify it, and to combine the pagination query filters using OR operation. (Currently, pagination keys are default values used when sort key is not specified in request body)

What do you think about it?

jiho-kr commented 2 weeks ago

Hi @JadenKim-dev

if sort key is not unique, it cannot logically check the next page just by requesting.

I think this can be solved by always including data of primary key in the nextCursor, and if entity don't have a primary key, it can be able to use the paginationKeys

Sounds like you have a similar idea.

But, I'm just wondering if it makes sense to take care of this in the library, since the request itself is invalid. Because the cursor getting bigger is also a problem.

This means that something needs more conditions to sort on, which may be not a good thing to add from the library.

JadenKim-dev commented 2 weeks ago

@jiho-kr Thank you for confirming! I understand the concerns you have raised. If necessary, I will add a primary key to the sort key directly as needed.