diff --git a/Framework/Kernel/src/ConfigObserver.cpp b/Framework/Kernel/src/ConfigObserver.cpp index 782e29f6f567f60d89562f2b9452a1402cbe8d78..9edcce92998f98ac40a44d3d6248e06ab249c11a 100644 --- a/Framework/Kernel/src/ConfigObserver.cpp +++ b/Framework/Kernel/src/ConfigObserver.cpp @@ -1,21 +1,43 @@ #include "MantidKernel/ConfigObserver.h" #include "MantidKernel/ConfigService.h" +#include "MantidKernel/System.h" namespace Mantid { namespace Kernel { +/** + * Begins listening to notifications from the global ConfigService. + */ ConfigObserver::ConfigObserver() : m_valueChangeListener(*this, &ConfigObserver::notifyValueChanged) { ConfigService::Instance().addObserver(m_valueChangeListener); } +/** + * Copying a config observer is the same as default constructing one. + * + * It is necessary to create a new listener object with the this pointer + * in order for the correct object to recieve the notification. + * + * @param other The observer to copy. + */ ConfigObserver::ConfigObserver(const ConfigObserver &other) - : m_valueChangeListener(other.m_valueChangeListener) { + : m_valueChangeListener(*this, &ConfigObserver::notifyValueChanged) { + UNUSED_ARG(other); ConfigService::Instance().addObserver(m_valueChangeListener); } +/** + * Nothing to do but we must overload the = operator to prevent a + * default copy which would produce incorrect results. + * + * We don't need to re-register with the ConfigService since it + * is not possible to create a ConfigObserver which is unregistered + * and we only ever unregister in the destructor. + * + * @param other The observer to copy. + */ ConfigObserver &ConfigObserver::operator=(const ConfigObserver &other) { - m_valueChangeListener = other.m_valueChangeListener; - ConfigService::Instance().addObserver(m_valueChangeListener); + UNUSED_ARG(other); return *this; } @@ -23,12 +45,24 @@ ConfigObserver::~ConfigObserver() noexcept { ConfigService::Instance().removeObserver(m_valueChangeListener); } +/** + * Called when a config property's value is changed. + * + * @param name The name of the property which changed. + * @param newValue The new value of the property. + * @param prevValue The old value of the property. + */ void ConfigObserver::notifyValueChanged(const std::string &name, const std::string &newValue, - const std::string &prevValue) { - onValueChanged(name, newValue, prevValue); + const std::string &oldValue) { + onValueChanged(name, newValue, oldValue); } +/** + * Called when a config property's value is changed. + * + * @param notification The Poco notification object. + */ void ConfigObserver::notifyValueChanged( ConfigValChangeNotification_ptr notification) { notifyValueChanged(notification->key(), notification->curValue(), diff --git a/Framework/Kernel/src/ConfigPropertyObserver.cpp b/Framework/Kernel/src/ConfigPropertyObserver.cpp index 7848daddd3bd855e24cece8d3808d2a762a8082e..baa15a84dc9687c21b41525a114c91dd28c596d4 100644 --- a/Framework/Kernel/src/ConfigPropertyObserver.cpp +++ b/Framework/Kernel/src/ConfigPropertyObserver.cpp @@ -3,9 +3,17 @@ namespace Mantid { namespace Kernel { + +/** + * Begins listening for change notifications from the global ConfigService + * concerning the property with the specified name. + */ ConfigPropertyObserver::ConfigPropertyObserver(std::string propertyName) : m_propertyName(std::move(propertyName)) {} +/** + * Filters out change notifications concerning other properties. + */ void ConfigPropertyObserver::onValueChanged(const std::string &name, const std::string &newValue, const std::string &prevValue) { diff --git a/Framework/Kernel/test/ConfigObserverTest.h b/Framework/Kernel/test/ConfigObserverTest.h index 0cccc9b5577aefbbc3e2d024e0099d412b880e3b..139120bb44d22fc37dbfe68bdd762807a51973b7 100644 --- a/Framework/Kernel/test/ConfigObserverTest.h +++ b/Framework/Kernel/test/ConfigObserverTest.h @@ -9,14 +9,14 @@ using namespace Mantid::Kernel; -template <typename Callback> class MockObserver : ConfigObserver { +template <typename Callback> class MockObserver : public ConfigObserver { public: MockObserver(Callback callback) : m_callback(callback) {} protected: void onValueChanged(const std::string &name, const std::string &newValue, const std::string &prevValue) override { - m_callback(name, newValue, prevValue); + m_callback(name, newValue, prevValue, static_cast<ConfigObserver*>(this)); } private: @@ -45,34 +45,40 @@ public: } void testRecievesCallbackForOutputDirectoryChange() { - auto call_count = 0; + auto callCount = 0; auto constexpr NUMBER_OF_PROPERTIES_CHANGED = 2; auto observer = makeMockObserver( - [&call_count](const std::string &name, const std::string &newValue, - const std::string &prevValue) -> void { + [&callCount](const std::string &name, const std::string &newValue, + const std::string &prevValue, ConfigObserver* self) -> void { UNUSED_ARG(name); UNUSED_ARG(newValue); UNUSED_ARG(prevValue); - call_count++; + UNUSED_ARG(self); + callCount++; }); ConfigService::Instance().setString("defaultsave.directory", "/dev/null"); - TS_ASSERT_EQUALS(NUMBER_OF_PROPERTIES_CHANGED, call_count); + TS_ASSERT_EQUALS(NUMBER_OF_PROPERTIES_CHANGED, callCount); } - void testCopysObserverOnCopyConstruction() { - auto call_count = 0; - auto constexpr NUMBER_OF_PROPERTIES_CHANGED = 2; + void testCreatesNewObserverOnCopyConstruction() { + ConfigObserver* lastCaller = nullptr; + auto callCount = 0; + auto constexpr NUMBER_OF_PROPERTIES_CHANGED = 1; auto observer = makeMockObserver( - [&call_count](const std::string &name, const std::string &newValue, - const std::string &prevValue) -> void { + [&callCount, &lastCaller](const std::string &name, const std::string &newValue, + const std::string &prevValue, ConfigObserver* self) -> void { UNUSED_ARG(name); UNUSED_ARG(newValue); UNUSED_ARG(prevValue); - call_count++; + callCount++; + if (lastCaller == nullptr) + lastCaller = self; + else + TSM_ASSERT("The same observer was notified twice", lastCaller != self); }); auto copyOfObserver = observer; - ConfigService::Instance().setString("defaultsave.directory", "/dev/null"); - TS_ASSERT_EQUALS(NUMBER_OF_PROPERTIES_CHANGED * 2, call_count); + ConfigService::Instance().setString("datasearch.directories", "/dev/null"); + TS_ASSERT_EQUALS(NUMBER_OF_PROPERTIES_CHANGED * 2, callCount); } private: