From 4d3d856d8fe18b53a10bd8d62384378a75919a57 Mon Sep 17 00:00:00 2001 From: John Thacker Date: Fri, 13 Jan 2023 23:44:48 -0500 Subject: [PATCH] Qt: Conversation/Endpoint table Filter by different QVariant types Return the percentage in the UNFORMATTED_DISPLAYDATA converted from our string so it it compares properly with an equality filter. Use QVariant comparisons where appropriate so that Filter by works as expected with types that are not integers, such as doubles the City/Country strings from GeoIP, or when absolute start time is used for conversation starts. Fix #18738. --- ui/qt/models/atap_data_model.cpp | 24 ++++++-- ui/qt/widgets/traffic_tree.cpp | 94 ++++++++++++++++++++++++++++++-- 2 files changed, 108 insertions(+), 10 deletions(-) diff --git a/ui/qt/models/atap_data_model.cpp b/ui/qt/models/atap_data_model.cpp index b6a52b0bb2..5751b60b5b 100644 --- a/ui/qt/models/atap_data_model.cpp +++ b/ui/qt/models/atap_data_model.cpp @@ -447,9 +447,14 @@ QVariant EndpointDataModel::data(const QModelIndex &idx, int role) const qlonglong totalPackets = (qlonglong)(item->tx_frames_total + item->rx_frames_total); qlonglong packets = (qlonglong)(item->tx_frames + item->rx_frames); percent = totalPackets == 0 ? 0 : (double) packets * 100 / (double) totalPackets; - return QString::number(percent, 'f', 2) + "%"; } - return role == Qt::DisplayRole ? QString::number(percent, 'f', 2) + "%" : (QVariant)percent; + QString rounded = QString::number(percent, 'f', 2); + /* Qt guarantees that this roundtrip conversion compares equally, + * so filtering with equality will work as expected. + * Perhaps the UNFORMATTED_DISPLAYDATA role shoud be split + * into one used for raw data export, and one used for comparisons. + */ + return role == Qt::DisplayRole ? rounded + "%" : QVariant(rounded.toDouble()); } case ENDP_COLUMN_PKT_AB: return role == Qt::DisplayRole ? QString("%L1").arg((qlonglong)item->tx_frames) : QVariant((qlonglong) item->tx_frames); @@ -704,7 +709,14 @@ QVariant ConversationDataModel::data(const QModelIndex &idx, int role) const qlonglong packets = (qlonglong)(conv_item->tx_frames + conv_item->rx_frames); percent = totalPackets == 0 ? 0 : (double) packets * 100 / (double) totalPackets; } - return role == Qt::DisplayRole ? QString::number(percent, 'f', 2) + "%" : (QVariant)percent; + QString rounded = QString::number(percent, 'f', 2); + /* Qt guarantees that this roundtrip conversion compares equally, + * so filtering with equality will work as expected. + * XXX: Perhaps the UNFORMATTED_DISPLAYDATA role shoud be split + * into one used for raw data export and comparisions with each + * other, and another for comparing with filters? + */ + return role == Qt::DisplayRole ? rounded + "%" : QVariant(rounded.toDouble()); } case CONV_COLUMN_PKT_AB: { @@ -727,7 +739,11 @@ QVariant ConversationDataModel::data(const QModelIndex &idx, int role) const if (_absoluteTime) { nstime_t *abs_time = &conv_item->start_abs_time; QDateTime abs_dt = QDateTime::fromMSecsSinceEpoch(nstime_to_msec(abs_time)); - return role == Qt::DisplayRole ? abs_dt.toString("hh:mm:ss.zzzz") : (QVariant)abs_dt; + /* XXX: Should the display include the date as well? More + * clutter, but captures can span midnight. It's probably + * fine so long as the capture isn't more than 24 hours. + */ + return role == Qt::DisplayRole ? abs_dt.time().toString(Qt::ISODateWithMs) : QVariant(abs_dt); } else { return role == Qt::DisplayRole ? QString::number(nstime_to_sec(&conv_item->start_time), 'f', width) : diff --git a/ui/qt/widgets/traffic_tree.cpp b/ui/qt/widgets/traffic_tree.cpp index 598b8c2908..50d2dc0f0b 100644 --- a/ui/qt/widgets/traffic_tree.cpp +++ b/ui/qt/widgets/traffic_tree.cpp @@ -329,13 +329,95 @@ bool TrafficDataFilterProxy::filterAcceptsRow(int source_row, const QModelIndex QVariant data = srcIdx.data(ATapDataModel::UNFORMATTED_DISPLAYDATA); bool filtered = false; - if (_filterOn == TrafficDataFilterProxy::TRAFFIC_DATA_LESS) - filtered = data.toLongLong() < _filterText.toLongLong(); - else if (_filterOn == TrafficDataFilterProxy::TRAFFIC_DATA_GREATER) - filtered = data.toLongLong() > _filterText.toLongLong(); - else if (_filterOn == TrafficDataFilterProxy::TRAFFIC_DATA_EQUAL) { - filtered = data.toLongLong() == _filterText.toLongLong(); + /* QVariant comparisons coerce to the first parameter type, so + * putting data first and converting the string to it is important. + */ +#if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0) + /* QVariant::compare coerces strings to numeric types, but does + * not try to automatically convert them to datetime related types. + */ + QVariant rhs = QVariant(_filterText); + if (data.userType() == QMetaType::QDateTime) { + /* When we display start time in absolute format, we only + * display the time portion, so users will expect to enter + * time-only filters. Convert to QTime instead of QDateTime. + * (Sorting in the table will use the date as well, but it's + * unreasonable to expect users to type in a date that they + * can't see when filtering.) + */ + QTime filterTime = QTime::fromString(_filterText, Qt::ISODateWithMs); + if (filterTime.isValid()) { + rhs.setValue(filterTime); + } else { + rhs = QVariant(); + } + data.setValue(data.toTime()); } + QPartialOrdering result = QVariant::compare(data, rhs); + if (_filterOn == TrafficDataFilterProxy::TRAFFIC_DATA_LESS) + filtered = result < 0; + else if (_filterOn == TrafficDataFilterProxy::TRAFFIC_DATA_GREATER) + filtered = result > 0; + else if (_filterOn == TrafficDataFilterProxy::TRAFFIC_DATA_EQUAL) + filtered = result == 0; +#else + /* The comparisons are deprecated in 5.15. This is most of the + * implementation of QAbstractItemModelPrivate::isVariantLessThan + * from the Qt source. + */ + if (_filterText.isEmpty()) + filtered = true; + else if (data.isNull()) + filtered = false; + else { + switch (data.userType()) { + case QMetaType::Int: + case QMetaType::UInt: + case QMetaType::LongLong: + if (_filterOn == TrafficDataFilterProxy::TRAFFIC_DATA_LESS) + filtered = data.toLongLong() < _filterText.toLongLong(); + else if (_filterOn == TrafficDataFilterProxy::TRAFFIC_DATA_GREATER) + filtered = data.toLongLong() > _filterText.toLongLong(); + else if (_filterOn == TrafficDataFilterProxy::TRAFFIC_DATA_EQUAL) + filtered = data.toLongLong() == _filterText.toLongLong(); + break; + case QMetaType::Float: + case QMetaType::Double: + if (_filterOn == TrafficDataFilterProxy::TRAFFIC_DATA_LESS) + filtered = data.toDouble() < _filterText.toDouble(); + else if (_filterOn == TrafficDataFilterProxy::TRAFFIC_DATA_GREATER) + filtered = data.toDouble() > _filterText.toDouble(); + else if (_filterOn == TrafficDataFilterProxy::TRAFFIC_DATA_EQUAL) + filtered = data.toDouble() == _filterText.toDouble(); + break; + case QMetaType::QDateTime: + case QMetaType::QTime: + /* When we display start time in absolute format, we only + * display the time portion, so users will expect to enter + * time-only filters. Convert to QTime instead of QDateTime. + */ + if (_filterOn == TrafficDataFilterProxy::TRAFFIC_DATA_LESS) + filtered = data.toTime() < QTime::fromString(_filterText, Qt::ISODateWithMs); + else if (_filterOn == TrafficDataFilterProxy::TRAFFIC_DATA_GREATER) + filtered = data.toTime() > QTime::fromString(_filterText, Qt::ISODateWithMs); + else if (_filterOn == TrafficDataFilterProxy::TRAFFIC_DATA_EQUAL) + filtered = data.toTime() == QTime::fromString(_filterText, Qt::ISODateWithMs); + break; + case QMetaType::QString: + default: + /* XXX: We don't do UTF-8 aware coallating in Packet List + * (because it's slow), but possibly could here. + */ + if (_filterOn == TrafficDataFilterProxy::TRAFFIC_DATA_LESS) + filtered = data.toString() < _filterText; + else if (_filterOn == TrafficDataFilterProxy::TRAFFIC_DATA_GREATER) + filtered = data.toString() > _filterText; + else if (_filterOn == TrafficDataFilterProxy::TRAFFIC_DATA_EQUAL) + filtered = data.toString() == _filterText; + break; + } + } +#endif if (!filtered) return false;