# HG changeset patch
# User Sebastien Jodogne <s.jodogne@gmail.com>
# Date 1571155900 -7200
# Node ID 05b2e71ed145a2f73690438603a418e8350479b1
# Parent  b537002f83a9cf568fdc26a85431eecf97e2fa7d
removed MessageForwarder, unit tests are ok

diff -r b537002f83a9 -r 05b2e71ed145 Framework/Messages/IObservable.cpp
--- a/Framework/Messages/IObservable.cpp	Tue Oct 15 15:39:39 2019 +0200
+++ b/Framework/Messages/IObservable.cpp	Tue Oct 15 18:11:40 2019 +0200
@@ -40,19 +40,6 @@
         delete *it2;
       }
     }
-
-    // unregister the forwarders but don't delete them (they'll be
-    // deleted by the observable they are observing as any other
-    // callable)
-    for (Forwarders::iterator it = forwarders_.begin();
-         it != forwarders_.end(); ++it)
-    {
-      IMessageForwarder* fw = *it;
-
-      // TODO - What to do with forwarders?
-      
-      //broker_.Unregister(dynamic_cast<IObserver&>(*fw));
-    }
   }
   
 
@@ -67,33 +54,6 @@
     callables_[id].insert(callable);
   }
 
-  void IObservable::Unregister(IObserver *observer)
-  {
-    LOG(TRACE) << "IObservable::Unregister for IObserver at addr: "
-      << std::hex << observer << std::dec;
-    // delete all callables from this observer
-    for (Callables::iterator itCallableSet = callables_.begin();
-         itCallableSet != callables_.end(); ++itCallableSet)
-    {
-      for (std::set<ICallable*>::const_iterator
-             itCallable = itCallableSet->second.begin(); itCallable != itCallableSet->second.end(); )
-      {
-        boost::shared_ptr<IObserver> shared((*itCallable)->GetObserver());
-
-        if (shared &&
-            shared.get() == observer)
-        {
-          LOG(TRACE) << "  ** IObservable::Unregister : deleting callable: "
-            << std::hex << (*itCallable) << std::dec;
-          delete *itCallable;
-          itCallableSet->second.erase(itCallable++);
-        }
-        else
-          ++itCallable;
-      }
-    }
-  }
-  
   void IObservable::EmitMessageInternal(const IObserver* receiver,
                                         const IMessage& message)
   {
@@ -108,7 +68,7 @@
       {
         assert(*it != NULL);
 
-        boost::shared_ptr<IObserver> observer((*it)->GetObserver());
+        boost::shared_ptr<IObserver> observer((*it)->GetObserver().lock());
 
         if (observer)
         {
@@ -141,14 +101,4 @@
       << std::hex << &observer << std::dec;
     EmitMessageInternal(&observer, message);
   }
-  
-  void IObservable::RegisterForwarder(IMessageForwarder* forwarder)
-  {
-    if (forwarder == NULL)
-    {
-      throw Orthanc::OrthancException(Orthanc::ErrorCode_NullPointer);
-    }
-    
-    forwarders_.insert(forwarder);
-  }
 }
diff -r b537002f83a9 -r 05b2e71ed145 Framework/Messages/IObservable.h
--- a/Framework/Messages/IObservable.h	Tue Oct 15 15:39:39 2019 +0200
+++ b/Framework/Messages/IObservable.h	Tue Oct 15 18:11:40 2019 +0200
@@ -24,7 +24,6 @@
 #include "../StoneEnumerations.h"
 #include "ICallable.h"
 #include "IObserver.h"
-#include "MessageForwarder.h"
 
 #include <set>
 #include <map>
@@ -36,10 +35,7 @@
   private:
     typedef std::map<MessageIdentifier, std::set<ICallable*> >  Callables;
 
-    typedef std::set<IMessageForwarder*>     Forwarders;
-
     Callables       callables_;
-    Forwarders      forwarders_;
 
     void EmitMessageInternal(const IObserver* receiver,
                              const IMessage& message);
@@ -50,15 +46,9 @@
     // Takes ownsership of the callable
     void RegisterCallable(ICallable* callable);
 
-    // TODO - Remove this?
-    void Unregister(IObserver* observer);
-
     void BroadcastMessage(const IMessage& message);
 
     void EmitMessage(const IObserver& observer,
                      const IMessage& message);
-
-    // Takes ownsership
-    void RegisterForwarder(IMessageForwarder* forwarder);
   };
 }
diff -r b537002f83a9 -r 05b2e71ed145 Framework/Messages/MessageForwarder.cpp
--- a/Framework/Messages/MessageForwarder.cpp	Tue Oct 15 15:39:39 2019 +0200
+++ /dev/null	Thu Jan 01 00:00:00 1970 +0000
@@ -1,38 +0,0 @@
-/**
- * Stone of Orthanc
- * Copyright (C) 2012-2016 Sebastien Jodogne, Medical Physics
- * Department, University Hospital of Liege, Belgium
- * Copyright (C) 2017-2019 Osimis S.A., Belgium
- *
- * This program is free software: you can redistribute it and/or
- * modify it under the terms of the GNU Affero General Public License
- * as published by the Free Software Foundation, either version 3 of
- * the License, or (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful, but
- * WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * Affero General Public License for more details.
- *
- * You should have received a copy of the GNU Affero General Public License
- * along with this program. If not, see <http://www.gnu.org/licenses/>.
- **/
-
-
-#include "MessageForwarder.h"
-
-#include "IObservable.h"
-
-namespace OrthancStone
-{
-
-  void IMessageForwarder::ForwardMessageInternal(const IMessage& message)
-  {
-    emitter_.BroadcastMessage(message);
-  }
-
-  void IMessageForwarder::RegisterForwarderInEmitter()
-  {
-    emitter_.RegisterForwarder(this);
-  }
-}
diff -r b537002f83a9 -r 05b2e71ed145 Framework/Messages/MessageForwarder.h
--- a/Framework/Messages/MessageForwarder.h	Tue Oct 15 15:39:39 2019 +0200
+++ /dev/null	Thu Jan 01 00:00:00 1970 +0000
@@ -1,85 +0,0 @@
-/**
- * Stone of Orthanc
- * Copyright (C) 2012-2016 Sebastien Jodogne, Medical Physics
- * Department, University Hospital of Liege, Belgium
- * Copyright (C) 2017-2019 Osimis S.A., Belgium
- *
- * This program is free software: you can redistribute it and/or
- * modify it under the terms of the GNU Affero General Public License
- * as published by the Free Software Foundation, either version 3 of
- * the License, or (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful, but
- * WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * Affero General Public License for more details.
- *
- * You should have received a copy of the GNU Affero General Public License
- * along with this program. If not, see <http://www.gnu.org/licenses/>.
- **/
-
-
-#pragma once
-
-#include "ICallable.h"
-#include "IObserver.h"
-
-#include <boost/noncopyable.hpp>
-
-namespace OrthancStone
-{
-
-  class IObservable;
-
-  class IMessageForwarder : public IObserver
-  {
-    IObservable& emitter_;
-  public:
-    IMessageForwarder(IObservable& emitter)
-      : emitter_(emitter)
-    {}
-    virtual ~IMessageForwarder() {}
-
-  protected:
-    void ForwardMessageInternal(const IMessage& message);
-    void RegisterForwarderInEmitter();
-
-  };
-
-  /* When an Observer (B) simply needs to re-emit a message it has received, instead of implementing
-   * a specific member function to forward the message, it can create a MessageForwarder.
-   * The MessageForwarder will re-emit the message "in the name of (B)"
-   *
-   * Consider the chain where
-   * A is an observable
-   * |
-   * B is an observer of A and observable
-   * |
-   * C is an observer of B and knows that B is re-emitting many messages from A
-   *
-   * instead of implementing a callback, B will create a MessageForwarder that will emit the messages in his name:
-   * A.RegisterObserverCallback(new MessageForwarder<A::MessageType>(broker, *this)  // where "this" is B
-   *
-   * in C:
-   * B.RegisterObserverCallback(new Callable<C, A:MessageTyper>(*this, &B::MyCallback))   // where "this" is C
-   */
-  template<typename TMessage>
-  class MessageForwarder : public IMessageForwarder, public Callable<MessageForwarder<TMessage>, TMessage>
-  {
-  public:
-    MessageForwarder(IObservable& emitter // the object that will emit the messages to forward
-                     )
-      : IMessageForwarder(emitter),
-        Callable<MessageForwarder<TMessage>, TMessage>(*this, &MessageForwarder::ForwardMessage)
-    {
-      RegisterForwarderInEmitter();
-    }
-
-protected:
-    void ForwardMessage(const TMessage& message)
-    {
-      ForwardMessageInternal(message);
-    }
-
-  };
-}
diff -r b537002f83a9 -r 05b2e71ed145 Framework/Messages/ObserverBase.h
--- a/Framework/Messages/ObserverBase.h	Tue Oct 15 15:39:39 2019 +0200
+++ b/Framework/Messages/ObserverBase.h	Tue Oct 15 18:11:40 2019 +0200
@@ -45,8 +45,10 @@
       }
       catch (boost::bad_weak_ptr&)
       {
-        throw Orthanc::OrthancException(Orthanc::ErrorCode_InternalError,
-                                        "Cannot get a shared pointer to an observer from a constructor");
+        throw Orthanc::OrthancException(
+          Orthanc::ErrorCode_InternalError,
+          "Cannot get a shared pointer to an observer from its constructor, "
+          "or the observer is not created as a shared pointer");
       }
     }
 
diff -r b537002f83a9 -r 05b2e71ed145 Framework/Radiography/RadiographyWidget.cpp
--- a/Framework/Radiography/RadiographyWidget.cpp	Tue Oct 15 15:39:39 2019 +0200
+++ b/Framework/Radiography/RadiographyWidget.cpp	Tue Oct 15 18:11:40 2019 +0200
@@ -279,11 +279,6 @@
 
   void RadiographyWidget::SetScene(boost::shared_ptr<RadiographyScene> scene)
   {
-    if (scene_ != NULL)
-    {
-      scene_->Unregister(this);
-    }
-
     scene_ = scene;
 
     Register<RadiographyScene::GeometryChangedMessage>(*scene_, &RadiographyWidget::OnGeometryChanged);
diff -r b537002f83a9 -r 05b2e71ed145 Framework/Scene2DViewport/MeasureTool.cpp
--- a/Framework/Scene2DViewport/MeasureTool.cpp	Tue Oct 15 15:39:39 2019 +0200
+++ b/Framework/Scene2DViewport/MeasureTool.cpp	Tue Oct 15 18:11:40 2019 +0200
@@ -28,14 +28,6 @@
 
 namespace OrthancStone
 {
-  MeasureTool::~MeasureTool()
-  {
-    // if the controller is dead, let's not bother.
-    boost::shared_ptr<ViewportController> controller = controllerW_.lock();
-    if (controller)
-      controller->Unregister(this);
-  }
-
   void MeasureTool::Enable()
   {
     enabled_ = true;
diff -r b537002f83a9 -r 05b2e71ed145 Framework/Scene2DViewport/MeasureTool.h
--- a/Framework/Scene2DViewport/MeasureTool.h	Tue Oct 15 15:39:39 2019 +0200
+++ b/Framework/Scene2DViewport/MeasureTool.h	Tue Oct 15 18:11:40 2019 +0200
@@ -41,7 +41,9 @@
   class MeasureTool : public ObserverBase<MeasureTool>
   {
   public:
-    virtual ~MeasureTool();
+    virtual ~MeasureTool()
+    {
+    }
 
     /**
     Enabled tools are rendered in the scene.
diff -r b537002f83a9 -r 05b2e71ed145 Resources/CMake/OrthancStoneConfiguration.cmake
--- a/Resources/CMake/OrthancStoneConfiguration.cmake	Tue Oct 15 15:39:39 2019 +0200
+++ b/Resources/CMake/OrthancStoneConfiguration.cmake	Tue Oct 15 18:11:40 2019 +0200
@@ -453,7 +453,6 @@
   ${ORTHANC_STONE_ROOT}/Framework/Messages/IMessage.h
   ${ORTHANC_STONE_ROOT}/Framework/Messages/IObservable.cpp
   ${ORTHANC_STONE_ROOT}/Framework/Messages/IObserver.h
-  ${ORTHANC_STONE_ROOT}/Framework/Messages/MessageForwarder.cpp
   ${ORTHANC_STONE_ROOT}/Framework/Oracle/GetOrthancImageCommand.cpp
   ${ORTHANC_STONE_ROOT}/Framework/Oracle/GetOrthancWebViewerJpegCommand.cpp
   ${ORTHANC_STONE_ROOT}/Framework/Oracle/OracleCommandWithPayload.cpp
diff -r b537002f83a9 -r 05b2e71ed145 UnitTestsSources/TestMessageBroker.cpp
--- a/UnitTestsSources/TestMessageBroker.cpp	Tue Oct 15 15:39:39 2019 +0200
+++ b/UnitTestsSources/TestMessageBroker.cpp	Tue Oct 15 18:11:40 2019 +0200
@@ -22,8 +22,7 @@
 #include "gtest/gtest.h"
 
 #include "Framework/Messages/IObservable.h"
-#include "Framework/Messages/IObserver.h"
-#include "Framework/Messages/MessageForwarder.h"
+#include "Framework/Messages/ObserverBase.h"
 
 
 int testCounter = 0;
@@ -48,26 +47,13 @@
     };
   };
 
-  class MyObserver : public IObserver
+  class MyObserver : public ObserverBase<MyObserver>
   {
   public:
     void HandleCompletedMessage(const MyObservable::MyCustomMessage& message)
     {
       testCounter += message.payload_;
     }
-
-  };
-
-
-  class MyIntermediate : public IObserver, public IObservable
-  {
-    IObservable& observedObject_;
-  public:
-    MyIntermediate(IObservable& observedObject)
-      : observedObject_(observedObject)
-    {
-      observedObject_.RegisterObserverCallback(new MessageForwarder<MyObservable::MyCustomMessage>(*this));
-    }
   };
 }
 
@@ -75,10 +61,10 @@
 TEST(MessageBroker, TestPermanentConnectionSimpleUseCase)
 {
   MyObservable  observable;
-  MyObserver    observer;
+  boost::shared_ptr<MyObserver>  observer(new MyObserver);
 
   // create a permanent connection between an observable and an observer
-  observable.RegisterObserverCallback(new Callable<MyObserver, MyObservable::MyCustomMessage>(observer, &MyObserver::HandleCompletedMessage));
+  observer->Register<MyObservable::MyCustomMessage>(observable, &MyObserver::HandleCompletedMessage);
 
   testCounter = 0;
   observable.BroadcastMessage(MyObservable::MyCustomMessage(12));
@@ -90,45 +76,26 @@
   ASSERT_EQ(20, testCounter);
 
   // Unregister the observer; make sure it's not called anymore
-  observable.Unregister(&observer);
+  observer.reset();
   testCounter = 0;
   observable.BroadcastMessage(MyObservable::MyCustomMessage(20));
   ASSERT_EQ(0, testCounter);
 }
 
-TEST(MessageBroker, TestMessageForwarderSimpleUseCase)
-{
-  MyObservable  observable;
-  MyIntermediate intermediate(observable);
-  MyObserver    observer;
-
-  // let the observer observers the intermediate that is actually forwarding the messages from the observable
-  intermediate.RegisterObserverCallback(new Callable<MyObserver, MyObservable::MyCustomMessage>(observer, &MyObserver::HandleCompletedMessage));
-
-  testCounter = 0;
-  observable.BroadcastMessage(MyObservable::MyCustomMessage(12));
-  ASSERT_EQ(12, testCounter);
-
-  // the connection is permanent; if we emit the same message again, the observer will be notified again
-  testCounter = 0;
-  observable.BroadcastMessage(MyObservable::MyCustomMessage(20));
-  ASSERT_EQ(20, testCounter);
-}
-
 TEST(MessageBroker, TestPermanentConnectionDeleteObserver)
 {
   MyObservable  observable;
-  MyObserver*   observer(new MyObserver);
+  boost::shared_ptr<MyObserver>  observer(new MyObserver);
 
   // create a permanent connection between an observable and an observer
-  observable.RegisterObserverCallback(new Callable<MyObserver, MyObservable::MyCustomMessage>(*observer, &MyObserver::HandleCompletedMessage));
+  observer->Register<MyObservable::MyCustomMessage>(observable, &MyObserver::HandleCompletedMessage);
 
   testCounter = 0;
   observable.BroadcastMessage(MyObservable::MyCustomMessage(12));
   ASSERT_EQ(12, testCounter);
 
   // delete the observer and check that the callback is not called anymore
-  delete observer;
+  observer.reset();
 
   // the connection is permanent; if we emit the same message again, the observer will be notified again
   testCounter = 0;
@@ -136,43 +103,6 @@
   ASSERT_EQ(0, testCounter);
 }
 
-TEST(MessageBroker, TestMessageForwarderDeleteIntermediate)
-{
-  MyObservable  observable;
-  MyIntermediate* intermediate(new MyIntermediate(observable));
-  MyObserver    observer;
-
-  // let the observer observers the intermediate that is actually forwarding the messages from the observable
-  intermediate->RegisterObserverCallback(new Callable<MyObserver, MyObservable::MyCustomMessage>(observer, &MyObserver::HandleCompletedMessage));
-
-  testCounter = 0;
-  observable.BroadcastMessage(MyObservable::MyCustomMessage(12));
-  ASSERT_EQ(12, testCounter);
-
-  delete intermediate;
-
-  observable.BroadcastMessage(MyObservable::MyCustomMessage(20));
-  ASSERT_EQ(12, testCounter);
-}
-
-TEST(MessageBroker, TestCustomMessage)
-{
-  MyObservable  observable;
-  MyIntermediate intermediate(observable);
-  MyObserver    observer;
-
-  // let the observer observers the intermediate that is actually forwarding the messages from the observable
-  intermediate.RegisterObserverCallback(new Callable<MyObserver, MyObservable::MyCustomMessage>(observer, &MyObserver::HandleCompletedMessage));
-
-  testCounter = 0;
-  observable.BroadcastMessage(MyObservable::MyCustomMessage(12));
-  ASSERT_EQ(12, testCounter);
-
-  // the connection is permanent; if we emit the same message again, the observer will be notified again
-  testCounter = 0;
-  observable.BroadcastMessage(MyObservable::MyCustomMessage(20));
-  ASSERT_EQ(20, testCounter);
-}
 
 
 #if 0 /* __cplusplus >= 201103L*/