Mercurial > hg > orthanc-stone
changeset 1067:05b2e71ed145 broker
removed MessageForwarder, unit tests are ok
author | Sebastien Jodogne <s.jodogne@gmail.com> |
---|---|
date | Tue, 15 Oct 2019 18:11:40 +0200 |
parents | b537002f83a9 |
children | 04a95ee91327 |
files | Framework/Messages/IObservable.cpp Framework/Messages/IObservable.h Framework/Messages/MessageForwarder.cpp Framework/Messages/MessageForwarder.h Framework/Messages/ObserverBase.h Framework/Radiography/RadiographyWidget.cpp Framework/Scene2DViewport/MeasureTool.cpp Framework/Scene2DViewport/MeasureTool.h Resources/CMake/OrthancStoneConfiguration.cmake UnitTestsSources/TestMessageBroker.cpp |
diffstat | 10 files changed, 16 insertions(+), 279 deletions(-) [+] |
line wrap: on
line diff
--- 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); - } }
--- 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); }; }
--- 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); - } -}
--- 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); - } - - }; -}
--- 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"); } }
--- 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);
--- 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;
--- 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.
--- 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
--- 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*/