From 638b7112a2d6e6af7618f18ac4b3df1e9f5dfb37 Mon Sep 17 00:00:00 2001 From: Oleg Drozdovich <44732463+OlegDO@users.noreply.github.com> Date: Wed, 13 Dec 2023 21:44:26 +0400 Subject: [PATCH] feat(endpoint): add count is allow distinct (#8) * feat(endpoint): add count is allow distinct * feat(endpoint): add is allow distinct default value * feat(endpoint): add tests * feat(endpoint): update tests * feat(endpoint): update count distinct (#9) * feat(endpoint): update count distinct * feat(endpoint): update deps * feat(endpoint): add supportive count with complex distinct * refactor(endpoint): decompose count handler * feat(endpoint): clean up * feat(endpoint): revert has empty condition reg exp * feat(endpoint): update distinct count source manager * feat(endpoint): clean up * feat(endpoint): clean up --- __tests__/services/endpoint-test.ts | 67 +++++++++++++++--------- package-lock.json | 32 ++++++------ package.json | 4 +- src/services/endpoint.ts | 79 +++++++++++++++++++++++------ 4 files changed, 123 insertions(+), 59 deletions(-) diff --git a/__tests__/services/endpoint-test.ts b/__tests__/services/endpoint-test.ts index 1aaacbf..dd0fa06 100644 --- a/__tests__/services/endpoint-test.ts +++ b/__tests__/services/endpoint-test.ts @@ -107,29 +107,12 @@ describe('services/endpoint', () => { expect(result).to.deep.equal({ ...countResult(), payloadParam: 1 }); }); - it('should correctly build default params with distinct: count', async () => { - // return typeorm from handler - handler.callsFake((query) => query); - - const result = await countHandler( - { - query: { - distinct: 'param', - }, - }, - endpointOptions, - ); - const [queryBuilder, params] = defaultHandlerStub.firstCall.args; - - expect(queryBuilder).to.be.instanceof(SelectQueryBuilder); - expect(params?.distinct).to.be.equal('param'); - expect(result).to.deep.equal({ ...countResult() }); - }); - it('handler - should return count entities without removed: default handler', async () => { defaultHandlerStub.restore(); - const result = await Endpoint.defaultHandler.count(repository.createQueryBuilder()); + const result = await Endpoint.defaultHandler.count(repository.createQueryBuilder(), { + repository, + }); expect(TypeormMock.queryBuilder.getCount).to.be.calledOnce; expect(result).to.deep.equal({ count: 0 }); @@ -138,31 +121,65 @@ describe('services/endpoint', () => { it('handler - should return count entities with removed: default handler', async () => { const qb = repository.createQueryBuilder(); const withDeletedSpy = sandbox.spy(qb, 'withDeleted'); - const result = await Endpoint.defaultHandler.count(qb, { hasRemoved: true }); + const result = await Endpoint.defaultHandler.count(qb, { repository, hasRemoved: true }); expect(withDeletedSpy).to.be.calledOnce; expect(TypeormMock.queryBuilder.getCount).to.be.calledOnce; expect(result).to.deep.equal({ count: 0 }); }); - it('handler - should return raw count entities', async () => { + it('handler - should return non-raw count entities', async () => { const qb = repository.createQueryBuilder(); - const result = await Endpoint.defaultHandler.count(qb, { distinct: 'param' }); + const result = await Endpoint.defaultHandler.count(qb, { + isAllowDistinct: false, + repository, + }); const [query, params] = qb.getQueryAndParameters(); expect(query).to.equal( - 'SELECT COUNT(DISTINCT "param")::integer AS "count" FROM "test_entity" "TestEntity"', + 'SELECT "TestEntity"."id" AS "TestEntity_id", "TestEntity"."param" AS "TestEntity_param", "TestEntity"."param2" AS "TestEntity_param2" FROM "test_entity" "TestEntity"', ); + expect(TypeormMock.entityManager.createQueryBuilder).to.calledOnce; expect(params).to.deep.equal([]); + expect(TypeormMock.queryBuilder.getCount).to.be.calledOnce; + expect(TypeormMock.queryBuilder.getRawOne).to.be.not.called; + expect(result).to.deep.equal({ count: 0 }); + }); + + it('handler - should return raw count entities', async () => { + const qb = repository.createQueryBuilder().select(['id', 'param']).distinctOn(['param']); + const result = await Endpoint.defaultHandler.count(qb, { + isAllowDistinct: true, + repository, + }); + + const [originalQuery, originalParams] = qb.getQueryAndParameters(); + + expect(originalQuery).to.equal( + 'SELECT DISTINCT ON (param) id, param FROM "test_entity" "TestEntity"', + ); + expect(TypeormMock.entityManager.createQueryBuilder).to.calledTwice; + expect(originalParams).to.deep.equal([]); expect(TypeormMock.queryBuilder.getCount).to.be.not.called; expect(TypeormMock.queryBuilder.getRawOne).to.be.calledOnce; expect(result).to.deep.equal({ count: 0 }); }); + it('handler - should throw error distinct is now allowed', async () => { + expect( + await waitResult( + Endpoint.defaultHandler.count( + repository.createQueryBuilder().select(['id', 'param']).distinctOn(['param']), + { repository }, + ), + ), + ).to.throw('Distinct select is not allowed.'); + }); + it('should run default count handler with query builder: typeorm case - cache', async () => { const qb = repository.createQueryBuilder(); - const res = await Endpoint.defaultHandler.count(qb, { cache: 100 }); + const res = await Endpoint.defaultHandler.count(qb, { repository, cache: 100 }); expect(TypeormMock.queryBuilder.getCount).to.be.calledOnce; expect(qb.expressionMap.cache).to.be.ok; diff --git a/package-lock.json b/package-lock.json index a6ce9d7..a3063fe 100644 --- a/package-lock.json +++ b/package-lock.json @@ -12,8 +12,8 @@ "@lomray/microservice-nodejs-lib": "^2.21.2", "@lomray/microservice-remote-middleware": "^1.8.2", "@lomray/microservices-client-api": "^2.28.0", - "@lomray/microservices-types": "^1.14.0", - "@lomray/typeorm-json-query": "^2.5.5", + "@lomray/microservices-types": "^1.15.0", + "@lomray/typeorm-json-query": "^2.6.0", "@opentelemetry/api": "^1.7.0", "@opentelemetry/exporter-metrics-otlp-http": "^0.45.1", "@opentelemetry/exporter-trace-otlp-http": "^0.45.1", @@ -1489,9 +1489,9 @@ } }, "node_modules/@lomray/microservices-types": { - "version": "1.14.0", - "resolved": "https://registry.npmjs.org/@lomray/microservices-types/-/microservices-types-1.14.0.tgz", - "integrity": "sha512-QoHz47xlGnUbxbGRRmq5iAwusRy3407yTo8u8T08NQa0DV1qHSHsa7Z1Kza3B+g+MwNZoCR3oD2vIrgzXE9FPA==" + "version": "1.15.0", + "resolved": "https://registry.npmjs.org/@lomray/microservices-types/-/microservices-types-1.15.0.tgz", + "integrity": "sha512-ocXYeb/eWGqqzjzEdIcqKvNvvfXQbsNHzNi3fO8YBCAIEkDBX6XvwjoCcRoNgCTyt3m7f1pH4QCjmBGdWfa9Vw==" }, "node_modules/@lomray/prettier-config": { "version": "1.2.0", @@ -1512,14 +1512,14 @@ } }, "node_modules/@lomray/typeorm-json-query": { - "version": "2.5.5", - "resolved": "https://registry.npmjs.org/@lomray/typeorm-json-query/-/typeorm-json-query-2.5.5.tgz", - "integrity": "sha512-ZUGGO+KDVnnx31ZqH8JrMkg8fQdah6xW0lM+eL7FSthtyRNU2VB4GBlGYUOMe6pE8FVhviA7KKTeLnDSSX4+BQ==", + "version": "2.6.0", + "resolved": "https://registry.npmjs.org/@lomray/typeorm-json-query/-/typeorm-json-query-2.6.0.tgz", + "integrity": "sha512-7DCsRkN8IHuXwhCazQX5DIab9M+VjTwLaOzkpOrmOJV0rroHt358vxLCmWjh17Rxnjh2T3Hmz2tMq2B0aV8+8g==", "bundleDependencies": [ "tslib" ], "dependencies": { - "@lomray/microservices-types": "^1.13.0", + "@lomray/microservices-types": "^1.15.0", "tslib": "*" }, "peerDependencies": { @@ -19697,9 +19697,9 @@ } }, "@lomray/microservices-types": { - "version": "1.14.0", - "resolved": "https://registry.npmjs.org/@lomray/microservices-types/-/microservices-types-1.14.0.tgz", - "integrity": "sha512-QoHz47xlGnUbxbGRRmq5iAwusRy3407yTo8u8T08NQa0DV1qHSHsa7Z1Kza3B+g+MwNZoCR3oD2vIrgzXE9FPA==" + "version": "1.15.0", + "resolved": "https://registry.npmjs.org/@lomray/microservices-types/-/microservices-types-1.15.0.tgz", + "integrity": "sha512-ocXYeb/eWGqqzjzEdIcqKvNvvfXQbsNHzNi3fO8YBCAIEkDBX6XvwjoCcRoNgCTyt3m7f1pH4QCjmBGdWfa9Vw==" }, "@lomray/prettier-config": { "version": "1.2.0", @@ -19715,11 +19715,11 @@ "requires": {} }, "@lomray/typeorm-json-query": { - "version": "2.5.5", - "resolved": "https://registry.npmjs.org/@lomray/typeorm-json-query/-/typeorm-json-query-2.5.5.tgz", - "integrity": "sha512-ZUGGO+KDVnnx31ZqH8JrMkg8fQdah6xW0lM+eL7FSthtyRNU2VB4GBlGYUOMe6pE8FVhviA7KKTeLnDSSX4+BQ==", + "version": "2.6.0", + "resolved": "https://registry.npmjs.org/@lomray/typeorm-json-query/-/typeorm-json-query-2.6.0.tgz", + "integrity": "sha512-7DCsRkN8IHuXwhCazQX5DIab9M+VjTwLaOzkpOrmOJV0rroHt358vxLCmWjh17Rxnjh2T3Hmz2tMq2B0aV8+8g==", "requires": { - "@lomray/microservices-types": "^1.13.0", + "@lomray/microservices-types": "^1.15.0", "tslib": "*" }, "dependencies": { diff --git a/package.json b/package.json index 6932732..41e9187 100644 --- a/package.json +++ b/package.json @@ -36,8 +36,8 @@ "@lomray/microservice-nodejs-lib": "^2.21.2", "@lomray/microservice-remote-middleware": "^1.8.2", "@lomray/microservices-client-api": "^2.28.0", - "@lomray/microservices-types": "^1.14.0", - "@lomray/typeorm-json-query": "^2.5.5", + "@lomray/microservices-types": "^1.15.0", + "@lomray/typeorm-json-query": "^2.6.0", "@opentelemetry/api": "^1.7.0", "@opentelemetry/exporter-metrics-otlp-http": "^0.45.1", "@opentelemetry/exporter-trace-otlp-http": "^0.45.1", diff --git a/src/services/endpoint.ts b/src/services/endpoint.ts index dbfb6b7..dea303e 100644 --- a/src/services/endpoint.ts +++ b/src/services/endpoint.ts @@ -22,6 +22,7 @@ enum CRUD_EXCEPTION_CODE { FAILED_RESTORE = -33486, ENTITY_NOT_FOUND = -33487, ENTITY_ALREADY_EXIST = -33488, + DISTINCT_SELECT_FORBIDDEN = -33489, } export type Constructable = new (...args: any[]) => TParams; @@ -97,6 +98,7 @@ export type IRequestPayload = TPayload & { isParallel?: boolean; shouldReturnEntity?: boolean; shouldResetCache?: boolean; + isAllowDistinct?: boolean; }; options?: Partial; query?: IJsonQuery; @@ -110,8 +112,9 @@ export interface IGetQueryParams { [key: string]: any; } -export interface IGetQueryCountParams extends IGetQueryParams { - distinct?: string; +export interface IGetQueryCountParams extends IGetQueryParams { + repository: Repository; + isAllowDistinct?: boolean; } export interface IGetQueryListParams extends Pick { @@ -334,6 +337,7 @@ export interface ICrudParams extends ICrudParams { cache?: number; + isAllowDistinct?: boolean; } export interface IListParams @@ -597,26 +601,69 @@ const defaultHandler = (query: TypeormJsonQuery): TypeormJsonQ */ const getQueryCount = async ( query: SelectQueryBuilder, - { hasRemoved = false, cache = 0, distinct }: IGetQueryCountParams = {}, + { + repository, + hasRemoved = false, + cache = 0, + isAllowDistinct = false, + }: IGetQueryCountParams, ): Promise => { if (hasRemoved) { query.withDeleted(); } - // Apply distinct select - if (distinct) { - query.select(`COUNT(DISTINCT "${distinct}")::integer`, 'count'); + if (query.getQuery().includes('DISTINCT')) { + return { count: await getQueryCountWithDistinct(query, repository, isAllowDistinct, cache) }; } + return { count: await getQueryCountWithoutDistinct(query, cache) }; +}; + +/** + * Returns count without distinct + */ +const getQueryCountWithoutDistinct = ( + query: SelectQueryBuilder, + cache: number, +): Promise => { if (cache) { - // Disable is only where condition - query.cache(getCrudCacheKey(query, CACHE_KEYS.count, { hasOnlyWhere: !distinct }), cache); + query.cache(getCrudCacheKey(query, CACHE_KEYS.count, { hasOnlyWhere: true }), cache); } - return { - // Returns raw count if distinct enabled - count: distinct ? (await query.getRawOne())?.count || 0 : await query.getCount(), - }; + return query.getCount(); +}; + +/** + * Returns count with distinct + */ +const getQueryCountWithDistinct = async ( + query: SelectQueryBuilder, + repository: Repository, + isAllowDistinct: boolean, + cache: number, +): Promise => { + if (!isAllowDistinct) { + throw new BaseException({ + code: CRUD_EXCEPTION_CODE.DISTINCT_SELECT_FORBIDDEN, + status: 422, + message: 'Distinct select is not allowed.', + }); + } + + if (cache) { + query.cache(getCrudCacheKey(query, CACHE_KEYS.count, { hasOnlyWhere: false }), cache); + } + + // Build result query + const resultQuery = repository.createQueryBuilder().select('COUNT(sub.*)::integer', 'result'); + + // Override result query expressions for preventing select from entity and then from sub query + resultQuery.expressionMap.aliases = []; + + // Add json query sub query as source + resultQuery.from(`(${query.getQuery()})`, 'sub'); + + return (await resultQuery.getRawOne<{ result: number }>())?.result ?? 0; }; /** @@ -1161,7 +1208,7 @@ class Endpoint { > { const countHandler: IReturn = async function (params, options) { - const { repository, queryOptions, cache } = countOptions(); + const { repository, queryOptions, cache, isAllowDistinct } = countOptions(); const typeQuery = createTypeQuery(repository.createQueryBuilder(), params, { relationOptions: ['*'], isDisableOrderBy: true, @@ -1169,12 +1216,12 @@ class Endpoint { ...queryOptions, }); const result = await handler(typeQuery, params, options); - const { hasRemoved, query: iJsonQuery } = params; + const { hasRemoved } = params; const defaultParams = { hasRemoved, cache, - // Check and cast to string from TEntity field - ...(typeof iJsonQuery?.distinct === 'string' ? { distinct: iJsonQuery.distinct } : {}), + isAllowDistinct, + repository, }; if (result instanceof TypeormJsonQuery) {