Merge pull request #810 from fcollonval/fcollonval/issue809

Fixes top repository refresh

Author: fcollonval

src/model.ts

@@ -137,26 +137,18 @@ export class GitExtension implements IGitExtension {
       const currentReady = this._readyPromise;
       this._pendingReadyPromise += 1;
       this._readyPromise = Promise.all([currentReady, this.showTopLevel(v)])
-        .then(r => {
-          const results = r[1];
-          if (results.code === 0) {
-            this._pathRepository = results.top_repo_path;
-            change.newValue = results.top_repo_path;
-          } else {
-            this._pathRepository = null;
-          }
+        .then(([_, path]) => {
+          change.newValue = this._pathRepository = path;
 
           if (change.newValue !== change.oldValue) {
             this.refresh().then(() => this._repositoryChanged.emit(change));
           }
+          this._pendingReadyPromise -= 1;
         })
         .catch(reason => {
+          this._pendingReadyPromise -= 1;
           console.error(`Fail to find Git top level for path ${v}.\n${reason}`);
         });
-
-      void this._readyPromise.then(() => {
-        this._pendingReadyPromise -= 1;
-      });
     }
   }
 
@@ -865,15 +857,31 @@ export class GitExtension implements IGitExtension {
    * @throws {Git.GitResponseError} If the server response is not ok
    * @throws {ServerConnection.NetworkError} If the request cannot be made
    */
-  async showPrefix(path: string): Promise<Git.IShowPrefixResult> {
-    return await this._taskHandler.execute<Git.IShowPrefixResult>(
-      'git:fetch:prefix_path',
-      async () => {
-        return await requestAPI<Git.IShowPrefixResult>('show_prefix', 'POST', {
-          current_path: path
-        });
+  async showPrefix(path: string): Promise<string | null> {
+    try {
+      const data = await this._taskHandler.execute<Git.IShowPrefixResult>(
+        'git:fetch:prefix_path',
+        async () => {
+          return await requestAPI<Git.IShowPrefixResult>(
+            'show_prefix',
+            'POST',
+            {
+              current_path: path
+            }
+          );
+        }
+      );
+      return data.under_repo_path || null;
+    } catch (error) {
+      if (
+        error instanceof Git.GitResponseError &&
+        error.response.status === 500 &&
+        error.json.code === 128
+      ) {
+        return null;
       }
-    );
+      throw error;
+    }
   }
 
   /**
@@ -885,19 +893,31 @@ export class GitExtension implements IGitExtension {
    * @throws {Git.GitResponseError} If the server response is not ok
    * @throws {ServerConnection.NetworkError} If the request cannot be made
    */
-  async showTopLevel(path: string): Promise<Git.IShowTopLevelResult> {
-    return await this._taskHandler.execute<Git.IShowTopLevelResult>(
-      'git:fetch:top_level_path',
-      async () => {
-        return await requestAPI<Git.IShowTopLevelResult>(
-          'show_top_level',
-          'POST',
-          {
-            current_path: path
-          }
-        );
+  async showTopLevel(path: string): Promise<string | null> {
+    try {
+      const data = await this._taskHandler.execute<Git.IShowTopLevelResult>(
+        'git:fetch:top_level_path',
+        async () => {
+          return await requestAPI<Git.IShowTopLevelResult>(
+            'show_top_level',
+            'POST',
+            {
+              current_path: path
+            }
+          );
+        }
+      );
+      return data.top_repo_path || null;
+    } catch (error) {
+      if (
+        error instanceof Git.GitResponseError &&
+        error.response.status === 500 &&
+        error.json.code === 128
+      ) {
+        return null;
       }
-    );
+      throw error;
+    }
   }
 
   /**

src/tokens.ts

@@ -374,7 +374,7 @@ export interface IGitExtension extends IDisposable {
   revertCommit(message: string, hash: string): Promise<void>;
 
   /**
-   * Make request for the prefix path of a directory 'path',
+   * Get the prefix path of a directory 'path',
    * with respect to the root directory of repository
    *
    * @param path Path for which the prefix is searched for
@@ -383,17 +383,17 @@ export interface IGitExtension extends IDisposable {
    * @throws {Git.GitResponseError} If the server response is not ok
    * @throws {ServerConnection.NetworkError} If the request cannot be made
    */
-  showPrefix(path: string): Promise<Git.IShowPrefixResult>;
+  showPrefix(path: string): Promise<string | null>;
 
   /**
-   * Make request for top level path of repository 'path'
+   * Get the top level path of repository 'path'
    *
    * @param path Path from which the top Git repository needs to be found
    *
    * @throws {Git.GitResponseError} If the server response is not ok
    * @throws {ServerConnection.NetworkError} If the request cannot be made
    */
-  showTopLevel(path: string): Promise<Git.IShowTopLevelResult>;
+  showTopLevel(path: string): Promise<string | null>;
 
   /**
    * Make request to list all the tags present in the remote repo

tests/GitExtension.spec.tsx renamed to tests/model.spec.tsx

@@ -119,6 +119,82 @@ describe('IGitExtension', () => {
     });
   });
 
+  describe('#showPrefix', () => {
+    it('should return a string if the folder is a git repository', async () => {
+      const fakeRepo = '/repo';
+      mockResponses['show_prefix'] = {
+        body: () => {
+          return { code: 0, under_repo_path: fakeRepo };
+        }
+      };
+      const topLevel = await model.showPrefix('/repo/cwd');
+      expect(topLevel).toEqual(fakeRepo);
+    });
+
+    it('should return null if the repository is not a git repository', async () => {
+      mockResponses['show_prefix'] = {
+        body: () => {
+          return { code: 128 };
+        },
+        status: 500
+      };
+      const topLevel = await model.showPrefix('/repo/cwd');
+      expect(topLevel).toBeNull();
+    });
+
+    it('should throw an exception if the server otherwise', async () => {
+      mockResponses['show_prefix'] = {
+        body: () => {
+          return { code: 128 };
+        },
+        status: 401
+      };
+      try {
+        await model.showPrefix('/repo/cwd');
+      } catch (error) {
+        expect(error).toBeInstanceOf(Git.GitResponseError);
+      }
+    });
+  });
+
+  describe('#showTopLevel', () => {
+    it('should return a string if the folder is a git repository', async () => {
+      const fakeRepo = '/path/to/repo';
+      mockResponses['show_top_level'] = {
+        body: () => {
+          return { code: 0, top_repo_path: fakeRepo };
+        }
+      };
+      const topLevel = await model.showTopLevel('/path/to/repo/cwd');
+      expect(topLevel).toEqual(fakeRepo);
+    });
+
+    it('should return null if the repository is not a git repository', async () => {
+      mockResponses['show_top_level'] = {
+        body: () => {
+          return { code: 128 };
+        },
+        status: 500
+      };
+      const topLevel = await model.showTopLevel('/path/to/repo/cwd');
+      expect(topLevel).toBeNull();
+    });
+
+    it('should throw an exception if the server otherwise', async () => {
+      mockResponses['show_top_level'] = {
+        body: () => {
+          return { code: 128 };
+        },
+        status: 401
+      };
+      try {
+        await model.showTopLevel('/path/to/repo/cwd');
+      } catch (error) {
+        expect(error).toBeInstanceOf(Git.GitResponseError);
+      }
+    });
+  });
+
   describe('#status', () => {
     it('should be an empty list if not in a git repository', async () => {
       let status: Git.IStatusFileResult[] = [];

tests/plugin.spec.ts

@@ -6,8 +6,7 @@ import { ISettingRegistry, SettingRegistry } from '@jupyterlab/settingregistry';
 import { JupyterLab } from '@jupyterlab/application';
 import { showErrorMessage } from '@jupyterlab/apputils';
 import { URLExt } from '@jupyterlab/coreutils';
-import { ReadonlyJSONObject } from '@lumino/coreutils';
-import { mockedRequestAPI } from './utils';
+import { IMockedResponses, mockedRequestAPI } from './utils';
 
 jest.mock('../src/git');
 jest.mock('@jupyterlab/application');
@@ -18,12 +17,7 @@ describe('plugin', () => {
   const mockGit = git as jest.Mocked<typeof git>;
   const fakeRoot = '/path/to/server';
   let app: jest.Mocked<JupyterLab>;
-  let mockResponses: {
-    [url: string]: {
-      body?: (request: Object) => ReadonlyJSONObject;
-      status?: number;
-    };
-  } = {};
+  let mockResponses: IMockedResponses = {};
   let settingRegistry: jest.Mocked<ISettingRegistry>;
 
   beforeAll(() => {