# HG changeset patch # User am@osimis.io # Date 1537286693 -7200 # Node ID 4a79193ffb58bda4940a7ce7122e22e4fe29ea2b # Parent 547e1cf7aa7b3dcd70191cab7cc0c9f3e88a4bb8 support for custom messages + no leaks in unit-tests diff -r 547e1cf7aa7b -r 4a79193ffb58 Framework/Messages/IMessage.h --- 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 + template struct BaseMessage : public IMessage { enum @@ -54,7 +54,7 @@ }; BaseMessage() - : IMessage(static_cast(Type)) + : IMessage(static_cast(Type)) {} }; diff -r 547e1cf7aa7b -r 4a79193ffb58 Framework/Messages/IObservable.h --- 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 #include + #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 > Callables; Callables callables_; + typedef std::set 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(**it)); + } } void RegisterObserverCallback(ICallable* callable) @@ -85,6 +99,11 @@ } } + void RegisterForwarder(IMessageForwarder* forwarder) + { + forwarders_.insert(forwarder); + } + }; } diff -r 547e1cf7aa7b -r 4a79193ffb58 Framework/Messages/MessageForwarder.cpp --- /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 . + **/ + + +#include "MessageForwarder.h" + +#include "IObservable.h" + +namespace OrthancStone +{ + + void IMessageForwarder::ForwardMessageInternal(const IMessage& message) + { + emitter_.EmitMessage(message); + } + + void IMessageForwarder::RegisterForwarderInEmitter() + { + emitter_.RegisterForwarder(this); + } +} diff -r 547e1cf7aa7b -r 4a79193ffb58 Framework/Messages/MessageForwarder.h --- 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 -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(broker, *this) // where this is B + * + * in C: + * B.RegisterObserverCallback(new Callable(*this, &B::MyCallback)) // where this is C + */ template - class MessageForwarder : public IObserver, public Callable, TMessage> + class MessageForwarder : public IMessageForwarder, public IObserver, public Callable, 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, TMessage>(*this, &MessageForwarder::ForwardMessage), - observable_(observable) + : IMessageForwarder(emitter), + IObserver(broker), + Callable, TMessage>(*this, &MessageForwarder::ForwardMessage) { + RegisterForwarderInEmitter(); } - protected: +protected: void ForwardMessage(const TMessage& message) { - observable_.EmitMessage(message); + ForwardMessageInternal(message); } + }; } diff -r 547e1cf7aa7b -r 4a79193ffb58 Framework/Messages/MessageType.h --- 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) }; } diff -r 547e1cf7aa7b -r 4a79193ffb58 Framework/Messages/Promise.h --- 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 +#include namespace OrthancStone { @@ -34,14 +35,12 @@ protected: MessageBroker& broker_; - ICallable* successCallable_; - ICallable* failureCallable_; + std::auto_ptr successCallable_; + std::auto_ptr 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; } diff -r 547e1cf7aa7b -r 4a79193ffb58 Resources/CMake/OrthancStoneConfiguration.cmake --- 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 diff -r 547e1cf7aa7b -r 4a79193ffb58 UnitTestsSources/TestMessageBroker2.cpp --- 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 + struct MyCustomMessage: public BaseMessage { 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(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(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) {