diff --git a/packages/rest-client/src/base.ts b/packages/rest-client/src/base.ts index aeb4bfb4cf..451b2f2008 100644 --- a/packages/rest-client/src/base.ts +++ b/packages/rest-client/src/base.ts @@ -1,6 +1,6 @@ import qs from 'qs' import { Params, Id, Query, NullableId, ServiceInterface } from '@feathersjs/feathers' -import { Unavailable, convert } from '@feathersjs/errors' +import { BadRequest, Unavailable, convert } from '@feathersjs/errors' import { _, stripSlashes } from '@feathersjs/commons' function toError(error: Error & { code: string }) { @@ -113,8 +113,8 @@ export abstract class Base< } _get(id: Id, params?: P) { - if (typeof id === 'undefined') { - return Promise.reject(new Error("id for 'get' can not be undefined")) + if (id === undefined || id === '') { + return Promise.reject(new BadRequest('id for .get is required')) } return this.request( @@ -148,10 +148,8 @@ export abstract class Base< } _update(id: NullableId, data: D, params?: P) { - if (typeof id === 'undefined') { - return Promise.reject( - new Error("id for 'update' can not be undefined, only 'null' when updating multiple entries") - ) + if (id === undefined || id === '') { + return Promise.reject(new BadRequest('id for .update is required')) } return this.request( @@ -170,10 +168,8 @@ export abstract class Base< } _patch(id: NullableId, data: D, params?: P) { - if (typeof id === 'undefined') { - return Promise.reject( - new Error("id for 'patch' can not be undefined, only 'null' when updating multiple entries") - ) + if (id === undefined || id === '') { + return Promise.reject(new BadRequest('id for .patch is required')) } return this.request( @@ -192,10 +188,8 @@ export abstract class Base< } _remove(id: NullableId, params?: P) { - if (typeof id === 'undefined') { - return Promise.reject( - new Error("id for 'remove' can not be undefined, only 'null' when removing multiple entries") - ) + if (id === undefined || id === '') { + return Promise.reject(new BadRequest('id for .remove is required')) } return this.request( diff --git a/packages/rest-client/test/index.test.ts b/packages/rest-client/test/index.test.ts index d81c98407b..6a88857054 100644 --- a/packages/rest-client/test/index.test.ts +++ b/packages/rest-client/test/index.test.ts @@ -46,25 +46,41 @@ describe('REST client tests', function () { } }) - it('errors when id property for get, patch, update or remove is undefined', async () => { + it('errors when id property for get, patch, update or remove is undefined or empty string', async () => { const app = feathers().configure(init('http://localhost:8889').fetch(fetch)) const service = app.service('todos') await assert.rejects(() => service.get(undefined), { - message: "id for 'get' can not be undefined" + message: 'id for .get is required' + }) + + await assert.rejects(() => service.get(''), { + message: 'id for .get is required' }) await assert.rejects(() => service.remove(undefined), { - message: "id for 'remove' can not be undefined, only 'null' when removing multiple entries" + message: 'id for .remove is required' + }) + + await assert.rejects(() => service.remove(''), { + message: 'id for .remove is required' }) await assert.rejects(() => service.update(undefined, {}), { - message: "id for 'update' can not be undefined, only 'null' when updating multiple entries" + message: 'id for .update is required' + }) + + await assert.rejects(() => service.update('', {}), { + message: 'id for .update is required' }) await assert.rejects(() => service.patch(undefined, {}), { - message: "id for 'patch' can not be undefined, only 'null' when updating multiple entries" + message: 'id for .patch is required' + }) + + await assert.rejects(() => service.patch('', {}), { + message: 'id for .patch is required' }) }) diff --git a/packages/transport-commons/src/client.ts b/packages/transport-commons/src/client.ts index 8ce2aae616..29d467b8fa 100644 --- a/packages/transport-commons/src/client.ts +++ b/packages/transport-commons/src/client.ts @@ -1,5 +1,5 @@ /* eslint-disable @typescript-eslint/ban-ts-comment */ -import { convert } from '@feathersjs/errors' +import { BadRequest, convert } from '@feathersjs/errors' import { createDebug } from '@feathersjs/commons' import { Id, NullableId, Params, ServiceInterface } from '@feathersjs/feathers' @@ -128,6 +128,10 @@ export class Service, P extends Params = Params> impleme } _get(id: Id, params: Params = {}) { + if (id === undefined || id === '') { + return Promise.reject(new BadRequest('id for .get is required')) + } + return this.send('get', id, params.query || {}, params.route || {}) } @@ -144,17 +148,26 @@ export class Service, P extends Params = Params> impleme } _update(id: NullableId, data: D, params: Params = {}) { - if (typeof id === 'undefined') { - return Promise.reject(new Error("id for 'update' can not be undefined")) + if (id === undefined || id === '') { + return Promise.reject(new BadRequest('id for .update is required')) } + return this.send('update', id, data, params.query || {}, params.route || {}) } update(id: NullableId, data: D, params: Params = {}) { + if (id === undefined || id === '') { + return Promise.reject(new BadRequest('id for .update is required')) + } + return this._update(id, data, params) } _patch(id: NullableId, data: D, params: Params = {}) { + if (id === undefined || id === '') { + return Promise.reject(new BadRequest('id for .patch is required')) + } + return this.send('patch', id, data, params.query || {}, params.route || {}) } @@ -163,6 +176,10 @@ export class Service, P extends Params = Params> impleme } _remove(id: NullableId, params: Params = {}) { + if (id === undefined || id === '') { + return Promise.reject(new BadRequest('id for .remove is required')) + } + return this.send('remove', id, params.query || {}, params.route || {}) } diff --git a/packages/transport-commons/test/client.test.ts b/packages/transport-commons/test/client.test.ts index ea307db776..de851a8826 100644 --- a/packages/transport-commons/test/client.test.ts +++ b/packages/transport-commons/test/client.test.ts @@ -27,6 +27,64 @@ describe('client', () => { assert.ok(service.events) }) + it('throws for .get, .update, .patch and .remove with undefined or empty string id', async () => { + try { + await service.get(undefined) + assert.ok(false, 'Should never get here') + } catch (e: any) { + assert.strictEqual(e.message, 'id for .get is required') + } + + try { + await service.get('') + assert.ok(false, 'Should never get here') + } catch (e: any) { + assert.strictEqual(e.message, 'id for .get is required') + } + + try { + await service.update(undefined, testData) + assert.ok(false, 'Should never get here') + } catch (e: any) { + assert.strictEqual(e.message, 'id for .update is required') + } + + try { + await service.update('', testData) + assert.ok(false, 'Should never get here') + } catch (e: any) { + assert.strictEqual(e.message, 'id for .update is required') + } + + try { + await service.patch(undefined, testData) + assert.ok(false, 'Should never get here') + } catch (e: any) { + assert.strictEqual(e.message, 'id for .patch is required') + } + + try { + await service.patch('', testData) + assert.ok(false, 'Should never get here') + } catch (e: any) { + assert.strictEqual(e.message, 'id for .patch is required') + } + + try { + await service.remove(undefined) + assert.ok(false, 'Should never get here') + } catch (e: any) { + assert.strictEqual(e.message, 'id for .remove is required') + } + + try { + await service.remove('') + assert.ok(false, 'Should never get here') + } catch (e: any) { + assert.strictEqual(e.message, 'id for .remove is required') + } + }) + it('throws an error when the emitter does not have the method', () => { const clientService = new Service({ name: 'todos',