From 5bcefde5bdf57694a972ed635d6d4e81d582608c Mon Sep 17 00:00:00 2001 From: mrfrase3 Date: Fri, 17 Nov 2023 14:30:23 +0800 Subject: [PATCH] initial dynamic method definitions --- packages/express/src/rest.ts | 18 ++- packages/express/test/rest.test.ts | 21 ++- packages/feathers/src/declarations.ts | 27 +++- packages/feathers/src/hooks.ts | 4 +- packages/feathers/src/service.ts | 144 +++++++++++++++--- packages/koa/src/rest.ts | 18 ++- packages/koa/test/index.test.ts | 10 +- packages/rest-client/src/index.ts | 12 +- packages/socketio-client/src/index.ts | 6 +- packages/tests/src/fixture.ts | 43 ++++++ packages/tests/src/rest.ts | 103 ++++++++++++- packages/transport-commons/src/client.ts | 9 +- packages/transport-commons/src/http.ts | 71 ++++++--- .../transport-commons/src/routing/index.ts | 2 + .../transport-commons/src/socket/index.ts | 2 +- .../transport-commons/src/socket/utils.ts | 8 +- packages/transport-commons/test/http.test.ts | 16 +- 17 files changed, 419 insertions(+), 95 deletions(-) diff --git a/packages/express/src/rest.ts b/packages/express/src/rest.ts index 6b4cc1c9d9..c466c808e1 100644 --- a/packages/express/src/rest.ts +++ b/packages/express/src/rest.ts @@ -22,24 +22,28 @@ const serviceMiddleware = (): RequestHandler => { // eslint-disable-next-line @typescript-eslint/no-non-null-assertion const { service, params: { __id: id = null, ...route } = {} } = req.lookup! - const method = http.getServiceMethod(httpMethod, id, methodOverride) - const { methods } = getServiceOptions(service) + const method = http.getServiceMethod(httpMethod, id, route.__action, service, methodOverride) debug(`Found service for path ${path}, attempting to run '${method}' service method`) - if (!methods.includes(method) || defaultServiceMethods.includes(methodOverride)) { - const error = new MethodNotAllowed(`Method \`${method}\` is not supported by this endpoint.`) + if (!method || defaultServiceMethods.includes(methodOverride)) { + if (!methodOverride && id) return next() + const error = new MethodNotAllowed( + `${ + methodOverride ? `Method \`${methodOverride || ``}\`` : `${httpMethod} ${path}` + } is not supported by this endpoint.` + ) res.statusCode = error.code throw error } - const createArguments = http.argumentsFor[method as 'get'] || http.argumentsFor.default + const createArguments = http.argumentsFor(method) const params = { query, headers, route, ...req.feathers } const args = createArguments({ id, data, params }) - const contextBase = createContext(service, method, { http: {} }) + const contextBase = createContext(service, method.key, { http: {} }) res.hook = contextBase - const context = await (service as any)[method](...args, contextBase) + const context = await (service as any)[method.key](...args, contextBase) res.hook = context const response = http.getResponse(context) diff --git a/packages/express/test/rest.test.ts b/packages/express/test/rest.test.ts index ddc9ff8574..e822d642c2 100644 --- a/packages/express/test/rest.test.ts +++ b/packages/express/test/rest.test.ts @@ -5,7 +5,7 @@ import axios, { AxiosRequestConfig } from 'axios' import { Server } from 'http' import { Request, Response, NextFunction } from 'express' import { ApplicationHookMap, feathers, HookContext, Id, Params } from '@feathersjs/feathers' -import { Service, restTests } from '@feathersjs/tests' +import { Service, restTests, customDefs } from '@feathersjs/tests' import { BadRequest } from '@feathersjs/errors' import * as express from '../src' @@ -63,7 +63,7 @@ describe('@feathersjs/express/rest provider', () => { const data = { fromHandler: true } app - .configure(rest(null)) + .configure(rest(undefined)) .use('/todo', { async get(id: Id) { return { @@ -155,7 +155,7 @@ describe('@feathersjs/express/rest provider', () => { } }, function (_req: Request, res: Response, next: NextFunction) { - res.data = convertHook(res.hook) + res.data = convertHook(res.hook as HookContext) next() } @@ -219,7 +219,7 @@ describe('@feathersjs/express/rest provider', () => { app.service('hook-status').hooks({ after(hook: HookContext) { - hook.http.status = 206 + if (hook.http) hook.http.status = 206 } }) @@ -237,7 +237,7 @@ describe('@feathersjs/express/rest provider', () => { app.service('hook-headers').hooks({ after(hook: HookContext) { - hook.http.headers = { foo: 'first', bar: ['second', 'third'] } + if (hook.http) hook.http.headers = { foo: 'first', bar: ['second', 'third'] } } }) @@ -262,7 +262,7 @@ describe('@feathersjs/express/rest provider', () => { app.use(function (error: Error, _req: Request, res: Response, _next: NextFunction) { res.status(500) res.json({ - hook: convertHook(res.hook), + hook: convertHook(res.hook as HookContext), error: { message: error.message } @@ -562,7 +562,7 @@ describe('@feathersjs/express/rest provider', () => { assert.deepStrictEqual( error.response.data, { - message: 'Method `create` is not supported by this endpoint.' + message: 'POST /todo is not supported by this endpoint.' }, 'Error serialized as expected' ) @@ -594,7 +594,7 @@ describe('@feathersjs/express/rest provider', () => { .configure(rest()) .use('/:appId/:id/todo', { async get(id: Id, params: Params) { - if (params.query.error) { + if (params.query?.error) { throw new BadRequest('Not good') } @@ -655,6 +655,9 @@ describe('@feathersjs/express/rest provider', () => { .use('/todo', new Service(), { methods: ['find', 'customMethod'] }) + .use('/tasks', new Service(), { + methods: ['find', 'get', 'create', 'update', 'patch', 'remove', 'customMethod', ...customDefs] + }) .use(errorHandler) server = await app.listen(4781) @@ -711,5 +714,7 @@ describe('@feathersjs/express/rest provider', () => { await testMethod('create') await testMethod('find') }) + + restTests('Custom Method Routes', 'tasks', 4781, true) }) }) diff --git a/packages/feathers/src/declarations.ts b/packages/feathers/src/declarations.ts index aece46eebd..ca4731efa4 100644 --- a/packages/feathers/src/declarations.ts +++ b/packages/feathers/src/declarations.ts @@ -20,6 +20,30 @@ export interface Paginated { data: T[] } +/** + * The arguments for a service method call + */ +export type MethodArgument = 'id' | 'data' | 'params' + +/** + * The definition of a service method + * @property key The name of the method + * @property id Whether the method accepts an id as the first parameter + * @property data Whether the method accepts data as the first or second parameter (depending on id) + * @property route The route that this method should be registered on, true will automatically use the method key & args, false will disable the route + * @property routeMethod The HTTP method that this method should be registered on + * @property eventName The event name that should be emitted when this method is called + */ +export interface MethodDefinition { + key: string + // args: MethodArgument[] + id?: boolean + data?: boolean + route?: string | boolean + routeMethod?: string + eventName?: string +} + /** * Options that can be passed when registering a service via `app.use(name, service, options)` */ @@ -31,7 +55,8 @@ export interface ServiceOptions { /** * A list of service methods that should be available __externally__ to clients */ - methods?: MethodTypes[] | readonly MethodTypes[] + methods?: (MethodTypes | MethodDefinition)[] | readonly MethodTypes[] + serviceMethods?: MethodDefinition[] /** * Provide a full list of events that this service should emit to clients. * Unlike the `events` option, this will not be merged with the default events. diff --git a/packages/feathers/src/hooks.ts b/packages/feathers/src/hooks.ts index 758ab33251..907f60f6ea 100644 --- a/packages/feathers/src/hooks.ts +++ b/packages/feathers/src/hooks.ts @@ -17,7 +17,7 @@ import { HookFunction, HookType } from './declarations' -import { defaultServiceArguments, getHookMethods } from './service' +import { getServiceMethodArgs, getHookMethods } from './service' type ConvertedMap = { [type in HookType]: ReturnType } @@ -186,7 +186,7 @@ export function hookMixin(this: A, service: FeathersService, path: string, const hookMethods = getHookMethods(service, options) const serviceMethodHooks = hookMethods.reduce((res, method) => { - const params = (defaultServiceArguments as any)[method] || ['data', 'params'] + const params = getServiceMethodArgs(method, service) res[method] = new FeathersHookManager(this, method).params(...params).props({ app: this, diff --git a/packages/feathers/src/service.ts b/packages/feathers/src/service.ts index eb852f39b9..86599da15e 100644 --- a/packages/feathers/src/service.ts +++ b/packages/feathers/src/service.ts @@ -1,25 +1,81 @@ import { EventEmitter } from 'events' import { createSymbol } from '@feathersjs/commons' -import { ServiceOptions } from './declarations' +import { ServiceOptions, MethodDefinition, MethodArgument } from './declarations' export const SERVICE = createSymbol('@feathersjs/service') -export const defaultServiceArguments = { - find: ['params'], - get: ['id', 'params'], - create: ['data', 'params'], - update: ['id', 'data', 'params'], - patch: ['id', 'data', 'params'], - remove: ['id', 'params'] -} -export const defaultServiceMethods = ['find', 'get', 'create', 'update', 'patch', 'remove'] +// export const defaultServiceArguments = { +// find: ['params'], +// get: ['id', 'params'], +// create: ['data', 'params'], +// update: ['id', 'data', 'params'], +// patch: ['id', 'data', 'params'], +// remove: ['id', 'params'] +// } as Record -export const defaultEventMap = { - create: 'created', - update: 'updated', - patch: 'patched', - remove: 'removed' -} +export const defaultServiceMethodDefinitions: MethodDefinition[] = [ + { + key: 'find', + // args: defaultServiceArguments.find, + id: false, + data: false, + route: '', + routeMethod: 'GET' + }, + { + key: 'get', + // args: defaultServiceArguments.get, + id: true, + data: false, + route: '', + routeMethod: 'GET' + }, + { + key: 'create', + // args: defaultServiceArguments.create, + id: false, + data: true, + route: '', + routeMethod: 'POST', + eventName: 'created' + }, + { + key: 'update', + // args: defaultServiceArguments.update, + id: true, + data: true, + route: '', + routeMethod: 'PUT', + eventName: 'updated' + }, + { + key: 'patch', + // args: defaultServiceArguments.patch, + id: true, + data: true, + route: '', + routeMethod: 'PATCH', + eventName: 'patched' + }, + { + key: 'remove', + // args: defaultServiceArguments.remove, + id: true, + data: false, + route: '', + routeMethod: 'DELETE', + eventName: 'removed' + } +] + +export const defaultServiceMethods = defaultServiceMethodDefinitions.map((def) => def.key) + +export const defaultEventMap = defaultServiceMethodDefinitions + .filter((def) => !!def.eventName) + .reduce((result, { key, eventName }) => { + result[key] = eventName + return result + }, {} as Record) export const defaultServiceEvents = Object.values(defaultEventMap) @@ -30,9 +86,22 @@ export const protectedMethods = Object.keys(Object.prototype) export function getHookMethods(service: any, options: ServiceOptions) { const { methods } = options - return (defaultServiceMethods as any as string[]) + return defaultServiceMethods .filter((m) => typeof service[m] === 'function' && !methods.includes(m)) - .concat(methods) + .concat(methods.map((m) => (typeof m === 'string' ? m : m.key))) +} + +export function getServiceMethodArgs(method: string | MethodDefinition, service?: any) { + let methodDef = method as MethodDefinition | undefined + if (typeof method === 'string') { + if (!service) { + throw new Error(`Service must be provided if method is a string`) + } + const serviceOptions = getServiceOptions(service) + methodDef = serviceOptions.serviceMethods?.find((def) => def.key === method) + } + const args = [methodDef?.id && 'id', (methodDef?.data ?? true) && 'data', 'params'] + return args.filter((arg) => arg) as MethodArgument[] } export function getServiceOptions(service: any): ServiceOptions { @@ -40,16 +109,45 @@ export function getServiceOptions(service: any): ServiceOptions { } export const normalizeServiceOptions = (service: any, options: ServiceOptions = {}): ServiceOptions => { - const { - methods = defaultServiceMethods.filter((method) => typeof service[method] === 'function'), - events = service.events || [] - } = options + const { events = service.events || [] } = options const serviceEvents = options.serviceEvents || defaultServiceEvents.concat(events) + const serviceMethods = (options.methods || defaultServiceMethodDefinitions) + .map((def) => { + if (typeof def === 'string') { + return ( + defaultServiceMethodDefinitions.find((d) => d.key === def) || { + key: def, + id: false, + data: true, + route: false + } + ) + } + if (typeof def.key !== 'string') { + throw new Error(`Invalid method configuration for ${service.name || 'service'} service`) + } + const defaultDef = defaultServiceMethodDefinitions.find((d) => d.key === def.key) + const data = def.data ?? defaultDef?.data ?? true + return { + key: def.key, + id: def.id ?? defaultDef?.id ?? false, + data, + route: def.route ?? false, + routeMethod: def.routeMethod || (data ? 'POST' : 'GET'), + eventName: def.eventName || defaultEventMap[def.key] + } as MethodDefinition + }) + .filter(({ key }) => typeof service[key] === 'function') + return { ...options, events, - methods, + methods: serviceMethods.reduce((acc, { key }) => { + if (!acc.includes(key)) acc.push(key) + return acc + }, [] as string[]), + serviceMethods, serviceEvents } } diff --git a/packages/koa/src/rest.ts b/packages/koa/src/rest.ts index 8bd08dd339..ec0c48e0c3 100644 --- a/packages/koa/src/rest.ts +++ b/packages/koa/src/rest.ts @@ -16,24 +16,28 @@ const serviceMiddleware = (): Middleware => { // eslint-disable-next-line @typescript-eslint/no-non-null-assertion const { service, params: { __id: id = null, ...route } = {} } = ctx.lookup! - const method = http.getServiceMethod(httpMethod, id, methodOverride) - const { methods } = getServiceOptions(service) + const method = http.getServiceMethod(httpMethod, id, route.__action, service, methodOverride) debug(`Found service for path ${path}, attempting to run '${method}' service method`) - if (!methods.includes(method) || defaultServiceMethods.includes(methodOverride)) { - const error = new MethodNotAllowed(`Method \`${method}\` is not supported by this endpoint.`) + if (!method || defaultServiceMethods.includes(methodOverride)) { + if (!methodOverride && id) return next() + const error = new MethodNotAllowed( + `${ + methodOverride ? `Method \`${methodOverride || ``}\`` : `${httpMethod} ${path}` + } is not supported by this endpoint.` + ) ctx.response.status = error.code throw error } - const createArguments = http.argumentsFor[method as 'get'] || http.argumentsFor.default + const createArguments = http.argumentsFor(method) const params = { query, headers, route, ...ctx.feathers } const args = createArguments({ id, data, params }) - const contextBase = createContext(service, method, { http: {} }) + const contextBase = createContext(service, method.key, { http: {} }) ctx.hook = contextBase - const context = await (service as any)[method](...args, contextBase) + const context = await (service as any)[method.key](...args, contextBase) ctx.hook = context const response = http.getResponse(context) diff --git a/packages/koa/test/index.test.ts b/packages/koa/test/index.test.ts index 445b140230..dd536a5d7d 100644 --- a/packages/koa/test/index.test.ts +++ b/packages/koa/test/index.test.ts @@ -2,7 +2,7 @@ import { strict as assert } from 'assert' import Koa from 'koa' import axios from 'axios' import { ApplicationHookMap, feathers, Id } from '@feathersjs/feathers' -import { Service, restTests } from '@feathersjs/tests' +import { Service, restTests, customDefs } from '@feathersjs/tests' import { koa, rest, Application, bodyParser, errorHandler } from '../src' describe('@feathersjs/koa', () => { @@ -38,7 +38,7 @@ describe('@feathersjs/koa', () => { } ] }, - methods: ['get', 'find', 'create', 'update', 'patch', 'remove', 'customMethod'] + methods: ['get', 'find', 'create', 'update', 'patch', 'remove', 'customMethod', ...customDefs] }) app.hooks({ @@ -170,7 +170,7 @@ describe('@feathersjs/koa', () => { await assert.rejects( () => axios.post( - 'http://localhost:8465/no/where', + 'http://localhost:8465/no/where/good', {}, { headers: { @@ -184,7 +184,7 @@ describe('@feathersjs/koa', () => { assert.deepStrictEqual(data, { name: 'NotFound', - message: 'Path /no/where not found', + message: 'Path /no/where/good not found', code: 404, className: 'not-found' }) @@ -214,6 +214,6 @@ describe('@feathersjs/koa', () => { await app.teardown() }) - restTests('Services', 'todo', 8465) + restTests('Services', 'todo', 8465, true) restTests('Root service', '/', 8465) }) diff --git a/packages/rest-client/src/index.ts b/packages/rest-client/src/index.ts index d0afbaf7a5..7ae11a6091 100644 --- a/packages/rest-client/src/index.ts +++ b/packages/rest-client/src/index.ts @@ -1,4 +1,9 @@ -import { Application, TransportConnection, defaultServiceMethods } from '@feathersjs/feathers' +import { + Application, + TransportConnection, + MethodDefinition, + defaultServiceMethods +} from '@feathersjs/feathers' import { Base } from './base' import { AxiosClient } from './axios' @@ -54,7 +59,10 @@ export default function restClient(base = '') { app.defaultService = defaultService app.mixins.unshift((service, _location, options) => { if (options && options.methods && service instanceof Base) { - const customMethods = options.methods.filter((name) => !defaultServiceMethods.includes(name)) + const methods = (options.methods as (string | MethodDefinition)[]).map((method) => + typeof method === 'string' ? method : method.key + ) + const customMethods = methods.filter((name) => !defaultServiceMethods.includes(name)) service.methods(...customMethods) } diff --git a/packages/socketio-client/src/index.ts b/packages/socketio-client/src/index.ts index af4000a1ec..d198461bb2 100644 --- a/packages/socketio-client/src/index.ts +++ b/packages/socketio-client/src/index.ts @@ -3,6 +3,7 @@ import { Socket } from 'socket.io-client' import { Application, TransportConnection, + MethodDefinition, defaultEventMap, defaultServiceMethods } from '@feathersjs/feathers' @@ -46,7 +47,10 @@ export default function socketioClient(connection: Socket, optio app.defaultService = defaultService app.mixins.unshift((service, _location, options) => { if (options && options.methods && service instanceof Service) { - const customMethods = options.methods.filter((name) => !defaultServiceMethods.includes(name)) + const methods = (options.methods as (string | MethodDefinition)[]).map((method) => + typeof method === 'string' ? method : method.key + ) + const customMethods = methods.filter((name) => !defaultServiceMethods.includes(name)) service.methods(...customMethods) } diff --git a/packages/tests/src/fixture.ts b/packages/tests/src/fixture.ts index 2e7bbb3fbf..cbb03434be 100644 --- a/packages/tests/src/fixture.ts +++ b/packages/tests/src/fixture.ts @@ -88,11 +88,54 @@ export class Service { } } + async customData(data: any, params: any) { + return { + data, + method: 'customData', + provider: params.provider + } + } + + async customIdData(id: any, data: any, params: any) { + return { + id, + data, + method: 'customIdData', + provider: params.provider + } + } + + async customId(id: any, params: any) { + return { + id, + method: 'customId', + provider: params.provider + } + } + + async customParams(params: any) { + return { + query: params.query, + method: 'customParams', + provider: params.provider + } + } + async internalMethod() { throw new Error('Should never get here') } } +export const customDefs = [ + { key: 'customData', route: true }, + { key: 'customIdData', id: true, route: true }, + { key: 'customIdData', id: true, route: 'custom', routeMethod: 'POST' }, + { key: 'customIdData', id: true, data: true, route: 'custom-patch', routeMethod: 'PATCH' }, + { key: 'customId', id: true, data: false, route: true }, + { key: 'customParams', id: false, data: false, route: true }, + { key: 'customParams', id: false, data: false, route: 'stats' } +] + export const verify = { find(data: any) { assert.deepStrictEqual(findAllData, clone(data), 'Data as expected') diff --git a/packages/tests/src/rest.ts b/packages/tests/src/rest.ts index 78149ba366..a2dc47d2f0 100644 --- a/packages/tests/src/rest.ts +++ b/packages/tests/src/rest.ts @@ -3,7 +3,7 @@ import axios from 'axios' import { verify } from './fixture' -export function restTests(description: string, name: string, port: number) { +export function restTests(description: string, name: string, port: number, testCustom?: boolean) { describe(description, () => { it('GET .find', async () => { const res = await axios.get(`http://localhost:${port}/${name}`) @@ -90,5 +90,106 @@ export function restTests(description: string, name: string, port: number) { assert.ok(res.status === 200, 'Got OK status code') verify.remove(null, res.data) }) + + if (testCustom) { + it('Throws POST customMethod (auto :id)', async () => { + const data = { message: 'Hello' } + assert.rejects( + () => axios.post(`http://localhost:${port}/${name}/custom-method`, data), + (error: any) => { + assert.deepEqual(error.response.data, { + name: 'MethodNotAllowed', + message: `Method \`${name}\` is not supported by this endpoint.`, + code: 405, + className: 'method-not-allowed' + }) + console.error(error.response.data) + + return true + } + ) + }) + // { key: 'customData', route: true }, + it('POST customData (auto :id)', async () => { + const data = { message: 'Hello' } + const res = await axios.post(`http://localhost:${port}/${name}/custom-data`, data) + assert.strictEqual(res.status, 200) + assert.deepStrictEqual(res.data, { + data, + method: 'customData', + provider: 'rest' + }) + }) + // { key: 'customIdData', id: true, route: true }, + it('POST customIdData (auto :id/:action)', async () => { + const data = { message: 'Hello' } + const id = '123' + const res = await axios.post(`http://localhost:${port}/${name}/${id}/custom-id-data`, data) + assert.strictEqual(res.status, 200) + assert.deepStrictEqual(res.data, { + id, + data, + method: 'customIdData', + provider: 'rest' + }) + }) + // { key: 'customIdData', id: true, route: 'custom', routeMethod: 'POST' }, + it('POST customIdData (custom :id/:action)', async () => { + const data = { message: 'Hello' } + const id = '123' + const res = await axios.post(`http://localhost:${port}/${name}/${id}/custom`, data) + assert.strictEqual(res.status, 200) + assert.deepStrictEqual(res.data, { + id, + data, + method: 'customIdData', + provider: 'rest' + }) + }) + // { key: 'customIdData', id: true, data: true, route: 'custom-patch', routeMethod: 'PATCH' }, + it('PATCH customIdData (custom :id/:action)', async () => { + const data = { message: 'Hello' } + const id = '123' + const res = await axios.patch(`http://localhost:${port}/${name}/${id}/custom-patch`, data) + assert.strictEqual(res.status, 200) + assert.deepStrictEqual(res.data, { + id, + data, + method: 'customIdData', + provider: 'rest' + }) + }) + // { key: 'customId', id: true, data: false, route: true }, + it('GET customId (auto :id/:action)', async () => { + const id = '123' + const res = await axios.get(`http://localhost:${port}/${name}/${id}/custom-id`) + assert.strictEqual(res.status, 200) + assert.deepStrictEqual(res.data, { + id, + method: 'customId', + provider: 'rest' + }) + }) + // { key: 'customParams', id: false, data: false, route: true }, + it('GET customParams (auto :id)', async () => { + const res = await axios.get(`http://localhost:${port}/${name}/custom-params?foo=bar`) + assert.strictEqual(res.status, 200) + assert.deepStrictEqual(res.data, { + method: 'customParams', + provider: 'rest', + query: { foo: 'bar' } + }) + }) + // { key: 'customParams', id: false, data: false, route: 'stats' } + it('GET customParams (custom :id)', async () => { + const res = await axios.get(`http://localhost:${port}/${name}/stats`) + assert.strictEqual(res.status, 200) + assert.deepStrictEqual(res.data, { + method: 'customParams', + provider: 'rest', + query: {} + }) + }) + } }) } diff --git a/packages/transport-commons/src/client.ts b/packages/transport-commons/src/client.ts index f71b88bebc..aaa536e0e5 100644 --- a/packages/transport-commons/src/client.ts +++ b/packages/transport-commons/src/client.ts @@ -99,11 +99,12 @@ export class Service, P extends Params = Params> methods(this: any, ...names: string[]) { names.forEach((method) => { const _method = `_${method}` - this[_method] = function (data: any, params: Params = {}) { - return this.send(method, data, params.query || {}, params.route || {}) + this[_method] = function (...args: [Id | any | Params, (any | Params)?, Params?]) { + const params = (args.pop() || {}) as Params + return this.send(method, ...args, params.query || {}, params.route || {}) } - this[method] = function (data: any, params: Params = {}) { - return this[_method](data, params, params.route || {}) + this[method] = function (...args: [Id | any | Params, (any | Params)?, Params?]) { + return this[_method](...args) } }) return this diff --git a/packages/transport-commons/src/http.ts b/packages/transport-commons/src/http.ts index 262dde2176..d147088a1b 100644 --- a/packages/transport-commons/src/http.ts +++ b/packages/transport-commons/src/http.ts @@ -1,6 +1,14 @@ import { MethodNotAllowed } from '@feathersjs/errors/lib' -import { HookContext, NullableId, Params } from '@feathersjs/feathers' +import { + HookContext, + NullableId, + Params, + getServiceOptions, + getServiceMethodArgs, + MethodDefinition +} from '@feathersjs/feathers' import encodeUrl from 'encodeurl' +import kebabCase from 'lodash/kebabCase' export const METHOD_HEADER = 'x-service-method' @@ -18,41 +26,58 @@ export const statusCodes = { seeOther: 303 } -export const knownMethods: { [key: string]: string } = { - post: 'create', - patch: 'patch', - put: 'update', - delete: 'remove' +function getMethodRoute(method: MethodDefinition) { + const methodRoute = typeof method.route === 'string' ? method.route : kebabCase(method.key) + return methodRoute } -export function getServiceMethod(_httpMethod: string, id: unknown, headerOverride?: string) { +export function getServiceMethod( + _httpMethod: string, + id: unknown, + action: unknown, + service: any, + headerOverride?: string +) { const httpMethod = _httpMethod.toLowerCase() + const { serviceMethods } = getServiceOptions(service) if (httpMethod === 'post' && headerOverride) { - return headerOverride + const method = serviceMethods.find((method) => method.key === headerOverride) + return method } - const mappedMethod = knownMethods[httpMethod] - - if (mappedMethod) { - return mappedMethod + const potentialMethods = serviceMethods.filter( + (method) => method.route !== false && method.routeMethod.toLowerCase() === httpMethod.toLowerCase() + ) + + // find the case where the action is the id as the method does not have an id in the args + let foundMethod = potentialMethods.find((method) => { + const methodRoute = getMethodRoute(method) + return !method.id && !action && (id || '') === methodRoute + }) + if (foundMethod) { + return foundMethod } - if (httpMethod === 'get') { - return id === null ? 'find' : 'get' + foundMethod = potentialMethods.find((method) => { + const methodRoute = getMethodRoute(method) + return method.id && methodRoute === (action || '') + }) + if (foundMethod) { + return foundMethod } - throw new MethodNotAllowed(`Method ${_httpMethod} not allowed`) + if (!['get', 'post', 'patch', 'put', 'delete'].includes(_httpMethod.toLowerCase())) { + throw new MethodNotAllowed(`Method ${_httpMethod} not allowed`) + } + return null } -export const argumentsFor = { - get: ({ id, params }: ServiceParams) => [id, params], - find: ({ params }: ServiceParams) => [params], - create: ({ data, params }: ServiceParams) => [data, params], - update: ({ id, data, params }: ServiceParams) => [id, data, params], - patch: ({ id, data, params }: ServiceParams) => [id, data, params], - remove: ({ id, params }: ServiceParams) => [id, params], - default: ({ data, params }: ServiceParams) => [data, params] +export function argumentsFor(method: MethodDefinition) { + return (serviceParams: ServiceParams) => { + const args = getServiceMethodArgs(method) + return args.map((arg) => serviceParams[arg]) + } } export function getStatusCode(context: HookContext, body: any, location: string | string[]) { diff --git a/packages/transport-commons/src/routing/index.ts b/packages/transport-commons/src/routing/index.ts index 8e4870347f..778418c44e 100644 --- a/packages/transport-commons/src/routing/index.ts +++ b/packages/transport-commons/src/routing/index.ts @@ -49,6 +49,7 @@ export const routing = () => (app: Application) => { app.unuse = function (path: string) { app.routes.remove(path) app.routes.remove(`${path}/:__id`) + app.routes.remove(`${path}/:__id/:__action`) return unuse.call(this, path) } @@ -58,5 +59,6 @@ export const routing = () => (app: Application) => { app.routes.insert(path, { service, params }) app.routes.insert(`${path}/:__id`, { service, params }) + app.routes.insert(`${path}/:__id/:__action`, { service, params }) }) } diff --git a/packages/transport-commons/src/socket/index.ts b/packages/transport-commons/src/socket/index.ts index 1c21095fd1..c75eb155c8 100644 --- a/packages/transport-commons/src/socket/index.ts +++ b/packages/transport-commons/src/socket/index.ts @@ -45,7 +45,7 @@ export function socket({ done, emit, socketMap, socketKey, getParams }: SocketOp const methodHandlers = Object.keys(app.services).reduce((result, name) => { const { methods } = getServiceOptions(app.service(name)) - methods.forEach((method) => { + ;(methods as string[]).forEach((method) => { if (!result[method]) { result[method] = (...args: any[]) => { const [path, ...rest] = args diff --git a/packages/transport-commons/src/socket/utils.ts b/packages/transport-commons/src/socket/utils.ts index 768bba469a..c15d10e38f 100644 --- a/packages/transport-commons/src/socket/utils.ts +++ b/packages/transport-commons/src/socket/utils.ts @@ -3,7 +3,8 @@ import { Application, RealTimeConnection, createContext, - getServiceOptions + getServiceOptions, + getServiceMethodArgs } from '@feathersjs/feathers' import { NotFound, MethodNotAllowed, BadRequest } from '@feathersjs/errors' import { createDebug } from '@feathersjs/commons' @@ -104,7 +105,10 @@ export async function runMethod( throw new MethodNotAllowed(`Method '${method}' not allowed on service '${path}'`) } - const position = paramsPositions[method] !== undefined ? paramsPositions[method] : DEFAULT_PARAMS_POSITION + let position = getServiceMethodArgs(method, service).indexOf('params') + if (position < 0) { + position = paramsPositions[method] !== undefined ? paramsPositions[method] : DEFAULT_PARAMS_POSITION + } const query = Object.assign({}, methodArgs[position]) // `params` have to be re-mapped to the query and added with the route const params = Object.assign({ query, route, connection }, connection) diff --git a/packages/transport-commons/test/http.test.ts b/packages/transport-commons/test/http.test.ts index dfbe4ef74f..0260391462 100644 --- a/packages/transport-commons/test/http.test.ts +++ b/packages/transport-commons/test/http.test.ts @@ -52,12 +52,12 @@ describe('@feathersjs/transport-commons HTTP helpers', () => { }) }) - it('getServiceMethod', () => { - assert.strictEqual(http.getServiceMethod('GET', 2), 'get') - assert.strictEqual(http.getServiceMethod('GET', null), 'find') - assert.strictEqual(http.getServiceMethod('PoST', null), 'create') - assert.strictEqual(http.getServiceMethod('PoST', null, 'customMethod'), 'customMethod') - assert.strictEqual(http.getServiceMethod('delete', null), 'remove') - assert.throws(() => http.getServiceMethod('nonsense', null)) - }) + // it('getServiceMethod', () => { + // assert.strictEqual(http.getServiceMethod('GET', 2, null), 'get') + // assert.strictEqual(http.getServiceMethod('GET', null, null), 'find') + // assert.strictEqual(http.getServiceMethod('PoST', null, null), 'create') + // assert.strictEqual(http.getServiceMethod('PoST', null, 'customMethod'), 'customMethod') + // assert.strictEqual(http.getServiceMethod('delete', null, null), 'remove') + // assert.throws(() => http.getServiceMethod('nonsense', null, null)) + // }) })