Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Handle when message confirmations arrive out of order. #10405

Merged
merged 1 commit into from
Apr 22, 2020
Merged

Handle when message confirmations arrive out of order. #10405

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 6 additions & 9 deletions src/status_im/events.cljs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -813,15 +813,12 @@
(handlers/register-handler-fx
:transport/message-sent
(fn [cofx [_ response messages-count]]
(let [{:keys [localChatId id messageType]} (-> response :messages first)]
(fx/merge cofx
(handle-update response)
(transport.message/set-message-envelope-hash localChatId id messageType messages-count)))))

(handlers/register-handler-fx
:transport/contact-message-sent
(fn [cofx [_ chat-id envelope-hash]]
(transport.message/set-contact-message-envelope-hash cofx chat-id envelope-hash)))
(let [set-hash-fxs (map (fn [{:keys [localChatId id messageType]}]
(transport.message/set-message-envelope-hash localChatId id messageType messages-count))
(:messages response))]
(apply fx/merge cofx
(conj set-hash-fxs
(handle-update response))))))

(handlers/register-handler-fx
:transport.callback/node-info-fetched
Expand Down
50 changes: 25 additions & 25 deletions src/status_im/transport/message/core.cljs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,37 +95,37 @@
confirmations)}))))

(fx/defn update-envelope-status
[{:keys [db] :as cofx} envelope-hash status]
(let [{:keys [chat-id message-type message-id]}
(get-in db [:transport/message-envelopes envelope-hash])]
(case message-type
:contact-message
(when (= :sent status)
(remove-hash cofx envelope-hash))

(when-let [{:keys [from]} (get-in db [:chats chat-id :messages message-id])]
(check-confirmations cofx status chat-id message-id)))))
[{:keys [db] :as cofx} message-id status]
(if-let [{:keys [chat-id]}
(get-in db [:transport/message-envelopes message-id])]
(when-let [{:keys [from]} (get-in db [:chats chat-id :messages message-id])]
(check-confirmations cofx status chat-id message-id))
;; We don't have a message-envelope for this, might be that the confirmation
;; came too early
{:db (update-in db [:transport/message-confirmations message-id] conj status)}))

(fx/defn update-envelopes-status
[{:keys [db] :as cofx} envelope-hashes status]
(apply fx/merge cofx (map #(update-envelope-status % status) envelope-hashes)))

(fx/defn set-contact-message-envelope-hash
[{:keys [db] :as cofx} chat-id envelope-hash]
{:db (assoc-in db [:transport/message-envelopes envelope-hash]
{:chat-id chat-id
:message-type :contact-message})})
[{:keys [db] :as cofx} message-id status]
(apply fx/merge cofx (map #(update-envelope-status % status) message-id)))

(fx/defn set-message-envelope-hash
"message-type is used for tracking"
[{:keys [db] :as cofx} chat-id message-id message-type messages-count]
{:db (-> db
(assoc-in [:transport/message-envelopes message-id]
{:chat-id chat-id
:message-id message-id
:message-type message-type})
(update-in [:transport/message-ids->confirmations message-id]
#(or % {:pending-confirmations messages-count})))})
;; Check first if the confirmation has already arrived
(let [statuses (get-in db [:transport/message-confirmations message-id])
check-confirmations-fx (map
#(check-confirmations % chat-id message-id)
statuses)

add-envelope-data (fn [{:keys [db]}]
{:db (-> db
(update :transport/message-confirmations dissoc message-id)
(assoc-in [:transport/message-envelopes message-id]
{:chat-id chat-id
:message-type message-type})
(update-in [:transport/message-ids->confirmations message-id]
#(or % {:pending-confirmations messages-count})))})]
(apply fx/merge cofx (conj check-confirmations-fx add-envelope-data))))

(defn- own-info [db]
(let [{:keys [name photo-path address]} (:multiaccount db)]
Expand Down
18 changes: 16 additions & 2 deletions test/cljs/status_im/test/transport/core.cljs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,15 +67,13 @@
(let [cofx (message/set-message-envelope-hash initial-cofx chat-id message-id :message-type 1)]
(testing "it sets the message-infos"
(is (= {:chat-id chat-id
:message-id message-id
:message-type :message-type}
(get-in cofx [:db :transport/message-envelopes message-id]))))
(testing "the message is sent"
(is (= :sent
(get-in
(message/update-envelope-status cofx message-id :sent)
[:db :chats chat-id :messages message-id :outgoing-status]))))

(testing "the message is not sent"
(is (= :not-sent
(get-in
Expand Down Expand Up @@ -107,6 +105,22 @@
(get-in
cofx
[:db :chats chat-id :messages message-id :outgoing-status]))))))
(testing "order of events is reversed"
(let [cofx (fx/merge
initial-cofx
(message/update-envelope-status message-id :sent)
(message/update-envelope-status message-id :sent)
(message/update-envelope-status message-id :sent)
(message/set-message-envelope-hash chat-id message-id :message-type 3))]
(testing "it removes the confirmations"
(is (not (get-in cofx [:db :transport/message-confirmations message-id])))
(is (not (get-in cofx [:db :transport/message-ids->confirmations message-id]))))

(testing "the message is sent"
(is (= :sent
(get-in
cofx
[:db :chats chat-id :messages message-id :outgoing-status]))))))
(testing "message not sent"
(let [cofx (fx/merge
initial-cofx
Expand Down