fix(MongoBinary): improve code

- add tsdoc to many functions
- getCachePath: remove being async
- getDownloadPath: change the times to be more understandable
- getDownloadPath: use "this.getCachePath" instead of "this.cache[]"
- getPath: remove unnecessary variables
- getPath: add an Error if "binaryPath" is still false-y
- improve debug logging

Author: hasezoey

packages/mongodb-memory-server-core/src/util/MongoBinary.ts

@@ -32,54 +32,65 @@ export interface MongoBinaryOpts {
 export default class MongoBinary {
   static cache: MongoBinaryCache = {};
 
+  /**
+   * Probe if the provided "systemBinary" is an existing path
+   * @param systemBinary The Path to probe for an System-Binary
+   * @return System Binary path or empty string
+   */
   static async getSystemPath(systemBinary: string): Promise<string> {
     let binaryPath = '';
 
     try {
       await promisify(fs.access)(systemBinary);
 
-      log(`MongoBinary: found sytem binary path at ${systemBinary}`);
+      log(`MongoBinary: found sytem binary path at "${systemBinary}"`);
       binaryPath = systemBinary;
     } catch (err) {
-      log(`MongoBinary: can't find system binary at ${systemBinary}. ${err.message}`);
+      log(`MongoBinary: can't find system binary at "${systemBinary}".\n${err.message}`);
     }
 
     return binaryPath;
   }
 
-  static async getCachePath(version: string): Promise<string> {
+  /**
+   * Check if specified version already exists in the cache
+   * @param version The Version to check for
+   */
+  static getCachePath(version: string): string {
     return this.cache[version];
   }
 
+  /**
+   * Probe download path and download the binary
+   * @param options Options Configuring which binary to download and to which path
+   * @returns The BinaryPath the binary has been downloaded to
+   */
   static async getDownloadPath(options: Required<MongoBinaryOpts>): Promise<string> {
     const { downloadDir, platform, arch, version, checkMD5 } = options;
-    // create downloadDir if not exists
+    // create downloadDir
     await mkdirp(downloadDir);
 
+    /** Lockfile path */
     const lockfile = path.resolve(downloadDir, `${version}.lock`);
-    // wait lock
+    // wait to get a lock
     await new Promise((resolve, reject) => {
       LockFile.lock(
         lockfile,
         {
-          wait: 120000,
+          wait: 1000 * 12, // 120 seconds
           pollPeriod: 100,
-          stale: 110000,
+          stale: 1000 * 11, // 110 seconds
           retries: 3,
           retryWait: 100,
         },
         (err: any) => {
-          if (err) {
-            reject(err);
-          } else {
-            resolve();
-          }
+          return err ? reject(err) : resolve();
         }
       );
     });
 
-    // again check cache, maybe other instance resolve it
-    if (!this.cache[version]) {
+    // check cache if it got already added to the cache
+    if (!this.getCachePath(version)) {
       const downloader = new MongoBinaryDownload({
         downloadDir,
         platform,
@@ -90,16 +101,22 @@ export default class MongoBinary {
       this.cache[version] = await downloader.getMongodPath();
     }
     // remove lock
-    LockFile.unlock(lockfile, (err: any) => {
+    LockFile.unlock(lockfile, (err) => {
       log(
         err
           ? `MongoBinary: Error when removing download lock ${err}`
           : `MongoBinary: Download lock removed`
       );
     });
-    return this.cache[version];
+    return this.getCachePath(version);
   }
 
+  /**
+   * Probe all supported paths for an binary and return the binary path
+   * @param opts Options configuring which binary to search for
+   * @throws {Error} if no valid BinaryPath has been found
+   * @return The first found BinaryPath
+   */
   static async getPath(opts: MongoBinaryOpts = {}): Promise<string> {
     const legacyDLDir = path.resolve(os.homedir(), '.cache/mongodb-binaries');
 
@@ -109,6 +126,7 @@ export default class MongoBinary {
       nodeModulesDLDir = path.resolve(nodeModulesDLDir, '..', '..');
     }
 
+    // "||" is still used here, because it should default if the value is false-y (like an empty string)
     const defaultOptions = {
       downloadDir:
         resolveConfig('DOWNLOAD_DIR') ||
@@ -128,17 +146,16 @@ export default class MongoBinary {
       checkMD5: envToBool(resolveConfig('MD5_CHECK') ?? ''),
     };
 
+    /** Provided Options combined with the Default Options */
     const options = { ...defaultOptions, ...opts };
-    log(`MongoBinary options: ${JSON.stringify(options)}`);
-
-    const { version, systemBinary } = options;
+    log(`MongoBinary options:`, JSON.stringify(options, null, 2));
 
     let binaryPath = '';
 
-    if (systemBinary) {
-      binaryPath = await this.getSystemPath(systemBinary);
+    if (options.systemBinary) {
+      binaryPath = await this.getSystemPath(options.systemBinary);
       if (binaryPath) {
-        if (~binaryPath.indexOf(' ')) {
+        if (binaryPath.indexOf(' ') >= 0) {
           binaryPath = `"${binaryPath}"`;
         }
 
@@ -147,30 +164,40 @@ export default class MongoBinary {
           .split('\n')[0]
           .split(' ')[2];
 
-        if (version !== LATEST_VERSION && version !== binaryVersion) {
+        if (options.version !== LATEST_VERSION && options.version !== binaryVersion) {
           // we will log the version number of the system binary and the version requested so the user can see the difference
           log(
             'MongoMemoryServer: Possible version conflict\n' +
               `  SystemBinary version: ${binaryVersion}\n` +
-              `  Requested version:    ${version}\n\n` +
+              `  Requested version:    ${options.version}\n\n` +
               '  Using SystemBinary!'
           );
         }
       }
     }
 
     if (!binaryPath) {
-      binaryPath = await this.getCachePath(version);
+      binaryPath = this.getCachePath(options.version);
     }
 
     if (!binaryPath) {
       binaryPath = await this.getDownloadPath(options);
     }
 
-    log(`MongoBinary: Mongod binary path: ${binaryPath}`);
+    if (!binaryPath) {
+      throw new Error(
+        `MongoBinary.getPath: could not find an valid binary path! (Got: "${binaryPath}")`
+      );
+    }
+
+    log(`MongoBinary: Mongod binary path: "${binaryPath}"`);
     return binaryPath;
   }
 
+  /**
+   * Purpose unkown, not used anywhere
+   * @param files Array of Strings
+   */
   static hasValidBinPath(files: string[]): boolean {
     if (files.length === 1) {
       return true;

packages/mongodb-memory-server-core/src/util/__tests__/MongoBinary-test.ts

@@ -45,7 +45,6 @@ describe('MongoBinary', () => {
 
   describe('getDownloadPath', () => {
     it('should download binary and keep it in cache', async () => {
-      // download
       const version = LATEST_VERSION;
       const binPath = await MongoBinary.getPath({
         downloadDir: tmpDir.name,
@@ -71,14 +70,14 @@ describe('MongoBinary', () => {
   describe('getCachePath', () => {
     it('should get the cache', async () => {
       MongoBinary.cache['3.4.2'] = '/bin/mongod';
-      await expect(MongoBinary.getCachePath('3.4.2')).resolves.toEqual('/bin/mongod');
+      expect(MongoBinary.getCachePath('3.4.2')).toEqual('/bin/mongod');
     });
   });
 
   describe('getSystemPath', () => {
     it('should use system binary if option is passed.', async () => {
       const accessSpy = jest.spyOn(fs, 'access');
-      await MongoBinary.getSystemPath('/usr/bin/mongod');
+      await MongoBinary.getSystemPath('/usr/bin/mongod'); // ignoring return, because this depends on the host system
 
       expect(accessSpy).toHaveBeenCalledWith('/usr/bin/mongod', expect.any(Function));