From e65abfb5388df6c19a11c565cf1076a29f32668d Mon Sep 17 00:00:00 2001 From: David Luecke Date: Sun, 28 Mar 2021 14:50:30 -0700 Subject: [PATCH] feat(core): Public custom service methods (#2270) --- packages/express/src/declarations.ts | 2 +- packages/express/src/index.ts | 12 +- packages/express/src/rest.ts | 11 +- packages/express/test/authentication.test.ts | 17 ++- packages/express/test/rest.test.ts | 141 ++++++++++-------- packages/feathers/src/declarations.ts | 4 + packages/feathers/src/service.ts | 4 +- packages/rest-client/package.json | 1 - packages/rest-client/src/base.ts | 37 +++-- packages/rest-client/src/fetch.ts | 4 +- packages/rest-client/src/index.ts | 2 + packages/rest-client/test/axios.test.ts | 16 +- packages/rest-client/test/declarations.ts | 6 + packages/rest-client/test/fetch.test.ts | 19 ++- packages/rest-client/test/server.ts | 23 ++- packages/rest-client/test/superagent.test.ts | 17 ++- packages/socketio-client/src/index.ts | 12 +- packages/socketio-client/test/index.test.ts | 23 ++- packages/socketio-client/test/server.ts | 14 +- packages/socketio/test/methods.ts | 10 +- packages/tests/src/fixture.ts | 12 ++ packages/tests/src/rest.ts | 2 +- .../transport-commons/src/channels/index.ts | 2 +- packages/transport-commons/src/client.ts | 39 +++-- .../transport-commons/src/socket/utils.ts | 7 +- .../test/channels/index.test.ts | 2 +- .../transport-commons/test/client.test.ts | 107 +++++++------ 27 files changed, 347 insertions(+), 199 deletions(-) create mode 100644 packages/rest-client/test/declarations.ts diff --git a/packages/express/src/declarations.ts b/packages/express/src/declarations.ts index 1537455219..8b7e27eafe 100644 --- a/packages/express/src/declarations.ts +++ b/packages/express/src/declarations.ts @@ -33,7 +33,7 @@ export type Application = declare module '@feathersjs/feathers/lib/declarations' { export interface ServiceOptions { - middleware: { + middleware?: { before: express.RequestHandler[], after: express.RequestHandler[] } diff --git a/packages/express/src/index.ts b/packages/express/src/index.ts index efd8545374..193f4a6012 100644 --- a/packages/express/src/index.ts +++ b/packages/express/src/index.ts @@ -35,12 +35,15 @@ export default function feathersExpress (feathersApp?: Feather const mixin: any = { use (location: string, ...rest: any[]) { let service: any; - const middleware = Array.from(arguments).slice(1) - .reduce(function (middleware, arg) { + let options = {}; + + const middleware = rest.reduce(function (middleware, arg) { if (typeof arg === 'function' || Array.isArray(arg)) { middleware[service ? 'after' : 'before'].push(arg); } else if (!service) { service = arg; + } else if (arg.methods || arg.events) { + options = arg; } else { throw new Error('Invalid options passed to app.use'); } @@ -62,7 +65,10 @@ export default function feathersExpress (feathersApp?: Feather debug('Registering service with middleware', middleware); // Since this is a service, call Feathers `.use` - (feathersApp as FeathersApplication).use.call(this, location, service, { middleware }); + (feathersApp as FeathersApplication).use.call(this, location, service, { + ...options, + middleware + }); return this; }, diff --git a/packages/express/src/rest.ts b/packages/express/src/rest.ts index 9d6e1621de..f2f8d283ab 100644 --- a/packages/express/src/rest.ts +++ b/packages/express/src/rest.ts @@ -1,7 +1,7 @@ import Debug from 'debug'; import { MethodNotAllowed } from '@feathersjs/errors'; import { HookContext } from '@feathersjs/hooks'; -import { createContext, getServiceOptions, NullableId, Params } from '@feathersjs/feathers'; +import { createContext, defaultServiceMethods, getServiceOptions, NullableId, Params } from '@feathersjs/feathers'; import { Request, Response, NextFunction, RequestHandler, Router } from 'express'; import { parseAuthentication } from './authentication'; @@ -94,14 +94,13 @@ export const serviceMiddleware = (callback: ServiceCallback) => } export const serviceMethodHandler = ( - service: any, methodName: string, getArgs: (opts: ServiceParams) => any[], header?: string + service: any, methodName: string, getArgs: (opts: ServiceParams) => any[], headerOverride?: string ) => serviceMiddleware(async (req, res, options) => { - const method = (typeof header === 'string' && req.headers[header]) - ? req.headers[header] as string - : methodName + const methodOverride = typeof headerOverride === 'string' && (req.headers[headerOverride] as string); + const method = methodOverride ? methodOverride : methodName const { methods } = getServiceOptions(service); - if (!methods.includes(method)) { + if (!methods.includes(method) || defaultServiceMethods.includes(methodOverride)) { res.status(statusCodes.methodNotAllowed); throw new MethodNotAllowed(`Method \`${method}\` is not supported by this endpoint.`); diff --git a/packages/express/test/authentication.test.ts b/packages/express/test/authentication.test.ts index f1063d487b..cc8117c29a 100644 --- a/packages/express/test/authentication.test.ts +++ b/packages/express/test/authentication.test.ts @@ -174,19 +174,20 @@ describe('@feathersjs/express/authentication', () => { }); }); - it.skip('protected endpoint fails with invalid Authorization header', () => { - return axios.get('/protected', { - headers: { - Authorization: 'Bearer: something wrong' - } - }).then(() => { + it.skip('protected endpoint fails with invalid Authorization header', async () => { + try { + await axios.get('/protected', { + headers: { + Authorization: 'Bearer: something wrong' + } + }); assert.fail('Should never get here'); - }).catch(error => { + } catch (error) { const { data } = error.response; assert.strictEqual(data.name, 'NotAuthenticated'); assert.strictEqual(data.message, 'Not authenticated'); - }); + } }); it('can request protected endpoint with JWT present', () => { diff --git a/packages/express/test/rest.test.ts b/packages/express/test/rest.test.ts index 8c847ff771..e0d5ecea7b 100644 --- a/packages/express/test/rest.test.ts +++ b/packages/express/test/rest.test.ts @@ -1,17 +1,20 @@ /* eslint-disable @typescript-eslint/no-unused-vars */ import { strict as assert } from 'assert'; -import axios from 'axios'; +import axios, { AxiosRequestConfig } from 'axios'; import { Server } from 'http'; +import { Request, Response, NextFunction } from 'express'; import { feathers, HookContext, Id, Params } from '@feathersjs/feathers'; -import { Service, testRest } from '@feathersjs/tests'; +import { Service, restTests } from '@feathersjs/tests'; +import { BadRequest } from '@feathersjs/errors'; import * as express from '../src' -import { Request, Response, NextFunction } from 'express'; -import { BadRequest } from '@feathersjs/errors/lib'; const expressify = express.default; const { rest } = express; +const errorHandler = express.errorHandler({ + logger: false +}); describe('@feathersjs/express/rest provider', () => { describe('base functionality', () => { @@ -99,9 +102,9 @@ describe('@feathersjs/express/rest provider', () => { after(done => server.close(done)); - testRest('Services', 'todo', 4777); - testRest('Root Service', '/', 4777); - testRest('Dynamic Services', 'tasks', 4777); + restTests('Services', 'todo', 4777); + restTests('Root Service', '/', 4777); + restTests('Dynamic Services', 'tasks', 4777); describe('res.hook', () => { const convertHook = (hook: HookContext) => { @@ -526,7 +529,7 @@ describe('@feathersjs/express/rest provider', () => { }; } }) - .use(express.errorHandler()); + .use(errorHandler); server = await app.listen(6880); }); @@ -566,62 +569,68 @@ describe('@feathersjs/express/rest provider', () => { }); }); - // describe('Custom methods', () => { - // let server: Server; - // let app: express.Application; - - // before(async () => { - // app = expressify(feathers()) - // .configure(rest()) - // .use(express.json()) - // .use('/todo', { - // async get (id) { - // return id; - // }, - // // httpMethod is usable as a decorator: @httpMethod('POST', '/:__feathersId/custom-path') - // custom: rest.httpMethod('POST')((feathers as any).activateHooks(['id', 'data', 'params'])( - // (id: any, data: any) => { - // return Promise.resolve({ - // id, - // data - // }); - // } - // )), - // other: rest.httpMethod('PATCH', ':__feathersId/second-method')( - // (feathers as any).activateHooks(['id', 'data', 'params'])( - // (id: any, data: any) => { - // return Promise.resolve({ - // id, - // data - // }); - // } - // ) - // ) - // }); - - // server = await app.listen(4781); - // }); - - // after(done => server.close(done)); - - // it('works with custom methods', async () => { - // const res = await axios.post('http://localhost:4781/todo/42/custom', { text: 'Do dishes' }); - - // assert.equal(res.headers.allow, 'GET,POST,PATCH'); - // assert.deepEqual(res.data, { - // id: '42', - // data: { text: 'Do dishes' } - // }); - // }); - - // it('works with custom methods - with route', async () => { - // const res = await axios.patch('http://localhost:4781/todo/12/second-method', { text: 'Hmm' }); - - // assert.equal(res.headers.allow, 'GET,POST,PATCH'); - // assert.deepEqual(res.data, { - // id: '12', - // data: { text: 'Hmm' } - // }); - // }); - // }); + describe('Custom methods', () => { + let server: Server; + let app: express.Application; + + before(async () => { + app = expressify(feathers()) + .configure(rest()) + .use(express.json()) + .use('/todo', new Service(), { + methods: ['find', 'customMethod'] + }) + .use(errorHandler); + + server = await app.listen(4781); + }); + + after(done => server.close(done)); + + it('calls .customMethod with X-Service-Method header', async () => { + const payload = { text: 'Do dishes' }; + const res = await axios.post('http://localhost:4781/todo', payload, { + headers: { + 'X-Service-Method': 'customMethod' + } + }); + + assert.deepEqual(res.data, { + data: payload, + method: 'customMethod', + provider: 'rest' + }); + }); + + it('throws MethodNotImplement for .setup, non option and default methods', async () => { + const options: AxiosRequestConfig = { + method: 'POST', + url: 'http://localhost:4781/todo', + data: { text: 'Do dishes' } + }; + const testMethod = (name: string) => { + return assert.rejects(() => axios({ + ...options, + headers: { + 'X-Service-Method': name + } + }), (error: any) => { + assert.deepEqual(error.response.data, { + name: 'MethodNotAllowed', + message: `Method \`${name}\` is not supported by this endpoint.`, + code: 405, + className: 'method-not-allowed' + }); + + return true; + }); + } + + await testMethod('setup'); + await testMethod('internalMethod'); + await testMethod('nonExisting'); + await testMethod('create'); + await testMethod('find'); + }); + }); }); diff --git a/packages/feathers/src/declarations.ts b/packages/feathers/src/declarations.ts index 581144335b..88e3efeb71 100644 --- a/packages/feathers/src/declarations.ts +++ b/packages/feathers/src/declarations.ts @@ -100,6 +100,10 @@ export interface ServiceHookOverloads { export type FeathersService> = S & ServiceAddons & OptionalPick, keyof S>; +export type CustomMethod = { + [k in Methods]: (data: any, params?: Params) => Promise; +} + export type ServiceMixin = (service: FeathersService, path: string, options?: ServiceOptions) => void; export type ServiceGenericType = S extends ServiceInterface ? T : any; diff --git a/packages/feathers/src/service.ts b/packages/feathers/src/service.ts index 9dd034e234..d026edfcdd 100644 --- a/packages/feathers/src/service.ts +++ b/packages/feathers/src/service.ts @@ -13,7 +13,7 @@ export const defaultServiceArguments = { remove: [ 'id', 'params' ] } -export const defaultServiceMethods = Object.keys(defaultServiceArguments).concat('setup'); +export const defaultServiceMethods = Object.keys(defaultServiceArguments); export const defaultEventMap = { create: 'created', @@ -60,7 +60,7 @@ export function wrapService ( const protoService = Object.create(service); const serviceOptions = getServiceOptions(service, options); - if (Object.keys(serviceOptions.methods).length === 0) { + if (Object.keys(serviceOptions.methods).length === 0 && typeof service.setup !== 'function') { throw new Error(`Invalid service object passed for path \`${location}\``); } diff --git a/packages/rest-client/package.json b/packages/rest-client/package.json index 8ef08b0c88..749d804752 100644 --- a/packages/rest-client/package.json +++ b/packages/rest-client/package.json @@ -64,7 +64,6 @@ "@types/mocha": "^8.2.2", "@types/node": "^14.14.35", "axios": "^0.21.1", - "body-parser": "^1.19.0", "mocha": "^8.3.2", "node-fetch": "^2.6.1", "rxjs": "^6.6.6", diff --git a/packages/rest-client/src/base.ts b/packages/rest-client/src/base.ts index 2b26257e37..6ea113e1a7 100644 --- a/packages/rest-client/src/base.ts +++ b/packages/rest-client/src/base.ts @@ -1,9 +1,7 @@ import qs from 'qs'; -import { Unavailable } from '@feathersjs/errors'; -import { _ } from '@feathersjs/commons'; -import { stripSlashes } from '@feathersjs/commons'; -import { convert } from '@feathersjs/errors'; -import { Params, Id, Query, NullableId } from '@feathersjs/feathers'; +import { Params, Id, Query, NullableId, ServiceInterface } from '@feathersjs/feathers'; +import { Unavailable, convert } from '@feathersjs/errors'; +import { _, stripSlashes } from '@feathersjs/commons'; function toError (error: Error & { code: string }) { if (error.code === 'ECONNREFUSED') { @@ -20,7 +18,7 @@ interface RestClientSettings { options: any; } -export abstract class Base { +export abstract class Base> implements ServiceInterface { name: string; base: string; connection: any; @@ -34,9 +32,10 @@ export abstract class Base { } makeUrl (query: Query, id?: string|number|null) { - query = query || {}; let url = this.base; + query = query || {}; + if (typeof id !== 'undefined' && id !== null) { url += `/${encodeURIComponent(id)}`; } @@ -56,6 +55,24 @@ export abstract class Base { abstract request (options: any, params: Params): any; + methods (this: any, ...names: string[]) { + names.forEach(method => { + this[method] = function (body: any, params: Params = {}) { + return this.request({ + body, + url: this.makeUrl(params.query), + method: 'POST', + headers: Object.assign({ + 'Content-Type': 'application/json', + 'X-Service-Method': method + }, params.headers) + }, params).catch(toError); + } + }); + + return this; + } + find (params: Params = {}) { return this.request({ url: this.makeUrl(params.query), @@ -76,7 +93,7 @@ export abstract class Base { }, params).catch(toError); } - create (body: any, params: Params = {}) { + create (body: D, params: Params = {}) { return this.request({ url: this.makeUrl(params.query), body, @@ -85,7 +102,7 @@ export abstract class Base { }, params).catch(toError); } - update (id: NullableId, body: any, params: Params = {}) { + update (id: NullableId, body: D, params: Params = {}) { if (typeof id === 'undefined') { return Promise.reject(new Error('id for \'update\' can not be undefined, only \'null\' when updating multiple entries')); } @@ -98,7 +115,7 @@ export abstract class Base { }, params).catch(toError); } - patch (id: NullableId, body: any, params: Params = {}) { + patch (id: NullableId, body: D, params: Params = {}) { if (typeof id === 'undefined') { return Promise.reject(new Error('id for \'patch\' can not be undefined, only \'null\' when updating multiple entries')); } diff --git a/packages/rest-client/src/fetch.ts b/packages/rest-client/src/fetch.ts index 8166ed3c93..5e0703974c 100644 --- a/packages/rest-client/src/fetch.ts +++ b/packages/rest-client/src/fetch.ts @@ -14,9 +14,7 @@ export class FetchClient extends Base { fetchOptions.body = JSON.stringify(options.body); } - const fetch = this.connection; - - return fetch(options.url, fetchOptions) + return this.connection(options.url, fetchOptions) .then(this.checkStatus) .then((response: any) => { if (response.status === 204) { diff --git a/packages/rest-client/src/index.ts b/packages/rest-client/src/index.ts index f1c0506f21..05cbb63df7 100644 --- a/packages/rest-client/src/index.ts +++ b/packages/rest-client/src/index.ts @@ -36,6 +36,8 @@ export interface Transport { axios: Handler; } +export type RestService> = Base; + export default function restClient (base = '') { const result: any = { Base }; diff --git a/packages/rest-client/test/axios.test.ts b/packages/rest-client/test/axios.test.ts index 98574c400f..cdce8b7ffd 100644 --- a/packages/rest-client/test/axios.test.ts +++ b/packages/rest-client/test/axios.test.ts @@ -8,14 +8,18 @@ import { NotAcceptable } from '@feathersjs/errors'; import createServer from './server'; import rest from '../src'; +import { ServiceTypes } from './declarations'; + describe('Axios REST connector', function () { const url = 'http://localhost:8889'; const setup = rest(url).axios(axios); - const app = feathers().configure(setup); + const app = feathers().configure(setup); const service = app.service('todos'); let server: Server; + service.methods('customMethod'); + before(async () => { server = await createServer().listen(8889); }); @@ -114,5 +118,15 @@ describe('Axios REST connector', function () { } }); + it('works with custom method .customMethod', async () => { + const result = await service.customMethod({ message: 'hi' }); + + assert.deepEqual(result, { + data: { message: 'hi' }, + provider: 'rest', + type: 'customMethod' + }); + }); + clientTests(service, 'todos'); }); diff --git a/packages/rest-client/test/declarations.ts b/packages/rest-client/test/declarations.ts new file mode 100644 index 0000000000..eab07a15e7 --- /dev/null +++ b/packages/rest-client/test/declarations.ts @@ -0,0 +1,6 @@ +import { CustomMethod } from '@feathersjs/feathers'; +import { RestService } from '../src'; + +export type ServiceTypes = { + todos: RestService & CustomMethod<'customMethod'> +} diff --git a/packages/rest-client/test/fetch.test.ts b/packages/rest-client/test/fetch.test.ts index 99ab5427dc..c36170e5df 100644 --- a/packages/rest-client/test/fetch.test.ts +++ b/packages/rest-client/test/fetch.test.ts @@ -4,18 +4,21 @@ import { feathers } from '@feathersjs/feathers'; import { clientTests } from '@feathersjs/tests'; import { NotAcceptable } from '@feathersjs/errors'; import fetch from 'node-fetch'; +import { Server } from 'http'; -import createServer from './server'; import rest from '../src'; -import { Server } from 'http'; +import createServer from './server'; +import { ServiceTypes } from './declarations'; describe('fetch REST connector', function () { const url = 'http://localhost:8889'; const setup = rest(url).fetch(fetch); - const app = feathers().configure(setup); + const app = feathers().configure(setup); const service = app.service('todos'); let server: Server; + service.methods('customMethod'); + before(async () => { server = await createServer().listen(8889); }); @@ -116,5 +119,15 @@ describe('fetch REST connector', function () { assert.strictEqual(response, null) }); + it('works with custom method .customMethod', async () => { + const result = await service.customMethod({ message: 'hi' }); + + assert.deepEqual(result, { + data: { message: 'hi' }, + provider: 'rest', + type: 'customMethod' + }); + }); + clientTests(service, 'todos'); }); diff --git a/packages/rest-client/test/server.ts b/packages/rest-client/test/server.ts index 117fb509d5..eea5574bfc 100644 --- a/packages/rest-client/test/server.ts +++ b/packages/rest-client/test/server.ts @@ -1,6 +1,5 @@ -import bodyParser from 'body-parser'; import { feathers, Id, NullableId, Params } from '@feathersjs/feathers'; -import expressify, { rest } from '@feathersjs/express'; +import expressify, { rest, urlencoded, json } from '@feathersjs/express'; import { Service } from '@feathersjs/adapter-memory'; import { FeathersError, NotAcceptable } from '@feathersjs/errors'; @@ -59,13 +58,21 @@ class TodoService extends Service { id, text: 'deleted many' }); } - + if (params.query.noContent) { return Promise.resolve(); } return super.remove(id, params); } + + async customMethod (data: any, { provider }: Params) { + return { + data, + provider, + type: 'customMethod' + } + } } export default (configurer?: any) => { @@ -92,10 +99,14 @@ export default (configurer?: any) => { next(); }) // Parse HTTP bodies - .use(bodyParser.json()) - .use(bodyParser.urlencoded({ extended: true })) + .use(json()) + .use(urlencoded({ extended: true })) // Host our Todos service on the /todos path - .use('/todos', new TodoService()) + .use('/todos', new TodoService(), { + methods: [ + 'find', 'get', 'create', 'patch', 'update', 'remove', 'customMethod' + ] + }) .use(errorHandler); if (typeof configurer === 'function') { diff --git a/packages/rest-client/test/superagent.test.ts b/packages/rest-client/test/superagent.test.ts index bd61987e13..8bef9b2498 100644 --- a/packages/rest-client/test/superagent.test.ts +++ b/packages/rest-client/test/superagent.test.ts @@ -6,16 +6,19 @@ import { feathers } from '@feathersjs/feathers'; import { clientTests } from '@feathersjs/tests'; import { NotAcceptable } from '@feathersjs/errors'; -import createServer from './server'; import rest from '../src'; +import createServer from './server'; +import { ServiceTypes } from './declarations'; describe('Superagent REST connector', function () { let server: Server; const url = 'http://localhost:8889'; const setup = rest(url).superagent(superagent); - const app = feathers().configure(setup); + const app = feathers().configure(setup); const service = app.service('todos'); + + service.methods('customMethod'); before(async () => { server = await createServer().listen(8889); @@ -97,5 +100,15 @@ describe('Superagent REST connector', function () { } }); + it('works with custom method .customMethod', async () => { + const result = await service.customMethod({ message: 'hi' }); + + assert.deepEqual(result, { + data: { message: 'hi' }, + provider: 'rest', + type: 'customMethod' + }); + }); + clientTests(service, 'todos'); }); diff --git a/packages/socketio-client/src/index.ts b/packages/socketio-client/src/index.ts index 183259442c..f4d5ed2ba4 100644 --- a/packages/socketio-client/src/index.ts +++ b/packages/socketio-client/src/index.ts @@ -1,12 +1,10 @@ -import { Service } from '@feathersjs/transport-commons/client'; +import { Service, SocketService } from '@feathersjs/transport-commons/client'; import { Socket } from 'socket.io-client'; import { defaultEventMap } from '@feathersjs/feathers'; -interface SocketIOClientOptions { - timeout?: number; -} +export { SocketService }; -function socketioClient (connection: Socket, options?: SocketIOClientOptions) { +export default function socketioClient (connection: Socket, options?: any) { if (!connection) { throw new Error('Socket.io connection needs to be provided'); } @@ -38,4 +36,6 @@ function socketioClient (connection: Socket, options?: SocketIOClientOptions) { return initialize; } -export = socketioClient; +if (typeof module !== 'undefined') { + module.exports = Object.assign(socketioClient, module.exports); +} diff --git a/packages/socketio-client/test/index.test.ts b/packages/socketio-client/test/index.test.ts index 8050cbc9b1..11b0215d64 100644 --- a/packages/socketio-client/test/index.test.ts +++ b/packages/socketio-client/test/index.test.ts @@ -1,14 +1,20 @@ import { strict as assert } from 'assert'; import { Server } from 'http'; -import { feathers } from '@feathersjs/feathers'; +import { CustomMethod, feathers } from '@feathersjs/feathers'; import { io, Socket } from 'socket.io-client'; import { clientTests } from '@feathersjs/tests'; import { createServer } from './server'; -import socketio from '../src'; +import socketio, { SocketService } from '../src'; + +type ServiceTypes = { + '/': SocketService, + 'todos': SocketService & CustomMethod<'customMethod'>, + [key: string]: any; +} describe('@feathersjs/socketio-client', () => { - const app = feathers(); + const app = feathers(); let socket: Socket; let server: Server; @@ -78,6 +84,17 @@ describe('@feathersjs/socketio-client', () => { } }); + it('calls .customMethod', async () => { + const service = app.service('todos').methods('customMethod'); + const result = await service.customMethod({ message: 'hi' }); + + assert.deepStrictEqual(result, { + data: { message: 'hi' }, + provider: 'socketio', + type: 'customMethod' + }); + }); + clientTests(app, 'todos'); clientTests(app, '/'); }); diff --git a/packages/socketio-client/test/server.ts b/packages/socketio-client/test/server.ts index b4c7824531..903a0cdddc 100644 --- a/packages/socketio-client/test/server.ts +++ b/packages/socketio-client/test/server.ts @@ -29,13 +29,25 @@ class TodoService extends Service { return Object.assign({ query: params.query }, data) } + + async customMethod (data: any, { provider }: Params) { + return { + data, + provider, + type: 'customMethod' + } + } } export function createServer () { const app = feathers() .configure(socketio()) .use('/', new TodoService()) - .use('/todos', new TodoService()); + .use('/todos', new TodoService(), { + methods: [ + 'find', 'get', 'create', 'update', 'patch', 'remove', 'customMethod' + ] + }); const service = app.service('todos'); const rootService = app.service('/'); const publisher = () => app.channel('general'); diff --git a/packages/socketio/test/methods.ts b/packages/socketio/test/methods.ts index 8a6fcf0067..0031339d01 100644 --- a/packages/socketio/test/methods.ts +++ b/packages/socketio/test/methods.ts @@ -13,11 +13,11 @@ export default (name: string, options: any) => { ); }); - it('invalid arguments cause an error', () => - call('find', 1, {}).catch(e => - assert.strictEqual(e.message, 'Too many arguments for \'find\' method') - ) - ); + it('invalid arguments cause an error', async () => { + await assert.rejects(() => call('find', 1, {}), { + message: 'Too many arguments for \'find\' method' + }); + }); it('.find', () => async () => { await call('find', {}).then(data => verify.find(data)); diff --git a/packages/tests/src/fixture.ts b/packages/tests/src/fixture.ts index eb5353759b..a4d1c5d449 100644 --- a/packages/tests/src/fixture.ts +++ b/packages/tests/src/fixture.ts @@ -73,6 +73,18 @@ export class Service { async remove (id: any) { return { id }; } + + async customMethod (data: any, params: any) { + return { + data, + method: 'customMethod', + provider: params.provider + }; + } + + async internalMethod () { + throw new Error('Should never get here'); + } } export const verify = { diff --git a/packages/tests/src/rest.ts b/packages/tests/src/rest.ts index bbcca56659..570211a0ed 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 testRest (description: string, name: string, port: number) { +export function restTests (description: string, name: string, port: number) { describe(description, () => { it('GET .find', () => { return axios.get(`http://localhost:${port}/${name}`) diff --git a/packages/transport-commons/src/channels/index.ts b/packages/transport-commons/src/channels/index.ts index 15f62989ba..a0751f1492 100644 --- a/packages/transport-commons/src/channels/index.ts +++ b/packages/transport-commons/src/channels/index.ts @@ -10,7 +10,7 @@ const debug = Debug('@feathersjs/transport-commons/channels'); const { CHANNELS } = keys; declare module '@feathersjs/feathers/lib/declarations' { - interface ServiceAddons extends EventEmitter { + interface ServiceAddons extends EventEmitter { // eslint-disable-line publish (publisher: Publisher>): this; publish (event: Event, publisher: Publisher>): this; diff --git a/packages/transport-commons/src/client.ts b/packages/transport-commons/src/client.ts index ae34bf5619..0fee28ade1 100644 --- a/packages/transport-commons/src/client.ts +++ b/packages/transport-commons/src/client.ts @@ -1,6 +1,6 @@ import Debug from 'debug'; import { convert } from '@feathersjs/errors'; -import { Params } from '@feathersjs/feathers'; +import { Id, NullableId, Params, ServiceInterface } from '@feathersjs/feathers'; const debug = Debug('@feathersjs/transport-commons/client'); @@ -59,7 +59,9 @@ interface ServiceOptions { events?: string[]; } -export class Service { +export type SocketService> = Service; + +export class Service> implements ServiceInterface { events: string[]; path: string; connection: any; @@ -74,8 +76,8 @@ export class Service { addEmitterMethods(this); } - send (method: string, ...args: any[]) { - return new Promise((resolve, reject) => { + send (method: string, ...args: any[]) { + return new Promise((resolve, reject) => { args.unshift(method, this.path); args.push(function (error: any, data: any) { return error ? reject(convert(error)) : resolve(data); @@ -87,28 +89,37 @@ export class Service { }); } + methods (this: any, ...names: string[]) { + names.forEach(name => { + this[name] = function (data: any, params: Params = {}) { + return this.send(name, data, params.query || {}); + } + }); + return this; + } + find (params: Params = {}) { - return this.send('find', params.query || {}); + return this.send('find', params.query || {}); } - get (id: number | string, params: Params = {}) { - return this.send('get', id, params.query || {}); + get (id: Id, params: Params = {}) { + return this.send('get', id, params.query || {}); } create (data: any, params: Params = {}) { - return this.send('create', data, params.query || {}); + return this.send('create', data, params.query || {}); } - update (id: number | string, data: any, params: Params = {}) { - return this.send('update', id, data, params.query || {}); + update (id: Id, data: any, params: Params = {}) { + return this.send ('update', id, data, params.query || {}); } - patch (id: number | string, data: any, params: Params = {}) { - return this.send('patch', id, data, params.query || {}); + patch (id: NullableId, data: any, params: Params = {}) { + return this.send ('patch', id, data, params.query || {}); } - remove (id: number | string, params: Params = {}) { - return this.send('remove', id, params.query || {}); + remove (id: NullableId, params: Params = {}) { + return this.send ('remove', id, params.query || {}); } // `off` is actually not part of the Node event emitter spec diff --git a/packages/transport-commons/src/socket/utils.ts b/packages/transport-commons/src/socket/utils.ts index d9c485bbad..863f7ea701 100644 --- a/packages/transport-commons/src/socket/utils.ts +++ b/packages/transport-commons/src/socket/utils.ts @@ -1,6 +1,6 @@ import Debug from 'debug'; import isEqual from 'lodash/isEqual'; -import { NotFound, MethodNotAllowed } from '@feathersjs/errors'; +import { NotFound, MethodNotAllowed, BadRequest } from '@feathersjs/errors'; import { HookContext, Application, createContext, getServiceOptions } from '@feathersjs/feathers'; import { CombinedChannel } from '../channels/channel/combined'; import { RealTimeConnection } from '../channels/channel/base'; @@ -95,6 +95,11 @@ export async function runMethod (app: Application, connection: RealTimeConnectio // `params` have to be re-mapped to the query and added with the route const params = Object.assign({ query, route, connection }, connection); + // `params` is always the last parameter. Error if we got more arguments. + if (methodArgs.length > (position + 1)) { + throw new BadRequest(`Too many arguments for '${method}' method`); + } + methodArgs[position] = params; const ctx = createContext(service, method); diff --git a/packages/transport-commons/test/channels/index.test.ts b/packages/transport-commons/test/channels/index.test.ts index 3e25537922..a79aee4ace 100644 --- a/packages/transport-commons/test/channels/index.test.ts +++ b/packages/transport-commons/test/channels/index.test.ts @@ -38,7 +38,7 @@ describe('feathers-channels', () => { } }); - const service = app.service('test') as any; + const service: any = app.service('test') ; assert.ok(!service[keys.PUBLISHERS]); }); diff --git a/packages/transport-commons/test/client.test.ts b/packages/transport-commons/test/client.test.ts index 9c7b70df17..923e0d0313 100644 --- a/packages/transport-commons/test/client.test.ts +++ b/packages/transport-commons/test/client.test.ts @@ -1,14 +1,15 @@ import assert from 'assert'; import { EventEmitter } from 'events'; +import { CustomMethod } from '@feathersjs/feathers'; import { NotAuthenticated } from '@feathersjs/errors'; -import { Service } from '../src/client'; +import { Service, SocketService } from '../src/client'; declare type DummyCallback = (err: any, data?: any) => void; describe('client', () => { let connection: any; let testData: any; - let service: Service & EventEmitter; + let service: SocketService & CustomMethod<'customMethod'> & EventEmitter; beforeEach(() => { connection = new EventEmitter(); @@ -18,7 +19,7 @@ describe('client', () => { name: 'todos', method: 'emit', connection - }) as Service & EventEmitter; + }) as any; }); it('sets `events` property on service', () => { @@ -73,78 +74,76 @@ describe('client', () => { connection.emit('todos thing', testData); }); - it('sends all service methods with acknowledgement', () => { + it('sends all service and custom methods with acknowledgement', async () => { const idCb = (_path: any, id: any, _params: any, callback: DummyCallback) => callback(null, { id }); const idDataCb = (_path: any, _id: any, data: any, _params: any, callback: DummyCallback) => callback(null, data); - - connection.once('create', (_path: any, data: any, _params: any, callback: DummyCallback) => { + const dataCb = (_path: any, data: any, _params: any, callback: DummyCallback) => { data.created = true; callback(null, data); + }; + + connection.once('create', dataCb); + service.methods('customMethod'); + + let res = await service.create(testData); + + assert.ok(res.created); + + connection.once('get', idCb); + res = await service.get(1); + assert.deepStrictEqual(res, { id: 1 }); + + connection.once('remove', idCb); + res = await service.remove(12) + assert.deepStrictEqual(res, { id: 12 }); + + connection.once('update', idDataCb); + res = await service.update(12, testData); + assert.deepStrictEqual(res, testData); + + connection.once('patch', idDataCb); + res = await service.patch(12, testData); + assert.deepStrictEqual(res, testData); + + connection.once('customMethod', dataCb); + res = await service.customMethod({ message: 'test' }); + assert.deepStrictEqual(res, { + created: true, + message: 'test' }); - return service.create(testData) - .then((result: any) => assert.ok(result.created)) - .then(() => { - connection.once('get', idCb); - - return service.get(1) - .then(res => assert.deepStrictEqual(res, { id: 1 })); - }) - .then(() => { - connection.once('remove', idCb); - - return service.remove(12) - .then(res => assert.deepStrictEqual(res, { id: 12 })); - }) - .then(() => { - connection.once('update', idDataCb); - - return service.update(12, testData) - .then(res => assert.deepStrictEqual(res, testData)); - }) - .then(() => { - connection.once('patch', idDataCb); - - return service.patch(12, testData) - .then(res => assert.deepStrictEqual(res, testData)); - }) - .then(() => { - connection.once('find', (_path: any, params: any, callback: DummyCallback) => - callback(null, { params }) - ); - - return service.find({ query: { test: true } }).then((res: any) => - assert.deepStrictEqual(res, { - params: { test: true } - }) - ); - }); + connection.once('find', (_path: any, params: any, callback: DummyCallback) => + callback(null, { params }) + ); + + res = await service.find({ query: { test: true } }); + assert.deepStrictEqual(res, { + params: { test: true } + }); }); - it('converts to feathers-errors (#19)', () => { + it('converts to feathers-errors (#19)', async () => { connection.once('create', (_path: any, _data: any, _params: any, callback: DummyCallback) => callback(new NotAuthenticated('Test', { hi: 'me' }).toJSON()) ); - return service.create(testData).catch(error => { - assert.ok(error instanceof NotAuthenticated); - assert.strictEqual(error.name, 'NotAuthenticated'); - assert.strictEqual(error.message, 'Test'); - assert.strictEqual(error.code, 401); - assert.deepStrictEqual(error.data, { hi: 'me' }); + await assert.rejects(() => service.create(testData), { + name: 'NotAuthenticated', + message: 'Test', + code: 401, + data: { hi: 'me' } }); }); - it('converts other errors (#19)', () => { + it('converts other errors (#19)', async () => { connection.once('create', (_path: string, _data: any, _params: any, callback: (x: string) => void) => callback('Something went wrong') // eslint-disable-line ); - return service.create(testData).catch(error => { - assert.ok(error instanceof Error); - assert.strictEqual(error.message, 'Something went wrong'); + await assert.rejects(() => service.create(testData), { + message: 'Something went wrong' }); });