From fbdae0df0a9603443fddd53fc1714095f01b025e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivar=20Conradi=20=C3=98sthus?= Date: Thu, 7 Oct 2021 21:10:15 +0200 Subject: [PATCH] fix: more unit tests --- src/lib/db/client-metrics-store-v2.ts | 3 + src/lib/routes/client-api/metrics.ts | 15 ++- src/lib/types/model.ts | 1 + .../client/feature.env.disabled.e2e.test.ts | 2 +- src/test/e2e/api/client/metrics.e2e.test.ts | 7 +- src/test/e2e/api/client/metricsV2.e2e.test.ts | 100 ++++++++++++++++++ src/test/e2e/helpers/test-helper.ts | 5 +- .../client-metrics-store-v2.e2e.test.ts | 14 +++ 8 files changed, 136 insertions(+), 11 deletions(-) create mode 100644 src/test/e2e/api/client/metricsV2.e2e.test.ts diff --git a/src/lib/db/client-metrics-store-v2.ts b/src/lib/db/client-metrics-store-v2.ts index dcf080f73c..ad818c0ff7 100644 --- a/src/lib/db/client-metrics-store-v2.ts +++ b/src/lib/db/client-metrics-store-v2.ts @@ -82,6 +82,9 @@ export class ClientMetricsStoreV2 implements IClientMetricsStoreV2 { // this function will collapse metrics before sending it to the database. async batchInsertMetrics(metrics: IClientMetricsEnv[]): Promise { + if (!metrics || metrics.length == 0) { + return; + } const rows = metrics.map(toRow); const batch = rows.reduce((prev, curr) => { diff --git a/src/lib/routes/client-api/metrics.ts b/src/lib/routes/client-api/metrics.ts index 8a136ce78c..4e7f032946 100644 --- a/src/lib/routes/client-api/metrics.ts +++ b/src/lib/routes/client-api/metrics.ts @@ -8,6 +8,8 @@ import { IAuthRequest } from '../unleash-types'; import ApiUser from '../../types/api-user'; import { ALL } from '../../types/models/api-token'; import ClientMetricsServiceV2 from '../../services/client-metrics/client-metrics-service-v2'; +import { User } from '../../server-impl'; +import { IClientApp } from '../../types/model'; export default class ClientMetricsController extends Controller { logger: Logger; @@ -42,13 +44,20 @@ export default class ClientMetricsController extends Controller { this.post('/', this.registerMetrics); } - async registerMetrics(req: IAuthRequest, res: Response): Promise { - const { body: data, ip: clientIp, user } = req; + private resolveEnvironment(user: User, data: IClientApp) { if (user instanceof ApiUser) { if (user.environment !== ALL) { - data.environment = user.environment; + return user.environment; + } else if (user.environment === ALL && data.environment) { + return data.environment; } } + return 'default'; + } + + async registerMetrics(req: IAuthRequest, res: Response): Promise { + const { body: data, ip: clientIp, user } = req; + data.environment = this.resolveEnvironment(user, data); await this.metrics.registerClientMetrics(data, clientIp); if (this.newServiceEnabled) { diff --git a/src/lib/types/model.ts b/src/lib/types/model.ts index 71ece8cc8f..9dd8a9f3bf 100644 --- a/src/lib/types/model.ts +++ b/src/lib/types/model.ts @@ -258,6 +258,7 @@ export interface IClientApp { appName: string; instanceId: string; clientIp?: string; + environment?: string; seenToggles?: string[]; metricsCount?: number; strategies?: string[] | Record[]; diff --git a/src/test/e2e/api/client/feature.env.disabled.e2e.test.ts b/src/test/e2e/api/client/feature.env.disabled.e2e.test.ts index b28d49bd1b..adb4fa5599 100644 --- a/src/test/e2e/api/client/feature.env.disabled.e2e.test.ts +++ b/src/test/e2e/api/client/feature.env.disabled.e2e.test.ts @@ -8,7 +8,7 @@ let db: ITestDb; const featureName = 'feature.default.1'; beforeAll(async () => { - db = await dbInit('feature_api_client', getLogger); + db = await dbInit('feature_env_api_client', getLogger); app = await setupApp(db.stores); await app.services.featureToggleServiceV2.createFeatureToggle( diff --git a/src/test/e2e/api/client/metrics.e2e.test.ts b/src/test/e2e/api/client/metrics.e2e.test.ts index 30ac24b378..da9e7f07e8 100644 --- a/src/test/e2e/api/client/metrics.e2e.test.ts +++ b/src/test/e2e/api/client/metrics.e2e.test.ts @@ -16,8 +16,7 @@ afterAll(async () => { await db.destroy(); }); -test('should be possble to send metrics', async () => { - expect.assertions(0); +test('should be possible to send metrics', async () => { return app.request .post('/api/client/metrics') .send(metricsExample) @@ -25,7 +24,6 @@ test('should be possble to send metrics', async () => { }); test('should require valid send metrics', async () => { - expect.assertions(0); return app.request .post('/api/client/metrics') .send({ @@ -34,8 +32,7 @@ test('should require valid send metrics', async () => { .expect(400); }); -test('should accept client metrics', async () => { - expect.assertions(0); +test('should accept empty client metrics', async () => { return app.request .post('/api/client/metrics') .send({ diff --git a/src/test/e2e/api/client/metricsV2.e2e.test.ts b/src/test/e2e/api/client/metricsV2.e2e.test.ts new file mode 100644 index 0000000000..dff65b4b50 --- /dev/null +++ b/src/test/e2e/api/client/metricsV2.e2e.test.ts @@ -0,0 +1,100 @@ +import { IUnleashTest, setupAppWithAuth } from '../../helpers/test-helper'; +import metricsExample from '../../../examples/client-metrics.json'; +import dbInit, { ITestDb } from '../../helpers/database-init'; +import getLogger from '../../../fixtures/no-logger'; +import { ApiTokenType } from '../../../../lib/types/models/api-token'; + +let app: IUnleashTest; +let db: ITestDb; + +let defaultToken; + +beforeAll(async () => { + db = await dbInit('metrics_two_api_client', getLogger); + app = await setupAppWithAuth(db.stores, { + experimental: { metricsV2: { enabled: true } }, + }); + defaultToken = await app.services.apiTokenService.createApiToken({ + type: ApiTokenType.CLIENT, + project: 'default', + environment: 'default', + username: 'tester', + }); +}); + +afterEach(async () => { + await db.stores.clientMetricsStoreV2.deleteAll(); +}); + +afterAll(async () => { + await app.destroy(); + await db.destroy(); +}); + +test('should be possible to send metrics', async () => { + return app.request + .post('/api/client/metrics') + .set('Authorization', defaultToken.secret) + .send(metricsExample) + .expect(202); +}); + +test('should require valid send metrics', async () => { + return app.request + .post('/api/client/metrics') + .set('Authorization', defaultToken.secret) + .send({ + appName: 'test', + }) + .expect(400); +}); + +test('should accept client metrics', async () => { + return app.request + .post('/api/client/metrics') + .set('Authorization', defaultToken.secret) + .send({ + appName: 'demo', + instanceId: '1', + bucket: { + start: Date.now(), + stop: Date.now(), + toggles: {}, + }, + }) + .expect(202); +}); + +test('should pick up environment from token', async () => { + const environment = 'test'; + await db.stores.environmentStore.create({ name: 'test', type: 'test' }); + const token = await app.services.apiTokenService.createApiToken({ + type: ApiTokenType.CLIENT, + project: 'default', + environment, + username: 'tester', + }); + + await app.request + .post('/api/client/metrics') + .set('Authorization', token.secret) + .send({ + appName: 'some-fancy-app', + instanceId: '1', + bucket: { + start: Date.now(), + stop: Date.now(), + toggles: { + test: { + yes: 100, + no: 50, + }, + }, + }, + }) + .expect(202); + + const metrics = await db.stores.clientMetricsStoreV2.getAll(); + expect(metrics[0].environment).toBe('test'); + expect(metrics[0].appName).toBe('some-fancy-app'); +}); diff --git a/src/test/e2e/helpers/test-helper.ts b/src/test/e2e/helpers/test-helper.ts index 5c4d8870a9..70f6d3f738 100644 --- a/src/test/e2e/helpers/test-helper.ts +++ b/src/test/e2e/helpers/test-helper.ts @@ -1,3 +1,4 @@ +/* eslint-disable @typescript-eslint/explicit-module-boundary-types */ import supertest from 'supertest'; import EventEmitter from 'events'; @@ -62,7 +63,6 @@ export async function setupApp(stores: IUnleashStores): Promise { export async function setupAppWithCustomConfig( stores: IUnleashStores, - // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types customOptions: any, ): Promise { return createApp(stores, undefined, undefined, customOptions); @@ -70,8 +70,9 @@ export async function setupAppWithCustomConfig( export async function setupAppWithAuth( stores: IUnleashStores, + customOptions?: any, ): Promise { - return createApp(stores, IAuthType.DEMO); + return createApp(stores, IAuthType.DEMO, undefined, customOptions); } export async function setupAppWithCustomAuth( diff --git a/src/test/e2e/stores/client-metrics-store-v2.e2e.test.ts b/src/test/e2e/stores/client-metrics-store-v2.e2e.test.ts index b1c618aa4a..82ea813a40 100644 --- a/src/test/e2e/stores/client-metrics-store-v2.e2e.test.ts +++ b/src/test/e2e/stores/client-metrics-store-v2.e2e.test.ts @@ -248,3 +248,17 @@ test('Should return seen applications using a feature toggle', async () => { expect(apps).toHaveLength(2); expect(apps).toStrictEqual(['backend-api', 'web']); }); + +test('Should not fail on empty list of metrics', async () => { + await clientMetricsStore.batchInsertMetrics([]); + const all = await clientMetricsStore.getAll(); + + expect(all).toHaveLength(0); +}); + +test('Should not fail on undefined list of metrics', async () => { + await clientMetricsStore.batchInsertMetrics(undefined); + const all = await clientMetricsStore.getAll(); + + expect(all).toHaveLength(0); +});