Skip to content

Commit

Permalink
feat(core): Public custom service methods (#2270)
Browse files Browse the repository at this point in the history
  • Loading branch information
daffl authored Mar 28, 2021
1 parent 28114c4 commit e65abfb
Show file tree
Hide file tree
Showing 27 changed files with 347 additions and 199 deletions.
2 changes: 1 addition & 1 deletion packages/express/src/declarations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export type Application<ServiceTypes = any, AppSettings = any> =

declare module '@feathersjs/feathers/lib/declarations' {
export interface ServiceOptions {
middleware: {
middleware?: {
before: express.RequestHandler[],
after: express.RequestHandler[]
}
Expand Down
12 changes: 9 additions & 3 deletions packages/express/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,15 @@ export default function feathersExpress<S = any, C = any> (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');
}
Expand All @@ -62,7 +65,10 @@ export default function feathersExpress<S = any, C = any> (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;
},
Expand Down
11 changes: 5 additions & 6 deletions packages/express/src/rest.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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.`);
Expand Down
17 changes: 9 additions & 8 deletions packages/express/test/authentication.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
141 changes: 75 additions & 66 deletions packages/express/test/rest.test.ts
Original file line number Diff line number Diff line change
@@ -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', () => {
Expand Down Expand Up @@ -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) => {
Expand Down Expand Up @@ -526,7 +529,7 @@ describe('@feathersjs/express/rest provider', () => {
};
}
})
.use(express.errorHandler());
.use(errorHandler);

server = await app.listen(6880);
});
Expand Down Expand Up @@ -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');
});
});
});
4 changes: 4 additions & 0 deletions packages/feathers/src/declarations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,10 @@ export interface ServiceHookOverloads<S> {
export type FeathersService<A = FeathersApplication, S = Service<any>> =
S & ServiceAddons<A, S> & OptionalPick<ServiceHookOverloads<S>, keyof S>;

export type CustomMethod<Methods extends string> = {
[k in Methods]: <X = any> (data: any, params?: Params) => Promise<X>;
}

export type ServiceMixin<A> = (service: FeathersService<A>, path: string, options?: ServiceOptions) => void;

export type ServiceGenericType<S> = S extends ServiceInterface<infer T> ? T : any;
Expand Down
4 changes: 2 additions & 2 deletions packages/feathers/src/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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}\``);
}

Expand Down
1 change: 0 additions & 1 deletion packages/rest-client/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
37 changes: 27 additions & 10 deletions packages/rest-client/src/base.ts
Original file line number Diff line number Diff line change
@@ -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') {
Expand All @@ -20,7 +18,7 @@ interface RestClientSettings {
options: any;
}

export abstract class Base {
export abstract class Base<T = any, D = Partial<T>> implements ServiceInterface<T, D> {
name: string;
base: string;
connection: any;
Expand All @@ -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)}`;
}
Expand All @@ -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),
Expand All @@ -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,
Expand All @@ -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'));
}
Expand All @@ -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'));
}
Expand Down
Loading

0 comments on commit e65abfb

Please # to comment.