From 85b4e92f5ca5a96561422f331d4d0cf2bff8fd05 Mon Sep 17 00:00:00 2001 From: aramissennyeydd Date: Sun, 7 Jul 2024 17:44:17 -0400 Subject: [PATCH] small fixes for CI checks Signed-off-by: aramissennyeydd --- packages/backend-openapi-utils/report.api.md | 8 ++ packages/backend-openapi-utils/src/index.ts | 6 +- .../src/schema/parameter-validation.ts | 88 +++++++++---------- .../src/schema/validation.test.ts | 4 +- .../backend-openapi-utils/src/testUtils.ts | 26 ++++-- .../src/service/createRouter.test.ts | 4 +- .../search-backend/src/service/router.test.ts | 4 +- 7 files changed, 84 insertions(+), 56 deletions(-) diff --git a/packages/backend-openapi-utils/report.api.md b/packages/backend-openapi-utils/report.api.md index a0495f39b5..5a2c89ee81 100644 --- a/packages/backend-openapi-utils/report.api.md +++ b/packages/backend-openapi-utils/report.api.md @@ -3,6 +3,8 @@ > Do not edit this file. It is a report generated by [API Extractor](https://api-extractor.com/). ```ts +/// + import type { ContentObject } from 'openapi3-ts'; import type core from 'express-serve-static-core'; import { Express as Express_2 } from 'express'; @@ -682,6 +684,9 @@ type SchemaRef = Schema extends { [Key in keyof Schema]: SchemaRef; }; +// @public +export function setupProxyHooks(): void; + // @public type TemplateToDocPath< Doc extends PathDoc, @@ -718,6 +723,9 @@ type ValueOf = T[keyof T]; // @public export const wrapInOpenApiTestServer: (app: Express_2) => Server | Express_2; +// @public +export function wrapServer(app: Express_2): Promise; + // Warnings were encountered during analysis: // // src/router.d.ts:8:5 - (ae-undocumented) Missing documentation for "get". diff --git a/packages/backend-openapi-utils/src/index.ts b/packages/backend-openapi-utils/src/index.ts index 57d9f755c2..24f4183311 100644 --- a/packages/backend-openapi-utils/src/index.ts +++ b/packages/backend-openapi-utils/src/index.ts @@ -32,4 +32,8 @@ export type { } from './utility'; export type { ApiRouter } from './router'; export { createValidatedOpenApiRouter, getOpenApiSpecRoute } from './stub'; -export { wrapInOpenApiTestServer, wrapServer } from './testUtils'; +export { + wrapInOpenApiTestServer, + wrapServer, + setupProxyHooks, +} from './testUtils'; diff --git a/packages/backend-openapi-utils/src/schema/parameter-validation.ts b/packages/backend-openapi-utils/src/schema/parameter-validation.ts index 5cdb3dbd6b..90953e6e2a 100644 --- a/packages/backend-openapi-utils/src/schema/parameter-validation.ts +++ b/packages/backend-openapi-utils/src/schema/parameter-validation.ts @@ -111,53 +111,51 @@ export class QueryParameterParser const remainingQueryParameters = new Set(searchParams.keys()); const queryParameters: Record = {}; + const parameterIterator = Object.entries(this.parameters); + // object parameters with form/explode style should be processed last as they collect all remaining parameters. - const parameterIterator = Object.entries(this.parameters).toSorted( - ([_, parameterA], [_B, parameterB]) => { - if ( - parameterA.schema.type !== 'object' && - parameterB.schema.type !== 'object' - ) { - return EQUAL; - } - if ( - parameterA.schema.type === 'object' && - parameterB.schema.type !== 'object' - ) { - return PLACE_A_AFTER_B; - } - if ( - parameterA.schema.type !== 'object' && - parameterB.schema.type === 'object' - ) { - return PLACE_A_BEFORE_B; - } - const isParameterAForm = - parameterA.style === 'form' || !parameterA.style; - const isParameterAFormExplode = - isParameterAForm && - (parameterA.explode || typeof parameterA.explode === 'undefined'); - const isParameterBForm = - parameterB.style === 'form' || !parameterB.style; - const isParameterBFormExplode = - isParameterBForm && - (parameterB.explode || typeof parameterB.explode === 'undefined'); - // Sort the form explode to the bottom of the array. - if (isParameterAFormExplode && isParameterBFormExplode) { - throw new OperationError( - this.operation, - 'Ambiguous query parameters, you cannot have 2 form explode parameters', - ); - } - if (isParameterAFormExplode) { - return PLACE_A_AFTER_B; - } - if (isParameterBFormExplode) { - return PLACE_A_BEFORE_B; - } + parameterIterator.sort(([_, parameterA], [_B, parameterB]) => { + if ( + parameterA.schema.type !== 'object' && + parameterB.schema.type !== 'object' + ) { return EQUAL; - }, - ); + } + if ( + parameterA.schema.type === 'object' && + parameterB.schema.type !== 'object' + ) { + return PLACE_A_AFTER_B; + } + if ( + parameterA.schema.type !== 'object' && + parameterB.schema.type === 'object' + ) { + return PLACE_A_BEFORE_B; + } + const isParameterAForm = parameterA.style === 'form' || !parameterA.style; + const isParameterAFormExplode = + isParameterAForm && + (parameterA.explode || typeof parameterA.explode === 'undefined'); + const isParameterBForm = parameterB.style === 'form' || !parameterB.style; + const isParameterBFormExplode = + isParameterBForm && + (parameterB.explode || typeof parameterB.explode === 'undefined'); + // Sort the form explode to the bottom of the array. + if (isParameterAFormExplode && isParameterBFormExplode) { + throw new OperationError( + this.operation, + 'Ambiguous query parameters, you cannot have 2 form explode parameters', + ); + } + if (isParameterAFormExplode) { + return PLACE_A_AFTER_B; + } + if (isParameterBFormExplode) { + return PLACE_A_BEFORE_B; + } + return EQUAL; + }); for (const [name, parameter] of parameterIterator) { if (!parameter.schema) { throw new OperationError( diff --git a/packages/backend-openapi-utils/src/schema/validation.test.ts b/packages/backend-openapi-utils/src/schema/validation.test.ts index fdb1350625..9c8c4589a3 100644 --- a/packages/backend-openapi-utils/src/schema/validation.test.ts +++ b/packages/backend-openapi-utils/src/schema/validation.test.ts @@ -17,7 +17,7 @@ import { findOperationByRequest, OpenApiProxyValidator } from './validation'; import { rest } from 'msw'; import { setupServer } from 'msw/node'; -import { setupRequestMockHandlers } from '@backstage/backend-test-utils'; +import { registerMswTestHooks } from '@backstage/test-utils'; import { CompletedBody, CompletedRequest, CompletedResponse } from 'mockttp'; import withResponseBody from './__fixtures__/schemas/withJsonResponseBody.json'; import withQueryParameter from './__fixtures__/schemas/withQueryParameter.json'; @@ -61,7 +61,7 @@ function createMockttpResponse(response: { } describe('OpenApiProxyValidator', () => { - setupRequestMockHandlers(server); + registerMswTestHooks(server); let validator: OpenApiProxyValidator; async function mockSchema(schema: any) { diff --git a/packages/backend-openapi-utils/src/testUtils.ts b/packages/backend-openapi-utils/src/testUtils.ts index d93029fa8e..ea329332bd 100644 --- a/packages/backend-openapi-utils/src/testUtils.ts +++ b/packages/backend-openapi-utils/src/testUtils.ts @@ -19,14 +19,28 @@ import { Proxy } from './proxy/setup'; const proxy = new Proxy(); -beforeAll(async () => { - await proxy.setup(); -}); +/** + * Setup the proxy hooks for the test suite. This will start the proxy before all tests and stop it after all tests. + * @public + */ +export function setupProxyHooks() { + beforeAll(async () => { + await proxy.setup(); + }); -afterAll(() => { - proxy.stop(); -}); + afterAll(() => { + proxy.stop(); + }); +} +/** + * !!! THIS CURRENTLY ONLY SUPPORTS SUPERTEST !!! + * Setup a server with a custom OpenAPI proxy. This proxy will capture all requests and responses and make sure they + * conform to the spec. + * @param app - express server, needed to ensure we have the correct ports for the proxy. + * @returns - a configured HTTP server that should be used with supertest. + * @public + */ export async function wrapServer(app: Express): Promise { const server = app.listen(proxy.forwardTo.port); await proxy.initialize(`http://localhost:${proxy.forwardTo.port}`, server); diff --git a/plugins/catalog-backend/src/service/createRouter.test.ts b/plugins/catalog-backend/src/service/createRouter.test.ts index 6654187526..3b478a2a84 100644 --- a/plugins/catalog-backend/src/service/createRouter.test.ts +++ b/plugins/catalog-backend/src/service/createRouter.test.ts @@ -38,7 +38,7 @@ import { RESOURCE_TYPE_CATALOG_ENTITY } from '@backstage/plugin-catalog-common/a import { CatalogProcessingOrchestrator } from '../processing/types'; import { z } from 'zod'; import { decodeCursor, encodeCursor } from './util'; -import { wrapServer } from '@backstage/backend-openapi-utils'; +import { setupProxyHooks, wrapServer } from '@backstage/backend-openapi-utils'; import { Server } from 'http'; import { mockCredentials, mockServices } from '@backstage/backend-test-utils'; import { LocationAnalyzer } from '@backstage/plugin-catalog-node'; @@ -53,6 +53,8 @@ describe('createRouter readonly disabled', () => { let locationAnalyzer: jest.Mocked; let permissionsService: jest.Mocked; + setupProxyHooks(); + beforeAll(async () => { entitiesCatalog = { entities: jest.fn(), diff --git a/plugins/search-backend/src/service/router.test.ts b/plugins/search-backend/src/service/router.test.ts index 5b6900ede1..41b03501a3 100644 --- a/plugins/search-backend/src/service/router.test.ts +++ b/plugins/search-backend/src/service/router.test.ts @@ -23,7 +23,7 @@ import { import express from 'express'; import request from 'supertest'; import { createRouter } from './router'; -import { wrapServer } from '@backstage/backend-openapi-utils'; +import { setupProxyHooks, wrapServer } from '@backstage/backend-openapi-utils'; import { Server } from 'http'; import { mockCredentials, @@ -55,6 +55,8 @@ describe('createRouter', () => { }, }; + setupProxyHooks(); + beforeAll(async () => { const logger = mockServices.logger.mock(); mockSearchEngine = {