From 924b9dad68a48612e8a58025e9985ecd9189bd9d Mon Sep 17 00:00:00 2001 From: Rhys Arkins <rhys@arkins.net> Date: Wed, 28 Feb 2024 06:22:16 +0100 Subject: [PATCH] feat(github): use REST for etag caching of issues (#26793) Co-authored-by: Michael Kriese <michael.kriese@visualon.de> --- lib/modules/platform/github/graphql.ts | 30 -- lib/modules/platform/github/index.spec.ts | 586 +++++++++------------- lib/modules/platform/github/index.ts | 37 +- lib/modules/platform/github/schema.ts | 11 + 4 files changed, 282 insertions(+), 382 deletions(-) create mode 100644 lib/modules/platform/github/schema.ts diff --git a/lib/modules/platform/github/graphql.ts b/lib/modules/platform/github/graphql.ts index 09f0addcc0..21d122e786 100644 --- a/lib/modules/platform/github/graphql.ts +++ b/lib/modules/platform/github/graphql.ts @@ -93,36 +93,6 @@ query($owner: String!, $name: String!, $count: Int, $cursor: String) { } `; -export const getIssuesQuery = ` -query( - $owner: String!, - $name: String!, - $user: String!, - $count: Int, - $cursor: String -) { - repository(owner: $owner, name: $name) { - issues( - orderBy: { field: UPDATED_AT, direction: DESC }, - filterBy: { createdBy: $user }, - first: $count, - after: $cursor - ) { - pageInfo { - endCursor - hasNextPage - } - nodes { - number - state - title - body - } - } - } -} -`; - export const vulnerabilityAlertsQuery = (filterByState: boolean): string => ` query($owner: String!, $name: String!) { repository(owner: $owner, name: $name) { diff --git a/lib/modules/platform/github/index.spec.ts b/lib/modules/platform/github/index.spec.ts index 11b8efe42f..24a754a597 100644 --- a/lib/modules/platform/github/index.spec.ts +++ b/lib/modules/platform/github/index.spec.ts @@ -1573,36 +1573,76 @@ describe('modules/platform/github/index', () => { }); }); + describe('getIssue()', () => { + it('defaults to use cache', async () => { + const scope = httpMock.scope(githubApiHost); + initRepoMock(scope, 'test/repo'); + await github.initRepo({ repository: 'test/repo' }); + scope + .get('/repos/test/repo/issues?creator=undefined&state=all') + .reply(200, [ + { + number: 1, + title: 'title-1', + body: 'body-1', + state: 'open', + labels: [ + { + name: 'label-1', + }, + ], + }, + { + number: 2, + title: 'title-1', + body: 'body-1', + }, + ]); + + const res = await github.getIssue(1); + expect(res).not.toBeNull(); + }); + + it('cache breaks', async () => { + const scope = httpMock.scope(githubApiHost); + initRepoMock(scope, 'test/repo'); + await github.initRepo({ repository: 'test/repo' }); + scope.get('/repos/test/repo/issues/1').reply(200, { + number: 1, + title: 'title-1b', + body: 'body-1b', + state: 'open', + labels: [ + { + name: 'label-1', + }, + ], + }); + + const res = await github.getIssue(1, false); + expect(res?.body).toBe('body-1b'); + }); + }); + describe('findIssue()', () => { it('returns null if no issue', async () => { httpMock .scope(githubApiHost) - .post('/graphql') - .reply(200, { - data: { - repository: { - issues: { - pageInfo: { - startCursor: null, - hasNextPage: false, - endCursor: null, - }, - nodes: [ - { - number: 2, - state: 'open', - title: 'title-2', - }, - { - number: 1, - state: 'open', - title: 'title-1', - }, - ], - }, - }, + .get('/repos/undefined/issues?creator=undefined&state=all') + .reply(200, [ + { + number: 2, + state: 'open', + title: 'title-2', + body: '', }, - }); + { + number: 1, + state: 'open', + title: 'title-1', + body: '', + }, + ]); const res = await github.findIssue('title-3'); expect(res).toBeNull(); }); @@ -1610,34 +1650,21 @@ describe('modules/platform/github/index', () => { it('finds issue', async () => { httpMock .scope(githubApiHost) - .post('/graphql') - .reply(200, { - data: { - repository: { - issues: { - pageInfo: { - startCursor: null, - hasNextPage: false, - endCursor: null, - }, - nodes: [ - { - number: 2, - state: 'open', - title: 'title-2', - }, - { - number: 1, - state: 'open', - title: 'title-1', - }, - ], - }, - }, + .get('/repos/undefined/issues?creator=undefined&state=all') + .reply(200, [ + { + number: 2, + state: 'open', + title: 'title-2', + body: '', }, - }) - .get('/repos/undefined/issues/2') - .reply(200, { body: 'new-content' }); + { + number: 1, + state: 'open', + title: 'title-1', + body: '', + }, + ]); const res = await github.findIssue('title-2'); expect(res).not.toBeNull(); }); @@ -1649,32 +1676,8 @@ describe('modules/platform/github/index', () => { initRepoMock(scope, 'some/repo'); await github.initRepo({ repository: 'some/repo' }); scope - .post('/graphql') - .reply(200, { - data: { - repository: { - issues: { - pageInfo: { - startCursor: null, - hasNextPage: false, - endCursor: null, - }, - nodes: [ - { - number: 2, - state: 'open', - title: 'title-2', - }, - { - number: 1, - state: 'open', - title: 'title-1', - }, - ], - }, - }, - }, - }) + .get('/repos/some/repo/issues?creator=undefined&state=all') + .reply(200, []) .post('/repos/some/repo/issues') .reply(200); const res = await github.ensureIssue({ @@ -1689,32 +1692,21 @@ describe('modules/platform/github/index', () => { initRepoMock(scope, 'some/repo'); await github.initRepo({ repository: 'some/repo' }); scope - .post('/graphql') - .reply(200, { - data: { - repository: { - issues: { - pageInfo: { - startCursor: null, - hasNextPage: false, - endCursor: null, - }, - nodes: [ - { - number: 2, - state: 'open', - title: 'title-2', - }, - { - number: 1, - state: 'closed', - title: 'title-1', - }, - ], - }, - }, + .get('/repos/some/repo/issues?creator=undefined&state=all') + .reply(200, [ + { + number: 2, + state: 'open', + title: 'title-2', + body: '', }, - }) + { + number: 1, + state: 'open', + title: 'title-1', + body: '', + }, + ]) .get('/repos/some/repo/issues/1') .reply(404); const res = await github.ensureIssue({ @@ -1728,31 +1720,22 @@ describe('modules/platform/github/index', () => { const scope = httpMock.scope(githubApiHost); initRepoMock(scope, 'some/repo'); await github.initRepo({ repository: 'some/repo' }); - scope.post('/graphql').reply(200, { - data: { - repository: { - issues: { - pageInfo: { - startCursor: null, - hasNextPage: false, - endCursor: null, - }, - nodes: [ - { - number: 2, - state: 'open', - title: 'title-2', - }, - { - number: 1, - state: 'closed', - title: 'title-1', - }, - ], - }, + scope + .get('/repos/some/repo/issues?creator=undefined&state=all') + .reply(200, [ + { + number: 2, + state: 'open', + title: 'title-2', + body: '', }, - }, - }); + { + number: 1, + state: 'closed', + title: 'title-1', + body: '', + }, + ]); const once = true; const res = await github.ensureIssue({ title: 'title-1', @@ -1762,26 +1745,44 @@ describe('modules/platform/github/index', () => { expect(res).toBeNull(); }); - it('creates issue with labels', async () => { + it('reopens issue', async () => { const scope = httpMock.scope(githubApiHost); initRepoMock(scope, 'some/repo'); await github.initRepo({ repository: 'some/repo' }); scope - .post('/graphql') - .reply(200, { - data: { - repository: { - issues: { - pageInfo: { - startCursor: null, - hasNextPage: false, - endCursor: null, - }, - nodes: [], - }, - }, + .get('/repos/some/repo/issues?creator=undefined&state=all') + .reply(200, [ + { + number: 2, + state: 'open', + title: 'title-2', + body: '', }, - }) + { + number: 1, + state: 'closed', + title: 'title-1', + body: '', + }, + ]) + .get('/repos/some/repo/issues/1') + .reply(200) + .patch('/repos/some/repo/issues/1') + .reply(200); + const res = await github.ensureIssue({ + title: 'title-1', + body: 'new-content', + }); + expect(res).not.toBeNull(); + }); + + it('creates issue with labels', async () => { + const scope = httpMock.scope(githubApiHost); + initRepoMock(scope, 'some/repo'); + await github.initRepo({ repository: 'some/repo' }); + scope + .get('/repos/some/repo/issues?creator=undefined&state=all') + .reply(200, []) .post('/repos/some/repo/issues') .reply(200); const res = await github.ensureIssue({ @@ -1797,37 +1798,27 @@ describe('modules/platform/github/index', () => { initRepoMock(scope, 'some/repo'); await github.initRepo({ repository: 'some/repo' }); scope - .post('/graphql') - .reply(200, { - data: { - repository: { - issues: { - pageInfo: { - startCursor: null, - hasNextPage: false, - endCursor: null, - }, - nodes: [ - { - number: 3, - state: 'open', - title: 'title-1', - }, - { - number: 2, - state: 'open', - title: 'title-2', - }, - { - number: 1, - state: 'closed', - title: 'title-1', - }, - ], - }, - }, + .get('/repos/some/repo/issues?creator=undefined&state=all') + .reply(200, [ + { + number: 3, + state: 'open', + title: 'title-1', + body: '', }, - }) + { + number: 2, + state: 'open', + title: 'title-2', + body: '', + }, + { + number: 1, + state: 'closed', + title: 'title-1', + body: '', + }, + ]) .get('/repos/some/repo/issues/3') .reply(404); const once = true; @@ -1844,32 +1835,21 @@ describe('modules/platform/github/index', () => { initRepoMock(scope, 'some/repo'); await github.initRepo({ repository: 'some/repo' }); scope - .post('/graphql') - .reply(200, { - data: { - repository: { - issues: { - pageInfo: { - startCursor: null, - hasNextPage: false, - endCursor: null, - }, - nodes: [ - { - number: 2, - state: 'open', - title: 'title-2', - }, - { - number: 1, - state: 'open', - title: 'title-1', - }, - ], - }, - }, + .get('/repos/some/repo/issues?creator=undefined&state=all') + .reply(200, [ + { + number: 2, + state: 'open', + title: 'title-2', + body: '', }, - }) + { + number: 1, + state: 'open', + title: 'title-1', + body: '', + }, + ]) .get('/repos/some/repo/issues/2') .reply(200, { body: 'new-content' }) .patch('/repos/some/repo/issues/2') @@ -1887,32 +1867,21 @@ describe('modules/platform/github/index', () => { initRepoMock(scope, 'some/repo'); await github.initRepo({ repository: 'some/repo' }); scope - .post('/graphql') - .reply(200, { - data: { - repository: { - issues: { - pageInfo: { - startCursor: null, - hasNextPage: false, - endCursor: null, - }, - nodes: [ - { - number: 2, - state: 'open', - title: 'title-2', - }, - { - number: 1, - state: 'open', - title: 'title-1', - }, - ], - }, - }, + .get('/repos/some/repo/issues?creator=undefined&state=all') + .reply(200, [ + { + number: 2, + state: 'open', + title: 'title-2', + body: '', }, - }) + { + number: 1, + state: 'open', + title: 'title-1', + body: '', + }, + ]) .get('/repos/some/repo/issues/2') .reply(200, { body: 'new-content' }) .patch('/repos/some/repo/issues/2') @@ -1931,32 +1900,21 @@ describe('modules/platform/github/index', () => { initRepoMock(scope, 'some/repo'); await github.initRepo({ repository: 'some/repo' }); scope - .post('/graphql') - .reply(200, { - data: { - repository: { - issues: { - pageInfo: { - startCursor: null, - hasNextPage: false, - endCursor: null, - }, - nodes: [ - { - number: 2, - state: 'open', - title: 'title-2', - }, - { - number: 1, - state: 'open', - title: 'title-1', - }, - ], - }, - }, + .get('/repos/some/repo/issues?creator=undefined&state=all') + .reply(200, [ + { + number: 2, + state: 'open', + title: 'title-2', + body: '', }, - }) + { + number: 1, + state: 'open', + title: 'title-1', + body: '', + }, + ]) .get('/repos/some/repo/issues/2') .reply(200, { body: 'newer-content' }); const res = await github.ensureIssue({ @@ -1971,32 +1929,21 @@ describe('modules/platform/github/index', () => { initRepoMock(scope, 'some/repo'); await github.initRepo({ repository: 'some/repo' }); scope - .post('/graphql') - .reply(200, { - data: { - repository: { - issues: { - pageInfo: { - startCursor: null, - hasNextPage: false, - endCursor: null, - }, - nodes: [ - { - number: 2, - state: 'open', - title: 'title-1', - }, - { - number: 1, - state: 'open', - title: 'title-1', - }, - ], - }, - }, + .get('/repos/some/repo/issues?creator=undefined&state=all') + .reply(200, [ + { + number: 2, + state: 'open', + title: 'title-1', + body: '', }, - }) + { + number: 1, + state: 'open', + title: 'title-1', + body: '', + }, + ]) .patch('/repos/some/repo/issues/1') .reply(200) .get('/repos/some/repo/issues/2') @@ -2013,27 +1960,15 @@ describe('modules/platform/github/index', () => { initRepoMock(scope, 'some/repo'); await github.initRepo({ repository: 'some/repo' }); scope - .post('/graphql') - .reply(200, { - data: { - repository: { - issues: { - pageInfo: { - startCursor: null, - hasNextPage: false, - endCursor: null, - }, - nodes: [ - { - number: 2, - state: 'close', - title: 'title-2', - }, - ], - }, - }, + .get('/repos/some/repo/issues?creator=undefined&state=all') + .reply(200, [ + { + number: 2, + state: 'closed', + title: 'title-2', + body: '', }, - }) + ]) .get('/repos/some/repo/issues/2') .reply(200, { body: 'new-content' }) .post('/repos/some/repo/issues') @@ -2052,27 +1987,15 @@ describe('modules/platform/github/index', () => { initRepoMock(scope, 'some/repo'); await github.initRepo({ repository: 'some/repo' }); scope - .post('/graphql') - .reply(200, { - data: { - repository: { - issues: { - pageInfo: { - startCursor: null, - hasNextPage: false, - endCursor: null, - }, - nodes: [ - { - number: 2, - state: 'open', - title: 'title-2', - }, - ], - }, - }, + .get('/repos/some/repo/issues?creator=undefined&state=all') + .reply(200, [ + { + number: 2, + state: 'open', + title: 'title-2', + body: '', }, - }) + ]) .get('/repos/some/repo/issues/2') .reply(200, { body: 'new-content' }); const res = await github.ensureIssue({ @@ -2089,32 +2012,21 @@ describe('modules/platform/github/index', () => { it('closes issue', async () => { httpMock .scope(githubApiHost) - .post('/graphql') - .reply(200, { - data: { - repository: { - issues: { - pageInfo: { - startCursor: null, - hasNextPage: false, - endCursor: null, - }, - nodes: [ - { - number: 2, - state: 'open', - title: 'title-2', - }, - { - number: 1, - state: 'open', - title: 'title-1', - }, - ], - }, - }, + .get('/repos/undefined/issues?creator=undefined&state=all') + .reply(200, [ + { + number: 2, + state: 'open', + title: 'title-2', + body: '', }, - }) + { + number: 1, + state: 'open', + title: 'title-1', + body: '', + }, + ]) .patch('/repos/undefined/issues/2') .reply(200); await expect(github.ensureIssueClosing('title-2')).toResolve(); diff --git a/lib/modules/platform/github/index.ts b/lib/modules/platform/github/index.ts index c34abf4abc..9790dc3176 100644 --- a/lib/modules/platform/github/index.ts +++ b/lib/modules/platform/github/index.ts @@ -69,12 +69,12 @@ import { remoteBranchExists } from './branch'; import { coerceRestPr, githubApi } from './common'; import { enableAutoMergeMutation, - getIssuesQuery, repoInfoQuery, vulnerabilityAlertsQuery, } from './graphql'; import { massageMarkdownLinks } from './massage-markdown-links'; import { getPrCache, updatePrCache } from './pr'; +import { IssuesSchema } from './schema'; import type { BranchProtection, CombinedBranchStatus, @@ -1181,23 +1181,22 @@ export async function setBranchStatus({ /* istanbul ignore next */ async function getIssues(): Promise<Issue[]> { - const result = await githubApi.queryRepoField<Issue>( - getIssuesQuery, - 'issues', - { - variables: { - owner: config.repositoryOwner, - name: config.repositoryName, - user: config.renovateUsername, + const issuesUrl = `repos/${config.parentRepo ?? config.repository}/issues?creator=${ + config.renovateUsername + }&state=all`; + const result = ( + await githubApi.getJson( + issuesUrl, + { + repoCache: true, + paginate: true, }, - }, - ); + IssuesSchema, + ) + ).body; logger.debug(`Retrieved ${result.length} issues`); - return result.map((issue) => ({ - ...issue, - state: issue.state?.toLowerCase(), - })); + return result; } export async function getIssueList(): Promise<Issue[]> { @@ -1220,6 +1219,14 @@ export async function getIssue( if (config.hasIssuesEnabled === false) { return null; } + if (useCache) { + const issueList = await getIssueList(); + const issue = issueList.find((i) => i.number === number); + if (issue) { + logger.debug(`Returning issue from cache`); + return issue; + } + } try { const issueBody = ( await githubApi.getJson<{ body: string }>( diff --git a/lib/modules/platform/github/schema.ts b/lib/modules/platform/github/schema.ts new file mode 100644 index 0000000000..7a849fbccc --- /dev/null +++ b/lib/modules/platform/github/schema.ts @@ -0,0 +1,11 @@ +import { z } from 'zod'; +import { LooseArray } from '../../../util/schema-utils'; + +export const IssueSchema = z.object({ + number: z.number(), + state: z.string().transform((state) => state.toLowerCase()), + title: z.string(), + body: z.string(), +}); + +export const IssuesSchema = LooseArray(IssueSchema); -- GitLab