From 8855505e4c0925599be7b00d16c79d4815b3557f Mon Sep 17 00:00:00 2001
From: Guillaume Roguez <guillaume.roguez@savoirfairelinux.com>
Date: Tue, 21 Feb 2017 15:53:28 -0500
Subject: [PATCH] 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: Andreas Traczyk <andreas.traczyk@savoirfairelinux.com>
---
 src/call.cpp | 42 +++++++++++++++++++++++++++---------------
 src/call.h   |  4 ++++
 2 files changed, 31 insertions(+), 15 deletions(-)

diff --git a/src/call.cpp b/src/call.cpp
index d98aac8dfd..5ffb2a685a 100644
--- a/src/call.cpp
+++ b/src/call.cpp
@@ -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
diff --git a/src/call.h b/src/call.h
index 27784aa03e..b9da627dc1 100644
--- a/src/call.h
+++ b/src/call.h
@@ -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_ {};
 
-- 
GitLab