Preserve body on request object passed to willSendRequest; fix string and Buffer body handling

.changeset/cuddly-cars-sparkle.md

@@ -1,10 +1,17 @@
 ---
-'@apollo/datasource-rest': patch
+'@apollo/datasource-rest': major
 ---
 
-The v4 update introduced multiple regressions w.r.t. the intermediary `modifiedRequest` object that was added.
+This change restores the full functionality of `willSendRequest` which
+previously existed in the v3 version of this package. The v4 change introduced a
+regression where the incoming request's `body` was no longer included in the
+object passed to the `willSendRequest` hook, it was always `undefined`.
 
-1. The `body` was no longer being added to the intermediary request object before calling `willSendRequest`
-2. `modifiedRequest.body` was never being set in the case that the incoming body was a `string` or `Buffer`
+For consistency and typings reasons, the `path` argument is now the first
+argument to the `willSendRequest` hook, followed by the `AugmentedRequest`
+request object.
 
-This change resolves both by reverting to what we previously had in v3 (preserving the properties on the incoming request object). The types have been updated accordingly.
+Related to the regression mentioned above, `string` and `Buffer` bodies were no
+longer being included at all on the outgoing request since they were just
+ignored and never appended to the `body`. `string` and `Buffer` bodies are now
+passed through to the outgoing request (without being JSON stringified).

src/RESTDataSource.ts

@@ -82,6 +82,7 @@ export abstract class RESTDataSource {
  }
 
  protected willSendRequest?(
+ path: string,
  requestOpts: AugmentedRequest,
  ): ValueOrPromise<void>;
 
@@ -221,7 +222,7 @@ export abstract class RESTDataSource {
  };
 
  if (this.willSendRequest) {
- await this.willSendRequest(augmentedRequest);
+ await this.willSendRequest(path, augmentedRequest);
  }
 
  const url = await this.resolveURL(path, augmentedRequest);

src/__tests__/RESTDataSource.test.ts

@@ -195,7 +195,7 @@ describe('RESTDataSource', () => {
  super(config);
  }
 
- override willSendRequest(request: AugmentedRequest) {
+ override willSendRequest(_path: string, request: AugmentedRequest) {
  request.params.set('apiKey', this.token);
  }
 
@@ -215,7 +215,7 @@ describe('RESTDataSource', () => {
  const dataSource = new (class extends RESTDataSource {
  override baseURL = 'https://api.example.com';
 
- override willSendRequest(request: AugmentedRequest) {
+ override willSendRequest(_path: string, request: AugmentedRequest) {
  request.headers = { ...request.headers, credentials: 'include' };
  }
 
@@ -237,7 +237,7 @@ describe('RESTDataSource', () => {
  super(config);
  }
 
- override willSendRequest(request: AugmentedRequest) {
+ override willSendRequest(_path: string, request: AugmentedRequest) {
  request.headers = { ...request.headers, authorization: this.token };
  }
 
@@ -891,28 +891,54 @@ describe('RESTDataSource', () => {
 
  describe('user hooks', () => {
  describe('willSendRequest', () => {
- it('sees the same request body as provided by the caller', async () => {
+ const obj = { foo: 'bar' };
+ const str = 'foo=bar';
+ const buffer = Buffer.from(str);
+
+ it.each([
+ ['object', obj, obj],
+ ['string', str, str],
+ ['buffer', buffer, buffer.toJSON()],
+ ])(`can set the body to a %s`, async (_, body, expected) => {
  const dataSource = new (class extends RESTDataSource {
  override baseURL = apiUrl;
 
- updateFoo(id: number, foo: { name: string }) {
+ updateFoo(id: number, foo: string | Buffer | { foo: string }) {
  return this.post(`foo/${id}`, { body: foo });
  }
 
- override async willSendRequest(requestOpts: AugmentedRequest) {
- expect(requestOpts.body).toMatchInlineSnapshot(`
-  {
-  "name": "blah",
-  }
- `);
+ override async willSendRequest(
+ path: string,
+ requestOpts: AugmentedRequest,
+ ) {
+ expect(path).toMatch('foo/1');
+ expect(requestOpts.body).toEqual(body);
  }
  })();
 
- nock(apiUrl)
- .post('/foo/1', JSON.stringify({ name: 'blah' }))
- .reply(200);
- await dataSource.updateFoo(1, { name: 'blah' });
+ nock(apiUrl).post('/foo/1', expected).reply(200);
+ await dataSource.updateFoo(1, body);
  });
+
+ it('is called with the correct path', async () => {
+ const dataSource = new (class extends RESTDataSource {
+ override baseURL = apiUrl;
+
+ updateFoo(id: number, foo: { foo: string }) {
+ return this.post(`foo/${id}`, { body: foo });
+ }
+
+ override async willSendRequest(
+ path: string,
+ _requestOpts: AugmentedRequest,
+ ) {
+ expect(path).toMatch('foo/1');
+ }
+ })();
+
+ nock(apiUrl).post('/foo/1', obj).reply(200);
+ await dataSource.updateFoo(1, obj);
+ })
  });
 
  describe('resolveURL', () => {