fix(persister): Handle concurrent find requests (#88)

Author: offirgolan

packages/@pollyjs/persister/src/index.js

@@ -8,8 +8,8 @@ const CREATOR_NAME = 'Polly.JS';
 export default class Persister {
   constructor(polly) {
     this.polly = polly;
-    this.cache = new Map();
     this.pending = new Map();
+    this._cache = new Map();
   }
 
   static get type() {
@@ -116,31 +116,40 @@ export default class Persister {
   }
 
   async find(recordingId) {
-    if (this.cache.has(recordingId)) {
-      return this.cache.get(recordingId);
-    }
+    const { _cache: cache } = this;
+
+    if (!cache.has(recordingId)) {
+      const findRecording = async () => {
+        const recording = await this.findRecording(recordingId);
+
+        if (recording) {
+          this.assert(
+            `Recording with id '${recordingId}' is invalid. Please delete the recording so a new one can be created.`,
+            recording.log && recording.log.creator.name === CREATOR_NAME
+          );
+
+          return recording;
+        } else {
+          cache.delete(recordingId);
 
-    const recording = await this.findRecording(recordingId);
+          return null;
+        }
+      };
 
-    if (recording) {
-      this.assert(
-        `Recording with id '${recordingId}' is invalid. Please delete the recording so a new one can be created.`,
-        recording.log && recording.log.creator.name === CREATOR_NAME
-      );
-      this.cache.set(recordingId, recording);
+      cache.set(recordingId, findRecording());
     }
 
-    return recording;
+    return cache.get(recordingId);
   }
 
   async save(recordingId) {
     await this.saveRecording(...arguments);
-    this.cache.delete(recordingId);
+    this._cache.delete(recordingId);
   }
 
   async delete(recordingId) {
     await this.deleteRecording(...arguments);
-    this.cache.delete(recordingId);
+    this._cache.delete(recordingId);
   }
 
   async findEntry(pollyRequest) {

packages/@pollyjs/persister/tests/unit/persister-test.js

@@ -1,4 +1,5 @@
 import Persister from '../../src';
+import { timeout } from '@pollyjs/utils';
 
 describe('Unit | Persister', function() {
   it('should exist', function() {
@@ -19,24 +20,37 @@ describe('Unit | Persister', function() {
       };
 
       class CustomPersister extends Persister {
-        findRecording() {
+        async findRecording() {
           callCounts.find++;
+          await timeout(1);
 
           return recording;
         }
 
-        saveRecording() {
+        async saveRecording() {
           callCounts.save++;
+          await timeout(1);
         }
 
-        deleteRecording() {
+        async deleteRecording() {
           callCounts.delete++;
+          await timeout(1);
         }
       }
 
       this.persister = new CustomPersister({});
     });
 
+    it('should handle concurrent find requests', async function() {
+      await Promise.all([
+        this.persister.find('test'),
+        this.persister.find('test'),
+        this.persister.find('test')
+      ]);
+
+      expect(callCounts.find).to.equal(1);
+    });
+
     it('caches', async function() {
       await this.persister.find('test');
       await this.persister.find('test');
@@ -49,7 +63,11 @@ describe('Unit | Persister', function() {
       recording = null;
 
       await this.persister.find('test');
-      await this.persister.find('test');
+      await Promise.all([
+        this.persister.find('test'),
+        this.persister.find('test'),
+        this.persister.find('test')
+      ]);
       await this.persister.find('test');
 
       expect(callCounts.find).to.equal(3);