changeset 302:4a79193ffb58 am-callable-and-promise

support for custom messages + no leaks in unit-tests
author am@osimis.io
date Tue, 18 Sep 2018 18:04:53 +0200
parents 547e1cf7aa7b
children ed1a4302154f
files Framework/Messages/IMessage.h Framework/Messages/IObservable.h Framework/Messages/MessageForwarder.cpp Framework/Messages/MessageForwarder.h Framework/Messages/MessageType.h Framework/Messages/Promise.h Resources/CMake/OrthancStoneConfiguration.cmake UnitTestsSources/TestMessageBroker2.cpp
diffstat 8 files changed, 166 insertions(+), 34 deletions(-) [+]
line wrap: on
line diff
--- a/Framework/Messages/IMessage.h	Tue Sep 18 15:34:28 2018 +0200
+++ b/Framework/Messages/IMessage.h	Tue Sep 18 18:04:53 2018 +0200
@@ -31,21 +31,21 @@
   // base message that are exchanged between IObservable and IObserver
   struct IMessage : public boost::noncopyable
   {
-    MessageType messageType_;
+    int messageType_;
   protected:
-    IMessage(const MessageType& messageType)
+    IMessage(const int& messageType)
       : messageType_(messageType)
     {}
   public:
     virtual ~IMessage() {}
 
-    MessageType GetType() const {return messageType_;}
+    virtual int GetType() const {return messageType_;}
   };
 
 
   // base class to derive from to implement your own messages
   // it handles the message type for you
-  template <MessageType type>
+  template <int type>
   struct BaseMessage : public IMessage
   {
     enum
@@ -54,7 +54,7 @@
     };
 
     BaseMessage()
-      : IMessage(static_cast<MessageType>(Type))
+      : IMessage(static_cast<int>(Type))
     {}
   };
 
--- a/Framework/Messages/IObservable.h	Tue Sep 18 15:34:28 2018 +0200
+++ b/Framework/Messages/IObservable.h	Tue Sep 18 18:04:53 2018 +0200
@@ -27,13 +27,16 @@
 #include <iostream>
 #include <map>
 
+
 #include "MessageBroker.h"
 #include "MessageType.h"
 #include "ICallable.h"
 #include "IObserver.h"
+#include "MessageForwarder.h"
 
 namespace OrthancStone {
 
+
   class IObservable : public boost::noncopyable
   {
   protected:
@@ -42,6 +45,9 @@
     typedef std::map<int, std::set<ICallable*> >   Callables;
     Callables                         callables_;
 
+    typedef std::set<IMessageForwarder*>      Forwarders;
+    Forwarders                        forwarders_;
+
   public:
 
     IObservable(MessageBroker& broker)
@@ -50,6 +56,7 @@
     }
     virtual ~IObservable()
     {
+      // delete all callables (this will also unregister them from the broker)
       for (Callables::const_iterator it = callables_.begin();
            it != callables_.end(); ++it)
       {
@@ -59,6 +66,13 @@
           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)
+      {
+        broker_.Unregister(dynamic_cast<IObserver&>(**it));
+      }
     }
 
     void RegisterObserverCallback(ICallable* callable)
@@ -85,6 +99,11 @@
       }
     }
 
+    void RegisterForwarder(IMessageForwarder* forwarder)
+    {
+      forwarders_.insert(forwarder);
+    }
+
   };
 
 }
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/Framework/Messages/MessageForwarder.cpp	Tue Sep 18 18:04:53 2018 +0200
@@ -0,0 +1,38 @@
+/**
+ * Stone of Orthanc
+ * Copyright (C) 2012-2016 Sebastien Jodogne, Medical Physics
+ * Department, University Hospital of Liege, Belgium
+ * Copyright (C) 2017-2018 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_.EmitMessage(message);
+  }
+
+  void IMessageForwarder::RegisterForwarderInEmitter()
+  {
+    emitter_.RegisterForwarder(this);
+  }
+}
--- a/Framework/Messages/MessageForwarder.h	Tue Sep 18 15:34:28 2018 +0200
+++ b/Framework/Messages/MessageForwarder.h	Tue Sep 18 18:04:53 2018 +0200
@@ -22,32 +22,66 @@
 #pragma once
 
 #include "ICallable.h"
-#include "IObservable.h"
 #include "IObserver.h"
 
 #include <boost/noncopyable.hpp>
 
-namespace OrthancStone {
+namespace OrthancStone
+{
+
+  class IObservable;
+
+  class IMessageForwarder : public boost::noncopyable
+  {
+    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 IObserver, public Callable<MessageForwarder<TMessage>, TMessage>
+  class MessageForwarder : public IMessageForwarder, public IObserver, public Callable<MessageForwarder<TMessage>, TMessage>
   {
-    IObservable& observable_;
   public:
     MessageForwarder(MessageBroker& broker,
-                     IObservable& observable // the object that will emit the forwarded message
+                     IObservable& emitter // the object that will emit the messages to forward
                      )
-      : IObserver(broker),
-        Callable<MessageForwarder<TMessage>, TMessage>(*this, &MessageForwarder::ForwardMessage),
-        observable_(observable)
+      : IMessageForwarder(emitter),
+        IObserver(broker),
+        Callable<MessageForwarder<TMessage>, TMessage>(*this, &MessageForwarder::ForwardMessage)
     {
+      RegisterForwarderInEmitter();
     }
 
-  protected:
+protected:
     void ForwardMessage(const TMessage& message)
     {
-      observable_.EmitMessage(message);
+      ForwardMessageInternal(message);
     }
+
   };
 }
--- a/Framework/Messages/MessageType.h	Tue Sep 18 15:34:28 2018 +0200
+++ b/Framework/Messages/MessageType.h	Tue Sep 18 18:04:53 2018 +0200
@@ -53,6 +53,8 @@
 
     // used in unit tests only
     MessageType_Test1,
-    MessageType_Test2
+    MessageType_Test2,
+
+    MessageType_CustomMessage // Custom messages ids ust be greater than this (this one must remain in last position)
   };
 }
--- a/Framework/Messages/Promise.h	Tue Sep 18 15:34:28 2018 +0200
+++ b/Framework/Messages/Promise.h	Tue Sep 18 18:04:53 2018 +0200
@@ -26,6 +26,7 @@
 #include "IMessage.h"
 
 #include <boost/noncopyable.hpp>
+#include <memory>
 
 namespace OrthancStone {
 
@@ -34,14 +35,12 @@
   protected:
     MessageBroker&                    broker_;
 
-    ICallable* successCallable_;
-    ICallable* failureCallable_;
+    std::auto_ptr<ICallable> successCallable_;
+    std::auto_ptr<ICallable> failureCallable_;
 
   public:
     Promise(MessageBroker& broker)
-      : broker_(broker),
-        successCallable_(NULL),
-        failureCallable_(NULL)
+      : broker_(broker)
     {
     }
 
@@ -65,21 +64,21 @@
 
     Promise& Then(ICallable* successCallable)
     {
-      if (successCallable_ != NULL)
+      if (successCallable_.get() != NULL)
       {
         // TODO: throw throw new "Promise may only have a single success target"
       }
-      successCallable_ = successCallable;
+      successCallable_.reset(successCallable);
       return *this;
     }
 
     Promise& Else(ICallable* failureCallable)
     {
-      if (failureCallable_ != NULL)
+      if (failureCallable_.get() != NULL)
       {
         // TODO: throw throw new "Promise may only have a single failure target"
       }
-      failureCallable_ = failureCallable;
+      failureCallable_.reset(failureCallable);
       return *this;
     }
 
--- a/Resources/CMake/OrthancStoneConfiguration.cmake	Tue Sep 18 15:34:28 2018 +0200
+++ b/Resources/CMake/OrthancStoneConfiguration.cmake	Tue Sep 18 18:04:53 2018 +0200
@@ -285,7 +285,7 @@
   ${ORTHANC_STONE_ROOT}/Framework/Messages/IObservable.h
   ${ORTHANC_STONE_ROOT}/Framework/Messages/IObserver.h
   ${ORTHANC_STONE_ROOT}/Framework/Messages/MessageBroker.h
-  ${ORTHANC_STONE_ROOT}/Framework/Messages/MessageForwarder.h
+  ${ORTHANC_STONE_ROOT}/Framework/Messages/MessageForwarder.cpp
   ${ORTHANC_STONE_ROOT}/Framework/Messages/MessageType.h
   ${ORTHANC_STONE_ROOT}/Framework/Messages/Promise.h
 
--- a/UnitTestsSources/TestMessageBroker2.cpp	Tue Sep 18 15:34:28 2018 +0200
+++ b/UnitTestsSources/TestMessageBroker2.cpp	Tue Sep 18 18:04:53 2018 +0200
@@ -286,22 +286,22 @@
 //    }
 
 //  };
+  using namespace OrthancStone;
 
 
-//  enum CustomMessageType
-//  {
-//    CustomMessageType_First = MessageType_LastGenericStoneMessage + 1,
+  enum CustomMessageType
+  {
+    CustomMessageType_First = MessageType_CustomMessage + 1,
 
-//    CustomMessageType_Completed,
-//    CustomMessageType_Increment
-//  };
+    CustomMessageType_Completed,
+    CustomMessageType_Increment
+  };
 
-  using namespace OrthancStone;
 
   class MyObservable : public IObservable
   {
   public:
-    struct MyCustomMessage: public BaseMessage<MessageType_Test1>
+    struct MyCustomMessage: public BaseMessage<CustomMessageType_Completed>
     {
       int payload_;
 
@@ -467,6 +467,46 @@
 }
 
 
+TEST(MessageBroker2, TestMessageForwarderDeleteIntermediate)
+{
+  MessageBroker broker;
+  MyObservable  observable(broker);
+  MyIntermediate* intermediate = new MyIntermediate(broker, observable);
+  MyObserver    observer(broker);
+
+  // 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.EmitMessage(MyObservable::MyCustomMessage(12));
+  ASSERT_EQ(12, testCounter);
+
+  delete intermediate;
+
+  observable.EmitMessage(MyObservable::MyCustomMessage(20));
+  ASSERT_EQ(12, testCounter);
+}
+
+TEST(MessageBroker2, TestCustomMessage)
+{
+  MessageBroker broker;
+  MyObservable  observable(broker);
+  MyIntermediate intermediate(broker, observable);
+  MyObserver    observer(broker);
+
+  // 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.EmitMessage(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.EmitMessage(MyObservable::MyCustomMessage(20));
+  ASSERT_EQ(20, testCounter);
+}
+
 
 TEST(MessageBroker2, TestPromiseSuccessFailure)
 {