Merge pull request atom#365 from atom/ku-mkt-fix-cache-clobbering

Fix bad logic in GithubPackage#repositoryForProjectDirectory

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