WIP: Dynamic Method Definitions

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)

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)
   })
 })

packages/feathers/src/declarations.ts

@@ -20,6 +20,30 @@ export interface Paginated<T> {
   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<MethodTypes = string> {
   /**
    * 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.

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<typeof convertHookData> }
 
@@ -186,7 +186,7 @@ export function hookMixin<A>(this: A, service: FeathersService<A>, 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<A>(this, method).params(...params).props({
       app: this,

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<string, MethodArgument[]>
 
-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<string, string>)
 
 export const defaultServiceEvents = Object.values(defaultEventMap)
 
@@ -30,26 +86,68 @@ 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 {
   return service[SERVICE]
 }
 
 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
   }
 }

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)