Fix bad logic in GithubPackage#repositoryForProjectDirectory

If this function was called multiple times before the first call to
Repository.open resolves, it ended up clobbering the cache, resulting in
multiple Repository objects returned for the same working directory.

Additionally, it depended on referential equality of the passed
Directory object instead of the path of that Directory, causing
different repos to be returned for Directories with the same working
directory path.

Author: kuychaco

lib/github-package.js

@@ -151,10 +151,18 @@ export default class GithubPackage {
   }
 
   async repositoryForProjectDirectory(projectDirectory) {
-    let repository = this.repositoriesByProjectDirectory.get(projectDirectory);
+    const projectDirectoryPath = projectDirectory.getPath()
+    let repository = this.repositoriesByProjectDirectory.get(projectDirectoryPath);
     if (!repository) {
       repository = await Repository.open(projectDirectory);
-      if (repository) { this.repositoriesByProjectDirectory.set(projectDirectory, repository); }
+      if (this.repositoriesByProjectDirectory.has(projectDirectoryPath)) {
+        // Someone else found and set the repo in the cache while we were nabbing it.
+        // Defer to that repo.
+        repository.destroy();
+        repository = this.repositoriesByProjectDirectory.get(projectDirectoryPath);
+      } else if (repository) {
+        this.repositoriesByProjectDirectory.set(projectDirectoryPath, repository);
+      }
     }
     return repository;
   }
@@ -164,11 +172,11 @@ export default class GithubPackage {
   }
 
   destroyRepositoriesForRemovedProjectFolders() {
-    const projectDirectories = this.project.getDirectories();
-    for (const [projectDirectory, repository] of this.repositoriesByProjectDirectory) {
-      if (projectDirectories.indexOf(projectDirectory) === -1) {
+    const projectDirectoryPaths = this.project.getDirectories().map(dir => dir.getPath());
+    for (const [projectDirectoryPath, repository] of this.repositoriesByProjectDirectory) {
+      if (projectDirectoryPaths.indexOf(projectDirectoryPath) === -1) {
         repository.destroy();
-        this.repositoriesByProjectDirectory.delete(projectDirectory);
+        this.repositoriesByProjectDirectory.delete(projectDirectoryPath);
       }
     }
   }

test/github-package.test.js

@@ -1,9 +1,12 @@
 /** @babel */
 
+import {Directory} from 'atom';
+
 import fs from 'fs';
 import path from 'path';
 import temp from 'temp';
 import sinon from 'sinon';
+
 import {cloneRepository} from './helpers';
 import GithubPackage from '../lib/github-package';
 
@@ -157,4 +160,27 @@ describe('GithubPackage', () => {
       assert.isNull(githubPackage.getActiveRepository());
     });
   });
+
+  describe('#repositoryForProjectDirectory', () => {
+    it('returns the same repository over multiple overlapping calls', async () => {
+      const workdirPath = await cloneRepository('three-files');
+      const dir = new Directory(workdirPath);
+
+      const repo1 = githubPackage.repositoryForProjectDirectory(dir);
+      const repo2 = githubPackage.repositoryForProjectDirectory(dir);
+
+      assert.equal(await repo1, await repo2);
+    });
+
+    it('returns the same repository for different Directories with the same path', async () => {
+      const workdirPath = await cloneRepository('three-files');
+      const dir1 = new Directory(workdirPath);
+      const dir2 = new Directory(workdirPath);
+
+      const repo1 = await githubPackage.repositoryForProjectDirectory(dir1);
+      const repo2 = await githubPackage.repositoryForProjectDirectory(dir2);
+
+      assert.equal(repo1, repo2);
+    });
+  });
 });