From a607dea2847265ea7b7385251ae08d70c8659b19 Mon Sep 17 00:00:00 2001 From: andreas-unleash <104830839+andreas-unleash@users.noreply.github.com> Date: Thu, 30 Jun 2022 13:27:12 +0300 Subject: [PATCH] OAS for client-api metrics.ts (#1753) * OAS for client-api metrics.ts * Fix PR comments * Fix PR comments * Fix test * Renamed and synced with proxy * Renamed and synced with proxy * Renamed and synced with proxy * add tests * Update python.md Revert doc * added 400 response, more tests * PR comment * PR comment --- src/lib/openapi/index.ts | 4 + .../client-metrics-schema.test.ts.snap | 18 ++++ .../spec/client-metrics-schema.test.ts | 63 +++++++++++ src/lib/openapi/spec/client-metrics-schema.ts | 61 +++++++++++ src/lib/openapi/spec/date-schema.ts | 9 ++ src/lib/openapi/util/standard-responses.ts | 5 + src/lib/routes/client-api/metrics.test.ts | 48 ++++++++- src/lib/routes/client-api/metrics.ts | 41 +++++-- .../__snapshots__/openapi.e2e.test.ts.snap | 102 ++++++++++++++++++ 9 files changed, 342 insertions(+), 9 deletions(-) create mode 100644 src/lib/openapi/spec/__snapshots__/client-metrics-schema.test.ts.snap create mode 100644 src/lib/openapi/spec/client-metrics-schema.test.ts create mode 100644 src/lib/openapi/spec/client-metrics-schema.ts create mode 100644 src/lib/openapi/spec/date-schema.ts diff --git a/src/lib/openapi/index.ts b/src/lib/openapi/index.ts index e39e2c80c8..3cf30adc43 100644 --- a/src/lib/openapi/index.ts +++ b/src/lib/openapi/index.ts @@ -89,6 +89,8 @@ import { eventSchema } from './spec/event-schema'; import { eventsSchema } from './spec/events-schema'; import { featureEventsSchema } from './spec/feature-events-schema'; import { clientApplicationSchema } from './spec/client-application-schema'; +import { clientMetricsSchema } from './spec/client-metrics-schema'; +import { dateSchema } from './spec/date-schema'; import { clientVariantSchema } from './spec/client-variant-schema'; import { IServerOption } from '../types'; import { URL } from 'url'; @@ -104,6 +106,7 @@ export const schemas = { applicationSchema, applicationsSchema, clientApplicationSchema, + clientMetricsSchema, cloneFeatureSchema, clientFeatureSchema, clientFeaturesSchema, @@ -117,6 +120,7 @@ export const schemas = { createFeatureSchema, createFeatureStrategySchema, createUserSchema, + dateSchema, emailSchema, environmentSchema, environmentsSchema, diff --git a/src/lib/openapi/spec/__snapshots__/client-metrics-schema.test.ts.snap b/src/lib/openapi/spec/__snapshots__/client-metrics-schema.test.ts.snap new file mode 100644 index 0000000000..c6c52e423f --- /dev/null +++ b/src/lib/openapi/spec/__snapshots__/client-metrics-schema.test.ts.snap @@ -0,0 +1,18 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`clientMetricsSchema should fail when required field is missing 1`] = ` +Object { + "errors": Array [ + Object { + "instancePath": "/bucket", + "keyword": "required", + "message": "must have required property 'stop'", + "params": Object { + "missingProperty": "stop", + }, + "schemaPath": "#/properties/bucket/required", + }, + ], + "schema": "#/components/schemas/clientMetricsSchema", +} +`; diff --git a/src/lib/openapi/spec/client-metrics-schema.test.ts b/src/lib/openapi/spec/client-metrics-schema.test.ts new file mode 100644 index 0000000000..05dd2f405d --- /dev/null +++ b/src/lib/openapi/spec/client-metrics-schema.test.ts @@ -0,0 +1,63 @@ +import { validateSchema } from '../validate'; +import { ClientMetricsSchema } from './client-metrics-schema'; + +test('clientMetricsSchema full', () => { + const data: ClientMetricsSchema = { + appName: 'a', + instanceId: 'some-id', + environment: 'some-env', + bucket: { + start: Date.now(), + stop: Date.now(), + toggles: { + someToggle: { + yes: 52, + no: 2, + variants: {}, + }, + }, + }, + }; + + expect( + validateSchema('#/components/schemas/clientMetricsSchema', data), + ).toBeUndefined(); +}); + +test('clientMetricsSchema should ignore additional properties without failing when required fields are there', () => { + expect( + validateSchema('#/components/schemas/clientMetricsSchema', { + appName: 'a', + someParam: 'some-value', + bucket: { + start: Date.now(), + stop: Date.now(), + toggles: { + someToggle: { + yes: 52, + variants: {}, + someOtherParam: 'some-other-value', + }, + }, + }, + }), + ).toBeUndefined(); +}); + +test('clientMetricsSchema should fail when required field is missing', () => { + expect( + validateSchema('#/components/schemas/clientMetricsSchema', { + appName: 'a', + bucket: { + start: Date.now(), + toggles: { + someToggle: { + yes: 52, + variants: {}, + someOtherParam: 'some-other-value', + }, + }, + }, + }), + ).toMatchSnapshot(); +}); diff --git a/src/lib/openapi/spec/client-metrics-schema.ts b/src/lib/openapi/spec/client-metrics-schema.ts new file mode 100644 index 0000000000..106e4b5437 --- /dev/null +++ b/src/lib/openapi/spec/client-metrics-schema.ts @@ -0,0 +1,61 @@ +import { FromSchema } from 'json-schema-to-ts'; +import { dateSchema } from './date-schema'; + +export const clientMetricsSchema = { + $id: '#/components/schemas/clientMetricsSchema', + type: 'object', + required: ['appName', 'bucket'], + properties: { + appName: { + type: 'string', + }, + instanceId: { + type: 'string', + }, + environment: { + type: 'string', + }, + bucket: { + type: 'object', + required: ['start', 'stop', 'toggles'], + properties: { + start: { + $ref: '#/components/schemas/dateSchema', + }, + stop: { + $ref: '#/components/schemas/dateSchema', + }, + toggles: { + type: 'object', + additionalProperties: { + type: 'object', + properties: { + yes: { + type: 'integer', + minimum: 0, + }, + no: { + type: 'integer', + minimum: 0, + }, + variants: { + type: 'object', + additionalProperties: { + type: 'integer', + minimum: 0, + }, + }, + }, + }, + }, + }, + }, + }, + components: { + schemas: { + dateSchema, + }, + }, +} as const; + +export type ClientMetricsSchema = FromSchema; diff --git a/src/lib/openapi/spec/date-schema.ts b/src/lib/openapi/spec/date-schema.ts new file mode 100644 index 0000000000..1c05696bd0 --- /dev/null +++ b/src/lib/openapi/spec/date-schema.ts @@ -0,0 +1,9 @@ +import { FromSchema } from 'json-schema-to-ts'; + +export const dateSchema = { + $id: '#/components/schemas/dateSchema', + oneOf: [{ type: 'string', format: 'date-time' }, { type: 'number' }], + components: {}, +} as const; + +export type DateSchema = FromSchema; diff --git a/src/lib/openapi/util/standard-responses.ts b/src/lib/openapi/util/standard-responses.ts index 620e08b56f..dca0ade3e1 100644 --- a/src/lib/openapi/util/standard-responses.ts +++ b/src/lib/openapi/util/standard-responses.ts @@ -3,7 +3,12 @@ export const unauthorizedResponse = { 'Authorization information is missing or invalid. Provide a valid API token as the `authorization` header, e.g. `authorization:*.*.my-admin-token`.', } as const; +export const badRequestResponse = { + description: 'The request data do not match what we expect.', +} as const; + const standardResponses = { + 400: badRequestResponse, 401: unauthorizedResponse, } as const; diff --git a/src/lib/routes/client-api/metrics.test.ts b/src/lib/routes/client-api/metrics.test.ts index d589e53719..98a7414803 100644 --- a/src/lib/routes/client-api/metrics.test.ts +++ b/src/lib/routes/client-api/metrics.test.ts @@ -4,8 +4,7 @@ import getApp from '../../app'; import { createTestConfig } from '../../../test/config/test-config'; import { clientMetricsSchema } from '../../services/client-metrics/schema'; import { createServices } from '../../services'; -import { IUnleashStores } from '../../types'; -import { IUnleashOptions } from '../../server-impl'; +import { IUnleashOptions, IUnleashStores } from '../../types'; async function getSetup(opts?: IUnleashOptions) { const stores = createStores(); @@ -209,3 +208,48 @@ test('should set lastSeen on toggle', async () => { expect(toggle.lastSeenAt).toBeTruthy(); }); + +test('should return a 400 when required fields are missing', async () => { + stores.featureToggleStore.create('default', { + name: 'toggleLastSeen', + }); + await request + .post('/api/client/metrics') + .send({ + appName: 'demo', + bucket: { + start: Date.now(), + toggles: { + toggleLastSeen: { + yes: 200, + no: 0, + }, + }, + }, + }) + .expect(400); +}); + +test('should return a 200 if required fields are there', async () => { + stores.featureToggleStore.create('default', { + name: 'toggleLastSeen', + }); + await request + .post('/api/client/metrics') + .send({ + appName: 'demo', + someParam: 'some-value', + somOtherParam: 'some--other-value', + bucket: { + start: Date.now(), + stop: Date.now(), + toggles: { + toggleLastSeen: { + yes: 200, + no: 0, + }, + }, + }, + }) + .expect(202); +}); diff --git a/src/lib/routes/client-api/metrics.ts b/src/lib/routes/client-api/metrics.ts index eecaea2fa2..4aacbeaea7 100644 --- a/src/lib/routes/client-api/metrics.ts +++ b/src/lib/routes/client-api/metrics.ts @@ -1,7 +1,6 @@ import { Response } from 'express'; import Controller from '../controller'; -import { IUnleashServices } from '../../types'; -import { IUnleashConfig } from '../../types/option'; +import { IUnleashConfig, IUnleashServices } from '../../types'; import ClientInstanceService from '../../services/client-metrics/instance-service'; import { Logger } from '../../logger'; import { IAuthRequest } from '../unleash-types'; @@ -11,21 +10,29 @@ import ClientMetricsServiceV2 from '../../services/client-metrics/metrics-servic import { User } from '../../server-impl'; import { IClientApp } from '../../types/model'; import { NONE } from '../../types/permissions'; +import { OpenApiService } from '../../services/openapi-service'; +import { createRequestSchema, createResponseSchema } from '../../openapi'; +import { getStandardResponses } from '../../openapi/util/standard-responses'; export default class ClientMetricsController extends Controller { logger: Logger; clientInstanceService: ClientInstanceService; + openApiService: OpenApiService; + metricsV2: ClientMetricsServiceV2; constructor( { clientInstanceService, clientMetricsServiceV2, + openApiService, }: Pick< IUnleashServices, - 'clientInstanceService' | 'clientMetricsServiceV2' + | 'clientInstanceService' + | 'clientMetricsServiceV2' + | 'openApiService' >, config: IUnleashConfig, ) { @@ -34,9 +41,26 @@ export default class ClientMetricsController extends Controller { this.logger = getLogger('/api/client/metrics'); this.clientInstanceService = clientInstanceService; + this.openApiService = openApiService; this.metricsV2 = clientMetricsServiceV2; - this.post('/', this.registerMetrics, NONE); + this.route({ + method: 'post', + path: '', + handler: this.registerMetrics, + permission: NONE, + middleware: [ + openApiService.validPath({ + tags: ['client'], + operationId: 'registerClientMetrics', + requestBody: createRequestSchema('clientMetricsSchema'), + responses: { + ...getStandardResponses(400), + 202: createResponseSchema('emptyResponse'), + }, + }), + ], + }); } private resolveEnvironment(user: User, data: IClientApp) { @@ -55,8 +79,11 @@ export default class ClientMetricsController extends Controller { data.environment = this.resolveEnvironment(user, data); await this.clientInstanceService.registerInstance(data, clientIp); - await this.metricsV2.registerClientMetrics(data, clientIp); - - return res.status(202).end(); + try { + await this.metricsV2.registerClientMetrics(data, clientIp); + return res.status(202).end(); + } catch (e) { + return res.status(400).end(); + } } } diff --git a/src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap b/src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap index 859f6a30fa..3b6fbfcfd7 100644 --- a/src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap +++ b/src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap @@ -466,6 +466,63 @@ Object { ], "type": "object", }, + "clientMetricsSchema": Object { + "properties": Object { + "appName": Object { + "type": "string", + }, + "bucket": Object { + "properties": Object { + "start": Object { + "$ref": "#/components/schemas/dateSchema", + }, + "stop": Object { + "$ref": "#/components/schemas/dateSchema", + }, + "toggles": Object { + "additionalProperties": Object { + "properties": Object { + "no": Object { + "minimum": 0, + "type": "integer", + }, + "variants": Object { + "additionalProperties": Object { + "minimum": 0, + "type": "integer", + }, + "type": "object", + }, + "yes": Object { + "minimum": 0, + "type": "integer", + }, + }, + "type": "object", + }, + "type": "object", + }, + }, + "required": Array [ + "start", + "stop", + "toggles", + ], + "type": "object", + }, + "environment": Object { + "type": "string", + }, + "instanceId": Object { + "type": "string", + }, + }, + "required": Array [ + "appName", + "bucket", + ], + "type": "object", + }, "clientVariantSchema": Object { "additionalProperties": false, "properties": Object { @@ -703,6 +760,17 @@ Object { ], "type": "object", }, + "dateSchema": Object { + "oneOf": Array [ + Object { + "format": "date-time", + "type": "string", + }, + Object { + "type": "number", + }, + ], + }, "emailSchema": Object { "additionalProperties": false, "properties": Object { @@ -5464,6 +5532,40 @@ If the provided project does not exist, the list of events will be empty.", ], }, }, + "/api/client/metrics": Object { + "post": Object { + "operationId": "registerClientMetrics", + "requestBody": Object { + "content": Object { + "application/json": Object { + "schema": Object { + "$ref": "#/components/schemas/clientMetricsSchema", + }, + }, + }, + "description": "clientMetricsSchema", + "required": true, + }, + "responses": Object { + "202": Object { + "content": Object { + "application/json": Object { + "schema": Object { + "$ref": "#/components/schemas/emptyResponse", + }, + }, + }, + "description": "emptyResponse", + }, + "400": Object { + "description": "The request data do not match what we expect.", + }, + }, + "tags": Array [ + "client", + ], + }, + }, "/api/client/register": Object { "post": Object { "operationId": "registerClientApplication",