Skip to content
Snippets Groups Projects
Commit 8855505e authored by Guillaume Roguez's avatar Guillaume Roguez Committed by Andreas Traczyk
Browse files

call: fix OCP violation


"RoboCop watching you"

IM dedicated code has been inserted into Call::setState()
and it's an Open Close Principle violation.
Move it into a dedicated function and assign it to call state
change as listener.

As code changed, also make a better state listening implementation
that do not depend on listener call order.

Notice: TODO have been inserted, it's for another patch.

Change-Id: Ibe37555320de1dec173338a62323320d3d93bb4a
Reviewed-by: default avatarAndreas Traczyk <andreas.traczyk@savoirfairelinux.com>
parent aae8ef1d
No related branches found
No related tags found
No related merge requests found
......@@ -44,6 +44,10 @@ Call::Call(Account& account, const std::string& id, Call::CallType type)
, type_(type)
, account_(account)
{
addStateListener([this](UNUSED Call::CallState call_state,
UNUSED Call::ConnectionState cnx_state,
UNUSED int code) { checkPendingIM(); });
time(&timestamp_start_);
account_.attachCall(id_);
}
......@@ -159,21 +163,6 @@ Call::setState(CallState call_state, ConnectionState cnx_state, signed code)
connectionState_ = cnx_state;
auto new_client_state = getStateStr();
if (call_state == CallState::OVER) {
RING_DBG("[call:%s] %lu subcalls %lu listeners", id_.c_str(), subcalls.size(), stateChangedListeners_.size());
if (not subcalls.empty()) {
auto subs = std::move(subcalls);
for (auto c : subs)
c->hangup(0);
pendingInMessages_.clear();
pendingOutMessages_.clear();
}
} else if (call_state == CallState::ACTIVE and connectionState_ == ConnectionState::CONNECTED and not pendingOutMessages_.empty()) {
for (const auto& msg : pendingOutMessages_)
sendTextMessage(msg.first, msg.second);
pendingOutMessages_.clear();
}
for (auto& l : stateChangedListeners_)
l(callState_, connectionState_, code);
......@@ -486,5 +475,28 @@ Call::merge(const std::shared_ptr<Call>& scall)
scall->removeCall();
}
void
Call::checkPendingIM()
{
using namespace DRing::Call;
auto state = getStateStr();
if (state == StateEvent::OVER) {
// Hangup device's call when parent is over
if (not subcalls.empty()) {
auto subs = std::move(subcalls);
for (auto c : subs)
c->hangup(0);
pendingInMessages_.clear();
pendingOutMessages_.clear();
}
} else if (state == StateEvent::CURRENT) {
// Peer connected, time to send pending IM
// TODO not thread safe
for (const auto& msg : pendingOutMessages_)
sendTextMessage(msg.first, msg.second);
pendingOutMessages_.clear();
}
}
} // namespace ring
......@@ -354,6 +354,8 @@ class Call : public Recordable, public std::enable_shared_from_this<Call> {
*/
Call(Account& account, const std::string& id, Call::CallType type);
// TODO all these members are not protected against multi-thread access
std::shared_ptr<IceTransport> iceTransport_ {};
bool isAudioMuted_{false};
......@@ -367,6 +369,8 @@ class Call : public Recordable, public std::enable_shared_from_this<Call> {
bool validStateTransition(CallState newState);
void checkPendingIM();
/** Protect every attribute that can be changed by two threads */
mutable std::recursive_mutex callMutex_ {};
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment