From 7771f8c4f4c56d69ff99587644085e4ba0606672 Mon Sep 17 00:00:00 2001 From: Ben Gotow Date: Wed, 9 Mar 2016 18:02:21 -0800 Subject: [PATCH] fix(bodies): MBP should cache "" so that it updates when new body arrives --- .../spec/message-item-body-spec.cjsx | 2 +- .../message-list/spec/message-item-spec.cjsx | 2 +- src/flux/stores/message-body-processor.es6 | 44 +++++++++++-------- 3 files changed, 28 insertions(+), 20 deletions(-) diff --git a/internal_packages/message-list/spec/message-item-body-spec.cjsx b/internal_packages/message-list/spec/message-item-body-spec.cjsx index c063af545..9a45fbba2 100644 --- a/internal_packages/message-list/spec/message-item-body-spec.cjsx +++ b/internal_packages/message-list/spec/message-item-body-spec.cjsx @@ -86,7 +86,7 @@ describe "MessageItem", -> return '/fake/path-inline.png' if f.id is file_inline.id return '/fake/path-downloading.png' if f.id is file_inline_downloading.id return null - spyOn(MessageBodyProcessor, 'addToCache').andCallFake -> + spyOn(MessageBodyProcessor, '_addToCache').andCallFake -> @downloads = 'file_1_id': download, diff --git a/internal_packages/message-list/spec/message-item-spec.cjsx b/internal_packages/message-list/spec/message-item-spec.cjsx index 9973e3bf4..2a2771abb 100644 --- a/internal_packages/message-list/spec/message-item-spec.cjsx +++ b/internal_packages/message-list/spec/message-item-spec.cjsx @@ -98,7 +98,7 @@ describe "MessageItem", -> spyOn(FileDownloadStore, 'downloadDataForFiles').andCallFake (ids) -> return {'file_1_id': download, 'file_inline_downloading_id': download_inline} - spyOn(MessageBodyProcessor, 'addToCache').andCallFake -> + spyOn(MessageBodyProcessor, '_addToCache').andCallFake -> @message = new Message id: "111" diff --git a/src/flux/stores/message-body-processor.es6 b/src/flux/stores/message-body-processor.es6 index 074311439..66455bbd4 100644 --- a/src/flux/stores/message-body-processor.es6 +++ b/src/flux/stores/message-body-processor.es6 @@ -24,7 +24,7 @@ class MessageBodyProcessor { this._recentlyProcessedA = []; this._recentlyProcessedD = {}; for (const {message, callback} of this._subscriptions) { - callback(this.process(message)); + callback(this.retrieve(message)); } } @@ -51,7 +51,7 @@ class MessageBodyProcessor { if (subscriptions.length > 0) { const updatedMessage = changedMessage.clone(); updatedMessage.body = updatedMessage.body || subscriptions[0].message.body; - const output = this.process(updatedMessage); + const output = this.retrieve(updatedMessage); for (const subscription of subscriptions) { subscription.callback(output); subscription.message = updatedMessage; @@ -59,18 +59,12 @@ class MessageBodyProcessor { } } - _key(message) { - // It's safe to key off of the message ID alone because we invalidate the - // cache whenever the message is persisted to the database. - return message.id; - } - version() { return this._version; } - processAndSubscribe(message, callback) { - _.defer(() => callback(this.process(message))); + subscribe(message, callback) { + _.defer(() => callback(this.retrieve(message))); const sub = {message, callback} this._subscriptions.push(sub); return () => { @@ -78,16 +72,30 @@ class MessageBodyProcessor { } } - process(message) { - let body = message.body; - if (!_.isString(message.body)) { - return ""; - } - + retrieve(message) { const key = this._key(message); if (this._recentlyProcessedD[key]) { return this._recentlyProcessedD[key].body; } + const body = this._process(message); + this._addToCache(key, body); + return body; + } + + // Private Methods + + _key(message) { + // It's safe to key off of the message ID alone because we invalidate the + // cache whenever the message is persisted to the database. + return message.id; + } + + _process(message) { + let body = message.body; + + if (!_.isString(body)) { + return ""; + } // Give each extension the message object to process the body, but don't // allow them to modify anything but the body for the time being. @@ -132,11 +140,11 @@ class MessageBodyProcessor { result = MessageUtils.cidRegex.exec(body); } - this.addToCache(key, body); + return body; } - addToCache(key, body) { + _addToCache(key, body) { if (this._recentlyProcessedA.length > 50) { const removed = this._recentlyProcessedA.pop() delete this._recentlyProcessedD[removed.key]