woowabros / nestjs-library-crud

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

Cursor pagination of search method is not working with ViewEntity which lacks PK #456

Closed Pigrabbit closed 4 months ago

Pigrabbit commented 7 months ago

Hi, there 👋🏼

I tried to use @Crud with TypeORM's ViewEntity and found out that Cursor pagination of search method is not working as I expected.

I'll share the test code I have written to let you recognize the problem below.

import { Controller, INestApplication, Module } from '@nestjs/common';
import { Test } from '@nestjs/testing';
import { InjectRepository, TypeOrmModule, getDataSourceToken } from '@nestjs/typeorm';
import { IsNumber } from 'class-validator';
import request from 'supertest';
import { DataSource, Repository, ViewColumn, ViewEntity } from 'typeorm';

import { Crud, CrudService, Method } from '../../src';
import { BaseEntity } from '../base/base.entity';
import { TestHelper } from '../test.helper';

@ViewEntity('view_base_name', {
    expression: `
        SELECT DISTINCT id, name, type % 2 as category FROM base
    `,
})
export class BaseNameView {
    @ViewColumn()
    id: string;

    @ViewColumn()
    name: string;

    @ViewColumn()
    @IsNumber({}, { groups: [Method.READ_MANY, Method.SEARCH] })
    category: number;
}

class BaseNameViewService extends CrudService<BaseNameView> {
    constructor(@InjectRepository(BaseNameView) repository: Repository<BaseNameView>) {
        super(repository);
    }
}

@Crud({ entity: BaseNameView, only: [Method.READ_MANY, Method.SEARCH] })
@Controller('base-name')
class BaseNameViewController {
    constructor(public readonly crudService: BaseNameViewService) {}
}

@Module({
    imports: [TypeOrmModule.forFeature([BaseEntity, BaseNameView])],
    controllers: [BaseNameViewController],
    providers: [BaseNameViewService],
})
class BaseNameViewModule {}

describe('Cursor Pagination View', () => {
    let app: INestApplication;
    let dataSource: DataSource;

    beforeAll(async () => {
        const module = await Test.createTestingModule({
            imports: [TestHelper.getTypeOrmPgsqlModule([BaseEntity, BaseNameView]), BaseNameViewModule],
        }).compile();

        app = module.createNestApplication();
        await app.init();

        dataSource = module.get<DataSource>(getDataSourceToken('default'));
        await dataSource.query('DELETE FROM base');
        await dataSource.query(`
            INSERT INTO base (name, type, description) VALUES 
                ('apple', 1, 'This is an apple'), 
                ('bear', 2, 'This is a bear'), 
                ('charlie', 3, 'This is charlie'),
                ('dog', 4, 'This is a dog'),
                ('elephant', 5, 'This is an elephant'),
                ('frog', 6, 'This is a frog'),
                ('giraffe', 7, 'This is a giraffe'),
                ('horse', 8, 'This is a horse'),
                ('iguana', 9, 'This is an iguana'),
                ('jaguar', 10, 'This is a jaguar'),
                ('kangaroo', 11, 'This is a kangaroo'),
                ('lion', 12, 'This is a lion'),
                ('monkey', 13, 'This is a monkey'),
                ('newt', 14, 'This is a newt'),
                ('owl', 15, 'This is an owl'),
                ('penguin', 16, 'This is a penguin'),
                ('quail', 17, 'This is a quail'),
                ('rabbit', 18, 'This is a rabbit'),
                ('snake', 19, 'This is a snake'),
                ('tiger', 20, 'This is a tiger'),
                ('unicorn', 21, 'This is a unicorn'),
                ('vulture', 22, 'This is a vulture'),
                ('whale', 23, 'This is a whale'),
                ('xerus', 24, 'This is a xerus'),
                ('yak', 25, 'This is a yak'),
                ('zebra', 26, 'This is a zebra')
        `);
    });

    afterAll(async () => {
        await dataSource.query('DELETE FROM base');
        await app.close();
    });

    it('should be able to paginate', async () => {
        const firstResponse = await request(app.getHttpServer())
            .post('/base-name/search')
            .send({ take: 5, order: { id: 'ASC' } });
        expect(firstResponse.status).toBe(200);
        expect(firstResponse.body.data.length).toBe(5);

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

        // each and every item in the second response should not be in the first response
        for (const item of secondResponse.body.data) {
            expect(firstResponse.body.data.every((firstItem: { id: any }) => firstItem.id !== item.id)).toBeTruthy();
        }
    });
});
Pigrabbit commented 7 months ago

I guess this issue is caused by implementation of creating nextCursor in CrudReadManyRequest.toResponse method. Since View does not have primary key, it just returns the empty object {} as a nextCursor.

In my opinion, we could solve this by let user to pass cursor pagination key according to their View implementation so that we could leverage it to create nextCursor.

What do u think @jiho-kr ?

jiho-kr commented 7 months ago

I confirmed the problem through the issue you reported.

This happens when using cursor-based pagination because the primary key is used as the pagination key for the cursor.

I agree with providing a pagination key as an option.


I think there are two problems.

  1. Cases such as views without a primary key
  2. When using Order by other than Primary key in Search

This problem can be solved by

  1. provide pagination keys as by option
  2. using the primary key as the default (current)
  3. when using order by in search, use the order keys.

Let's solve them one by one, starting from 1.

JadenKim-dev commented 4 months ago

@jiho-kr Are there any ongoing efforts related to this issue? Can I work on it? :)