Files.app: split query into AND conditioned keywords in metadata search.

BUG=514853
TEST=unit_tests:SearchMetadataTest.*;SearchMetadataSimpleTest.*

Review URL: https://codereview.chromium.org/1321553003

Cr-Commit-Position: refs/heads/master@{#347862}

components/drive/search_metadata.cc

@@ -10,6 +10,9 @@
 #include "base/bind.h"
 #include "base/i18n/string_search.h"
 #include "base/metrics/histogram.h"
+#include "base/strings/string_piece.h"
+#include "base/strings/string_split.h"
+#include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
 #include "components/drive/drive_api_util.h"
@@ -141,12 +144,13 @@ class HiddenEntryClassifier {
 
 // Used to implement SearchMetadata.
 // Adds entry to the result when appropriate.
-// In particular, if |query| is non-null, only adds files with the name matching
-// the query.
+// In particular, if size of |queries| is larger than 0, only adds files with
+// the name matching the query.
 FileError MaybeAddEntryToResult(
     ResourceMetadata* resource_metadata,
     ResourceMetadata::Iterator* it,
-    base::i18n::FixedPatternStringSearchIgnoringCaseAndAccents* query,
+    const ScopedVector<
+        base::i18n::FixedPatternStringSearchIgnoringCaseAndAccents>& queries,
     const SearchMetadataPredicate& predicate,
     size_t at_most_num_matches,
     HiddenEntryClassifier* hidden_entry_classifier,
@@ -168,7 +172,7 @@ FileError MaybeAddEntryToResult(
   // contain |query| to match the query.
   std::string highlighted;
   if (!predicate.Run(entry) ||
-      (query && !FindAndHighlight(entry.base_name(), query, &highlighted)))
+      !FindAndHighlight(entry.base_name(), queries, &highlighted))
     return FILE_ERROR_OK;
 
   // Hidden entry should not be returned.
@@ -194,8 +198,18 @@ FileError SearchMetadataOnBlockingPool(ResourceMetadata* resource_metadata,
                       ResultCandidateComparator> result_candidates;
 
   // Prepare data structure for searching.
-  base::i18n::FixedPatternStringSearchIgnoringCaseAndAccents query(
-      base::UTF8ToUTF16(query_text));
+  std::vector<base::string16> keywords =
+      base::SplitString(base::UTF8ToUTF16(query_text),
+                        base::StringPiece16(base::kWhitespaceUTF16),
+                        base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
+
+  ScopedVector<base::i18n::FixedPatternStringSearchIgnoringCaseAndAccents>
+      queries;
+  for (const auto& keyword : keywords) {
+    queries.push_back(
+        new base::i18n::FixedPatternStringSearchIgnoringCaseAndAccents(
+            keyword));
+  }
 
   // Prepare an object to filter out hidden entries.
   ResourceEntry mydrive;
@@ -210,9 +224,8 @@ FileError SearchMetadataOnBlockingPool(ResourceMetadata* resource_metadata,
   scoped_ptr<ResourceMetadata::Iterator> it = resource_metadata->GetIterator();
   for (; !it->IsAtEnd(); it->Advance()) {
     FileError error = MaybeAddEntryToResult(
-        resource_metadata, it.get(), query_text.empty() ? NULL : &query,
-        predicate, at_most_num_matches, &hidden_entry_classifier,
-        &result_candidates);
+        resource_metadata, it.get(), queries, predicate, at_most_num_matches,
+        &hidden_entry_classifier, &result_candidates);
     if (error != FILE_ERROR_OK)
       return error;
   }
@@ -253,6 +266,22 @@ void RunSearchMetadataCallback(const SearchMetadataCallback& callback,
                       base::TimeTicks::Now() - start_time);
 }
 
+// Appends substring of |original_text| to |highlighted_text| with highlight.
+void AppendStringWithHighlight(const base::string16& original_text,
+                               size_t start,
+                               size_t length,
+                               bool highlight,
+                               std::string* highlighted_text) {
+  if (highlight)
+    highlighted_text->append("<b>");
+
+  highlighted_text->append(net::EscapeForHTML(
+      base::UTF16ToUTF8(original_text.substr(start, length))));
+
+  if (highlight)
+    highlighted_text->append("</b>");
+}
+
 }  // namespace
 
 void SearchMetadata(
@@ -308,26 +337,45 @@ bool MatchesType(int options, const ResourceEntry& entry) {
 
 bool FindAndHighlight(
     const std::string& text,
-    base::i18n::FixedPatternStringSearchIgnoringCaseAndAccents* query,
+    const ScopedVector<
+        base::i18n::FixedPatternStringSearchIgnoringCaseAndAccents>& queries,
     std::string* highlighted_text) {
-  DCHECK(query);
   DCHECK(highlighted_text);
   highlighted_text->clear();
 
-  base::string16 text16 = base::UTF8ToUTF16(text);
+  // Check text matches with all queries.
   size_t match_start = 0;
   size_t match_length = 0;
-  if (!query->Search(text16, &match_start, &match_length))
-    return false;
 
-  base::string16 pre = text16.substr(0, match_start);
-  base::string16 match = text16.substr(match_start, match_length);
-  base::string16 post = text16.substr(match_start + match_length);
-  highlighted_text->append(net::EscapeForHTML(base::UTF16ToUTF8(pre)));
-  highlighted_text->append("<b>");
-  highlighted_text->append(net::EscapeForHTML(base::UTF16ToUTF8(match)));
-  highlighted_text->append("</b>");
-  highlighted_text->append(net::EscapeForHTML(base::UTF16ToUTF8(post)));
+  base::string16 text16 = base::UTF8ToUTF16(text);
+  std::vector<bool> highlights(text16.size(), false);
+  for (auto* query : queries) {
+    if (!query->Search(text16, &match_start, &match_length))
+      return false;
+
+    std::fill(highlights.begin() + match_start,
+              highlights.begin() + match_start + match_length, true);
+  }
+
+  // Generate highlighted text.
+  size_t start_current_segment = 0;
+
+  for (size_t i = 0; i < text16.size(); ++i) {
+    if (highlights[start_current_segment] == highlights[i])
+      continue;
+
+    AppendStringWithHighlight(
+        text16, start_current_segment, i - start_current_segment,
+        highlights[start_current_segment], highlighted_text);
+
+    start_current_segment = i;
+  }
+
+  DCHECK_GE(text16.size(), start_current_segment);
+  AppendStringWithHighlight(
+      text16, start_current_segment, text16.size() - start_current_segment,
+      highlights[start_current_segment], highlighted_text);
+
   return true;
 }
 

components/drive/search_metadata.h

@@ -7,6 +7,7 @@
 
 #include <string>
 
+#include "base/memory/scoped_vector.h"
 #include "components/drive/file_system_interface.h"
 
 namespace base {
@@ -24,11 +25,12 @@ typedef base::Callback<bool(const ResourceEntry&)> SearchMetadataPredicate;
 
 // Searches the local resource metadata, and returns the entries
 // |at_most_num_matches| that contain |query| in their base names. Search is
-// done in a case-insensitive fashion. The eligible entries are selected based
-// on the given |options|, which is a bit-wise OR of SearchMetadataOptions.
-// |callback| must not be null. Must be called on UI thread. Empty |query|
-// matches any base name. i.e. returns everything. |blocking_task_runner| must
-// be the same one as |resource_metadata| uses.
+// done in a case-insensitive fashion. |query| is splitted into keywords by
+// whitespace. All keywords are considered as AND condition. The eligible
+// entries are selected based on the given |options|, which is a bit-wise OR of
+// SearchMetadataOptions. |callback| must not be null. Must be called on UI
+// thread. Empty |query| matches any base name. i.e. returns everything.
+// |blocking_task_runner| must be the same one as |resource_metadata| uses.
 void SearchMetadata(
     scoped_refptr<base::SequencedTaskRunner> blocking_task_runner,
     ResourceMetadata* resource_metadata,
@@ -47,16 +49,18 @@ void SearchMetadata(
 // match with the query. This option can not be used with other options.
 bool MatchesType(int options, const ResourceEntry& entry);
 
-// Finds |query| in |text| while ignoring cases or accents. Cases of non-ASCII
+// Finds |queries| in |text| while ignoring cases or accents. Cases of non-ASCII
 // characters are also ignored; they are compared in the 'Primary Level' of
 // http://userguide.icu-project.org/collation/concepts.
-// Returns true if |query| is found. |highlighted_text| will have the original
+// Returns true if |queries| are found. |highlighted_text| will have the
+// original
 // text with matched portions highlighted with <b> tag (only the first match
 // is highlighted). Meta characters are escaped like &lt;. The original
 // contents of |highlighted_text| will be lost.
 bool FindAndHighlight(
     const std::string& text,
-    base::i18n::FixedPatternStringSearchIgnoringCaseAndAccents* query,
+    const ScopedVector<
+        base::i18n::FixedPatternStringSearchIgnoringCaseAndAccents>& queries,
     std::string* highlighted_text);
 
 }  // namespace internal

components/drive/search_metadata_unittest.cc

@@ -7,6 +7,7 @@
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/i18n/string_search.h"
+#include "base/memory/scoped_vector.h"
 #include "base/run_loop.h"
 #include "base/single_thread_task_runner.h"
 #include "base/strings/utf_string_conversions.h"
@@ -32,9 +33,12 @@ bool FindAndHighlightWrapper(
     const std::string& text,
     const std::string& query_text,
     std::string* highlighted_text) {
-  base::i18n::FixedPatternStringSearchIgnoringCaseAndAccents query(
-      base::UTF8ToUTF16(query_text));
-  return FindAndHighlight(text, &query, highlighted_text);
+  ScopedVector<base::i18n::FixedPatternStringSearchIgnoringCaseAndAccents>
+      queries;
+  queries.push_back(
+      new base::i18n::FixedPatternStringSearchIgnoringCaseAndAccents(
+          base::UTF8ToUTF16(query_text)));
+  return FindAndHighlight(text, queries, highlighted_text);
 }
 
 }  // namespace
@@ -77,18 +81,20 @@ class SearchMetadataTest : public testing::Test {
         util::GetDriveMyDriveRootPath(), &local_id));
     const std::string root_local_id = local_id;
 
-    // drive/root/Directory 1
-    EXPECT_EQ(FILE_ERROR_OK, resource_metadata_->AddEntry(GetDirectoryEntry(
-        "Directory 1", "dir1", 1, root_local_id), &local_id));
+    // drive/root/Directory-1
+    EXPECT_EQ(FILE_ERROR_OK,
+              resource_metadata_->AddEntry(
+                  GetDirectoryEntry("Directory-1", "dir1", 1, root_local_id),
+                  &local_id));
     const std::string dir1_local_id = local_id;
 
-    // drive/root/Directory 1/SubDirectory File 1.txt
+    // drive/root/Directory-1/SubDirectory File 1.txt
     EXPECT_EQ(FILE_ERROR_OK, resource_metadata_->AddEntry(GetFileEntry(
         "SubDirectory File 1.txt", "file1a", 2, dir1_local_id), &local_id));
     EXPECT_EQ(FILE_ERROR_OK, cache_->Store(
         local_id, temp_file_md5, temp_file, FileCache::FILE_OPERATION_COPY));
 
-    // drive/root/Directory 1/Shared To The Account Owner.txt
+    // drive/root/Directory-1/Shared To The Account Owner.txt
     entry = GetFileEntry(
         "Shared To The Account Owner.txt", "file1b", 3, dir1_local_id);
     entry.set_shared_with_me(true);
@@ -187,7 +193,7 @@ TEST_F(SearchMetadataTest, SearchMetadata_RegularFile) {
   EXPECT_EQ(FILE_ERROR_OK, error);
   ASSERT_TRUE(result);
   ASSERT_EQ(1U, result->size());
-  EXPECT_EQ("drive/root/Directory 1/SubDirectory File 1.txt",
+  EXPECT_EQ("drive/root/Directory-1/SubDirectory File 1.txt",
             result->at(0).path.AsUTF8Unsafe());
 }
 
@@ -207,7 +213,7 @@ TEST_F(SearchMetadataTest, SearchMetadata_CaseInsensitiveSearch) {
   EXPECT_EQ(FILE_ERROR_OK, error);
   ASSERT_TRUE(result);
   ASSERT_EQ(1U, result->size());
-  EXPECT_EQ("drive/root/Directory 1/SubDirectory File 1.txt",
+  EXPECT_EQ("drive/root/Directory-1/SubDirectory File 1.txt",
             result->at(0).path.AsUTF8Unsafe());
 }
 
@@ -228,7 +234,7 @@ TEST_F(SearchMetadataTest, SearchMetadata_RegularFiles) {
   // last accessed time in descending order.
   EXPECT_EQ("drive/root/Slash \xE2\x88\x95 in directory/Slash SubDir File.txt",
             result->at(0).path.AsUTF8Unsafe());
-  EXPECT_EQ("drive/root/Directory 1/SubDirectory File 1.txt",
+  EXPECT_EQ("drive/root/Directory-1/SubDirectory File 1.txt",
             result->at(1).path.AsUTF8Unsafe());
 }
 
@@ -257,14 +263,14 @@ TEST_F(SearchMetadataTest, SearchMetadata_Directory) {
 
   SearchMetadata(
       base::ThreadTaskRunnerHandle::Get(), resource_metadata_.get(),
-      "Directory 1", base::Bind(&MatchesType, SEARCH_METADATA_ALL),
+      "Directory-1", base::Bind(&MatchesType, SEARCH_METADATA_ALL),
       kDefaultAtMostNumMatches,
       google_apis::test_util::CreateCopyResultCallback(&error, &result));
   base::RunLoop().RunUntilIdle();
   EXPECT_EQ(FILE_ERROR_OK, error);
   ASSERT_TRUE(result);
   ASSERT_EQ(1U, result->size());
-  EXPECT_EQ("drive/root/Directory 1", result->at(0).path.AsUTF8Unsafe());
+  EXPECT_EQ("drive/root/Directory-1", result->at(0).path.AsUTF8Unsafe());
 }
 
 TEST_F(SearchMetadataTest, SearchMetadata_HostedDocument) {
@@ -312,7 +318,7 @@ TEST_F(SearchMetadataTest, SearchMetadata_SharedWithMe) {
   EXPECT_EQ(FILE_ERROR_OK, error);
   ASSERT_TRUE(result);
   ASSERT_EQ(1U, result->size());
-  EXPECT_EQ("drive/root/Directory 1/Shared To The Account Owner.txt",
+  EXPECT_EQ("drive/root/Directory-1/Shared To The Account Owner.txt",
             result->at(0).path.AsUTF8Unsafe());
 }
 
@@ -396,10 +402,53 @@ TEST_F(SearchMetadataTest, SearchMetadata_Offline) {
 
   EXPECT_EQ("drive/root/File 2.txt",
             result->at(1).path.AsUTF8Unsafe());
-  EXPECT_EQ("drive/root/Directory 1/SubDirectory File 1.txt",
+  EXPECT_EQ("drive/root/Directory-1/SubDirectory File 1.txt",
             result->at(2).path.AsUTF8Unsafe());
 }
 
+TEST_F(SearchMetadataTest, SearchMetadata_MultipleKeywords) {
+  FileError error = FILE_ERROR_FAILED;
+  scoped_ptr<MetadataSearchResultVector> result;
+
+  SearchMetadata(
+      base::ThreadTaskRunnerHandle::Get(), resource_metadata_.get(),
+      "Directory 1", base::Bind(&MatchesType, SEARCH_METADATA_ALL),
+      kDefaultAtMostNumMatches,
+      google_apis::test_util::CreateCopyResultCallback(&error, &result));
+
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(FILE_ERROR_OK, error);
+  ASSERT_TRUE(result);
+  ASSERT_EQ(2U, result->size());
+
+  EXPECT_EQ("drive/root/Directory-1/SubDirectory File 1.txt",
+            result->at(0).path.AsUTF8Unsafe());
+  EXPECT_EQ("drive/root/Directory-1", result->at(1).path.AsUTF8Unsafe());
+}
+
+TEST_F(SearchMetadataTest,
+       SearchMetadata_KeywordsSeparatedWithIdeographicSpace) {
+  FileError error = FILE_ERROR_FAILED;
+  scoped_ptr<MetadataSearchResultVector> result;
+
+  // \xE3\x80\x80 is ideographic space.
+  SearchMetadata(
+      base::ThreadTaskRunnerHandle::Get(), resource_metadata_.get(),
+      "Directory\xE3\x80\x80"
+      "1",
+      base::Bind(&MatchesType, SEARCH_METADATA_ALL), kDefaultAtMostNumMatches,
+      google_apis::test_util::CreateCopyResultCallback(&error, &result));
+
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(FILE_ERROR_OK, error);
+  ASSERT_TRUE(result);
+  ASSERT_EQ(2U, result->size());
+
+  EXPECT_EQ("drive/root/Directory-1/SubDirectory File 1.txt",
+            result->at(0).path.AsUTF8Unsafe());
+  EXPECT_EQ("drive/root/Directory-1", result->at(1).path.AsUTF8Unsafe());
+}
+
 TEST(SearchMetadataSimpleTest, FindAndHighlight_ZeroMatches) {
   std::string highlighted_text;
   EXPECT_FALSE(FindAndHighlightWrapper("text", "query", &highlighted_text));
@@ -410,6 +459,15 @@ TEST(SearchMetadataSimpleTest, FindAndHighlight_EmptyText) {
   EXPECT_FALSE(FindAndHighlightWrapper("", "query", &highlighted_text));
 }
 
+TEST(SearchMetadataSimpleTest, FindAndHighlight_EmptyQuery) {
+  ScopedVector<base::i18n::FixedPatternStringSearchIgnoringCaseAndAccents>
+      queries;
+
+  std::string highlighted_text;
+  EXPECT_TRUE(FindAndHighlight("hello", queries, &highlighted_text));
+  EXPECT_EQ("hello", highlighted_text);
+}
+
 TEST(SearchMetadataSimpleTest, FindAndHighlight_FullMatch) {
   std::string highlighted_text;
   EXPECT_TRUE(FindAndHighlightWrapper("hello", "hello", &highlighted_text));
@@ -473,14 +531,17 @@ TEST(SearchMetadataSimpleTest, FindAndHighlight_IgnoreCaseNonASCII) {
 }
 
 TEST(SearchMetadataSimpleTest, MultiTextBySingleQuery) {
-  base::i18n::FixedPatternStringSearchIgnoringCaseAndAccents query(
-      base::UTF8ToUTF16("hello"));
+  ScopedVector<base::i18n::FixedPatternStringSearchIgnoringCaseAndAccents>
+      queries;
+  queries.push_back(
+      new base::i18n::FixedPatternStringSearchIgnoringCaseAndAccents(
+          base::UTF8ToUTF16("hello")));
 
   std::string highlighted_text;
-  EXPECT_TRUE(FindAndHighlight("hello", &query, &highlighted_text));
+  EXPECT_TRUE(FindAndHighlight("hello", queries, &highlighted_text));
   EXPECT_EQ("<b>hello</b>", highlighted_text);
-  EXPECT_FALSE(FindAndHighlight("goodbye", &query, &highlighted_text));
-  EXPECT_TRUE(FindAndHighlight("1hello2", &query, &highlighted_text));
+  EXPECT_FALSE(FindAndHighlight("goodbye", queries, &highlighted_text));
+  EXPECT_TRUE(FindAndHighlight("1hello2", queries, &highlighted_text));
   EXPECT_EQ("1<b>hello</b>2", highlighted_text);
 }
 
@@ -496,5 +557,45 @@ TEST(SearchMetadataSimpleTest, FindAndHighlight_MoreMetaChars) {
   EXPECT_EQ("a&amp;<b>b&amp;c</b>&amp;d", highlighted_text);
 }
 
+TEST(SearchMetadataSimpleTest, FindAndHighlight_SurrogatePair) {
+  std::string highlighted_text;
+  // \xF0\x9F\x98\x81 (U+1F601) is a surrogate pair for smile icon of emoji.
+  EXPECT_TRUE(FindAndHighlightWrapper("hi\xF0\x9F\x98\x81hello",
+                                      "i\xF0\x9F\x98\x81", &highlighted_text));
+  EXPECT_EQ("h<b>i\xF0\x9F\x98\x81</b>hello", highlighted_text);
+}
+
+TEST(SearchMetadataSimpleTest, FindAndHighlight_MultipleQueries) {
+  ScopedVector<base::i18n::FixedPatternStringSearchIgnoringCaseAndAccents>
+      queries;
+  queries.push_back(
+      new base::i18n::FixedPatternStringSearchIgnoringCaseAndAccents(
+          base::UTF8ToUTF16("hello")));
+  queries.push_back(
+      new base::i18n::FixedPatternStringSearchIgnoringCaseAndAccents(
+          base::UTF8ToUTF16("good")));
+
+  std::string highlighted_text;
+  EXPECT_TRUE(
+      FindAndHighlight("good morning, hello", queries, &highlighted_text));
+  EXPECT_EQ("<b>good</b> morning, <b>hello</b>", highlighted_text);
+}
+
+TEST(SearchMetadataSimpleTest, FindAndHighlight_OverlappingHighlights) {
+  ScopedVector<base::i18n::FixedPatternStringSearchIgnoringCaseAndAccents>
+      queries;
+  queries.push_back(
+      new base::i18n::FixedPatternStringSearchIgnoringCaseAndAccents(
+          base::UTF8ToUTF16("morning")));
+  queries.push_back(
+      new base::i18n::FixedPatternStringSearchIgnoringCaseAndAccents(
+          base::UTF8ToUTF16("ing,")));
+
+  std::string highlighted_text;
+  EXPECT_TRUE(
+      FindAndHighlight("good morning, hello", queries, &highlighted_text));
+  EXPECT_EQ("good <b>morning,</b> hello", highlighted_text);
+}
+
 }  // namespace internal
 }  // namespace drive