feat(NotificationCenter): g++ build and refactoring #4414

Author: aleks-f

Foundation/include/Poco/AbstractObserver.h

@@ -37,8 +37,14 @@ class Foundation_API AbstractObserver
 	AbstractObserver& operator = (const AbstractObserver& observer);
 
 	virtual void notify(Notification* pNf) const = 0;
+
 	virtual bool equals(const AbstractObserver& observer) const = 0;
-	virtual bool accepts(Notification* pNf, const char* pName = 0) const = 0;
+
+	[[deprecated("use `Poco::Any accepts(Notification*)` instead")]]
+	virtual bool accepts(Notification* pNf, const char* pName) const = 0;
+
+	virtual bool accepts(const Notification::Ptr& pNf) const = 0;
+
 	virtual AbstractObserver* clone() const = 0;
 
 	virtual void start();

Foundation/include/Poco/AsyncObserver.h

@@ -23,6 +23,7 @@
 #include "Poco/Thread.h"
 #include "Poco/Stopwatch.h"
 #include "Poco/Debugger.h"
+#include "Poco/ErrorHandler.h"
 #include "Poco/Format.h"
 #include "Poco/RunnableAdapter.h"
 #include "Poco/NotificationQueue.h"
@@ -44,22 +45,22 @@ class AsyncObserver: public NObserver<C, N>
 {
 public:
 	using Type = AsyncObserver<C, N>;
-	using Matcher = bool (C::*)(const std::string&);
+	using Matcher = typename NObserver<C, N>::Matcher;
+	using Handler = typename NObserver<C, N>::Handler;
+	using NotificationPtr = typename NObserver<C, N>::NotificationPtr;
 
 	AsyncObserver() = delete;
 
 	AsyncObserver(C& object, Handler handler, Matcher matcher = nullptr):
-		NObserver(object, handler),
-		_matcher(matcher),
+		NObserver<C, N>(object, handler, matcher),
 		_ra(*this, &AsyncObserver::dequeue),
 		_started(false),
 		_done(false)
 	{
 	}
 
 	AsyncObserver(const AsyncObserver& observer):
-		NObserver(observer),
-		_matcher(observer._matcher),
+		NObserver<C, N>(observer),
 		_ra(*this, &AsyncObserver::dequeue),
 		_started(false),
 		_done(false)
@@ -77,45 +78,18 @@ class AsyncObserver: public NObserver<C, N>
 		if (&observer != this)
 		{
 			poco_assert(observer._nq.size() == 0);
-			_pObject = observer._pObject;
-			_handler  = observer._handler;
-			_matcher = observer._matcher;
+			setObject(observer._pObject);
+			setHandler(observer._handler);
+			setMatcher(observer._matcher);
 			_started = false;
 			_done =false;
 		}
 		return *this;
 	}
 
-	void notify(Notification* pNf) const
+	virtual void notify(Notification* pNf) const
 	{
-		Poco::ScopedLockWithUnlock l(_mutex);
-		if (_pObject)
-		{
-			if (!_matcher || (_pObject->*_matcher)(pNf->name()))
-			{
-				l.unlock();
-				N* pCastNf = dynamic_cast<N*>(pNf);
-				if (pCastNf)
-				{
-					NotificationPtr ptr(pCastNf, true);
-					_nq.enqueueNotification(ptr);
-				}
-			}
-		}
-	}
-
-	virtual bool equals(const AbstractObserver& abstractObserver) const
-	{
-		const AsyncObserver* pObs = dynamic_cast<const AsyncObserver*>(&abstractObserver);
-		return pObs && pObs->_pObject == _pObject && pObs->_method == _method && pObs->_matcher == _matcher;
-	}
-
-	virtual bool accepts(Notification* pNf, const char* pName = nullptr) const
-	{
-		if (!dynamic_cast<N*>(pNf)) return false;
-
-		Poco::ScopedLock l(_mutex);
-		return _pObject && _matcher && (_pObject->*_matcher)(pNf->name());
+		_nq.enqueueNotification(NotificationPtr(pNf, true));
 	}
 
 	virtual AbstractObserver* clone() const
@@ -125,7 +99,7 @@ class AsyncObserver: public NObserver<C, N>
 
 	virtual void start()
 	{
-		Poco::ScopedLock l(_mutex);
+		Poco::ScopedLock l(this->mutex());
 		if (_started)
 		{
 			throw Poco::InvalidAccessException(
@@ -149,7 +123,7 @@ class AsyncObserver: public NObserver<C, N>
 		_nq.wakeUpAll();
 		while (!_done) Thread::sleep(100);
 		_thread.join();
-		NObserver::disable();
+		NObserver<C, N>::disable();
 	}
 
 	virtual int backlog() const
@@ -162,19 +136,25 @@ class AsyncObserver: public NObserver<C, N>
 	{
 		NotificationPtr pNf;
 		_started = true;
+		_done = false;
 		while (pNf = _nq.waitDequeueNotification())
 		{
 			try
 			{
-				_mutex.lock();
-				try
-				{
-					(_pObject->*_method)(pNf);
-					_mutex.unlock();
-				}
-				catch (Poco::SystemException&) { break; }
-				catch (...) { _mutex.unlock(); }
-			} catch (Poco::SystemException&) {}
+				this->handle(pNf);
+			}
+			catch (Poco::Exception& ex)
+			{
+				Poco::ErrorHandler::handle(ex);
+			}
+			catch (std::exception& ex)
+			{
+				Poco::ErrorHandler::handle(ex);
+			}
+			catch (...)
+			{
+				Poco::ErrorHandler::handle();
+			}
 		}
 		_done = true;
 		_started = false;
@@ -183,7 +163,6 @@ class AsyncObserver: public NObserver<C, N>
 	using Adapter = RunnableAdapter<AsyncObserver<C, N>>;
 
 	Thread _thread;
-	Matcher _matcher;
 	mutable NotificationQueue _nq;
 	Adapter _ra;
 	std::atomic<bool> _started;

Foundation/include/Poco/NObserver.h

@@ -37,29 +37,39 @@ class NObserver: public AbstractObserver
 	/// to use this template class.
 	///
 	/// This class template is quite similar to the Observer class
-	/// template. The only difference is that the NObserver
-	/// expects the callback function to accept a const AutoPtr&
-	/// instead of a plain pointer as argument, thus simplifying memory
-	/// management.
+	/// template. The differences are:
+	///
+	///   - NObserver expects the callback function to accept a const AutoPtr&
+	///     instead of a plain pointer as argument, thus simplifying memory
+	///     management.
+	///
+	///   - In addition to dispatching notifications based on the Notification runtime
+	///     type, NObserver can also notify subscribers based on the Notification name.
+	///     To enable this functionality, a matcher function must be provided.
+	///     Null matcher means no matching is performed and all notificiations
+	///     of the type subscribed to are dispatched.
 {
 public:
 	using Type = NObserver<C, N>;
 	using NotificationPtr = AutoPtr<N>;
 	using Callback = void (C::*)(const NotificationPtr&);
 	using Handler = Callback;
+	using Matcher = bool (C::*)(const std::string&) const;
 
 	NObserver() = delete;
 
-	NObserver(C& object, Callback method):
+	NObserver(C& object, Handler method, Matcher matcher = nullptr):
 		_pObject(&object),
-		_method(method)
+		_handler(method),
+		_matcher(matcher)
 	{
 	}
 
 	NObserver(const NObserver& observer):
 		AbstractObserver(observer),
 		_pObject(observer._pObject),
-		_method(observer._method)
+		_handler(observer._handler),
+		_matcher(observer._matcher)
 	{
 	}
 
@@ -72,35 +82,35 @@ class NObserver: public AbstractObserver
 		if (&observer != this)
 		{
 			_pObject = observer._pObject;
-			_method  = observer._method;
+			_handler  = observer._handler;
+			_matcher  = observer._matcher;
 		}
 		return *this;
 	}
 
 	virtual void notify(Notification* pNf) const
 	{
 		Poco::Mutex::ScopedLock lock(_mutex);
-
 		if (_pObject)
-		{
-			N* pCastNf = dynamic_cast<N*>(pNf);
-			if (pCastNf)
-			{
-				NotificationPtr ptr(pCastNf, true);
-				(_pObject->*_method)(ptr);
-			}
-		}
+			(_pObject->*_handler)(NotificationPtr(pNf, true));
 	}
 
 	virtual bool equals(const AbstractObserver& abstractObserver) const
 	{
 		const NObserver* pObs = dynamic_cast<const NObserver*>(&abstractObserver);
-		return pObs && pObs->_pObject == _pObject && pObs->_method == _method;
+		return pObs && pObs->_pObject == _pObject && pObs->_handler == _handler && pObs->_matcher == _matcher;
 	}
 
-	virtual bool accepts(Notification* pNf, const char* pName = nullptr) const
+	[[deprecated("use `bool accepts(const Notification::Ptr&)` instead")]]
+	virtual bool accepts(Notification* pNf, const char* pName) const
 	{
-		return dynamic_cast<N*>(pNf) && (!pName || pNf->name() == pName);
+		return (!pName || pNf->name() == pName) && dynamic_cast<N*>(pNf) != nullptr;
+	}
+
+	virtual bool accepts(const Notification::Ptr& pNf) const
+	{
+		if(match(pNf)) return pNf.cast<N>() != nullptr;
+		return false;
 	}
 
 	virtual AbstractObserver* clone() const
@@ -116,8 +126,31 @@ class NObserver: public AbstractObserver
 	}
 
 protected:
+
+	void handle(const NotificationPtr& ptr)
+	{
+		Mutex::ScopedLock lock(_mutex);
+
+		if (_pObject)
+			(_pObject->*_handler)(ptr);
+	}
+
+	bool match(const NotificationPtr& ptr) const
+	{
+		Mutex::ScopedLock l(_mutex);
+
+		return _pObject && (!_matcher || (_pObject->*_matcher)(ptr->name()));
+	}
+
+	Mutex& mutex() const
+	{
+		return _mutex;
+	}
+
+private:
 	C*       _pObject;
-	Callback _method;
+	Callback _handler;
+	Matcher _matcher;
 	mutable Poco::Mutex _mutex;
 };
 

Foundation/include/Poco/NotificationCenter.h

@@ -132,10 +132,10 @@ class Foundation_API NotificationCenter
 		/// NotificationCenter.
 
 private:
-	typedef SharedPtr<AbstractObserver> AbstractObserverPtr;
-	typedef std::vector<AbstractObserverPtr> ObserverList;
+	using AbstractObserverPtr = SharedPtr<AbstractObserver>;
+	using ObserverList = std::vector<AbstractObserverPtr>;
 
-	ObserverList observersToNotify(Notification::Ptr pNotification) const;
+	ObserverList observersToNotify(const Notification::Ptr& pNotification) const;
 
 	ObserverList  _observers;
 	mutable Mutex _mutex;

Foundation/include/Poco/Observer.h

@@ -74,15 +74,10 @@ class Observer: public AbstractObserver
 	void notify(Notification* pNf) const
 	{
 		Poco::Mutex::ScopedLock lock(_mutex);
-
 		if (_pObject)
 		{
-			N* pCastNf = dynamic_cast<N*>(pNf);
-			if (pCastNf)
-			{
-				pCastNf->duplicate();
-				(_pObject->*_method)(pCastNf);
-			}
+			pNf->duplicate();
+			(_pObject->*_method)(static_cast<N*>(pNf));
 		}
 	}
 
@@ -92,9 +87,15 @@ class Observer: public AbstractObserver
 		return pObs && pObs->_pObject == _pObject && pObs->_method == _method;
 	}
 
-	bool accepts(Notification* pNf, const char* pName = 0) const
+	[[deprecated("use `bool accepts(const Notification::Ptr&)` instead")]]
+	bool accepts(Notification* pNf, const char* pName) const
+	{
+		return (!pName || pNf->name() == pName) && (dynamic_cast<N*>(pNf) != nullptr);
+	}
+
+	bool accepts(const Notification::Ptr& pNf) const
 	{
-		return dynamic_cast<N*>(pNf) && (!pName || pNf->name() == pName);
+		return (pNf.cast<N>() != nullptr);
 	}
 
 	AbstractObserver* clone() const

Foundation/src/NotificationCenter.cpp

@@ -76,7 +76,7 @@ bool NotificationCenter::hasObserver(const AbstractObserver& observer) const
 }
 
 
-NotificationCenter::ObserverList NotificationCenter::observersToNotify(Notification::Ptr pNotification) const
+NotificationCenter::ObserverList NotificationCenter::observersToNotify(const Notification::Ptr& pNotification) const
 {
 	ObserverList ret;
 	ScopedLock<Mutex> lock(_mutex);

Foundation/testsuite/src/NotificationCenterTest.cpp

@@ -31,7 +31,10 @@ class TestNotification: public Notification
 };
 
 
-NotificationCenterTest::NotificationCenterTest(const std::string& name): CppUnit::TestCase(name)
+NotificationCenterTest::NotificationCenterTest(const std::string& name):
+	CppUnit::TestCase(name),
+	_handle1Done(false),
+	_handle2Done(false)
 {
 }
 
@@ -162,11 +165,13 @@ void NotificationCenterTest::testNotificationCenterAsync()
 	nc.postNotification(new Notification("asyncNotification"));
 	nc.postNotification(new Notification("anotherNotification"));
 
-	while (_set.size() < 2) Poco::Thread::sleep(100);
+	while (!_handle1Done || !_handle2Done)
+		Poco::Thread::sleep(100);
 
 	nc.removeObserver(ObserverT(*this, &NotificationCenterTest::handleAsync1));
 	nc.removeObserver(ObserverT(*this, &NotificationCenterTest::handleAsync2));
 
+	Poco::Mutex::ScopedLock l(_mutex);
 	assertTrue(_set.size() == 2);
 	assertTrue(_set.find("handleAsync1") != _set.end());
 	assertTrue(_set.find("handleAsync2") != _set.end());
@@ -226,17 +231,19 @@ void NotificationCenterTest::handleAsync1(const AutoPtr<Notification>& pNf)
 {
 	Poco::Mutex::ScopedLock l(_mutex);
 	_set.insert("handleAsync1");
+	_handle1Done = true;
 }
 
 
 void NotificationCenterTest::handleAsync2(const AutoPtr<Notification>& pNf)
 {
 	Poco::Mutex::ScopedLock l(_mutex);
 	_set.insert("handleAsync2");
+	_handle2Done = true;
 }
 
 
-bool NotificationCenterTest::matchAsync(const std::string& name)
+bool NotificationCenterTest::matchAsync(const std::string& name) const
 {
 	return name.find("asyncNotification") == 0;
 }