From 4ff673c5f2c1ecec9207e3832f8d8454fc11341d Mon Sep 17 00:00:00 2001 From: Milton Hultgren Date: Mon, 13 May 2024 11:39:51 +0200 Subject: [PATCH] [kbn-server-route-repository] Allow custom response (#182679) This PR changes `registerRoutes` to: 1. Pass the `response` object into the `handler` inside of the `wrappedHandler` to give access to the route handler to create Kibana responses 2. Check if the result of the `handler` is already a Kibana response and only wrap it into a Kibana response if it isn't. --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> --- .../src/router/response.test.ts | 4 + .../core-http-server/src/router/response.ts | 6 +- .../src/register_routes.test.ts | 209 ++++++++++++++++++ .../src/register_routes.ts | 17 +- .../src/test_types.ts | 43 ++++ .../src/typings.ts | 5 +- .../kbn-server-route-repository/tsconfig.json | 2 + 7 files changed, 278 insertions(+), 8 deletions(-) create mode 100644 packages/kbn-server-route-repository/src/register_routes.test.ts diff --git a/packages/core/http/core-http-server/src/router/response.test.ts b/packages/core/http/core-http-server/src/router/response.test.ts index 21a01a08aca5e..e6e586323c6d5 100644 --- a/packages/core/http/core-http-server/src/router/response.test.ts +++ b/packages/core/http/core-http-server/src/router/response.test.ts @@ -82,4 +82,8 @@ describe('isKibanaResponse', () => { }) ).toEqual(false); }); + + it('handles undefined inputs', () => { + expect(isKibanaResponse(undefined)).toEqual(false); + }); }); diff --git a/packages/core/http/core-http-server/src/router/response.ts b/packages/core/http/core-http-server/src/router/response.ts index 66a5797909b0c..385a03f518e7f 100644 --- a/packages/core/http/core-http-server/src/router/response.ts +++ b/packages/core/http/core-http-server/src/router/response.ts @@ -55,7 +55,11 @@ export interface IKibanaResponse): response is IKibanaResponse { +export function isKibanaResponse(response?: Record): response is IKibanaResponse { + if (!response) { + return false; + } + const { status, options, payload, ...rest } = response; if (Object.keys(rest).length !== 0) { diff --git a/packages/kbn-server-route-repository/src/register_routes.test.ts b/packages/kbn-server-route-repository/src/register_routes.test.ts new file mode 100644 index 0000000000000..d6599241f5f72 --- /dev/null +++ b/packages/kbn-server-route-repository/src/register_routes.test.ts @@ -0,0 +1,209 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import * as t from 'io-ts'; +import { CoreSetup, kibanaResponseFactory } from '@kbn/core/server'; +import { loggerMock } from '@kbn/logging-mocks'; +import { registerRoutes } from './register_routes'; +import { routeValidationObject } from './route_validation_object'; +import { NEVER } from 'rxjs'; + +describe('registerRoutes', () => { + const get = jest.fn(); + + const getAddVersion = jest.fn(); + const getWithVersion = jest.fn((_options) => { + return { + addVersion: getAddVersion, + }; + }); + + const createRouter = jest.fn().mockReturnValue({ + get, + versioned: { + get: getWithVersion, + }, + }); + + const internalOptions = { + internal: true, + }; + const publicOptions = { + public: true, + }; + + const internalHandler = jest.fn().mockResolvedValue('internal'); + const publicHandler = jest + .fn() + .mockResolvedValue( + kibanaResponseFactory.custom({ statusCode: 201, body: { message: 'public' } }) + ); + const errorHandler = jest.fn().mockRejectedValue(new Error('error')); + + const mockLogger = loggerMock.create(); + const mockService = jest.fn(); + + const mockContext = {}; + const mockRequest = { + body: { + bodyParam: 'body', + }, + query: { + queryParam: 'query', + }, + params: { + pathParam: 'path', + }, + events: { + aborted$: NEVER, + }, + }; + + beforeEach(() => { + jest.clearAllMocks(); + + const coreSetup = { + http: { + createRouter, + }, + } as unknown as CoreSetup; + + const paramsRt = t.type({ + body: t.type({ + bodyParam: t.string, + }), + query: t.type({ + queryParam: t.string, + }), + path: t.type({ + pathParam: t.string, + }), + }); + + registerRoutes({ + core: coreSetup, + repository: { + internal: { + endpoint: 'GET /internal/app/feature', + handler: internalHandler, + params: paramsRt, + options: internalOptions, + }, + public: { + endpoint: 'GET /api/app/feature version', + handler: publicHandler, + params: paramsRt, + options: publicOptions, + }, + error: { + endpoint: 'GET /internal/app/feature/error', + handler: errorHandler, + params: paramsRt, + options: internalOptions, + }, + }, + dependencies: { + aService: mockService, + }, + logger: mockLogger, + }); + }); + + it('creates a router and defines the routes', () => { + expect(createRouter).toHaveBeenCalledTimes(1); + + expect(get).toHaveBeenCalledTimes(2); + + const [internalRoute] = get.mock.calls[0]; + expect(internalRoute.path).toEqual('/internal/app/feature'); + expect(internalRoute.options).toEqual(internalOptions); + expect(internalRoute.validate).toEqual(routeValidationObject); + + const [errorRoute] = get.mock.calls[1]; + expect(errorRoute.path).toEqual('/internal/app/feature/error'); + expect(errorRoute.options).toEqual(internalOptions); + expect(errorRoute.validate).toEqual(routeValidationObject); + + expect(getWithVersion).toHaveBeenCalledTimes(1); + const [publicRoute] = getWithVersion.mock.calls[0]; + expect(publicRoute.path).toEqual('/api/app/feature'); + expect(publicRoute.options).toEqual(publicOptions); + expect(publicRoute.access).toEqual('public'); + + expect(getAddVersion).toHaveBeenCalledTimes(1); + const [versionedRoute] = getAddVersion.mock.calls[0]; + expect(versionedRoute.version).toEqual('version'); + expect(versionedRoute.validate).toEqual({ + request: routeValidationObject, + }); + }); + + it('calls the route handler with all dependencies', async () => { + const [_, internalRouteHandler] = get.mock.calls[0]; + await internalRouteHandler(mockContext, mockRequest, kibanaResponseFactory); + + const [args] = internalHandler.mock.calls[0]; + expect(Object.keys(args).sort()).toEqual( + ['aService', 'request', 'response', 'context', 'params', 'logger'].sort() + ); + + const { params, logger, aService, request, response, context } = args; + expect(params).toEqual({ + body: { + bodyParam: 'body', + }, + query: { + queryParam: 'query', + }, + path: { + pathParam: 'path', + }, + }); + expect(request).toBe(mockRequest); + expect(response).toBe(kibanaResponseFactory); + expect(context).toBe(mockContext); + expect(aService).toBe(mockService); + expect(logger).toBe(mockLogger); + }); + + it('wraps a plain route handler result into a response', async () => { + const [_, internalRouteHandler] = get.mock.calls[0]; + const internalResult = await internalRouteHandler( + mockContext, + mockRequest, + kibanaResponseFactory + ); + + expect(internalHandler).toHaveBeenCalledTimes(1); + expect(internalResult).toEqual({ + status: 200, + payload: 'internal', + options: { body: 'internal' }, + }); + }); + + it('allows for route handlers to define a custom response', async () => { + const [_, publicRouteHandler] = getAddVersion.mock.calls[0]; + const publicResult = await publicRouteHandler(mockContext, mockRequest, kibanaResponseFactory); + + expect(publicHandler).toHaveBeenCalledTimes(1); + expect(publicResult).toEqual({ status: 201, payload: { message: 'public' }, options: {} }); + }); + + it('translates errors thrown in a route handler to an error response', async () => { + const [_, errorRouteHandler] = get.mock.calls[1]; + const errorResult = await errorRouteHandler(mockContext, mockRequest, kibanaResponseFactory); + + expect(errorHandler).toHaveBeenCalledTimes(1); + expect(errorResult).toEqual({ + status: 500, + payload: { message: 'error', attributes: { data: {} } }, + options: {}, + }); + }); +}); diff --git a/packages/kbn-server-route-repository/src/register_routes.ts b/packages/kbn-server-route-repository/src/register_routes.ts index e80d7a4d8cddf..6e15bc6cf9416 100644 --- a/packages/kbn-server-route-repository/src/register_routes.ts +++ b/packages/kbn-server-route-repository/src/register_routes.ts @@ -9,6 +9,7 @@ import { errors } from '@elastic/elasticsearch'; import { isBoom } from '@hapi/boom'; import type { RequestHandlerContext } from '@kbn/core-http-request-handler-context-server'; import type { KibanaRequest, KibanaResponseFactory } from '@kbn/core-http-server'; +import { isKibanaResponse } from '@kbn/core-http-server'; import type { CoreSetup } from '@kbn/core-lifecycle-server'; import type { Logger } from '@kbn/logging'; import * as t from 'io-ts'; @@ -58,9 +59,10 @@ export function registerRoutes({ runtimeType ); - const { aborted, data } = await Promise.race([ + const { aborted, result } = await Promise.race([ handler({ request, + response, context, params: validatedParams, logger, @@ -68,13 +70,13 @@ export function registerRoutes({ }).then((value) => { return { aborted: false, - data: value, + result: value, }; }), request.events.aborted$.toPromise().then(() => { return { aborted: true, - data: undefined, + result: undefined, }; }), ]); @@ -83,9 +85,12 @@ export function registerRoutes({ return response.custom(CLIENT_CLOSED_REQUEST); } - const body = data || {}; - - return response.ok({ body }); + if (isKibanaResponse(result)) { + return result; + } else { + const body = result || {}; + return response.ok({ body }); + } } catch (error) { logger.error(error); diff --git a/packages/kbn-server-route-repository/src/test_types.ts b/packages/kbn-server-route-repository/src/test_types.ts index 4cfd82de450b8..a55c1014317ac 100644 --- a/packages/kbn-server-route-repository/src/test_types.ts +++ b/packages/kbn-server-route-repository/src/test_types.ts @@ -6,6 +6,7 @@ * Side Public License, v 1. */ import * as t from 'io-ts'; +import { kibanaResponseFactory } from '@kbn/core/server'; import { createServerRouteFactory } from './create_server_route_factory'; import { decodeRequestParams } from './decode_request_params'; import { EndpointOf, ReturnOf, RouteRepositoryClient } from './typings'; @@ -124,6 +125,24 @@ const repository = { }; }, }), + ...createServerRoute({ + endpoint: 'GET /internal/endpoint_returning_result', + handler: async () => { + return { + result: true, + }; + }, + }), + ...createServerRoute({ + endpoint: 'GET /internal/endpoint_returning_kibana_response', + handler: async () => { + return kibanaResponseFactory.ok({ + body: { + result: true, + }, + }); + }, + }), }; type TestRepository = typeof repository; @@ -150,6 +169,14 @@ const noParamsInvalid: ReturnOf>({ + result: true, +}); + +assertType>({ + result: true, +}); + // RouteRepositoryClient type TestClient = RouteRepositoryClient; @@ -214,6 +241,22 @@ client('GET /internal/endpoint_with_params', { }>(res); }); +client('GET /internal/endpoint_returning_result', { + timeout: 1, +}).then((res) => { + assertType<{ + result: boolean; + }>(res); +}); + +client('GET /internal/endpoint_returning_kibana_response', { + timeout: 1, +}).then((res) => { + assertType<{ + result: boolean; + }>(res); +}); + // decodeRequestParams should return the type of the codec that is passed assertType<{ path: { serviceName: string } }>( decodeRequestParams( diff --git a/packages/kbn-server-route-repository/src/typings.ts b/packages/kbn-server-route-repository/src/typings.ts index 94605468202f1..28212f80ca5da 100644 --- a/packages/kbn-server-route-repository/src/typings.ts +++ b/packages/kbn-server-route-repository/src/typings.ts @@ -6,6 +6,7 @@ * Side Public License, v 1. */ +import { IKibanaResponse } from '@kbn/core-http-server'; import * as t from 'io-ts'; import { RequiredKeys } from 'utility-types'; @@ -94,7 +95,9 @@ export type ReturnOf< infer TReturnType, ServerRouteCreateOptions > - ? TReturnType + ? TReturnType extends IKibanaResponse + ? TWrappedResponseType + : TReturnType : never; export type DecodedRequestParamsOf< diff --git a/packages/kbn-server-route-repository/tsconfig.json b/packages/kbn-server-route-repository/tsconfig.json index 67a5631bca59e..2ab8bda824600 100644 --- a/packages/kbn-server-route-repository/tsconfig.json +++ b/packages/kbn-server-route-repository/tsconfig.json @@ -18,6 +18,8 @@ "@kbn/core-http-server", "@kbn/core-lifecycle-server", "@kbn/logging", + "@kbn/core", + "@kbn/logging-mocks", ], "exclude": [ "target/**/*",