From fb63f21d8a66eedaa21451465630f84f99db1f6c Mon Sep 17 00:00:00 2001 From: Jaanus Sellin Date: Thu, 22 Feb 2024 15:35:16 +0200 Subject: [PATCH] feat: project applications paging backend (#6312) --- .../MultiActionButton/MultiActionButton.tsx | 4 +- src/lib/db/client-applications-store.ts | 95 ++++++++++++------- .../feature-search-controller.ts | 27 +++--- .../features/feature-search/search-utils.ts | 45 +++++++++ .../metrics/instance/instance-service.ts | 36 +++---- .../features/project/project-controller.ts | 25 +++-- src/lib/routes/admin-api/metrics.ts | 41 ++++++-- .../types/stores/client-applications-store.ts | 18 +++- .../fake-client-applications-store.ts | 19 ++-- 9 files changed, 212 insertions(+), 98 deletions(-) diff --git a/frontend/src/component/common/MultiActionButton/MultiActionButton.tsx b/frontend/src/component/common/MultiActionButton/MultiActionButton.tsx index c212d68fef..64a3e086e2 100644 --- a/frontend/src/component/common/MultiActionButton/MultiActionButton.tsx +++ b/frontend/src/component/common/MultiActionButton/MultiActionButton.tsx @@ -1,6 +1,4 @@ -import React, { FC, useContext } from 'react'; -import { useRequiredPathParam } from 'hooks/useRequiredPathParam'; -import { useChangeRequest } from 'hooks/api/getters/useChangeRequest/useChangeRequest'; +import React, { FC } from 'react'; import { ClickAwayListener, diff --git a/src/lib/db/client-applications-store.ts b/src/lib/db/client-applications-store.ts index 8cbe91ed74..ea4958d5bc 100644 --- a/src/lib/db/client-applications-store.ts +++ b/src/lib/db/client-applications-store.ts @@ -2,12 +2,14 @@ import EventEmitter from 'events'; import NotFoundError from '../error/notfound-error'; import { IClientApplication, + IClientApplications, + IClientApplicationsSearchParams, IClientApplicationsStore, } from '../types/stores/client-applications-store'; import { Logger, LogProvider } from '../logger'; -import { IApplicationQuery } from '../types/query'; import { Db } from './db'; import { IApplicationOverview } from '../features/metrics/instance/models'; +import { applySearchFilters } from '../features/feature-search/search-utils'; const COLUMNS = [ 'app_name', @@ -64,7 +66,12 @@ const reduceRows = (rows: any[]): IClientApplication[] => { ...mapRow(row), usage: project && environment - ? [{ project, environments: [environment] }] + ? [ + { + project, + environments: [environment], + }, + ] : [], }; } @@ -173,38 +180,62 @@ export default class ClientApplicationsStore return this.db(TABLE).where('app_name', appName).del(); } - /** - * Could also be done in SQL: - * (not sure if it is faster though) - * - * SELECT app_name from ( - * SELECT app_name, json_array_elements(strategies)::text as strategyName from client_strategies - * ) as foo - * WHERE foo.strategyName = '"other"'; - */ - async getAppsForStrategy( - query: IApplicationQuery, - ): Promise { - const rows = await this.db - .select([ - ...COLUMNS.map((column) => `${TABLE}.${column}`), - 'project', - 'environment', - ]) - .from(TABLE) - .leftJoin( - TABLE_USAGE, - `${TABLE_USAGE}.app_name`, - `${TABLE}.app_name`, - ); - const apps = reduceRows(rows); + async getApplications( + params: IClientApplicationsSearchParams, + ): Promise { + const { limit, offset, sortOrder = 'asc', searchParams } = params; + const validatedSortOrder = + sortOrder === 'asc' || sortOrder === 'desc' ? sortOrder : 'asc'; - if (query.strategyName) { - return apps.filter((app) => - app.strategies.includes(query.strategyName), - ); + const query = this.db + .with('applications', (qb) => { + applySearchFilters(qb, searchParams, [ + 'client_applications.app_name', + ]); + qb.select([ + ...COLUMNS.map((column) => `${TABLE}.${column}`), + 'project', + 'environment', + this.db.raw( + `DENSE_RANK() OVER (ORDER BY client_applications.app_name ${validatedSortOrder}) AS rank`, + ), + ]) + .from(TABLE) + .leftJoin( + TABLE_USAGE, + `${TABLE_USAGE}.app_name`, + `${TABLE}.app_name`, + ); + }) + .with( + 'final_ranks', + this.db.raw( + 'select row_number() over (order by min(rank)) as final_rank from applications group by app_name', + ), + ) + .with( + 'total', + this.db.raw('select count(*) as total from final_ranks'), + ) + .select('*') + .from('applications') + .joinRaw('CROSS JOIN total') + .whereBetween('rank', [offset + 1, offset + limit]); + + const rows = await query; + + if (rows.length !== 0) { + const applications = reduceRows(rows); + return { + applications, + total: Number(rows[0].total) || 0, + }; } - return apps; + + return { + applications: [], + total: 0, + }; } async getUnannounced(): Promise { diff --git a/src/lib/features/feature-search/feature-search-controller.ts b/src/lib/features/feature-search/feature-search-controller.ts index 5590e12e43..5c8b4d0a6f 100644 --- a/src/lib/features/feature-search/feature-search-controller.ts +++ b/src/lib/features/feature-search/feature-search-controller.ts @@ -19,6 +19,7 @@ import { FeatureSearchQueryParameters, featureSearchQueryParameters, } from '../../openapi/spec/feature-search-query-parameters'; +import { normalizeQueryParams } from './search-utils'; const PATH = '/features'; @@ -83,17 +84,21 @@ export default class FeatureSearchController extends Controller { createdAt, state, status, - offset, - limit = '50', - sortOrder, - sortBy, favoritesFirst, } = req.query; const userId = req.user.id; - const normalizedQuery = query - ?.split(',') - .map((query) => query.trim()) - .filter((query) => query); + const { + normalizedQuery, + normalizedSortBy, + normalizedSortOrder, + normalizedOffset, + normalizedLimit, + } = normalizeQueryParams(req.query, { + limitDefault: 50, + maxLimit: 100, + sortByDefault: 'createdAt', + }); + const normalizedStatus = status ?.map((tag) => tag.split(':')) .filter( @@ -101,12 +106,6 @@ export default class FeatureSearchController extends Controller { tag.length === 2 && ['enabled', 'disabled'].includes(tag[1]), ); - const normalizedLimit = - Number(limit) > 0 && Number(limit) <= 100 ? Number(limit) : 25; - const normalizedOffset = Number(offset) > 0 ? Number(offset) : 0; - const normalizedSortBy: string = sortBy ? sortBy : 'createdAt'; - const normalizedSortOrder = - sortOrder === 'asc' || sortOrder === 'desc' ? sortOrder : 'asc'; const normalizedFavoritesFirst = favoritesFirst === 'true'; const { features, total } = await this.featureSearchService.search({ searchParams: normalizedQuery, diff --git a/src/lib/features/feature-search/search-utils.ts b/src/lib/features/feature-search/search-utils.ts index d7c69f694f..6dff611cc8 100644 --- a/src/lib/features/feature-search/search-utils.ts +++ b/src/lib/features/feature-search/search-utils.ts @@ -1,6 +1,13 @@ import { Knex } from 'knex'; import { IQueryParam } from '../feature-toggle/types/feature-toggle-strategies-store-type'; +export interface NormalizeParamsDefaults { + limitDefault: number; + maxLimit?: number; // Optional because you might not always want to enforce a max limit + sortByDefault: string; + typeDefault?: string; // Optional field for type, not required for every call +} + export const applySearchFilters = ( qb: Knex.QueryBuilder, searchParams: string[] | undefined, @@ -45,3 +52,41 @@ export const applyGenericQueryParams = ( } }); }; + +export const normalizeQueryParams = ( + params, + defaults: NormalizeParamsDefaults, +) => { + const { + query, + offset, + limit = defaults.limitDefault, + sortOrder, + sortBy = defaults.sortByDefault, + } = params; + + const normalizedQuery = query + ?.split(',') + .map((query) => query.trim()) + .filter((query) => query); + + const maxLimit = defaults.maxLimit || 1000; + const normalizedLimit = + Number(limit) > 0 && Number(limit) <= maxLimit + ? Number(limit) + : defaults.limitDefault; + + const normalizedOffset = Number(offset) > 0 ? Number(offset) : 0; + + const normalizedSortBy = sortBy; + const normalizedSortOrder = + sortOrder === 'asc' || sortOrder === 'desc' ? sortOrder : 'asc'; + + return { + normalizedQuery, + normalizedLimit, + normalizedOffset, + normalizedSortBy, + normalizedSortOrder, + }; +}; diff --git a/src/lib/features/metrics/instance/instance-service.ts b/src/lib/features/metrics/instance/instance-service.ts index f50c65f23a..0db69cf88a 100644 --- a/src/lib/features/metrics/instance/instance-service.ts +++ b/src/lib/features/metrics/instance/instance-service.ts @@ -5,12 +5,13 @@ import { IUnleashConfig } from '../../../types/option'; import { IEventStore } from '../../../types/stores/event-store'; import { IClientApplication, + IClientApplications, + IClientApplicationsSearchParams, IClientApplicationsStore, } from '../../../types/stores/client-applications-store'; import { IFeatureToggleStore } from '../../feature-toggle/types/feature-toggle-store-type'; import { IStrategyStore } from '../../../types/stores/strategy-store'; import { IClientInstanceStore } from '../../../types/stores/client-instance-store'; -import { IApplicationQuery } from '../../../types/query'; import { IClientApp } from '../../../types/model'; import { clientRegisterSchema } from '../shared/schema'; @@ -155,28 +156,31 @@ export default class ClientInstanceService { } async getApplications( - query: IApplicationQuery, + query: IClientApplicationsSearchParams, userId: number, - ): Promise { + ): Promise { const applications = - await this.clientApplicationsStore.getAppsForStrategy(query); + await this.clientApplicationsStore.getApplications(query); const accessibleProjects = await this.privateProjectChecker.getUserAccessibleProjects(userId); if (accessibleProjects.mode === 'all') { return applications; } else { - return applications.map((application) => { - return { - ...application, - usage: application.usage?.filter( - (usageItem) => - usageItem.project === ALL_PROJECTS || - accessibleProjects.projects.includes( - usageItem.project, - ), - ), - }; - }); + return { + applications: applications.applications.map((application) => { + return { + ...application, + usage: application.usage?.filter( + (usageItem) => + usageItem.project === ALL_PROJECTS || + accessibleProjects.projects.includes( + usageItem.project, + ), + ), + }; + }), + total: applications.total, + }; } } diff --git a/src/lib/features/project/project-controller.ts b/src/lib/features/project/project-controller.ts index 8c48b68fd7..e979386cea 100644 --- a/src/lib/features/project/project-controller.ts +++ b/src/lib/features/project/project-controller.ts @@ -39,6 +39,7 @@ import { } from '../../openapi/spec/project-applications-schema'; import { NotFoundError } from '../../error'; import { projectApplicationsQueryParameters } from '../../openapi/spec/project-applications-query-parameters'; +import { normalizeQueryParams } from '../feature-search/search-utils'; export default class ProjectController extends Controller { private projectService: ProjectService; @@ -272,21 +273,19 @@ export default class ProjectController extends Controller { throw new NotFoundError(); } - const { query, offset, limit = '50', sortOrder, sortBy } = req.query; - const { projectId } = req.params; - 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 { + normalizedQuery, + normalizedSortBy, + normalizedSortOrder, + normalizedOffset, + normalizedLimit, + } = normalizeQueryParams(req.query, { + limitDefault: 50, + maxLimit: 100, + sortByDefault: 'appName', + }); const applications = await this.projectService.getApplications({ searchParams: normalizedQuery, diff --git a/src/lib/routes/admin-api/metrics.ts b/src/lib/routes/admin-api/metrics.ts index db1a733770..975e68868c 100644 --- a/src/lib/routes/admin-api/metrics.ts +++ b/src/lib/routes/admin-api/metrics.ts @@ -24,6 +24,7 @@ import { } from '../../openapi/spec/application-overview-schema'; import { OpenApiService } from '../../services'; import { applicationsQueryParameters } from '../../openapi/spec/applications-query-parameters'; +import { normalizeQueryParams } from '../../features/feature-search/search-utils'; class MetricsController extends Controller { private logger: Logger; @@ -162,7 +163,9 @@ class MetricsController extends Controller { } async deleteApplication( - req: Request<{ appName: string }>, + req: Request<{ + appName: string; + }>, res: Response, ): Promise { const { appName } = req.params; @@ -172,7 +175,13 @@ class MetricsController extends Controller { } async createApplication( - req: Request<{ appName: string }, unknown, CreateApplicationSchema>, + req: Request< + { + appName: string; + }, + unknown, + CreateApplicationSchema + >, res: Response, ): Promise { const input = { @@ -188,15 +197,29 @@ class MetricsController extends Controller { res: Response, ): Promise { const { user } = req; - const query = req.query.strategyName - ? { strategyName: req.query.strategyName as string } - : {}; + const { + normalizedQuery, + normalizedSortBy, + normalizedSortOrder, + normalizedOffset, + normalizedLimit, + } = normalizeQueryParams(req.query, { + limitDefault: 1000, + maxLimit: 1000, + sortByDefault: 'appName', + }); + const applications = await this.clientInstanceService.getApplications( - query, + { + searchParams: normalizedQuery, + offset: normalizedOffset, + limit: normalizedLimit, + sortBy: normalizedSortBy, + sortOrder: normalizedSortOrder, + }, extractUserIdFromUser(user), ); - // todo: change to total with pagination later - res.json({ applications, total: applications.length }); + res.json(applications); } async getApplication( @@ -209,6 +232,7 @@ class MetricsController extends Controller { await this.clientInstanceService.getApplication(appName); res.json(appDetails); } + async getApplicationOverview( req: Request, res: Response, @@ -228,4 +252,5 @@ class MetricsController extends Controller { ); } } + export default MetricsController; diff --git a/src/lib/types/stores/client-applications-store.ts b/src/lib/types/stores/client-applications-store.ts index dc0ec008ad..317b56ca9c 100644 --- a/src/lib/types/stores/client-applications-store.ts +++ b/src/lib/types/stores/client-applications-store.ts @@ -1,5 +1,4 @@ import { Store } from './store'; -import { IApplicationQuery } from '../query'; import { IApplicationOverview } from '../../features/metrics/instance/models'; export interface IClientApplicationUsage { @@ -22,11 +21,26 @@ export interface IClientApplication { usage?: IClientApplicationUsage[]; } +export interface IClientApplications { + applications: IClientApplication[]; + total: number; +} + +export interface IClientApplicationsSearchParams { + searchParams?: string[]; + offset: number; + limit: number; + sortBy: string; + sortOrder: 'asc' | 'desc'; +} + export interface IClientApplicationsStore extends Store { upsert(details: Partial): Promise; bulkUpsert(details: Partial[]): Promise; - getAppsForStrategy(query: IApplicationQuery): Promise; + getApplications( + params: IClientApplicationsSearchParams, + ): Promise; getUnannounced(): Promise; setUnannouncedToAnnounced(): Promise; getApplicationOverview(appName: string): Promise; diff --git a/src/test/fixtures/fake-client-applications-store.ts b/src/test/fixtures/fake-client-applications-store.ts index b1f76aef27..441bf25233 100644 --- a/src/test/fixtures/fake-client-applications-store.ts +++ b/src/test/fixtures/fake-client-applications-store.ts @@ -1,9 +1,10 @@ import { IClientApplication, + IClientApplications, + IClientApplicationsSearchParams, IClientApplicationsStore, } from '../../lib/types/stores/client-applications-store'; import NotFoundError from '../../lib/error/notfound-error'; -import { IApplicationQuery } from '../../lib/types/query'; import { IApplicationOverview } from '../../lib/features/metrics/instance/models'; export default class FakeClientApplicationsStore @@ -55,15 +56,13 @@ export default class FakeClientApplicationsStore return this.get(appName); } - async getAppsForStrategy( - query: IApplicationQuery, - ): Promise { - if (query.strategyName) { - return this.apps.filter((a) => - a.strategies.includes(query.strategyName), - ); - } - return this.apps; + async getApplications( + query: IClientApplicationsSearchParams, + ): Promise { + return { + applications: this.apps, + total: this.apps.length, + }; } async getUnannounced(): Promise {