From 608c64e88d32f44231cbe7ce8ccde950b89ffd68 Mon Sep 17 00:00:00 2001 From: Andrea Maria Piana Date: Wed, 6 Jun 2018 11:32:07 +0200 Subject: [PATCH] Dont run receive-whisper-messages if the user is logged out This handles a bug whereby we'd run receive-whisper-messages when the user is logged out. I could not replicate locally, but a few issues are apparent from just inspecting the code: 1) there are some race-conditions on logout as we don't wait for all the filters to be removed. Changing this behaviour is non trivial and not sure if we can actually handle this completely (status-go-has-a-message->remove-filter->logout->status-go-deliver-message). 2) no error handling is made in receive-whisper-messages. This PR defensively handles both cases. Signed-off-by: Andrea Maria Piana --- src/status_im/models/account.cljs | 5 +++++ src/status_im/transport/filters.cljs | 5 +++-- src/status_im/transport/handlers.cljs | 19 ++++++++++------ .../ui/screens/network_settings/events.cljs | 19 ++-------------- src/status_im/utils/handlers.cljs | 10 +++++++++ test/cljs/status_im/test/models/account.cljs | 9 ++++++++ test/cljs/status_im/test/runner.cljs | 8 +++++-- .../status_im/test/transport/handlers.cljs | 22 +++++++++++++++++++ 8 files changed, 69 insertions(+), 28 deletions(-) create mode 100644 src/status_im/models/account.cljs create mode 100644 test/cljs/status_im/test/models/account.cljs create mode 100644 test/cljs/status_im/test/transport/handlers.cljs diff --git a/src/status_im/models/account.cljs b/src/status_im/models/account.cljs new file mode 100644 index 0000000000..df3c3b1db4 --- /dev/null +++ b/src/status_im/models/account.cljs @@ -0,0 +1,5 @@ +(ns status-im.models.account) + +(defn logged-in? [cofx] + (boolean + (get-in cofx [:db :account/account]))) diff --git a/src/status_im/transport/filters.cljs b/src/status_im/transport/filters.cljs index ada813a699..c674f456c7 100644 --- a/src/status_im/transport/filters.cljs +++ b/src/status_im/transport/filters.cljs @@ -9,8 +9,9 @@ (defn remove-filter! [filter] (.stopWatching filter (fn [error _] - (when error - (log/warn :remove-filter-error filter error)))) + (if error + (log/warn :remove-filter-error filter error) + (log/debug :removed-filter filter)))) (log/debug :stop-watching filter)) (defn add-shh-filter! diff --git a/src/status_im/transport/handlers.cljs b/src/status_im/transport/handlers.cljs index e77807d70e..3d6a686815 100644 --- a/src/status_im/transport/handlers.cljs +++ b/src/status_im/transport/handlers.cljs @@ -39,16 +39,21 @@ (aget array i))) (defn receive-whisper-messages [{:keys [now] :as cofx} [js-error js-messages chat-id]] - (let [now-in-s (quot now 1000)] - (handlers-macro/merge-effects - cofx - (fn [message temp-cofx] - (receive-message temp-cofx now-in-s chat-id message)) - (js-array->seq js-messages)))) + (if (and (not js-error) + js-messages) + (let [now-in-s (quot now 1000)] + (handlers-macro/merge-effects + cofx + (fn [message temp-cofx] + (receive-message temp-cofx now-in-s chat-id message)) + (js-array->seq js-messages))) + (log/error "Something went wrong" js-error js-messages))) (handlers/register-handler-fx :protocol/receive-whisper-message - [re-frame/trim-v (re-frame/inject-cofx :random-id)] + [re-frame/trim-v + handlers/logged-in + (re-frame/inject-cofx :random-id)] receive-whisper-messages) (handlers/register-handler-fx diff --git a/src/status_im/ui/screens/network_settings/events.cljs b/src/status_im/ui/screens/network_settings/events.cljs index 7af6a38039..a334bee49e 100644 --- a/src/status_im/ui/screens/network_settings/events.cljs +++ b/src/status_im/ui/screens/network_settings/events.cljs @@ -13,19 +13,6 @@ ;; handlers -(handlers/register-handler-fx - :add-networks - (fn [{{:networks/keys [networks] :as db} :db} [_ new-networks]] - (let [identities (set (keys networks)) - new-networks' (->> new-networks - (remove #(identities (:id %))) - (map #(vector (:id %) %)) - (into {}))] - {:db (-> db - (update :networks merge new-networks') - (assoc :new-networks (vals new-networks'))) - :save-networks new-networks'}))) - (handlers/register-handler-fx ::close-application (fn [_ _] @@ -55,11 +42,9 @@ chats (:transport/chats db)] (if (utils/network-with-upstream-rpc? current-network) (handlers-macro/merge-fx cofx - {:dispatch-n [[:load-accounts] - [:navigate-to-clean :accounts]]} - (transport/stop-whisper) (accounts.utils/account-update {:network network - :last-updated now})) + :last-updated now} + [:logout])) {:show-confirmation {:title (i18n/label :t/close-app-title) :content (i18n/label :t/close-app-content) :confirm-button-text (i18n/label :t/close-app-button) diff --git a/src/status_im/utils/handlers.cljs b/src/status_im/utils/handlers.cljs index b653fd3158..ceff08c02c 100644 --- a/src/status_im/utils/handlers.cljs +++ b/src/status_im/utils/handlers.cljs @@ -5,6 +5,7 @@ [re-frame.interceptor :refer [->interceptor get-coeffect get-effect]] [status-im.utils.instabug :as instabug] [status-im.utils.mixpanel :as mixpanel] + [status-im.models.account :as models.account] [cljs.core.async :as async] [taoensso.timbre :as log])) @@ -37,6 +38,15 @@ (log/debug "Handling re-frame event: " (pretty-print-event context)) context))) +(def logged-in + "Interceptor which stops the event chain if the user is logged out" + (->interceptor + :id :logged-in + :before (fn logged-in-before + [context] + (when (models.account/logged-in? (:coeffects context)) + context)))) + (defn- check-spec-msg-path-problem [problem] (str "Spec: " (-> problem :via last) "\n" "Predicate: " (subs (str (:pred problem)) 0 50))) diff --git a/test/cljs/status_im/test/models/account.cljs b/test/cljs/status_im/test/models/account.cljs new file mode 100644 index 0000000000..e0d6505381 --- /dev/null +++ b/test/cljs/status_im/test/models/account.cljs @@ -0,0 +1,9 @@ +(ns status-im.test.models.account + (:require [cljs.test :refer-macros [deftest is testing]] + [status-im.models.account :as model])) + +(deftest logged-in-test + (testing "account/account is defined" + (is (model/logged-in? {:db {:account/account {}}}))) + (testing "account/account is not there" + (is (not (model/logged-in? {:db {}}))))) diff --git a/test/cljs/status_im/test/runner.cljs b/test/cljs/status_im/test/runner.cljs index 9e1c9ab4e8..f50fdc49a0 100644 --- a/test/cljs/status_im/test/runner.cljs +++ b/test/cljs/status_im/test/runner.cljs @@ -12,7 +12,10 @@ [status-im.test.bots.events] [status-im.test.models.mailserver] [status-im.test.models.bootnode] + [status-im.test.models.account] [status-im.test.transport.core] + [status-im.test.transport.inbox] + [status-im.test.transport.handlers] [status-im.test.chat.models] [status-im.test.chat.models.input] [status-im.test.chat.models.message] @@ -20,7 +23,6 @@ [status-im.test.chat.views.message] [status-im.test.chat.views.photos] [status-im.test.i18n] - [status-im.test.transport.inbox] [status-im.test.protocol.web3.inbox] [status-im.test.utils.utils] [status-im.test.utils.money] @@ -59,8 +61,8 @@ 'status-im.test.data-store.realm.core 'status-im.test.models.mailserver 'status-im.test.models.bootnode + 'status-im.test.models.account 'status-im.test.bots.events - 'status-im.test.transport.core 'status-im.test.wallet.subs 'status-im.test.wallet.transactions.subs 'status-im.test.wallet.transactions.views @@ -69,7 +71,9 @@ 'status-im.test.chat.views.message 'status-im.test.chat.views.photos 'status-im.test.i18n + 'status-im.test.transport.core 'status-im.test.transport.inbox + 'status-im.test.transport.handlers 'status-im.test.protocol.web3.inbox 'status-im.test.utils.utils 'status-im.test.utils.handlers-macro diff --git a/test/cljs/status_im/test/transport/handlers.cljs b/test/cljs/status_im/test/transport/handlers.cljs new file mode 100644 index 0000000000..42dd97b963 --- /dev/null +++ b/test/cljs/status_im/test/transport/handlers.cljs @@ -0,0 +1,22 @@ +(ns status-im.test.transport.handlers + (:require [cljs.test :refer-macros [deftest is testing]] + [status-im.transport.handlers :as handlers])) + +(def messages #js [{:sig "0x04325367620ae20dd878dbb39f69f02c567d789dd21af8a88623dc5b529827c2812571c380a2cd8236a2851b8843d6486481166c39debf60a5d30b9099c66213e4" + :ttl 10 + :timestamp 1527692015 + :topic "0x9c22ff5f" + :payload "0x5b227e236334222c5b2246222c22746578742f706c61696e222c227e3a7075626c69632d67726f75702d757365722d6d657373616765222c3135323736393230313433383130312c313532373639323031343337375d5d" + :padding "0xbf06347cc7f9aa18b4a846032264a88f559d9b14079975d14b10648847c0543a77a80624e101c082d19b502ae3b4f97958d18abf59eb0a82afc1301aa22470495fac739a30c2f563599fa8d8e09363a43d39311596b7f119dee7b046989c08224f1ef5cdc385" + :pow 0.002631578947368421 + :hash "0x220ef9994a4fae64c112b27ed07ef910918159cbe6fcf8ac515ee2bf9a6711a0"}]) + +(deftest receive-whisper-messages-test + (testing "an error is reported" + (is (nil? (handlers/receive-whisper-messages {} ["error" #js [] nil])))) + (testing "messages is undefined" + (is (nil? (handlers/receive-whisper-messages {} [nil js/undefined nil])))) + (testing "happy path" + (let [actual (handlers/receive-whisper-messages {} [nil messages "1"])] + (testing "it add an fx for the message" + (is (:chat-received-message/add-fx actual))))))