fix(MongoBinary): Check again cache after obtaining lock

nodkz · Jun 7, 2017 · 97e1b6c · 97e1b6c
1 parent e46ab53
commit 97e1b6c
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 10 deletions.
diff --git a/src/util/MongoBinary.js b/src/util/MongoBinary.js
@@ -8,7 +8,7 @@ import lockFile from 'proper-lockfile';
 import mkdirp from 'mkdirp';
 
 export type MongoBinaryCache = {
-  [version: string]: Promise<string>,
+  [version: string]: string,
 };
 
 export type MongoBinaryOpts = {
@@ -52,7 +52,7 @@ export default class MongoBinary {
           else resolve();
         });
       });
-      this.cache[version] = new Promise((resolve, reject) => {
+      await new Promise((resolve, reject) => {
         lockFile.lock(
           downloadDir,
           {
@@ -63,6 +63,13 @@ export default class MongoBinary {
           (err, releaseLock) => {
             debug('MongoBinary: Download lock created');
 
+            // cache may be populated by previous process
+            // check again
+            if (this.cache[version]) {
+              debug(`MongoBinary: found cached binary path for ${version}`);
+              resolve(this.cache[version]);
+            }
+
             if (err) {
               reject(err);
               return;
@@ -91,7 +98,8 @@ export default class MongoBinary {
                 return this.findBinPath(releaseDir);
               })
               .then(binPath => {
-                resolve(binPath);
+                this.cache[version] = binPath;
+                resolve();
               })
               .catch(e => {
                 debug(`MongoBinary: Error with mongod binary path: ${e}`);
@@ -102,10 +110,8 @@ export default class MongoBinary {
       });
     }
 
-    return this.cache[version].then(binPath => {
-      debug(`MongoBinary: Mongod binary path: ${binPath}`);
-      return binPath;
-    });
+    debug(`MongoBinary: Mongod binary path: ${this.cache[version]}`);
+    return this.cache[version];
   }
 
   static findBinPath(releaseDir: string): Promise<string> {

diff --git a/src/util/__tests__/MongoBinary-test.js b/src/util/__tests__/MongoBinary-test.js
@@ -21,15 +21,19 @@ describe('MongoBinary', () => {
 
     // reuse cache
     expect(MongoBinary.cache[version]).toBeDefined();
-    expect(MongoBinary.cache[version]).toBeInstanceOf(Promise);
-    await expect(MongoBinary.cache.latest).resolves.toEqual(binPath);
+    expect(MongoBinary.cache[version]).toEqual(binPath);
+    const binPathAgain = await MongoBinary.getPath({
+      downloadDir: tmpDir.name,
+      version,
+    });
+    expect(binPathAgain).toEqual(binPath);
 
     // cleanup
     tmpDir.removeCallback();
   });
 
   it('should use cache', async () => {
-    MongoBinary.cache['3.4.2'] = Promise.resolve('/bin/mongod');
+    MongoBinary.cache['3.4.2'] = '/bin/mongod';
     await expect(MongoBinary.getPath({ version: '3.4.2' })).resolves.toEqual('/bin/mongod');
   });
 });