From ada71160a746a0e0e541346931de6a73c4d7ee95 Mon Sep 17 00:00:00 2001
From: Richard Lau <rlau@redhat.com>
Date: Fri, 27 Aug 2021 10:34:42 +0100
Subject: [PATCH] fix: cache GitHub API requests by full URL (#65)

Cache GitHub API requests based on the URL fetched and not the passed
in target.

Also extends tests to cover caching for the non-GitHub API. Update
test URLs to use domains reserved for testing [1]. Clear the cache
after every test.

[1] https://www.rfc-editor.org/rfc/rfc2606.html
---
 src/lib/network.js     |  8 +++----
 test/lib/fetch.test.js | 50 +++++++++++++++++++++++++++++++++++++-----
 2 files changed, 48 insertions(+), 10 deletions(-)

diff --git a/src/lib/network.js b/src/lib/network.js
index 06b5ca9..68e8175 100644
--- a/src/lib/network.js
+++ b/src/lib/network.js
@@ -15,11 +15,11 @@ const fetch = async (url) => {
 };
 
 const fetchGithub = async (target, token, override = false) => {
-  // check if we already have the response in cache
-  if (cache.has(target)) return cache.get(target).data;
-
   const github = 'https://api.github.com';
   const url = override ? target : `${github}${target}`;
+  // check if we already have the response in cache
+  if (cache.has(url)) return cache.get(url).data;
+
   const options = {
     headers: {
       Authorization: token ? `token ${token}` : ''
@@ -27,7 +27,7 @@ const fetchGithub = async (target, token, override = false) => {
   };
   const response = await axios.get(url, options);
   // save response in cache for future usage
-  cache.set(target, response);
+  cache.set(url, response);
   return response.data;
 };
 
diff --git a/test/lib/fetch.test.js b/test/lib/fetch.test.js
index a55c3c0..86ed525 100644
--- a/test/lib/fetch.test.js
+++ b/test/lib/fetch.test.js
@@ -6,6 +6,7 @@ const network = require('../../src/lib/network');
 jest.mock('axios');
 
 afterEach(() => {
+  network.clearCache();
   jest.clearAllMocks();
 });
 
@@ -36,11 +37,10 @@ it('should not prepend the url with github api base when override is true', asyn
     };
   });
 
-  await network.fetchGithub('https://sample.com/test/test', null, true);
-  network.clearCache();
+  await network.fetchGithub('https://example.test/test/test', null, true);
 
   expect(axios.get).toHaveBeenCalled();
-  expect(axios.get).toHaveBeenCalledWith('https://sample.com/test/test', {
+  expect(axios.get).toHaveBeenCalledWith('https://example.test/test/test', {
     headers: {
       Authorization: ''
     }
@@ -58,7 +58,6 @@ it('should pass the auth token to the request header', async () => {
   const token = 'gh_123456789123456789';
 
   await network.fetchGithub('/test/test', token);
-  network.clearCache();
 
   expect(axios.get).toHaveBeenCalled();
   expect(axios.get).toHaveBeenCalledWith('https://api.github.com/test/test', {
@@ -76,9 +75,48 @@ it('should use the cache on requests with the same target', async () => {
     };
   });
 
-  const content1 = await network.fetchGithub('https://sample.com/test/test');
-  const content2 = await network.fetchGithub('https://sample.com/test/test');
+  const content1 = await network.fetch('https://example.test/test/test');
+  const content2 = await network.fetch('https://example.test/test/test');
+
+  expect(axios.get).toHaveBeenCalledTimes(1);
+  expect(axios.get).toHaveBeenCalledWith('https://example.test/test/test');
+  expect(content2).toBe(content1);
+});
+
+it('should use the cache on Github API requests with the same target', async () => {
+  // mocking axios get request
+  axios.get.mockImplementation(() => {
+    return {
+      data: ''
+    };
+  });
+
+  const content1 = await network.fetchGithub('/repos/nodeshift/npcheck');
+  const content2 = await network.fetchGithub('/repos/nodeshift/npcheck');
+
+  expect(axios.get).toHaveBeenCalledTimes(1);
+  expect(axios.get).toHaveBeenCalledWith(
+    'https://api.github.com/repos/nodeshift/npcheck',
+    { headers: { 'Authorization': '' }}
+  );
+  expect(content2).toBe(content1);
+});
+
+it('should share cache between override and non-override Github API requests', async () => {
+  // mocking axios get request
+  axios.get.mockImplementation(() => {
+    return {
+      data: ''
+    };
+  });
+
+  const content1 = await network.fetchGithub('https://api.github.com/repos/nodeshift/npcheck', null, true);
+  const content2 = await network.fetchGithub('/repos/nodeshift/npcheck');
 
   expect(axios.get).toHaveBeenCalledTimes(1);
+  expect(axios.get).toHaveBeenCalledWith(
+    'https://api.github.com/repos/nodeshift/npcheck',
+    { headers: { 'Authorization': '' }}
+  );
   expect(content2).toBe(content1);
 });