From 8af22fe02edf5b1ac776bbb85029cb7399f0c604 Mon Sep 17 00:00:00 2001
From: David Glasser <glasser@apollographql.com>
Date: Mon, 5 Dec 2022 10:06:18 -0800
Subject: [PATCH] HTTPCache: FetcherResponse.url should exist consistently
 (#105)

Previously, the `url` on the `FetcherResponse` returned from an
HTTPCache would be set properly if this was an actual fetch, but set to
'' if it were read from the cache.

This issue was introduced in
https://github.com/apollographql/apollo-server/pull/1362 (as part of the
initial creation of RESTDataSource) as part of allowing for custom cache
keys.

Fixes #35.
---
 .changeset/flat-roses-draw.md   | 5 +++++
 src/HTTPCache.ts                | 7 +++++--
 src/__tests__/HTTPCache.test.ts | 4 +++-
 3 files changed, 13 insertions(+), 3 deletions(-)
 create mode 100644 .changeset/flat-roses-draw.md

diff --git a/.changeset/flat-roses-draw.md b/.changeset/flat-roses-draw.md
new file mode 100644
index 0000000..71b6b22
--- /dev/null
+++ b/.changeset/flat-roses-draw.md
@@ -0,0 +1,5 @@
+---
+'@apollo/datasource-rest': patch
+---
+
+The fetch Response now consistently has a non-empty `url` property; previously, `url` was an empty string if the response was read from the HTTP cache.
diff --git a/src/HTTPCache.ts b/src/HTTPCache.ts
index 825387e..575ef7e 100644
--- a/src/HTTPCache.ts
+++ b/src/HTTPCache.ts
@@ -67,7 +67,10 @@ export class HTTPCache {
     const { policy: policyRaw, ttlOverride, body } = JSON.parse(entry);
 
     const policy = CachePolicy.fromObject(policyRaw);
-    // Remove url from the policy, because otherwise it would never match a request with a custom cache key
+    // Remove url from the policy, because otherwise it would never match a
+    // request with a custom cache key (ie, we want users to be able to tell us
+    // that two requests should be treated as the same even if the URL differs).
+    const urlFromPolicy = policy._url;
     policy._url = undefined;
 
     if (
@@ -79,7 +82,7 @@ export class HTTPCache {
     ) {
       const headers = policy.responseHeaders();
       return new Response(body, {
-        url: policy._url,
+        url: urlFromPolicy,
         status: policy._status,
         headers,
       });
diff --git a/src/__tests__/HTTPCache.test.ts b/src/__tests__/HTTPCache.test.ts
index 7dd4406..02a2720 100644
--- a/src/__tests__/HTTPCache.test.ts
+++ b/src/__tests__/HTTPCache.test.ts
@@ -66,12 +66,14 @@ describe('HTTPCache', () => {
   it('returns a cached response when not expired', async () => {
     mockGetAdaLovelace({ 'cache-control': 'max-age=30' });
 
-    await httpCache.fetch(adaUrl);
+    const firstResponse = await httpCache.fetch(adaUrl);
+    expect(firstResponse.url).toBe(adaUrl.toString());
 
     jest.advanceTimersByTime(10000);
 
     const response = await httpCache.fetch(adaUrl);
 
+    expect(response.url).toBe(adaUrl.toString());
     expect(await response.json()).toEqual({ name: 'Ada Lovelace' });
     expect(response.headers.get('age')).toEqual('10');
   });