Return a null object from the pool when a directory is not present

Author: smashwilson

lib/github-package.js

@@ -83,8 +83,6 @@ export default class GithubPackage {
       this.workspace.addOpener(IssueishPaneItem.opener),
     );
 
-    this.deserialize(this.savedState);
-
     this.rerender();
   }
 
@@ -106,12 +104,6 @@ export default class GithubPackage {
     };
   }
 
-  async deserialize(state) {
-    if (state.activeRepositoryPath) {
-      await this.didChangeProjectPaths([state.activeRepositoryPath]);
-    }
-  }
-
   @autobind
   rerender() {
     if (!this.element) {
@@ -207,44 +199,61 @@ export default class GithubPackage {
     return this.activeResolutionProgress;
   }
 
-  async scheduleActiveModelUpdate() {
-    await this.activeModelQueue.push(this.updateActiveModels, {parallel: false});
+  async scheduleActiveModelUpdate(isActivating = false) {
+    await this.activeModelQueue.push(this.updateActiveModels.bind(this, isActivating), {parallel: false});
   }
 
-  @autobind
-  async updateActiveModels() {
-    let nextActiveRepository = null;
-    let nextActiveResolutionProgress = null;
-    let nextActiveProjectPath = null;
+  async updateActiveModels(isActivating = false) {
+    let foundActiveModels = false;
+
+    let nextActiveRepository = this.activeRepository;
+    let nextActiveResolutionProgress = this.activeResolutionProgress;
+    let nextActiveProjectPath = this.activeProjectPath;
+
+    const setByProjectPath = async projectPath => {
+      if (!projectPath) {
+        return;
+      }
+
+      nextActiveProjectPath = projectPath;
+      nextActiveRepository = await this.getRepositoryForWorkdirPath(nextActiveProjectPath);
+      nextActiveResolutionProgress = await this.getResolutionProgressForWorkdirPath(nextActiveProjectPath);
+
+      foundActiveModels = true;
+    };
 
     const activeItem = this.workspace.getActivePaneItem();
-    if (activeItem && typeof activeItem.getPath === 'function') {
-      const projectPath = this.projectPathForItemPath(activeItem.getPath());
-      if (projectPath) {
-        nextActiveProjectPath = projectPath;
-        await Promise.all([
-          this.getRepositoryForWorkdirPath(projectPath).then(r => { nextActiveRepository = r; }),
-          this.getResolutionProgressForWorkdirPath(projectPath).then(rp => { nextActiveResolutionProgress = rp; }),
-        ]);
+
+    if (activeItem) {
+      if (typeof activeItem.getRepository === 'function') {
+        // FilePatchController or other GitHub package pane
+        const repository = activeItem.getRepository();
+        if (!repository.isDestroyed()) {
+          nextActiveRepository = repository;
+          nextActiveResolutionProgress = this.props.activeResolutionProgress;
+          nextActiveProjectPath = this.props.activeProjectPath;
+
+          foundActiveModels = true;
+        }
+
+      } else if (typeof activeItem.getPath === 'function') {
+        // TextEditor-like
+        const projectPath = this.projectPathForItemPath(activeItem.getPath());
+        await setByProjectPath(projectPath);
       }
-    } else {
-      nextActiveRepository = this.activeRepository;
-      nextActiveResolutionProgress = this.activeResolutionProgress;
-      nextActiveProjectPath = this.activeProjectPath;
     }
 
-    if (activeItem instanceof FilePatchController) {
-      if (!activeItem.getRepository().isDestroyed()) {
-        nextActiveRepository = activeItem.props.repository;
-        nextActiveResolutionProgress = activeItem.props.resolutionProgress;
-        nextActiveProjectPath = nextActiveRepository.getWorkingDirectoryPath();
-      }
+    if (!foundActiveModels && this.project.getPaths().length === 1) {
+      // Single project
+      await setByProjectPath(this.project.getPaths()[0]);
     }
 
-    if (!activeItem && this.project.getPaths().length === 1) {
-      nextActiveProjectPath = this.project.getPaths()[0];
-      nextActiveRepository = await this.getRepositoryForWorkdirPath(nextActiveProjectPath);
-      nextActiveResolutionProgress = await this.getResolutionProgressForWorkdirPath(nextActiveProjectPath);
+    if (!foundActiveModels && isActivating) {
+      // Restore models from saved state
+      if (this.savedState.activeRepositoryPath) {
+        const projectPath = this.projectPathForItemPath(this.savedState.activeRepositoryPath);
+        await setByProjectPath(projectPath);
+      }
     }
 
     this.setActiveModels(nextActiveProjectPath, nextActiveRepository, nextActiveResolutionProgress);

lib/models/workdir-context-pool.js

@@ -1,6 +1,6 @@
 import compareSets from 'compare-sets';
 
-import WorkdirContext from './workdir-context';
+import WorkdirContext, {nullWorkdirContext} from './workdir-context';
 
 /**
  * Manage a WorkdirContext for each open directory.
@@ -24,7 +24,7 @@ export default class WorkdirContextPool {
    * Access the context mapped to a known directory.
    */
   getContext(directory) {
-    return this.contexts.get(directory);
+    return this.contexts.get(directory) || nullWorkdirContext;
   }
 
   add(directory, options = {}) {

lib/models/workdir-context.js

@@ -113,6 +113,10 @@ export default class WorkdirContext {
     ]);
   }
 
+  isPresent() {
+    return true;
+  }
+
   isDestroyed() {
     return this.destroyed;
   }
@@ -170,3 +174,27 @@ export default class WorkdirContext {
     }
   }
 }
+
+class NullWorkdirContext extends WorkdirContext {
+  constructor() {
+    super(null, {});
+
+    this.repositoryHolder.setValue(null);
+    this.changeObserverHolder.setValue(null);
+    this.resolutionProgressHolder.setValue(null);
+  }
+
+  create() {
+    return Promise.resolve();
+  }
+
+  isPresent() {
+    return false;
+  }
+
+  destroy() {
+    return Promise.resolve();
+  }
+}
+
+export const nullWorkdirContext = new NullWorkdirContext();

test/github-package.test.js

@@ -64,17 +64,94 @@ describe('GithubPackage', function() {
   }
 
   describe('activate()', function() {
-    it('updates the active repository', async function() {
+    it('uses models from preexisting projects', async function() {
+      const workdirPath1 = await cloneRepository('three-files');
+      const workdirPath2 = await cloneRepository('three-files');
+      project.setPaths([workdirPath1, workdirPath2]);
+
+      await githubPackage.activate();
+
+      await assert.async.isNotNull(await githubPackage.getRepositoryForWorkdirPath(workdirPath1));
+      await assert.async.isNotNull(await githubPackage.getRepositoryForWorkdirPath(workdirPath2));
+      assert.isNull(githubPackage.getActiveRepository());
+    });
+
+    it('uses an active model from a single preexisting project', async function() {
+      const workdirPath = await cloneRepository('three-files');
+      project.setPaths([workdirPath]);
+
       await githubPackage.activate();
+
+      await assert.async.isNotNull(githubPackage.getActiveRepository());
+      assert.equal(
+        await githubPackage.getActiveRepository().getWorkingDirectoryPath(),
+        workdirPath,
+      );
+    });
+
+    it('uses an active model from a preexisting active pane item', async function() {
       const workdirPath1 = await cloneRepository('three-files');
       const workdirPath2 = await cloneRepository('three-files');
       project.setPaths([workdirPath1, workdirPath2]);
-      fs.writeFileSync(path.join(workdirPath1, 'a.txt'), 'change 1', 'utf8');
-      fs.writeFileSync(path.join(workdirPath1, 'b.txt'), 'change 2', 'utf8');
+      await workspace.open(path.join(workdirPath2, 'a.txt'));
 
-      await workspace.open(path.join(workdirPath1, 'a.txt'));
-      const repository = await githubPackage.getRepositoryForWorkdirPath(workdirPath1);
-      await until(() => githubPackage.getActiveRepository() === repository);
+      await githubPackage.activate();
+
+      await assert.async.isNotNull(githubPackage.getActiveRepository());
+      assert.equal(
+        await githubPackage.getActiveRepository().getWorkingDirectoryPath(),
+        workdirPath2,
+      );
+    });
+
+    it('uses an active model from serialized state', async function() {
+      const workdirPath1 = await cloneRepository('three-files');
+      const workdirPath2 = await cloneRepository('three-files');
+      const workdirPath3 = await cloneRepository('three-files');
+      project.setPaths([workdirPath1, workdirPath2, workdirPath3]);
+
+      await githubPackage.activate({
+        activeRepositoryPath: workdirPath2,
+      });
+
+      await assert.async.isNotNull(githubPackage.getActiveRepository());
+      assert.equal(
+        await githubPackage.getActiveRepository().getWorkingDirectoryPath(),
+        workdirPath2,
+      );
+    });
+
+    it('prefers the active model from an active pane item to serialized state', async function() {
+      const workdirPath1 = await cloneRepository('three-files');
+      const workdirPath2 = await cloneRepository('three-files');
+      project.setPaths([workdirPath1, workdirPath2]);
+      await workspace.open(path.join(workdirPath2, 'b.txt'));
+
+      await githubPackage.activate({
+        activeRepositoryPath: workdirPath1,
+      });
+
+      await assert.async.isNotNull(githubPackage.getActiveRepository());
+      assert.equal(
+        await githubPackage.getActiveRepository().getWorkingDirectoryPath(),
+        workdirPath2,
+      );
+    });
+
+    it('prefers the active model from a single project to serialized state', async function() {
+      const workdirPath1 = await cloneRepository('three-files');
+      const workdirPath2 = await cloneRepository('three-files');
+      project.setPaths([workdirPath1]);
+
+      await githubPackage.activate({
+        activeRepositoryPath: workdirPath2,
+      });
+
+      await assert.async.isNotNull(githubPackage.getActiveRepository());
+      assert.equal(
+        await githubPackage.getActiveRepository().getWorkingDirectoryPath(),
+        workdirPath1,
+      );
     });
   });
 

test/models/workdir-context-pool.test.js

@@ -28,12 +28,12 @@ describe('WorkdirContextPool', function() {
 
     it('adds a WorkdirContext for a new working directory', function() {
       assert.equal(pool.size(), 0);
-      assert.isUndefined(pool.getContext(workingDirectory));
+      assert.isFalse(pool.getContext(workingDirectory).isPresent());
 
       pool.add(workingDirectory);
 
       assert.equal(pool.size(), 1);
-      assert.isDefined(pool.getContext(workingDirectory));
+      assert.isTrue(pool.getContext(workingDirectory).isPresent());
     });
 
     it('optionally provides a preinitialized repository', async function() {
@@ -50,7 +50,7 @@ describe('WorkdirContextPool', function() {
       assert.equal(pool.size(), 1);
 
       const context = pool.getContext(workingDirectory);
-      assert.isDefined(context);
+      assert.isTrue(context.isPresent());
 
       pool.add(workingDirectory);
       assert.equal(pool.size(), 1);
@@ -79,7 +79,7 @@ describe('WorkdirContextPool', function() {
     it('removes a WorkdirContext for an existing working directory', function() {
       assert.equal(pool.size(), 1);
       pool.remove(existingDirectory);
-      assert.isUndefined(pool.getContext(existingDirectory));
+      assert.isFalse(pool.getContext(existingDirectory).isPresent());
       assert.equal(pool.size(), 0);
     });
 
@@ -122,9 +122,34 @@ describe('WorkdirContextPool', function() {
 
       assert.equal(pool.size(), 2);
 
+      assert.isFalse(pool.getContext(dir0).isPresent());
+      assert.isTrue(pool.getContext(dir1).isPresent());
+      assert.isTrue(pool.getContext(dir2).isPresent());
+
       assert.isTrue(context0.isDestroyed());
       assert.isFalse(context1.isDestroyed());
-      assert.isDefined(pool.getContext(dir2));
+    });
+  });
+
+  describe('getContext', function() {
+    let dir;
+
+    beforeEach(async function() {
+      dir = await cloneRepository('three-files');
+      pool.add(dir);
+    });
+
+    it('returns a context by directory', async function() {
+      const context = pool.getContext(dir);
+      assert.isTrue(context.isPresent());
+
+      const repo = await context.getRepositoryPromise();
+      assert.strictEqual(dir, repo.getWorkingDirectoryPath());
+    });
+
+    it('returns a null context when missing', function() {
+      const context = pool.getContext('/nope');
+      assert.isFalse(context.isPresent());
     });
   });
 });