From 3d778254932675011b1e27e422d6906523285dc6 Mon Sep 17 00:00:00 2001 From: Jaanus Sellin Date: Wed, 14 Feb 2024 13:03:44 +0200 Subject: [PATCH] feat: project applications server side paging and sorting and filtering (#6236) Uses exactly same pattern as search-store. Nothing too crazy here. Most code is in tests. --- .../UsersChart/UsersChart.tsx | 1 - .../feature-search/feature-search-store.ts | 52 +--- .../features/feature-search/search-utils.ts | 47 +++ .../project/project-applications.e2e.test.ts | 279 +++++++++++++++--- .../features/project/project-controller.ts | 24 +- src/lib/features/project/project-service.ts | 9 +- .../features/project/project-store-type.ts | 15 +- src/lib/features/project/project-store.ts | 74 +++-- .../spec/project-applications-schema.test.ts | 29 +- .../spec/project-applications-schema.ts | 18 +- src/lib/types/model.ts | 5 + src/test/fixtures/fake-project-store.ts | 7 +- 12 files changed, 435 insertions(+), 125 deletions(-) create mode 100644 src/lib/features/feature-search/search-utils.ts diff --git a/frontend/src/component/executiveDashboard/UsersChart/UsersChart.tsx b/frontend/src/component/executiveDashboard/UsersChart/UsersChart.tsx index a004628276..44d590feda 100644 --- a/frontend/src/component/executiveDashboard/UsersChart/UsersChart.tsx +++ b/frontend/src/component/executiveDashboard/UsersChart/UsersChart.tsx @@ -7,7 +7,6 @@ import { LineChart, NotEnoughData, } from '../LineChart/LineChart'; -import { type ScriptableContext } from 'chart.js'; interface IUsersChartProps { userTrends: ExecutiveSummarySchema['userTrends']; diff --git a/src/lib/features/feature-search/feature-search-store.ts b/src/lib/features/feature-search/feature-search-store.ts index 11e4706b42..086a7773da 100644 --- a/src/lib/features/feature-search/feature-search-store.ts +++ b/src/lib/features/feature-search/feature-search-store.ts @@ -16,6 +16,7 @@ import { IFeatureSearchParams, IQueryParam, } from '../feature-toggle/types/feature-toggle-strategies-store-type'; +import { applyGenericQueryParams, applySearchFilters } from './search-utils'; const sortEnvironments = (overview: IFeatureOverview[]) => { return overview.map((data: IFeatureOverview) => ({ @@ -87,28 +88,10 @@ class FeatureSearchStore implements IFeatureSearchStore { query.from('features'); applyQueryParams(query, queryParams); - - const hasSearchParams = searchParams?.length; - if (hasSearchParams) { - const sqlParameters = searchParams.map( - (item) => `%${item}%`, - ); - const sqlQueryParameters = sqlParameters - .map(() => '?') - .join(','); - - query.where((builder) => { - builder - .orWhereRaw( - `(??) ILIKE ANY (ARRAY[${sqlQueryParameters}])`, - ['features.name', ...sqlParameters], - ) - .orWhereRaw( - `(??) ILIKE ANY (ARRAY[${sqlQueryParameters}])`, - ['features.description', ...sqlParameters], - ); - }); - } + applySearchFilters(query, searchParams, [ + 'features.name', + 'features.description', + ]); if (type) { query.whereIn('features.type', type); @@ -282,7 +265,6 @@ class FeatureSearchStore implements IFeatureSearchStore { ) .joinRaw('CROSS JOIN total_features') .whereBetween('final_rank', [offset + 1, offset + limit]); - const rows = await finalQuery; stopTimer(); if (rows.length > 0) { @@ -417,30 +399,6 @@ const applyQueryParams = ( ); }; -const applyGenericQueryParams = ( - query: Knex.QueryBuilder, - queryParams: IQueryParam[], -): void => { - queryParams.forEach((param) => { - switch (param.operator) { - case 'IS': - case 'IS_ANY_OF': - query.whereIn(param.field, param.values); - break; - case 'IS_NOT': - case 'IS_NONE_OF': - query.whereNotIn(param.field, param.values); - break; - case 'IS_BEFORE': - query.where(param.field, '<', param.values[0]); - break; - case 'IS_ON_OR_AFTER': - query.where(param.field, '>=', param.values[0]); - break; - } - }); -}; - const applyMultiQueryParams = ( query: Knex.QueryBuilder, queryParams: IQueryParam[], diff --git a/src/lib/features/feature-search/search-utils.ts b/src/lib/features/feature-search/search-utils.ts new file mode 100644 index 0000000000..d7c69f694f --- /dev/null +++ b/src/lib/features/feature-search/search-utils.ts @@ -0,0 +1,47 @@ +import { Knex } from 'knex'; +import { IQueryParam } from '../feature-toggle/types/feature-toggle-strategies-store-type'; + +export const applySearchFilters = ( + qb: Knex.QueryBuilder, + searchParams: string[] | undefined, + columns: string[], +): void => { + const hasSearchParams = searchParams?.length; + if (hasSearchParams) { + const sqlParameters = searchParams.map((item) => `%${item}%`); + const sqlQueryParameters = sqlParameters.map(() => '?').join(','); + + qb.where((builder) => { + columns.forEach((column) => { + builder.orWhereRaw( + `(${column}) ILIKE ANY (ARRAY[${sqlQueryParameters}])`, + sqlParameters, + ); + }); + }); + } +}; + +export const applyGenericQueryParams = ( + query: Knex.QueryBuilder, + queryParams: IQueryParam[], +): void => { + queryParams.forEach((param) => { + switch (param.operator) { + case 'IS': + case 'IS_ANY_OF': + query.whereIn(param.field, param.values); + break; + case 'IS_NOT': + case 'IS_NONE_OF': + query.whereNotIn(param.field, param.values); + break; + case 'IS_BEFORE': + query.where(param.field, '<', param.values[0]); + break; + case 'IS_ON_OR_AFTER': + query.where(param.field, '>=', param.values[0]); + break; + } + }); +}; diff --git a/src/lib/features/project/project-applications.e2e.test.ts b/src/lib/features/project/project-applications.e2e.test.ts index 2678cd157e..7633a31d59 100644 --- a/src/lib/features/project/project-applications.e2e.test.ts +++ b/src/lib/features/project/project-applications.e2e.test.ts @@ -89,19 +89,22 @@ test('should return applications', async () => { .expect('Content-Type', /json/) .expect(200); - expect(body).toMatchObject([ - { - environments: ['default'], - instances: ['instanceId'], - name: 'appName', - sdks: [ - { - name: 'unleash-client-test', - versions: ['1.2'], - }, - ], - }, - ]); + expect(body).toMatchObject({ + applications: [ + { + environments: ['default'], + instances: ['instanceId'], + name: 'appName', + sdks: [ + { + name: 'unleash-client-test', + versions: ['1.2'], + }, + ], + }, + ], + total: 1, + }); }); test('should return applications if sdk was not in database', async () => { @@ -128,14 +131,17 @@ test('should return applications if sdk was not in database', async () => { .expect('Content-Type', /json/) .expect(200); - expect(body).toMatchObject([ - { - environments: ['default'], - instances: ['instanceId'], - name: 'appName', - sdks: [], - }, - ]); + expect(body).toMatchObject({ + applications: [ + { + environments: ['default'], + instances: ['instanceId'], + name: 'appName', + sdks: [], + }, + ], + total: 1, + }); }); test('should return application without version if sdk has just name', async () => { @@ -163,17 +169,222 @@ test('should return application without version if sdk has just name', async () .expect('Content-Type', /json/) .expect(200); - expect(body).toMatchObject([ - { - environments: ['default'], - instances: ['instanceId'], - name: 'appName', - sdks: [ - { - name: 'unleash-client-test', - versions: [], - }, - ], - }, - ]); + expect(body).toMatchObject({ + applications: [ + { + environments: ['default'], + instances: ['instanceId'], + name: 'appName', + sdks: [ + { + name: 'unleash-client-test', + versions: [], + }, + ], + }, + ], + total: 1, + }); +}); + +test('should sort by appName descending', async () => { + await app.createFeature('toggle-name-1'); + + await app.request.post('/api/client/register').send({ + appName: metrics.appName, + instanceId: metrics.instanceId, + strategies: ['default'], + sdkVersion: 'unleash-client-test', + started: Date.now(), + interval: 10, + }); + const secondApp = 'second-app'; + await app.request.post('/api/client/register').send({ + appName: secondApp, + instanceId: metrics.instanceId, + strategies: ['default'], + sdkVersion: 'unleash-client-test', + started: Date.now(), + interval: 10, + }); + await app.services.clientInstanceService.bulkAdd(); + await app.request + .post('/api/client/metrics') + .set('Authorization', defaultToken.secret) + .send(metrics) + .expect(202); + + await app.request + .post('/api/client/metrics') + .set('Authorization', defaultToken.secret) + .send({ + ...metrics, + appName: secondApp, + }) + .expect(202); + + await app.services.clientMetricsServiceV2.bulkAdd(); + + const { body } = await app.request + .get('/api/admin/projects/default/applications?sortOrder=desc') + .expect('Content-Type', /json/) + .expect(200); + + expect(body).toMatchObject({ + applications: [ + { + environments: ['default'], + instances: ['instanceId'], + name: 'second-app', + sdks: [ + { + name: 'unleash-client-test', + versions: [], + }, + ], + }, + { + environments: ['default'], + instances: ['instanceId'], + name: 'appName', + sdks: [ + { + name: 'unleash-client-test', + versions: [], + }, + ], + }, + ], + total: 2, + }); +}); + +test('should filter by sdk', async () => { + await app.createFeature('toggle-name-1'); + + await app.request.post('/api/client/register').send({ + appName: metrics.appName, + instanceId: metrics.instanceId, + strategies: ['default'], + sdkVersion: 'unleash-java-test', + started: Date.now(), + interval: 10, + }); + const secondApp = 'second-app'; + await app.request.post('/api/client/register').send({ + appName: secondApp, + instanceId: metrics.instanceId, + strategies: ['default'], + sdkVersion: 'unleash-client-test', + started: Date.now(), + interval: 10, + }); + await app.services.clientInstanceService.bulkAdd(); + await app.request + .post('/api/client/metrics') + .set('Authorization', defaultToken.secret) + .send(metrics) + .expect(202); + + await app.request + .post('/api/client/metrics') + .set('Authorization', defaultToken.secret) + .send({ + ...metrics, + appName: secondApp, + }) + .expect(202); + + await app.services.clientMetricsServiceV2.bulkAdd(); + + const { body } = await app.request + .get('/api/admin/projects/default/applications?&query=java') + .expect('Content-Type', /json/) + .expect(200); + + expect(body).toMatchObject({ + applications: [ + { + environments: ['default'], + instances: ['instanceId'], + name: 'appName', + sdks: [ + { + name: 'unleash-java-test', + versions: [], + }, + ], + }, + ], + total: 1, + }); +}); + +test('should show correct number of total', async () => { + await app.createFeature('toggle-name-1'); + + await app.request.post('/api/client/register').send({ + appName: metrics.appName, + instanceId: metrics.instanceId, + strategies: ['default'], + sdkVersion: 'unleash-client-test', + started: Date.now(), + interval: 10, + }); + await app.request.post('/api/client/register').send({ + appName: metrics.appName, + instanceId: 'another-instance', + strategies: ['default'], + sdkVersion: 'unleash-client-test', + started: Date.now(), + interval: 10, + }); + const secondApp = 'second-app'; + await app.request.post('/api/client/register').send({ + appName: secondApp, + instanceId: metrics.instanceId, + strategies: ['default'], + sdkVersion: 'unleash-client-test', + started: Date.now(), + interval: 10, + }); + await app.services.clientInstanceService.bulkAdd(); + await app.request + .post('/api/client/metrics') + .set('Authorization', defaultToken.secret) + .send(metrics) + .expect(202); + + await app.request + .post('/api/client/metrics') + .set('Authorization', defaultToken.secret) + .send({ + ...metrics, + appName: secondApp, + }) + .expect(202); + + await app.services.clientMetricsServiceV2.bulkAdd(); + + const { body } = await app.request + .get('/api/admin/projects/default/applications?sortOrder=desc&limit=1') + .expect('Content-Type', /json/) + .expect(200); + + expect(body).toMatchObject({ + applications: [ + { + environments: ['default'], + instances: ['instanceId'], + name: 'second-app', + sdks: [ + { + name: 'unleash-client-test', + versions: [], + }, + ], + }, + ], + total: 2, + }); }); diff --git a/src/lib/features/project/project-controller.ts b/src/lib/features/project/project-controller.ts index 88445891a3..61543fff1b 100644 --- a/src/lib/features/project/project-controller.ts +++ b/src/lib/features/project/project-controller.ts @@ -267,10 +267,30 @@ export default class ProjectController extends Controller { throw new NotFoundError(); } + const { query, offset, limit = '50', sortOrder, sortBy } = req.query; + const { projectId } = req.params; - const applications = - await this.projectService.getApplications(projectId); + const normalizedQuery = query + ?.split(',') + .map((query) => query.trim()) + .filter((query) => query); + + const normalizedLimit = + Number(limit) > 0 && Number(limit) <= 100 ? Number(limit) : 25; + const normalizedOffset = Number(offset) > 0 ? Number(offset) : 0; + const normalizedSortBy: string = sortBy ? sortBy : 'appName'; + const normalizedSortOrder = + sortOrder === 'asc' || sortOrder === 'desc' ? sortOrder : 'asc'; + + const applications = await this.projectService.getApplications({ + searchParams: normalizedQuery, + project: projectId, + offset: normalizedOffset, + limit: normalizedLimit, + sortBy: normalizedSortBy, + sortOrder: normalizedSortOrder, + }); this.openApiService.respondWithValidation( 200, diff --git a/src/lib/features/project/project-service.ts b/src/lib/features/project/project-service.ts index a4f0120ea1..27964e55e0 100644 --- a/src/lib/features/project/project-service.ts +++ b/src/lib/features/project/project-service.ts @@ -42,8 +42,8 @@ import { IProjectUpdate, IProjectHealth, SYSTEM_USER, - IProjectApplication, IProjectStore, + IProjectApplications, } from '../../types'; import { IProjectAccessModel, @@ -65,6 +65,7 @@ import { checkFeatureNamingData } from '../feature-naming-pattern/feature-naming import { IPrivateProjectChecker } from '../private-project/privateProjectCheckerType'; import EventService from '../events/event-service'; import { + IProjectApplicationsSearchParams, IProjectEnterpriseSettingsUpdate, IProjectQuery, } from './project-store-type'; @@ -901,9 +902,11 @@ export default class ProjectService { }; } - async getApplications(projectId: string): Promise { + async getApplications( + searchParams: IProjectApplicationsSearchParams, + ): Promise { const applications = - await this.projectStore.getApplicationsByProject(projectId); + await this.projectStore.getApplicationsByProject(searchParams); return applications; } diff --git a/src/lib/features/project/project-store-type.ts b/src/lib/features/project/project-store-type.ts index 4ad439e175..49490c3641 100644 --- a/src/lib/features/project/project-store-type.ts +++ b/src/lib/features/project/project-store-type.ts @@ -7,7 +7,7 @@ import { IEnvironment, IFeatureNaming, IProject, - IProjectApplication, + IProjectApplications, IProjectWithCount, ProjectMode, } from '../../types/model'; @@ -55,6 +55,15 @@ export type ProjectEnvironment = { defaultStrategy?: CreateFeatureStrategySchema; }; +export interface IProjectApplicationsSearchParams { + searchParams?: string[]; + project?: string; + offset: number; + limit: number; + sortBy: string; + sortOrder: 'asc' | 'desc'; +} + export interface IProjectStore extends Store { hasProject(id: string): Promise; @@ -122,5 +131,7 @@ export interface IProjectStore extends Store { isFeatureLimitReached(id: string): Promise; getProjectModeCounts(): Promise; - getApplicationsByProject(projectId: string): Promise; + getApplicationsByProject( + searchParams: IProjectApplicationsSearchParams, + ): Promise; } diff --git a/src/lib/features/project/project-store.ts b/src/lib/features/project/project-store.ts index 02395dab13..8d5be980bc 100644 --- a/src/lib/features/project/project-store.ts +++ b/src/lib/features/project/project-store.ts @@ -7,6 +7,7 @@ import { IFlagResolver, IProject, IProjectApplication, + IProjectApplications, IProjectUpdate, IProjectWithCount, ProjectMode, @@ -19,6 +20,7 @@ import { IProjectEnterpriseSettingsUpdate, IProjectStore, ProjectEnvironment, + IProjectApplicationsSearchParams, } from '../../features/project/project-store-type'; import { DEFAULT_ENV } from '../../util'; import metricsHelper from '../../util/metrics-helper'; @@ -27,6 +29,7 @@ import EventEmitter from 'events'; import { Db } from '../../db/db'; import Raw = Knex.Raw; import { CreateFeatureStrategySchema } from '../../openapi'; +import { applySearchFilters } from '../feature-search/search-utils'; const COLUMNS = [ 'id', @@ -579,32 +582,66 @@ class ProjectStore implements IProjectStore { } async getApplicationsByProject( - projectId: string, - ): Promise { + params: IProjectApplicationsSearchParams, + ): Promise { + const { project, limit, sortOrder, sortBy, searchParams, offset } = + params; + const validatedSortOrder = + sortOrder === 'asc' || sortOrder === 'desc' ? sortOrder : 'asc'; const query = this.db .with('applications', (qb) => { qb.select('project', 'app_name', 'environment') .distinct() .from('client_metrics_env as cme') .leftJoin('features as f', 'cme.feature_name', 'f.name') - .where('project', projectId); + .where('project', project); }) - .select( - 'a.app_name', - 'a.environment', - 'ci.instance_id', - 'ci.sdk_version', - ) - .from('applications as a') - .leftJoin('client_instances as ci', function () { - this.on('ci.app_name', 'a.app_name').andOn( - 'ci.environment', + .with('ranked', (qb) => { + applySearchFilters(qb, searchParams, [ + 'a.app_name', 'a.environment', - ); - }); + 'ci.instance_id', + 'ci.sdk_version', + ]); + + qb.select( + 'a.app_name', + 'a.environment', + 'ci.instance_id', + 'ci.sdk_version', + this.db.raw( + `DENSE_RANK() OVER (ORDER BY a.app_name ${validatedSortOrder}) AS rank`, + ), + ) + .from('applications as a') + .leftJoin('client_instances as ci', function () { + this.on('ci.app_name', '=', 'a.app_name').andOn( + 'ci.environment', + '=', + 'a.environment', + ); + }); + }) + .with( + 'final_ranks', + this.db.raw( + 'select row_number() over (order by min(rank)) as final_rank from ranked group by app_name', + ), + ) + .with( + 'total', + this.db.raw('select count(*) as total from final_ranks'), + ) + .select('*') + .from('ranked') + .joinRaw('CROSS JOIN total') + .whereBetween('rank', [offset + 1, offset + limit]); const rows = await query; const applications = this.getAggregatedApplicationsData(rows); - return applications; + return { + applications, + total: Number(rows[0].total) || 0, + }; } async getDefaultStrategy( @@ -751,7 +788,10 @@ class ProjectStore implements IProjectStore { let sdk = entry.sdks.find((sdk) => sdk.name === sdkName); if (!sdk) { - sdk = { name: sdkName, versions: [] }; + sdk = { + name: sdkName, + versions: [], + }; entry.sdks.push(sdk); } diff --git a/src/lib/openapi/spec/project-applications-schema.test.ts b/src/lib/openapi/spec/project-applications-schema.test.ts index cd2258c3de..fee0ee2184 100644 --- a/src/lib/openapi/spec/project-applications-schema.test.ts +++ b/src/lib/openapi/spec/project-applications-schema.test.ts @@ -2,19 +2,22 @@ import { validateSchema } from '../validate'; import { ProjectApplicationsSchema } from './project-applications-schema'; test('projectApplicationsSchema', () => { - const data: ProjectApplicationsSchema = [ - { - name: 'my-weba-app', - environments: ['development', 'production'], - instances: ['instance-414122'], - sdks: [ - { - name: 'unleash-client-node', - versions: ['4.1.1'], - }, - ], - }, - ]; + const data: ProjectApplicationsSchema = { + total: 55, + applications: [ + { + name: 'my-weba-app', + environments: ['development', 'production'], + instances: ['instance-414122'], + sdks: [ + { + name: 'unleash-client-node', + versions: ['4.1.1'], + }, + ], + }, + ], + }; expect( validateSchema('#/components/schemas/projectApplicationsSchema', data), diff --git a/src/lib/openapi/spec/project-applications-schema.ts b/src/lib/openapi/spec/project-applications-schema.ts index 0a1ad31064..131e48e657 100644 --- a/src/lib/openapi/spec/project-applications-schema.ts +++ b/src/lib/openapi/spec/project-applications-schema.ts @@ -4,10 +4,22 @@ import { projectApplicationSdkSchema } from './project-application-sdk-schema'; export const projectApplicationsSchema = { $id: '#/components/schemas/projectApplicationsSchema', - type: 'array', + type: 'object', description: 'A list of project applications', - items: { - $ref: '#/components/schemas/projectApplicationSchema', + required: ['total', 'applications'], + properties: { + total: { + type: 'integer', + example: 50, + description: 'The total number of project applications.', + }, + applications: { + type: 'array', + items: { + $ref: '#/components/schemas/projectApplicationSchema', + }, + description: 'All applications defined for a specific project.', + }, }, components: { schemas: { diff --git a/src/lib/types/model.ts b/src/lib/types/model.ts index 6231461a2e..79d85fed08 100644 --- a/src/lib/types/model.ts +++ b/src/lib/types/model.ts @@ -478,6 +478,11 @@ export interface IProject { featureNaming?: IFeatureNaming; } +export interface IProjectApplications { + applications: IProjectApplication[]; + total: number; +} + export interface IProjectApplication { name: string; environments: string[]; diff --git a/src/test/fixtures/fake-project-store.ts b/src/test/fixtures/fake-project-store.ts index 3415034faa..5adbdbd654 100644 --- a/src/test/fixtures/fake-project-store.ts +++ b/src/test/fixtures/fake-project-store.ts @@ -1,7 +1,7 @@ import { IEnvironment, IProject, - IProjectApplication, + IProjectApplications, IProjectStore, IProjectWithCount, } from '../../lib/types'; @@ -13,6 +13,7 @@ import { } from '../../lib/features/project/project-store'; import { CreateFeatureStrategySchema } from '../../lib/openapi'; import { + IProjectApplicationsSearchParams, IProjectHealthUpdate, IProjectInsert, ProjectEnvironment, @@ -209,8 +210,8 @@ export default class FakeProjectStore implements IProjectStore { } // eslint-disable-next-line @typescript-eslint/no-unused-vars getApplicationsByProject( - projectId: string, - ): Promise { + searchParams: IProjectApplicationsSearchParams, + ): Promise { throw new Error('Method not implemented.'); } }