Skip to content

Commit af8f023

Browse files
authored
Merge pull request #1485 from github/koesie10/add-github-download-button
Remove canary requirement for GitHub database download
2 parents 84bd029 + 23a0e03 commit af8f023

File tree

7 files changed

+56
-60
lines changed

7 files changed

+56
-60
lines changed

extensions/ql-vscode/CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
## [UNRELEASED]
44

5+
- Add ability for users to download databases directly from GitHub. [#1485](https://github.com/github/vscode-codeql/pull/1485)
6+
57
## 1.6.11 - 25 August 2022
68

79
No user facing changes.

extensions/ql-vscode/package.json

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -664,7 +664,7 @@
664664
},
665665
{
666666
"command": "codeQLDatabases.chooseDatabaseGithub",
667-
"when": "config.codeQL.canary && view == codeQLDatabases",
667+
"when": "view == codeQLDatabases",
668668
"group": "navigation"
669669
},
670670
{
@@ -926,10 +926,6 @@
926926
"command": "codeQL.viewCfg",
927927
"when": "resourceScheme == codeql-zip-archive && config.codeQL.canary"
928928
},
929-
{
930-
"command": "codeQL.chooseDatabaseGithub",
931-
"when": "config.codeQL.canary"
932-
},
933929
{
934930
"command": "codeQLDatabases.setCurrentDatabase",
935931
"when": "false"
@@ -1175,7 +1171,7 @@
11751171
},
11761172
{
11771173
"view": "codeQLDatabases",
1178-
"contents": "Add a CodeQL database:\n[From a folder](command:codeQLDatabases.chooseDatabaseFolder)\n[From an archive](command:codeQLDatabases.chooseDatabaseArchive)\n[From a URL (as a zip file)](command:codeQLDatabases.chooseDatabaseInternet)\n[From LGTM](command:codeQLDatabases.chooseDatabaseLgtm)"
1174+
"contents": "Add a CodeQL database:\n[From a folder](command:codeQLDatabases.chooseDatabaseFolder)\n[From an archive](command:codeQLDatabases.chooseDatabaseArchive)\n[From a URL (as a zip file)](command:codeQLDatabases.chooseDatabaseInternet)\n[From GitHub](command:codeQLDatabases.chooseDatabaseGithub)\n[From LGTM](command:codeQLDatabases.chooseDatabaseLgtm)"
11791175
},
11801176
{
11811177
"view": "codeQLEvalLogViewer",

extensions/ql-vscode/src/authentication.ts

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -76,16 +76,27 @@ export class Credentials {
7676
}));
7777
}
7878

79-
async getOctokit(): Promise<Octokit.Octokit> {
79+
/**
80+
* Creates or returns an instance of Octokit.
81+
*
82+
* @param requireAuthentication Whether the Octokit instance needs to be authenticated as user.
83+
* @returns An instance of Octokit.
84+
*/
85+
async getOctokit(requireAuthentication = true): Promise<Octokit.Octokit> {
8086
if (this.octokit) {
8187
return this.octokit;
8288
}
8389

84-
this.octokit = await this.createOctokit(true);
85-
// octokit shouldn't be undefined, since we've set "createIfNone: true".
86-
// The following block is mainly here to prevent a compiler error.
90+
this.octokit = await this.createOctokit(requireAuthentication);
91+
8792
if (!this.octokit) {
88-
throw new Error('Did not initialize Octokit.');
93+
if (requireAuthentication) {
94+
throw new Error('Did not initialize Octokit.');
95+
}
96+
97+
// We don't want to set this in this.octokit because that would prevent
98+
// authenticating when requireCredentials is true.
99+
return new Octokit.Octokit({ retry });
89100
}
90101
return this.octokit;
91102
}

extensions/ql-vscode/src/databaseFetcher.ts

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ import {
1010
import { CodeQLCliServer } from './cli';
1111
import * as fs from 'fs-extra';
1212
import * as path from 'path';
13+
import * as Octokit from '@octokit/rest';
14+
import { retry } from '@octokit/plugin-retry';
1315

1416
import { DatabaseManager, DatabaseItem } from './databases';
1517
import {
@@ -76,7 +78,7 @@ export async function promptImportInternetDatabase(
7678
export async function promptImportGithubDatabase(
7779
databaseManager: DatabaseManager,
7880
storagePath: string,
79-
credentials: Credentials,
81+
credentials: Credentials | undefined,
8082
progress: ProgressCallback,
8183
token: CancellationToken,
8284
cli?: CodeQLCliServer
@@ -99,14 +101,15 @@ export async function promptImportGithubDatabase(
99101
throw new Error(`Invalid GitHub repository: ${githubRepo}`);
100102
}
101103

102-
const result = await convertGithubNwoToDatabaseUrl(githubRepo, credentials, progress);
104+
const octokit = credentials ? await credentials.getOctokit(true) : new Octokit.Octokit({ retry });
105+
106+
const result = await convertGithubNwoToDatabaseUrl(githubRepo, octokit, progress);
103107
if (!result) {
104108
return;
105109
}
106110

107111
const { databaseUrl, name, owner } = result;
108112

109-
const octokit = await credentials.getOctokit();
110113
/**
111114
* The 'token' property of the token object returned by `octokit.auth()`.
112115
* The object is undocumented, but looks something like this:
@@ -118,14 +121,9 @@ export async function promptImportGithubDatabase(
118121
* We only need the actual token string.
119122
*/
120123
const octokitToken = (await octokit.auth() as { token: string })?.token;
121-
if (!octokitToken) {
122-
// Just print a generic error message for now. Ideally we could show more debugging info, like the
123-
// octokit object, but that would expose a user token.
124-
throw new Error('Unable to get GitHub token.');
125-
}
126124
const item = await databaseArchiveFetcher(
127125
databaseUrl,
128-
{ 'Accept': 'application/zip', 'Authorization': `Bearer ${octokitToken}` },
126+
{ 'Accept': 'application/zip', 'Authorization': octokitToken ? `Bearer ${octokitToken}` : '' },
129127
databaseManager,
130128
storagePath,
131129
`${owner}/${name}`,
@@ -523,7 +521,7 @@ function convertGitHubUrlToNwo(githubUrl: string): string | undefined {
523521

524522
export async function convertGithubNwoToDatabaseUrl(
525523
githubRepo: string,
526-
credentials: Credentials,
524+
octokit: Octokit.Octokit,
527525
progress: ProgressCallback): Promise<{
528526
databaseUrl: string,
529527
owner: string,
@@ -533,7 +531,6 @@ export async function convertGithubNwoToDatabaseUrl(
533531
const nwo = convertGitHubUrlToNwo(githubRepo) || githubRepo;
534532
const [owner, repo] = nwo.split('/');
535533

536-
const octokit = await credentials.getOctokit();
537534
const response = await octokit.request('GET /repos/:owner/:repo/code-scanning/codeql/databases', { owner, repo });
538535

539536
const languages = response.data.map((db: any) => db.language);

extensions/ql-vscode/src/databases-ui.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ import {
4040
import { CancellationToken } from 'vscode';
4141
import { asyncFilter, getErrorMessage } from './pure/helpers-pure';
4242
import { Credentials } from './authentication';
43+
import { isCanary } from './config';
4344

4445
type ThemableIconPath = { light: string; dark: string } | string;
4546

@@ -301,7 +302,7 @@ export class DatabaseUI extends DisposableObject {
301302
progress: ProgressCallback,
302303
token: CancellationToken
303304
) => {
304-
const credentials = await this.getCredentials();
305+
const credentials = isCanary() ? await this.getCredentials() : undefined;
305306
await this.handleChooseDatabaseGithub(credentials, progress, token);
306307
},
307308
{
@@ -480,7 +481,7 @@ export class DatabaseUI extends DisposableObject {
480481
};
481482

482483
handleChooseDatabaseGithub = async (
483-
credentials: Credentials,
484+
credentials: Credentials | undefined,
484485
progress: ProgressCallback,
485486
token: CancellationToken
486487
): Promise<DatabaseItem | undefined> => {

extensions/ql-vscode/src/extension.ts

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -972,7 +972,7 @@ async function activateWithInstalledDistribution(
972972
progress: ProgressCallback,
973973
token: CancellationToken
974974
) => {
975-
const credentials = await Credentials.initialize(ctx);
975+
const credentials = isCanary() ? await Credentials.initialize(ctx) : undefined;
976976
await databaseUI.handleChooseDatabaseGithub(credentials, progress, token);
977977
},
978978
{
@@ -1020,19 +1020,16 @@ async function activateWithInstalledDistribution(
10201020
}
10211021
};
10221022

1023-
// The "authenticateToGitHub" command is internal-only.
10241023
ctx.subscriptions.push(
10251024
commandRunner('codeQL.authenticateToGitHub', async () => {
1026-
if (isCanary()) {
1027-
/**
1028-
* Credentials for authenticating to GitHub.
1029-
* These are used when making API calls.
1030-
*/
1031-
const credentials = await Credentials.initialize(ctx);
1032-
const octokit = await credentials.getOctokit();
1033-
const userInfo = await octokit.users.getAuthenticated();
1034-
void showAndLogInformationMessage(`Authenticated to GitHub as user: ${userInfo.data.login}`);
1035-
}
1025+
/**
1026+
* Credentials for authenticating to GitHub.
1027+
* These are used when making API calls.
1028+
*/
1029+
const credentials = await Credentials.initialize(ctx);
1030+
const octokit = await credentials.getOctokit();
1031+
const userInfo = await octokit.users.getAuthenticated();
1032+
void showAndLogInformationMessage(`Authenticated to GitHub as user: ${userInfo.data.login}`);
10361033
}));
10371034

10381035
ctx.subscriptions.push(

extensions/ql-vscode/src/vscode-tests/no-workspace/databaseFetcher.test.ts

Lines changed: 16 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,14 @@ import { expect } from 'chai';
66
import { window } from 'vscode';
77

88
import {
9+
convertGithubNwoToDatabaseUrl,
910
convertLgtmUrlToDatabaseUrl,
1011
looksLikeLgtmUrl,
1112
findDirWithFile,
1213
looksLikeGithubRepo,
1314
} from '../../databaseFetcher';
1415
import { ProgressCallback } from '../../commandRunner';
15-
import * as pq from 'proxyquire';
16-
17-
const proxyquire = pq.noPreserveCache();
16+
import * as Octokit from '@octokit/rest';
1817

1918
describe('databaseFetcher', function() {
2019
// These tests make API calls and may need extra time to complete.
@@ -25,20 +24,16 @@ describe('databaseFetcher', function() {
2524
let quickPickSpy: sinon.SinonStub;
2625
let progressSpy: ProgressCallback;
2726
let mockRequest: sinon.SinonStub;
28-
let mod: any;
29-
30-
const credentials = getMockCredentials(0);
27+
let octokit: Octokit.Octokit;
3128

3229
beforeEach(() => {
3330
sandbox = sinon.createSandbox();
3431
quickPickSpy = sandbox.stub(window, 'showQuickPick');
3532
progressSpy = sandbox.spy();
3633
mockRequest = sandbox.stub();
37-
mod = proxyquire('../../databaseFetcher', {
38-
'./authentication': {
39-
Credentials: credentials,
40-
},
41-
});
34+
octokit = ({
35+
request: mockRequest,
36+
}) as unknown as Octokit.Octokit;
4237
});
4338

4439
afterEach(() => {
@@ -90,11 +85,17 @@ describe('databaseFetcher', function() {
9085
mockRequest.resolves(mockApiResponse);
9186
quickPickSpy.resolves('javascript');
9287
const githubRepo = 'github/codeql';
93-
const { databaseUrl, name, owner } = await mod.convertGithubNwoToDatabaseUrl(
88+
const result = await convertGithubNwoToDatabaseUrl(
9489
githubRepo,
95-
credentials,
90+
octokit,
9691
progressSpy
9792
);
93+
expect(result).not.to.be.undefined;
94+
if (result === undefined) {
95+
return;
96+
}
97+
98+
const { databaseUrl, name, owner } = result;
9899

99100
expect(databaseUrl).to.equal(
100101
'https://api.github.com/repos/github/codeql/code-scanning/codeql/databases/javascript'
@@ -119,7 +120,7 @@ describe('databaseFetcher', function() {
119120
mockRequest.resolves(mockApiResponse);
120121
const githubRepo = 'foo/bar-not-real';
121122
await expect(
122-
mod.convertGithubNwoToDatabaseUrl(githubRepo, credentials, progressSpy)
123+
convertGithubNwoToDatabaseUrl(githubRepo, octokit, progressSpy)
123124
).to.be.rejectedWith(/Unable to get database/);
124125
expect(progressSpy).to.have.callCount(0);
125126
});
@@ -133,19 +134,10 @@ describe('databaseFetcher', function() {
133134
mockRequest.resolves(mockApiResponse);
134135
const githubRepo = 'foo/bar-with-no-dbs';
135136
await expect(
136-
mod.convertGithubNwoToDatabaseUrl(githubRepo, credentials, progressSpy)
137+
convertGithubNwoToDatabaseUrl(githubRepo, octokit, progressSpy)
137138
).to.be.rejectedWith(/Unable to get database/);
138139
expect(progressSpy).to.have.been.calledOnce;
139140
});
140-
141-
function getMockCredentials(response: any) {
142-
mockRequest = sinon.stub().resolves(response);
143-
return {
144-
getOctokit: () => ({
145-
request: mockRequest,
146-
}),
147-
};
148-
}
149141
});
150142

151143
describe('convertLgtmUrlToDatabaseUrl', () => {

0 commit comments

Comments
 (0)