From 03ed11b92ccb71f6b52a5bd27188ec63ea7af485 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Blin?=
<sebastien.blin@savoirfairelinux.com>
Date: Mon, 8 Aug 2022 10:26:33 -0400
Subject: [PATCH] messageengine: allow user to replace a sending message
As we only need one git notification, we can just refresh the
last sending token to avoid to populate the cache with lot of
notifications that will be unused.
Change-Id: Icb8252c8a4b4068e0bd8475a8cc70d77224858da
---
src/account.h | 3 ++-
src/im/message_engine.cpp | 27 ++++++++++++++++++--------
src/im/message_engine.h | 3 ++-
src/jamidht/conversation_module.cpp | 19 +++++++++++-------
src/jamidht/conversation_module.h | 3 ++-
src/jamidht/jamiaccount.cpp | 27 +++++++++++++-------------
src/jamidht/jamiaccount.h | 13 +++++++------
src/sip/sipaccount.cpp | 10 +++++-----
src/sip/sipaccount.h | 10 +++++-----
src/sip/sipaccountbase.h | 30 +++++++++++++++++++----------
10 files changed, 88 insertions(+), 57 deletions(-)
diff --git a/src/account.h b/src/account.h
index 95df7ecab3..6911100af9 100644
--- a/src/account.h
+++ b/src/account.h
@@ -163,7 +163,8 @@ public:
* @return a token to query the message status
*/
virtual uint64_t sendTextMessage(const std::string& /*to*/,
- const std::map<std::string, std::string>& /*payloads*/)
+ const std::map<std::string, std::string>& /*payloads*/,
+ uint64_t /*refreshToken*/ = 0)
{
return 0;
}
diff --git a/src/im/message_engine.cpp b/src/im/message_engine.cpp
index 2b7bf384e6..a7340fe878 100644
--- a/src/im/message_engine.cpp
+++ b/src/im/message_engine.cpp
@@ -43,7 +43,9 @@ MessageEngine::MessageEngine(SIPAccountBase& acc, const std::string& path)
}
MessageToken
-MessageEngine::sendMessage(const std::string& to, const std::map<std::string, std::string>& payloads)
+MessageEngine::sendMessage(const std::string& to,
+ const std::map<std::string, std::string>& payloads,
+ uint64_t refreshToken)
{
if (payloads.empty() or to.empty())
return 0;
@@ -51,12 +53,21 @@ MessageEngine::sendMessage(const std::string& to, const std::map<std::string, st
{
std::lock_guard<std::mutex> lock(messagesMutex_);
auto& peerMessages = messages_[to];
- do {
- token = std::uniform_int_distribution<MessageToken> {1, JAMI_ID_MAX_VAL}(account_.rand);
- } while (peerMessages.find(token) != peerMessages.end());
- auto m = peerMessages.emplace(token, Message {});
- m.first->second.to = to;
- m.first->second.payloads = payloads;
+ auto previousIt = peerMessages.find(refreshToken);
+ if (previousIt != peerMessages.end() && previousIt->second.status != MessageStatus::SENT) {
+ JAMI_DBG("[message %ld] Replace content", refreshToken);
+ token = refreshToken;
+ previousIt->second.to = to;
+ previousIt->second.payloads = payloads;
+ } else {
+ do {
+ token = std::uniform_int_distribution<MessageToken> {1, JAMI_ID_MAX_VAL}(
+ account_.rand);
+ } while (peerMessages.find(token) != peerMessages.end());
+ auto m = peerMessages.emplace(token, Message {});
+ m.first->second.to = to;
+ m.first->second.payloads = payloads;
+ }
save_();
}
runOnMainThread([this, to]() { retrySend(to); });
@@ -108,7 +119,7 @@ MessageEngine::retrySend(const std::string& peer, bool retryOnTimeout)
p.to,
std::to_string(p.token),
(int) DRing::Account::MessageStates::SENDING);
- account_.sendTextMessage(p.to, p.payloads, p.token, retryOnTimeout);
+ account_.sendMessage(p.to, p.payloads, p.token, retryOnTimeout);
}
}
diff --git a/src/im/message_engine.h b/src/im/message_engine.h
index 962eaaa5bd..2057db0753 100644
--- a/src/im/message_engine.h
+++ b/src/im/message_engine.h
@@ -42,7 +42,8 @@ public:
MessageEngine(SIPAccountBase&, const std::string& path);
MessageToken sendMessage(const std::string& to,
- const std::map<std::string, std::string>& payloads);
+ const std::map<std::string, std::string>& payloads,
+ uint64_t refreshToken);
MessageStatus getStatus(MessageToken t) const;
diff --git a/src/jamidht/conversation_module.cpp b/src/jamidht/conversation_module.cpp
index bf347d10c5..2d13bdde21 100644
--- a/src/jamidht/conversation_module.cpp
+++ b/src/jamidht/conversation_module.cpp
@@ -268,6 +268,7 @@ public:
// Replay conversations (after erasing/re-adding)
std::mutex replayMtx_;
std::map<std::string, std::vector<std::map<std::string, std::string>>> replay_;
+ std::map<std::string, uint64_t> refreshMessage;
};
ConversationModule::Impl::Impl(std::weak_ptr<JamiAccount>&& account,
@@ -467,7 +468,8 @@ ConversationModule::Impl::fetchNewCommits(const std::string& peer,
accountId_.c_str(),
conversationId.c_str());
sendMsgCb_(peer,
- std::map<std::string, std::string> {{"application/invite", conversationId}});
+ std::map<std::string, std::string> {{"application/invite", conversationId}},
+ 0);
}
}
@@ -769,8 +771,10 @@ ConversationModule::Impl::sendMessageNotification(const Conversation& conversati
const auto text = Json::writeString(builder, message);
for (const auto& member : conversation.memberUris(sync ? "" : username_)) {
// Announce to all members that a new message is sent
- sendMsgCb_(member,
- std::map<std::string, std::string> {{"application/im-gitmessage-id", text}});
+ refreshMessage[member] = sendMsgCb_(member,
+ std::map<std::string, std::string> {
+ {"application/im-gitmessage-id", text}},
+ refreshMessage[member]);
}
}
@@ -1075,7 +1079,7 @@ ConversationModule::onNeedConversationRequest(const std::string& from,
auto invite = itConv->second->generateInvitation();
lk.unlock();
JAMI_DBG("%s is asking a new invite for %s", from.c_str(), conversationId.c_str());
- pimpl_->sendMsgCb_(from, std::move(invite));
+ pimpl_->sendMsgCb_(from, std::move(invite), 0);
}
}
@@ -1499,7 +1503,8 @@ ConversationModule::onNewCommit(const std::string& peer,
conversationId.c_str());
pimpl_->sendMsgCb_(peer,
std::map<std::string, std::string> {
- {"application/invite", conversationId}});
+ {"application/invite", conversationId}},
+ 0);
return;
}
JAMI_DBG("[Account %s] on new commit notification from %s, for %s, commit %s",
@@ -1532,7 +1537,7 @@ ConversationModule::addConversationMember(const std::string& conversationId,
// we should not forbid new invites
auto invite = it->second->generateInvitation();
lk.unlock();
- pimpl_->sendMsgCb_(contactUri, std::move(invite));
+ pimpl_->sendMsgCb_(contactUri, std::move(invite), 0);
return;
}
@@ -1550,7 +1555,7 @@ ConversationModule::addConversationMember(const std::string& conversationId,
if (sendRequest) {
auto invite = it->second->generateInvitation();
lk.unlock();
- pimpl_->sendMsgCb_(contactUri, std::move(invite));
+ pimpl_->sendMsgCb_(contactUri, std::move(invite), 0);
}
}
}
diff --git a/src/jamidht/conversation_module.h b/src/jamidht/conversation_module.h
index ac5d947a5a..a4e4d4ae4f 100644
--- a/src/jamidht/conversation_module.h
+++ b/src/jamidht/conversation_module.h
@@ -40,7 +40,8 @@ struct SyncMsg
using ChannelCb = std::function<bool(const std::shared_ptr<ChannelSocket>&)>;
using NeedSocketCb = std::function<void(const std::string&, const std::string&, ChannelCb&&)>;
-using SengMsgCb = std::function<void(const std::string&, std::map<std::string, std::string>&&)>;
+using SengMsgCb
+ = std::function<uint64_t(const std::string&, std::map<std::string, std::string>&&, uint64_t)>;
using NeedsSyncingCb = std::function<void()>;
using UpdateConvReq = std::function<void(const std::string&, const std::string&, bool)>;
diff --git a/src/jamidht/jamiaccount.cpp b/src/jamidht/jamiaccount.cpp
index 4e1c11ffc2..293593e4e6 100644
--- a/src/jamidht/jamiaccount.cpp
+++ b/src/jamidht/jamiaccount.cpp
@@ -2350,11 +2350,11 @@ JamiAccount::convModule()
shared->syncModule()->syncWithConnected();
});
},
- [this](auto&& uri, auto&& msg) {
- runOnMainThread([w = weak(), uri, msg] {
- if (auto shared = w.lock())
- shared->sendTextMessage(uri, msg);
- });
+ [this](auto&& uri, auto&& msg, auto token = 0) {
+ // No need to retrigger, sendTextMessage will call
+ // messageEngine_.sendMessage, already retriggering on
+ // main thread.
+ return sendTextMessage(uri, msg, token);
},
[this](const auto& convId, const auto& deviceId, auto&& cb) {
runOnMainThread([w = weak(), convId, deviceId, cb = std::move(cb)] {
@@ -3147,7 +3147,8 @@ JamiAccount::forEachDevice(const dht::InfoHash& to,
uint64_t
JamiAccount::sendTextMessage(const std::string& to,
- const std::map<std::string, std::string>& payloads)
+ const std::map<std::string, std::string>& payloads,
+ uint64_t refreshToken)
{
Uri uri(to);
if (uri.scheme() == Uri::Scheme::SWARM) {
@@ -3166,15 +3167,15 @@ JamiAccount::sendTextMessage(const std::string& to,
JAMI_ERR("Multi-part im is not supported yet by JamiAccount");
return 0;
}
- return SIPAccountBase::sendTextMessage(toUri, payloads);
+ return SIPAccountBase::sendTextMessage(toUri, payloads, refreshToken);
}
void
-JamiAccount::sendTextMessage(const std::string& to,
- const std::map<std::string, std::string>& payloads,
- uint64_t token,
- bool retryOnTimeout,
- bool onlyConnected)
+JamiAccount::sendMessage(const std::string& to,
+ const std::map<std::string, std::string>& payloads,
+ uint64_t token,
+ bool retryOnTimeout,
+ bool onlyConnected)
{
std::string toUri;
try {
@@ -3640,7 +3641,7 @@ JamiAccount::sendInstantMessage(const std::string& convId,
auto uri = m.at("uri");
auto token = std::uniform_int_distribution<uint64_t> {1, JAMI_ID_MAX_VAL}(rand);
// Announce to all members that a new message is sent
- sendTextMessage(uri, msg, token, false, true);
+ sendMessage(uri, msg, token, false, true);
}
}
diff --git a/src/jamidht/jamiaccount.h b/src/jamidht/jamiaccount.h
index d47867f14b..f9eb1f832a 100644
--- a/src/jamidht/jamiaccount.h
+++ b/src/jamidht/jamiaccount.h
@@ -332,13 +332,14 @@ public:
std::map<std::string, std::string> getContactDetails(const std::string& uri) const;
void sendTrustRequest(const std::string& to, const std::vector<uint8_t>& payload);
- void sendTextMessage(const std::string& to,
- const std::map<std::string, std::string>& payloads,
- uint64_t id,
- bool retryOnTimeout = true,
- bool onlyConnected = false) override;
+ void sendMessage(const std::string& to,
+ const std::map<std::string, std::string>& payloads,
+ uint64_t id,
+ bool retryOnTimeout = true,
+ bool onlyConnected = false) override;
uint64_t sendTextMessage(const std::string& to,
- const std::map<std::string, std::string>& payloads) override;
+ const std::map<std::string, std::string>& payloads,
+ uint64_t refreshToken = 0) override;
void sendInstantMessage(const std::string& convId,
const std::map<std::string, std::string>& msg);
void onIsComposing(const std::string& conversationId, const std::string& peer, bool isWriting);
diff --git a/src/sip/sipaccount.cpp b/src/sip/sipaccount.cpp
index 7654fbe0ba..08e55cb6cf 100644
--- a/src/sip/sipaccount.cpp
+++ b/src/sip/sipaccount.cpp
@@ -2147,11 +2147,11 @@ static pjsip_accept_hdr* im_create_accept(pj_pool_t *pool)
#endif
void
-SIPAccount::sendTextMessage(const std::string& to,
- const std::map<std::string, std::string>& payloads,
- uint64_t id,
- bool,
- bool)
+SIPAccount::sendMessage(const std::string& to,
+ const std::map<std::string, std::string>& payloads,
+ uint64_t id,
+ bool,
+ bool)
{
if (to.empty() or payloads.empty()) {
JAMI_WARN("No sender or payload");
diff --git a/src/sip/sipaccount.h b/src/sip/sipaccount.h
index 5cb2ad4df7..525cda54d9 100644
--- a/src/sip/sipaccount.h
+++ b/src/sip/sipaccount.h
@@ -442,11 +442,11 @@ public:
void onRegister(pjsip_regc_cbparam* param);
- virtual void sendTextMessage(const std::string& to,
- const std::map<std::string, std::string>& payloads,
- uint64_t id,
- bool retryOnTimeout = true,
- bool onlyConnected = false) override;
+ virtual void sendMessage(const std::string& to,
+ const std::map<std::string, std::string>& payloads,
+ uint64_t id,
+ bool retryOnTimeout = true,
+ bool onlyConnected = false) override;
void connectivityChanged() override;
diff --git a/src/sip/sipaccountbase.h b/src/sip/sipaccountbase.h
index b9b245fcd9..0d9c4a459a 100644
--- a/src/sip/sipaccountbase.h
+++ b/src/sip/sipaccountbase.h
@@ -233,23 +233,30 @@ public:
* file, that can be used directly by PJSIP to initialize
* an alternate UDP transport.
*/
- std::string getStunServer() const { return stunServer_; }
+ std::string getStunServer() const
+ {
+ return stunServer_;
+ }
- void setStunServer(const std::string& srv) { stunServer_ = srv; }
+ void setStunServer(const std::string& srv)
+ {
+ stunServer_ = srv;
+ }
IceTransportOptions getIceOptions() const noexcept;
- virtual void sendTextMessage(const std::string& to,
- const std::map<std::string, std::string>& payloads,
- uint64_t id,
- bool retryOnTimeout = true,
- bool onlyConnected = false)
+ virtual void sendMessage(const std::string& to,
+ const std::map<std::string, std::string>& payloads,
+ uint64_t id,
+ bool retryOnTimeout = true,
+ bool onlyConnected = false)
= 0;
virtual uint64_t sendTextMessage(const std::string& to,
- const std::map<std::string, std::string>& payloads) override
+ const std::map<std::string, std::string>& payloads,
+ uint64_t refreshToken = 0) override
{
- return messageEngine_.sendMessage(to, payloads);
+ return messageEngine_.sendMessage(to, payloads, refreshToken);
}
im::MessageStatus getMessageStatus(uint64_t id) const override
@@ -257,7 +264,10 @@ public:
return messageEngine_.getStatus(id);
}
- bool cancelMessage(uint64_t id) override { return messageEngine_.cancel(id); }
+ bool cancelMessage(uint64_t id) override
+ {
+ return messageEngine_.cancel(id);
+ }
virtual void onTextMessage(const std::string& id,
const std::string& from,
--
GitLab