From 4ad141d4fbb7cbc871bc9b36bec32403ff8acd69 Mon Sep 17 00:00:00 2001
From: Mark Vayngrib <mark@tradle.io>
Date: Fri, 5 Feb 2016 16:44:44 -0800
Subject: [PATCH] multiGet breaking test and fix

Summary:
the flush + optimized multiGet result in an obscure bug that results when two multiGet requests with overlapping key sets get issued. The result array for both requests ends up bigger than the key array (because it has duplicates)
Closes https://github.com/facebook/react-native/pull/5514

Reviewed By: svcscm

Differential Revision: D2908264

Pulled By: nicklockwood

fb-gh-sync-id: 60be1bce4acfc47083e4ae28bb8b63f9dfa56039
---
 IntegrationTests/AsyncStorageTest.js | 18 ++++++++++++++++++
 Libraries/Storage/AsyncStorage.js    | 18 +++++++++++++-----
 2 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/IntegrationTests/AsyncStorageTest.js b/IntegrationTests/AsyncStorageTest.js
index 3bd313a7fd0..06ea818e451 100644
--- a/IntegrationTests/AsyncStorageTest.js
+++ b/IntegrationTests/AsyncStorageTest.js
@@ -142,12 +142,30 @@ function testMerge() {
         expectAsyncNoError('testMerge/setItem', err3);
         expectEqual(JSON.parse(result), VAL_MERGE_EXPECT, 'testMerge');
         updateMessage('objects deeply merged\nDone!');
+        runTestCase('multi set and get', testOptimizedMultiGet);
+      });
+    });
+  });
+}
+
+function testOptimizedMultiGet() {
+  let batch = [[KEY_1, VAL_1], [KEY_2, VAL_2]];
+  let keys = batch.map(([key, value]) => key);
+  AsyncStorage.multiSet(batch, (err1) => {
+    // yes, twice on purpose
+    ;[1, 2].forEach((i) => {
+      expectAsyncNoError(`${i} testOptimizedMultiGet/multiSet`, err1);
+      AsyncStorage.multiGet(keys, (err2, result) => {
+        expectAsyncNoError(`${i} testOptimizedMultiGet/multiGet`, err2);
+        expectEqual(result, batch, `${i} testOptimizedMultiGet multiGet`);
+        updateMessage('multiGet([key_1, key_2]) correctly returned ' + JSON.stringify(result));
         done();
       });
     });
   });
 }
 
+
 var AsyncStorageTest = React.createClass({
   getInitialState() {
     return {
diff --git a/Libraries/Storage/AsyncStorage.js b/Libraries/Storage/AsyncStorage.js
index eb41c8a736f..f1ed35c3c1c 100644
--- a/Libraries/Storage/AsyncStorage.js
+++ b/Libraries/Storage/AsyncStorage.js
@@ -180,14 +180,16 @@ var AsyncStorage = {
       // Even though the runtime complexity of this is theoretically worse vs if we used a map,
       // it's much, much faster in practice for the data sets we deal with (we avoid
       // allocating result pair arrays). This was heavily benchmarked.
+      //
+      // Is there a way to avoid using the map but fix the bug in this breaking test?
+      // https://github.com/facebook/react-native/commit/8dd8ad76579d7feef34c014d387bf02065692264
+      let map = {};
+      result.forEach(([key, value]) => map[key] = value);
       const reqLength = getRequests.length;
       for (let i = 0; i < reqLength; i++) {
         const request = getRequests[i];
         const requestKeys = request.keys;
-        var requestResult = result.filter(function(resultPair) {
-          return requestKeys.indexOf(resultPair[0]) !== -1;
-        });
-
+        let requestResult = requestKeys.map(key => [key, map[key]]);
         request.callback && request.callback(null, requestResult);
         request.resolve && request.resolve(requestResult);
       }
@@ -214,6 +216,7 @@ var AsyncStorage = {
     var getRequest = {
       keys: keys,
       callback: callback,
+      // do we need this?
       keyIndex: this._getKeys.length,
       resolve: null,
       reject: null,
@@ -225,7 +228,12 @@ var AsyncStorage = {
     });
 
     this._getRequests.push(getRequest);
-    this._getKeys.push.apply(this._getKeys, keys);
+    // avoid fetching duplicates
+    keys.forEach(key => {
+      if (this._getKeys.indexOf(key) === -1) {
+        this._getKeys.push(key);
+      }
+    });
 
     return promiseResult;
   },