Re #21427: Corrected copy behaviour and added doxygen comments

- Fixed copy behaviour which lead to the original observer getting
notifications aimed at the copy.
- Added doxygen comments.

Framework/Kernel/src/ConfigObserver.cpp

 #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(),

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) {

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: