From a6f55375f23e56fd32ce7b6c65c02ef5905fbcfc Mon Sep 17 00:00:00 2001 From: BelkinaDasha Date: Tue, 21 Apr 2020 20:52:43 +0500 Subject: [PATCH 1/6] feat(reply headers): rfc, scaffold --- ...ceful_shutdown.md => graceful-shutdown.md} | 0 packages/core/docs/rfc/service-request.md | 204 ++++++++++++++++++ packages/core/src/constants.ts | 2 + .../core/src/plugins/amqp/router/adapter.ts | 27 +-- .../amqp/router/service-request-factory.ts | 19 ++ .../src/plugins/http/handlers/hapi/index.ts | 10 +- .../http/handlers/hapi/router/adapter.ts | 43 ++-- .../http/handlers/hapi/router/attach.ts | 4 +- .../hapi/router/service-request-factory.ts | 20 ++ packages/core/src/plugins/router.ts | 35 ++- .../core/src/plugins/router/dispatcher.ts | 12 +- .../router/extensions/sharedHandlers.ts | 5 +- .../validate/query-string-parser.ts | 6 +- .../extensions/validate/schemaLessAction.ts | 4 +- .../extensions/validate/transport-options.ts | 6 +- .../src/plugins/router/modules/allowed.ts | 6 +- .../core/src/plugins/router/modules/auth.ts | 10 +- .../src/plugins/router/modules/handler.ts | 4 +- .../src/plugins/router/modules/request.ts | 6 +- .../src/plugins/router/modules/response.ts | 4 +- .../src/plugins/router/modules/validate.ts | 8 +- .../plugins/router/routes/generic/health.ts | 4 +- .../src/plugins/socketIO/router/adapter.ts | 46 ++-- .../router/service-request-factory.ts | 20 ++ packages/core/src/types.ts | 29 ++- packages/core/src/types/module.d.ts | 1 + packages/core/src/utils/service-request.ts | 58 +++++ .../helpers/actions/success-remove-header.js | 11 + .../helpers/actions/success-set-header.js | 9 + .../core/test/amqp/helpers/actions/success.js | 9 + packages/core/test/config.js | 6 +- .../helpers/actions/success-remove-header.js | 11 + .../helpers/actions/success-set-header.js | 9 + .../helpers/actions/success-set-header.js | 9 + packages/core/test/suites/amqp.js | 36 ++++ packages/core/test/suites/http.hapi.js | 63 +++++- packages/core/test/suites/socketIO.js | 30 ++- 37 files changed, 640 insertions(+), 146 deletions(-) rename packages/core/docs/rfc/{graceful_shutdown.md => graceful-shutdown.md} (100%) create mode 100644 packages/core/docs/rfc/service-request.md create mode 100644 packages/core/src/plugins/amqp/router/service-request-factory.ts create mode 100644 packages/core/src/plugins/http/handlers/hapi/router/service-request-factory.ts create mode 100644 packages/core/src/plugins/socketIO/router/service-request-factory.ts create mode 100644 packages/core/src/utils/service-request.ts create mode 100644 packages/core/test/amqp/helpers/actions/success-remove-header.js create mode 100644 packages/core/test/amqp/helpers/actions/success-set-header.js create mode 100644 packages/core/test/amqp/helpers/actions/success.js create mode 100644 packages/core/test/hapi/helpers/actions/success-remove-header.js create mode 100644 packages/core/test/hapi/helpers/actions/success-set-header.js create mode 100644 packages/core/test/socketIO/helpers/actions/success-set-header.js diff --git a/packages/core/docs/rfc/graceful_shutdown.md b/packages/core/docs/rfc/graceful-shutdown.md similarity index 100% rename from packages/core/docs/rfc/graceful_shutdown.md rename to packages/core/docs/rfc/graceful-shutdown.md diff --git a/packages/core/docs/rfc/service-request.md b/packages/core/docs/rfc/service-request.md new file mode 100644 index 000000000..a340f6dfb --- /dev/null +++ b/packages/core/docs/rfc/service-request.md @@ -0,0 +1,204 @@ +# Service Request + +## Overview and Motivation +The Service handles incoming messages via several action Transports. On each incoming message Transport Router Adapter +builds a Service Request instance and passes it to the Dispatcher. This instance becomes available in the Action Handler +as an argument. Its structure is abstract and functionality is basically transport-agnostic. + + +Most transport protocols natively have headers in messages. While currently every Transport Router Adapter is able to +parse incoming messages including its optional headers, in order to fully support messaging protocols the Service should +provide a way to set, modify and remove response message headers. + + +Although every transport could have its own protocol limitations (considerations) over the headers, the Service Request +instance could only take responsibility for collecting them. In order to be able to validate and adapt resulting +collection independently, these tasks should be performed on the Transport level. + + +Considering Node V8 hidden classes concept, in order to minimize hidden class trees number and respectfully maximize the +performance, the Service Request instance must always aim to have same object shape and its properties initialization +order. +Our secret Node.JS performance expert claims that functions work even faster regarding hidden class optimization than +ES6 classes. +To meet these requirements the Service Request must be initialized using functions and preset every property value on +construction. + +## Service Request Interface + +### Properties + +#### `.transport: TransportTypes` +Transport name that handles the incoming request. + +Must be set by Transport Router Adapter. + +#### `.method: RequestMethods` +Request method. + +Virtual value, which, depending on transport design, should either preserve its original request method name or provide +its custom name. + +Must be set by Transport Router Adapter. + +#### `.query: object` +Original message may contain query params. + +Transport Router Adapter should extract query data and set it as query. + +Notice that `.query` value may be possibly modified during the Request step of the Validation Lifecycle: it could be +filtered, assigned by default values and underlying data types could be coerced. + +#### `.headers: any` +Original message may contain headers. Transport Router Adapter should extract headers data and set it as params. +The responsibility for extracting request headers and setting it to the Service Request must lay on the +Transport Router Adapter implementation. + +#### `.params: any` +Original message may contain params. + +Transport Router Adapter should extract request data and set it as params. + +Notice that `.params` value may be possibly modified during the Request step of the Validation Lifecycle: it could be +filtered, assigned by default values and underlying data types could be coerced. + +#### `.transportRequest?: any` +Third-party request instance. + +May be set by Transport Router Adapter. + +#### `.log?: { trace(...args: any[]): void; debug(...args: any[]): void; info(...args: any[]): void; warn(...args: any[]): void; error(...args: any[]): void; fatal(...args: any[]): void; }` +Router must set Log child instance with a unique Request identifier. + +#### `.socket?: NodeJS.EventEmitter` +In order to provide web sockets protocol support we need to operate on socket instance... But could we live without it? +Can we manage it through service request extensions mechanism? + +#### `.parentSpan` +When a Tracer is enabled, property may hold a tracer parent span, which context must be supplied by the Transport. + +#### `.route: string` +Route name may contain two parts joined by dot - optional Router prefix and required path to the Action Handler, +transformed to dot case. It shall result into the following format: +``` +'router-prefix.path.to.the.action.handler' +``` +Assuming that the Router plugin prefix configured as `'payments'`, the path to the action +relative to the routes directory defined by Router plugin configuration is `'transactions/create'`, resulting route +value will be `payments.transactions.create`. + +*Notice: Route name should be transport-agnostic and therefore must not contain Transport route prefix.* + +Route name must be set during the Request step of the Request Lifecycle. + +#### `.action: ServiceAction` +When the route match is found, the Router must provide an Action instance to the Service Request. + +Action must be set during the Request step of the Request Lifecycle. + +#### `.auth: any` +Original message may contain authentication data. Considering the Action authentication strategy it may be resolved +during the Auth step of Request Lifecycle and set as the `.auth` property value. + +#### `.span` +When a Tracer is enabled, property must hold a tracer span, initialized as a `.parentSpan` child. + +#### `.locals` +By design, this property recommended usage is to share data between Request Lifecycle steps, as well as pass it through +when using Internal Transport. Could be set anywhere during the Request Lifecycle. + +#### `.[kReplyHeaders]: Map` +Reply message headers container. Could be set anywhere during the Request Lifecycle with. Should be used to collect and +deliver headers to original reply message. + +### Methods + +#### `.setReplyHeader(title: string, value: number|string|array): void` +Sets reply header to the map. + +Must normalizes title. +Must validate and cast value. + +Headers considerations: +- HTTP: When value is array, joins values by semicolon +- HTTP: *`Set-Cookie` must not be joined.* https://tools.ietf.org/html/rfc7230#section-3.2.2 (Hapi handles it correctly, +but we do not proxy set header calls, we collect them, that's why we allow raw array of strings as a value) +- HTTP: Should allow both 'x-' prefixed and not prefixed headers + +Questions: +- AMQP: Reject headers starting with anything except 'x-' not let it to be overriden? What about `x-death`? +- Internal: Is there any sense of implementing headers for internal transport? + +Usage: +```js +function actionHandler(serviceRequest) { + const { state } = this.config.cookie + serviceRequest.setReplyHeader('x-rate-limit', 10000) + serviceRequest.setReplyHeader('set-cookie', [...state]) +} +``` + +#### `.removeReplyHeader(title: string): void` +Normalizes title and removes header from headers map. + +Usage: +```js +function actionHandler(serviceRequest) { + serviceRequest.removeReplyHeader('x-rate-limit') + serviceRequest.removeReplyHeader('set-cookie') +} +``` + +#### `.getReplyHeaders(): Map` +Gets all previously initialized headers from headers map. + +Usage: +```js +function actionHandler(serviceRequest) { + serviceRequest.removeReplyHeader('x-rate-limit') + serviceRequest.removeReplyHeader('set-cookie') +} +``` + +#### `.getReplyHeader(title: string): number|string|string[]` +Gets header from map. + +Must normalize title. + +## Implementation design +### AMQP Transport +Extract headers collection and set it under `kReplyHeaders` key of `raw.properties`, where `kReplyHeaders` is a Symbol +defined by `@microfleet/transport-amqp` library. + +### HTTP Transport +Consider HTTP RFC relative to [`set-cookie` header](https://tools.ietf.org/html/rfc7230#section-3.2.2). Since this is +common thing for any HTTP handler, it should be implemented in a composable way. + +#### Hapi Handler +Extract headers collection and set it to the response using Hapi Response Toolkit. + +### SocketIO Transport +Expect new optional argument `options` to be passed on `.emit`: +``` +.emit(action: string, body: any, options: SocketIOMessageOptions, callback: RequestCallback): void +``` + +When present, options may contain `simpleResponse` setting, which defaults to true. +When `simpleResponse` option is disabled, callback `result` must be resolved with `data` containing response message, +and `headers` containing headers that had user could set. + +``` +{ headers: { [key: string]: string }, data?: unknown } +``` + +Usage +``` +client.emit('echo', { message: 'ok' }, { simpleResponse: false }, (error, result) => { + const { data, headers } = result; +}); +``` + +### Internal Transport +If there any sense of implementing internal transport headers, its returning Promise may also be resolved with `data` +and `headers`. However, I don't see the `dispatch` method argument list extended by some response options like +`simpleResponse`, because it does not seem like an appropriate place for that. diff --git a/packages/core/src/constants.ts b/packages/core/src/constants.ts index 4f812a3a9..a6bec95bb 100644 --- a/packages/core/src/constants.ts +++ b/packages/core/src/constants.ts @@ -96,3 +96,5 @@ export const PluginsPriority = [ export const PLUGIN_STATUS_OK = 'ok' export const PLUGIN_STATUS_FAIL = 'fail' + +export const kReplyHeaders = Symbol('replyHeaders') diff --git a/packages/core/src/plugins/amqp/router/adapter.ts b/packages/core/src/plugins/amqp/router/adapter.ts index 7ebb1c238..42e64e854 100644 --- a/packages/core/src/plugins/amqp/router/adapter.ts +++ b/packages/core/src/plugins/amqp/router/adapter.ts @@ -3,8 +3,9 @@ import get = require('get-value') import is = require('is') import noop = require('lodash/noop') import { ActionTransport } from '../../..' -import { ServiceRequest } from '../../../types' import { Router } from '../../router/factory' +import { kReplyHeaders } from '@microfleet/transport-amqp/lib/constants' +import { createServiceRequest } from './service-request-factory' // cached var const { amqp } = ActionTransport @@ -41,28 +42,14 @@ function getAMQPRouterAdapter(router: Router, config: any) { const routingKey = properties.headers['routing-key'] || properties.routingKey const actionName = normalizeActionName(routingKey) - const opts: ServiceRequest = { - // initiate action to ensure that we have prepared proto fo the object - // input params - // make sure we standardize the request - // to provide similar interfaces - params, - action: noop as any, - headers: properties, - locals: Object.create(null), - log: console as any, - method: amqp as ServiceRequest['method'], - parentSpan: raw.span, - query: Object.create(null), - route: '', - span: undefined, - transport: amqp, - transportRequest: Object.create(null), - } + const serviceRequest = createServiceRequest(properties, params, raw.span); increaseCounter() try { - const promise = dispatch(actionName, opts) + const promise = dispatch(actionName, serviceRequest) + .finally(() => { + raw.properties[kReplyHeaders] = Object.fromEntries(serviceRequest.getReplyHeaders()) + }); const response = await wrapDispatch(promise, actionName, raw) setImmediate(next, null, response) } catch (e) { diff --git a/packages/core/src/plugins/amqp/router/service-request-factory.ts b/packages/core/src/plugins/amqp/router/service-request-factory.ts new file mode 100644 index 000000000..5804e6790 --- /dev/null +++ b/packages/core/src/plugins/amqp/router/service-request-factory.ts @@ -0,0 +1,19 @@ +import { ActionTransport, ServiceRequestInterface } from '../../..'; +import { ServiceRequest } from '../../../utils/service-request'; + +export const createServiceRequest = ( + properties: any, + params: any, + parentSpan: any +): ServiceRequestInterface => ( + new ServiceRequest( + ActionTransport.amqp, + 'amqp', + Object.create(null), + properties, + params, + Object.create(null), + undefined, + parentSpan, + ) +); diff --git a/packages/core/src/plugins/http/handlers/hapi/index.ts b/packages/core/src/plugins/http/handlers/hapi/index.ts index f9310cf8d..0bf02908c 100644 --- a/packages/core/src/plugins/http/handlers/hapi/index.ts +++ b/packages/core/src/plugins/http/handlers/hapi/index.ts @@ -61,12 +61,12 @@ function createHapiServer(config: any, service: Microfleet): PluginInterface { } const registrations = [] - for (const pluguinConfiguration of plugins) { + for (const pluginConfiguration of plugins) { registrations.push({ - options: pluguinConfiguration.options, - plugin: typeof pluguinConfiguration.plugin === 'string' - ? require(pluguinConfiguration.plugin) - : pluguinConfiguration.plugin, + options: pluginConfiguration.options, + plugin: typeof pluginConfiguration.plugin === 'string' + ? require(pluginConfiguration.plugin) + : pluginConfiguration.plugin, }) } diff --git a/packages/core/src/plugins/http/handlers/hapi/router/adapter.ts b/packages/core/src/plugins/http/handlers/hapi/router/adapter.ts index 3e37b3211..d4dc5d86e 100644 --- a/packages/core/src/plugins/http/handlers/hapi/router/adapter.ts +++ b/packages/core/src/plugins/http/handlers/hapi/router/adapter.ts @@ -1,13 +1,19 @@ import { HttpStatusError } from '@microfleet/validation' import Bluebird = require('bluebird') import Errors = require('common-errors') -import { Request } from '@hapi/hapi' -import noop = require('lodash/noop') +import { Request, ResponseObject, ResponseToolkit } from '@hapi/hapi' import { FORMAT_HTTP_HEADERS } from 'opentracing' -import { ActionTransport, Microfleet } from '../../../../..' -import { ServiceRequest, RequestMethods } from '../../../../../types' +import { Microfleet, ReplyHeaderValue } from '../../../../..' import _require from '../../../../../utils/require' import { Router } from '../../../../router/factory' +import { createServiceRequest} from "./service-request-factory"; + +const setReplyHeader = (response: ResponseObject) => (value: ReplyHeaderValue, title: string) => { + // set-cookie header exceptional case is correctly implemented by hapi + return Array.isArray(value) + ? value.forEach(item => response.header(title, item)) + : response.header(title, value) +} export default function getHapiAdapter(actionName: string, service: Microfleet) { const Boom = _require('@hapi/boom') @@ -61,7 +67,7 @@ export default function getHapiAdapter(actionName: string, service: Microfleet) // pre-wrap the function so that we do not need to actually do fromNode(next) const dispatch = Bluebird.promisify(router.dispatch, { context: router }) - return async function handler(request: Request) { + return async function handler(request: Request, h: ResponseToolkit) { const { headers } = request let parentSpan @@ -69,28 +75,15 @@ export default function getHapiAdapter(actionName: string, service: Microfleet) parentSpan = service.tracer.extract(headers, FORMAT_HTTP_HEADERS) } - const serviceRequest: ServiceRequest = { - // defaults for consistent object map - // opentracing - // set to console - // transport type - headers, - parentSpan, - action: noop as any, - locals: Object.create(null), - log: console as any, - method: request.method.toLowerCase() as RequestMethods, - params: request.payload, - query: request.query, - route: '', - span: undefined, - transport: ActionTransport.http, - transportRequest: request, - } + const serviceRequest = createServiceRequest(request, parentSpan); + + let response: ResponseObject - let response try { - response = await dispatch(actionName, serviceRequest) + const responseData = await dispatch(actionName, serviceRequest) + + response = h.response(responseData) + serviceRequest.getReplyHeaders().forEach(setReplyHeader(response)) } catch (e) { response = reformatError(e) } diff --git a/packages/core/src/plugins/http/handlers/hapi/router/attach.ts b/packages/core/src/plugins/http/handlers/hapi/router/attach.ts index 262d65f75..6c8844273 100644 --- a/packages/core/src/plugins/http/handlers/hapi/router/attach.ts +++ b/packages/core/src/plugins/http/handlers/hapi/router/attach.ts @@ -58,10 +58,10 @@ export default function attachRouter(service: Microfleet, config: any): HapiPlug server.route({ method: ['GET', 'POST'], path: '/{any*}', - async handler(request: Request) { + async handler(request: Request, h: ResponseToolkit) { const actionName = fromPathToName(request.path, config.prefix) const handler = hapiRouterAdapter(actionName, service) - return handler(request) + return handler(request, h) }, }) diff --git a/packages/core/src/plugins/http/handlers/hapi/router/service-request-factory.ts b/packages/core/src/plugins/http/handlers/hapi/router/service-request-factory.ts new file mode 100644 index 000000000..75ebd24f5 --- /dev/null +++ b/packages/core/src/plugins/http/handlers/hapi/router/service-request-factory.ts @@ -0,0 +1,20 @@ +import { Request } from '@hapi/hapi' +import SpanContext from 'opentracing/src/span_context'; +import { ActionTransport, RequestMethods, ServiceRequestInterface } from '../../../../..'; +import { ServiceRequest } from '../../../../../utils/service-request'; + +export const createServiceRequest = ( + request: Request, + parentSpan?: SpanContext +): ServiceRequestInterface => ( + new ServiceRequest( + ActionTransport.http, + request.method.toLowerCase() as RequestMethods, + request.query, + request.headers, + request.payload, + request, + undefined, + parentSpan, + ) +) diff --git a/packages/core/src/plugins/router.ts b/packages/core/src/plugins/router.ts index d1bfcbb92..d2660a5d1 100644 --- a/packages/core/src/plugins/router.ts +++ b/packages/core/src/plugins/router.ts @@ -3,10 +3,12 @@ import rfdc = require('rfdc') import { NotFoundError, NotSupportedError } from 'common-errors' import { ActionTransport, PluginTypes, identity } from '../constants' import { Microfleet } from '../' -import { ServiceRequest } from '../types' +import { ServiceRequestInterface } from '../types' import { getRouter, Router, RouterConfig, LifecycleRequestType } from './router/factory' import { ValidatorPlugin } from './validator' import { object as isObject } from 'is' +import { ServiceRequest } from '../utils/service-request'; + const { internal } = ActionTransport /** @@ -21,7 +23,7 @@ export { Router, RouterConfig, LifecycleRequestType } */ export interface RouterPlugin { router: Router; - dispatch: (route: string, request: Partial) => PromiseLike; + dispatch: (route: string, request: Partial) => PromiseLike; } /** @@ -52,27 +54,16 @@ const deepClone = rfdc() * @param request - service request. * @returns Prepared service request. */ -const prepareRequest = (request: Partial): ServiceRequest => ({ - // initiate action to ensure that we have prepared proto fo the object - // input params - // make sure we standardize the request - // to provide similar interfaces - action: null as any, - headers: shallowObjectClone(request.headers), - locals: shallowObjectClone(request.locals), - auth: shallowObjectClone(request.auth), - log: console as any, - method: internal as ServiceRequest['method'], - params: request.params != null +const prepareRequest = (request: Partial): ServiceRequestInterface => new ServiceRequest( + internal, + internal, + Object.create(null), + shallowObjectClone(request.headers), + request.params != null ? deepClone(request.params) : Object.create(null), - parentSpan: undefined, - query: Object.create(null), - route: '', - span: undefined, - transport: internal, - transportRequest: Object.create(null), -}) + Object.create(null) +) /** * Enables router plugin. @@ -97,7 +88,7 @@ export function attach(this: Microfleet & ValidatorPlugin & RouterPlugin, opts: : identity // dispatcher - this.dispatch = (route: string, request: Partial) => { + this.dispatch = (route: string, request: Partial) => { const msg = prepareRequest(request) return router.dispatch(assemble(route), msg) } diff --git a/packages/core/src/plugins/router/dispatcher.ts b/packages/core/src/plugins/router/dispatcher.ts index 4de83bfa3..f95f4c0c3 100644 --- a/packages/core/src/plugins/router/dispatcher.ts +++ b/packages/core/src/plugins/router/dispatcher.ts @@ -3,13 +3,13 @@ import _debug = require('debug') import is = require('is') import { Tags } from 'opentracing' import uuid = require('uuid') -import { ServiceRequest } from '../../types' +import { ServiceRequestInterface } from '../../types' import { Router } from './factory' const debug = _debug('mservice:router:dispatch') const { ERROR, COMPONENT } = Tags -const wrapPromise = (span: any, promise: any, callback?: (err: any, result?: any) => void) => ( +const wrapPromise = (span: any, promise: any, callback?: RequestCallback) => ( promise .catch((err: Error) => { span.setTag(ERROR, true) @@ -27,7 +27,7 @@ const wrapPromise = (span: any, promise: any, callback?: (err: any, result?: any .asCallback(callback) ) -function reflectToProps(this: ServiceRequest, reflection: Bluebird.Inspection) { +function reflectToProps(this: ServiceRequestInterface, reflection: Bluebird.Inspection) { return reflection.isRejected() ? [reflection.reason(), undefined, this] : [null, reflection.value(), this] @@ -35,9 +35,9 @@ function reflectToProps(this: ServiceRequest, reflection: Bluebird.Inspection void -function dispatch(this: Router, route: string, request: ServiceRequest): Bluebird -function dispatch(this: Router, route: string, request: ServiceRequest, callback: RequestCallback): void -function dispatch(this: Router, route: string, request: ServiceRequest, callback?: RequestCallback) { +function dispatch(this: Router, route: string, request: ServiceRequestInterface): Bluebird +function dispatch(this: Router, route: string, request: ServiceRequestInterface, callback: RequestCallback): void +function dispatch(this: Router, route: string, request: ServiceRequestInterface, callback?: RequestCallback) { const { modules, service } = this debug('initiating request on route %s', route) diff --git a/packages/core/src/plugins/router/extensions/sharedHandlers.ts b/packages/core/src/plugins/router/extensions/sharedHandlers.ts index b59138df0..9e678e545 100644 --- a/packages/core/src/plugins/router/extensions/sharedHandlers.ts +++ b/packages/core/src/plugins/router/extensions/sharedHandlers.ts @@ -1,12 +1,11 @@ - import { LifecyclePoints } from '.' -import { ServiceRequest } from '../../../types' +import { ServiceRequestInterface } from '../../../types' interface RequestStartExtension { started: [number, number]; executionTotal: [number, number]; } -export type ServiceRequestWithStart = ServiceRequest & RequestStartExtension +export type ServiceRequestWithStart = ServiceRequestInterface & RequestStartExtension export function storeRequestTimeFactory() { return { diff --git a/packages/core/src/plugins/router/extensions/validate/query-string-parser.ts b/packages/core/src/plugins/router/extensions/validate/query-string-parser.ts index d74872ca3..8bf425161 100644 --- a/packages/core/src/plugins/router/extensions/validate/query-string-parser.ts +++ b/packages/core/src/plugins/router/extensions/validate/query-string-parser.ts @@ -1,10 +1,10 @@ import identity = require('lodash/identity') import { parse } from 'qs' -import { ServiceRequest } from '../../../../types' +import { ServiceRequestInterface } from '../../../../types' import { LifecyclePoints } from '..' -type QSParserAugmentedAction = ServiceRequest & { - action: ServiceRequest['action'] & { +type QSParserAugmentedAction = ServiceRequestInterface & { + action: ServiceRequestInterface['action'] & { transformQuery?: (...args: any[]) => any; transformOpts?: any; }; diff --git a/packages/core/src/plugins/router/extensions/validate/schemaLessAction.ts b/packages/core/src/plugins/router/extensions/validate/schemaLessAction.ts index 4fe1b4d61..445722279 100644 --- a/packages/core/src/plugins/router/extensions/validate/schemaLessAction.ts +++ b/packages/core/src/plugins/router/extensions/validate/schemaLessAction.ts @@ -1,7 +1,7 @@ -import { ServiceRequest } from '../../../../types' +import { ServiceRequestInterface } from '../../../../types' import { LifecyclePoints } from '..' -export type ServiceRequestWithSchema = ServiceRequest & { +export type ServiceRequestWithSchema = ServiceRequestInterface & { schema?: string; } diff --git a/packages/core/src/plugins/router/extensions/validate/transport-options.ts b/packages/core/src/plugins/router/extensions/validate/transport-options.ts index ecd3bd1b6..0fe8a1c09 100644 --- a/packages/core/src/plugins/router/extensions/validate/transport-options.ts +++ b/packages/core/src/plugins/router/extensions/validate/transport-options.ts @@ -1,10 +1,10 @@ import Bluebird = require('bluebird') import { NotSupportedError } from 'common-errors' -import { ServiceRequest } from '../../../../types' +import { ServiceRequestInterface } from '../../../../types' import { LifecyclePoints } from '..' -export type TransportOptionsAugmentedRequest = ServiceRequest & { - action: ServiceRequest['action'] & { +export type TransportOptionsAugmentedRequest = ServiceRequestInterface & { + action: ServiceRequestInterface['action'] & { transportsOptions: { [transport: string]: { methods: string[]; diff --git a/packages/core/src/plugins/router/modules/allowed.ts b/packages/core/src/plugins/router/modules/allowed.ts index 42de60420..bc6a495b7 100644 --- a/packages/core/src/plugins/router/modules/allowed.ts +++ b/packages/core/src/plugins/router/modules/allowed.ts @@ -3,10 +3,10 @@ import { identity } from '../../../constants' import { HttpStatusError, NotPermittedError } from 'common-errors' import is = require('is') import { Microfleet } from '../../../' -import { ServiceRequest } from '../../../types' +import { ServiceRequestInterface } from '../../../types' import moduleLifecycle from './lifecycle' -function allowed(this: Microfleet, request: ServiceRequest) { +function allowed(this: Microfleet, request: ServiceRequestInterface) { return Bluebird .resolve(request) .bind(this) @@ -24,7 +24,7 @@ function allowed(this: Microfleet, request: ServiceRequest) { }) } -function allowedHandler(this: Microfleet, request: ServiceRequest) { +function allowedHandler(this: Microfleet, request: ServiceRequestInterface) { const allowedFn = is.undefined(request.action.allowed) ? identity : allowed diff --git a/packages/core/src/plugins/router/modules/auth.ts b/packages/core/src/plugins/router/modules/auth.ts index 745472f66..de2988b97 100644 --- a/packages/core/src/plugins/router/modules/auth.ts +++ b/packages/core/src/plugins/router/modules/auth.ts @@ -2,12 +2,12 @@ import Bluebird = require('bluebird') import { AuthenticationRequiredError, NotImplementedError } from 'common-errors' import is = require('is') import { Microfleet, RouterPlugin } from '../../../' -import { AuthConfig, ServiceRequest } from '../../../types' +import { AuthConfig, ServiceRequestInterface } from '../../../types' import { identity } from '../../../constants' import moduleLifecycle from './lifecycle' export interface AuthStrategy { - (this: Microfleet, request: ServiceRequest): PromiseLike; + (this: Microfleet, request: ServiceRequestInterface): PromiseLike; } export interface AuthOptions { @@ -40,7 +40,7 @@ const isObligatory = (strategy: string) => { } } -const retrieveStrategy = (request: ServiceRequest, strategies: AuthOptions['strategies']) => { +const retrieveStrategy = (request: ServiceRequestInterface, strategies: AuthOptions['strategies']) => { const { action } = request const authConfig = action.auth @@ -84,7 +84,7 @@ const retrieveStrategy = (request: ServiceRequest, strategies: AuthOptions['stra } } -function auth(this: Microfleet, request: ServiceRequest, strategies: AuthOptions['strategies']) { +function auth(this: Microfleet, request: ServiceRequestInterface, strategies: AuthOptions['strategies']) { const authSchema = retrieveStrategy(request, strategies) if (authSchema.strategy == null) { @@ -107,7 +107,7 @@ function auth(this: Microfleet, request: ServiceRequest, strategies: AuthOptions } function assignStrategies(strategies: AuthOptions['strategies']) { - return function authHandler(this: Microfleet & RouterPlugin, request: ServiceRequest): PromiseLike { + return function authHandler(this: Microfleet & RouterPlugin, request: ServiceRequestInterface): PromiseLike { const authFn = is.undefined(request.action.auth) ? identity : auth diff --git a/packages/core/src/plugins/router/modules/handler.ts b/packages/core/src/plugins/router/modules/handler.ts index a8f1a105d..eba2ac26c 100644 --- a/packages/core/src/plugins/router/modules/handler.ts +++ b/packages/core/src/plugins/router/modules/handler.ts @@ -1,9 +1,9 @@ import Bluebird = require('bluebird') import { Microfleet, RouterPlugin } from '../../../' -import { ServiceRequest } from '../../../types' +import { ServiceRequestInterface } from '../../../types' import moduleLifecycle from './lifecycle' -function handler(this: Microfleet & RouterPlugin, request: ServiceRequest): Bluebird { +function handler(this: Microfleet & RouterPlugin, request: ServiceRequestInterface): Bluebird { const { extensions } = this.router return moduleLifecycle('handler', request.action, extensions, [request], this) } diff --git a/packages/core/src/plugins/router/modules/request.ts b/packages/core/src/plugins/router/modules/request.ts index 8fa9c2357..98dfb2837 100644 --- a/packages/core/src/plugins/router/modules/request.ts +++ b/packages/core/src/plugins/router/modules/request.ts @@ -3,12 +3,12 @@ import { ArgumentError, NotFoundError, HttpStatusError } from 'common-errors' import _debug = require('debug') import is = require('is') import { Microfleet, RouterPlugin } from '../../../' -import { ServiceRequest } from '../../../types' +import { ServiceRequestInterface } from '../../../types' import moduleLifecycle from './lifecycle' const debug = _debug('mservice:router:module:request') -function getAction(this: Microfleet & RouterPlugin, route: string, request: ServiceRequest) { +function getAction(this: Microfleet & RouterPlugin, route: string, request: ServiceRequestInterface) { debug('handler for module "request"') const { transport } = request @@ -33,7 +33,7 @@ function getAction(this: Microfleet & RouterPlugin, route: string, request: Serv return request } -function requestHandler(this: Microfleet & RouterPlugin, route: string, request: ServiceRequest) { +function requestHandler(this: Microfleet & RouterPlugin, route: string, request: ServiceRequestInterface) { const { extensions } = this.router return moduleLifecycle('request', getAction, extensions, [route, request], this) diff --git a/packages/core/src/plugins/router/modules/response.ts b/packages/core/src/plugins/router/modules/response.ts index 1c465df60..198b2cb4c 100644 --- a/packages/core/src/plugins/router/modules/response.ts +++ b/packages/core/src/plugins/router/modules/response.ts @@ -15,7 +15,7 @@ import { ValidationError } from 'common-errors' import { Microfleet } from '../../../' -import { ServiceRequest } from '../../../types' +import { ServiceRequestInterface } from '../../../types' import moduleLifecycle from './lifecycle' function response(this: Microfleet, err: Error | null, result: any) { @@ -57,7 +57,7 @@ function response(this: Microfleet, err: Error | null, result: any) { return Bluebird.resolve(result) } -function responseHandler(this: Microfleet, params: [Error | null, any, ServiceRequest]) { +function responseHandler(this: Microfleet, params: [Error | null, any, ServiceRequestInterface]) { return moduleLifecycle('response', response, this.router.extensions, params, this) } diff --git a/packages/core/src/plugins/router/modules/validate.ts b/packages/core/src/plugins/router/modules/validate.ts index 4ba42716f..d2d89cc39 100644 --- a/packages/core/src/plugins/router/modules/validate.ts +++ b/packages/core/src/plugins/router/modules/validate.ts @@ -4,13 +4,13 @@ import { Error } from 'common-errors' import is = require('is') import { Microfleet } from '../../../' import { DATA_KEY_SELECTOR } from '../../../constants' -import { ServiceRequest } from '../../../types' +import { ServiceRequestInterface } from '../../../types' import { ValidatorPlugin } from '../../validator' import moduleLifecycle from './lifecycle' type ParamsKey = 'query' | 'params' -async function validate(this: Microfleet & ValidatorPlugin, request: ServiceRequest): Promise { +async function validate(this: Microfleet & ValidatorPlugin, request: ServiceRequestInterface): Promise { const { validator } = this const paramsKey: ParamsKey = DATA_KEY_SELECTOR[request.method] @@ -26,11 +26,11 @@ async function validate(this: Microfleet & ValidatorPlugin, request: ServiceRequ } } -function passThrough(request: ServiceRequest): ServiceRequest { +function passThrough(request: ServiceRequestInterface): ServiceRequestInterface { return request } -function validateHandler(this: Microfleet & ValidatorPlugin, request: ServiceRequest): Bluebird { +function validateHandler(this: Microfleet & ValidatorPlugin, request: ServiceRequestInterface): Bluebird { const validateFn = is.undefined(request.action.schema) ? passThrough : validate diff --git a/packages/core/src/plugins/router/routes/generic/health.ts b/packages/core/src/plugins/router/routes/generic/health.ts index 31e68cb55..5222445fd 100644 --- a/packages/core/src/plugins/router/routes/generic/health.ts +++ b/packages/core/src/plugins/router/routes/generic/health.ts @@ -1,11 +1,11 @@ import { HttpStatusError } from 'common-errors' import { PLUGIN_STATUS_FAIL } from '../../../../constants' import { ActionTransport, Microfleet } from '../../../..' -import { ServiceRequest } from '../../../../types' +import { ServiceRequestInterface } from '../../../../types' const kUnhealthy = new HttpStatusError(500, 'unhealthy') -async function genericHealthCheck(this: Microfleet, request: ServiceRequest) { +async function genericHealthCheck(this: Microfleet, request: ServiceRequestInterface) { const data = await this.getHealthStatus() if (PLUGIN_STATUS_FAIL === data.status) { diff --git a/packages/core/src/plugins/socketIO/router/adapter.ts b/packages/core/src/plugins/socketIO/router/adapter.ts index 27f7809be..8dd05d4e1 100644 --- a/packages/core/src/plugins/socketIO/router/adapter.ts +++ b/packages/core/src/plugins/socketIO/router/adapter.ts @@ -1,24 +1,32 @@ import _debug = require('debug') -import noop = require('lodash/noop') -import { ActionTransport } from '../../..' -import { ServiceRequest } from '../../../types' +import { ActionTransport, ServiceRequestInterface } from '../../..' import { Router } from '../../router/factory' import { RequestCallback } from '../../router/dispatcher' +import { createServiceRequest } from './service-request-factory' const debug = _debug('mservice:router:socket.io') const { socketIO } = ActionTransport +export interface SocketIOMessageOptions { + simpleResponse?: boolean; +} + export interface SocketIOMessage { - data: [string, any, RequestCallback]; + data: [string, any, SocketIOMessageOptions, RequestCallback]; } /* Decrease request count on response */ -function wrapCallback(router: Router, callback: RequestCallback) { +function wrapCallback(router: Router, options: SocketIOMessageOptions, serviceRequest: ServiceRequestInterface, callback: RequestCallback) { return (err: any, result?: any) => { router.requestCountTracker.decrease(socketIO) - if (callback) { - callback(err, result) - } + + if (!callback) return + + const response = options.simpleResponse === false + ? { headers: Object.fromEntries(serviceRequest.getReplyHeaders()), data: result } + : result + + callback(err, response) } } @@ -28,26 +36,12 @@ function getSocketIORouterAdapter(_: any, router: Router) { /* Increase request count on message */ router.requestCountTracker.increase(socketIO) - const [actionName, params, callback] = packet.data - const request: ServiceRequest = { - socket, - params, - action: noop as any, - headers: Object.create(null), - locals: Object.create(null), - log: console as any, - method: 'socketio', - parentSpan: undefined, - query: Object.create(null), - route: '', - span: undefined, - transport: socketIO, - transportRequest: packet, - } + const [actionName, params, options, callback] = packet.data + const serviceRequest = createServiceRequest(params, packet, socket) debug('prepared request with', packet.data) - const wrappedCallback = wrapCallback(router, callback) - router.dispatch.call(router, actionName, request, wrappedCallback) + const wrappedCallback = wrapCallback(router, options, serviceRequest, callback) + router.dispatch.call(router, actionName, serviceRequest, wrappedCallback) }) } } diff --git a/packages/core/src/plugins/socketIO/router/service-request-factory.ts b/packages/core/src/plugins/socketIO/router/service-request-factory.ts new file mode 100644 index 000000000..a1c6c5cf6 --- /dev/null +++ b/packages/core/src/plugins/socketIO/router/service-request-factory.ts @@ -0,0 +1,20 @@ +import { ServiceRequest } from '../../../utils/service-request'; +import { ActionTransport, ServiceRequestInterface } from '../../..'; +import { SocketIOMessage } from './adapter'; + +export const createServiceRequest = ( + params: any, + packet: SocketIOMessage, + socket: NodeJS.EventEmitter +): ServiceRequestInterface => ( + new ServiceRequest( + ActionTransport.socketIO, + 'socketio', + Object.create(null), + Object.create(null), + params, + packet, + socket, + undefined, + ) +); diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index a30f2101f..56cbf997e 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -6,7 +6,8 @@ import { DESTRUCTORS_PROPERTY, PLUGIN_STATUS_FAIL, PLUGIN_STATUS_OK, - PluginTypes + PluginTypes, + kReplyHeaders, } from './constants' import { ClientRequest } from 'http' @@ -67,7 +68,7 @@ export type HandlerProperties = typeof CONNECTORS_PROPERTY | typeof DESTRUCTORS_ export type TransportTypes = $Values export type TConnectorsTypes = $Values export type RequestMethods = $Keys -export type GetAuthName = (req: ServiceRequest) => string +export type GetAuthName = (req: ServiceRequestInterface) => string export type ServiceActionStep = (...args: any[]) => PromiseLike export declare interface ServiceAction extends ServiceActionStep { @@ -80,7 +81,21 @@ export declare interface ServiceAction extends ServiceActionStep { readonly?: boolean; } -export declare interface ServiceRequest { +export declare interface ServiceRequestInterface { + // eslint-disable-next-line @typescript-eslint/no-misused-new + new( + transport: TransportTypes, + method: RequestMethods, + query: any, + headers: any, + params: any, + transportRequest: any | ClientRequest, + socket?: NodeJS.EventEmitter, + parentSpan?: any, + ): ServiceRequestInterface; + + (): void; + route: string; params: any; headers: any; @@ -102,6 +117,14 @@ export declare interface ServiceRequest { error(...args: any[]): void; fatal(...args: any[]): void; }; + [kReplyHeaders]: ReplyHeaders; + setReplyHeader: (title: string, value: string) => ServiceRequestInterface; + removeReplyHeader: (title: string) => ServiceRequestInterface; + getReplyHeaders: () => ReplyHeaders; + getReplyHeader?: ReplyHeaderValue; } +export type ReplyHeaderValue = string[]|string +export type ReplyHeaders = Map + export type PluginStatus = typeof PLUGIN_STATUS_OK | typeof PLUGIN_STATUS_FAIL diff --git a/packages/core/src/types/module.d.ts b/packages/core/src/types/module.d.ts index 24562b09d..5c66bddee 100644 --- a/packages/core/src/types/module.d.ts +++ b/packages/core/src/types/module.d.ts @@ -1,4 +1,5 @@ declare module '@microfleet/transport-amqp/lib/utils/serialization' +declare module '@microfleet/transport-amqp/lib/constants' declare module 'sort-by' declare module 'rfdc' declare module 'socketio-wildcard' diff --git a/packages/core/src/utils/service-request.ts b/packages/core/src/utils/service-request.ts new file mode 100644 index 000000000..8dd2b567e --- /dev/null +++ b/packages/core/src/utils/service-request.ts @@ -0,0 +1,58 @@ +import {RequestMethods, TransportTypes, ReplyHeaders, ServiceRequestInterface, ReplyHeaderValue } from '../types' +import { ClientRequest } from 'http' +import noop = require('lodash/noop') +import { kReplyHeaders } from '../constants' + +export const ServiceRequest = function( + this: ServiceRequestInterface, + transport: TransportTypes, + method: RequestMethods, + query: any, + headers: any, + params: any, + transportRequest: any | ClientRequest, + socket?: NodeJS.EventEmitter, + parentSpan?: any, +) { + this.transportRequest = transportRequest + this.transport = transport + this.method = method + this.query = query + this.params = params + this.headers = headers + this.socket = socket + this.parentSpan = parentSpan + + // todo get rid of any + this.log = console as any + this.route = '' + this.action = noop as any + this.auth = Object.create(null) + this.span = undefined + this.locals = Object.create(null) + this[kReplyHeaders] = new Map() +} as ServiceRequestInterface + +ServiceRequest.prototype.getReplyHeader = function (title: string): ReplyHeaders { + return this[kReplyHeaders].get(title.toLowerCase()) +} + +ServiceRequest.prototype.getReplyHeaders = function (): ReplyHeaders { + return this[kReplyHeaders] +} + +ServiceRequest.prototype.hasReplyHeader = function (title: string): boolean { + return this[kReplyHeaders].has(title) +} + +ServiceRequest.prototype.removeReplyHeader = function (title: string): ServiceRequestInterface { + this[kReplyHeaders].delete(title) + + return this +} + +ServiceRequest.prototype.setReplyHeader = function (title: string, value: ReplyHeaderValue): ServiceRequestInterface { + this[kReplyHeaders].set(title.toLowerCase(), value) + + return this +} diff --git a/packages/core/test/amqp/helpers/actions/success-remove-header.js b/packages/core/test/amqp/helpers/actions/success-remove-header.js new file mode 100644 index 000000000..28d71617c --- /dev/null +++ b/packages/core/test/amqp/helpers/actions/success-remove-header.js @@ -0,0 +1,11 @@ +const { ActionTransport } = require('./../../../../src'); + +function SuccessRemoveHeaderAction(serviceRequest) { + serviceRequest.setReplyHeader('x-your-response-header', 'header value'); + serviceRequest.setReplyHeader('x-remove-me', 'not so valuable'); + serviceRequest.removeReplyHeader('x-remove-me'); +} + +SuccessRemoveHeaderAction.transports = [ActionTransport.amqp]; + +module.exports = SuccessRemoveHeaderAction; diff --git a/packages/core/test/amqp/helpers/actions/success-set-header.js b/packages/core/test/amqp/helpers/actions/success-set-header.js new file mode 100644 index 000000000..fa11f6637 --- /dev/null +++ b/packages/core/test/amqp/helpers/actions/success-set-header.js @@ -0,0 +1,9 @@ +const { ActionTransport } = require('./../../../../src'); + +function SuccessSetHeaderAction(serviceRequest) { + serviceRequest.setReplyHeader('x-your-response-header', 'header value'); +} + +SuccessSetHeaderAction.transports = [ActionTransport.amqp]; + +module.exports = SuccessSetHeaderAction; diff --git a/packages/core/test/amqp/helpers/actions/success.js b/packages/core/test/amqp/helpers/actions/success.js new file mode 100644 index 000000000..4a93c84ae --- /dev/null +++ b/packages/core/test/amqp/helpers/actions/success.js @@ -0,0 +1,9 @@ +const { ActionTransport } = require('./../../../../src'); + +function SuccessAction() { + return { redirected: true }; +} + +SuccessAction.transports = [ActionTransport.amqp]; + +module.exports = SuccessAction; diff --git a/packages/core/test/config.js b/packages/core/test/config.js index e68710e53..71ee59b5f 100644 --- a/packages/core/test/config.js +++ b/packages/core/test/config.js @@ -76,16 +76,20 @@ global.SERVICES = { http: { server: { attachSocketIO: true, - handler: 'restify', + handler: 'hapi', handlerConfig: {}, port: 3000, }, + router: { + enabled: true + } }, router: { routes: { directory: path.resolve(__dirname, './socketIO/helpers/actions'), enabled: { echo: 'echo', + 'success-set-header': 'success-set-header', }, transports: [ActionTransport.socketIO], }, diff --git a/packages/core/test/hapi/helpers/actions/success-remove-header.js b/packages/core/test/hapi/helpers/actions/success-remove-header.js new file mode 100644 index 000000000..393b8c91f --- /dev/null +++ b/packages/core/test/hapi/helpers/actions/success-remove-header.js @@ -0,0 +1,11 @@ +const { ActionTransport } = require('./../../../../src'); + +function SuccessRemoveHeaderAction(serviceRequest) { + serviceRequest.setReplyHeader('x-your-response-header', 'header value'); + serviceRequest.setReplyHeader('x-remove-me', 'not so valuable'); + serviceRequest.removeReplyHeader('x-remove-me'); +} + +SuccessRemoveHeaderAction.transports = [ActionTransport.http]; + +module.exports = SuccessRemoveHeaderAction; diff --git a/packages/core/test/hapi/helpers/actions/success-set-header.js b/packages/core/test/hapi/helpers/actions/success-set-header.js new file mode 100644 index 000000000..84e2049cc --- /dev/null +++ b/packages/core/test/hapi/helpers/actions/success-set-header.js @@ -0,0 +1,9 @@ +const { ActionTransport } = require('./../../../../src'); + +function SuccessSetHeaderAction(serviceRequest) { + serviceRequest.setReplyHeader('x-your-response-header', 'header value'); +} + +SuccessSetHeaderAction.transports = [ActionTransport.http]; + +module.exports = SuccessSetHeaderAction; diff --git a/packages/core/test/socketIO/helpers/actions/success-set-header.js b/packages/core/test/socketIO/helpers/actions/success-set-header.js new file mode 100644 index 000000000..33b00f8a0 --- /dev/null +++ b/packages/core/test/socketIO/helpers/actions/success-set-header.js @@ -0,0 +1,9 @@ +const { ActionTransport } = require('./../../../../src'); + +function SuccessSetHeaderAction(serviceRequest) { + serviceRequest.setReplyHeader('x-your-response-header', 'header value'); +} + +SuccessSetHeaderAction.transports = [ActionTransport.socketIO]; + +module.exports = SuccessSetHeaderAction; diff --git a/packages/core/test/suites/amqp.js b/packages/core/test/suites/amqp.js index f458e117c..62572fc13 100644 --- a/packages/core/test/suites/amqp.js +++ b/packages/core/test/suites/amqp.js @@ -1,3 +1,4 @@ +const path = require('path'); const Promise = require('bluebird'); const assert = require('assert'); const sinon = require('sinon'); @@ -91,6 +92,41 @@ describe('AMQP suite: basic routing', function testSuite() { assert.deepStrictEqual(response, { foo: 'bar' }); }); + + it('able to send request and get simple response', async () => { + const { amqp } = service; + const result = await amqp.publishAndWait('success', null); + + assert.deepEqual(result, { redirected: true }); + }); + + it('able to set response header', async () => { + const { amqp } = service; + const result = await amqp.publishAndWait('success-set-header', null, { simpleResponse: false }); + + assert.deepEqual(result, { + headers: { + timeout: 10000, + 'x-your-response-header': 'header value' + }, + data: undefined + }); + }); + + it('able to remove response header', async () => { + const { amqp } = service; + const result = await amqp.publishAndWait('success-remove-header', null, { simpleResponse: false }); + + assert.deepEqual(result, { + headers: { + timeout: 10000, + 'x-your-response-header': 'header value' + }, + data: undefined + }); + + assert.strictEqual(result.headers['x-remove-me'], undefined); + }); }); describe('AMQP suite: prefixed routing', function testSuite() { diff --git a/packages/core/test/suites/http.hapi.js b/packages/core/test/suites/http.hapi.js index 58ced9434..f19060c90 100644 --- a/packages/core/test/suites/http.hapi.js +++ b/packages/core/test/suites/http.hapi.js @@ -65,7 +65,7 @@ describe('Http server with \'hapi\' handler', function testSuite() { await new Promise((resolve, reject) => { const client = SocketIOClient('http://0.0.0.0:3000'); client.on('error', reject); - client.emit('echo', { message: 'foo' }, (error, response) => { + client.emit('echo', { message: 'foo' }, { simpleResponse: true }, (error, response) => { client.close(); assert.equal(error, null); assert.deepEqual(response, { message: 'foo' }); @@ -326,6 +326,67 @@ describe('Http server with \'hapi\' handler', function testSuite() { } }); + describe('should support editing response headers', async () => { + before(async () => { + service = new Microfleet({ + name: 'tester', + plugins: ['validator', 'logger', 'opentracing', 'router', 'http'], + http: { + server: { + handler: 'hapi', + port: 3000, + }, + router: { + enabled: true, + }, + }, + logger: { + defaultLogger: true, + }, + router: { + routes: { + directory: path.resolve(__dirname, './../hapi/helpers/actions'), + enabled: { + 'success-set-header': 'success-set-header', + 'success-remove-header': 'success-remove-header', + }, + transports: ['http'], + }, + extensions: { register: [] }, + }, + }); + + await service.connect(); + }); + + after(() => service.close()); + + it('should be able to set header', async () => { + const response = await request({ + method: 'POST', + resolveWithFullResponse: true, + simple: false, + uri: 'http://0.0.0.0:3000/success-set-header', + body: '', + }); + + assert.strictEqual(response.headers['x-your-response-header'], 'header value'); + }); + + it('should be able to remove header', async () => { + const response = await request({ + method: 'POST', + resolveWithFullResponse: true, + simple: false, + uri: 'http://0.0.0.0:3000/success-remove-header', + body: '', + }); + + assert.strictEqual(response.headers['x-remove-me'], undefined); + assert.strictEqual(response.headers['x-your-response-header'], 'header value'); + }) + }); + describe('should be able to use hapi\'s plugins', async () => { before(async () => { service = new Microfleet({ diff --git a/packages/core/test/suites/socketIO.js b/packages/core/test/suites/socketIO.js index d858beacd..4e249f31c 100644 --- a/packages/core/test/suites/socketIO.js +++ b/packages/core/test/suites/socketIO.js @@ -9,7 +9,7 @@ describe('"socketIO" plugin', function testSuite() { require('../config'); const { Microfleet } = require('../..'); - it('should throw error when plugin isn\'t included', function test() { + it('should not init socketIO when plugin is not included', function test() { const service = new Microfleet({ name: 'tester', plugins: [] }); assert(!service.socketIO); }); @@ -32,10 +32,11 @@ describe('"socketIO" plugin', function testSuite() { }); service.socketIO.listen(3000); const client = socketIOClient('http://0.0.0.0:3000'); - client.emit('echo', { message: 'foo' }, (error, response) => { + client.emit('echo', { message: 'foo' }, { simpleResponse: true }, (error, response) => { expect(error).to.be.equals(null); expect(response).to.be.deep.equals({ message: 'foo' }); - Promise.resolve(service.close()).asCallback(done); + service.socketIO.close(); + done(); }); }); @@ -61,4 +62,27 @@ describe('"socketIO" plugin', function testSuite() { expect(service.socketIO.sockets.adapter.transport).to.be.instanceof(AdapterTransport); done(); }); + + it('should be able to set response header', (done) => { + const service = new Microfleet({ + name: 'tester', + plugins: ['validator', 'logger', 'router', 'socketIO'], + socketIO: global.SERVICES.socketIO, + router: global.SERVICES.router, + }); + + service.socketIO.listen(3000); + const client = socketIOClient('http://0.0.0.0:3000'); + client.emit('success-set-header', {}, { simpleResponse: false }, (error, response) => { + expect(error).to.be.equals(null); + expect(response).to.be.deep.equals({ + headers: { + 'x-your-response-header': 'header value', + } + }); + + service.socketIO.close(); + done(); + }); + }); }); From c439ca8962dc505ebbbb119ed582b7200e031e50 Mon Sep 17 00:00:00 2001 From: BelkinaDasha Date: Mon, 27 Apr 2020 19:26:45 +0500 Subject: [PATCH 2/6] feat(reply headers): rfc --- packages/core/docs/rfc/service-request.md | 43 ++++++++++++++--------- 1 file changed, 26 insertions(+), 17 deletions(-) diff --git a/packages/core/docs/rfc/service-request.md b/packages/core/docs/rfc/service-request.md index a340f6dfb..d774317ec 100644 --- a/packages/core/docs/rfc/service-request.md +++ b/packages/core/docs/rfc/service-request.md @@ -116,27 +116,37 @@ deliver headers to original reply message. #### `.setReplyHeader(title: string, value: number|string|array): void` Sets reply header to the map. -Must normalizes title. +Must normalize title. Must validate and cast value. -Headers considerations: -- HTTP: When value is array, joins values by semicolon -- HTTP: *`Set-Cookie` must not be joined.* https://tools.ietf.org/html/rfc7230#section-3.2.2 (Hapi handles it correctly, -but we do not proxy set header calls, we collect them, that's why we allow raw array of strings as a value) -- HTTP: Should allow both 'x-' prefixed and not prefixed headers - -Questions: -- AMQP: Reject headers starting with anything except 'x-' not let it to be overriden? What about `x-death`? -- Internal: Is there any sense of implementing headers for internal transport? +If normalized `title` is `set-cookie`, it's being appended to previously set `set-cookie` value. +Else it`s value is being overridden: -Usage: ```js function actionHandler(serviceRequest) { - const { state } = this.config.cookie serviceRequest.setReplyHeader('x-rate-limit', 10000) - serviceRequest.setReplyHeader('set-cookie', [...state]) + serviceRequest.setReplyHeader('x-rate-limit', 20000) + + serviceRequest.setReplyHeader('set-cookie', 'state=sg687gjjsdg69847gjsh; Domain=app.com; Secure; HttpOnly') + serviceRequest.setReplyHeader('set-cookie', 'visitor=798; Domain=app.com; Secure; SameSite=Lax') + + serviceRequest.setReplyHeader('x-location', 'lat=58.259624, lng=55.919243') + serviceRequest.setReplyHeader('X-LOCATION', 'lat=64.547589, lng=39.758303') +} +``` + +This will make the reply headers map be like: +``` +Map { + 'set-cookie' => [ + 'state=sg687gjjsdg69847gjsh; Domain=app.com; Secure; HttpOnly', + 'visitor=798; Domain=app.com; Secure; SameSite=Lax' + ], + 'x-rate-limit' => '20000', + 'x-location' => 'lat=64.547589, lng=39.758303', } ``` +Which will result in two different header lines for HTTP transport. #### `.removeReplyHeader(title: string): void` Normalizes title and removes header from headers map. @@ -149,7 +159,7 @@ function actionHandler(serviceRequest) { } ``` -#### `.getReplyHeaders(): Map` +#### `.getReplyHeaders(): Map` Gets all previously initialized headers from headers map. Usage: @@ -160,7 +170,7 @@ function actionHandler(serviceRequest) { } ``` -#### `.getReplyHeader(title: string): number|string|string[]` +#### `.getReplyHeader(title: string): string|string[]` Gets header from map. Must normalize title. @@ -171,8 +181,7 @@ Extract headers collection and set it under `kReplyHeaders` key of `raw.properti defined by `@microfleet/transport-amqp` library. ### HTTP Transport -Consider HTTP RFC relative to [`set-cookie` header](https://tools.ietf.org/html/rfc7230#section-3.2.2). Since this is -common thing for any HTTP handler, it should be implemented in a composable way. +Consider HTTP RFC relative to [`set-cookie` header](https://tools.ietf.org/html/rfc7230#section-3.2.2). #### Hapi Handler Extract headers collection and set it to the response using Hapi Response Toolkit. From 63d0e2572511eec52fa6b3d335c68f47d9ea3c10 Mon Sep 17 00:00:00 2001 From: BelkinaDasha Date: Wed, 29 Apr 2020 23:42:47 +0500 Subject: [PATCH 3/6] fix(reply headers): tests --- .../core/src/plugins/socketIO/router/adapter.ts | 13 +++++++++---- .../core/test/router/helpers/requests/socketIO.js | 2 +- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/packages/core/src/plugins/socketIO/router/adapter.ts b/packages/core/src/plugins/socketIO/router/adapter.ts index 8dd05d4e1..1f844a4c6 100644 --- a/packages/core/src/plugins/socketIO/router/adapter.ts +++ b/packages/core/src/plugins/socketIO/router/adapter.ts @@ -15,14 +15,19 @@ export interface SocketIOMessage { data: [string, any, SocketIOMessageOptions, RequestCallback]; } -/* Decrease request count on response */ -function wrapCallback(router: Router, options: SocketIOMessageOptions, serviceRequest: ServiceRequestInterface, callback: RequestCallback) { +function wrapCallback( + router: Router, + serviceRequest: ServiceRequestInterface, + options: SocketIOMessageOptions, + callback: RequestCallback +) { return (err: any, result?: any) => { + // Decrease request count on response router.requestCountTracker.decrease(socketIO) if (!callback) return - const response = options.simpleResponse === false + const response = options && options.simpleResponse === false ? { headers: Object.fromEntries(serviceRequest.getReplyHeaders()), data: result } : result @@ -40,7 +45,7 @@ function getSocketIORouterAdapter(_: any, router: Router) { const serviceRequest = createServiceRequest(params, packet, socket) debug('prepared request with', packet.data) - const wrappedCallback = wrapCallback(router, options, serviceRequest, callback) + const wrappedCallback = wrapCallback(router, serviceRequest, options, callback) router.dispatch.call(router, actionName, serviceRequest, wrappedCallback) }) } diff --git a/packages/core/test/router/helpers/requests/socketIO.js b/packages/core/test/router/helpers/requests/socketIO.js index 8c95c1baa..180c9ca0b 100644 --- a/packages/core/test/router/helpers/requests/socketIO.js +++ b/packages/core/test/router/helpers/requests/socketIO.js @@ -1,7 +1,7 @@ const Promise = require('bluebird'); function getSocketIORequest(client) { - return (action, params) => Promise.fromCallback((callback) => client.emit(action, params, callback)); + return (action, params, options) => Promise.fromCallback((callback) => client.emit(action, params, options, callback)); } module.exports = getSocketIORequest; From 8c3a5d2ef898a705fa7edfe5f31b762dfc7831d4 Mon Sep 17 00:00:00 2001 From: BelkinaDasha Date: Thu, 30 Apr 2020 00:22:46 +0500 Subject: [PATCH 4/6] feat(reply headers): update rfc --- packages/core/docs/rfc/service-request.md | 48 +++++++++++++---------- packages/core/src/constants.ts | 2 +- packages/core/src/plugins/router.ts | 25 ++++++++---- 3 files changed, 46 insertions(+), 29 deletions(-) diff --git a/packages/core/docs/rfc/service-request.md b/packages/core/docs/rfc/service-request.md index d774317ec..f606981b8 100644 --- a/packages/core/docs/rfc/service-request.md +++ b/packages/core/docs/rfc/service-request.md @@ -2,27 +2,33 @@ ## Overview and Motivation The Service handles incoming messages via several action Transports. On each incoming message Transport Router Adapter -builds a Service Request instance and passes it to the Dispatcher. This instance becomes available in the Action Handler -as an argument. Its structure is abstract and functionality is basically transport-agnostic. +builds an object with `ServiceRequest` type and passes it to the Dispatcher. There is no common constructor function now. +This object becomes available in the Action Handler as an argument. Its structure is abstract and functionality is +basically transport-agnostic. Most transport protocols natively have headers in messages. While currently every Transport Router Adapter is able to parse incoming messages including its optional headers, in order to fully support messaging protocols the Service should -provide a way to set, modify and remove response message headers. +provide a way to set, modify and remove response message headers. That's why we need to implement **common reply headers +API**. -Although every transport could have its own protocol limitations (considerations) over the headers, the Service Request -instance could only take responsibility for collecting them. In order to be able to validate and adapt resulting -collection independently, these tasks should be performed on the Transport level. +Considering Node V8 hidden classes concept, to minimize hidden class trees number and respectfully maximize the +performance, the Service Request instance must always aim to have same object shape and its properties initialization +order. In order to make it even more performant we have to choose functions over ES6 classes for the Service Request +implementation. Due to that we need to use **single constructor function** to instantiate Service Request objects +anywhere. -Considering Node V8 hidden classes concept, in order to minimize hidden class trees number and respectfully maximize the -performance, the Service Request instance must always aim to have same object shape and its properties initialization -order. -Our secret Node.JS performance expert claims that functions work even faster regarding hidden class optimization than -ES6 classes. -To meet these requirements the Service Request must be initialized using functions and preset every property value on -construction. +Developer experience could be improved in two ways: early headers validation and **strict and common validation policy**. +Early validation will help a developer to avoid errors on transport level. Since the limitations may be different, we +must comply with the strictest among all transports validation rules, so that any Action Handler could start to support +each of the Transports effortlessly. + + +In order to respect backwards compatibility, we should be able to choose whether the reply will contain **only +message body or include headers** as well. This will be resolved with boolean **simpleResponse** option on the Transport +Plugin level, which value must be `true` by default. ## Service Request Interface @@ -71,8 +77,8 @@ May be set by Transport Router Adapter. Router must set Log child instance with a unique Request identifier. #### `.socket?: NodeJS.EventEmitter` -In order to provide web sockets protocol support we need to operate on socket instance... But could we live without it? -Can we manage it through service request extensions mechanism? +In order to provide web sockets protocol support we need to operate on socket instance. It should be set whenever +socket instance is available. #### `.parentSpan` When a Tracer is enabled, property may hold a tracer parent span, which context must be supplied by the Transport. @@ -176,6 +182,7 @@ Gets header from map. Must normalize title. ## Implementation design + ### AMQP Transport Extract headers collection and set it under `kReplyHeaders` key of `raw.properties`, where `kReplyHeaders` is a Symbol defined by `@microfleet/transport-amqp` library. @@ -192,9 +199,9 @@ Expect new optional argument `options` to be passed on `.emit`: .emit(action: string, body: any, options: SocketIOMessageOptions, callback: RequestCallback): void ``` -When present, options may contain `simpleResponse` setting, which defaults to true. -When `simpleResponse` option is disabled, callback `result` must be resolved with `data` containing response message, -and `headers` containing headers that had user could set. +When present, options may contain `simpleResponse` setting. +When `simpleResponse` option is disabled, callback `result` must be resolved with `data` containing reply message, +and `headers` containing headers. ``` { headers: { [key: string]: string }, data?: unknown } @@ -208,6 +215,5 @@ client.emit('echo', { message: 'ok' }, { simpleResponse: false }, (error, result ``` ### Internal Transport -If there any sense of implementing internal transport headers, its returning Promise may also be resolved with `data` -and `headers`. However, I don't see the `dispatch` method argument list extended by some response options like -`simpleResponse`, because it does not seem like an appropriate place for that. +Router plugin exposes two `dispatch` methods to the Service: `.dispatch()` and `.router.dispatch()`. +Extend Router plugin `.dispatch` method signature with optional `options` argument to pass `simpleResponse` flag. diff --git a/packages/core/src/constants.ts b/packages/core/src/constants.ts index a6bec95bb..d72722c2d 100644 --- a/packages/core/src/constants.ts +++ b/packages/core/src/constants.ts @@ -97,4 +97,4 @@ export const PluginsPriority = [ export const PLUGIN_STATUS_OK = 'ok' export const PLUGIN_STATUS_FAIL = 'fail' -export const kReplyHeaders = Symbol('replyHeaders') +export const kReplyHeaders = Symbol.for('microfleet:replyHeaders') diff --git a/packages/core/src/plugins/router.ts b/packages/core/src/plugins/router.ts index d2660a5d1..5633512a2 100644 --- a/packages/core/src/plugins/router.ts +++ b/packages/core/src/plugins/router.ts @@ -1,12 +1,13 @@ +import Bluebird = require('bluebird') import assert = require('assert') import rfdc = require('rfdc') import { NotFoundError, NotSupportedError } from 'common-errors' +import { object as isObject } from 'is' import { ActionTransport, PluginTypes, identity } from '../constants' import { Microfleet } from '../' import { ServiceRequestInterface } from '../types' import { getRouter, Router, RouterConfig, LifecycleRequestType } from './router/factory' import { ValidatorPlugin } from './validator' -import { object as isObject } from 'is' import { ServiceRequest } from '../utils/service-request'; const { internal } = ActionTransport @@ -16,6 +17,9 @@ const { internal } = ActionTransport */ export const name = 'router' export { Router, RouterConfig, LifecycleRequestType } +export interface DispatchOptionsInterface { + simpleResponse?: boolean +} /** * Defines extension points of @@ -23,7 +27,7 @@ export { Router, RouterConfig, LifecycleRequestType } */ export interface RouterPlugin { router: Router; - dispatch: (route: string, request: Partial) => PromiseLike; + dispatch: (route: string, request: Partial, options?: DispatchOptionsInterface) => PromiseLike; } /** @@ -87,9 +91,16 @@ export function attach(this: Microfleet & ValidatorPlugin & RouterPlugin, opts: ? (route: string) => `${prefix}.${route}` : identity - // dispatcher - this.dispatch = (route: string, request: Partial) => { - const msg = prepareRequest(request) - return router.dispatch(assemble(route), msg) - } + // internal dispatcher + const dispatch = async (route: string, request: Partial, options?: DispatchOptionsInterface) => { + const serviceRequest = prepareRequest(request) + const data = await router.dispatch(assemble(route), serviceRequest) + const includeHeaders = options && options.simpleResponse === false; + + return includeHeaders + ? { data, headers: Object.fromEntries(serviceRequest.getReplyHeaders()) } + : data; + }; + + this.dispatch = Bluebird.method(dispatch) } From 01787410c9473f979c3468cb3bd39eda157ee7bd Mon Sep 17 00:00:00 2001 From: BelkinaDasha Date: Thu, 30 Apr 2020 00:37:32 +0500 Subject: [PATCH 5/6] feat(reply headers): update rfc --- packages/core/docs/rfc/service-request.md | 9 ++++++++- packages/core/src/utils/service-request.ts | 14 +++++++++++++- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/packages/core/docs/rfc/service-request.md b/packages/core/docs/rfc/service-request.md index f606981b8..2b1f212ef 100644 --- a/packages/core/docs/rfc/service-request.md +++ b/packages/core/docs/rfc/service-request.md @@ -123,7 +123,8 @@ deliver headers to original reply message. Sets reply header to the map. Must normalize title. -Must validate and cast value. +Must cast numeric value to string. +Must validate title and value. Must throw exception if any of arguments is invalid. If normalized `title` is `set-cookie`, it's being appended to previously set `set-cookie` value. Else it`s value is being overridden: @@ -181,6 +182,12 @@ Gets header from map. Must normalize title. +### `.isValidReplyHeader(title: string, value: string|string[]): boolean` +Must validate title and value. +Validation rules are: +* Is ASCII +Empty strings are allowed. + ## Implementation design ### AMQP Transport diff --git a/packages/core/src/utils/service-request.ts b/packages/core/src/utils/service-request.ts index 8dd2b567e..69b693cf8 100644 --- a/packages/core/src/utils/service-request.ts +++ b/packages/core/src/utils/service-request.ts @@ -1,8 +1,11 @@ -import {RequestMethods, TransportTypes, ReplyHeaders, ServiceRequestInterface, ReplyHeaderValue } from '../types' +import assert from 'assert' import { ClientRequest } from 'http' import noop = require('lodash/noop') +import {RequestMethods, TransportTypes, ReplyHeaders, ServiceRequestInterface, ReplyHeaderValue } from '../types' import { kReplyHeaders } from '../constants' +// const isASCII = string => /^[\x00-\x7F]*$/.test(string); + export const ServiceRequest = function( this: ServiceRequestInterface, transport: TransportTypes, @@ -33,6 +36,13 @@ export const ServiceRequest = function( this[kReplyHeaders] = new Map() } as ServiceRequestInterface +// @ts-ignore-line +ServiceRequest.prototype.isValidReplyHeader = function(title: string, value: ReplyHeaderValue): boolean { + // todo implement + // check if is ASCII + return true; +} + ServiceRequest.prototype.getReplyHeader = function (title: string): ReplyHeaders { return this[kReplyHeaders].get(title.toLowerCase()) } @@ -52,6 +62,8 @@ ServiceRequest.prototype.removeReplyHeader = function (title: string): ServiceRe } ServiceRequest.prototype.setReplyHeader = function (title: string, value: ReplyHeaderValue): ServiceRequestInterface { + assert(this.isValidReplyHeader(title, value), 'Reply header is invalid'); + this[kReplyHeaders].set(title.toLowerCase(), value) return this From 3d4ff6557ef117fae641c6bc71f35f0bbfbcf644 Mon Sep 17 00:00:00 2001 From: BelkinaDasha Date: Thu, 30 Apr 2020 00:40:09 +0500 Subject: [PATCH 6/6] feat(reply headers): update rfc --- packages/core/docs/rfc/service-request.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/core/docs/rfc/service-request.md b/packages/core/docs/rfc/service-request.md index 2b1f212ef..18f85d2a2 100644 --- a/packages/core/docs/rfc/service-request.md +++ b/packages/core/docs/rfc/service-request.md @@ -207,8 +207,8 @@ Expect new optional argument `options` to be passed on `.emit`: ``` When present, options may contain `simpleResponse` setting. -When `simpleResponse` option is disabled, callback `result` must be resolved with `data` containing reply message, -and `headers` containing headers. +When `simpleResponse` option value is `false`, callback `result` must be resolved with `data` containing reply message, +and `headers` containing headers. Otherwise, callback `result` must be resolved with reply message. ``` { headers: { [key: string]: string }, data?: unknown } @@ -224,3 +224,5 @@ client.emit('echo', { message: 'ok' }, { simpleResponse: false }, (error, result ### Internal Transport Router plugin exposes two `dispatch` methods to the Service: `.dispatch()` and `.router.dispatch()`. Extend Router plugin `.dispatch` method signature with optional `options` argument to pass `simpleResponse` flag. +When `simpleResponse` option value is `false`, callback `result` must be resolved with `data` containing reply message, +and `headers` containing headers. Otherwise, callback `result` must be resolved with reply message.