From 39cd02e16aa104c892b304aa3982b39eb38aeb26 Mon Sep 17 00:00:00 2001
From: Richard Lau <rlau@redhat.com>
Date: Wed, 25 Aug 2021 20:09:02 +0100
Subject: [PATCH] fix: cache GitHub API requests by full URL

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);
 });