From 97c2b3c089c29c79f97b30a2417333efcd4f631c Mon Sep 17 00:00:00 2001 From: Thomas Heartman Date: Fri, 23 Sep 2022 15:02:09 +0200 Subject: [PATCH] openapi: improve validation testing (#2058) ## What This PR adds an extra layer of OpenAPI validation testing to what we already have. It also fixes any issues that make the new tests fail. ## Why While the current OpenAPI validation takes care of _some_ things, there's also things it misses, as shown by #2055. By adding the OpenAPI Enforcer package, we should hopefully be able to catch more of these errors in the future. The enforcer _does_ flag the issue in #2055 as an error. ## How By adding the OpenAPI Enforcer package and making whatever changes it picks up on. By adding location headers to all our 201 endpoints. I also had to change some signatures on `create` store methods so that they actually return something (a lot of them just returned `void`). ## Discussion points ### Code changes Some of the code changes may not be necessary or we may want to change more code to align with what changes have been done. It may be worth standardizing on a pattern for `*store.create` methods, so that they always return an identifier or the stored object, for instance. ### On relative URIs The 201 location headers use relative URIs to point to the created resources. This seemed to be the easiest way to me, as we don't need to worry about figuring out what the absolute URL of the instance is (though we could probably just concat it to the request URI?). The algorithm for determining relative URIs is described in [RFC 3986 section 5](https://www.rfc-editor.org/rfc/rfc3986#section-5). There's also some places where I'm not sure we _can_ provide accurate location url. I think they're supposed to point _directly at_ whatever the resource is, but for some resources (such as api tokens), you can only get it as part of a collection. From [RFC 9110 on the location field](https://httpwg.org/specs/rfc9110.html#field.location) (emphasis mine): > the Location header field in a [201 (Created)](https://httpwg.org/specs/rfc9110.html#status.201) response is supposed to provide a URI that is **specific** to the created resource. A link to a collection is not specific. I'm not sure what best to do about this. ### Inline comments I've added a number of inline PR comments that I'd love to get some feedback on too. Have a look and let me know what you think! ### Unfinished business I've added some juicy comments to some of the files here. They contain non-blocking issues that I'm tracking (via github issues). We should resolve them in the future, but right now they can stay as they are. ## Commits * Feat: add openapi-enforcer + tests; fix _some_ issues * Test: allow non-standard string formats * validation: fix _some_ 201 created location header endpoints * #1391: fix remaining 201 location headers missing * Refactor: use the ajv options object instead of add* methods * #1391: flag validation errors and warnings as test failures * #1391: modify patch schema to specify either object or array We don't provide many patch endpoints, so we _could_ create separate patch operation objects for each one. I think that makes sense to do as part of the larger cleanup. For now, I think it's worth to simply turn it into one of these. While it's not entirely accurate, it's better than what we had before. * Refactor: make tests easier to read * #1391: use enum for valid token types This was previously only a description. This may seem like a breaking change because OpenAPI would previously accept any string. However, Joi also performs validation on this, so invalid values wouldn't work previously either. * #1391: Comment out default parameter values for now This isn't the _right_ way, but it's the pragmatic solution. It's not a big deal and this works as a stopgap solution. * #1391: add todo note for api token schema fixes * #1391: update snapshot * Revert "#1391: modify patch schema to specify either object or array" This reverts commit 0dd5d0faa1554a394178c0f632915e096204cac7. Turns out we need to allow much more than just objects and arrays. I'll leave this as is for now. * Chore(#1391): update comment explaining api token schema TODO * #1391: modify some test code and add comment * #1391: update tests and spec to allow 'any' type in schema * Chore: remove comment * #1391: add tests for context field stores * #1391: add location header for public signup links * #1391: fix query parameter description. There was indeed a bug in the package, and this has been addressed now, so we can go back to describing the params properly. --- package.json | 1 + src/lib/db/context-field-store.ts | 18 ++- src/lib/openapi/spec/api-token-schema.ts | 2 +- .../openapi/spec/create-api-token-schema.ts | 18 ++- src/lib/openapi/spec/set-ui-config-schema.ts | 1 - .../openapi/util/create-response-schema.ts | 24 ++++ src/lib/openapi/validate.ts | 15 +-- src/lib/routes/admin-api/api-token.ts | 8 +- src/lib/routes/admin-api/context.ts | 21 +++- src/lib/routes/admin-api/feature.ts | 11 +- src/lib/routes/admin-api/public-signup.ts | 10 +- src/lib/routes/admin-api/strategy.ts | 21 +++- src/lib/routes/admin-api/tag-type.ts | 13 +- src/lib/routes/admin-api/tag.ts | 18 ++- src/lib/services/context-service.ts | 6 +- src/lib/services/openapi-service.ts | 5 + src/lib/services/strategy-service.ts | 3 +- src/lib/services/tag-service.ts | 4 +- .../__snapshots__/openapi.e2e.test.ts.snap | 111 ++++++++++++++++-- src/test/e2e/api/openapi/openapi.e2e.test.ts | 30 ++++- .../stores/context-field-store.e2e.test.ts | 96 +++++++++++++++ yarn.lock | 26 ++++ 22 files changed, 409 insertions(+), 53 deletions(-) create mode 100644 src/test/e2e/stores/context-field-store.e2e.test.ts diff --git a/package.json b/package.json index eaa7934959..9cd43720a3 100644 --- a/package.json +++ b/package.json @@ -174,6 +174,7 @@ "jest": "29.0.1", "lint-staged": "13.0.3", "nock": "13.2.9", + "openapi-enforcer": "1.21.1", "prettier": "2.7.1", "proxyquire": "2.1.3", "source-map-support": "0.5.21", diff --git a/src/lib/db/context-field-store.ts b/src/lib/db/context-field-store.ts index 46be952224..904a4f5ec6 100644 --- a/src/lib/db/context-field-store.ts +++ b/src/lib/db/context-field-store.ts @@ -4,6 +4,7 @@ import { IContextField, IContextFieldDto, IContextFieldStore, + ILegalValue, } from '../types/stores/context-field-store'; const COLUMNS = [ @@ -16,7 +17,16 @@ const COLUMNS = [ ]; const TABLE = 'context_fields'; -const mapRow: (object) => IContextField = (row) => ({ +type ContextFieldDB = { + name: string; + description: string; + stickiness: boolean; + sort_order: number; + legal_values: ILegalValue[]; + created_at: Date; +}; + +const mapRow = (row: ContextFieldDB): IContextField => ({ name: row.name, description: row.description, stickiness: row.stickiness, @@ -88,15 +98,17 @@ class ContextFieldStore implements IContextFieldStore { return present; } + // TODO: write tests for the changes you made here? async create(contextField: IContextFieldDto): Promise { - const row = await this.db(TABLE) + const [row] = await this.db(TABLE) .insert(this.fieldToRow(contextField)) .returning('*'); + return mapRow(row); } async update(data: IContextFieldDto): Promise { - const row = await this.db(TABLE) + const [row] = await this.db(TABLE) .where({ name: data.name }) .update(this.fieldToRow(data)) .returning('*'); diff --git a/src/lib/openapi/spec/api-token-schema.ts b/src/lib/openapi/spec/api-token-schema.ts index 579b3b6c22..8be4a7bcfb 100644 --- a/src/lib/openapi/spec/api-token-schema.ts +++ b/src/lib/openapi/spec/api-token-schema.ts @@ -15,7 +15,7 @@ export const apiTokenSchema = { }, type: { type: 'string', - description: `${Object.values(ApiTokenType).join(', ')}.`, + enum: Object.values(ApiTokenType), }, environment: { type: 'string', diff --git a/src/lib/openapi/spec/create-api-token-schema.ts b/src/lib/openapi/spec/create-api-token-schema.ts index 04d5a3fc20..30fbfc0771 100644 --- a/src/lib/openapi/spec/create-api-token-schema.ts +++ b/src/lib/openapi/spec/create-api-token-schema.ts @@ -1,6 +1,22 @@ import { FromSchema } from 'json-schema-to-ts'; import { ApiTokenType } from '../../types/models/api-token'; +// TODO: (openapi) this schema isn't entirely correct: `project` and `projects` +// are mutually exclusive. +// +// That is, when creating a token, you can provide either `project` _or_ +// `projects`, but *not* both. +// +// We should be able to annotate this using `oneOf` and `allOf`, but making +// `oneOf` only valid for _either_ `project` _or_ `projects` is tricky. +// +// I've opened an issue to get some help (thought it was a bug initially). +// There's more info available at: +// +// https://github.com/ajv-validator/ajv/issues/2096 +// +// This also applies to apiTokenSchema and potentially other related schemas. + export const createApiTokenSchema = { $id: '#/components/schemas/createApiTokenSchema', type: 'object', @@ -14,7 +30,7 @@ export const createApiTokenSchema = { }, type: { type: 'string', - description: `${Object.values(ApiTokenType).join(', ')}.`, + enum: Object.values(ApiTokenType), }, environment: { type: 'string', diff --git a/src/lib/openapi/spec/set-ui-config-schema.ts b/src/lib/openapi/spec/set-ui-config-schema.ts index 27f7153a2d..c566b46f80 100644 --- a/src/lib/openapi/spec/set-ui-config-schema.ts +++ b/src/lib/openapi/spec/set-ui-config-schema.ts @@ -12,7 +12,6 @@ export const setUiConfigSchema = { properties: { frontendApiOrigins: { type: 'array', - additionalProperties: false, items: { type: 'string' }, }, }, diff --git a/src/lib/openapi/util/create-response-schema.ts b/src/lib/openapi/util/create-response-schema.ts index f8886fbacb..d2d58f6341 100644 --- a/src/lib/openapi/util/create-response-schema.ts +++ b/src/lib/openapi/util/create-response-schema.ts @@ -14,3 +14,27 @@ export const createResponseSchema = ( }, }; }; + +export const resourceCreatedResponseSchema = ( + schemaName: string, +): OpenAPIV3.ResponseObject => { + return { + headers: { + location: { + description: 'The location of the newly created resource.', + schema: { + type: 'string', + format: 'uri', + }, + }, + }, + description: `The resource was successfully created.`, + content: { + 'application/json': { + schema: { + $ref: `#/components/schemas/${schemaName}`, + }, + }, + }, + }; +}; diff --git a/src/lib/openapi/validate.ts b/src/lib/openapi/validate.ts index 7d2e59ba78..48a1a59e22 100644 --- a/src/lib/openapi/validate.ts +++ b/src/lib/openapi/validate.ts @@ -1,5 +1,4 @@ import Ajv, { ErrorObject } from 'ajv'; -import addFormats from 'ajv-formats'; import { SchemaId, schemas } from './index'; import { omitKeys } from '../util/omit-keys'; @@ -12,14 +11,16 @@ const ajv = new Ajv({ schemas: Object.values(schemas).map((schema) => omitKeys(schema, 'components'), ), + + // example was superseded by examples in openapi 3.1, but we're still on 3.0, so + // let's add it back in! + keywords: ['example'], + formats: { + 'date-time': true, + uri: true, + }, }); -addFormats(ajv, ['date-time']); - -// example was superseded by examples in openapi 3.1, but we're still on 3.0, so -// let's add it back in! -ajv.addKeyword('example'); - export const validateSchema = ( schema: SchemaId, data: unknown, diff --git a/src/lib/routes/admin-api/api-token.ts b/src/lib/routes/admin-api/api-token.ts index aeba07c675..799019ddd8 100644 --- a/src/lib/routes/admin-api/api-token.ts +++ b/src/lib/routes/admin-api/api-token.ts @@ -19,7 +19,10 @@ import { createApiToken } from '../../schema/api-token-schema'; import { OpenApiService } from '../../services/openapi-service'; import { IUnleashServices } from '../../types'; import { createRequestSchema } from '../../openapi/util/create-request-schema'; -import { createResponseSchema } from '../../openapi/util/create-response-schema'; +import { + createResponseSchema, + resourceCreatedResponseSchema, +} from '../../openapi/util/create-response-schema'; import { apiTokensSchema, ApiTokensSchema, @@ -96,7 +99,7 @@ export class ApiTokenController extends Controller { operationId: 'createApiToken', requestBody: createRequestSchema('createApiTokenSchema'), responses: { - 201: createResponseSchema('apiTokenSchema'), + 201: resourceCreatedResponseSchema('apiTokenSchema'), }, }), ], @@ -162,6 +165,7 @@ export class ApiTokenController extends Controller { res, apiTokenSchema.$id, serializeDates(token), + { location: `api-tokens` }, ); } diff --git a/src/lib/routes/admin-api/context.ts b/src/lib/routes/admin-api/context.ts index 139fe4945a..04dab48951 100644 --- a/src/lib/routes/admin-api/context.ts +++ b/src/lib/routes/admin-api/context.ts @@ -24,7 +24,10 @@ import { import { ContextFieldsSchema } from '../../openapi/spec/context-fields-schema'; import { UpsertContextFieldSchema } from '../../openapi/spec/upsert-context-field-schema'; import { createRequestSchema } from '../../openapi/util/create-request-schema'; -import { createResponseSchema } from '../../openapi/util/create-response-schema'; +import { + createResponseSchema, + resourceCreatedResponseSchema, +} from '../../openapi/util/create-response-schema'; import { serializeDates } from '../../types/serialize-dates'; import NotFoundError from '../../error/notfound-error'; import { NameSchema } from '../../openapi/spec/name-schema'; @@ -98,7 +101,9 @@ export class ContextController extends Controller { 'upsertContextFieldSchema', ), responses: { - 201: emptyResponse, + 201: resourceCreatedResponseSchema( + 'contextFieldSchema', + ), }, }), ], @@ -189,13 +194,19 @@ export class ContextController extends Controller { async createContextField( req: IAuthRequest, - res: Response, + res: Response, ): Promise { const value = req.body; const userName = extractUsername(req); - await this.contextService.createContextField(value, userName); - res.status(201).end(); + const result = await this.contextService.createContextField( + value, + userName, + ); + res.status(201) + .header('location', `context/${result.name}`) + .json(serializeDates(result)) + .end(); } async updateContextField( diff --git a/src/lib/routes/admin-api/feature.ts b/src/lib/routes/admin-api/feature.ts index 9f98537cc8..d88f98d80d 100644 --- a/src/lib/routes/admin-api/feature.ts +++ b/src/lib/routes/admin-api/feature.ts @@ -25,7 +25,10 @@ import { TagsSchema } from '../../openapi/spec/tags-schema'; import { serializeDates } from '../../types/serialize-dates'; import { OpenApiService } from '../../services/openapi-service'; import { createRequestSchema } from '../../openapi/util/create-request-schema'; -import { createResponseSchema } from '../../openapi/util/create-response-schema'; +import { + createResponseSchema, + resourceCreatedResponseSchema, +} from '../../openapi/util/create-response-schema'; import { emptyResponse } from '../../openapi/util/standard-responses'; const version = 1; @@ -123,7 +126,9 @@ class FeatureController extends Controller { tags: ['Features'], operationId: 'addTag', requestBody: createRequestSchema('tagSchema'), - responses: { 201: createResponseSchema('tagSchema') }, + responses: { + 201: resourceCreatedResponseSchema('tagSchema'), + }, }), ], }); @@ -221,7 +226,7 @@ class FeatureController extends Controller { req.body, userName, ); - res.status(201).json(tag); + res.status(201).header('location', `${featureName}/tags`).json(tag); } // TODO diff --git a/src/lib/routes/admin-api/public-signup.ts b/src/lib/routes/admin-api/public-signup.ts index 7c767e523a..2e2e36a315 100644 --- a/src/lib/routes/admin-api/public-signup.ts +++ b/src/lib/routes/admin-api/public-signup.ts @@ -8,7 +8,10 @@ import { IAuthRequest } from '../unleash-types'; import { IUnleashConfig, IUnleashServices } from '../../types'; import { OpenApiService } from '../../services/openapi-service'; import { createRequestSchema } from '../../openapi/util/create-request-schema'; -import { createResponseSchema } from '../../openapi/util/create-response-schema'; +import { + createResponseSchema, + resourceCreatedResponseSchema, +} from '../../openapi/util/create-response-schema'; import { serializeDates } from '../../types/serialize-dates'; import { emptyResponse } from '../../openapi/util/standard-responses'; import { PublicSignupTokenService } from '../../services/public-signup-token-service'; @@ -92,7 +95,9 @@ export class PublicSignupController extends Controller { 'publicSignupTokenCreateSchema', ), responses: { - 201: createResponseSchema('publicSignupTokenSchema'), + 201: resourceCreatedResponseSchema( + 'publicSignupTokenSchema', + ), }, }), ], @@ -255,6 +260,7 @@ export class PublicSignupController extends Controller { res, publicSignupTokensSchema.$id, serializeDates(token), + { location: `tokens/${token.secret}` }, ); } diff --git a/src/lib/routes/admin-api/strategy.ts b/src/lib/routes/admin-api/strategy.ts index d29d6005d0..07e08c2629 100644 --- a/src/lib/routes/admin-api/strategy.ts +++ b/src/lib/routes/admin-api/strategy.ts @@ -15,7 +15,10 @@ import { IAuthRequest } from '../unleash-types'; import { OpenApiService } from '../../services/openapi-service'; import { emptyResponse } from '../../openapi/util/standard-responses'; import { createRequestSchema } from '../../openapi/util/create-request-schema'; -import { createResponseSchema } from '../../openapi/util/create-response-schema'; +import { + createResponseSchema, + resourceCreatedResponseSchema, +} from '../../openapi/util/create-response-schema'; import { strategySchema, StrategySchema, @@ -102,7 +105,9 @@ class StrategyController extends Controller { tags: ['Strategies'], operationId: 'createStrategy', requestBody: createRequestSchema('upsertStrategySchema'), - responses: { 201: emptyResponse }, + responses: { + 201: resourceCreatedResponseSchema('strategySchema'), + }, }), ], }); @@ -193,12 +198,18 @@ class StrategyController extends Controller { async createStrategy( req: IAuthRequest, - res: Response, + res: Response, ): Promise { const userName = extractUsername(req); - await this.strategyService.createStrategy(req.body, userName); - res.status(201).end(); + const strategy = await this.strategyService.createStrategy( + req.body, + userName, + ); + res.header('location', `strategies/${strategy.name}`) + .status(201) + .json(strategy) + .end(); } async updateStrategy( diff --git a/src/lib/routes/admin-api/tag-type.ts b/src/lib/routes/admin-api/tag-type.ts index 3880f11eb7..5dc0bfc3c9 100644 --- a/src/lib/routes/admin-api/tag-type.ts +++ b/src/lib/routes/admin-api/tag-type.ts @@ -13,7 +13,10 @@ import TagTypeService from '../../services/tag-type-service'; import { Logger } from '../../logger'; import { IAuthRequest } from '../unleash-types'; import { createRequestSchema } from '../../openapi/util/create-request-schema'; -import { createResponseSchema } from '../../openapi/util/create-response-schema'; +import { + createResponseSchema, + resourceCreatedResponseSchema, +} from '../../openapi/util/create-response-schema'; import { TagTypesSchema } from '../../openapi/spec/tag-types-schema'; import { ValidateTagTypeSchema } from '../../openapi/spec/validate-tag-type-schema'; import { @@ -66,7 +69,9 @@ class TagTypeController extends Controller { openApiService.validPath({ tags: ['Tags'], operationId: 'createTagType', - responses: { 201: createResponseSchema('tagTypeSchema') }, + responses: { + 201: resourceCreatedResponseSchema('tagTypeSchema'), + }, requestBody: createRequestSchema('tagTypeSchema'), }), ], @@ -164,7 +169,9 @@ class TagTypeController extends Controller { req.body, userName, ); - res.status(201).json(tagType); + res.status(201) + .header('location', `tag-types/${tagType.name}`) + .json(tagType); } async updateTagType( diff --git a/src/lib/routes/admin-api/tag.ts b/src/lib/routes/admin-api/tag.ts index e859d0df90..f15ffcc6e7 100644 --- a/src/lib/routes/admin-api/tag.ts +++ b/src/lib/routes/admin-api/tag.ts @@ -10,7 +10,10 @@ import { NONE, UPDATE_FEATURE } from '../../types/permissions'; import { extractUsername } from '../../util/extract-user'; import { IAuthRequest } from '../unleash-types'; import { createRequestSchema } from '../../openapi/util/create-request-schema'; -import { createResponseSchema } from '../../openapi/util/create-response-schema'; +import { + createResponseSchema, + resourceCreatedResponseSchema, +} from '../../openapi/util/create-response-schema'; import { tagsSchema, TagsSchema } from '../../openapi/spec/tags-schema'; import { TagSchema } from '../../openapi/spec/tag-schema'; import { OpenApiService } from '../../services/openapi-service'; @@ -64,7 +67,9 @@ class TagController extends Controller { tags: ['Tags'], operationId: 'createTag', responses: { - 201: emptyResponse, + 201: resourceCreatedResponseSchema( + 'tagWithVersionSchema', + ), }, requestBody: createRequestSchema('tagSchema'), }), @@ -157,11 +162,14 @@ class TagController extends Controller { async createTag( req: IAuthRequest, - res: Response, + res: Response, ): Promise { const userName = extractUsername(req); - await this.tagService.createTag(req.body, userName); - res.status(201).end(); + const tag = await this.tagService.createTag(req.body, userName); + res.status(201) + .header('location', `tags/${tag.type}/${tag.value}`) + .json({ version, tag }) + .end(); } async deleteTag( diff --git a/src/lib/services/context-service.ts b/src/lib/services/context-service.ts index 305310453e..584613c54c 100644 --- a/src/lib/services/context-service.ts +++ b/src/lib/services/context-service.ts @@ -55,18 +55,20 @@ class ContextService { async createContextField( value: IContextFieldDto, userName: string, - ): Promise { + ): Promise { // validations await this.validateUniqueName(value); const contextField = await contextSchema.validateAsync(value); // creations - await this.contextFieldStore.create(value); + const createdField = await this.contextFieldStore.create(value); await this.eventStore.store({ type: CONTEXT_FIELD_CREATED, createdBy: userName, data: contextField, }); + + return createdField; } async updateContextField( diff --git a/src/lib/services/openapi-service.ts b/src/lib/services/openapi-service.ts index f8f3104eed..58a4d48e6c 100644 --- a/src/lib/services/openapi-service.ts +++ b/src/lib/services/openapi-service.ts @@ -69,6 +69,7 @@ export class OpenApiService { res: Response, schema: SchemaId, data: T, + headers: { [header: string]: string } = {}, ): void { const errors = validateSchema(schema, data); @@ -76,6 +77,10 @@ export class OpenApiService { this.logger.debug('Invalid response:', errors); } + Object.entries(headers).forEach(([header, value]) => + res.header(header, value), + ); + res.status(status).json(data); } } diff --git a/src/lib/services/strategy-service.ts b/src/lib/services/strategy-service.ts index 26343aff42..a6c4dcd343 100644 --- a/src/lib/services/strategy-service.ts +++ b/src/lib/services/strategy-service.ts @@ -101,7 +101,7 @@ class StrategyService { async createStrategy( value: IMinimalStrategy, userName: string, - ): Promise { + ): Promise { const strategy = await strategySchema.validateAsync(value); strategy.deprecated = false; await this._validateStrategyName(strategy); @@ -111,6 +111,7 @@ class StrategyService { createdBy: userName, data: strategy, }); + return this.strategyStore.get(strategy.name); } async updateStrategy( diff --git a/src/lib/services/tag-service.ts b/src/lib/services/tag-service.ts index 8bed11b3f8..6f7c62050e 100644 --- a/src/lib/services/tag-service.ts +++ b/src/lib/services/tag-service.ts @@ -52,7 +52,7 @@ export default class TagService { return data; } - async createTag(tag: ITag, userName: string): Promise { + async createTag(tag: ITag, userName: string): Promise { const data = await this.validate(tag); await this.tagStore.createTag(data); await this.eventStore.store({ @@ -60,6 +60,8 @@ export default class TagService { createdBy: userName, data, }); + + return data; } async deleteTag(tag: ITag, userName: string): Promise { 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 7d5ae3830d..d3af0de552 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 @@ -227,7 +227,11 @@ exports[`should serve the OpenAPI spec 1`] = ` "type": "string", }, "type": { - "description": "client, admin, frontend.", + "enum": [ + "client", + "admin", + "frontend", + ], "type": "string", }, "username": { @@ -664,7 +668,11 @@ exports[`should serve the OpenAPI spec 1`] = ` "type": "string", }, "type": { - "description": "client, admin, frontend.", + "enum": [ + "client", + "admin", + "frontend", + ], "type": "string", }, "username": { @@ -2651,7 +2659,6 @@ exports[`should serve the OpenAPI spec 1`] = ` "additionalProperties": false, "properties": { "frontendApiOrigins": { - "additionalProperties": false, "items": { "type": "string", }, @@ -3620,7 +3627,16 @@ exports[`should serve the OpenAPI spec 1`] = ` }, }, }, - "description": "apiTokenSchema", + "description": "The resource was successfully created.", + "headers": { + "location": { + "description": "The location of the newly created resource.", + "schema": { + "format": "uri", + "type": "string", + }, + }, + }, }, }, "tags": [ @@ -3902,7 +3918,23 @@ exports[`should serve the OpenAPI spec 1`] = ` }, "responses": { "201": { - "description": "This response has no body.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/contextFieldSchema", + }, + }, + }, + "description": "The resource was successfully created.", + "headers": { + "location": { + "description": "The location of the newly created resource.", + "schema": { + "format": "uri", + "type": "string", + }, + }, + }, }, }, "tags": [ @@ -4348,7 +4380,16 @@ If the provided project does not exist, the list of events will be empty.", }, }, }, - "description": "tagSchema", + "description": "The resource was successfully created.", + "headers": { + "location": { + "description": "The location of the newly created resource.", + "schema": { + "format": "uri", + "type": "string", + }, + }, + }, }, }, "tags": [ @@ -4508,7 +4549,16 @@ If the provided project does not exist, the list of events will be empty.", }, }, }, - "description": "publicSignupTokenSchema", + "description": "The resource was successfully created.", + "headers": { + "location": { + "description": "The location of the newly created resource.", + "schema": { + "format": "uri", + "type": "string", + }, + }, + }, }, }, "tags": [ @@ -6121,7 +6171,23 @@ If the provided project does not exist, the list of events will be empty.", }, "responses": { "201": { - "description": "This response has no body.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/strategySchema", + }, + }, + }, + "description": "The resource was successfully created.", + "headers": { + "location": { + "description": "The location of the newly created resource.", + "schema": { + "format": "uri", + "type": "string", + }, + }, + }, }, }, "tags": [ @@ -6301,7 +6367,16 @@ If the provided project does not exist, the list of events will be empty.", }, }, }, - "description": "tagTypeSchema", + "description": "The resource was successfully created.", + "headers": { + "location": { + "description": "The location of the newly created resource.", + "schema": { + "format": "uri", + "type": "string", + }, + }, + }, }, }, "tags": [ @@ -6457,7 +6532,23 @@ If the provided project does not exist, the list of events will be empty.", }, "responses": { "201": { - "description": "This response has no body.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/tagWithVersionSchema", + }, + }, + }, + "description": "The resource was successfully created.", + "headers": { + "location": { + "description": "The location of the newly created resource.", + "schema": { + "format": "uri", + "type": "string", + }, + }, + }, }, }, "tags": [ diff --git a/src/test/e2e/api/openapi/openapi.e2e.test.ts b/src/test/e2e/api/openapi/openapi.e2e.test.ts index 39400f6b5a..9f4ee011a1 100644 --- a/src/test/e2e/api/openapi/openapi.e2e.test.ts +++ b/src/test/e2e/api/openapi/openapi.e2e.test.ts @@ -2,6 +2,7 @@ import { setupApp } from '../../helpers/test-helper'; import dbInit from '../../helpers/database-init'; import getLogger from '../../../fixtures/no-logger'; import SwaggerParser from '@apidevtools/swagger-parser'; +import enforcer from 'openapi-enforcer'; import semver from 'semver'; let app; @@ -69,6 +70,33 @@ test('the generated OpenAPI spec is valid', async () => { await SwaggerParser.validate(body); } catch (err) { console.error(err); - return false; + // there's an error here, so let's exit after showing it in the console. + expect(true).toBe(false); } + + const [, enforcerError, enforcerWarning] = await enforcer(body, { + fullResult: true, + componentOptions: { + exceptionSkipCodes: [ + // allow non-standard formats for strings (including 'uri') + 'WSCH001', + + // Schemas with an indeterminable type cannot serialize, + // deserialize, or validate values. [WSCH005] + // + // This allows specifying the 'any' type for schemas (such as the + // patchSchema) + 'WSCH005', + ], + }, + }); + + if (enforcerWarning !== undefined) { + console.warn(enforcerWarning); + } + if (enforcerError !== undefined) { + console.error(enforcerError); + } + + expect(enforcerWarning ?? enforcerError).toBe(undefined); }); diff --git a/src/test/e2e/stores/context-field-store.e2e.test.ts b/src/test/e2e/stores/context-field-store.e2e.test.ts new file mode 100644 index 0000000000..66d4bee466 --- /dev/null +++ b/src/test/e2e/stores/context-field-store.e2e.test.ts @@ -0,0 +1,96 @@ +import dbInit, { ITestDb } from '../helpers/database-init'; +import getLogger from '../../fixtures/no-logger'; +import { IContextFieldDto } from 'lib/types/stores/context-field-store'; +import fc, { Arbitrary } from 'fast-check'; +import { IUnleashStores } from 'lib/types'; + +let stores: IUnleashStores; +let db: ITestDb; + +beforeAll(async () => { + db = await dbInit('context_store_serial', getLogger); + stores = db.stores; +}); + +afterAll(async () => { + await db.destroy(); +}); + +const cleanup = async () => { + await stores.contextFieldStore.deleteAll(); +}; + +const contextFieldDto = (): Arbitrary => + fc.record( + { + name: fc.uuid(), + sortOrder: fc.integer(), + stickiness: fc.boolean(), + description: fc.lorem({ mode: 'sentences' }), + legalValues: fc.array( + fc.record( + { + value: fc.lorem({ maxCount: 1 }), + description: fc.lorem({ mode: 'sentences' }), + }, + { requiredKeys: ['value'] }, + ), + ), + }, + { requiredKeys: ['name'] }, + ); + +test('creating an arbitrary context field should return the created context field', async () => { + await fc.assert( + fc + .asyncProperty(contextFieldDto(), async (input) => { + const { createdAt, ...storedData } = + await stores.contextFieldStore.create(input); + + Object.entries(input).forEach(([key, value]) => { + expect(storedData[key]).toStrictEqual(value); + }); + }) + .afterEach(cleanup), + ); +}); + +test('updating a context field should update the specified fields and leave everything else untouched', async () => { + await fc.assert( + fc + .asyncProperty( + contextFieldDto(), + contextFieldDto(), + async (original, { name, ...updateData }) => { + await stores.contextFieldStore.create(original); + + const { createdAt, ...updatedData } = + await stores.contextFieldStore.update({ + name: original.name, + ...updateData, + }); + + const allKeys = [ + 'sortOrder', + 'stickiness', + 'description', + 'legalValues', + ]; + const updateKeys = Object.keys(updateData); + + const unchangedKeys = allKeys.filter( + (k) => !updateKeys.includes(k), + ); + + Object.entries(updateData).forEach(([key, value]) => { + expect(updatedData[key]).toStrictEqual(value); + }); + + for (const key in unchangedKeys) { + expect(updatedData[key]).toStrictEqual(original[key]); + } + }, + ) + .afterEach(cleanup), + ); +}); diff --git a/yarn.lock b/yarn.lock index 387ac78b81..5a93f4ee11 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2752,6 +2752,11 @@ dotenv@^5.0.1: resolved "https://registry.npmjs.org/dotenv/-/dotenv-5.0.1.tgz" integrity sha512-4As8uPrjfwb7VXC+WnLCbXK7y+Ueb2B3zgNCePYfhxS1PYeaO1YTeplffTEcbfLhvFNGLAz90VvJs9yomG7bow== +drange@^1.0.2: + version "1.1.1" + resolved "https://registry.yarnpkg.com/drange/-/drange-1.1.1.tgz#b2aecec2aab82fcef11dbbd7b9e32b83f8f6c0b8" + integrity sha512-pYxfDYpued//QpnLIm4Avk7rsNtAtQkUES2cwAYSvD/wd2pKD71gN2Ebj3e7klzXwjocvE8c5vx/1fxwpqmSxA== + duplexer@~0.1.1: version "0.1.2" resolved "https://registry.npmjs.org/duplexer/-/duplexer-0.1.2.tgz" @@ -5480,6 +5485,14 @@ onetime@^6.0.0: dependencies: mimic-fn "^4.0.0" +openapi-enforcer@1.21.1: + version "1.21.1" + resolved "https://registry.yarnpkg.com/openapi-enforcer/-/openapi-enforcer-1.21.1.tgz#7ee234744ab9be6f556074fea5421cd16bcb464e" + integrity sha512-3prImYTGEsbQo0KC2/pCDprqaSFa52g9vyiEIzop1MspBxWd8YEAftpyYrNfyI/oxPHVPTD7GbQwp7mknun0Kg== + dependencies: + js-yaml "^4.1.0" + randexp "^0.5.3" + openapi-types@^12.0.0: version "12.0.0" resolved "https://registry.yarnpkg.com/openapi-types/-/openapi-types-12.0.0.tgz#458a99d048f9eae1c067e15d56a8bfb3726041f1" @@ -5938,6 +5951,14 @@ quick-lru@^5.1.1: resolved "https://registry.npmjs.org/quick-lru/-/quick-lru-5.1.1.tgz" integrity sha512-WuyALRjWPDGtt/wzJiadO5AXY+8hZ80hVpe6MyivgraREW751X3SbhRvG3eLKOYN+8VEvqLcf3wdnt44Z4S4SA== +randexp@^0.5.3: + version "0.5.3" + resolved "https://registry.yarnpkg.com/randexp/-/randexp-0.5.3.tgz#f31c2de3148b30bdeb84b7c3f59b0ebb9fec3738" + integrity sha512-U+5l2KrcMNOUPYvazA3h5ekF80FHTUG+87SEAmHZmolh1M+i/WyTCxVzmi+tidIa1tM4BSe8g2Y/D3loWDjj+w== + dependencies: + drange "^1.0.2" + ret "^0.2.0" + random-bytes@~1.0.0: version "1.0.0" resolved "https://registry.npmjs.org/random-bytes/-/random-bytes-1.0.0.tgz" @@ -6152,6 +6173,11 @@ restore-cursor@^3.1.0: onetime "^5.1.0" signal-exit "^3.0.2" +ret@^0.2.0: + version "0.2.2" + resolved "https://registry.yarnpkg.com/ret/-/ret-0.2.2.tgz#b6861782a1f4762dce43402a71eb7a283f44573c" + integrity sha512-M0b3YWQs7R3Z917WRQy1HHA7Ba7D8hvZg6UE5mLykJxQVE2ju0IXbGlaHPPlkY+WN7wFP+wUMXmBFA0aV6vYGQ== + retry@^0.12.0: version "0.12.0" resolved "https://registry.npmjs.org/retry/-/retry-0.12.0.tgz"