From 7241e317af3bf924c4bfb023656d4282b72721dc Mon Sep 17 00:00:00 2001 From: Nora Date: Tue, 20 Dec 2022 10:19:42 +0000 Subject: [PATCH] Move helper to new file and minor refactor --- .../common/github-url-identifier-helper.ts | 71 +++++++++++++++++++ extensions/ql-vscode/src/databaseFetcher.ts | 20 +++--- .../ql-vscode/src/databases/github-nwo.ts | 51 ------------- .../no-workspace/databaseFetcher.test.ts | 20 ------ .../github-url-identifier-helper.test.ts | 71 +++++++++++++++++++ 5 files changed, 150 insertions(+), 83 deletions(-) create mode 100644 extensions/ql-vscode/src/common/github-url-identifier-helper.ts delete mode 100644 extensions/ql-vscode/src/databases/github-nwo.ts create mode 100644 extensions/ql-vscode/test/pure-tests/common/github-url-identifier-helper.test.ts diff --git a/extensions/ql-vscode/src/common/github-url-identifier-helper.ts b/extensions/ql-vscode/src/common/github-url-identifier-helper.ts new file mode 100644 index 00000000000..d93f45feb99 --- /dev/null +++ b/extensions/ql-vscode/src/common/github-url-identifier-helper.ts @@ -0,0 +1,71 @@ +import { OWNER_REGEX, REPO_REGEX } from "../pure/helpers-pure"; + +/** + * Checks if a string is a valid GitHub NWO. + * @param identifier The GitHub NWO + * @returns + */ +export function validGitHubNwo(identifier: string): boolean { + return validGitHubNwoOrOwner(identifier, "nwo"); +} + +/** + * Checks if a string is a valid GitHub owner. + * @param identifier The GitHub owner + * @returns + */ +export function validGitHubOwner(identifier: string): boolean { + return validGitHubNwoOrOwner(identifier, "owner"); +} + +function validGitHubNwoOrOwner( + identifier: string, + kind: "owner" | "nwo", +): boolean { + return kind === "owner" + ? OWNER_REGEX.test(identifier) + : REPO_REGEX.test(identifier); +} + +/** + * Extracts an NOW from a GitHub URL. + * @param githubUrl The GitHub repository URL + * @return The corresponding NWO, or undefined if the URL is not valid + */ +export function getNwoFromGitHubUrl(githubUrl: string): string | undefined { + return getNwoOrOwnerFromGitHubUrl(githubUrl, "nwo"); +} + +/** + * Extracts an owner from a GitHub URL. + * @param githubUrl The GitHub repository URL + * @return The corresponding Owner, or undefined if the URL is not valid + */ +export function getOwnerFromGitHubUrl(githubUrl: string): string | undefined { + return getNwoOrOwnerFromGitHubUrl(githubUrl, "owner"); +} + +function getNwoOrOwnerFromGitHubUrl( + githubUrl: string, + kind: "owner" | "nwo", +): string | undefined { + try { + const uri = new URL(githubUrl); + if (uri.protocol !== "https:") { + return; + } + if (uri.hostname !== "github.com" && uri.hostname !== "www.github.com") { + return; + } + const paths = uri.pathname.split("/").filter((segment: string) => segment); + const owner = `${paths[0]}`; + if (kind === "owner") { + return owner ? owner : undefined; + } + const nwo = `${paths[0]}/${paths[1]}`; + return paths[1] ? nwo : undefined; + } catch (e) { + // Ignore the error here, since we catch failures at a higher level. + return; + } +} diff --git a/extensions/ql-vscode/src/databaseFetcher.ts b/extensions/ql-vscode/src/databaseFetcher.ts index f3d46e37024..e41753d5575 100644 --- a/extensions/ql-vscode/src/databaseFetcher.ts +++ b/extensions/ql-vscode/src/databaseFetcher.ts @@ -23,9 +23,9 @@ import { extLogger } from "./common"; import { Credentials } from "./authentication"; import { getErrorMessage } from "./pure/helpers-pure"; import { - convertGitHubUrlToNwo, - looksLikeGithubRepo, -} from "./databases/github-nwo"; + getNwoFromGitHubUrl, + validGitHubNwo, +} from "./common/github-url-identifier-helper"; /** * Prompts a user to fetch a database from a remote location. Database is assumed to be an archive file. @@ -100,7 +100,8 @@ export async function promptImportGithubDatabase( return; } - if (!looksLikeGithubRepo(githubRepo)) { + const nwo = getNwoFromGitHubUrl(githubRepo) || githubRepo; + if (!validGitHubNwo(nwo)) { throw new Error(`Invalid GitHub repository: ${githubRepo}`); } @@ -108,11 +109,7 @@ export async function promptImportGithubDatabase( ? await credentials.getOctokit(true) : new Octokit.Octokit({ retry }); - const result = await convertGithubNwoToDatabaseUrl( - githubRepo, - octokit, - progress, - ); + const result = await convertGithubNwoToDatabaseUrl(nwo, octokit, progress); if (!result) { return; } @@ -446,7 +443,7 @@ export async function findDirWithFile( } export async function convertGithubNwoToDatabaseUrl( - githubRepo: string, + nwo: string, octokit: Octokit.Octokit, progress: ProgressCallback, ): Promise< @@ -458,7 +455,6 @@ export async function convertGithubNwoToDatabaseUrl( | undefined > { try { - const nwo = convertGitHubUrlToNwo(githubRepo) || githubRepo; const [owner, repo] = nwo.split("/"); const response = await octokit.request( @@ -480,7 +476,7 @@ export async function convertGithubNwoToDatabaseUrl( }; } catch (e) { void extLogger.log(`Error: ${getErrorMessage(e)}`); - throw new Error(`Unable to get database for '${githubRepo}'`); + throw new Error(`Unable to get database for '${nwo}'`); } } diff --git a/extensions/ql-vscode/src/databases/github-nwo.ts b/extensions/ql-vscode/src/databases/github-nwo.ts deleted file mode 100644 index 5943e6db0f0..00000000000 --- a/extensions/ql-vscode/src/databases/github-nwo.ts +++ /dev/null @@ -1,51 +0,0 @@ -import { Uri } from "vscode"; -import { REPO_REGEX } from "../pure/helpers-pure"; - -/** - * The URL pattern is https://github.com/{owner}/{name}/{subpages}. - * - * This function accepts any URL that matches the pattern above. It also accepts just the - * name with owner (NWO): `/`. - * - * @param githubRepo The GitHub repository URL or NWO - * - * @return true if this looks like a valid GitHub repository URL or NWO - */ -export function looksLikeGithubRepo( - githubRepo: string | undefined, -): githubRepo is string { - if (!githubRepo) { - return false; - } - if (REPO_REGEX.test(githubRepo) || convertGitHubUrlToNwo(githubRepo)) { - return true; - } - return false; -} - -/** - * Converts a GitHub repository URL to the corresponding NWO. - * @param githubUrl The GitHub repository URL - * @return The corresponding NWO, or undefined if the URL is not valid - */ -export function convertGitHubUrlToNwo(githubUrl: string): string | undefined { - try { - const uri = Uri.parse(githubUrl, true); - if (uri.scheme !== "https") { - return; - } - if (uri.authority !== "github.com" && uri.authority !== "www.github.com") { - return; - } - const paths = uri.path.split("/").filter((segment: string) => segment); - const nwo = `${paths[0]}/${paths[1]}`; - if (REPO_REGEX.test(nwo)) { - return nwo; - } - return; - } catch (e) { - // Ignore the error here, since we catch failures at a higher level. - // In particular: returning undefined leads to an error in 'promptImportGithubDatabase'. - return; - } -} diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/databaseFetcher.test.ts b/extensions/ql-vscode/src/vscode-tests/no-workspace/databaseFetcher.test.ts index 15d7e4018f5..88c4a2b8431 100644 --- a/extensions/ql-vscode/src/vscode-tests/no-workspace/databaseFetcher.test.ts +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/databaseFetcher.test.ts @@ -8,7 +8,6 @@ import { findDirWithFile, } from "../../databaseFetcher"; import * as Octokit from "@octokit/rest"; -import { looksLikeGithubRepo } from "../../databases/github-nwo"; // These tests make API calls and may need extra time to complete. jest.setTimeout(10000); @@ -129,25 +128,6 @@ describe("databaseFetcher", () => { }); }); - describe("looksLikeGithubRepo", () => { - it("should handle invalid urls", () => { - expect(looksLikeGithubRepo("")).toBe(false); - expect(looksLikeGithubRepo("http://github.com/foo/bar")).toBe(false); - expect(looksLikeGithubRepo("https://ww.github.com/foo/bar")).toBe(false); - expect(looksLikeGithubRepo("https://ww.github.com/foo")).toBe(false); - expect(looksLikeGithubRepo("foo")).toBe(false); - }); - - it("should handle valid urls", () => { - expect(looksLikeGithubRepo("https://github.com/foo/bar")).toBe(true); - expect(looksLikeGithubRepo("https://www.github.com/foo/bar")).toBe(true); - expect(looksLikeGithubRepo("https://github.com/foo/bar/sub/pages")).toBe( - true, - ); - expect(looksLikeGithubRepo("foo/bar")).toBe(true); - }); - }); - describe("findDirWithFile", () => { let dir: tmp.DirResult; beforeEach(() => { diff --git a/extensions/ql-vscode/test/pure-tests/common/github-url-identifier-helper.test.ts b/extensions/ql-vscode/test/pure-tests/common/github-url-identifier-helper.test.ts new file mode 100644 index 00000000000..2c47f08ae09 --- /dev/null +++ b/extensions/ql-vscode/test/pure-tests/common/github-url-identifier-helper.test.ts @@ -0,0 +1,71 @@ +import { + getNwoFromGitHubUrl, + getOwnerFromGitHubUrl, + validGitHubNwo, + validGitHubOwner, +} from "../../../src/common/github-url-identifier-helper"; + +describe("github url identifier helper", () => { + describe("valid GitHub Nwo Or Owner method", () => { + it("should return true for valid owner", () => { + expect(validGitHubOwner("github")).toBe(true); + }); + it("should return true for valid NWO", () => { + expect(validGitHubNwo("github/codeql")).toBe(true); + }); + it("should return false for invalid owner", () => { + expect(validGitHubOwner("github/codeql")).toBe(false); + }); + it("should return false for invalid NWO", () => { + expect(validGitHubNwo("githubl")).toBe(false); + }); + }); + + describe("getNwoFromGitHubUrl method", () => { + it("should handle invalid urls", () => { + expect(getNwoFromGitHubUrl("")).toBe(undefined); + expect(getNwoFromGitHubUrl("http://github.com/foo/bar")).toBe(undefined); + expect(getNwoFromGitHubUrl("https://ww.github.com/foo/bar")).toBe( + undefined, + ); + expect(getNwoFromGitHubUrl("https://www.github.com/foo")).toBe(undefined); + expect(getNwoFromGitHubUrl("foo")).toBe(undefined); + expect(getNwoFromGitHubUrl("foo/bar")).toBe(undefined); + }); + + it("should handle valid urls", () => { + expect(getNwoFromGitHubUrl("https://github.com/foo/bar")).toBe("foo/bar"); + expect(getNwoFromGitHubUrl("https://www.github.com/foo/bar")).toBe( + "foo/bar", + ); + expect(getNwoFromGitHubUrl("https://github.com/foo/bar/sub/pages")).toBe( + "foo/bar", + ); + }); + }); + + describe("getOwnerFromGitHubUrl method", () => { + it("should handle invalid urls", () => { + expect(getOwnerFromGitHubUrl("")).toBe(undefined); + expect(getOwnerFromGitHubUrl("http://github.com/foo/bar")).toBe( + undefined, + ); + expect(getOwnerFromGitHubUrl("https://ww.github.com/foo/bar")).toBe( + undefined, + ); + expect(getOwnerFromGitHubUrl("foo")).toBe(undefined); + expect(getOwnerFromGitHubUrl("foo/bar")).toBe(undefined); + }); + + it("should handle valid urls", () => { + expect(getOwnerFromGitHubUrl("https://github.com/foo/bar")).toBe("foo"); + expect(getOwnerFromGitHubUrl("https://www.github.com/foo/bar")).toBe( + "foo", + ); + expect( + getOwnerFromGitHubUrl("https://github.com/foo/bar/sub/pages"), + ).toBe("foo"); + expect(getOwnerFromGitHubUrl("https://www.github.com/foo")).toBe("foo"); + }); + }); +});