From 6237146152392757e0c626b081b1547623d4e24e Mon Sep 17 00:00:00 2001
From: Andrej Burcev <andrej.burcev@mailonline.co.uk>
Date: Wed, 26 Sep 2018 15:51:16 +0100
Subject: [PATCH] perf: add conversations hash by userId

in order to access a required conversation instance in a more performant way

Closes: https://github.com/Charca/bootbot/issues/155
---
 lib/BootBot.js       | 41 +++++++++++++++++++++++++-----------
 test/BootBot.spec.js | 49 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 78 insertions(+), 12 deletions(-)

diff --git a/lib/BootBot.js b/lib/BootBot.js
index 1b60321..8ab641b 100644
--- a/lib/BootBot.js
+++ b/lib/BootBot.js
@@ -34,6 +34,7 @@ class BootBot extends EventEmitter {
     this.app.use(bodyParser.json({ verify: this._verifyRequestSignature.bind(this) }));
     this._hearMap = [];
     this._conversations = [];
+    this._conversationsByUserIdHash = {};
   }
 
   /**
@@ -473,13 +474,27 @@ class BootBot extends EventEmitter {
       return console.error(`You need to specify a recipient and a callback to start a conversation`);
     }
     const convo = new Conversation(this, recipientId);
+
+    this._addConversation(convo);
+
+    factory.apply(this, [convo]);
+    return convo;
+  }
+
+  /**
+   * Used to add Conversation instance with maintaining appropriate hash by userId.
+   * @param {Conversation} conversation will be added to the instance's conversations list.
+   */
+  _addConversation(convo) {
+    const userId = convo.userId;
+
+    this._conversationsByUserIdHash[userId] = convo;
     this._conversations.push(convo);
+
     convo.on('end', (endedConvo) => {
-      const removeIndex = this._conversations.indexOf(endedConvo);
-      this._conversations.splice(removeIndex, 1);
+      this._conversations.splice(this._conversations.indexOf(endedConvo), 1);
+      delete this._conversationsByUserIdHash[endedConvo.userId];
     });
-    factory.apply(this, [convo]);
-    return convo;
   }
 
   /**
@@ -588,14 +603,16 @@ class BootBot extends EventEmitter {
 
   _handleConversationResponse(type, event) {
     const userId = event.sender.id;
-    let captured = false;
-    this._conversations.forEach(convo => {
-      if (userId && userId === convo.userId && convo.isActive()) {
-        captured = true;
-        return convo.respond(event, { type });
-      }
-    });
-    return captured;
+
+    const convo = this._conversationsByUserIdHash[userId];
+
+    if (!convo || !convo.isActive()) {
+      return false;
+    }
+
+    convo.respond(event, { type });
+
+    return true;
   }
 
   /**
diff --git a/test/BootBot.spec.js b/test/BootBot.spec.js
index 417753f..33217f1 100644
--- a/test/BootBot.spec.js
+++ b/test/BootBot.spec.js
@@ -1,6 +1,7 @@
 'use strict';
 const expect = require('chai').expect;
 const sinon = require('sinon');
+const EventEmitter = require('events');
 const BootBot = require('../lib/BootBot');
 
 describe('BootBot', () => {
@@ -409,6 +410,54 @@ describe('BootBot', () => {
     expect(callback.calledWith(event)).to.equal(true);
   });
 
+  describe('bot._addConversation() tests', () => {
+    const createMockConvo = (userId) => {
+      const convo = new EventEmitter();
+
+      convo.userId = userId;
+
+      return convo;
+    };
+    let mockConvo;
+
+    beforeEach(() => {
+      mockConvo = createMockConvo(123);
+    });
+
+    it('should add conversation to _conversations list', () => {
+      expect(bot._conversations.length).to.equal(0);
+      bot._addConversation(mockConvo);
+      expect(bot._conversations).to.deep.equal([mockConvo]);
+    });
+
+    it('should add conversation to conversations by userId hash', () => {
+      expect(bot._conversationsByUserIdHash[mockConvo.userId]).to.equal(undefined);
+      bot._addConversation(mockConvo);
+      expect(bot._conversationsByUserIdHash[mockConvo.userId]).to.equal(mockConvo);
+    });
+
+    it('should clean up after conversation is ended', () => {
+      const convos = [createMockConvo(1), createMockConvo(2), createMockConvo(3)];
+
+      convos.forEach((convo) => bot._addConversation(convo));
+
+      expect(bot._conversationsByUserIdHash).to.deep.equal({1: convos[0], 2: convos[1], 3: convos[2]});
+      expect(bot._conversations).to.deep.equal(convos);
+
+      convos[1].emit('end', convos[1]);
+      expect(bot._conversationsByUserIdHash).to.deep.equal({1: convos[0], 3: convos[2]});
+      expect(bot._conversations).to.deep.equal([convos[0], convos[2]]);
+
+      convos[2].emit('end', convos[2]);
+      expect(bot._conversationsByUserIdHash).to.deep.equal({1: convos[0]});
+      expect(bot._conversations).to.deep.equal([convos[0]]);
+
+      convos[0].emit('end', convos[0]);
+      expect(bot._conversationsByUserIdHash).to.deep.equal({});
+      expect(bot._conversations).to.deep.equal([]);
+    });
+  });
+
   describe('bot.say() tests', () => {
     it('should call sendTextMessage when calling .say() with a string', () => {
       const spy = sinon.spy(bot, 'sendTextMessage');