Add ability to define repo lists in a file outside of settings (#1402)

Author: charisk

extensions/ql-vscode/src/config.ts

@@ -342,6 +342,21 @@ export async function setRemoteRepositoryLists(lists: Record<string, string[]> |
   await REMOTE_REPO_LISTS.updateValue(lists, ConfigurationTarget.Global);
 }
 
+/**
+ * Path to a file that contains lists of GitHub repositories that you want to query remotely via 
+ * the "Run Variant Analysis" command.
+ * Note: This command is only available for internal users.
+ * 
+ * This setting should be a path to a JSON file that contains a JSON object where each key is a
+ * user-specified name (string), and the value is an array of GitHub repositories 
+ * (of the form `<owner>/<repo>`).
+ */
+const REPO_LISTS_PATH = new Setting('repositoryListsPath', REMOTE_QUERIES_SETTING);
+
+export function getRemoteRepositoryListsPath(): string | undefined {
+  return REPO_LISTS_PATH.getValue<string>() || undefined;
+}
+
 /**
  * The name of the "controller" repository that you want to use with the "Run Variant Analysis" command.
  * Note: This command is only available for internal users.

extensions/ql-vscode/src/remote-queries/repository-selection.ts

@@ -1,6 +1,7 @@
+import * as fs from 'fs-extra';
 import { QuickPickItem, window } from 'vscode';
 import { logger } from '../logging';
-import { getRemoteRepositoryLists } from '../config';
+import { getRemoteRepositoryLists, getRemoteRepositoryListsPath } from '../config';
 import { OWNER_REGEX, REPO_REGEX } from '../pure/helpers-pure';
 import { UserCancellationException } from '../commandRunner';
 
@@ -17,6 +18,11 @@ interface RepoListQuickPickItem extends QuickPickItem {
   useAllReposOfOwner?: boolean;
 }
 
+interface RepoList {
+  label: string;
+  repositories: string[];
+}
+
 /**
  * Gets the repositories or repository lists to run the query against.
  * @returns The user selection.
@@ -26,7 +32,7 @@ export async function getRepositorySelection(): Promise<RepositorySelection> {
     createCustomRepoQuickPickItem(),
     createAllReposOfOwnerQuickPickItem(),
     ...createSystemDefinedRepoListsQuickPickItems(),
-    ...createUserDefinedRepoListsQuickPickItems(),
+    ...(await createUserDefinedRepoListsQuickPickItems()),
   ];
 
   const options = {
@@ -88,20 +94,81 @@ function createSystemDefinedRepoListsQuickPickItems(): RepoListQuickPickItem[] {
   } as RepoListQuickPickItem));
 }
 
-function createUserDefinedRepoListsQuickPickItems(): RepoListQuickPickItem[] {
+async function readExternalRepoLists(): Promise<RepoList[]> {
+  const repoLists: RepoList[] = [];
+
+  const path = getRemoteRepositoryListsPath();
+  if (!path) {
+    return repoLists;
+  }
+
+  await validateExternalRepoListsFile(path);
+  const json = await readExternalRepoListsJson(path);
+
+  for (const [repoListName, repositories] of Object.entries(json)) {
+    if (!Array.isArray(repositories)) {
+      throw Error('Invalid repository lists file. It should contain an array of repositories for each list.');
+    }
+
+    repoLists.push({
+      label: repoListName,
+      repositories
+    });
+  }
+
+  return repoLists;
+}
+
+async function validateExternalRepoListsFile(path: string): Promise<void> {
+  const pathExists = await fs.pathExists(path);
+  if (!pathExists) {
+    throw Error(`External repository lists file does not exist at ${path}`);
+  }
+
+  const pathStat = await fs.stat(path);
+  if (pathStat.isDirectory()) {
+    throw Error('External repository lists path should not point to a directory');
+  }
+}
+
+async function readExternalRepoListsJson(path: string): Promise<Record<string, unknown>> {
+  let json;
+
+  try {
+    const fileContents = await fs.readFile(path, 'utf8');
+    json = await JSON.parse(fileContents);
+  } catch (error) {
+    throw Error('Invalid repository lists file. It should contain valid JSON.');
+  }
+
+  if (Array.isArray(json)) {
+    throw Error('Invalid repository lists file. It should be an object mapping names to a list of repositories.');
+  }
+
+  return json;
+}
+
+function readRepoListsFromSettings(): RepoList[] {
   const repoLists = getRemoteRepositoryLists();
   if (!repoLists) {
     return [];
   }
 
-  return Object.entries(repoLists).map<RepoListQuickPickItem>(([label, repositories]) => (
+  return Object.entries(repoLists).map<RepoList>(([label, repositories]) => (
     {
-      label,            // the name of the repository list
-      repositories  // the actual array of repositories
+      label,
+      repositories
     }
   ));
 }
 
+async function createUserDefinedRepoListsQuickPickItems(): Promise<RepoListQuickPickItem[]> {
+  const repoListsFromSetings = readRepoListsFromSettings();
+  const repoListsFromExternalFile = await readExternalRepoLists();
+
+  return [...repoListsFromSetings, ...repoListsFromExternalFile];
+}
+
 function createCustomRepoQuickPickItem(): RepoListQuickPickItem {
   return {
     label: '$(edit) Enter a GitHub repository',

extensions/ql-vscode/src/vscode-tests/no-workspace/remote-queries/repository-selection.test.ts

@@ -2,34 +2,57 @@ import * as sinon from 'sinon';
 import { expect } from 'chai';
 import { window } from 'vscode';
 import * as pq from 'proxyquire';
+import * as fs from 'fs-extra';
 import { UserCancellationException } from '../../../commandRunner';
 
 const proxyquire = pq.noPreserveCache();
 
-describe('repository-selection', function() {
-
-  describe('getRepositorySelection', () => {
-    let sandbox: sinon.SinonSandbox;
-    let quickPickSpy: sinon.SinonStub;
-    let showInputBoxSpy: sinon.SinonStub;
-    let getRemoteRepositoryListsSpy: sinon.SinonStub;
-    let mod: any;
-    beforeEach(() => {
-      sandbox = sinon.createSandbox();
-      quickPickSpy = sandbox.stub(window, 'showQuickPick');
-      showInputBoxSpy = sandbox.stub(window, 'showInputBox');
-      getRemoteRepositoryListsSpy = sandbox.stub();
-      mod = proxyquire('../../../remote-queries/repository-selection', {
-        '../config': {
-          getRemoteRepositoryLists: getRemoteRepositoryListsSpy
-        },
-      });
-    });
+describe('repository selection', async () => {
+  let sandbox: sinon.SinonSandbox;
+
+  let quickPickSpy: sinon.SinonStub;
+  let showInputBoxSpy: sinon.SinonStub;
+
+  let getRemoteRepositoryListsSpy: sinon.SinonStub;
+  let getRemoteRepositoryListsPathSpy: sinon.SinonStub;
+
+  let pathExistsStub: sinon.SinonStub;
+  let fsStatStub: sinon.SinonStub;
+  let fsReadFileStub: sinon.SinonStub;
+
+  let mod: any;
 
-    afterEach(() => {
-      sandbox.restore();
+  beforeEach(() => {
+    sandbox = sinon.createSandbox();
+
+    quickPickSpy = sandbox.stub(window, 'showQuickPick');
+    showInputBoxSpy = sandbox.stub(window, 'showInputBox');
+
+    getRemoteRepositoryListsSpy = sandbox.stub();
+    getRemoteRepositoryListsPathSpy = sandbox.stub();
+
+    pathExistsStub = sandbox.stub(fs, 'pathExists');
+    fsStatStub = sandbox.stub(fs, 'stat');
+    fsReadFileStub = sandbox.stub(fs, 'readFile');
+
+    mod = proxyquire('../../../remote-queries/repository-selection', {
+      '../config': {
+        getRemoteRepositoryLists: getRemoteRepositoryListsSpy,
+        getRemoteRepositoryListsPath: getRemoteRepositoryListsPathSpy
+      },
+      'fs-extra': {
+        pathExists: pathExistsStub,
+        stat: fsStatStub,
+        readFile: fsReadFileStub
+      }
     });
+  });
 
+  afterEach(() => {
+    sandbox.restore();
+  });
+
+  describe('repo lists from settings', async () => {
     it('should allow selection from repo lists from your pre-defined config', async () => {
       // Fake return values
       quickPickSpy.resolves(
@@ -52,7 +75,9 @@ describe('repository-selection', function() {
         ['foo/bar', 'foo/baz']
       );
     });
+  });
 
+  describe('system level repo lists', async () => {
     it('should allow selection from repo lists defined at the system level', async () => {
       // Fake return values
       quickPickSpy.resolves(
@@ -121,7 +146,9 @@ describe('repository-selection', function() {
         await expect(mod.getRepositorySelection()).to.be.rejectedWith(Error, `Invalid user or organization: ${owner}`);
       });
     });
+  });
 
+  describe('custom repo', async () => {
     // Test the repo regex in various "good" cases
     const goodRepos = [
       'owner/repo',
@@ -169,6 +196,85 @@ describe('repository-selection', function() {
         await expect(mod.getRepositorySelection()).to.be.rejectedWith(UserCancellationException, 'Invalid repository format');
       });
     });
+  });
+
+  describe('external repository lists file', async () => {
+    it('should fail if path does not exist', async () => {
+      const fakeFilePath = '/path/that/does/not/exist.json';
+      getRemoteRepositoryListsPathSpy.returns(fakeFilePath);
+      pathExistsStub.resolves(false);
+
+      await expect(mod.getRepositorySelection()).to.be.rejectedWith(Error, `External repository lists file does not exist at ${fakeFilePath}`);
+    });
+
+    it('should fail if path points to directory', async () => {
+      const fakeFilePath = '/path/to/dir';
+      getRemoteRepositoryListsPathSpy.returns(fakeFilePath);
+      pathExistsStub.resolves(true);
+      fsStatStub.resolves({ isDirectory: () => true } as any);
+
+      await expect(mod.getRepositorySelection()).to.be.rejectedWith(Error, 'External repository lists path should not point to a directory');
+    });
 
+    it('should fail if file does not have valid JSON', async () => {
+      const fakeFilePath = '/path/to/file.json';
+      getRemoteRepositoryListsPathSpy.returns(fakeFilePath);
+      pathExistsStub.resolves(true);
+      fsStatStub.resolves({ isDirectory: () => false } as any);
+      fsReadFileStub.resolves('not-json' as any as Buffer);
+
+      await expect(mod.getRepositorySelection()).to.be.rejectedWith(Error, 'Invalid repository lists file. It should contain valid JSON.');
+    });
+
+    it('should fail if file contains array', async () => {
+      const fakeFilePath = '/path/to/file.json';
+      getRemoteRepositoryListsPathSpy.returns(fakeFilePath);
+      pathExistsStub.resolves(true);
+      fsStatStub.resolves({ isDirectory: () => false } as any);
+      fsReadFileStub.resolves('[]' as any as Buffer);
+
+      await expect(mod.getRepositorySelection()).to.be.rejectedWith(Error, 'Invalid repository lists file. It should be an object mapping names to a list of repositories.');
+    });
+
+    it('should fail if file does not contain repo lists in the right format', async () => {
+      const fakeFilePath = '/path/to/file.json';
+      getRemoteRepositoryListsPathSpy.returns(fakeFilePath);
+      pathExistsStub.resolves(true);
+      fsStatStub.resolves({ isDirectory: () => false } as any);
+      const repoLists = {
+        'list1': 'owner1/repo1',
+      };
+      fsReadFileStub.resolves(JSON.stringify(repoLists) as any as Buffer);
+
+      await expect(mod.getRepositorySelection()).to.be.rejectedWith(Error, 'Invalid repository lists file. It should contain an array of repositories for each list.');
+    });
+
+    it('should get repo lists from file', async () => {
+      const fakeFilePath = '/path/to/file.json';
+      getRemoteRepositoryListsPathSpy.returns(fakeFilePath);
+      pathExistsStub.resolves(true);
+      fsStatStub.resolves({ isDirectory: () => false } as any);
+      const repoLists = {
+        'list1': ['owner1/repo1', 'owner2/repo2'],
+        'list2': ['owner3/repo3']
+      };
+      fsReadFileStub.resolves(JSON.stringify(repoLists) as any as Buffer);
+      getRemoteRepositoryListsSpy.returns(
+        {
+          'list3': ['onwer4/repo4'],
+          'list4': [],
+        }
+      );
+
+      quickPickSpy.resolves(
+        { repositories: ['owner3/repo3'] }
+      );
+
+      const repoSelection = await mod.getRepositorySelection();
+
+      expect(repoSelection.repositoryLists).to.be.undefined;
+      expect(repoSelection.owners).to.be.undefined;
+      expect(repoSelection.repositories).to.deep.eq(['owner3/repo3']);
+    });
   });
 });