gitrest: add FS Manager interface (#9496)

server/gitrest/package.json

@@ -59,7 +59,7 @@
     "@types/mocha": "^8.2.2",
     "@types/morgan": "^1.7.32",
     "@types/nconf": "^0.10.0",
-    "@types/node": "^12.12.54",
+    "@types/node": "^14.18.12",
     "@types/nodegit": "^0.27.3",
     "@types/rimraf": "^2.0.2",
     "@types/supertest": "^2.0.7",

server/gitrest/src/app.ts

@@ -11,9 +11,8 @@ import nconf from "nconf";
 import split from "split";
 import winston from "winston";
 import { bindCorrelationId } from "@fluidframework/server-services-utils";
-import { IExternalStorageManager } from "./externalStorageManager";
 import * as routes from "./routes";
-import { NodegitRepositoryManagerFactory } from "./utils";
+import { IRepositoryManagerFactory } from "./utils";
 
 /**
  * Basic stream logging interface for libraries that require a stream to pipe output to
@@ -24,7 +23,7 @@ const stream = split().on("data", (message) => {
 
 export function create(
     store: nconf.Provider,
-    externalStorageManager: IExternalStorageManager,
+    repositoryManagerFactory: IRepositoryManagerFactory,
 ) {
     // Express app configuration
     const app: Express = express();
@@ -55,8 +54,8 @@ export function create(
     app.use(bindCorrelationId());
 
     app.use(cors());
-    const repoManagerFactory = new NodegitRepositoryManagerFactory(store.get("storageDir"), externalStorageManager);
-    const apiRoutes = routes.create(store, repoManagerFactory);
+
+    const apiRoutes = routes.create(store, repositoryManagerFactory);
     app.use(apiRoutes.git.blobs);
     app.use(apiRoutes.git.refs);
     app.use(apiRoutes.git.repos);

server/gitrest/src/runner.ts

@@ -8,7 +8,7 @@ import { IWebServer, IWebServerFactory, IRunner } from "@fluidframework/server-s
 import { Provider } from "nconf";
 import * as winston from "winston";
 import * as app from "./app";
-import { IExternalStorageManager } from "./externalStorageManager";
+import { IRepositoryManagerFactory } from "./utils";
 
 export class GitrestRunner implements IRunner {
     private server: IWebServer;
@@ -18,13 +18,13 @@ export class GitrestRunner implements IRunner {
         private readonly serverFactory: IWebServerFactory,
         private readonly config: Provider,
         private readonly port: string | number,
-        private readonly externalStorageManager: IExternalStorageManager) {
+        private readonly repositoryManagerFactory: IRepositoryManagerFactory) {
     }
 
     public async start(): Promise<void> {
         this.runningDeferred = new Deferred<void>();
         // Create the gitrest app
-        const gitrest = app.create(this.config, this.externalStorageManager);
+        const gitrest = app.create(this.config, this.repositoryManagerFactory);
         gitrest.set("port", this.port);
 
         this.server = this.serverFactory.create(gitrest);

server/gitrest/src/runnerFactory.ts

@@ -3,20 +3,22 @@
  * Licensed under the MIT License.
  */
 
+import fsPromises from "fs/promises";
 import { Provider } from "nconf";
 import * as services from "@fluidframework/server-services-shared";
 import * as core from "@fluidframework/server-services-core";
 import { normalizePort } from "@fluidframework/server-services-utils";
-import { ExternalStorageManager, IExternalStorageManager } from "./externalStorageManager";
+import { ExternalStorageManager } from "./externalStorageManager";
 import { GitrestRunner } from "./runner";
+import { IRepositoryManagerFactory, NodegitRepositoryManagerFactory } from "./utils";
 
 export class GitrestResources implements core.IResources {
     public webServerFactory: core.IWebServerFactory;
 
     constructor(
         public readonly config: Provider,
         public readonly port: string | number,
-        public readonly externalStorageManager: IExternalStorageManager) {
+        public readonly repositoryManagerFactory: IRepositoryManagerFactory) {
         this.webServerFactory = new services.BasicWebServerFactory();
     }
 
@@ -28,9 +30,15 @@ export class GitrestResources implements core.IResources {
 export class GitrestResourcesFactory implements core.IResourcesFactory<GitrestResources> {
     public async create(config: Provider): Promise<GitrestResources> {
         const port = normalizePort(process.env.PORT || "3000");
+        const fileSystemManager = fsPromises;
         const externalStorageManager = new ExternalStorageManager(config);
+        const repositoryManagerFactory = new NodegitRepositoryManagerFactory(
+            config.get("storageDir"),
+            fileSystemManager,
+            externalStorageManager,
+        );
 
-        return new GitrestResources(config, port, externalStorageManager);
+        return new GitrestResources(config, port, repositoryManagerFactory);
     }
 }
 
@@ -40,6 +48,6 @@ export class GitrestRunnerFactory implements core.IRunnerFactory<GitrestResource
             resources.webServerFactory,
             resources.config,
             resources.port,
-            resources.externalStorageManager);
+            resources.repositoryManagerFactory);
     }
 }

server/gitrest/src/test/routes.spec.ts

@@ -4,6 +4,7 @@
  */
 
 import assert from "assert";
+import fsPromises from "fs/promises";
 import {
     ICreateBlobParams,
     ICreateBlobResponse,
@@ -29,7 +30,6 @@ const commitEmail = "kurtb@microsoft.com";
 const commitName = "Kurt Berglund";
 
 async function createRepo(supertest: request.SuperTest<request.Test>, owner: string, name: string) {
-    console.log("Entered create repo");
     return supertest
         .post(`/${owner}/repos`)
         .set("Accept", "application/json")
@@ -148,13 +148,19 @@ describe("GitRest", () => {
         };
 
         const externalStorageManager = new ExternalStorageManager(testUtils.defaultProvider);
+        const getRepoManagerFactory = () => new NodegitRepositoryManagerFactory(
+            testUtils.defaultProvider.get("storageDir"),
+            fsPromises,
+            externalStorageManager,
+        );
 
         testUtils.initializeBeforeAfterTestHooks(testUtils.defaultProvider);
 
         // Create the git repo before and after each test
         let supertest: request.SuperTest<request.Test>;
         beforeEach(() => {
-            const testApp = app.create(testUtils.defaultProvider, externalStorageManager);
+            const repoManagerFactory = getRepoManagerFactory();
+            const testApp = app.create(testUtils.defaultProvider, repoManagerFactory);
             supertest = request(testApp);
         });
 
@@ -417,10 +423,7 @@ describe("GitRest", () => {
                 const MaxParagraphs = 200;
 
                 await initBaseRepo(supertest, testOwnerName, testRepoName, testBlob, testTree, testCommit, testRef);
-                const repoManagerFactory = new NodegitRepositoryManagerFactory(
-                    testUtils.defaultProvider.get("storageDir"),
-                    externalStorageManager,
-                );
+                const repoManagerFactory = getRepoManagerFactory();
                 const repoManager = await repoManagerFactory.open(testOwnerName, testRepoName);
 
                 let lastCommit;

server/gitrest/src/utils/definitions.ts

@@ -3,6 +3,7 @@
  * Licensed under the MIT License.
  */
 
+import fsPromises from "fs/promises";
 import * as git from "@fluidframework/gitresources";
 
 export interface IExternalWriterConfig {
@@ -28,6 +29,24 @@ export interface IRepositoryManager {
     createTag(tagParams: git.ICreateTagParams): Promise<git.ITag>;
 }
 
+/**
+ * Subset of Node.js `fs/promises` API.
+ */
+export interface IFileSystemManager {
+    readFile: typeof fsPromises.readFile;
+    writeFile: typeof fsPromises.writeFile;
+    unlink: typeof fsPromises.unlink;
+    readdir: typeof fsPromises.readdir;
+    mkdir: typeof fsPromises.mkdir;
+    rmdir: typeof fsPromises.rmdir;
+    stat: typeof fsPromises.stat;
+    lstat: typeof fsPromises.lstat;
+    readlink: typeof fsPromises.readlink;
+    symlink: typeof fsPromises.symlink;
+    chmod: typeof fsPromises.chmod;
+    rm: typeof fsPromises.rm;
+}
+
 export interface IRepositoryManagerFactory {
     /**
      * Create a new repository and return its manager instance.

server/gitrest/src/utils/nodegitManager.ts

@@ -3,9 +3,8 @@
  * Licensed under the MIT License.
  */
 
-import * as fs from "fs";
+import { PathLike } from "fs";
 import * as path from "path";
-import * as util from "util";
 import nodegit from "nodegit";
 import winston from "winston";
 import safeStringify from "json-stringify-safe";
@@ -14,9 +13,22 @@ import { NetworkError } from "@fluidframework/server-services-client";
 import { IExternalStorageManager } from "../externalStorageManager";
 import * as helpers from "./helpers";
 import * as conversions from "./nodegitConversions";
-import { IRepositoryManagerFactory, GitObjectType, IExternalWriterConfig, IRepositoryManager } from "./definitions";
-
-const exists = util.promisify(fs.exists);
+import {
+    IRepositoryManagerFactory,
+    GitObjectType,
+    IExternalWriterConfig,
+    IRepositoryManager,
+    IFileSystemManager,
+} from "./definitions";
+
+const exists = async (fileSystemManager: IFileSystemManager, fileOrDirectoryPath: PathLike): Promise<boolean> => {
+    try {
+        await fileSystemManager.stat(fileOrDirectoryPath);
+        return true;
+    } catch (e) {
+        return false;
+    }
+};
 
 export class NodegitRepositoryManager implements IRepositoryManager {
     constructor(
@@ -325,7 +337,11 @@ export class NodegitRepositoryManagerFactory implements IRepositoryManagerFactor
     // Cache repositories to allow for reuse
     private repositoryPCache: { [key: string]: Promise<nodegit.Repository> } = {};
 
-    constructor(private readonly baseDir, private readonly externalStorageManager: IExternalStorageManager) {
+    constructor(
+        private readonly baseDir,
+        private readonly fileSystemManager: IFileSystemManager,
+        private readonly externalStorageManager: IExternalStorageManager,
+    ) {
     }
 
     public async create(owner: string, name: string): Promise<NodegitRepositoryManager> {
@@ -338,7 +354,11 @@ export class NodegitRepositoryManagerFactory implements IRepositoryManagerFactor
         this.repositoryPCache[repoPath] = repositoryP;
 
         const repository = await this.repositoryPCache[repoPath];
-        const repoManager = new NodegitRepositoryManager(owner, name, repository, this.externalStorageManager);
+        const repoManager = new NodegitRepositoryManager(
+            owner,
+            name,
+            repository,
+            this.externalStorageManager);
         winston.info(`Created a new repo for owner ${owner} reponame: ${name}`);
 
         return repoManager;
@@ -350,7 +370,8 @@ export class NodegitRepositoryManagerFactory implements IRepositoryManagerFactor
         if (!(repoPath in this.repositoryPCache)) {
             const directory = `${this.baseDir}/${repoPath}`;
 
-            if (!await exists(directory)) {
+            const repoExists = await exists(this.fileSystemManager, directory);
+            if (!repoExists) {
                 winston.info(`Repo does not exist ${directory}`);
                 // services-client/getOrCreateRepository depends on a 400 response code
                 throw new NetworkError(400, `Repo does not exist ${directory}`);
@@ -360,7 +381,11 @@ export class NodegitRepositoryManagerFactory implements IRepositoryManagerFactor
         }
 
         const repository = await this.repositoryPCache[repoPath];
-        const repoManager = new NodegitRepositoryManager(owner, name, repository, this.externalStorageManager);
+        const repoManager = new NodegitRepositoryManager(
+            owner,
+            name,
+            repository,
+            this.externalStorageManager);
         return repoManager;
     }