Make caches not to update beyond the hit

Reviewed By: davidaurelio

Differential Revision: D7098171

fbshipit-source-id: 2e934054fd265c40887a9ff5458bf294b2a61132

flow-typed/jest.js

@@ -1,5 +1,8 @@
 /**
- * Copyright 2004-present Facebook. All Rights Reserved.
+ * Copyright (c) 2004-present, Facebook, Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
  *
  * Modified from https://raw.githubusercontent.com/flowtype/flow-typed/e3b0f3034929e0f0fb85c790450a201b380ac2fd/definitions/npm/jest_v17.x.x/flow_v0.33.x-/jest_v17.x.x.js
  * Duplicated from www/flow/shared/jest.js
@@ -123,7 +126,6 @@ declare function spyOn(value: mixed, method: string): Object;
 type Jest = {
   autoMockOff(): Jest,
   autoMockOn(): Jest,
-  resetAllMocks(): Jest,
   clearAllTimers(): void,
   currentTestPath(): void,
   disableAutomock(): Jest,
@@ -134,8 +136,10 @@ type Jest = {
   isMockFunction(fn: Function): boolean,
   genMockFromModule(moduleName: string): any,
   mock(moduleName: string, moduleFactory?: any): Jest,
+  resetAllMocks(): Jest,
   resetModuleRegistry(): Jest, // undocumented alias for resetModuleRegistry
   resetModules(): Jest,
+  restoreAllMocks(): Jest,
   runAllTicks(): Jest,
   runAllTimers(): Jest,
   runTimersToTime(msToRun: number): Jest,

packages/metro-cache/src/Cache.js

@@ -15,7 +15,10 @@ import type {CacheStore} from 'metro-cache';
 class Cache<T> {
   _stores: $ReadOnlyArray<CacheStore<T>>;
 
+  _hits: WeakMap<Buffer, CacheStore<T>>;
+
   constructor(stores: $ReadOnlyArray<CacheStore<T>>) {
+    this._hits = new WeakMap();
     this._stores = stores;
   }
 
@@ -31,6 +34,8 @@ class Cache<T> {
       }
 
       if (value != null) {
+        this._hits.set(key, stores[i]);
+
         return value;
       }
     }
@@ -39,7 +44,16 @@ class Cache<T> {
   }
 
   set(key: Buffer, value: T): void {
-    Promise.all(this._stores.map(store => store.set(key, value))).catch(err => {
+    const stores = this._stores;
+    const stop = this._hits.get(key);
+    const length = stores.length;
+    const promises = [];
+
+    for (let i = 0; i < length && stores[i] !== stop; i++) {
+      promises.push(stores[i].set(key, value));
+    }
+
+    Promise.all(promises).catch(err => {
       process.nextTick(() => {
         throw err;
       });

packages/metro-cache/src/__tests__/Cache.test.js

@@ -6,6 +6,7 @@
  *
  * @emails oncall+javascript_foundation
  * @format
+ * @flow
  */
 
 'use strict';
@@ -15,19 +16,22 @@ const Cache = require('../Cache');
 describe('Cache', () => {
   function createStore(i) {
     return {
-      name: 'store' + i,
       get: jest.fn().mockImplementation(() => null),
       set: jest.fn(),
     };
   }
 
+  afterEach(() => {
+    jest.restoreAllMocks();
+  });
+
   it('returns null when no result is found', async () => {
     const store1 = createStore();
     const store2 = createStore();
     const cache = new Cache([store1, store2]);
 
     // Calling a wrapped method.
-    const result = await cache.get('arg');
+    const result = await cache.get(Buffer.from('foo'));
 
     expect(result).toBe(null);
     expect(store1.get).toHaveBeenCalledTimes(1);
@@ -41,19 +45,35 @@ describe('Cache', () => {
     const cache = new Cache([store1, store2, store3]);
 
     // Only cache 2 can return results.
-    store2.get.mockImplementation(() => 'foo');
+    store2.get.mockImplementation(() => 'hit!');
 
-    const result = await cache.get('arg');
+    const result = await cache.get(Buffer.from('foo'));
 
-    expect(result).toBe('foo');
+    expect(result).toBe('hit!');
     expect(store1.get).toHaveBeenCalledTimes(1);
     expect(store2.get).toHaveBeenCalledTimes(1);
     expect(store3.get).not.toHaveBeenCalled();
   });
 
-  it('awaits for promises on stores, even if they return undefined', async () => {
+  it('skips all cache stores when a hit is produced, based on the same key', () => {
-    jest.useFakeTimers();
+    const store1 = createStore();
+    const store2 = createStore();
+    const store3 = createStore();
+    const cache = new Cache([store1, store2, store3]);
+    const key = Buffer.from('foo');
 
+    store2.get.mockImplementation(() => 'hit!');
+
+    // Get and set. Set should only affect store 1, not 2 (hit) and 3 (after).
+    cache.get(key);
+    cache.set(key);
+
+    expect(store1.set).toHaveBeenCalledTimes(1);
+    expect(store2.set).not.toHaveBeenCalled();
+    expect(store3.set).not.toHaveBeenCalled();
+  });
+
+  it('awaits for promises on stores, even if they return undefined', async () => {
     let resolve;
 
     const store1 = createStore();
@@ -62,55 +82,59 @@ describe('Cache', () => {
     const cache = new Cache([store1, store2]);
 
     store1.get.mockImplementation(() => promise);
-    cache.get('foo');
+    const get = cache.get(Buffer.from('foo'));
 
     // Store 1 returns a promise, so store 2 is not called until it resolves.
     expect(store1.get).toHaveBeenCalledTimes(1);
     expect(store2.get).not.toHaveBeenCalled();
 
-    resolve(undefined);
+    if (!resolve) {
+      throw new Error('Flow needs this');
+    }
 
-    await promise;
+    resolve(undefined);
-    jest.runAllTimers();
+    await Promise.all([promise, get]);
 
     expect(store1.get).toHaveBeenCalledTimes(1);
     expect(store2.get).toHaveBeenCalledTimes(1);
   });
 
-  it('throws on a buggy store', async () => {
+  it('throws on a buggy store set', async () => {
     jest.useFakeTimers();
 
     const store1 = createStore();
     const store2 = createStore();
     const cache = new Cache([store1, store2]);
+    let error = null;
 
-    let err1 = null;
+    store1.set.mockImplementation(() => null);
-    let err2 = null;
+    store2.set.mockImplementation(() => Promise.reject(new RangeError('foo')));
-
-    // Try sets.
-    store1.set.mockImplementation(() => Promise.reject(new RangeError('foo')));
-    store2.set.mockImplementation(() => null);
-
-    expect(() => cache.set('arg')).not.toThrow(); // Async throw.
 
     try {
-      jest.runAllTimers(); // Advancing the timer will make the cache throw.
+      cache.set(Buffer.from('foo'), 'arg');
+      jest.runAllTimers();
     } catch (err) {
-      err1 = err;
+      error = err;
     }
 
-    expect(err1).toBeInstanceOf(RangeError);
+    expect(error).toBeInstanceOf(RangeError);
+  });
 
-    // Try gets.
+  it('throws on a buggy store get', async () => {
-    store1.get.mockImplementation(() => Promise.reject(new TypeError('bar')));
+    const store1 = createStore();
-    store2.get.mockImplementation(() => null);
+    const store2 = createStore();
+    const cache = new Cache([store1, store2]);
+    let error = null;
+
+    store1.get.mockImplementation(() => null);
+    store2.get.mockImplementation(() => Promise.reject(new TypeError('bar')));
 
     try {
-      await cache.get('arg');
+      await cache.get(Buffer.from('foo'));
     } catch (err) {
-      err2 = err;
+      error = err;
     }
 
-    expect(err2).toBeInstanceOf(TypeError);
+    expect(error).toBeInstanceOf(TypeError);
   });
 });