diff --git a/.changeset/funny-dolls-cheer.md b/.changeset/funny-dolls-cheer.md new file mode 100644 index 0000000..bd43760 --- /dev/null +++ b/.changeset/funny-dolls-cheer.md @@ -0,0 +1,5 @@ +--- +'@apollo/datasource-rest': patch +--- + +Correctly identify and serialize all plain objects (like those with a null prototype) \ No newline at end of file diff --git a/.changeset/wet-birds-occur.md b/.changeset/wet-birds-occur.md new file mode 100644 index 0000000..6af77a3 --- /dev/null +++ b/.changeset/wet-birds-occur.md @@ -0,0 +1,5 @@ +--- +'@apollo/datasource-rest': minor +--- + +Add a new overridable method `shouldJSONSerializeBody` for customizing body serialization behavior. This method should return a `boolean` in order to inform RESTDataSource as to whether or not it should call `JSON.stringify` on the request body. diff --git a/README.md b/README.md index 2443cc7..2c00350 100644 --- a/README.md +++ b/README.md @@ -162,6 +162,13 @@ If you override this behavior, be sure to implement the proper error handling. ##### `didEncounterError` By default, this method just throws the `error` it was given. If you override this method, you can choose to either perform some additional logic and still throw, or to swallow the error by not throwing the error result. +#### `shouldJSONSerializeBody` +By default, this method returns `true` if the request body is: +- a plain object or an array +- an object with a `toJSON` method (which isn't a `Buffer` or an instance of a class named `FormData`) + +You can override this method in order to serialize other objects such as custom classes as JSON. + ### HTTP Methods The `get` method on the [`RESTDataSource`](https://github.com/apollographql/datasource-rest/tree/main/src/RESTDataSource.ts) makes an HTTP `GET` request. Similarly, there are methods built-in to allow for `POST`, `PUT`, `PATCH`, and `DELETE` requests. diff --git a/cspell-dict.txt b/cspell-dict.txt index ae86530..862e3a5 100644 --- a/cspell-dict.txt +++ b/cspell-dict.txt @@ -9,7 +9,9 @@ circleci codesandbox datasource direnv +falsey httpcache +isplainobject keyvaluecache opde revalidates diff --git a/package-lock.json b/package-lock.json index fb92b1b..7a756f6 100644 --- a/package-lock.json +++ b/package-lock.json @@ -12,6 +12,7 @@ "@apollo/utils.fetcher": "^1.0.0", "@apollo/utils.keyvaluecache": "1.0.2", "http-cache-semantics": "^4.1.0", + "lodash.isplainobject": "^4.0.6", "node-fetch": "^2.6.7" }, "devDependencies": { @@ -21,8 +22,10 @@ "@changesets/cli": "2.25.2", "@types/http-cache-semantics": "4.0.1", "@types/jest": "29.2.3", + "@types/lodash.isplainobject": "4.0.7", "@types/node": "14.18.33", "cspell": "6.15.0", + "form-data": "4.0.0", "graphql": "16.6.0", "jest": "29.3.1", "jest-junit": "14.0.1", @@ -2697,6 +2700,21 @@ "pretty-format": "^29.0.0" } }, + "node_modules/@types/lodash": { + "version": "4.14.188", + "resolved": "https://registry.npmjs.org/@types/lodash/-/lodash-4.14.188.tgz", + "integrity": "sha512-zmEmF5OIM3rb7SbLCFYoQhO4dGt2FRM9AMkxvA3LaADOF1n8in/zGJlWji9fmafLoNyz+FoL6FE0SLtGIArD7w==", + "dev": true + }, + "node_modules/@types/lodash.isplainobject": { + "version": "4.0.7", + "resolved": "https://registry.npmjs.org/@types/lodash.isplainobject/-/lodash.isplainobject-4.0.7.tgz", + "integrity": "sha512-fdHtdjpy7fXydEaRHx1dPa+2AVmLbmk2Gv7f0w/90BKCH0DkWo8pIrzygnB3rvgo6oKNb3cO9VaPpj9GLCr5Ug==", + "dev": true, + "dependencies": { + "@types/lodash": "*" + } + }, "node_modules/@types/long": { "version": "4.0.2", "resolved": "https://registry.npmjs.org/@types/long/-/long-4.0.2.tgz", @@ -2731,6 +2749,20 @@ "form-data": "^3.0.0" } }, + "node_modules/@types/node-fetch/node_modules/form-data": { + "version": "3.0.1", + "resolved": "https://registry.npmjs.org/form-data/-/form-data-3.0.1.tgz", + "integrity": "sha512-RHkBKtLWUVwd7SqRIvCZMEvAMoGUp0XU+seQiZejj0COz3RI3hWP4sCv3gZWWLjJTd7rGwcsF5eKZGii0r/hbg==", + "dev": true, + "dependencies": { + "asynckit": "^0.4.0", + "combined-stream": "^1.0.8", + "mime-types": "^2.1.12" + }, + "engines": { + "node": ">= 6" + } + }, "node_modules/@types/normalize-package-data": { "version": "2.4.1", "resolved": "https://registry.npmjs.org/@types/normalize-package-data/-/normalize-package-data-2.4.1.tgz", @@ -4906,9 +4938,9 @@ "dev": true }, "node_modules/form-data": { - "version": "3.0.1", - "resolved": "https://registry.npmjs.org/form-data/-/form-data-3.0.1.tgz", - "integrity": "sha512-RHkBKtLWUVwd7SqRIvCZMEvAMoGUp0XU+seQiZejj0COz3RI3hWP4sCv3gZWWLjJTd7rGwcsF5eKZGii0r/hbg==", + "version": "4.0.0", + "resolved": "https://registry.npmjs.org/form-data/-/form-data-4.0.0.tgz", + "integrity": "sha512-ETEklSGi5t0QMZuiXoA/Q6vcnxcLQP5vdugSpuAyi6SVGi2clPPp+xgEhuMaHC+zGgn31Kd235W35f7Hykkaww==", "dev": true, "dependencies": { "asynckit": "^0.4.0", @@ -7613,6 +7645,11 @@ "integrity": "sha512-v2kDEe57lecTulaDIuNTPy3Ry4gLGJ6Z1O3vE1krgXZNrsQ+LFTGHVxVjcXPs17LhbZVGedAJv8XZ1tvj5FvSg==", "dev": true }, + "node_modules/lodash.isplainobject": { + "version": "4.0.6", + "resolved": "https://registry.npmjs.org/lodash.isplainobject/-/lodash.isplainobject-4.0.6.tgz", + "integrity": "sha512-oSXzaWypCMHkPC3NvBEaPHf0KsA5mvPrOPgQWDsbg8n7orZ290M0BmC/jgRZ4vcJ6DTAhjrsSYgdsW/F+MFOBA==" + }, "node_modules/lodash.memoize": { "version": "4.1.2", "resolved": "https://registry.npmjs.org/lodash.memoize/-/lodash.memoize-4.1.2.tgz", @@ -12259,6 +12296,21 @@ "pretty-format": "^29.0.0" } }, + "@types/lodash": { + "version": "4.14.188", + "resolved": "https://registry.npmjs.org/@types/lodash/-/lodash-4.14.188.tgz", + "integrity": "sha512-zmEmF5OIM3rb7SbLCFYoQhO4dGt2FRM9AMkxvA3LaADOF1n8in/zGJlWji9fmafLoNyz+FoL6FE0SLtGIArD7w==", + "dev": true + }, + "@types/lodash.isplainobject": { + "version": "4.0.7", + "resolved": "https://registry.npmjs.org/@types/lodash.isplainobject/-/lodash.isplainobject-4.0.7.tgz", + "integrity": "sha512-fdHtdjpy7fXydEaRHx1dPa+2AVmLbmk2Gv7f0w/90BKCH0DkWo8pIrzygnB3rvgo6oKNb3cO9VaPpj9GLCr5Ug==", + "dev": true, + "requires": { + "@types/lodash": "*" + } + }, "@types/long": { "version": "4.0.2", "resolved": "https://registry.npmjs.org/@types/long/-/long-4.0.2.tgz", @@ -12291,6 +12343,19 @@ "requires": { "@types/node": "*", "form-data": "^3.0.0" + }, + "dependencies": { + "form-data": { + "version": "3.0.1", + "resolved": "https://registry.npmjs.org/form-data/-/form-data-3.0.1.tgz", + "integrity": "sha512-RHkBKtLWUVwd7SqRIvCZMEvAMoGUp0XU+seQiZejj0COz3RI3hWP4sCv3gZWWLjJTd7rGwcsF5eKZGii0r/hbg==", + "dev": true, + "requires": { + "asynckit": "^0.4.0", + "combined-stream": "^1.0.8", + "mime-types": "^2.1.12" + } + } } }, "@types/normalize-package-data": { @@ -13986,9 +14051,9 @@ "dev": true }, "form-data": { - "version": "3.0.1", - "resolved": "https://registry.npmjs.org/form-data/-/form-data-3.0.1.tgz", - "integrity": "sha512-RHkBKtLWUVwd7SqRIvCZMEvAMoGUp0XU+seQiZejj0COz3RI3hWP4sCv3gZWWLjJTd7rGwcsF5eKZGii0r/hbg==", + "version": "4.0.0", + "resolved": "https://registry.npmjs.org/form-data/-/form-data-4.0.0.tgz", + "integrity": "sha512-ETEklSGi5t0QMZuiXoA/Q6vcnxcLQP5vdugSpuAyi6SVGi2clPPp+xgEhuMaHC+zGgn31Kd235W35f7Hykkaww==", "dev": true, "requires": { "asynckit": "^0.4.0", @@ -15961,6 +16026,11 @@ "integrity": "sha512-v2kDEe57lecTulaDIuNTPy3Ry4gLGJ6Z1O3vE1krgXZNrsQ+LFTGHVxVjcXPs17LhbZVGedAJv8XZ1tvj5FvSg==", "dev": true }, + "lodash.isplainobject": { + "version": "4.0.6", + "resolved": "https://registry.npmjs.org/lodash.isplainobject/-/lodash.isplainobject-4.0.6.tgz", + "integrity": "sha512-oSXzaWypCMHkPC3NvBEaPHf0KsA5mvPrOPgQWDsbg8n7orZ290M0BmC/jgRZ4vcJ6DTAhjrsSYgdsW/F+MFOBA==" + }, "lodash.memoize": { "version": "4.1.2", "resolved": "https://registry.npmjs.org/lodash.memoize/-/lodash.memoize-4.1.2.tgz", diff --git a/package.json b/package.json index b15cfbf..b1d7ba4 100644 --- a/package.json +++ b/package.json @@ -37,8 +37,10 @@ "@changesets/cli": "2.25.2", "@types/http-cache-semantics": "4.0.1", "@types/jest": "29.2.3", + "@types/lodash.isplainobject": "4.0.7", "@types/node": "14.18.33", "cspell": "6.15.0", + "form-data": "4.0.0", "graphql": "16.6.0", "jest": "29.3.1", "jest-junit": "14.0.1", @@ -52,6 +54,7 @@ "@apollo/utils.fetcher": "^1.0.0", "@apollo/utils.keyvaluecache": "1.0.2", "http-cache-semantics": "^4.1.0", + "lodash.isplainobject": "^4.0.6", "node-fetch": "^2.6.7" }, "peerDependencies": { diff --git a/src/RESTDataSource.ts b/src/RESTDataSource.ts index bba73d6..a69e65e 100644 --- a/src/RESTDataSource.ts +++ b/src/RESTDataSource.ts @@ -1,12 +1,13 @@ -import { HTTPCache } from './HTTPCache'; -import { GraphQLError } from 'graphql'; -import type { KeyValueCache } from '@apollo/utils.keyvaluecache'; import type { Fetcher, FetcherRequestInit, FetcherResponse, } from '@apollo/utils.fetcher'; +import type { KeyValueCache } from '@apollo/utils.keyvaluecache'; import type { WithRequired } from '@apollo/utils.withrequired'; +import { GraphQLError } from 'graphql'; +import isPlainObject from 'lodash.isplainobject'; +import { HTTPCache } from './HTTPCache'; type ValueOrPromise = T | Promise; @@ -208,6 +209,25 @@ export abstract class RESTDataSource { } } + protected shouldJSONSerializeBody(body: RequestWithBody['body']): boolean { + return !!( + // We accept arbitrary objects and arrays as body and serialize them as JSON. + ( + Array.isArray(body) || + isPlainObject(body) || + // We serialize any objects that have a toJSON method (except Buffers or things that look like FormData) + (body && + typeof body === 'object' && + 'toJSON' in body && + typeof (body as any).toJSON === 'function' && + !(body instanceof Buffer) && + // XXX this is a bit of a hacky check for FormData-like objects (in + // case a FormData implementation has a toJSON method on it) + (body as any).constructor?.name !== 'FormData') + ) + ); + } + protected async errorFromResponse(response: FetcherResponse) { const message = `${response.status}: ${response.statusText}`; @@ -308,16 +328,7 @@ export abstract class RESTDataSource { url.searchParams.append(name, value); } - // We accept arbitrary objects and arrays as body and serialize them as JSON. - // `string`, `Buffer`, and `undefined` are passed through up above as-is. - if ( - augmentedRequest.body != null && - !(augmentedRequest.body instanceof Buffer) && - (augmentedRequest.body.constructor === Object || - Array.isArray(augmentedRequest.body) || - ((augmentedRequest.body as any).toJSON && - typeof (augmentedRequest.body as any).toJSON === 'function')) - ) { + if (this.shouldJSONSerializeBody(augmentedRequest.body)) { augmentedRequest.body = JSON.stringify(augmentedRequest.body); // If Content-Type header has not been previously set, set to application/json if (!augmentedRequest.headers) { diff --git a/src/__tests__/RESTDataSource.test.ts b/src/__tests__/RESTDataSource.test.ts index 1697cff..2340800 100644 --- a/src/__tests__/RESTDataSource.test.ts +++ b/src/__tests__/RESTDataSource.test.ts @@ -3,15 +3,16 @@ import { AuthenticationError, CacheOptions, DataSourceConfig, - RequestDeduplicationPolicy, ForbiddenError, + RequestDeduplicationPolicy, RequestOptions, RESTDataSource, } from '../RESTDataSource'; -import { nockAfterEach, nockBeforeEach } from './nockAssertions'; -import nock from 'nock'; +import FormData from 'form-data'; import { GraphQLError } from 'graphql'; +import nock from 'nock'; +import { nockAfterEach, nockBeforeEach } from './nockAssertions'; const apiUrl = 'https://api.example.com'; @@ -322,7 +323,7 @@ describe('RESTDataSource', () => { await dataSource.postFoo(model); }); - it('does not serialize a request body that is not an object', async () => { + it('does not serialize FormData', async () => { const dataSource = new (class extends RESTDataSource { override baseURL = 'https://api.example.com'; @@ -331,10 +332,17 @@ describe('RESTDataSource', () => { } })(); - class FormData {} const form = new FormData(); + form.append('foo', 'bar'); - nock(apiUrl).post('/foo').reply(200); + nock(apiUrl) + .post('/foo', (body) => { + expect(body).toMatch( + 'Content-Disposition: form-data; name="foo"\r\n\r\nbar\r\n', + ); + return true; + }) + .reply(200); await dataSource.postFoo(form); }); @@ -382,6 +390,26 @@ describe('RESTDataSource', () => { await dataSource.updateFoo(1, Buffer.from(expectedData)); }); + it('serializes a request body that is an object with a null prototype', async () => { + interface Foo { + hello: string; + } + const dataSource = new (class extends RESTDataSource { + override baseURL = 'https://api.example.com'; + + postFoo(foo: Foo) { + return this.post('foo', { body: foo }); + } + })(); + + const foo: Foo = Object.create(null); + foo.hello = 'world'; + + nock(apiUrl).post('/foo', { hello: 'world' }).reply(200); + + await dataSource.postFoo(foo); + }); + describe('all methods', () => { const dataSource = new (class extends RESTDataSource { override baseURL = 'https://api.example.com'; @@ -1093,6 +1121,30 @@ describe('RESTDataSource', () => { await dataSource.updateFoo(1, { name: 'blah' }); }); }); + + describe('shouldJSONSerializeBody', () => { + it('can be overridden', async () => { + let calls = 0; + const dataSource = new (class extends RESTDataSource { + override baseURL = apiUrl; + + updateFoo(id: number, foo: { name: string }) { + return this.post(`foo/${id}`, { body: foo }); + } + + override shouldJSONSerializeBody( + body: string | object | Buffer | undefined, + ) { + calls++; + return super.shouldJSONSerializeBody(body); + } + })(); + + nock(apiUrl).post('/foo/1', { name: 'bar' }).reply(200); + await dataSource.updateFoo(1, { name: 'bar' }); + expect(calls).toBe(1); + }); + }); }); }); });