Merge pull request #1888 from github/nora/refactor-githubnwoowner-helper

Extract github url identifier helper

Author: norascheuch

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

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,19 +100,16 @@ export async function promptImportGithubDatabase(
     return;
   }
 
-  if (!looksLikeGithubRepo(githubRepo)) {
+  const nwo = getNwoFromGitHubUrl(githubRepo) || githubRepo;
+  if (!validGitHubNwo(nwo)) {
     throw new Error(`Invalid GitHub repository: ${githubRepo}`);
   }
 
   const octokit = credentials
     ? 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}'`);
   }
 }
 

extensions/ql-vscode/src/databases/github-nwo.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(() => {

extensions/ql-vscode/src/vscode-tests/no-workspace/databaseFetcher.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");
+    });
+  });
+});