From 524bcc73df78731d58a10165fb2f36eb0cd07d9e Mon Sep 17 00:00:00 2001 From: ronso0 Date: Sun, 8 Oct 2023 00:14:49 +0200 Subject: [PATCH 1/3] Search: [text property]:="string" finds only equal strings Co-authored-by: Swiftb0y <12380386+Swiftb0y@users.noreply.github.com> --- src/library/searchquery.cpp | 29 +++++++++++++++++++++++------ src/library/searchquery.h | 9 ++++++++- src/library/searchqueryparser.cpp | 23 +++++++++++++++++++---- src/library/searchqueryparser.h | 3 ++- src/test/searchqueryparsertest.cpp | 26 ++++++++++++++++++++++++++ 5 files changed, 78 insertions(+), 12 deletions(-) diff --git a/src/library/searchquery.cpp b/src/library/searchquery.cpp index c2e23e5490f..a7d269f1286 100644 --- a/src/library/searchquery.cpp +++ b/src/library/searchquery.cpp @@ -168,10 +168,12 @@ QString NotNode::toSql() const { TextFilterNode::TextFilterNode(const QSqlDatabase& database, const QStringList& sqlColumns, - const QString& argument) + const QString& argument, + const StringMatch matchMode) : m_database(database), m_sqlColumns(sqlColumns), - m_argument(argument) { + m_argument(argument), + m_matchMode(matchMode) { mixxx::DbConnection::makeStringLatinLow(&m_argument); } @@ -184,8 +186,14 @@ bool TextFilterNode::match(const TrackPointer& pTrack) const { QString strValue = value.toString(); mixxx::DbConnection::makeStringLatinLow(&strValue); - if (strValue.contains(m_argument)) { - return true; + if (m_matchMode == StringMatch::Equals) { + if (strValue == m_argument) { + return true; + } + } else { + if (strValue.contains(m_argument)) { + return true; + } } } return false; @@ -201,8 +209,17 @@ QString TextFilterNode::toSql() const { argument.append('_'); } } - QString escapedArgument = escaper.escapeString( - kSqlLikeMatchAll + argument + kSqlLikeMatchAll); + QString escapedArgument; + // Using a switch-case without default case to get a compile-time -Wswitch warning + switch (m_matchMode) { + case StringMatch::Contains: + escapedArgument = escaper.escapeString( + kSqlLikeMatchAll + argument + kSqlLikeMatchAll); + break; + case StringMatch::Equals: + escapedArgument = escaper.escapeString(argument); + break; + } QStringList searchClauses; for (const auto& sqlColumn : m_sqlColumns) { searchClauses << QString("%1 LIKE %2").arg(sqlColumn, escapedArgument); diff --git a/src/library/searchquery.h b/src/library/searchquery.h index 9f2882f2cf0..522d092a7b9 100644 --- a/src/library/searchquery.h +++ b/src/library/searchquery.h @@ -16,6 +16,11 @@ const QString kMissingFieldSearchTerm = "\"\""; // "" searches for an empty string +enum class StringMatch { + Contains = 0, + Equals, +}; + class QueryNode { public: QueryNode(const QueryNode&) = delete; // prevent copying @@ -72,7 +77,8 @@ class TextFilterNode : public QueryNode { public: TextFilterNode(const QSqlDatabase& database, const QStringList& sqlColumns, - const QString& argument); + const QString& argument, + const StringMatch matchMode = StringMatch::Contains); bool match(const TrackPointer& pTrack) const override; QString toSql() const override; @@ -81,6 +87,7 @@ class TextFilterNode : public QueryNode { QSqlDatabase m_database; QStringList m_sqlColumns; QString m_argument; + StringMatch m_matchMode; }; class NullOrEmptyTextFilterNode : public QueryNode { diff --git a/src/library/searchqueryparser.cpp b/src/library/searchqueryparser.cpp index 032ec1c60b1..d9bc940499d 100644 --- a/src/library/searchqueryparser.cpp +++ b/src/library/searchqueryparser.cpp @@ -88,7 +88,11 @@ void SearchQueryParser::setSearchColumns(QStringList searchColumns) { } QString SearchQueryParser::getTextArgument(QString argument, - QStringList* tokens) const { + QStringList* tokens, + StringMatch* matchMode) const { + if (matchMode != nullptr) { + *matchMode = StringMatch::Contains; + } // If the argument is empty, assume the user placed a space after an // advanced search command. Consume another token and treat that as the // argument. @@ -99,6 +103,11 @@ QString SearchQueryParser::getTextArgument(QString argument, } } + bool shouldMatchExactly = false; + if (argument.startsWith("=")) { + argument = argument.mid(1); + shouldMatchExactly = true; + } // Deal with quoted arguments. If this token started with a quote, then // search for the closing quote. if (argument.startsWith("\"")) { @@ -128,8 +137,12 @@ QString SearchQueryParser::getTextArgument(QString argument, // return it as "" to distinguish it from an unfinished empty string argument = kMissingFieldSearchTerm; } else { + // Found a closing quote. // Slice off the quote and everything after. argument = argument.left(quote_index); + if (matchMode != nullptr && shouldMatchExactly) { + *matchMode = StringMatch::Equals; + } } } @@ -155,8 +168,8 @@ void SearchQueryParser::parseTokens(QStringList tokens, // TODO(XXX): implement this feature. } else if (textFilterMatch.hasMatch()) { QString field = textFilterMatch.captured(1); - QString argument = getTextArgument( - textFilterMatch.captured(2), &tokens); + StringMatch matchMode = StringMatch::Contains; + QString argument = getTextArgument(textFilterMatch.captured(2), &tokens, &matchMode); if (argument == kMissingFieldSearchTerm) { qDebug() << "argument explicit empty"; @@ -176,7 +189,9 @@ void SearchQueryParser::parseTokens(QStringList tokens, } else { pNode = std::make_unique( m_pTrackCollection->database(), - m_fieldToSqlColumns[field], argument); + m_fieldToSqlColumns[field], + argument, + matchMode); } } } else if (numericFilterMatch.hasMatch()) { diff --git a/src/library/searchqueryparser.h b/src/library/searchqueryparser.h index cbfe88e8916..851267a100c 100644 --- a/src/library/searchqueryparser.h +++ b/src/library/searchqueryparser.h @@ -30,7 +30,8 @@ class SearchQueryParser { AndNode* pQuery) const; QString getTextArgument(QString argument, - QStringList* tokens) const; + QStringList* tokens, + StringMatch* matchMode = nullptr) const; TrackCollection* m_pTrackCollection; QStringList m_queryColumns; diff --git a/src/test/searchqueryparsertest.cpp b/src/test/searchqueryparsertest.cpp index 14c4071d576..614c5097d39 100644 --- a/src/test/searchqueryparsertest.cpp +++ b/src/test/searchqueryparsertest.cpp @@ -178,6 +178,32 @@ TEST_F(SearchQueryParserTest, TextFilter) { qPrintable(pQuery->toSql())); } +TEST_F(SearchQueryParserTest, TextFilterEquals) { + m_parser.setSearchColumns({"artist", "album"}); + auto pQuery(m_parser.parseQuery("comment:=\"asdf\"", QString())); + + TrackPointer pTrack(Track::newTemporary()); + pTrack->setComment("test ASDF test"); + EXPECT_FALSE(pQuery->match(pTrack)); + pTrack->setComment("ASDF"); + EXPECT_TRUE(pQuery->match(pTrack)); + + EXPECT_STREQ( + qPrintable(QString("comment LIKE 'asdf'")), + qPrintable(pQuery->toSql())); + + // Incomplete quoting should use StringMatch::Contains, + // i.e. equal to 'comment:asdf' + pQuery = m_parser.parseQuery("comment:=\"asdf", QString()); + + pTrack->setComment("test ASDF test"); + EXPECT_TRUE(pQuery->match(pTrack)); + + EXPECT_STREQ( + qPrintable(QString("comment LIKE '%asdf%'")), + qPrintable(pQuery->toSql())); +} + TEST_F(SearchQueryParserTest, TextFilterEmpty) { m_parser.setSearchColumns({"artist", "album"}); // An empty argument should pass everything. From d938e9a606e918f407c8a0487e4fc5bda2fd5840 Mon Sep 17 00:00:00 2001 From: Swiftb0y <12380386+Swiftb0y@users.noreply.github.com> Date: Mon, 9 Oct 2023 17:32:31 +0200 Subject: [PATCH 2/3] refactor: remove unnecessary ~SearchQueryParser --- src/library/searchqueryparser.cpp | 3 --- src/library/searchqueryparser.h | 2 -- 2 files changed, 5 deletions(-) diff --git a/src/library/searchqueryparser.cpp b/src/library/searchqueryparser.cpp index d9bc940499d..c0c029bf1dc 100644 --- a/src/library/searchqueryparser.cpp +++ b/src/library/searchqueryparser.cpp @@ -71,9 +71,6 @@ SearchQueryParser::SearchQueryParser(TrackCollection* pTrackCollection, QStringL QString("^[~-]?(%1):(.*)$").arg(m_specialFilters.join("|"))); } -SearchQueryParser::~SearchQueryParser() { -} - void SearchQueryParser::setSearchColumns(QStringList searchColumns) { m_queryColumns = std::move(searchColumns); diff --git a/src/library/searchqueryparser.h b/src/library/searchqueryparser.h index 851267a100c..fb32fc1efe3 100644 --- a/src/library/searchqueryparser.h +++ b/src/library/searchqueryparser.h @@ -12,8 +12,6 @@ class SearchQueryParser { public: explicit SearchQueryParser(TrackCollection* pTrackCollection, QStringList searchColumns); - virtual ~SearchQueryParser(); - void setSearchColumns(QStringList searchColumns); std::unique_ptr parseQuery( From ce9128f82a339f59853bd6ffea776d0691c3f867 Mon Sep 17 00:00:00 2001 From: Swiftb0y <12380386+Swiftb0y@users.noreply.github.com> Date: Mon, 9 Oct 2023 18:21:55 +0200 Subject: [PATCH 3/3] refactor: SearchQueryParser::getTextArgument --- src/library/searchqueryparser.cpp | 117 ++++++++++++++++-------------- src/library/searchqueryparser.h | 10 ++- 2 files changed, 71 insertions(+), 56 deletions(-) diff --git a/src/library/searchqueryparser.cpp b/src/library/searchqueryparser.cpp index c0c029bf1dc..5270b97ce52 100644 --- a/src/library/searchqueryparser.cpp +++ b/src/library/searchqueryparser.cpp @@ -1,8 +1,57 @@ #include "library/searchqueryparser.h" #include +#include +#include "library/searchquery.h" #include "track/keyutils.h" +#include "util/assert.h" + +namespace { + +enum class Quoted : bool { + Incomplete, + Complete, +}; + +std::pair consumeQuotedArgument(QString argument, + QStringList* tokens) { + DEBUG_ASSERT(argument.startsWith("\"")); + + argument = argument.mid(1); + + int quote_index = argument.indexOf("\""); + while (quote_index == -1 && tokens->length() > 0) { + argument += " " + tokens->takeFirst(); + quote_index = argument.indexOf("\""); + } + + if (quote_index == -1) { + // No ending quote found. Since we think they are going to close the + // quote eventually, treat the entire token list as the argument for + // now. + return {argument, Quoted::Incomplete}; + } + + // Stuff the rest of the argument after the quote back into tokens. + QString remaining = argument.mid(quote_index + 1).trimmed(); + if (remaining.size() != 0) { + tokens->push_front(remaining); + } + + if (quote_index == 0) { + // We have found an explicit empty string "" + // return it as "" to distinguish it from an unfinished empty string + argument = kMissingFieldSearchTerm; + } else { + // Found a closing quote. + // Slice off the quote and everything after. + argument = argument.left(quote_index); + } + return {argument, Quoted::Complete}; +} + +} // anonymous namespace constexpr char kNegatePrefix[] = "-"; constexpr char kFuzzyPrefix[] = "~"; @@ -84,12 +133,8 @@ void SearchQueryParser::setSearchColumns(QStringList searchColumns) { } } -QString SearchQueryParser::getTextArgument(QString argument, - QStringList* tokens, - StringMatch* matchMode) const { - if (matchMode != nullptr) { - *matchMode = StringMatch::Contains; - } +SearchQueryParser::TextArgumentResult SearchQueryParser::getTextArgument(QString argument, + QStringList* tokens) const { // If the argument is empty, assume the user placed a space after an // advanced search command. Consume another token and treat that as the // argument. @@ -99,51 +144,20 @@ QString SearchQueryParser::getTextArgument(QString argument, argument = tokens->takeFirst(); } } - - bool shouldMatchExactly = false; + StringMatch mode = StringMatch::Contains; if (argument.startsWith("=")) { + // strip the '=' from the argument argument = argument.mid(1); - shouldMatchExactly = true; + mode = StringMatch::Equals; } - // Deal with quoted arguments. If this token started with a quote, then - // search for the closing quote. if (argument.startsWith("\"")) { - argument = argument.mid(1); - - int quote_index = argument.indexOf("\""); - while (quote_index == -1 && tokens->length() > 0) { - argument += " " + tokens->takeFirst(); - quote_index = argument.indexOf("\""); - } - - if (quote_index == -1) { - // No ending quote found. Since we think they are going to close the - // quote eventually, treat the entire token list as the argument for - // now. - return argument; - } - - // Stuff the rest of the argument after the quote back into tokens. - QString remaining = argument.mid(quote_index+1).trimmed(); - if (remaining.size() != 0) { - tokens->push_front(remaining); - } - - if (quote_index == 0) { - // We have found an explicit empty string "" - // return it as "" to distinguish it from an unfinished empty string - argument = kMissingFieldSearchTerm; - } else { - // Found a closing quote. - // Slice off the quote and everything after. - argument = argument.left(quote_index); - if (matchMode != nullptr && shouldMatchExactly) { - *matchMode = StringMatch::Equals; - } - } + Quoted quoted; + std::tie(argument, quoted) = consumeQuotedArgument(argument, tokens); + mode = quoted == Quoted::Complete && mode == StringMatch::Equals + ? StringMatch::Equals + : StringMatch::Contains; } - - return argument; + return {argument, mode}; } void SearchQueryParser::parseTokens(QStringList tokens, @@ -165,8 +179,7 @@ void SearchQueryParser::parseTokens(QStringList tokens, // TODO(XXX): implement this feature. } else if (textFilterMatch.hasMatch()) { QString field = textFilterMatch.captured(1); - StringMatch matchMode = StringMatch::Contains; - QString argument = getTextArgument(textFilterMatch.captured(2), &tokens, &matchMode); + auto [argument, matchMode] = getTextArgument(textFilterMatch.captured(2), &tokens); if (argument == kMissingFieldSearchTerm) { qDebug() << "argument explicit empty"; @@ -193,9 +206,7 @@ void SearchQueryParser::parseTokens(QStringList tokens, } } else if (numericFilterMatch.hasMatch()) { QString field = numericFilterMatch.captured(1); - QString argument = getTextArgument( - numericFilterMatch.captured(2), &tokens) - .trimmed(); + QString argument = getTextArgument(numericFilterMatch.captured(2), &tokens).argument; if (!argument.isEmpty()) { if (argument == kMissingFieldSearchTerm) { @@ -211,7 +222,7 @@ void SearchQueryParser::parseTokens(QStringList tokens, QString field = specialFilterMatch.captured(1); QString argument = getTextArgument( specialFilterMatch.captured(2), &tokens) - .trimmed(); + .argument; if (!argument.isEmpty()) { if (field == "key") { mixxx::track::io::key::ChromaticKey key = @@ -249,7 +260,7 @@ void SearchQueryParser::parseTokens(QStringList tokens, } // Don't trigger on a lone minus sign. if (!token.isEmpty()) { - QString argument = getTextArgument(token, &tokens); + QString argument = getTextArgument(token, &tokens).argument; // For untagged strings we search the track fields as well // as the crate names the track is in. This allows the user // to use crates like tags diff --git a/src/library/searchqueryparser.h b/src/library/searchqueryparser.h index fb32fc1efe3..5aa13b28390 100644 --- a/src/library/searchqueryparser.h +++ b/src/library/searchqueryparser.h @@ -27,9 +27,13 @@ class SearchQueryParser { void parseTokens(QStringList tokens, AndNode* pQuery) const; - QString getTextArgument(QString argument, - QStringList* tokens, - StringMatch* matchMode = nullptr) const; + struct TextArgumentResult { + QString argument; + StringMatch mode; + }; + + TextArgumentResult getTextArgument(QString argument, + QStringList* tokens) const; TrackCollection* m_pTrackCollection; QStringList m_queryColumns;