From 53701255cadb1fb1e3f29ce0f291297f4fb4f1c2 Mon Sep 17 00:00:00 2001 From: Marius Kittler Date: Thu, 16 Nov 2017 17:54:42 +0100 Subject: [PATCH] WIP: Rework notifications --- mediafileinfo.cpp | 2 +- mp4/mp4container.cpp | 2 +- statusprovider.cpp | 33 ++++++++++++++---- statusprovider.h | 80 ++++++++++++++++++++++++++++---------------- tests/utils.cpp | 2 +- 5 files changed, 81 insertions(+), 38 deletions(-) diff --git a/mediafileinfo.cpp b/mediafileinfo.cpp index e53b6ea..d2ea1c0 100644 --- a/mediafileinfo.cpp +++ b/mediafileinfo.cpp @@ -673,7 +673,7 @@ void MediaFileInfo::applyChanges() if(hasId3v2Tag()) { addNotification(NotificationType::Warning, "Assigned ID3v2 tag can't be attached and will be ignored.", context); } - m_container->forwardStatusUpdateCalls(this); + m_container->forwardStatus(this); m_tracksParsingStatus = ParsingStatus::NotParsedYet; m_tagsParsingStatus = ParsingStatus::NotParsedYet; try { diff --git a/mp4/mp4container.cpp b/mp4/mp4container.cpp index 4ddb834..63f3fac 100644 --- a/mp4/mp4container.cpp +++ b/mp4/mp4container.cpp @@ -685,7 +685,7 @@ calculatePadding: // update status updateStatus("Writing atom: " + level0Atom->idToString()); // copy atom entirely and forward status update calls - level0Atom->forwardStatusUpdateCalls(this); + level0Atom->forwardStatus(this); level0Atom->copyEntirely(outputStream); } } diff --git a/statusprovider.cpp b/statusprovider.cpp index a8cf815..09989e3 100644 --- a/statusprovider.cpp +++ b/statusprovider.cpp @@ -52,6 +52,10 @@ size_t StatusProvider::registerCallback(CallbackFunction callback) */ void StatusProvider::addNotification(const Notification ¬ification) { + if(m_forward) { + m_forward->addNotification(notification); + return; + } m_notifications.push_back(notification); m_worstNotificationType |= notification.type(); invokeCallbacks(); @@ -63,6 +67,10 @@ void StatusProvider::addNotification(const Notification ¬ification) */ void StatusProvider::addNotification(NotificationType type, const string &message, const string &context) { + if(m_forward) { + m_forward->addNotification(type, message, context); + return; + } m_notifications.emplace_back(type, message, context); m_worstNotificationType |= type; invokeCallbacks(); @@ -72,15 +80,19 @@ void StatusProvider::addNotification(NotificationType type, const string &messag * \brief This method is meant to be called by the derived class to add all notifications \a from another * StatusProvider instance. */ -void StatusProvider::addNotifications(const StatusProvider &from) +/*void StatusProvider::addNotifications(const StatusProvider &from) { + if(m_forward) { + m_forward->addNotifications(from); + return; + } if(&from == this) { return; } m_notifications.insert(m_notifications.end(), from.m_notifications.cbegin(), from.m_notifications.cend()); m_worstNotificationType |= from.worstNotificationType(); invokeCallbacks(); -} +}*/ /*! * \brief This method is meant to be called by the derived class to add all notifications \a from another @@ -88,21 +100,29 @@ void StatusProvider::addNotifications(const StatusProvider &from) * * The specified \a higherContext is concatenated with the original context string. */ -void StatusProvider::addNotifications(const string &higherContext, const StatusProvider &from) +/*void StatusProvider::addNotifications(const string &higherContext, const StatusProvider &from) { + if(m_forward) { + m_forward->addNotifications(higherContext, from); + return; + } if(&from == this) { return; } for(const auto ¬ification : from.m_notifications) { addNotification(notification.type(), notification.message(), higherContext % ',' % ' ' + notification.context()); } -} +}*/ /*! * \brief This method is meant to be called by the derived class to add the specified \a notifications. */ -void StatusProvider::addNotifications(const NotificationList ¬ifications) +/*void StatusProvider::addNotifications(const NotificationList ¬ifications) { + if(m_forward) { + m_forward->addNotifications(notifications); + return; + } m_notifications.insert(m_notifications.end(), notifications.cbegin(), notifications.cend()); if(m_worstNotificationType != Notification::worstNotificationType()) { for(const Notification ¬ification : notifications) { @@ -112,6 +132,7 @@ void StatusProvider::addNotifications(const NotificationList ¬ifications) } } invokeCallbacks(); -} +}*/ + } diff --git a/statusprovider.h b/statusprovider.h index fc349a8..9cef5ad 100644 --- a/statusprovider.h +++ b/statusprovider.h @@ -29,7 +29,7 @@ public: size_t registerCallback(CallbackFunction callback); void unregisterCallback(size_t id); void unregisterAllCallbacks(); - void forwardStatusUpdateCalls(StatusProvider *other = nullptr); + void forwardStatus(StatusProvider *other = nullptr); StatusProvider *usedProvider(); void tryToAbort(); bool isAborted() const; @@ -40,9 +40,9 @@ public: void updatePercentage(double percentage); void addNotification(const Notification ¬ification); void addNotification(NotificationType type, const std::string &message, const std::string &context); - void addNotifications(const StatusProvider &from); - void addNotifications(const std::string &higherContext, const StatusProvider &from); - void addNotifications(const NotificationList ¬ifications); + //void addNotifications(const StatusProvider &from); + //void addNotifications(const std::string &higherContext, const StatusProvider &from); + //void addNotifications(const NotificationList ¬ifications); protected: StatusProvider(); @@ -50,15 +50,15 @@ protected: private: void invokeCallbacks(); void updateWorstNotificationType(NotificationType notificationType); - void transferNotifications(StatusProvider &from); + //void transferNotifications(StatusProvider &from); - NotificationList m_notifications; - NotificationType m_worstNotificationType; + StatusProvider *m_forward; std::string m_status; double m_percentage; CallbackVector m_callbacks; + NotificationList m_notifications; + NotificationType m_worstNotificationType; bool m_abort; - StatusProvider *m_forward; }; /*! @@ -66,6 +66,10 @@ private: */ inline void StatusProvider::updateStatus(const std::string &status) { + if(m_forward) { + m_forward->updateStatus(status); + return; + } m_status = status; invokeCallbacks(); } @@ -77,6 +81,10 @@ inline void StatusProvider::updateStatus(const std::string &status) */ inline void StatusProvider::updateStatus(const std::string &status, double percentage) { + if(m_forward) { + m_forward->updateStatus(status, percentage); + return; + } m_status = status; m_percentage = percentage; invokeCallbacks(); @@ -89,6 +97,10 @@ inline void StatusProvider::updateStatus(const std::string &status, double perce */ inline void StatusProvider::updatePercentage(double percentage) { + if(m_forward) { + m_forward->updatePercentage(percentage); + return; + } m_percentage = percentage; invokeCallbacks(); } @@ -96,7 +108,7 @@ inline void StatusProvider::updatePercentage(double percentage) /*! * \brief Returns the provider which callback functions will be called when the status or the percentage is updated. * - * The default is the current instance. This can be changed using the forwardStatusUpdateCalls() method. + * The default is the current instance. This can be changed using the forwardStatus() method. */ inline StatusProvider *StatusProvider::usedProvider() { @@ -160,7 +172,7 @@ inline double StatusProvider::currentPercentage() const /*! * \brief Returns an indication whether the current operation should be aborted. * - * This can be tested when implementing an operation that should be able to be + * This flag can be tested when implementing an operation that should be able to be * aborted. */ inline bool StatusProvider::isAborted() const @@ -192,26 +204,30 @@ inline void StatusProvider::unregisterAllCallbacks() } /*! - * \brief Forwards all status updates calls to the specified \a statusProvider. + * \brief Forwards all status updates calls and notifications to the specified \a statusProvider. * - * Not the callback methods associated to the current instance will be called - * to inform about status updates. Instead the callback methods associated to - * the specified instance will be called. + * This basically forwards status information updates and notifications to the specified instance. * - * The current instance is still the sender. - * - * The current instance is considered as abortet if the specified provider is - * abortet even if tryToAbort() has not been called. - * - * The current instance will return the status and percentage of the specified - * provider if it provides no own status or percentage. - * - * Provide nullptr to revert to the default behaviour. - * - * \remarks Leads to endless recursion if \a statusProvider forwards (indirectly - * or directly) to the current instance. + * \remarks + * - Any notifications will *not* be added to the current instance anymore. Instead the + * notifications will be added to the specified instance. + * - The callback methods assigned to the current instance will *not* be called + * to inform about status updates anymore. Instead the callback methods associated to + * the specified instance will be called. + * - The current instance is still passed as the sender when invoking callback methods. + * - The current instance is also considered as aborted if the specified provider is + * aborted - even if tryToAbort() has not been called on the current instance. + * - The current instance will return the status and percentage of the specified + * provider if it provides no own status or percentage. + * - Provide nullptr to revert to the default behaviour. + * - The methods invalidateStatus() and invalidateNotifications() are *not* forwarded. + * - The methods updateStatus() and updatePercentage() are *not* forwarded. + * - The method transferNotifications() is *not* forwarded. + * - Leads to endless recursion if \a statusProvider forwards (indirectly + * or directly) to the current instance. (So better prevent it.) + * - Set \a other to nullptr to restore the usual behaviour. */ -inline void StatusProvider::forwardStatusUpdateCalls(StatusProvider *other) +inline void StatusProvider::forwardStatus(StatusProvider *other) { m_forward = other; } @@ -272,6 +288,10 @@ inline void StatusProvider::invokeCallbacks() */ inline void StatusProvider::updateWorstNotificationType(NotificationType notificationType) { + if(m_forward) { + m_forward->updateWorstNotificationType(notificationType); + return; + } if(m_worstNotificationType < notificationType) { m_worstNotificationType = notificationType; } @@ -282,12 +302,14 @@ inline void StatusProvider::updateWorstNotificationType(NotificationType notific * \remarks In constrast to the similar addNotifications() overload, this method does not copy the notifications. Instead * the notifications are transfered. It also doesn't check whether \a from is the current instance and doesn't * invoke callbacks. + * \deprecated This method should likely be removed after the status provider rework. */ -inline void StatusProvider::transferNotifications(StatusProvider &from) +/*inline void StatusProvider::transferNotifications(StatusProvider &from) { m_notifications.splice(m_notifications.end(), from.m_notifications); m_worstNotificationType |= from.worstNotificationType(); -} +}*/ + } diff --git a/tests/utils.cpp b/tests/utils.cpp index 7ee3bcb..b8d513e 100644 --- a/tests/utils.cpp +++ b/tests/utils.cpp @@ -156,7 +156,7 @@ void UtilitiesTests::testStatusProvider() // forwarding TestStatusProvider forwardReceiver; - status.forwardStatusUpdateCalls(&forwardReceiver); + status.forwardStatus(&forwardReceiver); statusUpdateReceived = false; forwardReceiver.registerCallback([&status, &statusUpdateReceived] (StatusProvider &sender) { CPPUNIT_ASSERT(&status == &sender);