diff --git a/api/index.js b/api/index.js index b449d43b49080..1da9f57824b22 100644 --- a/api/index.js +++ b/api/index.js @@ -48,12 +48,13 @@ export default async (req, res) => { } try { - const stats = await fetchStats( + const statsResp = await fetchStats( username, parseBoolean(count_private), parseBoolean(include_all_commits), parseArray(exclude_repo), ); + const stats = statsResp.stats; const cacheSeconds = clampValue( parseInt(cache_seconds || CONSTANTS.FOUR_HOURS, 10), @@ -61,12 +62,18 @@ export default async (req, res) => { CONSTANTS.ONE_DAY, ); - res.setHeader( - "Cache-Control", - `max-age=${ - cacheSeconds / 2 - }, s-maxage=${cacheSeconds}, stale-while-revalidate=${CONSTANTS.ONE_DAY}`, - ); + if (statsResp.success) { + res.setHeader( + "Cache-Control", + `max-age=${ + cacheSeconds / 2 + }, s-maxage=${cacheSeconds}, stale-while-revalidate=${ + CONSTANTS.ONE_DAY + }`, + ); + } else { + res.setHeader("Cache-Control", `no-cache, no-store, must-revalidate`); // Don't cache unsuccessful responses. + } return res.send( renderStatsCard(stats, { diff --git a/src/fetchers/stats-fetcher.js b/src/fetchers/stats-fetcher.js index 7f6cb9e5e95b4..ad9e0e9c376eb 100644 --- a/src/fetchers/stats-fetcher.js +++ b/src/fetchers/stats-fetcher.js @@ -63,8 +63,8 @@ const fetcher = (variables, token) => { * Fetch first 100 repositories for a given username. * * @param {import('axios').AxiosRequestHeaders} variables Fetcher variables. - * @param {string} token GitHub token. - * @returns {Promise} Repositories fetcher response. + * @param {string} token Github token. + * @returns {Promise} Repositories fetcher response. */ const repositoriesFetcher = (variables, token) => { return request( @@ -95,46 +95,54 @@ const repositoriesFetcher = (variables, token) => { ); }; +/** + * Fetch all commits for a given username. + * + * @param {import('axios').AxiosRequestHeaders} variables Fetcher variables. + * @param {string} token Github token. + * @returns {Promise} Total commits fetcher response. + * + * @see https://developer.github.com/v3/search/#search-commits. + */ +const totalCommitsFetcher = (variables, token) => { + return axios({ + method: "get", + url: `https://api.github.com/search/commits?q=author:${variables.login}`, + headers: { + "Content-Type": "application/json", + Accept: "application/vnd.github.cloak-preview", + Authorization: `token ${token}`, + }, + }); +}; + /** * Fetch all the commits for all the repositories of a given username. * - * @param {*} username GitHub username. - * @returns {Promise} Total commits. + * @param {*} username Github username. + * @returns {Promise} Object containing the total number of commits and a success boolean. * * @description Done like this because the GitHub API does not provide a way to fetch all the commits. See * #92#issuecomment-661026467 and #211 for more information. */ -const totalCommitsFetcher = async (username) => { +const fetchTotalCommits = async (username) => { if (!githubUsernameRegex.test(username)) { logger.log("Invalid username"); - return 0; + return { totalCommits: 0, success: false }; } - // https://developer.github.com/v3/search/#search-commits - const fetchTotalCommits = (variables, token) => { - return axios({ - method: "get", - url: `https://api.github.com/search/commits?q=author:${variables.login}`, - headers: { - "Content-Type": "application/json", - Accept: "application/vnd.github.cloak-preview", - Authorization: `token ${token}`, - }, - }); - }; - try { - let res = await retryer(fetchTotalCommits, { login: username }); + let res = await retryer(totalCommitsFetcher, { login: username }); let total_count = res.data.total_count; if (!!total_count && !isNaN(total_count)) { - return res.data.total_count; + return { totalCommits: res.data.total_count, success: true }; } } catch (err) { logger.log(err); } // just return 0 if there is something wrong so that // we don't break the whole app - return 0; + return { totalCommits: 0, success: false }; }; /** @@ -183,7 +191,7 @@ const totalStarsFetcher = async (username, repoToHide) => { * @param {string} username GitHub username. * @param {boolean} count_private Include private contributions. * @param {boolean} include_all_commits Include all commits. - * @returns {Promise} Stats data. + * @returns {Promise} Object containing the stats data a success boolean that can be used to handle the caching behavior. */ const fetchStats = async ( username, @@ -244,9 +252,13 @@ const fetchStats = async ( stats.totalCommits = user.contributionsCollection.totalCommitContributions; // if include_all_commits then just get that, - // since totalCommitsFetcher already sends totalCommits no need to += + // NOTE: Since fetchTotalCommits already sends totalCommits no need to +=. + let totalCommitsResp; + let success = true; if (include_all_commits) { - stats.totalCommits = await totalCommitsFetcher(username); + totalCommitsResp = await fetchTotalCommits(username); + stats.totalCommits = totalCommitsResp.totalCommits; + success = totalCommitsResp.success; } // if count_private then add private commits to totalCommits so far. @@ -271,7 +283,7 @@ const fetchStats = async ( issues: stats.totalIssues, }); - return stats; + return { stats, success }; }; export { fetchStats }; diff --git a/tests/fetchStats.test.js b/tests/fetchStats.test.js index 192146ea5fbe0..652638fa680d6 100644 --- a/tests/fetchStats.test.js +++ b/tests/fetchStats.test.js @@ -108,7 +108,7 @@ afterEach(() => { describe("Test fetchStats", () => { it("should fetch correct stats", async () => { - let stats = await fetchStats("anuraghazra"); + let { stats } = await fetchStats("anuraghazra"); const rank = calculateRank({ totalCommits: 100, totalRepos: 5, @@ -140,7 +140,7 @@ describe("Test fetchStats", () => { .onPost("https://api.github.com/graphql") .replyOnce(200, repositoriesWithZeroStarsData); - let stats = await fetchStats("anuraghazra"); + let { stats } = await fetchStats("anuraghazra"); const rank = calculateRank({ totalCommits: 100, totalRepos: 5, @@ -172,7 +172,7 @@ describe("Test fetchStats", () => { }); it("should fetch and add private contributions", async () => { - let stats = await fetchStats("anuraghazra", true); + let { stats } = await fetchStats("anuraghazra", true); const rank = calculateRank({ totalCommits: 150, totalRepos: 5, @@ -201,7 +201,7 @@ describe("Test fetchStats", () => { .onGet("https://api.github.com/search/commits?q=author:anuraghazra") .reply(200, { total_count: 1000 }); - let stats = await fetchStats("anuraghazra", true, true); + let { stats } = await fetchStats("anuraghazra", true, true); const rank = calculateRank({ totalCommits: 1050, totalRepos: 5, @@ -225,12 +225,37 @@ describe("Test fetchStats", () => { }); }); + it("should return `true` success boolean when 'include_all_commits' is `false`", async () => { + let { success } = await fetchStats("anuraghazra", true, false); + expect(success).toStrictEqual(true); + }); + + it("should return `true` success boolean when 'include_all_commits' is `true` and total commits were fetched successfully", async () => { + mock + .onGet("https://api.github.com/search/commits?q=author:anuraghazra") + .reply(200, { total_count: 1000 }); + + let { success } = await fetchStats("anuraghazra", true, true); + expect(success).toStrictEqual(true); + }); + + it("should return `false` success boolean when 'include_all_commits' is `true` and total commits could not be fetched", async () => { + mock + .onGet("https://api.github.com/search/commits?q=author:anuraghazra") + .reply(404, "Could not fetch total commits"); + + let { success } = await fetchStats("anuraghazra", true, true); + expect(success).toStrictEqual(false); + }); + it("should exclude stars of the `test-repo-1` repository", async () => { mock .onGet("https://api.github.com/search/commits?q=author:anuraghazra") .reply(200, { total_count: 1000 }); - let stats = await fetchStats("anuraghazra", true, true, ["test-repo-1"]); + let { stats } = await fetchStats("anuraghazra", true, true, [ + "test-repo-1", + ]); const rank = calculateRank({ totalCommits: 1050, totalRepos: 5,