Improve filter API of QOpcUaMonitoringParameters

This patch removes the generic setFilter() method and adds setters for
the two acceptable types.
The filter result is now stored in a separate data member.

Change-Id: I0e93a1a30448d00d420d60e5119f09fa46f423d6
Reviewed-by: Alex Blasche <alexander.blasche@qt.io>

9 files changed, 134 insertions, 34 deletions

diff --git a/src/opcua/client/qopcuamonitoringparameters.cpp b/src/opcua/client/qopcuamonitoringparameters.cpp
index e4cb93a..70e056a 100644
--- a/src/opcua/client/qopcuamonitoringparameters.cpp
+++ b/src/opcua/client/qopcuamonitoringparameters.cpp
@@ -451,7 +451,8 @@ void QOpcUaMonitoringParameters::setQueueSize(quint32 queueSize)
 }
 
 /*!
-    Returns the filter result. Empty for DataChangeFilter.
+    Returns the current filter.
+    \sa setFilter()
 */
 QVariant QOpcUaMonitoringParameters::filter() const
 {
@@ -459,33 +460,75 @@ QVariant QOpcUaMonitoringParameters::filter() const
 }
 
 /*!
-    Request \l DataChangeFilter \a filter as filter for the monitored item.
-    \sa setFilter() setEventFilter()
+    Sets \l DataChangeFilter \a filter as filter for the monitored item.
+    If another data change filter or an event filter is present, it will be replaced.
+
+    If the server does not accept the filter, this will be indicated by the
+    status code after the \l enableMonitoring() request has finished.
+
+    \sa filter()
 */
-void QOpcUaMonitoringParameters::setDataChangeFilter(const QOpcUaMonitoringParameters::DataChangeFilter &filter)
+void QOpcUaMonitoringParameters::setFilter(const QOpcUaMonitoringParameters::DataChangeFilter &filter)
 {
     d_ptr->filter = QVariant::fromValue(filter);
 }
 
 /*!
     Request \a eventFilter as filter for the monitored item.
-    \sa setFilter() setDataChangeFilter()
+    If another event filter or a data change filter is present, it will be replaced.
+    If the server does not accept the filter, this will be indicated by the
+    status code and the event filter result after the \l enableMonitoring()
+    request has finished.
+
+    \sa filter()
 */
-void QOpcUaMonitoringParameters::setEventFilter(const EventFilter &eventFilter)
+void QOpcUaMonitoringParameters::setFilter(const EventFilter &eventFilter)
 {
     d_ptr->filter = QVariant::fromValue(eventFilter);
 }
 
 /*!
-    Request \a filter as filter for the monitored item.
+    Removes the current filter from the monitoring parameters.
+
+    \sa filter() setFilter()
+*/
+void QOpcUaMonitoringParameters::clearFilter()
+{
+    d_ptr->filter.clear();
+}
+
+/*!
+    Returns the filter result.
+
+    This value is empty for an attribute monitoring. In case of an event monitoring,
+    the filter result can be empty if the server did not detect any errors in the filter.
+*/
+QVariant QOpcUaMonitoringParameters::filterResult() const
+{
+    return d_ptr->filterResult;
+}
+
+/*!
+    Sets the event filter result to \a eventFilterResult.
 
-    For general use, the type-safe versions that are listed below are preferred.
+    This method must only be used by the backend, setting an event filter result as a user
+    does not have any effect.
 
-    \sa setDataChangeFilter() setEventFilter()
+    \sa filterResult()
 */
-void QOpcUaMonitoringParameters::setFilter(const QVariant &filter)
+void QOpcUaMonitoringParameters::setFilterResult(const QOpcUa::QEventFilterResult &eventFilterResult)
 {
-    d_ptr->filter = filter;
+    d_ptr->filterResult = QVariant::fromValue(eventFilterResult);
+}
+
+/*!
+    Removes the current filter result from the monitoring parameters.
+
+    \sa filterResult() setFilterResult()
+*/
+void QOpcUaMonitoringParameters::clearFilterResult()
+{
+    d_ptr->filterResult.clear();
 }
 
 /*!
@@ -731,6 +774,14 @@ QOpcUaMonitoringParameters::EventFilter &QOpcUaMonitoringParameters::EventFilter
 }
 
 /*!
+    Returns \c true if this event filter has the same value as \a rhs.
+*/
+bool QOpcUaMonitoringParameters::EventFilter::operator==(const QOpcUaMonitoringParameters::EventFilter &rhs) const
+{
+    return selectClauses() == rhs.selectClauses() && whereClause() == rhs.whereClause();
+}
+
+/*!
     Adds the content filter element \a whereClauseElement to the where clause of this event filter.
 */
 QOpcUaMonitoringParameters::EventFilter &QOpcUaMonitoringParameters::EventFilter::operator<<(const QOpcUa::QContentFilterElement &whereClauseElement)
diff --git a/src/opcua/client/qopcuamonitoringparameters.h b/src/opcua/client/qopcuamonitoringparameters.h
index 3ce4729..411e567 100644
--- a/src/opcua/client/qopcuamonitoringparameters.h
+++ b/src/opcua/client/qopcuamonitoringparameters.h
@@ -124,6 +124,7 @@ public:
         EventFilter(const EventFilter &);
         EventFilter &operator=(const EventFilter &);
         operator QVariant const();
+        bool operator==(const QOpcUaMonitoringParameters::EventFilter &rhs) const;
         EventFilter &operator<<(const QOpcUa::QContentFilterElement &whereClauseElement);
         EventFilter &operator<<(const QOpcUa::QSimpleAttributeOperand &selectClauseElement);
         ~EventFilter();
@@ -149,9 +150,12 @@ public:
     double samplingInterval() const;
     void setSamplingInterval(double samplingInterval);
     QVariant filter() const;
-    void setDataChangeFilter(const QOpcUaMonitoringParameters::DataChangeFilter &filter);
-    void setEventFilter(const QOpcUaMonitoringParameters::EventFilter &eventFilter);
-    void setFilter(const QVariant &filter);
+    void setFilter(const QOpcUaMonitoringParameters::DataChangeFilter &filter);
+    void setFilter(const QOpcUaMonitoringParameters::EventFilter &eventFilter);
+    void clearFilter();
+    QVariant filterResult() const;
+    void setFilterResult(const QOpcUa::QEventFilterResult &eventFilterResult);
+    void clearFilterResult();
     quint32 queueSize() const;
     void setQueueSize(quint32 queueSize);
     bool discardOldest() const;
diff --git a/src/opcua/client/qopcuamonitoringparameters_p.h b/src/opcua/client/qopcuamonitoringparameters_p.h
index cb05e4e..589dce3 100644
--- a/src/opcua/client/qopcuamonitoringparameters_p.h
+++ b/src/opcua/client/qopcuamonitoringparameters_p.h
@@ -78,6 +78,7 @@ public:
     // MonitoredItem
     double samplingInterval;
     QVariant filter;
+    QVariant filterResult;
     quint32 queueSize;
     bool discardOldest;
     QOpcUaMonitoringParameters::MonitoringMode monitoringMode;
diff --git a/src/opcua/client/qopcuanode_p.h b/src/opcua/client/qopcuanode_p.h
index bd61c09..d78aeb9 100644
--- a/src/opcua/client/qopcuanode_p.h
+++ b/src/opcua/client/qopcuanode_p.h
@@ -149,8 +149,18 @@ public:
                     it->setPriority(param.priority());
                 if (items & QOpcUaMonitoringParameters::Parameter::SamplingInterval)
                     it->setSamplingInterval(param.samplingInterval());
-                if (items & QOpcUaMonitoringParameters::Parameter::Filter)
-                    it->setFilter(param.filter());
+                if (items & QOpcUaMonitoringParameters::Parameter::Filter) {
+                    if (param.filter().canConvert<QOpcUaMonitoringParameters::DataChangeFilter>())
+                        it->setFilter(param.filter().value<QOpcUaMonitoringParameters::DataChangeFilter>());
+                    else if (param.filter().canConvert<QOpcUaMonitoringParameters::EventFilter>())
+                        it->setFilter(param.filter().value<QOpcUaMonitoringParameters::EventFilter>());
+                    else if (param.filter().isNull())
+                        it->clearFilter();
+                    if (param.filterResult().canConvert<QOpcUa::QEventFilterResult>())
+                        it->setFilterResult(param.filterResult().value<QOpcUa::QEventFilterResult>());
+                    else if (param.filterResult().isNull())
+                        it->clearFilterResult();
+                }
                 if (items & QOpcUaMonitoringParameters::Parameter::QueueSize)
                     it->setQueueSize(param.queueSize());
                 if (items & QOpcUaMonitoringParameters::Parameter::DiscardOldest)
diff --git a/src/opcua/client/qopcuatype.cpp b/src/opcua/client/qopcuatype.cpp
index 4d97802..1416a26 100644
--- a/src/opcua/client/qopcuatype.cpp
+++ b/src/opcua/client/qopcuatype.cpp
@@ -2208,6 +2208,14 @@ QOpcUa::QContentFilterElement &QOpcUa::QContentFilterElement::operator=(const QO
 }
 
 /*!
+    Returns \c true if this content filter element has the same value as \a rhs.
+*/
+bool QOpcUa::QContentFilterElement::operator==(const QOpcUa::QContentFilterElement &rhs) const
+{
+    return filterOperator() == rhs.filterOperator() && filterOperands() == rhs.filterOperands();
+}
+
+/*!
     Converts this content filter element to \l QVariant.
 */
 QOpcUa::QContentFilterElement::operator QVariant() const
@@ -2576,6 +2584,15 @@ QOpcUa::QSimpleAttributeOperand &QOpcUa::QSimpleAttributeOperand::operator=(cons
 }
 
 /*!
+    Returns \c true if this simple attribute operand has the same value as \a rhs.
+*/
+bool QOpcUa::QSimpleAttributeOperand::operator==(const QOpcUa::QSimpleAttributeOperand &rhs) const
+{
+    return attributeId() == rhs.attributeId() && browsePath() == rhs.browsePath() &&
+            indexRange() == rhs.indexRange() && typeId() == rhs.typeId();
+}
+
+/*!
     Converts this simple attribute operand to \l QVariant.
 */
 QOpcUa::QSimpleAttributeOperand::operator QVariant() const
diff --git a/src/opcua/client/qopcuatype.h b/src/opcua/client/qopcuatype.h
index 1aea724..f64ebe3 100644
--- a/src/opcua/client/qopcuatype.h
+++ b/src/opcua/client/qopcuatype.h
@@ -825,6 +825,7 @@ public:
     QSimpleAttributeOperand(QOpcUa::NodeAttribute attributeId,
                             const QString &typeId = QStringLiteral("ns=0;i=2041")); // BaseEventType
     QSimpleAttributeOperand &operator=(const QOpcUa::QSimpleAttributeOperand &);
+    bool operator==(const QSimpleAttributeOperand &rhs) const;
     operator QVariant() const;
     ~QSimpleAttributeOperand();
 
@@ -883,6 +884,7 @@ public:
     QContentFilterElement();
     QContentFilterElement(const QContentFilterElement &);
     QContentFilterElement &operator=(const QContentFilterElement &);
+    bool operator==(const QContentFilterElement &rhs) const;
     operator QVariant() const;
     ~QContentFilterElement();
 
diff --git a/src/plugins/opcua/open62541/qopen62541subscription.cpp b/src/plugins/opcua/open62541/qopen62541subscription.cpp
index 8ab7512..cf8ac3f 100644
--- a/src/plugins/opcua/open62541/qopen62541subscription.cpp
+++ b/src/plugins/opcua/open62541/qopen62541subscription.cpp
@@ -304,10 +304,11 @@ bool QOpen62541Subscription::addAttributeMonitoredItem(quint64 handle, QOpcUa::N
     temp->parameters = s;
     temp->clientHandle = m_clientHandle;
 
-    if (settings.filter().canConvert<QOpcUaMonitoringParameters::EventFilter>())
-        s.setFilter(QVariant::fromValue(convertEventFilterResult(&res.filterResult)));
+    if (res.filterResult.encoding >= UA_EXTENSIONOBJECT_DECODED &&
+            res.filterResult.content.decoded.type == &UA_TYPES[UA_TYPES_EVENTFILTERRESULT])
+        s.setFilterResult(convertEventFilterResult(&res.filterResult));
     else
-        s.setFilter(QVariant());
+        s.clearFilterResult();
 
     emit m_backend->monitoringEnableDisable(handle, attr, true, s);
 
@@ -830,13 +831,18 @@ bool QOpen62541Subscription::modifyMonitoredItemParameters(quint64 handle, QOpcU
                 changed |= QOpcUaMonitoringParameters::Parameter::DiscardOldest;
             }
 
-            if (res.results[0].filterResult.content.decoded.type == &UA_TYPES[UA_TYPES_EVENTFILTERRESULT]) {
-                if (item == QOpcUaMonitoringParameters::Parameter::Filter)
-                    changed |= QOpcUaMonitoringParameters::Parameter::Filter;
-                p.setFilter(QVariant::fromValue(convertEventFilterResult(&res.results[0].filterResult)));
+            if (item == QOpcUaMonitoringParameters::Parameter::Filter) {
+                changed |= QOpcUaMonitoringParameters::Parameter::Filter;
+                if (value.canConvert<QOpcUaMonitoringParameters::DataChangeFilter>())
+                    p.setFilter(value.value<QOpcUaMonitoringParameters::DataChangeFilter>());
+                else if (value.canConvert<QOpcUaMonitoringParameters::EventFilter>())
+                    p.setFilter(value.value<QOpcUaMonitoringParameters::EventFilter>());
+                if (res.results[0].filterResult.content.decoded.type == &UA_TYPES[UA_TYPES_EVENTFILTERRESULT])
+                    p.setFilterResult(convertEventFilterResult(&res.results[0].filterResult));
             }
 
             emit m_backend->monitoringStatusChanged(handle, attr, changed, p);
+
             monItem->parameters = p;
         }
         return true;
diff --git a/src/plugins/opcua/uacpp/quacppsubscription.cpp b/src/plugins/opcua/uacpp/quacppsubscription.cpp
index b02c53b..a4a8edf 100644
--- a/src/plugins/opcua/uacpp/quacppsubscription.cpp
+++ b/src/plugins/opcua/uacpp/quacppsubscription.cpp
@@ -154,9 +154,9 @@ bool QUACppSubscription::addAttributeMonitoredItem(quint64 handle, QOpcUa::NodeA
     monitorId++;
 
     if (UaNodeId(createResults[0].FilterResult.TypeId.NodeId) == UaNodeId(OpcUaId_EventFilterResult_Encoding_DefaultBinary, 0))
-        s.setFilter(QVariant::fromValue(convertEventFilterResult(createResults[0].FilterResult)));
+        s.setFilterResult(convertEventFilterResult(createResults[0].FilterResult));
     else
-        s.setFilter(QVariant()); // The server did not return an EventFilterResult
+        s.clearFilterResult(); // The server did not return an EventFilterResult
     emit m_backend->monitoringEnableDisable(handle, attr, true, s);
 
     return true;
@@ -734,15 +734,19 @@ bool QUACppSubscription::modifyMonitoredItemParameters(quint64 handle, QOpcUa::N
                 p.setDiscardOldest(value.toBool());
                 changed | QOpcUaMonitoringParameters::Parameter::DiscardOldest;
             }
-            if (item == QOpcUaMonitoringParameters::Parameter::Filter &&
-                    UaNodeId(results[0].FilterResult.TypeId.NodeId) == UaNodeId(OpcUaId_EventFilterResult_Encoding_DefaultBinary, 0)) {
-                p.setFilter(QVariant::fromValue(convertEventFilterResult(results[0].FilterResult)));
-                changed | QOpcUaMonitoringParameters::Parameter::Filter;
+
+            if (item == QOpcUaMonitoringParameters::Parameter::Filter) {
+                changed |= QOpcUaMonitoringParameters::Parameter::Filter;
+                if (value.canConvert<QOpcUaMonitoringParameters::DataChangeFilter>())
+                    p.setFilter(value.value<QOpcUaMonitoringParameters::DataChangeFilter>());
+                else if (value.canConvert<QOpcUaMonitoringParameters::EventFilter>())
+                    p.setFilter(value.value<QOpcUaMonitoringParameters::EventFilter>());
+                if (UaNodeId(results[0].FilterResult.TypeId.NodeId) == UaNodeId(OpcUaId_EventFilterResult_Encoding_DefaultBinary, 0))
+                    p.setFilterResult(convertEventFilterResult(results[0].FilterResult));
             }
 
             emit m_backend->monitoringStatusChanged(handle, attr, changed, p);
-            if (item == QOpcUaMonitoringParameters::Parameter::Filter)
-                p.setFilter(m_monitoredItems[key].second.filter()); // Don't overwrite the filter
+
             m_monitoredItems[key].second = p;
         }
         return true;
diff --git a/tests/manual/eventsubscription/tst_eventsubscription.cpp b/tests/manual/eventsubscription/tst_eventsubscription.cpp
index a2ad471..49ab75b 100644
--- a/tests/manual/eventsubscription/tst_eventsubscription.cpp
+++ b/tests/manual/eventsubscription/tst_eventsubscription.cpp
@@ -183,7 +183,7 @@ void EventsubscriptionTest::eventSubscription()
 //    filter << whereElement;
 
     QOpcUaMonitoringParameters p(0);
-    p.setEventFilter(filter);
+    p.setFilter(filter);
 
     serverNode->enableMonitoring(QOpcUa::NodeAttribute::EventNotifier, p);
     enabledSpy.wait();
@@ -191,9 +191,12 @@ void EventsubscriptionTest::eventSubscription()
     QCOMPARE(enabledSpy.at(0).at(0).value<QOpcUa::NodeAttribute>(), QOpcUa::NodeAttribute::EventNotifier);
     QCOMPARE(enabledSpy.at(0).at(1).value<QOpcUa::UaStatusCode>(), QOpcUa::UaStatusCode::Good);
 
+    QCOMPARE(serverNode->monitoringStatus(QOpcUa::NodeAttribute::EventNotifier).filter().value<QOpcUaMonitoringParameters::EventFilter>(),
+             filter);
+
 //    Disabled because the open62541 server does not currently return an EventFilterResult
-//    QVERIFY(serverNode->monitoringStatus(QOpcUa::NodeAttribute::EventNotifier).filter().isValid());
-//    QOpcUa::QEventFilterResult res = serverNode->monitoringStatus(QOpcUa::NodeAttribute::EventNotifier).filter().value<QOpcUa::QEventFilterResult>();
+//    QVERIFY(serverNode->monitoringStatus(QOpcUa::NodeAttribute::EventNotifier).filterResult().isValid());
+//    QOpcUa::QEventFilterResult res = serverNode->monitoringStatus(QOpcUa::NodeAttribute::EventNotifier).filterResult().value<QOpcUa::QEventFilterResult>();
 //    QVERIFY(res.isGood() == true);
 
     qDebug() << "Monitoring enabled, waiting for event...";
@@ -224,6 +227,8 @@ void EventsubscriptionTest::eventSubscription()
     QCOMPARE(modifySpy.at(0).at(0).value<QOpcUa::NodeAttribute>(), QOpcUa::NodeAttribute::EventNotifier);
     QVERIFY(modifySpy.at(0).at(1).value<QOpcUaMonitoringParameters::Parameters>().testFlag(QOpcUaMonitoringParameters::Parameter::Filter) == true);
     QCOMPARE(modifySpy.at(0).at(2).value<QOpcUa::UaStatusCode>(), QOpcUa::UaStatusCode::Good);
+    QCOMPARE(serverNode->monitoringStatus(QOpcUa::NodeAttribute::EventNotifier).filter().value<QOpcUaMonitoringParameters::EventFilter>(),
+             filter);
     qDebug() << "EventFilter modified, waiting for event with additional SourceNode field...";
 
     objectsNode->callMethod(QStringLiteral("ns=1;i=62541")); // Trigger event