(search) Fix searching tags with spaces

Summary:
Tags containing blank spaces were not handled properly in the search widget.
Now we enclose them in quotes and strip the quotes before setting them to the widget.

{F7854247}

Test Plan:
No artifacts when searching tags containing spaces
Added test cases to `bin/dolphinquerytest`

Reviewers: #dolphin, elvisangelaccio, ngraham, meven

Reviewed By: #dolphin, elvisangelaccio, ngraham

Subscribers: meven, kfm-devel

Tags: #dolphin

Differential Revision: https://phabricator.kde.org/D26369

diff --git a/src/search/dolphinfacetswidget.cpp b/src/search/dolphinfacetswidget.cpp
index 33c3cea2452b90b7a59ad43cf6b36453ce004a95..d9943abcd4867ea425629fc6b9bb74710b4f2a49 100644 (file)
--- a/src/search/dolphinfacetswidget.cpp
+++ b/src/search/dolphinfacetswidget.cpp
@@ -131,7 +131,11 @@ QStringList DolphinFacetsWidget::searchTerms() const
 
     if (!m_searchTags.isEmpty()) {
         for (auto const &tag : m_searchTags) {
 
     if (!m_searchTags.isEmpty()) {
         for (auto const &tag : m_searchTags) {

-            terms << QStringLiteral("tag:%1").arg(tag);
+            if (tag.contains(QLatin1Char(' '))) {
+                terms << QStringLiteral("tag:\"%1\"").arg(tag);
+            } else {
+                terms << QStringLiteral("tag:%1").arg(tag);
+            }

         }
     }
 
         }
     }
 

diff --git a/src/search/dolphinquery.cpp b/src/search/dolphinquery.cpp
index 95efe9162a773b3951543e20786fdbb6017e951c..ab107f43fab736e3a39407017dd5a7dd28f2abbf 100644 (file)
--- a/src/search/dolphinquery.cpp
+++ b/src/search/dolphinquery.cpp
@@ -27,12 +27,13 @@
 #endif
 
 namespace {
 #endif
 
 namespace {

-    /** Checks if a given term in the Baloo::Query::searchString() is a special search term.
-     * This is a copy of `DolphinFacetsWidget::isSearchTerm()` method.
+    /** Checks if a given term in the Baloo::Query::searchString() is a special search term
+     * @return: the specific search token of the term, or an empty QString() if none is found

      */
      */

-    bool isSearchTerm(const QString& term)
+    QString searchTermToken(const QString& term)

     {
         static const QLatin1String searchTokens[] {
     {
         static const QLatin1String searchTokens[] {

+            QLatin1String("filename:"),

             QLatin1String("modified>="),
             QLatin1String("rating>="),
             QLatin1String("tag:"), QLatin1String("tag=")
             QLatin1String("modified>="),
             QLatin1String("rating>="),
             QLatin1String("tag:"), QLatin1String("tag=")


@@ -40,27 +41,24 @@ namespace {
 
         for (const auto &searchToken : searchTokens) {
             if (term.startsWith(searchToken)) {
 
         for (const auto &searchToken : searchTokens) {
             if (term.startsWith(searchToken)) {

-                return true;
+                return searchToken;

             }
         }
             }
         }

-        return false;
+        return QString();

     }
 
     QString stripQuotes(const QString& text)
     {
     }
 
     QString stripQuotes(const QString& text)
     {

-        QString cleanedText = text;
-        if (!cleanedText.isEmpty() && cleanedText.at(0) == QLatin1Char('"')) {
-            cleanedText = cleanedText.mid(1);
+        if (text.length() >= 2 && text.at(0) == QLatin1Char('"')
+                               && text.back() == QLatin1Char('"')) {
+            return text.mid(1, text.size() - 2);

         }
         }

-        if (!cleanedText.isEmpty() && cleanedText.back() == QLatin1Char('"')) {
-            cleanedText = cleanedText.mid(0, cleanedText.size() - 1);
-        }
-        return cleanedText;
+        return text;

     }
 
     QStringList splitOutsideQuotes(const QString& text)
     {
     }
 
     QStringList splitOutsideQuotes(const QString& text)
     {

-        const QRegularExpression subTermsRegExp("([^ ]*\"[^\"]*\"|(?<= |^)[^ ]+(?= |$))");
+        const QRegularExpression subTermsRegExp("(\\S*?\"[^\"]*?\"|(?<=\\s|^)\\S+(?=\\s|$))");

         auto subTermsMatchIterator = subTermsRegExp.globalMatch(text);
 
         QStringList textParts;
         auto subTermsMatchIterator = subTermsRegExp.globalMatch(text);
 
         QStringList textParts;


@@ -89,23 +87,23 @@ DolphinQuery DolphinQuery::fromBalooSearchUrl(const QUrl& searchUrl)
 
     const QStringList subTerms = splitOutsideQuotes(query.searchString());
     foreach (const QString& subTerm, subTerms) {
 
     const QStringList subTerms = splitOutsideQuotes(query.searchString());
     foreach (const QString& subTerm, subTerms) {

-        if (subTerm.startsWith(QLatin1String("filename:"))) {
-            fileName = stripQuotes(subTerm.mid(9));
-            if (!fileName.isEmpty()) {
+        const QString token = searchTermToken(subTerm);
+        const QString value = stripQuotes(subTerm.mid(token.length()));
+
+        if (token == QLatin1String("filename:")) {
+            if (!value.isEmpty()) {
+                fileName = value;

                 model.m_hasFileName = true;
             }
             continue;
                 model.m_hasFileName = true;
             }
             continue;

-        } else if (isSearchTerm(subTerm)) {
-            model.m_searchTerms << subTerm;
+        } else if (!token.isEmpty()) {
+            model.m_searchTerms << token + value;

             continue;
         } else if (subTerm == QLatin1String("AND") && subTerm != subTerms.at(0) && subTerm != subTerms.back()) {
             continue;
             continue;
         } else if (subTerm == QLatin1String("AND") && subTerm != subTerms.at(0) && subTerm != subTerms.back()) {
             continue;

-        } else {
-            const QString cleanedTerm = stripQuotes(subTerm);
-            if (!cleanedTerm.isEmpty()) {
-                textParts << cleanedTerm;
-                model.m_hasContentSearch = true;
-            }
+        } else if (!value.isEmpty()) {
+            textParts << value;
+            model.m_hasContentSearch = true;

         }
     }
 
         }
     }
 

diff --git a/src/tests/dolphinquerytest.cpp b/src/tests/dolphinquerytest.cpp
index 65dc7350e9f98d07e5711953cfb0df76797816d6..b65ee037fdcc343c6fc3af31589f074574fc0603 100644 (file)
--- a/src/tests/dolphinquerytest.cpp
+++ b/src/tests/dolphinquerytest.cpp
@@ -41,12 +41,17 @@ private slots:
  */
 void DolphinSearchBoxTest::testBalooSearchParsing_data()
 {
  */
 void DolphinSearchBoxTest::testBalooSearchParsing_data()
 {

-    const QString text = QStringLiteral("abc xyz");
+    const QString text = QStringLiteral("abc");
+    const QString textS = QStringLiteral("abc xyz");

     const QString filename = QStringLiteral("filename:\"%1\"").arg(text);
     const QString filename = QStringLiteral("filename:\"%1\"").arg(text);

+    const QString filenameS = QStringLiteral("filename:\"%1\"").arg(textS);
+

     const QString rating = QStringLiteral("rating>=2");
     const QString rating = QStringLiteral("rating>=2");

-    const QString modified = QString("modified>=2019-08-07");
-    const QString tagA = QString("tag:tagA");
-    const QString tagB = QString("tag:tagB");
+    const QString modified = QStringLiteral("modified>=2019-08-07");
+
+    const QString tag = QStringLiteral("tag:tagA");
+    const QString tagS = QStringLiteral("tag:\"tagB with spaces\"");  // in search url
+    const QString tagR = QStringLiteral("tag:tagB with spaces");      // in result term

 
     QTest::addColumn<QString>("searchString");
     QTest::addColumn<QString>("expectedText");
 
     QTest::addColumn<QString>("searchString");
     QTest::addColumn<QString>("expectedText");


@@ -55,19 +60,23 @@ void DolphinSearchBoxTest::testBalooSearchParsing_data()
     QTest::addColumn<bool>("hasFileName");
 
     // Test for "Content"
     QTest::addColumn<bool>("hasFileName");
 
     // Test for "Content"

-    QTest::newRow("content")              << text    << text << QStringList() << true  << false;
-    QTest::newRow("content/empty")        << ""      << ""   << QStringList() << false << false;
-    QTest::newRow("content/singleQuote")  << "\""    << ""   << QStringList() << false << false;
-    QTest::newRow("content/doubleQuote")  << "\"\""  << ""   << QStringList() << false << false;
+    QTest::newRow("content")              << text    << text  << QStringList() << true  << false;
+    QTest::newRow("content/space")        << textS   << textS << QStringList() << true  << false;
+    QTest::newRow("content/empty")        << ""      << ""    << QStringList() << false << false;
+    QTest::newRow("content/single_quote") << "\""    << "\""  << QStringList() << true  << false;
+    QTest::newRow("content/double_quote") << "\"\""  << ""    << QStringList() << false << false;

 
     // Test for "FileName"
 
     // Test for "FileName"

-    QTest::newRow("filename")             << filename         << text << QStringList() << false << true;
-    QTest::newRow("filename/empty")       << "filename:"      << ""   << QStringList() << false << false;
-    QTest::newRow("filename/singleQuote") << "filename:\""    << ""   << QStringList() << false << false;
-    QTest::newRow("filename/doubleQuote") << "filename:\"\""  << ""   << QStringList() << false << false;
+    QTest::newRow("filename")              << filename        << text  << QStringList() << false << true;
+    QTest::newRow("filename/space")        << filenameS       << textS << QStringList() << false << true;
+    QTest::newRow("filename/empty")        << "filename:"     << ""    << QStringList() << false << false;
+    QTest::newRow("filename/single_quote") << "filename:\""   << "\""  << QStringList() << false << true;
+    QTest::newRow("filename/double_quote") << "filename:\"\"" << ""    << QStringList() << false << false;

 
     // Combined content and filename search
 
     // Combined content and filename search

-    QTest::newRow("content+filename") << text + " " + filename << text + " " + filename << QStringList() << true << true;
+    QTest::newRow("content+filename")
+        << text + " " + filename
+        << text + " " + filename << QStringList() << true << true;

 
     // Test for rating
     QTest::newRow("rating")          << rating                  << ""   << QStringList({rating}) << false << false;
 
     // Test for rating
     QTest::newRow("rating")          << rating                  << ""   << QStringList({rating}) << false << false;


@@ -80,27 +89,32 @@ void DolphinSearchBoxTest::testBalooSearchParsing_data()
     QTest::newRow("modified+filename") << modified + " " + filename << text << QStringList({modified}) << false << true;
 
     // Test for tags
     QTest::newRow("modified+filename") << modified + " " + filename << text << QStringList({modified}) << false << true;
 
     // Test for tags

-    QTest::newRow("tag")          << tagA                  << ""   << QStringList({tagA})       << false << false;
-    QTest::newRow("tag/double")   << tagA + " " + tagB     << ""   << QStringList({tagA, tagB}) << false << false;
-    QTest::newRow("tag+content")  << tagA + " " + text     << text << QStringList({tagA})       << true  << false;
-    QTest::newRow("tag+filename") << tagA + " " + filename << text << QStringList({tagA})       << false << true;
+    QTest::newRow("tag")          << tag                  << ""   << QStringList({tag})       << false << false;
+    QTest::newRow("tag/space" )   << tagS                 << ""   << QStringList({tagR})      << false << false;
+    QTest::newRow("tag/double")   << tag + " " + tagS     << ""   << QStringList({tag, tagR}) << false << false;
+    QTest::newRow("tag+content")  << tag + " " + text     << text << QStringList({tag})       << true  << false;
+    QTest::newRow("tag+filename") << tag + " " + filename << text << QStringList({tag})       << false << true;

 
     // Combined search terms
 
     // Combined search terms

-    QTest::newRow("allTerms")
-        << rating + " AND " + modified + " AND " + tagA + " AND " + tagB
-        << "" << QStringList({modified, rating, tagA, tagB}) << false << false;
+    QTest::newRow("searchTerms")
+        << rating + " AND " + modified + " AND " + tag + " AND " + tagS
+        << "" << QStringList({modified, rating, tag, tagR}) << false << false;

 
 

-    QTest::newRow("allTerms+content")
-        << rating + " AND " + modified + " " + text + " " + tagA + " AND " + tagB
-        << text << QStringList({modified, rating, tagA, tagB}) << true << false;
+    QTest::newRow("searchTerms+content")
+        << rating + " AND " + modified + " " + text + " " + tag + " AND " + tagS
+        << text << QStringList({modified, rating, tag, tagR}) << true << false;

 
 

-    QTest::newRow("allTerms+filename")
-        << rating + " AND " + modified + " " + filename + " " + tagA + " AND " + tagB
-        << text << QStringList({modified, rating, tagA, tagB}) << false << true;
+    QTest::newRow("searchTerms+filename")
+        << rating + " AND " + modified + " " + filename + " " + tag + " AND " + tagS
+        << text << QStringList({modified, rating, tag, tagR}) << false << true;
+
+    QTest::newRow("allTerms")
+        << text + " " + filename + " " + rating + " AND " + modified + " AND " + tag
+        << text + " " + filename << QStringList({modified, rating, tag}) << true << true;

 
 

-    QTest::newRow("allTerms+content+filename")
-        << text + " " + filename + " " + rating + " AND " + modified + " AND " + tagA + " AND " + tagB
-        << text + " " + filename << QStringList({modified, rating, tagA, tagB}) << true << true;
+    QTest::newRow("allTerms/space")
+        << textS + " " + filenameS + " " + rating + " AND " + modified + " AND " + tagS
+        << textS + " " + filenameS << QStringList({modified, rating, tagR}) << true << true;

 }
 
 /**
 }
 
 /**