diff options
| author | Tim Barron <tjbarron@google.com> | 2021-03-15 22:11:17 +0000 |
|---|---|---|
| committer | Tim Barron <tjbarron@google.com> | 2021-03-15 22:23:35 +0000 |
| commit | 89d8c837c38b94ae0f49fa4cb685685eaaedb950 (patch) | |
| tree | 25dfec6e49ef5a96c09af81954011b5e7117dfaf /icing | |
| parent | 0b4a315c51613479f98b4c2741c2dba6389a750b (diff) | |
| download | icing-89d8c837c38b94ae0f49fa4cb685685eaaedb950.tar.gz | |
Sync from upstream.
Descriptions:
============
Add result grouping to the ResultSpecProto to allow callers to limit the number of results returned per group of namespace.
============
Implement PersistToDisk(LITE) to expose a LITE option for PersistToDisk that only persists the ground truth.
============
Add per-namespace storage info.
============
Add score to SearchResult.
============
Migrate IndexProcessorTest, DocHitInfoIteratorFilterTest, DocHitInfoIteratorSectionRestrictTest to use SchemaBuilder
============
Migrate ScorerTest and ScoringProcessorTest to use SchemaBuilder
============
Add licensing header for storage.proto.
Change-Id: I5cbdd8045036dc38cf00191a2eac56cc6f2d3443
Diffstat (limited to 'icing')
23 files changed, 1880 insertions, 609 deletions
diff --git a/icing/icing-search-engine-with-icu-file_test.cc b/icing/icing-search-engine-with-icu-file_test.cc index 24523c9..48e81e5 100644 --- a/icing/icing-search-engine-with-icu-file_test.cc +++ b/icing/icing-search-engine-with-icu-file_test.cc @@ -123,8 +123,8 @@ TEST(IcingSearchEngineWithIcuFileTest, ShouldIndexAndSearch) { // The token is a random number so we don't verify it. expected_search_result_proto.set_next_page_token( search_result_proto.next_page_token()); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); } } // namespace diff --git a/icing/icing-search-engine.cc b/icing/icing-search-engine.cc index b50e412..047fc81 100644 --- a/icing/icing-search-engine.cc +++ b/icing/icing-search-engine.cc @@ -97,6 +97,21 @@ libtextclassifier3::Status ValidateResultSpec( return absl_ports::InvalidArgumentError( "ResultSpecProto.num_per_page cannot be negative."); } + std::unordered_set<std::string> unique_namespaces; + for (const ResultSpecProto::ResultGrouping& result_grouping : + result_spec.result_groupings()) { + if (result_grouping.max_results() <= 0) { + return absl_ports::InvalidArgumentError( + "Cannot specify a result grouping with max results <= 0."); + } + for (const std::string& name_space : result_grouping.namespaces()) { + if (unique_namespaces.count(name_space) > 0) { + return absl_ports::InvalidArgumentError( + "Namespaces must be unique across result groups."); + } + unique_namespaces.insert(name_space); + } + } return libtextclassifier3::Status::OK; } @@ -241,14 +256,13 @@ IcingSearchEngine::IcingSearchEngine( filesystem_(std::move(filesystem)), icing_filesystem_(std::move(icing_filesystem)), clock_(std::move(clock)), - result_state_manager_(performance_configuration_.max_num_total_hits), jni_cache_(std::move(jni_cache)) { ICING_VLOG(1) << "Creating IcingSearchEngine in dir: " << options_.base_dir(); } IcingSearchEngine::~IcingSearchEngine() { if (initialized_) { - if (PersistToDisk().status().code() != StatusProto::OK) { + if (PersistToDisk(PersistType::FULL).status().code() != StatusProto::OK) { ICING_LOG(ERROR) << "Error persisting to disk in IcingSearchEngine destructor"; } @@ -283,9 +297,6 @@ InitializeResultProto IcingSearchEngine::InternalInitialize() { return result_proto; } - // Releases result / query cache if any - result_state_manager_.InvalidateAllResultStates(); - libtextclassifier3::Status status = InitializeMembers(initialize_stats); if (!status.ok()) { TransformStatus(status, result_status); @@ -365,6 +376,9 @@ libtextclassifier3::Status IcingSearchEngine::InitializeMembers( ICING_RETURN_IF_ERROR(InitializeSchemaStore(initialize_stats)); ICING_RETURN_IF_ERROR(InitializeDocumentStore(initialize_stats)); + result_state_manager_ = std::make_unique<ResultStateManager>( + performance_configuration_.max_num_total_hits, *document_store_); + // TODO(b/156383798) : Resolve how to specify the locale. language_segmenter_factory::SegmenterOptions segmenter_options( ULOC_US, jni_cache_.get()); @@ -994,7 +1008,8 @@ DeleteByQueryResultProto IcingSearchEngine::DeleteByQuery( return result_proto; } -PersistToDiskResultProto IcingSearchEngine::PersistToDisk() { +PersistToDiskResultProto IcingSearchEngine::PersistToDisk( + PersistType::Code persist_type) { ICING_VLOG(1) << "Persisting data to disk"; PersistToDiskResultProto result_proto; @@ -1007,7 +1022,7 @@ PersistToDiskResultProto IcingSearchEngine::PersistToDisk() { return result_proto; } - auto status = InternalPersistToDisk(); + auto status = InternalPersistToDisk(persist_type); TransformStatus(status, result_status); return result_proto; } @@ -1041,11 +1056,8 @@ OptimizeResultProto IcingSearchEngine::Optimize() { optimize_stats->set_storage_size_before(-1); } - // Releases result / query cache if any - result_state_manager_.InvalidateAllResultStates(); - // Flushes data to disk before doing optimization - auto status = InternalPersistToDisk(); + auto status = InternalPersistToDisk(PersistType::FULL); if (!status.ok()) { TransformStatus(status, result_status); return result_proto; @@ -1217,9 +1229,13 @@ StorageInfoResultProto IcingSearchEngine::GetStorageInfo() { return result; } -libtextclassifier3::Status IcingSearchEngine::InternalPersistToDisk() { +libtextclassifier3::Status IcingSearchEngine::InternalPersistToDisk( + PersistType::Code persist_type) { + if (persist_type == PersistType::LITE) { + return document_store_->PersistToDisk(persist_type); + } ICING_RETURN_IF_ERROR(schema_store_->PersistToDisk()); - ICING_RETURN_IF_ERROR(document_store_->PersistToDisk()); + ICING_RETURN_IF_ERROR(document_store_->PersistToDisk(PersistType::FULL)); ICING_RETURN_IF_ERROR(index_->PersistToDisk()); // Update the combined checksum and write to header file. @@ -1382,9 +1398,9 @@ SearchResultProto IcingSearchEngine::Search( component_timer = clock_->GetNewTimer(); // Ranks and paginates results libtextclassifier3::StatusOr<PageResultState> page_result_state_or = - result_state_manager_.RankAndPaginate(ResultState( + result_state_manager_->RankAndPaginate(ResultState( std::move(result_document_hits), std::move(query_results.query_terms), - search_spec, scoring_spec, result_spec)); + search_spec, scoring_spec, result_spec, *document_store_)); if (!page_result_state_or.ok()) { TransformStatus(page_result_state_or.status(), result_status); return result_proto; @@ -1400,7 +1416,7 @@ SearchResultProto IcingSearchEngine::Search( ResultRetriever::Create(document_store_.get(), schema_store_.get(), language_segmenter_.get(), normalizer_.get()); if (!result_retriever_or.ok()) { - result_state_manager_.InvalidateResultState( + result_state_manager_->InvalidateResultState( page_result_state.next_page_token); TransformStatus(result_retriever_or.status(), result_status); return result_proto; @@ -1411,7 +1427,7 @@ SearchResultProto IcingSearchEngine::Search( libtextclassifier3::StatusOr<std::vector<SearchResultProto::ResultProto>> results_or = result_retriever->RetrieveResults(page_result_state); if (!results_or.ok()) { - result_state_manager_.InvalidateResultState( + result_state_manager_->InvalidateResultState( page_result_state.next_page_token); TransformStatus(results_or.status(), result_status); return result_proto; @@ -1457,7 +1473,7 @@ SearchResultProto IcingSearchEngine::GetNextPage(uint64_t next_page_token) { std::unique_ptr<Timer> overall_timer = clock_->GetNewTimer(); libtextclassifier3::StatusOr<PageResultState> page_result_state_or = - result_state_manager_.GetNextPage(next_page_token); + result_state_manager_->GetNextPage(next_page_token); if (!page_result_state_or.ok()) { if (absl_ports::IsNotFound(page_result_state_or.status())) { @@ -1528,7 +1544,7 @@ void IcingSearchEngine::InvalidateNextPageToken(uint64_t next_page_token) { ICING_LOG(ERROR) << "IcingSearchEngine has not been initialized!"; return; } - result_state_manager_.InvalidateResultState(next_page_token); + result_state_manager_->InvalidateResultState(next_page_token); } libtextclassifier3::Status IcingSearchEngine::OptimizeDocumentStore( @@ -1559,7 +1575,9 @@ libtextclassifier3::Status IcingSearchEngine::OptimizeDocumentStore( optimize_status.error_message()); } - // Resets before swapping + // result_state_manager_ depends on document_store_. So we need to reset it at + // the same time that we reset the document_store_. + result_state_manager_.reset(); document_store_.reset(); // When swapping files, always put the current working directory at the @@ -1596,6 +1614,8 @@ libtextclassifier3::Status IcingSearchEngine::OptimizeDocumentStore( create_result_or.status().error_message()); } document_store_ = std::move(create_result_or.ValueOrDie().document_store); + result_state_manager_ = std::make_unique<ResultStateManager>( + performance_configuration_.max_num_total_hits, *document_store_); // Potential data loss // TODO(b/147373249): Find a way to detect true data loss error @@ -1616,6 +1636,8 @@ libtextclassifier3::Status IcingSearchEngine::OptimizeDocumentStore( "instance can't be created"); } document_store_ = std::move(create_result_or.ValueOrDie().document_store); + result_state_manager_ = std::make_unique<ResultStateManager>( + performance_configuration_.max_num_total_hits, *document_store_); // Deletes tmp directory if (!filesystem_->DeleteDirectoryRecursively( diff --git a/icing/icing-search-engine.h b/icing/icing-search-engine.h index 9adef7f..fa1e0c8 100644 --- a/icing/icing-search-engine.h +++ b/icing/icing-search-engine.h @@ -336,6 +336,19 @@ class IcingSearchEngine { // to disk. If the app crashes after a call to PersistToDisk(), Icing // would be able to fully recover all data written up to this point. // + // If persist_type is PersistType::LITE, then only the ground truth will be + // synced. This should be relatively lightweight to do (order of microseconds) + // and ensures that there will be no data loss. At worst, Icing may need to + // recover internal data structures by replaying the document log upon the + // next startup. Clients should call PersistToDisk(LITE) after each batch of + // mutations. + // + // If persist_type is PersistType::FULL, then all internal data structures in + // Icing will be synced. This is a heavier operation (order of milliseconds). + // It ensures that Icing will not need to recover internal data structures + // upon the next startup. Clients should call PersistToDisk(FULL) before their + // process dies. + // // NOTE: It is not necessary to call PersistToDisk() to read back data // that was recently written. All read APIs will include the most recent // updates/deletes regardless of the data being flushed to disk. @@ -344,7 +357,8 @@ class IcingSearchEngine { // OK on success // FAILED_PRECONDITION IcingSearchEngine has not been initialized yet // INTERNAL on I/O error - PersistToDiskResultProto PersistToDisk() ICING_LOCKS_EXCLUDED(mutex_); + PersistToDiskResultProto PersistToDisk(PersistType::Code persist_type) + ICING_LOCKS_EXCLUDED(mutex_); // Allows Icing to run tasks that are too expensive and/or unnecessary to be // executed in real-time, but are useful to keep it fast and be @@ -424,7 +438,8 @@ class IcingSearchEngine { // acquired first in order to adhere to the global lock ordering: // 1. mutex_ // 2. result_state_manager_.lock_ - ResultStateManager result_state_manager_ ICING_GUARDED_BY(mutex_); + std::unique_ptr<ResultStateManager> result_state_manager_ + ICING_GUARDED_BY(mutex_); // Used to provide reader and writer locks absl_ports::shared_mutex mutex_; @@ -450,8 +465,8 @@ class IcingSearchEngine { // separate method so that other public methods don't need to call // PersistToDisk(). Public methods calling each other may cause deadlock // issues. - libtextclassifier3::Status InternalPersistToDisk() - ICING_EXCLUSIVE_LOCKS_REQUIRED(mutex_); + libtextclassifier3::Status InternalPersistToDisk( + PersistType::Code persist_type) ICING_EXCLUSIVE_LOCKS_REQUIRED(mutex_); // Helper method to the actual work to Initialize. We need this separate // method so that other public methods don't need to call Initialize(). Public diff --git a/icing/icing-search-engine_flush_benchmark.cc b/icing/icing-search-engine_flush_benchmark.cc new file mode 100644 index 0000000..de8f550 --- /dev/null +++ b/icing/icing-search-engine_flush_benchmark.cc @@ -0,0 +1,200 @@ +// Copyright (C) 2019 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include <unistd.h> + +#include <fstream> +#include <iostream> +#include <memory> +#include <ostream> +#include <random> +#include <sstream> +#include <stdexcept> +#include <string> +#include <string_view> +#include <unordered_set> +#include <vector> + +#include "testing/base/public/benchmark.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" +#include "icing/document-builder.h" +#include "icing/file/filesystem.h" +#include "icing/icing-search-engine.h" +#include "icing/proto/document.pb.h" +#include "icing/proto/initialize.pb.h" +#include "icing/proto/schema.pb.h" +#include "icing/proto/status.pb.h" +#include "icing/proto/term.pb.h" +#include "icing/testing/common-matchers.h" +#include "icing/testing/document-generator.h" +#include "icing/testing/random-string.h" +#include "icing/testing/schema-generator.h" +#include "icing/testing/tmp-directory.h" + +// Run on a Linux workstation: +// $ blaze build -c opt --dynamic_mode=off --copt=-gmlt +// //icing:icing-search-engine_flush_benchmark +// +// $ blaze-bin/icing/icing-search-engine_flush_benchmark +// --benchmarks=all --benchmark_memory_usage +// +// Run on an Android device: +// $ blaze build --copt="-DGOOGLE_COMMANDLINEFLAGS_FULL_API=1" +// --config=android_arm64 -c opt --dynamic_mode=off --copt=-gmlt +// //icing:icing-search-engine_flush_benchmark +// +// $ adb push blaze-bin/icing/icing-search-engine_flush_benchmark +// /data/local/tmp/ +// +// $ adb shell /data/local/tmp/icing-search-engine_flush_benchmark +// --benchmarks=all + +namespace icing { +namespace lib { + +namespace { + +// Assume that there will be roughly 10 packages, each using 3 of its own types. +constexpr int kAvgNumNamespaces = 10; +constexpr int kAvgNumTypes = 3; + +// ASSUME: Types will have at most ten properties. Types will be created with +// [1, 10] properties. +constexpr int kMaxNumProperties = 10; + +// Based on logs from Icing GMSCore. +constexpr int kAvgDocumentSize = 300; + +// ASSUME: ~75% of the document's size comes from its content. +constexpr float kContentSizePct = 0.7; + +// Average length of word in English is 4.7 characters. +constexpr int kAvgTokenLen = 5; +// Made up value. This results in a fairly reasonable language - the majority of +// generated words are 3-9 characters, ~3% of words are >=20 chars, and the +// longest ones are 27 chars, (roughly consistent with the longest, +// non-contrived English words +// https://en.wikipedia.org/wiki/Longest_word_in_English) +constexpr int kTokenStdDev = 7; +constexpr int kLanguageSize = 1000; + +// The number of documents to index. +constexpr int kNumDocuments = 1024; + +std::vector<std::string> CreateNamespaces(int num_namespaces) { + std::vector<std::string> namespaces; + while (--num_namespaces >= 0) { + namespaces.push_back("comgooglepackage" + std::to_string(num_namespaces)); + } + return namespaces; +} + +// Creates a vector containing num_words randomly-generated words for use by +// documents. +template <typename Rand> +std::vector<std::string> CreateLanguage(int num_words, Rand* r) { + std::vector<std::string> language; + std::normal_distribution<> norm_dist(kAvgTokenLen, kTokenStdDev); + while (--num_words >= 0) { + int word_length = 0; + while (word_length < 1) { + word_length = std::round(norm_dist(*r)); + } + language.push_back(RandomString(kAlNumAlphabet, word_length, r)); + } + return language; +} + +class DestructibleDirectory { + public: + explicit DestructibleDirectory(const Filesystem& filesystem, + const std::string& dir) + : filesystem_(filesystem), dir_(dir) { + filesystem_.CreateDirectoryRecursively(dir_.c_str()); + } + ~DestructibleDirectory() { + filesystem_.DeleteDirectoryRecursively(dir_.c_str()); + } + + private: + Filesystem filesystem_; + std::string dir_; +}; + +void BM_FlushBenchmark(benchmark::State& state) { + PersistType::Code persist_type = + (state.range(0)) ? PersistType::LITE : PersistType::FULL; + int num_documents_per_persist = state.range(1); + + // Initialize the filesystem + std::string test_dir = GetTestTempDir() + "/icing/benchmark/flush"; + Filesystem filesystem; + DestructibleDirectory ddir(filesystem, test_dir); + + // Create the schema. + std::default_random_engine random; + int num_types = kAvgNumNamespaces * kAvgNumTypes; + ExactStringPropertyGenerator property_generator; + RandomSchemaGenerator<std::default_random_engine, + ExactStringPropertyGenerator> + schema_generator(&random, &property_generator); + SchemaProto schema = + schema_generator.GenerateSchema(num_types, kMaxNumProperties); + EvenDistributionTypeSelector type_selector(schema); + + std::vector<std::string> namespaces = CreateNamespaces(kAvgNumNamespaces); + EvenDistributionNamespaceSelector namespace_selector(namespaces); + + std::vector<std::string> language = CreateLanguage(kLanguageSize, &random); + UniformDistributionLanguageTokenGenerator<std::default_random_engine> + token_generator(language, &random); + + DocumentGenerator< + EvenDistributionNamespaceSelector, EvenDistributionTypeSelector, + UniformDistributionLanguageTokenGenerator<std::default_random_engine>> + generator(&namespace_selector, &type_selector, &token_generator, + kAvgDocumentSize * kContentSizePct); + + IcingSearchEngineOptions options; + options.set_base_dir(test_dir); + std::unique_ptr<IcingSearchEngine> icing = + std::make_unique<IcingSearchEngine>(options); + + ASSERT_THAT(icing->Initialize().status(), ProtoIsOk()); + ASSERT_THAT(icing->SetSchema(schema).status(), ProtoIsOk()); + for (auto s : state) { + for (int i = 0; i < kNumDocuments; ++i) { + icing->Put(generator.generateDoc()); + + if (i % num_documents_per_persist == num_documents_per_persist - 1) { + icing->PersistToDisk(persist_type); + } + } + } +} +BENCHMARK(BM_FlushBenchmark) + // First argument: lite_flush, + // Second argument: num_document_per_lite_flush + ->ArgPair(true, 1) + ->ArgPair(false, 1) + ->ArgPair(true, 32) + ->ArgPair(false, 32) + ->ArgPair(true, 1024) + ->ArgPair(false, 1024); + +} // namespace + +} // namespace lib +} // namespace icing diff --git a/icing/icing-search-engine_test.cc b/icing/icing-search-engine_test.cc index 64c62d0..471cc7b 100644 --- a/icing/icing-search-engine_test.cc +++ b/icing/icing-search-engine_test.cc @@ -32,6 +32,7 @@ #include "icing/portable/equals-proto.h" #include "icing/proto/document.pb.h" #include "icing/proto/initialize.pb.h" +#include "icing/proto/persist.pb.h" #include "icing/proto/optimize.pb.h" #include "icing/proto/schema.pb.h" #include "icing/proto/scoring.pb.h" @@ -103,7 +104,7 @@ class TestIcingSearchEngine : public IcingSearchEngine { TestIcingSearchEngine(const IcingSearchEngineOptions& options, std::unique_ptr<const Filesystem> filesystem, std::unique_ptr<const IcingFilesystem> icing_filesystem, - std::unique_ptr<FakeClock> clock, + std::unique_ptr<Clock> clock, std::unique_ptr<JniCache> jni_cache) : IcingSearchEngine(options, std::move(filesystem), std::move(icing_filesystem), std::move(clock), @@ -406,23 +407,23 @@ TEST_F(IcingSearchEngineTest, MaxTokenLenReturnsOkAndTruncatesTokens) { SearchResultProto actual_results = icing.Search(search_spec, GetDefaultScoringSpec(), ResultSpecProto::default_instance()); - EXPECT_THAT(actual_results, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(actual_results, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); // The query token is also truncated to length of 1, so "me"->"m" matches "m" search_spec.set_query("me"); actual_results = icing.Search(search_spec, GetDefaultScoringSpec(), ResultSpecProto::default_instance()); - EXPECT_THAT(actual_results, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(actual_results, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); // The query token is still truncated to length of 1, so "massage"->"m" // matches "m" search_spec.set_query("massage"); actual_results = icing.Search(search_spec, GetDefaultScoringSpec(), ResultSpecProto::default_instance()); - EXPECT_THAT(actual_results, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(actual_results, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); } TEST_F(IcingSearchEngineTest, @@ -458,8 +459,8 @@ TEST_F(IcingSearchEngineTest, SearchResultProto actual_results = icing.Search(search_spec, GetDefaultScoringSpec(), ResultSpecProto::default_instance()); - EXPECT_THAT(actual_results, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(actual_results, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); } TEST_F(IcingSearchEngineTest, FailToCreateDocStore) { @@ -574,7 +575,7 @@ TEST_F(IcingSearchEngineTest, FailToWriteSchema) { HasSubstr("Unable to open file for write")); } -TEST_F(IcingSearchEngineTest, SetSchemaDelete2) { +TEST_F(IcingSearchEngineTest, SetSchemaIncompatibleFails) { { IcingSearchEngine icing(GetDefaultIcingOptions(), GetTestJniCache()); ASSERT_THAT(icing.Initialize().status(), ProtoIsOk()); @@ -617,15 +618,18 @@ TEST_F(IcingSearchEngineTest, SetSchemaDelete2) { property->set_data_type(PropertyConfigProto::DataType::STRING); property->set_cardinality(PropertyConfigProto::Cardinality::OPTIONAL); - EXPECT_THAT(icing.SetSchema(schema, false).status(), - ProtoStatusIs(StatusProto::FAILED_PRECONDITION)); + EXPECT_THAT( + icing.SetSchema(schema, /*ignore_errors_and_delete_documents=*/false) + .status(), + ProtoStatusIs(StatusProto::FAILED_PRECONDITION)); - // 4. Try to delete by email type. + // 4. Try to delete by email type. This should succeed because email wasn't + // deleted in step 3. EXPECT_THAT(icing.DeleteBySchemaType("Email").status(), ProtoIsOk()); } } -TEST_F(IcingSearchEngineTest, SetSchemaDelete) { +TEST_F(IcingSearchEngineTest, SetSchemaIncompatibleForceOverrideSucceeds) { { IcingSearchEngine icing(GetDefaultIcingOptions(), GetTestJniCache()); ASSERT_THAT(icing.Initialize().status(), ProtoIsOk()); @@ -659,7 +663,8 @@ TEST_F(IcingSearchEngineTest, SetSchemaDelete) { IcingSearchEngine icing(GetDefaultIcingOptions(), GetTestJniCache()); ASSERT_THAT(icing.Initialize().status(), ProtoIsOk()); - // 3. Set a schema that deletes email. This should fail. + // 3. Set a schema that deletes email with force override. This should + // succeed and delete the email type. SchemaProto schema; SchemaTypeConfigProto* type = schema.add_types(); type->set_schema_type("Message"); @@ -670,7 +675,8 @@ TEST_F(IcingSearchEngineTest, SetSchemaDelete) { EXPECT_THAT(icing.SetSchema(schema, true).status(), ProtoIsOk()); - // 4. Try to delete by email type. + // 4. Try to delete by email type. This should fail because email was + // already deleted. EXPECT_THAT(icing.DeleteBySchemaType("Email").status(), ProtoStatusIs(StatusProto::NOT_FOUND)); } @@ -1004,7 +1010,8 @@ TEST_F(IcingSearchEngineTest, SetSchemaTriggersIndexRestorationAndReturnsOk) { SearchResultProto actual_results = icing.Search(search_spec, GetDefaultScoringSpec(), ResultSpecProto::default_instance()); - EXPECT_THAT(actual_results, EqualsSearchResultIgnoreStats(empty_result)); + EXPECT_THAT(actual_results, + EqualsSearchResultIgnoreStatsAndScores(empty_result)); SchemaProto schema_with_indexed_property = CreateMessageSchema(); // Index restoration should be triggered here because new schema requires more @@ -1018,8 +1025,8 @@ TEST_F(IcingSearchEngineTest, SetSchemaTriggersIndexRestorationAndReturnsOk) { CreateMessageDocument("namespace", "uri"); actual_results = icing.Search(search_spec, GetDefaultScoringSpec(), ResultSpecProto::default_instance()); - EXPECT_THAT(actual_results, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(actual_results, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); } TEST_F(IcingSearchEngineTest, SetSchemaRevalidatesDocumentsAndReturnsOk) { @@ -1501,8 +1508,79 @@ TEST_F(IcingSearchEngineTest, SearchReturnsValidResults) { SearchResultProto actual_results = icing.Search(search_spec, GetDefaultScoringSpec(), ResultSpecProto::default_instance()); - EXPECT_THAT(actual_results, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(actual_results, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); +} + +TEST_F(IcingSearchEngineTest, SearchReturnsScoresDocumentScore) { + IcingSearchEngine icing(GetDefaultIcingOptions(), GetTestJniCache()); + ASSERT_THAT(icing.Initialize().status(), ProtoIsOk()); + ASSERT_THAT(icing.SetSchema(CreateMessageSchema()).status(), ProtoIsOk()); + + DocumentProto document_one = CreateMessageDocument("namespace", "uri1"); + document_one.set_score(93); + document_one.set_creation_timestamp_ms(10000); + ASSERT_THAT(icing.Put(document_one).status(), ProtoIsOk()); + + DocumentProto document_two = CreateMessageDocument("namespace", "uri2"); + document_two.set_score(15); + document_two.set_creation_timestamp_ms(12000); + ASSERT_THAT(icing.Put(document_two).status(), ProtoIsOk()); + + SearchSpecProto search_spec; + search_spec.set_term_match_type(TermMatchType::PREFIX); + search_spec.set_query("message"); + + // Rank by DOCUMENT_SCORE and ensure that the score field is populated with + // document score. + ScoringSpecProto scoring_spec; + scoring_spec.set_rank_by(ScoringSpecProto::RankingStrategy::DOCUMENT_SCORE); + + SearchResultProto results = icing.Search(search_spec, scoring_spec, + ResultSpecProto::default_instance()); + EXPECT_THAT(results.status(), ProtoIsOk()); + EXPECT_THAT(results.results(), SizeIs(2)); + + EXPECT_THAT(results.results(0).document(), EqualsProto(document_one)); + EXPECT_THAT(results.results(0).score(), 93); + EXPECT_THAT(results.results(1).document(), EqualsProto(document_two)); + EXPECT_THAT(results.results(1).score(), 15); +} + +TEST_F(IcingSearchEngineTest, SearchReturnsScoresCreationTimestamp) { + IcingSearchEngine icing(GetDefaultIcingOptions(), GetTestJniCache()); + ASSERT_THAT(icing.Initialize().status(), ProtoIsOk()); + ASSERT_THAT(icing.SetSchema(CreateMessageSchema()).status(), ProtoIsOk()); + + DocumentProto document_one = CreateMessageDocument("namespace", "uri1"); + document_one.set_score(93); + document_one.set_creation_timestamp_ms(10000); + ASSERT_THAT(icing.Put(document_one).status(), ProtoIsOk()); + + DocumentProto document_two = CreateMessageDocument("namespace", "uri2"); + document_two.set_score(15); + document_two.set_creation_timestamp_ms(12000); + ASSERT_THAT(icing.Put(document_two).status(), ProtoIsOk()); + + SearchSpecProto search_spec; + search_spec.set_term_match_type(TermMatchType::PREFIX); + search_spec.set_query("message"); + + // Rank by CREATION_TS and ensure that the score field is populated with + // creation ts. + ScoringSpecProto scoring_spec; + scoring_spec.set_rank_by( + ScoringSpecProto::RankingStrategy::CREATION_TIMESTAMP); + + SearchResultProto results = icing.Search(search_spec, scoring_spec, + ResultSpecProto::default_instance()); + EXPECT_THAT(results.status(), ProtoIsOk()); + EXPECT_THAT(results.results(), SizeIs(2)); + + EXPECT_THAT(results.results(0).document(), EqualsProto(document_two)); + EXPECT_THAT(results.results(0).score(), 12000); + EXPECT_THAT(results.results(1).document(), EqualsProto(document_one)); + EXPECT_THAT(results.results(1).score(), 10000); } TEST_F(IcingSearchEngineTest, SearchReturnsOneResult) { @@ -1534,8 +1612,8 @@ TEST_F(IcingSearchEngineTest, SearchReturnsOneResult) { // The token is a random number so we don't verify it. expected_search_result_proto.set_next_page_token( search_result_proto.next_page_token()); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); } TEST_F(IcingSearchEngineTest, SearchZeroResultLimitReturnsEmptyResults) { @@ -1553,8 +1631,8 @@ TEST_F(IcingSearchEngineTest, SearchZeroResultLimitReturnsEmptyResults) { expected_search_result_proto.mutable_status()->set_code(StatusProto::OK); SearchResultProto actual_results = icing.Search(search_spec, GetDefaultScoringSpec(), result_spec); - EXPECT_THAT(actual_results, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(actual_results, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); } TEST_F(IcingSearchEngineTest, SearchNegativeResultLimitReturnsInvalidArgument) { @@ -1575,8 +1653,8 @@ TEST_F(IcingSearchEngineTest, SearchNegativeResultLimitReturnsInvalidArgument) { "ResultSpecProto.num_per_page cannot be negative."); SearchResultProto actual_results = icing.Search(search_spec, GetDefaultScoringSpec(), result_spec); - EXPECT_THAT(actual_results, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(actual_results, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); } TEST_F(IcingSearchEngineTest, SearchWithPersistenceReturnsValidResults) { @@ -1620,8 +1698,8 @@ TEST_F(IcingSearchEngineTest, SearchWithPersistenceReturnsValidResults) { SearchResultProto actual_results = icing.Search(search_spec, GetDefaultScoringSpec(), ResultSpecProto::default_instance()); - EXPECT_THAT(actual_results, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(actual_results, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); search_spec.set_query("foo"); @@ -1629,7 +1707,8 @@ TEST_F(IcingSearchEngineTest, SearchWithPersistenceReturnsValidResults) { empty_result.mutable_status()->set_code(StatusProto::OK); actual_results = icing.Search(search_spec, GetDefaultScoringSpec(), ResultSpecProto::default_instance()); - EXPECT_THAT(actual_results, EqualsSearchResultIgnoreStats(empty_result)); + EXPECT_THAT(actual_results, + EqualsSearchResultIgnoreStatsAndScores(empty_result)); } } @@ -1650,8 +1729,8 @@ TEST_F(IcingSearchEngineTest, SearchShouldReturnEmpty) { icing.Search(search_spec, GetDefaultScoringSpec(), ResultSpecProto::default_instance()); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); } TEST_F(IcingSearchEngineTest, SearchShouldReturnMultiplePages) { @@ -1691,8 +1770,8 @@ TEST_F(IcingSearchEngineTest, SearchShouldReturnMultiplePages) { uint64_t next_page_token = search_result_proto.next_page_token(); // Since the token is a random number, we don't need to verify expected_search_result_proto.set_next_page_token(next_page_token); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); // Second page, 2 results expected_search_result_proto.clear_results(); @@ -1701,8 +1780,8 @@ TEST_F(IcingSearchEngineTest, SearchShouldReturnMultiplePages) { *expected_search_result_proto.mutable_results()->Add()->mutable_document() = document2; search_result_proto = icing.GetNextPage(next_page_token); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); // Third page, 1 result expected_search_result_proto.clear_results(); @@ -1712,14 +1791,14 @@ TEST_F(IcingSearchEngineTest, SearchShouldReturnMultiplePages) { // token. expected_search_result_proto.clear_next_page_token(); search_result_proto = icing.GetNextPage(next_page_token); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); // No more results expected_search_result_proto.clear_results(); search_result_proto = icing.GetNextPage(next_page_token); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); } TEST_F(IcingSearchEngineTest, SearchWithNoScoringShouldReturnMultiplePages) { @@ -1762,8 +1841,8 @@ TEST_F(IcingSearchEngineTest, SearchWithNoScoringShouldReturnMultiplePages) { uint64_t next_page_token = search_result_proto.next_page_token(); // Since the token is a random number, we don't need to verify expected_search_result_proto.set_next_page_token(next_page_token); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); // Second page, 2 results expected_search_result_proto.clear_results(); @@ -1772,8 +1851,8 @@ TEST_F(IcingSearchEngineTest, SearchWithNoScoringShouldReturnMultiplePages) { *expected_search_result_proto.mutable_results()->Add()->mutable_document() = document2; search_result_proto = icing.GetNextPage(next_page_token); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); // Third page, 1 result expected_search_result_proto.clear_results(); @@ -1783,14 +1862,14 @@ TEST_F(IcingSearchEngineTest, SearchWithNoScoringShouldReturnMultiplePages) { // token. expected_search_result_proto.clear_next_page_token(); search_result_proto = icing.GetNextPage(next_page_token); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); // No more results expected_search_result_proto.clear_results(); search_result_proto = icing.GetNextPage(next_page_token); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); } TEST_F(IcingSearchEngineTest, ShouldReturnMultiplePagesWithSnippets) { @@ -1908,8 +1987,8 @@ TEST_F(IcingSearchEngineTest, ShouldInvalidateNextPageToken) { uint64_t next_page_token = search_result_proto.next_page_token(); // Since the token is a random number, we don't need to verify expected_search_result_proto.set_next_page_token(next_page_token); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); // Now document1 is still to be fetched. // Invalidates token @@ -1919,8 +1998,8 @@ TEST_F(IcingSearchEngineTest, ShouldInvalidateNextPageToken) { expected_search_result_proto.clear_results(); expected_search_result_proto.clear_next_page_token(); search_result_proto = icing.GetNextPage(next_page_token); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); } TEST_F(IcingSearchEngineTest, @@ -1952,8 +2031,8 @@ TEST_F(IcingSearchEngineTest, uint64_t next_page_token = search_result_proto.next_page_token(); // Since the token is a random number, we don't need to verify expected_search_result_proto.set_next_page_token(next_page_token); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); // Now document1 is still to be fetched. OptimizeResultProto optimize_result_proto; @@ -1968,8 +2047,8 @@ TEST_F(IcingSearchEngineTest, expected_search_result_proto.clear_results(); expected_search_result_proto.clear_next_page_token(); search_result_proto = icing.GetNextPage(next_page_token); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); } TEST_F(IcingSearchEngineTest, OptimizationShouldRemoveDeletedDocs) { @@ -2385,8 +2464,8 @@ TEST_F(IcingSearchEngineTest, DeleteBySchemaType) { SearchResultProto search_result_proto = icing.Search(search_spec, GetDefaultScoringSpec(), ResultSpecProto::default_instance()); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); } TEST_F(IcingSearchEngineTest, DeleteSchemaTypeByQuery) { @@ -2460,8 +2539,8 @@ TEST_F(IcingSearchEngineTest, DeleteSchemaTypeByQuery) { SearchResultProto search_result_proto = icing.Search(search_spec, GetDefaultScoringSpec(), ResultSpecProto::default_instance()); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); } TEST_F(IcingSearchEngineTest, DeleteByNamespace) { @@ -2561,8 +2640,8 @@ TEST_F(IcingSearchEngineTest, DeleteByNamespace) { SearchResultProto search_result_proto = icing.Search(search_spec, GetDefaultScoringSpec(), ResultSpecProto::default_instance()); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); } TEST_F(IcingSearchEngineTest, DeleteNamespaceByQuery) { @@ -2631,8 +2710,8 @@ TEST_F(IcingSearchEngineTest, DeleteNamespaceByQuery) { SearchResultProto search_result_proto = icing.Search(search_spec, GetDefaultScoringSpec(), ResultSpecProto::default_instance()); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); } TEST_F(IcingSearchEngineTest, DeleteByQuery) { @@ -2713,8 +2792,8 @@ TEST_F(IcingSearchEngineTest, DeleteByQuery) { SearchResultProto search_result_proto = icing.Search(search_spec, GetDefaultScoringSpec(), ResultSpecProto::default_instance()); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); } TEST_F(IcingSearchEngineTest, DeleteByQueryNotFound) { @@ -2786,8 +2865,8 @@ TEST_F(IcingSearchEngineTest, DeleteByQueryNotFound) { SearchResultProto search_result_proto = icing.Search(search_spec, GetDefaultScoringSpec(), ResultSpecProto::default_instance()); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); } TEST_F(IcingSearchEngineTest, SetSchemaShouldWorkAfterOptimization) { @@ -2850,8 +2929,8 @@ TEST_F(IcingSearchEngineTest, SearchShouldWorkAfterOptimization) { SearchResultProto search_result_proto = icing.Search(search_spec, GetDefaultScoringSpec(), ResultSpecProto::default_instance()); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); } // Destroys IcingSearchEngine to make sure nothing is cached. IcingSearchEngine icing(GetDefaultIcingOptions(), GetTestJniCache()); @@ -2859,8 +2938,8 @@ TEST_F(IcingSearchEngineTest, SearchShouldWorkAfterOptimization) { SearchResultProto search_result_proto = icing.Search(search_spec, GetDefaultScoringSpec(), ResultSpecProto::default_instance()); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); } TEST_F(IcingSearchEngineTest, IcingShouldWorkFineIfOptimizationIsAborted) { @@ -2915,8 +2994,8 @@ TEST_F(IcingSearchEngineTest, IcingShouldWorkFineIfOptimizationIsAborted) { SearchResultProto search_result_proto = icing.Search(search_spec, GetDefaultScoringSpec(), ResultSpecProto::default_instance()); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); } TEST_F(IcingSearchEngineTest, @@ -2976,8 +3055,8 @@ TEST_F(IcingSearchEngineTest, SearchResultProto search_result_proto = icing.Search(search_spec, GetDefaultScoringSpec(), ResultSpecProto::default_instance()); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); search_spec.set_query("n"); @@ -2987,8 +3066,8 @@ TEST_F(IcingSearchEngineTest, // Searching new content returns the new document search_result_proto = icing.Search(search_spec, GetDefaultScoringSpec(), ResultSpecProto::default_instance()); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); } TEST_F(IcingSearchEngineTest, OptimizationShouldRecoverIfDataFilesAreMissing) { @@ -3048,8 +3127,8 @@ TEST_F(IcingSearchEngineTest, OptimizationShouldRecoverIfDataFilesAreMissing) { SearchResultProto search_result_proto = icing.Search(search_spec, GetDefaultScoringSpec(), ResultSpecProto::default_instance()); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); search_spec.set_query("n"); @@ -3059,8 +3138,8 @@ TEST_F(IcingSearchEngineTest, OptimizationShouldRecoverIfDataFilesAreMissing) { // Searching new content returns the new document search_result_proto = icing.Search(search_spec, GetDefaultScoringSpec(), ResultSpecProto::default_instance()); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); } TEST_F(IcingSearchEngineTest, SearchIncludesDocumentsBeforeTtl) { @@ -3112,8 +3191,8 @@ TEST_F(IcingSearchEngineTest, SearchIncludesDocumentsBeforeTtl) { SearchResultProto search_result_proto = icing.Search(search_spec, GetDefaultScoringSpec(), ResultSpecProto::default_instance()); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); } TEST_F(IcingSearchEngineTest, SearchDoesntIncludeDocumentsPastTtl) { @@ -3163,8 +3242,8 @@ TEST_F(IcingSearchEngineTest, SearchDoesntIncludeDocumentsPastTtl) { SearchResultProto search_result_proto = icing.Search(search_spec, GetDefaultScoringSpec(), ResultSpecProto::default_instance()); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); } TEST_F(IcingSearchEngineTest, SearchWorksAfterSchemaTypesCompatiblyModified) { @@ -3202,8 +3281,8 @@ TEST_F(IcingSearchEngineTest, SearchWorksAfterSchemaTypesCompatiblyModified) { SearchResultProto search_result_proto = icing.Search(search_spec, GetDefaultScoringSpec(), ResultSpecProto::default_instance()); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); // With just the schema type filter, we can search for the message search_spec.Clear(); @@ -3214,8 +3293,8 @@ TEST_F(IcingSearchEngineTest, SearchWorksAfterSchemaTypesCompatiblyModified) { search_result_proto = icing.Search(search_spec, GetDefaultScoringSpec(), ResultSpecProto::default_instance()); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); // Since SchemaTypeIds are assigned based on order in the SchemaProto, this // will force a change in the DocumentStore's cached SchemaTypeIds @@ -3246,8 +3325,8 @@ TEST_F(IcingSearchEngineTest, SearchWorksAfterSchemaTypesCompatiblyModified) { // We can still search for the message document search_result_proto = icing.Search(search_spec, GetDefaultScoringSpec(), ResultSpecProto::default_instance()); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); } TEST_F(IcingSearchEngineTest, RecoverFromMissingHeaderFile) { @@ -3278,8 +3357,8 @@ TEST_F(IcingSearchEngineTest, RecoverFromMissingHeaderFile) { SearchResultProto search_result_proto = icing.Search(search_spec, GetDefaultScoringSpec(), ResultSpecProto::default_instance()); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); } // This should shut down IcingSearchEngine and persist anything it needs to EXPECT_TRUE(filesystem()->DeleteFile(GetHeaderFilename().c_str())); @@ -3297,8 +3376,8 @@ TEST_F(IcingSearchEngineTest, RecoverFromMissingHeaderFile) { SearchResultProto search_result_proto = icing.Search(search_spec, GetDefaultScoringSpec(), ResultSpecProto::default_instance()); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); // Checks that Schema is still since it'll be needed to validate the document EXPECT_THAT(icing.Put(CreateMessageDocument("namespace", "uri")).status(), @@ -3333,8 +3412,8 @@ TEST_F(IcingSearchEngineTest, RecoverFromInvalidHeaderMagic) { SearchResultProto search_result_proto = icing.Search(search_spec, GetDefaultScoringSpec(), ResultSpecProto::default_instance()); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); } // This should shut down IcingSearchEngine and persist anything it needs to // Change the header's magic value @@ -3356,8 +3435,8 @@ TEST_F(IcingSearchEngineTest, RecoverFromInvalidHeaderMagic) { SearchResultProto search_result_proto = icing.Search(search_spec, GetDefaultScoringSpec(), ResultSpecProto::default_instance()); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); // Checks that Schema is still since it'll be needed to validate the document EXPECT_THAT(icing.Put(CreateMessageDocument("namespace", "uri")).status(), @@ -3392,8 +3471,8 @@ TEST_F(IcingSearchEngineTest, RecoverFromInvalidHeaderChecksum) { SearchResultProto search_result_proto = icing.Search(search_spec, GetDefaultScoringSpec(), ResultSpecProto::default_instance()); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); } // This should shut down IcingSearchEngine and persist anything it needs to // Change the header's checksum value @@ -3416,8 +3495,8 @@ TEST_F(IcingSearchEngineTest, RecoverFromInvalidHeaderChecksum) { SearchResultProto search_result_proto = icing.Search(search_spec, GetDefaultScoringSpec(), ResultSpecProto::default_instance()); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); // Checks that Schema is still since it'll be needed to validate the document EXPECT_THAT(icing.Put(CreateMessageDocument("namespace", "uri")).status(), @@ -3534,8 +3613,8 @@ TEST_F(IcingSearchEngineTest, RecoverFromInconsistentSchemaStore) { SearchResultProto search_result_proto = icing.Search(search_spec, GetDefaultScoringSpec(), ResultSpecProto::default_instance()); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); } // This should shut down IcingSearchEngine and persist anything it needs to { @@ -3617,8 +3696,8 @@ TEST_F(IcingSearchEngineTest, RecoverFromInconsistentSchemaStore) { SearchResultProto search_result_proto = icing.Search(search_spec, GetDefaultScoringSpec(), ResultSpecProto::default_instance()); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); } TEST_F(IcingSearchEngineTest, RecoverFromInconsistentDocumentStore) { @@ -3686,8 +3765,8 @@ TEST_F(IcingSearchEngineTest, RecoverFromInconsistentDocumentStore) { SearchResultProto search_result_proto = icing.Search(search_spec, GetDefaultScoringSpec(), ResultSpecProto::default_instance()); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); } TEST_F(IcingSearchEngineTest, RecoverFromInconsistentIndex) { @@ -3710,8 +3789,8 @@ TEST_F(IcingSearchEngineTest, RecoverFromInconsistentIndex) { SearchResultProto search_result_proto = icing.Search(search_spec, GetDefaultScoringSpec(), ResultSpecProto::default_instance()); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); } // This should shut down IcingSearchEngine and persist anything it needs to // Pretend we lost the entire index @@ -3725,8 +3804,8 @@ TEST_F(IcingSearchEngineTest, RecoverFromInconsistentIndex) { SearchResultProto search_result_proto = icing.Search(search_spec, GetDefaultScoringSpec(), ResultSpecProto::default_instance()); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); } TEST_F(IcingSearchEngineTest, RecoverFromCorruptIndex) { @@ -3749,8 +3828,8 @@ TEST_F(IcingSearchEngineTest, RecoverFromCorruptIndex) { SearchResultProto search_result_proto = icing.Search(search_spec, GetDefaultScoringSpec(), ResultSpecProto::default_instance()); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); } // This should shut down IcingSearchEngine and persist anything it needs to // Pretend index is corrupted @@ -3766,8 +3845,8 @@ TEST_F(IcingSearchEngineTest, RecoverFromCorruptIndex) { SearchResultProto search_result_proto = icing.Search(search_spec, GetDefaultScoringSpec(), ResultSpecProto::default_instance()); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); } TEST_F(IcingSearchEngineTest, SearchResultShouldBeRankedByDocumentScore) { @@ -3827,8 +3906,8 @@ TEST_F(IcingSearchEngineTest, SearchResultShouldBeRankedByDocumentScore) { scoring_spec.set_rank_by(ScoringSpecProto::RankingStrategy::DOCUMENT_SCORE); SearchResultProto search_result_proto = icing.Search( search_spec, scoring_spec, ResultSpecProto::default_instance()); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); } TEST_F(IcingSearchEngineTest, SearchShouldAllowNoScoring) { @@ -3886,8 +3965,8 @@ TEST_F(IcingSearchEngineTest, SearchShouldAllowNoScoring) { scoring_spec.set_rank_by(ScoringSpecProto::RankingStrategy::NONE); SearchResultProto search_result_proto = icing.Search( search_spec, scoring_spec, ResultSpecProto::default_instance()); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); } TEST_F(IcingSearchEngineTest, SearchResultShouldBeRankedByCreationTimestamp) { @@ -3942,8 +4021,8 @@ TEST_F(IcingSearchEngineTest, SearchResultShouldBeRankedByCreationTimestamp) { ScoringSpecProto::RankingStrategy::CREATION_TIMESTAMP); SearchResultProto search_result_proto = icing.Search( search_spec, scoring_spec, ResultSpecProto::default_instance()); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); } TEST_F(IcingSearchEngineTest, SearchResultShouldBeRankedByUsageCount) { @@ -4013,8 +4092,8 @@ TEST_F(IcingSearchEngineTest, SearchResultShouldBeRankedByUsageCount) { ScoringSpecProto::RankingStrategy::USAGE_TYPE1_COUNT); SearchResultProto search_result_proto = icing.Search( search_spec, scoring_spec, ResultSpecProto::default_instance()); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); } TEST_F(IcingSearchEngineTest, @@ -4071,8 +4150,8 @@ TEST_F(IcingSearchEngineTest, ScoringSpecProto::RankingStrategy::USAGE_TYPE1_COUNT); SearchResultProto search_result_proto = icing.Search( search_spec, scoring_spec, ResultSpecProto::default_instance()); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); } TEST_F(IcingSearchEngineTest, SearchResultShouldBeRankedByUsageTimestamp) { @@ -4141,8 +4220,8 @@ TEST_F(IcingSearchEngineTest, SearchResultShouldBeRankedByUsageTimestamp) { ScoringSpecProto::RankingStrategy::USAGE_TYPE1_LAST_USED_TIMESTAMP); SearchResultProto search_result_proto = icing.Search( search_spec, scoring_spec, ResultSpecProto::default_instance()); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); } TEST_F(IcingSearchEngineTest, Bm25fRelevanceScoringOneNamespace) { @@ -4582,8 +4661,8 @@ TEST_F(IcingSearchEngineTest, ScoringSpecProto::RankingStrategy::USAGE_TYPE1_LAST_USED_TIMESTAMP); SearchResultProto search_result_proto = icing.Search( search_spec, scoring_spec, ResultSpecProto::default_instance()); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); } TEST_F(IcingSearchEngineTest, OlderUsageTimestampShouldNotOverrideNewerOnes) { @@ -4651,8 +4730,8 @@ TEST_F(IcingSearchEngineTest, OlderUsageTimestampShouldNotOverrideNewerOnes) { ScoringSpecProto::RankingStrategy::USAGE_TYPE1_LAST_USED_TIMESTAMP); SearchResultProto search_result_proto = icing.Search( search_spec, scoring_spec, ResultSpecProto::default_instance()); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); } TEST_F(IcingSearchEngineTest, SearchResultShouldBeRankedAscendingly) { @@ -4713,8 +4792,218 @@ TEST_F(IcingSearchEngineTest, SearchResultShouldBeRankedAscendingly) { scoring_spec.set_order_by(ScoringSpecProto::Order::ASC); SearchResultProto search_result_proto = icing.Search( search_spec, scoring_spec, ResultSpecProto::default_instance()); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); +} + +TEST_F(IcingSearchEngineTest, + SearchResultGroupingDuplicateNamespaceShouldReturnError) { + IcingSearchEngine icing(GetDefaultIcingOptions(), GetTestJniCache()); + EXPECT_THAT(icing.Initialize().status(), ProtoIsOk()); + EXPECT_THAT(icing.SetSchema(CreateMessageSchema()).status(), ProtoIsOk()); + + // Creates 2 documents and ensures the relationship in terms of document + // score is: document1 < document2 + DocumentProto document1 = + DocumentBuilder() + .SetKey("namespace1", "uri/1") + .SetSchema("Message") + .AddStringProperty("body", "message1") + .SetScore(1) + .SetCreationTimestampMs(kDefaultCreationTimestampMs) + .Build(); + DocumentProto document2 = + DocumentBuilder() + .SetKey("namespace2", "uri/2") + .SetSchema("Message") + .AddStringProperty("body", "message2") + .SetScore(2) + .SetCreationTimestampMs(kDefaultCreationTimestampMs) + .Build(); + + ASSERT_THAT(icing.Put(document1).status(), ProtoIsOk()); + ASSERT_THAT(icing.Put(document2).status(), ProtoIsOk()); + + // "m" will match all 2 documents + SearchSpecProto search_spec; + search_spec.set_term_match_type(TermMatchType::PREFIX); + search_spec.set_query("m"); + + ScoringSpecProto scoring_spec = GetDefaultScoringSpec(); + scoring_spec.set_rank_by(ScoringSpecProto::RankingStrategy::DOCUMENT_SCORE); + + // Specify "namespace1" twice. This should result in an error. + ResultSpecProto result_spec; + ResultSpecProto::ResultGrouping* result_grouping = + result_spec.add_result_groupings(); + result_grouping->set_max_results(1); + result_grouping->add_namespaces("namespace1"); + result_grouping->add_namespaces("namespace2"); + result_grouping = result_spec.add_result_groupings(); + result_grouping->set_max_results(1); + result_grouping->add_namespaces("namespace1"); + + SearchResultProto search_result_proto = + icing.Search(search_spec, scoring_spec, result_spec); + EXPECT_THAT(search_result_proto.status(), + ProtoStatusIs(StatusProto::INVALID_ARGUMENT)); +} + +TEST_F(IcingSearchEngineTest, + SearchResultGroupingNonPositiveMaxResultsShouldReturnError) { + IcingSearchEngine icing(GetDefaultIcingOptions(), GetTestJniCache()); + EXPECT_THAT(icing.Initialize().status(), ProtoIsOk()); + EXPECT_THAT(icing.SetSchema(CreateMessageSchema()).status(), ProtoIsOk()); + + // Creates 2 documents and ensures the relationship in terms of document + // score is: document1 < document2 + DocumentProto document1 = + DocumentBuilder() + .SetKey("namespace1", "uri/1") + .SetSchema("Message") + .AddStringProperty("body", "message1") + .SetScore(1) + .SetCreationTimestampMs(kDefaultCreationTimestampMs) + .Build(); + DocumentProto document2 = + DocumentBuilder() + .SetKey("namespace2", "uri/2") + .SetSchema("Message") + .AddStringProperty("body", "message2") + .SetScore(2) + .SetCreationTimestampMs(kDefaultCreationTimestampMs) + .Build(); + + ASSERT_THAT(icing.Put(document1).status(), ProtoIsOk()); + ASSERT_THAT(icing.Put(document2).status(), ProtoIsOk()); + + // "m" will match all 2 documents + SearchSpecProto search_spec; + search_spec.set_term_match_type(TermMatchType::PREFIX); + search_spec.set_query("m"); + + ScoringSpecProto scoring_spec = GetDefaultScoringSpec(); + scoring_spec.set_rank_by(ScoringSpecProto::RankingStrategy::DOCUMENT_SCORE); + + // Specify zero results. This should result in an error. + ResultSpecProto result_spec; + ResultSpecProto::ResultGrouping* result_grouping = + result_spec.add_result_groupings(); + result_grouping->set_max_results(0); + result_grouping->add_namespaces("namespace1"); + result_grouping->add_namespaces("namespace2"); + + SearchResultProto search_result_proto = + icing.Search(search_spec, scoring_spec, result_spec); + EXPECT_THAT(search_result_proto.status(), + ProtoStatusIs(StatusProto::INVALID_ARGUMENT)); + + // Specify negative results. This should result in an error. + result_spec.mutable_result_groupings(0)->set_max_results(-1); + EXPECT_THAT(search_result_proto.status(), + ProtoStatusIs(StatusProto::INVALID_ARGUMENT)); +} + +TEST_F(IcingSearchEngineTest, SearchResultGroupingMultiNamespaceGrouping) { + IcingSearchEngine icing(GetDefaultIcingOptions(), GetTestJniCache()); + EXPECT_THAT(icing.Initialize().status(), ProtoIsOk()); + EXPECT_THAT(icing.SetSchema(CreateMessageSchema()).status(), ProtoIsOk()); + + // Creates 3 documents and ensures the relationship in terms of document + // score is: document1 < document2 < document3 < document4 < document5 < + // document6 + DocumentProto document1 = + DocumentBuilder() + .SetKey("namespace1", "uri/1") + .SetSchema("Message") + .AddStringProperty("body", "message1") + .SetScore(1) + .SetCreationTimestampMs(kDefaultCreationTimestampMs) + .Build(); + DocumentProto document2 = + DocumentBuilder() + .SetKey("namespace1", "uri/2") + .SetSchema("Message") + .AddStringProperty("body", "message2") + .SetScore(2) + .SetCreationTimestampMs(kDefaultCreationTimestampMs) + .Build(); + DocumentProto document3 = + DocumentBuilder() + .SetKey("namespace2", "uri/3") + .SetSchema("Message") + .AddStringProperty("body", "message3") + .SetScore(3) + .SetCreationTimestampMs(kDefaultCreationTimestampMs) + .Build(); + DocumentProto document4 = + DocumentBuilder() + .SetKey("namespace2", "uri/4") + .SetSchema("Message") + .AddStringProperty("body", "message1") + .SetScore(4) + .SetCreationTimestampMs(kDefaultCreationTimestampMs) + .Build(); + DocumentProto document5 = + DocumentBuilder() + .SetKey("namespace3", "uri/5") + .SetSchema("Message") + .AddStringProperty("body", "message3") + .SetScore(5) + .SetCreationTimestampMs(kDefaultCreationTimestampMs) + .Build(); + DocumentProto document6 = + DocumentBuilder() + .SetKey("namespace3", "uri/6") + .SetSchema("Message") + .AddStringProperty("body", "message1") + .SetScore(6) + .SetCreationTimestampMs(kDefaultCreationTimestampMs) + .Build(); + + ASSERT_THAT(icing.Put(document1).status(), ProtoIsOk()); + ASSERT_THAT(icing.Put(document2).status(), ProtoIsOk()); + ASSERT_THAT(icing.Put(document3).status(), ProtoIsOk()); + ASSERT_THAT(icing.Put(document4).status(), ProtoIsOk()); + ASSERT_THAT(icing.Put(document5).status(), ProtoIsOk()); + ASSERT_THAT(icing.Put(document6).status(), ProtoIsOk()); + + // "m" will match all 6 documents + SearchSpecProto search_spec; + search_spec.set_term_match_type(TermMatchType::PREFIX); + search_spec.set_query("m"); + + ScoringSpecProto scoring_spec = GetDefaultScoringSpec(); + scoring_spec.set_rank_by(ScoringSpecProto::RankingStrategy::DOCUMENT_SCORE); + + ResultSpecProto result_spec; + ResultSpecProto::ResultGrouping* result_grouping = + result_spec.add_result_groupings(); + result_grouping->set_max_results(1); + result_grouping->add_namespaces("namespace1"); + result_grouping = result_spec.add_result_groupings(); + result_grouping->set_max_results(2); + result_grouping->add_namespaces("namespace2"); + result_grouping->add_namespaces("namespace3"); + + SearchResultProto search_result_proto = + icing.Search(search_spec, scoring_spec, result_spec); + + // The last result (document1) in namespace "namespace1" should not be + // included. "namespace2" and "namespace3" are grouped together. So only the + // two highest scored documents between the two (both of which are in + // "namespace3") should be returned. + SearchResultProto expected_search_result_proto; + expected_search_result_proto.mutable_status()->set_code(StatusProto::OK); + *expected_search_result_proto.mutable_results()->Add()->mutable_document() = + document6; + *expected_search_result_proto.mutable_results()->Add()->mutable_document() = + document5; + *expected_search_result_proto.mutable_results()->Add()->mutable_document() = + document2; + EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EqualsSearchResultIgnoreStatsAndScores(expected_search_result_proto)); } TEST_F(IcingSearchEngineTest, @@ -4796,8 +5085,8 @@ TEST_F(IcingSearchEngineTest, SetSchemaCanDetectPreviousSchemaWasLost) { SearchResultProto search_result_proto = icing.Search(search_spec, GetDefaultScoringSpec(), ResultSpecProto::default_instance()); - EXPECT_THAT(search_result_proto, - EqualsSearchResultIgnoreStats(expected_search_result_proto)); + EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); } // This should shut down IcingSearchEngine and persist anything it needs to ASSERT_TRUE(filesystem()->DeleteDirectoryRecursively(GetSchemaDir().c_str())); @@ -4823,7 +5112,8 @@ TEST_F(IcingSearchEngineTest, SetSchemaCanDetectPreviousSchemaWasLost) { SearchResultProto search_result_proto = icing.Search(search_spec, GetDefaultScoringSpec(), ResultSpecProto::default_instance()); - EXPECT_THAT(search_result_proto, EqualsSearchResultIgnoreStats(empty_result)); + EXPECT_THAT(search_result_proto, + EqualsSearchResultIgnoreStatsAndScores(empty_result)); } TEST_F(IcingSearchEngineTest, PersistToDisk) { @@ -4840,7 +5130,7 @@ TEST_F(IcingSearchEngineTest, PersistToDisk) { ProtoIsOk()); // Persisting shouldn't affect anything - EXPECT_THAT(icing.PersistToDisk().status(), ProtoIsOk()); + EXPECT_THAT(icing.PersistToDisk(PersistType::FULL).status(), ProtoIsOk()); EXPECT_THAT( icing.Get("namespace", "uri", GetResultSpecProto::default_instance()), @@ -4854,6 +5144,48 @@ TEST_F(IcingSearchEngineTest, PersistToDisk) { EqualsProto(expected_get_result_proto)); } +TEST_F(IcingSearchEngineTest, NoPersistToDiskLiteDoesntPersistPut) { + IcingSearchEngine icing1(GetDefaultIcingOptions(), GetTestJniCache()); + EXPECT_THAT(icing1.Initialize().status(), ProtoIsOk()); + EXPECT_THAT(icing1.SetSchema(CreateMessageSchema()).status(), ProtoIsOk()); + DocumentProto document1 = CreateMessageDocument("namespace", "uri"); + EXPECT_THAT(icing1.Put(document1).status(), ProtoIsOk()); + EXPECT_THAT( + icing1.Get("namespace", "uri", GetResultSpecProto::default_instance()) + .document(), + EqualsProto(document1)); + + IcingSearchEngine icing2(GetDefaultIcingOptions(), GetTestJniCache()); + EXPECT_THAT(icing2.Initialize().status(), ProtoIsOk()); + // The document shouldn't be found because we forgot to call + // PersistToDisk(LITE)! + EXPECT_THAT( + icing2.Get("namespace", "uri", GetResultSpecProto::default_instance()) + .status(), + ProtoStatusIs(StatusProto::NOT_FOUND)); +} + +TEST_F(IcingSearchEngineTest, PersistToDiskLitePersistsPut) { + IcingSearchEngine icing1(GetDefaultIcingOptions(), GetTestJniCache()); + EXPECT_THAT(icing1.Initialize().status(), ProtoIsOk()); + EXPECT_THAT(icing1.SetSchema(CreateMessageSchema()).status(), ProtoIsOk()); + DocumentProto document1 = CreateMessageDocument("namespace", "uri"); + EXPECT_THAT(icing1.Put(document1).status(), ProtoIsOk()); + EXPECT_THAT(icing1.PersistToDisk(PersistType::LITE).status(), ProtoIsOk()); + EXPECT_THAT( + icing1.Get("namespace", "uri", GetResultSpecProto::default_instance()) + .document(), + EqualsProto(document1)); + + IcingSearchEngine icing2(GetDefaultIcingOptions(), GetTestJniCache()); + EXPECT_THAT(icing2.Initialize().status(), ProtoIsOk()); + // The document should be found because we called PersistToDisk(LITE)! + EXPECT_THAT( + icing2.Get("namespace", "uri", GetResultSpecProto::default_instance()) + .document(), + EqualsProto(document1)); +} + TEST_F(IcingSearchEngineTest, ResetOk) { SchemaProto message_schema = CreateMessageSchema(); SchemaProto empty_schema = SchemaProto(message_schema); @@ -5151,7 +5483,7 @@ TEST_F(IcingSearchEngineTest, UninitializedInstanceFailsSafely) { ProtoStatusIs(StatusProto::FAILED_PRECONDITION)); icing.InvalidateNextPageToken(kSomePageToken); // Verify this doesn't crash. - EXPECT_THAT(icing.PersistToDisk().status(), + EXPECT_THAT(icing.PersistToDisk(PersistType::FULL).status(), ProtoStatusIs(StatusProto::FAILED_PRECONDITION)); EXPECT_THAT(icing.Optimize().status(), ProtoStatusIs(StatusProto::FAILED_PRECONDITION)); @@ -6581,7 +6913,7 @@ TEST_F(IcingSearchEngineTest, OptimizeStatsProtoTest) { // Delete the first document. ASSERT_THAT(icing->Delete(document1.namespace_(), document1.uri()).status(), ProtoIsOk()); - ASSERT_THAT(icing->PersistToDisk().status(), ProtoIsOk()); + ASSERT_THAT(icing->PersistToDisk(PersistType::FULL).status(), ProtoIsOk()); OptimizeStatsProto expected; expected.set_latency_ms(5); diff --git a/icing/index/index-processor_test.cc b/icing/index/index-processor_test.cc index e6bb615..20e14e9 100644 --- a/icing/index/index-processor_test.cc +++ b/icing/index/index-processor_test.cc @@ -39,6 +39,7 @@ #include "icing/proto/document.pb.h" #include "icing/proto/schema.pb.h" #include "icing/proto/term.pb.h" +#include "icing/schema-builder.h" #include "icing/schema/schema-store.h" #include "icing/schema/schema-util.h" #include "icing/schema/section-manager.h" @@ -103,6 +104,22 @@ using ::testing::Eq; using ::testing::IsEmpty; using ::testing::Test; +constexpr PropertyConfigProto_DataType_Code TYPE_STRING = + PropertyConfigProto_DataType_Code_STRING; +constexpr PropertyConfigProto_DataType_Code TYPE_BYTES = + PropertyConfigProto_DataType_Code_BYTES; + +constexpr PropertyConfigProto_Cardinality_Code CARDINALITY_OPTIONAL = + PropertyConfigProto_Cardinality_Code_OPTIONAL; +constexpr PropertyConfigProto_Cardinality_Code CARDINALITY_REPEATED = + PropertyConfigProto_Cardinality_Code_REPEATED; + +constexpr StringIndexingConfig_TokenizerType_Code TOKENIZER_PLAIN = + StringIndexingConfig_TokenizerType_Code_PLAIN; + +constexpr TermMatchType_Code MATCH_EXACT = TermMatchType_Code_EXACT_ONLY; +constexpr TermMatchType_Code MATCH_PREFIX = TermMatchType_Code_PREFIX; + class IndexProcessorTest : public Test { protected: void SetUp() override { @@ -131,7 +148,49 @@ class IndexProcessorTest : public Test { ICING_ASSERT_OK_AND_ASSIGN( schema_store_, SchemaStore::Create(&filesystem_, GetTestTempDir(), &fake_clock_)); - SchemaProto schema = CreateFakeSchema(); + SchemaProto schema = + SchemaBuilder() + .AddType( + SchemaTypeConfigBuilder() + .SetType(kFakeType) + .AddProperty( + PropertyConfigBuilder() + .SetName(kExactProperty) + .SetDataTypeString(MATCH_EXACT, TOKENIZER_PLAIN) + .SetCardinality(CARDINALITY_OPTIONAL)) + .AddProperty( + PropertyConfigBuilder() + .SetName(kPrefixedProperty) + .SetDataTypeString(MATCH_PREFIX, TOKENIZER_PLAIN) + .SetCardinality(CARDINALITY_OPTIONAL)) + .AddProperty(PropertyConfigBuilder() + .SetName(kUnindexedProperty1) + .SetDataType(TYPE_STRING) + .SetCardinality(CARDINALITY_OPTIONAL)) + .AddProperty(PropertyConfigBuilder() + .SetName(kUnindexedProperty2) + .SetDataType(TYPE_BYTES) + .SetCardinality(CARDINALITY_OPTIONAL)) + .AddProperty( + PropertyConfigBuilder() + .SetName(kRepeatedProperty) + .SetDataTypeString(MATCH_PREFIX, TOKENIZER_PLAIN) + .SetCardinality(CARDINALITY_REPEATED)) + .AddProperty( + PropertyConfigBuilder() + .SetName(kSubProperty) + .SetDataTypeDocument( + kNestedType, /*index_nested_properties=*/true) + .SetCardinality(CARDINALITY_OPTIONAL))) + .AddType( + SchemaTypeConfigBuilder() + .SetType(kNestedType) + .AddProperty( + PropertyConfigBuilder() + .SetName(kNestedProperty) + .SetDataTypeString(MATCH_PREFIX, TOKENIZER_PLAIN) + .SetCardinality(CARDINALITY_OPTIONAL))) + .Build(); ICING_ASSERT_OK(schema_store_->SetSchema(schema)); IndexProcessor::Options processor_options; @@ -162,72 +221,6 @@ class IndexProcessorTest : public Test { std::unique_ptr<Index> index_; std::unique_ptr<SchemaStore> schema_store_; std::unique_ptr<IndexProcessor> index_processor_; - - private: - static void AddStringProperty(std::string_view name, DataType::Code type, - Cardinality::Code cardinality, - TermMatchType::Code term_match_type, - SchemaTypeConfigProto* type_config) { - auto* prop = type_config->add_properties(); - prop->set_property_name(std::string(name)); - prop->set_data_type(type); - prop->set_cardinality(cardinality); - prop->mutable_string_indexing_config()->set_term_match_type( - term_match_type); - prop->mutable_string_indexing_config()->set_tokenizer_type( - StringIndexingConfig::TokenizerType::PLAIN); - } - - static void AddNonIndexedProperty(std::string_view name, DataType::Code type, - Cardinality::Code cardinality, - SchemaTypeConfigProto* type_config) { - auto* prop = type_config->add_properties(); - prop->set_property_name(std::string(name)); - prop->set_data_type(type); - prop->set_cardinality(cardinality); - } - - static SchemaProto CreateFakeSchema() { - SchemaProto schema; - - // Add top-level type - auto* type_config = schema.add_types(); - type_config->set_schema_type(std::string(kFakeType)); - - AddStringProperty(std::string(kExactProperty), DataType::STRING, - Cardinality::OPTIONAL, TermMatchType::EXACT_ONLY, - type_config); - - AddStringProperty(std::string(kPrefixedProperty), DataType::STRING, - Cardinality::OPTIONAL, TermMatchType::PREFIX, - type_config); - - AddNonIndexedProperty(std::string(kUnindexedProperty1), DataType::STRING, - Cardinality::OPTIONAL, type_config); - - AddNonIndexedProperty(std::string(kUnindexedProperty2), DataType::BYTES, - Cardinality::OPTIONAL, type_config); - - AddStringProperty(std::string(kRepeatedProperty), DataType::STRING, - Cardinality::REPEATED, TermMatchType::PREFIX, - type_config); - - auto* prop = type_config->add_properties(); - prop->set_property_name(std::string(kSubProperty)); - prop->set_data_type(DataType::DOCUMENT); - prop->set_cardinality(Cardinality::OPTIONAL); - prop->set_schema_type(std::string(kNestedType)); - prop->mutable_document_indexing_config()->set_index_nested_properties(true); - - // Add nested type - type_config = schema.add_types(); - type_config->set_schema_type(std::string(kNestedType)); - - AddStringProperty(kNestedProperty, DataType::STRING, Cardinality::OPTIONAL, - TermMatchType::PREFIX, type_config); - - return schema; - } }; std::vector<DocHitInfo> GetHits(std::unique_ptr<DocHitInfoIterator> iterator) { diff --git a/icing/index/iterator/doc-hit-info-iterator-filter_test.cc b/icing/index/iterator/doc-hit-info-iterator-filter_test.cc index e0a8cd0..8e48123 100644 --- a/icing/index/iterator/doc-hit-info-iterator-filter_test.cc +++ b/icing/index/iterator/doc-hit-info-iterator-filter_test.cc @@ -28,6 +28,7 @@ #include "icing/index/iterator/doc-hit-info-iterator-test-util.h" #include "icing/index/iterator/doc-hit-info-iterator.h" #include "icing/proto/document.pb.h" +#include "icing/schema-builder.h" #include "icing/schema/schema-store.h" #include "icing/schema/section.h" #include "icing/store/document-id.h" @@ -59,10 +60,10 @@ class DocHitInfoIteratorDeletedFilterTest : public ::testing::Test { test_document3_ = DocumentBuilder().SetKey("icing", "email/3").SetSchema("email").Build(); - SchemaProto schema; - auto type_config = schema.add_types(); - type_config->set_schema_type("email"); - + SchemaProto schema = + SchemaBuilder() + .AddType(SchemaTypeConfigBuilder().SetType("email")) + .Build(); ICING_ASSERT_OK_AND_ASSIGN( schema_store_, SchemaStore::Create(&filesystem_, test_dir_, &fake_clock_)); @@ -226,10 +227,10 @@ class DocHitInfoIteratorNamespaceFilterTest : public ::testing::Test { .SetSchema("email") .Build(); - SchemaProto schema; - auto type_config = schema.add_types(); - type_config->set_schema_type("email"); - + SchemaProto schema = + SchemaBuilder() + .AddType(SchemaTypeConfigBuilder().SetType("email")) + .Build(); ICING_ASSERT_OK_AND_ASSIGN( schema_store_, SchemaStore::Create(&filesystem_, test_dir_, &fake_clock_)); @@ -379,14 +380,12 @@ class DocHitInfoIteratorSchemaTypeFilterTest : public ::testing::Test { document4_schema1_ = DocumentBuilder().SetKey("namespace", "4").SetSchema(schema1_).Build(); - SchemaProto schema; - auto type_config = schema.add_types(); - type_config->set_schema_type(schema1_); - type_config = schema.add_types(); - type_config->set_schema_type(schema2_); - type_config = schema.add_types(); - type_config->set_schema_type(schema3_); - + SchemaProto schema = + SchemaBuilder() + .AddType(SchemaTypeConfigBuilder().SetType(schema1_)) + .AddType(SchemaTypeConfigBuilder().SetType(schema2_)) + .AddType(SchemaTypeConfigBuilder().SetType(schema3_)) + .Build(); ICING_ASSERT_OK_AND_ASSIGN( schema_store_, SchemaStore::Create(&filesystem_, test_dir_, &fake_clock_)); @@ -523,10 +522,10 @@ class DocHitInfoIteratorExpirationFilterTest : public ::testing::Test { void SetUp() override { filesystem_.CreateDirectoryRecursively(test_dir_.c_str()); - SchemaProto schema; - auto type_config = schema.add_types(); - type_config->set_schema_type(email_schema_); - + SchemaProto schema = + SchemaBuilder() + .AddType(SchemaTypeConfigBuilder().SetType(email_schema_)) + .Build(); ICING_ASSERT_OK_AND_ASSIGN( schema_store_, SchemaStore::Create(&filesystem_, test_dir_, &fake_clock_)); @@ -713,12 +712,11 @@ class DocHitInfoIteratorFilterTest : public ::testing::Test { .SetTtlMs(100) .Build(); - SchemaProto schema; - auto type_config = schema.add_types(); - type_config->set_schema_type(schema1_); - type_config = schema.add_types(); - type_config->set_schema_type(schema2_); - + SchemaProto schema = + SchemaBuilder() + .AddType(SchemaTypeConfigBuilder().SetType(schema1_)) + .AddType(SchemaTypeConfigBuilder().SetType(schema2_)) + .Build(); ICING_ASSERT_OK_AND_ASSIGN( schema_store_, SchemaStore::Create(&filesystem_, test_dir_, &fake_clock_)); diff --git a/icing/index/iterator/doc-hit-info-iterator-section-restrict_test.cc b/icing/index/iterator/doc-hit-info-iterator-section-restrict_test.cc index 21b3f8f..43a846b 100644 --- a/icing/index/iterator/doc-hit-info-iterator-section-restrict_test.cc +++ b/icing/index/iterator/doc-hit-info-iterator-section-restrict_test.cc @@ -29,6 +29,7 @@ #include "icing/proto/document.pb.h" #include "icing/proto/schema.pb.h" #include "icing/proto/term.pb.h" +#include "icing/schema-builder.h" #include "icing/schema/schema-store.h" #include "icing/schema/section.h" #include "icing/store/document-id.h" @@ -47,6 +48,14 @@ using ::testing::ElementsAreArray; using ::testing::Eq; using ::testing::IsEmpty; +constexpr PropertyConfigProto_Cardinality_Code CARDINALITY_OPTIONAL = + PropertyConfigProto_Cardinality_Code_OPTIONAL; + +constexpr StringIndexingConfig_TokenizerType_Code TOKENIZER_PLAIN = + StringIndexingConfig_TokenizerType_Code_PLAIN; + +constexpr TermMatchType_Code MATCH_EXACT = TermMatchType_Code_EXACT_ONLY; + class DocHitInfoIteratorSectionRestrictTest : public ::testing::Test { protected: DocHitInfoIteratorSectionRestrictTest() @@ -57,18 +66,18 @@ class DocHitInfoIteratorSectionRestrictTest : public ::testing::Test { document_ = DocumentBuilder().SetKey("namespace", "uri").SetSchema("email").Build(); - auto type_config = schema_.add_types(); - type_config->set_schema_type("email"); - - // Add an indexed property so we generate section metadata on it - auto property = type_config->add_properties(); - property->set_property_name(indexed_property_); - property->set_data_type(PropertyConfigProto::DataType::STRING); - property->set_cardinality(PropertyConfigProto::Cardinality::OPTIONAL); - property->mutable_string_indexing_config()->set_term_match_type( - TermMatchType::EXACT_ONLY); - property->mutable_string_indexing_config()->set_tokenizer_type( - StringIndexingConfig::TokenizerType::PLAIN); + schema_ = SchemaBuilder() + .AddType(SchemaTypeConfigBuilder() + .SetType("email") + // Add an indexed property so we generate section + // metadata on it + .AddProperty( + PropertyConfigBuilder() + .SetName(indexed_property_) + .SetDataTypeString(MATCH_EXACT, + TOKENIZER_PLAIN) + .SetCardinality(CARDINALITY_OPTIONAL))) + .Build(); // First and only indexed property, so it gets the first id of 0 indexed_section_id_ = 0; diff --git a/icing/jni/icing-search-engine-jni.cc b/icing/jni/icing-search-engine-jni.cc index a8fb0e2..ea2bcf7 100644 --- a/icing/jni/icing-search-engine-jni.cc +++ b/icing/jni/icing-search-engine-jni.cc @@ -357,12 +357,19 @@ Java_com_google_android_icing_IcingSearchEngine_nativeDeleteByQuery( JNIEXPORT jbyteArray JNICALL Java_com_google_android_icing_IcingSearchEngine_nativePersistToDisk( - JNIEnv* env, jclass clazz, jobject object) { + JNIEnv* env, jclass clazz, jobject object, jint persist_type_code) { icing::lib::IcingSearchEngine* icing = GetIcingSearchEnginePointer(env, object); + if (!icing::lib::PersistType::Code_IsValid(persist_type_code)) { + ICING_LOG(ERROR) << persist_type_code + << " is an invalid value for PersistType::Code"; + return nullptr; + } + icing::lib::PersistType::Code persist_type_code_enum = + static_cast<icing::lib::PersistType::Code>(persist_type_code); icing::lib::PersistToDiskResultProto persist_to_disk_result_proto = - icing->PersistToDisk(); + icing->PersistToDisk(persist_type_code_enum); return SerializeProtoToJniByteArray(env, persist_to_disk_result_proto); } diff --git a/icing/result/result-retriever.cc b/icing/result/result-retriever.cc index 85e78a8..943350c 100644 --- a/icing/result/result-retriever.cc +++ b/icing/result/result-retriever.cc @@ -107,6 +107,7 @@ ResultRetriever::RetrieveResults( // Add the document, itself. *result.mutable_document() = std::move(document); + result.set_score(scored_document_hit.score()); search_results.push_back(std::move(result)); } return search_results; diff --git a/icing/result/result-retriever_test.cc b/icing/result/result-retriever_test.cc index 46830ef..8d61dd9 100644 --- a/icing/result/result-retriever_test.cc +++ b/icing/result/result-retriever_test.cc @@ -228,9 +228,9 @@ TEST_F(ResultRetrieverTest, ShouldRetrieveSimpleResults) { GetSectionId("Email", "body")}; SectionIdMask hit_section_id_mask = CreateSectionIdMask(hit_section_ids); std::vector<ScoredDocumentHit> scored_document_hits = { - {document_id1, hit_section_id_mask, /*score=*/0}, - {document_id2, hit_section_id_mask, /*score=*/0}, - {document_id3, hit_section_id_mask, /*score=*/0}}; + {document_id1, hit_section_id_mask, /*score=*/19}, + {document_id2, hit_section_id_mask, /*score=*/5}, + {document_id3, hit_section_id_mask, /*score=*/1}}; ICING_ASSERT_OK_AND_ASSIGN( std::unique_ptr<ResultRetriever> result_retriever, ResultRetriever::Create(doc_store.get(), schema_store_.get(), @@ -238,10 +238,13 @@ TEST_F(ResultRetrieverTest, ShouldRetrieveSimpleResults) { SearchResultProto::ResultProto result1; *result1.mutable_document() = CreateDocument(/*id=*/1); + result1.set_score(19); SearchResultProto::ResultProto result2; *result2.mutable_document() = CreateDocument(/*id=*/2); + result2.set_score(5); SearchResultProto::ResultProto result3; *result3.mutable_document() = CreateDocument(/*id=*/3); + result3.set_score(1); SnippetContext snippet_context( /*query_terms_in=*/{}, @@ -277,8 +280,8 @@ TEST_F(ResultRetrieverTest, IgnoreErrors) { GetSectionId("Email", "body")}; SectionIdMask hit_section_id_mask = CreateSectionIdMask(hit_section_ids); std::vector<ScoredDocumentHit> scored_document_hits = { - {document_id1, hit_section_id_mask, /*score=*/0}, - {document_id2, hit_section_id_mask, /*score=*/0}, + {document_id1, hit_section_id_mask, /*score=*/12}, + {document_id2, hit_section_id_mask, /*score=*/4}, {invalid_document_id, hit_section_id_mask, /*score=*/0}}; ICING_ASSERT_OK_AND_ASSIGN( std::unique_ptr<ResultRetriever> result_retriever, @@ -288,8 +291,10 @@ TEST_F(ResultRetrieverTest, IgnoreErrors) { SearchResultProto::ResultProto result1; *result1.mutable_document() = CreateDocument(/*id=*/1); + result1.set_score(12); SearchResultProto::ResultProto result2; *result2.mutable_document() = CreateDocument(/*id=*/2); + result2.set_score(4); SnippetContext snippet_context( /*query_terms_in=*/{}, diff --git a/icing/result/result-state-manager.cc b/icing/result/result-state-manager.cc index 19dabb8..d606e79 100644 --- a/icing/result/result-state-manager.cc +++ b/icing/result/result-state-manager.cc @@ -22,8 +22,10 @@ namespace icing { namespace lib { -ResultStateManager::ResultStateManager(int max_total_hits) - : max_total_hits_(max_total_hits), +ResultStateManager::ResultStateManager(int max_total_hits, + const DocumentStore& document_store) + : document_store_(document_store), + max_total_hits_(max_total_hits), num_total_hits_(0), random_generator_(GetSteadyTimeNanoseconds()) {} @@ -39,7 +41,7 @@ ResultStateManager::RankAndPaginate(ResultState result_state) { int num_per_page = result_state.num_per_page(); std::vector<ScoredDocumentHit> page_result_document_hits = - result_state.GetNextPage(); + result_state.GetNextPage(document_store_); SnippetContext snippet_context_copy = result_state.snippet_context(); @@ -90,7 +92,7 @@ libtextclassifier3::StatusOr<PageResultState> ResultStateManager::GetNextPage( int num_returned = state_iterator->second.num_returned(); int num_per_page = state_iterator->second.num_per_page(); std::vector<ScoredDocumentHit> result_of_page = - state_iterator->second.GetNextPage(); + state_iterator->second.GetNextPage(document_store_); if (result_of_page.empty()) { // This shouldn't happen, all our active states should contain results, but // a sanity check here in case of any data inconsistency. diff --git a/icing/result/result-state-manager.h b/icing/result/result-state-manager.h index cf5d8c2..c04217f 100644 --- a/icing/result/result-state-manager.h +++ b/icing/result/result-state-manager.h @@ -37,7 +37,8 @@ inline constexpr uint64_t kInvalidNextPageToken = 0; // Used to store and manage ResultState. class ResultStateManager { public: - explicit ResultStateManager(int max_total_hits); + explicit ResultStateManager(int max_total_hits, + const DocumentStore& document_store); ResultStateManager(const ResultStateManager&) = delete; ResultStateManager& operator=(const ResultStateManager&) = delete; @@ -77,6 +78,8 @@ class ResultStateManager { private: absl_ports::shared_mutex mutex_; + const DocumentStore& document_store_; + // The maximum number of scored document hits that all result states may // have. When a new result state is added such that num_total_hits_ would // exceed max_total_hits_, the oldest result states are evicted until diff --git a/icing/result/result-state-manager_test.cc b/icing/result/result-state-manager_test.cc index afddeb5..32e45aa 100644 --- a/icing/result/result-state-manager_test.cc +++ b/icing/result/result-state-manager_test.cc @@ -14,9 +14,15 @@ #include "icing/result/result-state-manager.h" +#include "gmock/gmock.h" #include "gtest/gtest.h" +#include "icing/file/filesystem.h" #include "icing/portable/equals-proto.h" +#include "icing/schema/schema-store.h" +#include "icing/store/document-store.h" #include "icing/testing/common-matchers.h" +#include "icing/testing/tmp-directory.h" +#include "icing/util/clock.h" namespace icing { namespace lib { @@ -27,10 +33,6 @@ using ::testing::Eq; using ::testing::Gt; using ::testing::IsEmpty; -ScoredDocumentHit CreateScoredDocumentHit(DocumentId document_id) { - return ScoredDocumentHit(document_id, kSectionIdMaskNone, /*score=*/1); -} - ScoringSpecProto CreateScoringSpec() { ScoringSpecProto scoring_spec; scoring_spec.set_rank_by(ScoringSpecProto::RankingStrategy::DOCUMENT_SCORE); @@ -43,23 +45,73 @@ ResultSpecProto CreateResultSpec(int num_per_page) { return result_spec; } -ResultState CreateResultState( - const std::vector<ScoredDocumentHit>& scored_document_hits, - int num_per_page) { - return ResultState(scored_document_hits, /*query_terms=*/{}, - SearchSpecProto::default_instance(), CreateScoringSpec(), - CreateResultSpec(num_per_page)); +ScoredDocumentHit CreateScoredHit(DocumentId document_id) { + return ScoredDocumentHit(document_id, kSectionIdMaskNone, /*score=*/1); } -TEST(ResultStateManagerTest, ShouldRankAndPaginateOnePage) { +class ResultStateManagerTest : public testing::Test { + protected: + void SetUp() override { + schema_store_base_dir_ = GetTestTempDir() + "/schema_store"; + filesystem_.CreateDirectoryRecursively(schema_store_base_dir_.c_str()); + ICING_ASSERT_OK_AND_ASSIGN( + schema_store_, + SchemaStore::Create(&filesystem_, schema_store_base_dir_, &clock_)); + SchemaProto schema; + schema.add_types()->set_schema_type("Document"); + ICING_ASSERT_OK(schema_store_->SetSchema(std::move(schema))); + + doc_store_base_dir_ = GetTestTempDir() + "/document_store"; + filesystem_.CreateDirectoryRecursively(doc_store_base_dir_.c_str()); + ICING_ASSERT_OK_AND_ASSIGN( + DocumentStore::CreateResult result, + DocumentStore::Create(&filesystem_, doc_store_base_dir_, &clock_, + schema_store_.get())); + document_store_ = std::move(result.document_store); + } + + void TearDown() override { + filesystem_.DeleteDirectoryRecursively(doc_store_base_dir_.c_str()); + filesystem_.DeleteDirectoryRecursively(schema_store_base_dir_.c_str()); + } + + ResultState CreateResultState( + const std::vector<ScoredDocumentHit>& scored_document_hits, + int num_per_page) { + return ResultState(scored_document_hits, /*query_terms=*/{}, + SearchSpecProto::default_instance(), CreateScoringSpec(), + CreateResultSpec(num_per_page), *document_store_); + } + + ScoredDocumentHit AddScoredDocument(DocumentId document_id) { + DocumentProto document; + document.set_namespace_("namespace"); + document.set_uri(std::to_string(document_id)); + document.set_schema("Document"); + document_store_->Put(std::move(document)); + return ScoredDocumentHit(document_id, kSectionIdMaskNone, /*score=*/1); + } + + const DocumentStore& document_store() const { return *document_store_; } + + private: + Filesystem filesystem_; + std::string doc_store_base_dir_; + std::string schema_store_base_dir_; + Clock clock_; + std::unique_ptr<DocumentStore> document_store_; + std::unique_ptr<SchemaStore> schema_store_; +}; + +TEST_F(ResultStateManagerTest, ShouldRankAndPaginateOnePage) { ResultState original_result_state = - CreateResultState({CreateScoredDocumentHit(/*document_id=*/1), - CreateScoredDocumentHit(/*document_id=*/2), - CreateScoredDocumentHit(/*document_id=*/3)}, + CreateResultState({AddScoredDocument(/*document_id=*/0), + AddScoredDocument(/*document_id=*/1), + AddScoredDocument(/*document_id=*/2)}, /*num_per_page=*/10); ResultStateManager result_state_manager( - /*max_total_hits=*/std::numeric_limits<int>::max()); + /*max_total_hits=*/std::numeric_limits<int>::max(), document_store()); ICING_ASSERT_OK_AND_ASSIGN( PageResultState page_result_state, result_state_manager.RankAndPaginate(std::move(original_result_state))); @@ -69,23 +121,22 @@ TEST(ResultStateManagerTest, ShouldRankAndPaginateOnePage) { // Should get the original scored document hits EXPECT_THAT( page_result_state.scored_document_hits, - ElementsAre( - EqualsScoredDocumentHit(CreateScoredDocumentHit(/*document_id=*/3)), - EqualsScoredDocumentHit(CreateScoredDocumentHit(/*document_id=*/2)), - EqualsScoredDocumentHit(CreateScoredDocumentHit(/*document_id=*/1)))); + ElementsAre(EqualsScoredDocumentHit(CreateScoredHit(/*document_id=*/2)), + EqualsScoredDocumentHit(CreateScoredHit(/*document_id=*/1)), + EqualsScoredDocumentHit(CreateScoredHit(/*document_id=*/0)))); } -TEST(ResultStateManagerTest, ShouldRankAndPaginateMultiplePages) { +TEST_F(ResultStateManagerTest, ShouldRankAndPaginateMultiplePages) { ResultState original_result_state = - CreateResultState({CreateScoredDocumentHit(/*document_id=*/1), - CreateScoredDocumentHit(/*document_id=*/2), - CreateScoredDocumentHit(/*document_id=*/3), - CreateScoredDocumentHit(/*document_id=*/4), - CreateScoredDocumentHit(/*document_id=*/5)}, + CreateResultState({AddScoredDocument(/*document_id=*/0), + AddScoredDocument(/*document_id=*/1), + AddScoredDocument(/*document_id=*/2), + AddScoredDocument(/*document_id=*/3), + AddScoredDocument(/*document_id=*/4)}, /*num_per_page=*/2); ResultStateManager result_state_manager( - /*max_total_hits=*/std::numeric_limits<int>::max()); + /*max_total_hits=*/std::numeric_limits<int>::max(), document_store()); // First page, 2 results ICING_ASSERT_OK_AND_ASSIGN( @@ -93,9 +144,8 @@ TEST(ResultStateManagerTest, ShouldRankAndPaginateMultiplePages) { result_state_manager.RankAndPaginate(std::move(original_result_state))); EXPECT_THAT( page_result_state1.scored_document_hits, - ElementsAre( - EqualsScoredDocumentHit(CreateScoredDocumentHit(/*document_id=*/5)), - EqualsScoredDocumentHit(CreateScoredDocumentHit(/*document_id=*/4)))); + ElementsAre(EqualsScoredDocumentHit(CreateScoredHit(/*document_id=*/4)), + EqualsScoredDocumentHit(CreateScoredHit(/*document_id=*/3)))); uint64_t next_page_token = page_result_state1.next_page_token; @@ -104,46 +154,45 @@ TEST(ResultStateManagerTest, ShouldRankAndPaginateMultiplePages) { result_state_manager.GetNextPage(next_page_token)); EXPECT_THAT( page_result_state2.scored_document_hits, - ElementsAre( - EqualsScoredDocumentHit(CreateScoredDocumentHit(/*document_id=*/3)), - EqualsScoredDocumentHit(CreateScoredDocumentHit(/*document_id=*/2)))); + ElementsAre(EqualsScoredDocumentHit(CreateScoredHit(/*document_id=*/2)), + EqualsScoredDocumentHit(CreateScoredHit(/*document_id=*/1)))); // Third page, 1 result ICING_ASSERT_OK_AND_ASSIGN(PageResultState page_result_state3, result_state_manager.GetNextPage(next_page_token)); - EXPECT_THAT(page_result_state3.scored_document_hits, - ElementsAre(EqualsScoredDocumentHit( - CreateScoredDocumentHit(/*document_id=*/1)))); + EXPECT_THAT( + page_result_state3.scored_document_hits, + ElementsAre(EqualsScoredDocumentHit(CreateScoredHit(/*document_id=*/0)))); // No results EXPECT_THAT(result_state_manager.GetNextPage(next_page_token), StatusIs(libtextclassifier3::StatusCode::NOT_FOUND)); } -TEST(ResultStateManagerTest, EmptyStateShouldReturnError) { +TEST_F(ResultStateManagerTest, EmptyStateShouldReturnError) { ResultState empty_result_state = CreateResultState({}, /*num_per_page=*/1); ResultStateManager result_state_manager( - /*max_total_hits=*/std::numeric_limits<int>::max()); + /*max_total_hits=*/std::numeric_limits<int>::max(), document_store()); EXPECT_THAT( result_state_manager.RankAndPaginate(std::move(empty_result_state)), StatusIs(libtextclassifier3::StatusCode::INVALID_ARGUMENT)); } -TEST(ResultStateManagerTest, ShouldInvalidateOneToken) { +TEST_F(ResultStateManagerTest, ShouldInvalidateOneToken) { ResultState result_state1 = - CreateResultState({CreateScoredDocumentHit(/*document_id=*/1), - CreateScoredDocumentHit(/*document_id=*/2), - CreateScoredDocumentHit(/*document_id=*/3)}, + CreateResultState({AddScoredDocument(/*document_id=*/0), + AddScoredDocument(/*document_id=*/1), + AddScoredDocument(/*document_id=*/2)}, /*num_per_page=*/1); ResultState result_state2 = - CreateResultState({CreateScoredDocumentHit(/*document_id=*/4), - CreateScoredDocumentHit(/*document_id=*/5), - CreateScoredDocumentHit(/*document_id=*/6)}, + CreateResultState({AddScoredDocument(/*document_id=*/3), + AddScoredDocument(/*document_id=*/4), + AddScoredDocument(/*document_id=*/5)}, /*num_per_page=*/1); ResultStateManager result_state_manager( - /*max_total_hits=*/std::numeric_limits<int>::max()); + /*max_total_hits=*/std::numeric_limits<int>::max(), document_store()); ICING_ASSERT_OK_AND_ASSIGN( PageResultState page_result_state1, result_state_manager.RankAndPaginate(std::move(result_state1))); @@ -163,25 +212,25 @@ TEST(ResultStateManagerTest, ShouldInvalidateOneToken) { ICING_ASSERT_OK_AND_ASSIGN( page_result_state2, result_state_manager.GetNextPage(page_result_state2.next_page_token)); - EXPECT_THAT(page_result_state2.scored_document_hits, - ElementsAre(EqualsScoredDocumentHit( - CreateScoredDocumentHit(/*document_id=*/5)))); + EXPECT_THAT( + page_result_state2.scored_document_hits, + ElementsAre(EqualsScoredDocumentHit(CreateScoredHit(/*document_id=*/4)))); } -TEST(ResultStateManagerTest, ShouldInvalidateAllTokens) { +TEST_F(ResultStateManagerTest, ShouldInvalidateAllTokens) { ResultState result_state1 = - CreateResultState({CreateScoredDocumentHit(/*document_id=*/1), - CreateScoredDocumentHit(/*document_id=*/2), - CreateScoredDocumentHit(/*document_id=*/3)}, + CreateResultState({AddScoredDocument(/*document_id=*/0), + AddScoredDocument(/*document_id=*/1), + AddScoredDocument(/*document_id=*/2)}, /*num_per_page=*/1); ResultState result_state2 = - CreateResultState({CreateScoredDocumentHit(/*document_id=*/4), - CreateScoredDocumentHit(/*document_id=*/5), - CreateScoredDocumentHit(/*document_id=*/6)}, + CreateResultState({AddScoredDocument(/*document_id=*/3), + AddScoredDocument(/*document_id=*/4), + AddScoredDocument(/*document_id=*/5)}, /*num_per_page=*/1); ResultStateManager result_state_manager( - /*max_total_hits=*/std::numeric_limits<int>::max()); + /*max_total_hits=*/std::numeric_limits<int>::max(), document_store()); ICING_ASSERT_OK_AND_ASSIGN( PageResultState page_result_state1, result_state_manager.RankAndPaginate(std::move(result_state1))); @@ -202,21 +251,22 @@ TEST(ResultStateManagerTest, ShouldInvalidateAllTokens) { StatusIs(libtextclassifier3::StatusCode::NOT_FOUND)); } -TEST(ResultStateManagerTest, ShouldRemoveOldestResultState) { +TEST_F(ResultStateManagerTest, ShouldRemoveOldestResultState) { ResultState result_state1 = - CreateResultState({CreateScoredDocumentHit(/*document_id=*/1), - CreateScoredDocumentHit(/*document_id=*/2)}, + CreateResultState({AddScoredDocument(/*document_id=*/0), + AddScoredDocument(/*document_id=*/1)}, /*num_per_page=*/1); ResultState result_state2 = - CreateResultState({CreateScoredDocumentHit(/*document_id=*/3), - CreateScoredDocumentHit(/*document_id=*/4)}, + CreateResultState({AddScoredDocument(/*document_id=*/2), + AddScoredDocument(/*document_id=*/3)}, /*num_per_page=*/1); ResultState result_state3 = - CreateResultState({CreateScoredDocumentHit(/*document_id=*/5), - CreateScoredDocumentHit(/*document_id=*/6)}, + CreateResultState({AddScoredDocument(/*document_id=*/4), + AddScoredDocument(/*document_id=*/5)}, /*num_per_page=*/1); - ResultStateManager result_state_manager(/*max_total_hits=*/2); + ResultStateManager result_state_manager(/*max_total_hits=*/2, + document_store()); ICING_ASSERT_OK_AND_ASSIGN( PageResultState page_result_state1, result_state_manager.RankAndPaginate(std::move(result_state1))); @@ -236,37 +286,38 @@ TEST(ResultStateManagerTest, ShouldRemoveOldestResultState) { page_result_state2, result_state_manager.GetNextPage(page_result_state2.next_page_token)); EXPECT_THAT(page_result_state2.scored_document_hits, - ElementsAre(EqualsScoredDocumentHit(CreateScoredDocumentHit( - /*document_id=*/3)))); + ElementsAre(EqualsScoredDocumentHit(CreateScoredHit( + /*document_id=*/2)))); ICING_ASSERT_OK_AND_ASSIGN( page_result_state3, result_state_manager.GetNextPage(page_result_state3.next_page_token)); EXPECT_THAT(page_result_state3.scored_document_hits, - ElementsAre(EqualsScoredDocumentHit(CreateScoredDocumentHit( - /*document_id=*/5)))); + ElementsAre(EqualsScoredDocumentHit(CreateScoredHit( + /*document_id=*/4)))); } -TEST(ResultStateManagerTest, - InvalidatedResultStateShouldDecreaseCurrentHitsCount) { +TEST_F(ResultStateManagerTest, + InvalidatedResultStateShouldDecreaseCurrentHitsCount) { ResultState result_state1 = - CreateResultState({CreateScoredDocumentHit(/*document_id=*/1), - CreateScoredDocumentHit(/*document_id=*/2)}, + CreateResultState({AddScoredDocument(/*document_id=*/0), + AddScoredDocument(/*document_id=*/1)}, /*num_per_page=*/1); ResultState result_state2 = - CreateResultState({CreateScoredDocumentHit(/*document_id=*/3), - CreateScoredDocumentHit(/*document_id=*/4)}, + CreateResultState({AddScoredDocument(/*document_id=*/2), + AddScoredDocument(/*document_id=*/3)}, /*num_per_page=*/1); ResultState result_state3 = - CreateResultState({CreateScoredDocumentHit(/*document_id=*/5), - CreateScoredDocumentHit(/*document_id=*/6)}, + CreateResultState({AddScoredDocument(/*document_id=*/4), + AddScoredDocument(/*document_id=*/5)}, /*num_per_page=*/1); // Add the first three states. Remember, the first page for each result state // won't be cached (since it is returned immediately from RankAndPaginate). // Each result state has a page size of 1 and a result set of 2 hits. So each // result will take up one hit of our three hit budget. - ResultStateManager result_state_manager(/*max_total_hits=*/3); + ResultStateManager result_state_manager(/*max_total_hits=*/3, + document_store()); ICING_ASSERT_OK_AND_ASSIGN( PageResultState page_result_state1, result_state_manager.RankAndPaginate(std::move(result_state1))); @@ -286,8 +337,8 @@ TEST(ResultStateManagerTest, // then adding state 4 should still be within our budget and no other result // states should be evicted. ResultState result_state4 = - CreateResultState({CreateScoredDocumentHit(/*document_id=*/7), - CreateScoredDocumentHit(/*document_id=*/8)}, + CreateResultState({AddScoredDocument(/*document_id=*/6), + AddScoredDocument(/*document_id=*/7)}, /*num_per_page=*/1); ICING_ASSERT_OK_AND_ASSIGN( PageResultState page_result_state4, @@ -297,8 +348,8 @@ TEST(ResultStateManagerTest, page_result_state1, result_state_manager.GetNextPage(page_result_state1.next_page_token)); EXPECT_THAT(page_result_state1.scored_document_hits, - ElementsAre(EqualsScoredDocumentHit(CreateScoredDocumentHit( - /*document_id=*/1)))); + ElementsAre(EqualsScoredDocumentHit(CreateScoredHit( + /*document_id=*/0)))); EXPECT_THAT( result_state_manager.GetNextPage(page_result_state2.next_page_token), @@ -308,37 +359,38 @@ TEST(ResultStateManagerTest, page_result_state3, result_state_manager.GetNextPage(page_result_state3.next_page_token)); EXPECT_THAT(page_result_state3.scored_document_hits, - ElementsAre(EqualsScoredDocumentHit(CreateScoredDocumentHit( - /*document_id=*/5)))); + ElementsAre(EqualsScoredDocumentHit(CreateScoredHit( + /*document_id=*/4)))); ICING_ASSERT_OK_AND_ASSIGN( page_result_state4, result_state_manager.GetNextPage(page_result_state4.next_page_token)); EXPECT_THAT(page_result_state4.scored_document_hits, - ElementsAre(EqualsScoredDocumentHit(CreateScoredDocumentHit( - /*document_id=*/7)))); + ElementsAre(EqualsScoredDocumentHit(CreateScoredHit( + /*document_id=*/6)))); } -TEST(ResultStateManagerTest, - InvalidatedAllResultStatesShouldResetCurrentHitCount) { +TEST_F(ResultStateManagerTest, + InvalidatedAllResultStatesShouldResetCurrentHitCount) { ResultState result_state1 = - CreateResultState({CreateScoredDocumentHit(/*document_id=*/1), - CreateScoredDocumentHit(/*document_id=*/2)}, + CreateResultState({AddScoredDocument(/*document_id=*/0), + AddScoredDocument(/*document_id=*/1)}, /*num_per_page=*/1); ResultState result_state2 = - CreateResultState({CreateScoredDocumentHit(/*document_id=*/3), - CreateScoredDocumentHit(/*document_id=*/4)}, + CreateResultState({AddScoredDocument(/*document_id=*/2), + AddScoredDocument(/*document_id=*/3)}, /*num_per_page=*/1); ResultState result_state3 = - CreateResultState({CreateScoredDocumentHit(/*document_id=*/5), - CreateScoredDocumentHit(/*document_id=*/6)}, + CreateResultState({AddScoredDocument(/*document_id=*/4), + AddScoredDocument(/*document_id=*/5)}, /*num_per_page=*/1); // Add the first three states. Remember, the first page for each result state // won't be cached (since it is returned immediately from RankAndPaginate). // Each result state has a page size of 1 and a result set of 2 hits. So each // result will take up one hit of our three hit budget. - ResultStateManager result_state_manager(/*max_total_hits=*/3); + ResultStateManager result_state_manager(/*max_total_hits=*/3, + document_store()); ICING_ASSERT_OK_AND_ASSIGN( PageResultState page_result_state1, result_state_manager.RankAndPaginate(std::move(result_state1))); @@ -356,16 +408,16 @@ TEST(ResultStateManagerTest, // then the entirety of state 4 should still be within our budget and no other // result states should be evicted. ResultState result_state4 = - CreateResultState({CreateScoredDocumentHit(/*document_id=*/7), - CreateScoredDocumentHit(/*document_id=*/8)}, + CreateResultState({AddScoredDocument(/*document_id=*/6), + AddScoredDocument(/*document_id=*/7)}, /*num_per_page=*/1); ResultState result_state5 = - CreateResultState({CreateScoredDocumentHit(/*document_id=*/9), - CreateScoredDocumentHit(/*document_id=*/10)}, + CreateResultState({AddScoredDocument(/*document_id=*/8), + AddScoredDocument(/*document_id=*/9)}, /*num_per_page=*/1); ResultState result_state6 = - CreateResultState({CreateScoredDocumentHit(/*document_id=*/11), - CreateScoredDocumentHit(/*document_id=*/12)}, + CreateResultState({AddScoredDocument(/*document_id=*/10), + AddScoredDocument(/*document_id=*/11)}, /*num_per_page=*/1); ICING_ASSERT_OK_AND_ASSIGN( PageResultState page_result_state4, @@ -393,44 +445,46 @@ TEST(ResultStateManagerTest, page_result_state4, result_state_manager.GetNextPage(page_result_state4.next_page_token)); EXPECT_THAT(page_result_state4.scored_document_hits, - ElementsAre(EqualsScoredDocumentHit(CreateScoredDocumentHit( - /*document_id=*/7)))); + ElementsAre(EqualsScoredDocumentHit(CreateScoredHit( + /*document_id=*/6)))); ICING_ASSERT_OK_AND_ASSIGN( page_result_state5, result_state_manager.GetNextPage(page_result_state5.next_page_token)); EXPECT_THAT(page_result_state5.scored_document_hits, - ElementsAre(EqualsScoredDocumentHit(CreateScoredDocumentHit( - /*document_id=*/9)))); + ElementsAre(EqualsScoredDocumentHit(CreateScoredHit( + /*document_id=*/8)))); ICING_ASSERT_OK_AND_ASSIGN( page_result_state6, result_state_manager.GetNextPage(page_result_state6.next_page_token)); EXPECT_THAT(page_result_state6.scored_document_hits, - ElementsAre(EqualsScoredDocumentHit(CreateScoredDocumentHit( - /*document_id=*/11)))); + ElementsAre(EqualsScoredDocumentHit(CreateScoredHit( + /*document_id=*/10)))); } -TEST(ResultStateManagerTest, - InvalidatedResultStateShouldDecreaseCurrentHitsCountByExactStateHitCount) { +TEST_F( + ResultStateManagerTest, + InvalidatedResultStateShouldDecreaseCurrentHitsCountByExactStateHitCount) { ResultState result_state1 = - CreateResultState({CreateScoredDocumentHit(/*document_id=*/1), - CreateScoredDocumentHit(/*document_id=*/2)}, + CreateResultState({AddScoredDocument(/*document_id=*/0), + AddScoredDocument(/*document_id=*/1)}, /*num_per_page=*/1); ResultState result_state2 = - CreateResultState({CreateScoredDocumentHit(/*document_id=*/3), - CreateScoredDocumentHit(/*document_id=*/4)}, + CreateResultState({AddScoredDocument(/*document_id=*/2), + AddScoredDocument(/*document_id=*/3)}, /*num_per_page=*/1); ResultState result_state3 = - CreateResultState({CreateScoredDocumentHit(/*document_id=*/5), - CreateScoredDocumentHit(/*document_id=*/6)}, + CreateResultState({AddScoredDocument(/*document_id=*/4), + AddScoredDocument(/*document_id=*/5)}, /*num_per_page=*/1); // Add the first three states. Remember, the first page for each result state // won't be cached (since it is returned immediately from RankAndPaginate). // Each result state has a page size of 1 and a result set of 2 hits. So each // result will take up one hit of our three hit budget. - ResultStateManager result_state_manager(/*max_total_hits=*/3); + ResultStateManager result_state_manager(/*max_total_hits=*/3, + document_store()); ICING_ASSERT_OK_AND_ASSIGN( PageResultState page_result_state1, result_state_manager.RankAndPaginate(std::move(result_state1))); @@ -450,8 +504,8 @@ TEST(ResultStateManagerTest, // then adding state 4 should still be within our budget and no other result // states should be evicted. ResultState result_state4 = - CreateResultState({CreateScoredDocumentHit(/*document_id=*/7), - CreateScoredDocumentHit(/*document_id=*/8)}, + CreateResultState({AddScoredDocument(/*document_id=*/6), + AddScoredDocument(/*document_id=*/7)}, /*num_per_page=*/1); ICING_ASSERT_OK_AND_ASSIGN( PageResultState page_result_state4, @@ -461,8 +515,8 @@ TEST(ResultStateManagerTest, // to 2 and adding state 4 correctly incremented it to 3, then adding this // result state should trigger the eviction of state 1. ResultState result_state5 = - CreateResultState({CreateScoredDocumentHit(/*document_id=*/9), - CreateScoredDocumentHit(/*document_id=*/10)}, + CreateResultState({AddScoredDocument(/*document_id=*/8), + AddScoredDocument(/*document_id=*/9)}, /*num_per_page=*/1); ICING_ASSERT_OK_AND_ASSIGN( PageResultState page_result_state5, @@ -480,43 +534,44 @@ TEST(ResultStateManagerTest, page_result_state3, result_state_manager.GetNextPage(page_result_state3.next_page_token)); EXPECT_THAT(page_result_state3.scored_document_hits, - ElementsAre(EqualsScoredDocumentHit(CreateScoredDocumentHit( - /*document_id=*/5)))); + ElementsAre(EqualsScoredDocumentHit(CreateScoredHit( + /*document_id=*/4)))); ICING_ASSERT_OK_AND_ASSIGN( page_result_state4, result_state_manager.GetNextPage(page_result_state4.next_page_token)); EXPECT_THAT(page_result_state4.scored_document_hits, - ElementsAre(EqualsScoredDocumentHit(CreateScoredDocumentHit( - /*document_id=*/7)))); + ElementsAre(EqualsScoredDocumentHit(CreateScoredHit( + /*document_id=*/6)))); ICING_ASSERT_OK_AND_ASSIGN( page_result_state5, result_state_manager.GetNextPage(page_result_state5.next_page_token)); EXPECT_THAT(page_result_state5.scored_document_hits, - ElementsAre(EqualsScoredDocumentHit(CreateScoredDocumentHit( - /*document_id=*/9)))); + ElementsAre(EqualsScoredDocumentHit(CreateScoredHit( + /*document_id=*/8)))); } -TEST(ResultStateManagerTest, GetNextPageShouldDecreaseCurrentHitsCount) { +TEST_F(ResultStateManagerTest, GetNextPageShouldDecreaseCurrentHitsCount) { ResultState result_state1 = - CreateResultState({CreateScoredDocumentHit(/*document_id=*/1), - CreateScoredDocumentHit(/*document_id=*/2)}, + CreateResultState({AddScoredDocument(/*document_id=*/0), + AddScoredDocument(/*document_id=*/1)}, /*num_per_page=*/1); ResultState result_state2 = - CreateResultState({CreateScoredDocumentHit(/*document_id=*/3), - CreateScoredDocumentHit(/*document_id=*/4)}, + CreateResultState({AddScoredDocument(/*document_id=*/2), + AddScoredDocument(/*document_id=*/3)}, /*num_per_page=*/1); ResultState result_state3 = - CreateResultState({CreateScoredDocumentHit(/*document_id=*/5), - CreateScoredDocumentHit(/*document_id=*/6)}, + CreateResultState({AddScoredDocument(/*document_id=*/4), + AddScoredDocument(/*document_id=*/5)}, /*num_per_page=*/1); // Add the first three states. Remember, the first page for each result state // won't be cached (since it is returned immediately from RankAndPaginate). // Each result state has a page size of 1 and a result set of 2 hits. So each // result will take up one hit of our three hit budget. - ResultStateManager result_state_manager(/*max_total_hits=*/3); + ResultStateManager result_state_manager(/*max_total_hits=*/3, + document_store()); ICING_ASSERT_OK_AND_ASSIGN( PageResultState page_result_state1, result_state_manager.RankAndPaginate(std::move(result_state1))); @@ -533,15 +588,15 @@ TEST(ResultStateManagerTest, GetNextPageShouldDecreaseCurrentHitsCount) { page_result_state1, result_state_manager.GetNextPage(page_result_state1.next_page_token)); EXPECT_THAT(page_result_state1.scored_document_hits, - ElementsAre(EqualsScoredDocumentHit(CreateScoredDocumentHit( - /*document_id=*/1)))); + ElementsAre(EqualsScoredDocumentHit(CreateScoredHit( + /*document_id=*/0)))); // If retrieving the next page for result state 1 correctly decremented the // current hit count to 2, then adding state 4 should still be within our // budget and no other result states should be evicted. ResultState result_state4 = - CreateResultState({CreateScoredDocumentHit(/*document_id=*/7), - CreateScoredDocumentHit(/*document_id=*/8)}, + CreateResultState({AddScoredDocument(/*document_id=*/6), + AddScoredDocument(/*document_id=*/7)}, /*num_per_page=*/1); ICING_ASSERT_OK_AND_ASSIGN( PageResultState page_result_state4, @@ -555,44 +610,45 @@ TEST(ResultStateManagerTest, GetNextPageShouldDecreaseCurrentHitsCount) { page_result_state2, result_state_manager.GetNextPage(page_result_state2.next_page_token)); EXPECT_THAT(page_result_state2.scored_document_hits, - ElementsAre(EqualsScoredDocumentHit(CreateScoredDocumentHit( - /*document_id=*/3)))); + ElementsAre(EqualsScoredDocumentHit(CreateScoredHit( + /*document_id=*/2)))); ICING_ASSERT_OK_AND_ASSIGN( page_result_state3, result_state_manager.GetNextPage(page_result_state3.next_page_token)); EXPECT_THAT(page_result_state3.scored_document_hits, - ElementsAre(EqualsScoredDocumentHit(CreateScoredDocumentHit( - /*document_id=*/5)))); + ElementsAre(EqualsScoredDocumentHit(CreateScoredHit( + /*document_id=*/4)))); ICING_ASSERT_OK_AND_ASSIGN( page_result_state4, result_state_manager.GetNextPage(page_result_state4.next_page_token)); EXPECT_THAT(page_result_state4.scored_document_hits, - ElementsAre(EqualsScoredDocumentHit(CreateScoredDocumentHit( - /*document_id=*/7)))); + ElementsAre(EqualsScoredDocumentHit(CreateScoredHit( + /*document_id=*/6)))); } -TEST(ResultStateManagerTest, - GetNextPageShouldDecreaseCurrentHitsCountByExactlyOnePage) { +TEST_F(ResultStateManagerTest, + GetNextPageShouldDecreaseCurrentHitsCountByExactlyOnePage) { ResultState result_state1 = - CreateResultState({CreateScoredDocumentHit(/*document_id=*/1), - CreateScoredDocumentHit(/*document_id=*/2)}, + CreateResultState({AddScoredDocument(/*document_id=*/0), + AddScoredDocument(/*document_id=*/1)}, /*num_per_page=*/1); ResultState result_state2 = - CreateResultState({CreateScoredDocumentHit(/*document_id=*/3), - CreateScoredDocumentHit(/*document_id=*/4)}, + CreateResultState({AddScoredDocument(/*document_id=*/2), + AddScoredDocument(/*document_id=*/3)}, /*num_per_page=*/1); ResultState result_state3 = - CreateResultState({CreateScoredDocumentHit(/*document_id=*/5), - CreateScoredDocumentHit(/*document_id=*/6)}, + CreateResultState({AddScoredDocument(/*document_id=*/4), + AddScoredDocument(/*document_id=*/5)}, /*num_per_page=*/1); // Add the first three states. Remember, the first page for each result state // won't be cached (since it is returned immediately from RankAndPaginate). // Each result state has a page size of 1 and a result set of 2 hits. So each // result will take up one hit of our three hit budget. - ResultStateManager result_state_manager(/*max_total_hits=*/3); + ResultStateManager result_state_manager(/*max_total_hits=*/3, + document_store()); ICING_ASSERT_OK_AND_ASSIGN( PageResultState page_result_state1, result_state_manager.RankAndPaginate(std::move(result_state1))); @@ -609,15 +665,15 @@ TEST(ResultStateManagerTest, page_result_state1, result_state_manager.GetNextPage(page_result_state1.next_page_token)); EXPECT_THAT(page_result_state1.scored_document_hits, - ElementsAre(EqualsScoredDocumentHit(CreateScoredDocumentHit( - /*document_id=*/1)))); + ElementsAre(EqualsScoredDocumentHit(CreateScoredHit( + /*document_id=*/0)))); // If retrieving the next page for result state 1 correctly decremented the // current hit count to 2, then adding state 4 should still be within our // budget and no other result states should be evicted. ResultState result_state4 = - CreateResultState({CreateScoredDocumentHit(/*document_id=*/7), - CreateScoredDocumentHit(/*document_id=*/8)}, + CreateResultState({AddScoredDocument(/*document_id=*/6), + AddScoredDocument(/*document_id=*/7)}, /*num_per_page=*/1); ICING_ASSERT_OK_AND_ASSIGN( PageResultState page_result_state4, @@ -627,8 +683,8 @@ TEST(ResultStateManagerTest, // current hit count to 2 and adding state 4 correctly incremented it to 3, // then adding this result state should trigger the eviction of state 2. ResultState result_state5 = - CreateResultState({CreateScoredDocumentHit(/*document_id=*/9), - CreateScoredDocumentHit(/*document_id=*/10)}, + CreateResultState({AddScoredDocument(/*document_id=*/8), + AddScoredDocument(/*document_id=*/9)}, /*num_per_page=*/1); ICING_ASSERT_OK_AND_ASSIGN( PageResultState page_result_state5, @@ -646,39 +702,41 @@ TEST(ResultStateManagerTest, page_result_state3, result_state_manager.GetNextPage(page_result_state3.next_page_token)); EXPECT_THAT(page_result_state3.scored_document_hits, - ElementsAre(EqualsScoredDocumentHit(CreateScoredDocumentHit( - /*document_id=*/5)))); + ElementsAre(EqualsScoredDocumentHit(CreateScoredHit( + /*document_id=*/4)))); ICING_ASSERT_OK_AND_ASSIGN( page_result_state4, result_state_manager.GetNextPage(page_result_state4.next_page_token)); EXPECT_THAT(page_result_state4.scored_document_hits, - ElementsAre(EqualsScoredDocumentHit(CreateScoredDocumentHit( - /*document_id=*/7)))); + ElementsAre(EqualsScoredDocumentHit(CreateScoredHit( + /*document_id=*/6)))); ICING_ASSERT_OK_AND_ASSIGN( page_result_state5, result_state_manager.GetNextPage(page_result_state5.next_page_token)); EXPECT_THAT(page_result_state5.scored_document_hits, - ElementsAre(EqualsScoredDocumentHit(CreateScoredDocumentHit( - /*document_id=*/9)))); + ElementsAre(EqualsScoredDocumentHit(CreateScoredHit( + /*document_id=*/8)))); } -TEST(ResultStateManagerTest, AddingOverBudgetResultStateShouldEvictAllStates) { +TEST_F(ResultStateManagerTest, + AddingOverBudgetResultStateShouldEvictAllStates) { ResultState result_state1 = - CreateResultState({CreateScoredDocumentHit(/*document_id=*/1), - CreateScoredDocumentHit(/*document_id=*/2), - CreateScoredDocumentHit(/*document_id=*/3)}, + CreateResultState({AddScoredDocument(/*document_id=*/0), + AddScoredDocument(/*document_id=*/1), + AddScoredDocument(/*document_id=*/2)}, /*num_per_page=*/1); ResultState result_state2 = - CreateResultState({CreateScoredDocumentHit(/*document_id=*/4), - CreateScoredDocumentHit(/*document_id=*/5)}, + CreateResultState({AddScoredDocument(/*document_id=*/3), + AddScoredDocument(/*document_id=*/4)}, /*num_per_page=*/1); // Add the first two states. Remember, the first page for each result state // won't be cached (since it is returned immediately from RankAndPaginate). // Each result state has a page size of 1. So 3 hits will remain cached. - ResultStateManager result_state_manager(/*max_total_hits=*/4); + ResultStateManager result_state_manager(/*max_total_hits=*/4, + document_store()); ICING_ASSERT_OK_AND_ASSIGN( PageResultState page_result_state1, result_state_manager.RankAndPaginate(std::move(result_state1))); @@ -691,12 +749,12 @@ TEST(ResultStateManagerTest, AddingOverBudgetResultStateShouldEvictAllStates) { // result state 3 being returned and the next four hits being cached (the last // hit should be dropped because it exceeds the max). ResultState result_state3 = - CreateResultState({CreateScoredDocumentHit(/*document_id=*/6), - CreateScoredDocumentHit(/*document_id=*/7), - CreateScoredDocumentHit(/*document_id=*/8), - CreateScoredDocumentHit(/*document_id=*/9), - CreateScoredDocumentHit(/*document_id=*/10), - CreateScoredDocumentHit(/*document_id=*/11)}, + CreateResultState({AddScoredDocument(/*document_id=*/5), + AddScoredDocument(/*document_id=*/6), + AddScoredDocument(/*document_id=*/7), + AddScoredDocument(/*document_id=*/8), + AddScoredDocument(/*document_id=*/9), + AddScoredDocument(/*document_id=*/10)}, /*num_per_page=*/1); ICING_ASSERT_OK_AND_ASSIGN( PageResultState page_result_state3, @@ -716,29 +774,29 @@ TEST(ResultStateManagerTest, AddingOverBudgetResultStateShouldEvictAllStates) { page_result_state3, result_state_manager.GetNextPage(page_result_state3.next_page_token)); EXPECT_THAT(page_result_state3.scored_document_hits, - ElementsAre(EqualsScoredDocumentHit(CreateScoredDocumentHit( - /*document_id=*/10)))); + ElementsAre(EqualsScoredDocumentHit(CreateScoredHit( + /*document_id=*/9)))); ICING_ASSERT_OK_AND_ASSIGN( page_result_state3, result_state_manager.GetNextPage(page_result_state3.next_page_token)); EXPECT_THAT(page_result_state3.scored_document_hits, - ElementsAre(EqualsScoredDocumentHit(CreateScoredDocumentHit( - /*document_id=*/9)))); + ElementsAre(EqualsScoredDocumentHit(CreateScoredHit( + /*document_id=*/8)))); ICING_ASSERT_OK_AND_ASSIGN( page_result_state3, result_state_manager.GetNextPage(page_result_state3.next_page_token)); EXPECT_THAT(page_result_state3.scored_document_hits, - ElementsAre(EqualsScoredDocumentHit(CreateScoredDocumentHit( - /*document_id=*/8)))); + ElementsAre(EqualsScoredDocumentHit(CreateScoredHit( + /*document_id=*/7)))); ICING_ASSERT_OK_AND_ASSIGN( page_result_state3, result_state_manager.GetNextPage(page_result_state3.next_page_token)); EXPECT_THAT(page_result_state3.scored_document_hits, - ElementsAre(EqualsScoredDocumentHit(CreateScoredDocumentHit( - /*document_id=*/7)))); + ElementsAre(EqualsScoredDocumentHit(CreateScoredHit( + /*document_id=*/6)))); // The final result should have been dropped because it exceeded the budget. EXPECT_THAT( @@ -746,18 +804,19 @@ TEST(ResultStateManagerTest, AddingOverBudgetResultStateShouldEvictAllStates) { StatusIs(libtextclassifier3::StatusCode::NOT_FOUND)); } -TEST(ResultStateManagerTest, - AddingResultStateShouldEvictOverBudgetResultState) { - ResultStateManager result_state_manager(/*max_total_hits=*/4); +TEST_F(ResultStateManagerTest, + AddingResultStateShouldEvictOverBudgetResultState) { + ResultStateManager result_state_manager(/*max_total_hits=*/4, + document_store()); // Add a result state that is larger than the entire budget. The entire result // state will still be cached ResultState result_state1 = - CreateResultState({CreateScoredDocumentHit(/*document_id=*/0), - CreateScoredDocumentHit(/*document_id=*/1), - CreateScoredDocumentHit(/*document_id=*/2), - CreateScoredDocumentHit(/*document_id=*/3), - CreateScoredDocumentHit(/*document_id=*/4), - CreateScoredDocumentHit(/*document_id=*/5)}, + CreateResultState({AddScoredDocument(/*document_id=*/0), + AddScoredDocument(/*document_id=*/1), + AddScoredDocument(/*document_id=*/2), + AddScoredDocument(/*document_id=*/3), + AddScoredDocument(/*document_id=*/4), + AddScoredDocument(/*document_id=*/5)}, /*num_per_page=*/1); ICING_ASSERT_OK_AND_ASSIGN( PageResultState page_result_state1, @@ -766,8 +825,8 @@ TEST(ResultStateManagerTest, // Add a result state. Because state2 + state1 is larger than the budget, // state1 should be evicted. ResultState result_state2 = - CreateResultState({CreateScoredDocumentHit(/*document_id=*/6), - CreateScoredDocumentHit(/*document_id=*/7)}, + CreateResultState({AddScoredDocument(/*document_id=*/6), + AddScoredDocument(/*document_id=*/7)}, /*num_per_page=*/1); ICING_ASSERT_OK_AND_ASSIGN( PageResultState page_result_state2, @@ -782,11 +841,11 @@ TEST(ResultStateManagerTest, page_result_state2, result_state_manager.GetNextPage(page_result_state2.next_page_token)); EXPECT_THAT(page_result_state2.scored_document_hits, - ElementsAre(EqualsScoredDocumentHit(CreateScoredDocumentHit( + ElementsAre(EqualsScoredDocumentHit(CreateScoredHit( /*document_id=*/6)))); } -TEST(ResultStateManagerTest, ShouldGetSnippetContext) { +TEST_F(ResultStateManagerTest, ShouldGetSnippetContext) { ResultSpecProto result_spec = CreateResultSpec(/*num_per_page=*/1); result_spec.mutable_snippet_spec()->set_num_to_snippet(5); result_spec.mutable_snippet_spec()->set_num_matches_per_property(5); @@ -799,12 +858,13 @@ TEST(ResultStateManagerTest, ShouldGetSnippetContext) { query_terms_map.emplace("term1", std::unordered_set<std::string>()); ResultState original_result_state = ResultState( - /*scored_document_hits=*/{CreateScoredDocumentHit(/*document_id=*/1), - CreateScoredDocumentHit(/*document_id=*/2)}, - query_terms_map, search_spec, CreateScoringSpec(), result_spec); + /*scored_document_hits=*/{AddScoredDocument(/*document_id=*/0), + AddScoredDocument(/*document_id=*/1)}, + query_terms_map, search_spec, CreateScoringSpec(), result_spec, + document_store()); ResultStateManager result_state_manager( - /*max_total_hits=*/std::numeric_limits<int>::max()); + /*max_total_hits=*/std::numeric_limits<int>::max(), document_store()); ICING_ASSERT_OK_AND_ASSIGN( PageResultState page_result_state, result_state_manager.RankAndPaginate(std::move(original_result_state))); @@ -819,7 +879,7 @@ TEST(ResultStateManagerTest, ShouldGetSnippetContext) { EqualsProto(result_spec.snippet_spec())); } -TEST(ResultStateManagerTest, ShouldGetDefaultSnippetContext) { +TEST_F(ResultStateManagerTest, ShouldGetDefaultSnippetContext) { ResultSpecProto result_spec = CreateResultSpec(/*num_per_page=*/1); // 0 indicates no snippeting result_spec.mutable_snippet_spec()->set_num_to_snippet(0); @@ -833,12 +893,13 @@ TEST(ResultStateManagerTest, ShouldGetDefaultSnippetContext) { query_terms_map.emplace("term1", std::unordered_set<std::string>()); ResultState original_result_state = ResultState( - /*scored_document_hits=*/{CreateScoredDocumentHit(/*document_id=*/1), - CreateScoredDocumentHit(/*document_id=*/2)}, - query_terms_map, search_spec, CreateScoringSpec(), result_spec); + /*scored_document_hits=*/{AddScoredDocument(/*document_id=*/0), + AddScoredDocument(/*document_id=*/1)}, + query_terms_map, search_spec, CreateScoringSpec(), result_spec, + document_store()); ResultStateManager result_state_manager( - /*max_total_hits=*/std::numeric_limits<int>::max()); + /*max_total_hits=*/std::numeric_limits<int>::max(), document_store()); ICING_ASSERT_OK_AND_ASSIGN( PageResultState page_result_state, result_state_manager.RankAndPaginate(std::move(original_result_state))); @@ -853,17 +914,17 @@ TEST(ResultStateManagerTest, ShouldGetDefaultSnippetContext) { Eq(TermMatchType::UNKNOWN)); } -TEST(ResultStateManagerTest, ShouldGetCorrectNumPreviouslyReturned) { +TEST_F(ResultStateManagerTest, ShouldGetCorrectNumPreviouslyReturned) { ResultState original_result_state = - CreateResultState({CreateScoredDocumentHit(/*document_id=*/1), - CreateScoredDocumentHit(/*document_id=*/2), - CreateScoredDocumentHit(/*document_id=*/3), - CreateScoredDocumentHit(/*document_id=*/4), - CreateScoredDocumentHit(/*document_id=*/5)}, + CreateResultState({AddScoredDocument(/*document_id=*/0), + AddScoredDocument(/*document_id=*/1), + AddScoredDocument(/*document_id=*/2), + AddScoredDocument(/*document_id=*/3), + AddScoredDocument(/*document_id=*/4)}, /*num_per_page=*/2); ResultStateManager result_state_manager( - /*max_total_hits=*/std::numeric_limits<int>::max()); + /*max_total_hits=*/std::numeric_limits<int>::max(), document_store()); // First page, 2 results ICING_ASSERT_OK_AND_ASSIGN( @@ -897,18 +958,19 @@ TEST(ResultStateManagerTest, ShouldGetCorrectNumPreviouslyReturned) { StatusIs(libtextclassifier3::StatusCode::NOT_FOUND)); } -TEST(ResultStateManagerTest, ShouldStoreAllHits) { - ScoredDocumentHit scored_hit_1 = CreateScoredDocumentHit(/*document_id=*/1); - ScoredDocumentHit scored_hit_2 = CreateScoredDocumentHit(/*document_id=*/2); - ScoredDocumentHit scored_hit_3 = CreateScoredDocumentHit(/*document_id=*/3); - ScoredDocumentHit scored_hit_4 = CreateScoredDocumentHit(/*document_id=*/4); - ScoredDocumentHit scored_hit_5 = CreateScoredDocumentHit(/*document_id=*/5); +TEST_F(ResultStateManagerTest, ShouldStoreAllHits) { + ScoredDocumentHit scored_hit_1 = AddScoredDocument(/*document_id=*/0); + ScoredDocumentHit scored_hit_2 = AddScoredDocument(/*document_id=*/1); + ScoredDocumentHit scored_hit_3 = AddScoredDocument(/*document_id=*/2); + ScoredDocumentHit scored_hit_4 = AddScoredDocument(/*document_id=*/3); + ScoredDocumentHit scored_hit_5 = AddScoredDocument(/*document_id=*/4); ResultState original_result_state = CreateResultState( {scored_hit_1, scored_hit_2, scored_hit_3, scored_hit_4, scored_hit_5}, /*num_per_page=*/2); - ResultStateManager result_state_manager(/*max_total_hits=*/4); + ResultStateManager result_state_manager(/*max_total_hits=*/4, + document_store()); // The 5 input scored document hits will not be truncated. The first page of // two hits will be returned immediately and the other three hits will fit diff --git a/icing/result/result-state.cc b/icing/result/result-state.cc index 82738a9..fc89185 100644 --- a/icing/result/result-state.cc +++ b/icing/result/result-state.cc @@ -16,6 +16,7 @@ #include "icing/result/projection-tree.h" #include "icing/scoring/ranker.h" +#include "icing/store/namespace-id.h" #include "icing/util/logging.h" namespace icing { @@ -39,7 +40,8 @@ ResultState::ResultState(std::vector<ScoredDocumentHit> scored_document_hits, SectionRestrictQueryTermsMap query_terms, const SearchSpecProto& search_spec, const ScoringSpecProto& scoring_spec, - const ResultSpecProto& result_spec) + const ResultSpecProto& result_spec, + const DocumentStore& document_store) : scored_document_hits_(std::move(scored_document_hits)), snippet_context_(CreateSnippetContext(std::move(query_terms), search_spec, result_spec)), @@ -52,14 +54,82 @@ ResultState::ResultState(std::vector<ScoredDocumentHit> scored_document_hits, projection_tree_map_.insert( {type_field_mask.schema_type(), ProjectionTree(type_field_mask)}); } + + for (const ResultSpecProto::ResultGrouping& result_grouping : + result_spec.result_groupings()) { + int group_id = group_result_limits_.size(); + group_result_limits_.push_back(result_grouping.max_results()); + for (const std::string& name_space : result_grouping.namespaces()) { + auto namespace_id_or = document_store.GetNamespaceId(name_space); + if (!namespace_id_or.ok()) { + continue; + } + namespace_group_id_map_.insert({namespace_id_or.ValueOrDie(), group_id}); + } + } BuildHeapInPlace(&scored_document_hits_, scored_document_hit_comparator_); } -std::vector<ScoredDocumentHit> ResultState::GetNextPage() { - std::vector<ScoredDocumentHit> scored_document_hits = PopTopResultsFromHeap( - &scored_document_hits_, num_per_page_, scored_document_hit_comparator_); - num_returned_ += scored_document_hits.size(); - return scored_document_hits; +class GroupResultLimiter { + public: + GroupResultLimiter( + const std::unordered_map<NamespaceId, int>& namespace_group_id_map, + std::vector<int>& group_result_limits, + const DocumentStore& document_store) + : namespace_group_id_map_(namespace_group_id_map), + group_result_limits_(&group_result_limits), + document_store_(document_store) {} + + // Returns true if the scored_document_hit should be removed. + bool operator()(const ScoredDocumentHit& scored_document_hit) { + auto document_filter_data_or = document_store_.GetDocumentFilterData( + scored_document_hit.document_id()); + if (!document_filter_data_or.ok()) { + return true; + } + NamespaceId namespace_id = + document_filter_data_or.ValueOrDie().namespace_id(); + auto iter = namespace_group_id_map_.find(namespace_id); + if (iter == namespace_group_id_map_.end()) { + return false; + } + int& count = group_result_limits_->at(iter->second); + if (count <= 0) { + return true; + } + --count; + return false; + } + + private: + const std::unordered_map<NamespaceId, int>& namespace_group_id_map_; + std::vector<int>* group_result_limits_; + const DocumentStore& document_store_; +}; + +std::vector<ScoredDocumentHit> ResultState::GetNextPage( + const DocumentStore& document_store) { + int num_requested = num_per_page_; + bool more_results_available = true; + std::vector<ScoredDocumentHit> final_scored_document_hits; + while (more_results_available && num_requested > 0) { + std::vector<ScoredDocumentHit> scored_document_hits = PopTopResultsFromHeap( + &scored_document_hits_, num_requested, scored_document_hit_comparator_); + more_results_available = scored_document_hits.size() == num_requested; + auto itr = std::remove_if( + scored_document_hits.begin(), scored_document_hits.end(), + GroupResultLimiter(namespace_group_id_map_, group_result_limits_, + document_store)); + scored_document_hits.erase(itr, scored_document_hits.end()); + final_scored_document_hits.reserve(final_scored_document_hits.size() + + scored_document_hits.size()); + std::move(scored_document_hits.begin(), scored_document_hits.end(), + std::back_inserter(final_scored_document_hits)); + num_requested = num_per_page_ - final_scored_document_hits.size(); + } + + num_returned_ += final_scored_document_hits.size(); + return final_scored_document_hits; } void ResultState::TruncateHitsTo(int new_size) { diff --git a/icing/result/result-state.h b/icing/result/result-state.h index de810fb..303d610 100644 --- a/icing/result/result-state.h +++ b/icing/result/result-state.h @@ -23,6 +23,8 @@ #include "icing/result/projection-tree.h" #include "icing/result/snippet-context.h" #include "icing/scoring/scored-document-hit.h" +#include "icing/store/document-store.h" +#include "icing/store/namespace-id.h" namespace icing { namespace lib { @@ -31,17 +33,19 @@ namespace lib { // same query. Stored in ResultStateManager. class ResultState { public: - explicit ResultState(std::vector<ScoredDocumentHit> scored_document_hits, - SectionRestrictQueryTermsMap query_terms, - const SearchSpecProto& search_spec, - const ScoringSpecProto& scoring_spec, - const ResultSpecProto& result_spec); + ResultState(std::vector<ScoredDocumentHit> scored_document_hits, + SectionRestrictQueryTermsMap query_terms, + const SearchSpecProto& search_spec, + const ScoringSpecProto& scoring_spec, + const ResultSpecProto& result_spec, + const DocumentStore& document_store); // Returns the next page of results. The size of page is passed in from // ResultSpecProto in constructor. Calling this method could increase the // value of num_returned(), so be careful of the order of calling these // methods. - std::vector<ScoredDocumentHit> GetNextPage(); + std::vector<ScoredDocumentHit> GetNextPage( + const DocumentStore& document_store); // Truncates the vector of ScoredDocumentHits to the given size. The best // ScoredDocumentHits are kept. @@ -83,6 +87,13 @@ class ResultState { // Information needed for projection. std::unordered_map<std::string, ProjectionTree> projection_tree_map_; + // A map between namespace id and the id of the group that it appears in. + std::unordered_map<NamespaceId, int> namespace_group_id_map_; + + // The count of remaining results to return for a group where group id is the + // index. + std::vector<int> group_result_limits_; + // Number of results to return in each page. int num_per_page_; diff --git a/icing/result/result-state_test.cc b/icing/result/result-state_test.cc index 85cb242..f2121a5 100644 --- a/icing/result/result-state_test.cc +++ b/icing/result/result-state_test.cc @@ -15,9 +15,15 @@ #include "icing/result/result-state.h" #include "gtest/gtest.h" +#include "icing/document-builder.h" +#include "icing/file/filesystem.h" #include "icing/portable/equals-proto.h" +#include "icing/schema/schema-store.h" #include "icing/scoring/scored-document-hit.h" +#include "icing/store/document-store.h" #include "icing/testing/common-matchers.h" +#include "icing/testing/tmp-directory.h" +#include "icing/util/clock.h" namespace icing { namespace lib { @@ -50,42 +56,90 @@ ResultSpecProto CreateResultSpec(int num_per_page) { return result_spec; } +class ResultStateTest : public testing::Test { + protected: + void SetUp() override { + schema_store_base_dir_ = GetTestTempDir() + "/schema_store"; + filesystem_.CreateDirectoryRecursively(schema_store_base_dir_.c_str()); + ICING_ASSERT_OK_AND_ASSIGN( + schema_store_, + SchemaStore::Create(&filesystem_, schema_store_base_dir_, &clock_)); + SchemaProto schema; + schema.add_types()->set_schema_type("Document"); + ICING_ASSERT_OK(schema_store_->SetSchema(std::move(schema))); + + doc_store_base_dir_ = GetTestTempDir() + "/document_store"; + filesystem_.CreateDirectoryRecursively(doc_store_base_dir_.c_str()); + ICING_ASSERT_OK_AND_ASSIGN( + DocumentStore::CreateResult result, + DocumentStore::Create(&filesystem_, doc_store_base_dir_, &clock_, + schema_store_.get())); + document_store_ = std::move(result.document_store); + } + + void TearDown() override { + filesystem_.DeleteDirectoryRecursively(doc_store_base_dir_.c_str()); + filesystem_.DeleteDirectoryRecursively(schema_store_base_dir_.c_str()); + } + + ScoredDocumentHit AddScoredDocument(DocumentId document_id) { + DocumentProto document; + document.set_namespace_("namespace"); + document.set_uri(std::to_string(document_id)); + document.set_schema("Document"); + document_store_->Put(std::move(document)); + return ScoredDocumentHit(document_id, kSectionIdMaskNone, /*score=*/1); + } + + DocumentStore& document_store() { return *document_store_; } + + private: + Filesystem filesystem_; + std::string doc_store_base_dir_; + std::string schema_store_base_dir_; + Clock clock_; + std::unique_ptr<DocumentStore> document_store_; + std::unique_ptr<SchemaStore> schema_store_; +}; + // ResultState::ResultState() and ResultState::GetNextPage() are calling // Ranker::BuildHeapInPlace() and Ranker::PopTopResultsFromHeap() directly, so // we don't need to test much on what order is returned as that is tested in // Ranker's tests. Here we just need one sanity test to make sure that the // correct functions are called. -TEST(ResultStateTest, ShouldReturnNextPage) { +TEST_F(ResultStateTest, ShouldReturnNextPage) { + ScoredDocumentHit scored_hit_0 = AddScoredDocument(/*document_id=*/0); + ScoredDocumentHit scored_hit_1 = AddScoredDocument(/*document_id=*/1); + ScoredDocumentHit scored_hit_2 = AddScoredDocument(/*document_id=*/2); + ScoredDocumentHit scored_hit_3 = AddScoredDocument(/*document_id=*/3); + ScoredDocumentHit scored_hit_4 = AddScoredDocument(/*document_id=*/4); std::vector<ScoredDocumentHit> scored_document_hits = { - CreateScoredDocumentHit(/*document_id=*/2), - CreateScoredDocumentHit(/*document_id=*/1), - CreateScoredDocumentHit(/*document_id=*/3), - CreateScoredDocumentHit(/*document_id=*/5), - CreateScoredDocumentHit(/*document_id=*/4)}; + scored_hit_1, scored_hit_0, scored_hit_2, scored_hit_4, scored_hit_3}; ResultState result_state(scored_document_hits, /*query_terms=*/{}, CreateSearchSpec(TermMatchType::EXACT_ONLY), CreateScoringSpec(/*is_descending_order=*/true), - CreateResultSpec(/*num_per_page=*/2)); + CreateResultSpec(/*num_per_page=*/2), + document_store()); EXPECT_THAT( - result_state.GetNextPage(), + result_state.GetNextPage(document_store()), ElementsAre( - EqualsScoredDocumentHit(CreateScoredDocumentHit(/*document_id=*/5)), - EqualsScoredDocumentHit(CreateScoredDocumentHit(/*document_id=*/4)))); + EqualsScoredDocumentHit(CreateScoredDocumentHit(/*document_id=*/4)), + EqualsScoredDocumentHit(CreateScoredDocumentHit(/*document_id=*/3)))); EXPECT_THAT( - result_state.GetNextPage(), + result_state.GetNextPage(document_store()), ElementsAre( - EqualsScoredDocumentHit(CreateScoredDocumentHit(/*document_id=*/3)), - EqualsScoredDocumentHit(CreateScoredDocumentHit(/*document_id=*/2)))); + EqualsScoredDocumentHit(CreateScoredDocumentHit(/*document_id=*/2)), + EqualsScoredDocumentHit(CreateScoredDocumentHit(/*document_id=*/1)))); - EXPECT_THAT(result_state.GetNextPage(), + EXPECT_THAT(result_state.GetNextPage(document_store()), ElementsAre(EqualsScoredDocumentHit( - CreateScoredDocumentHit(/*document_id=*/1)))); + CreateScoredDocumentHit(/*document_id=*/0)))); } -TEST(ResultStateTest, ShouldReturnSnippetContextAccordingToSpecs) { +TEST_F(ResultStateTest, ShouldReturnSnippetContextAccordingToSpecs) { ResultSpecProto result_spec = CreateResultSpec(/*num_per_page=*/2); result_spec.mutable_snippet_spec()->set_num_to_snippet(5); result_spec.mutable_snippet_spec()->set_num_matches_per_property(5); @@ -97,7 +151,8 @@ TEST(ResultStateTest, ShouldReturnSnippetContextAccordingToSpecs) { ResultState result_state( /*scored_document_hits=*/{}, query_terms_map, CreateSearchSpec(TermMatchType::EXACT_ONLY), - CreateScoringSpec(/*is_descending_order=*/true), result_spec); + CreateScoringSpec(/*is_descending_order=*/true), result_spec, + document_store()); const SnippetContext& snippet_context = result_state.snippet_context(); @@ -117,7 +172,7 @@ TEST(ResultStateTest, ShouldReturnSnippetContextAccordingToSpecs) { EXPECT_THAT(snippet_context2.match_type, Eq(TermMatchType::EXACT_ONLY)); } -TEST(ResultStateTest, NoSnippetingShouldReturnNull) { +TEST_F(ResultStateTest, NoSnippetingShouldReturnNull) { ResultSpecProto result_spec = CreateResultSpec(/*num_per_page=*/2); // Setting num_to_snippet to 0 so that snippeting info won't be // stored. @@ -131,7 +186,7 @@ TEST(ResultStateTest, NoSnippetingShouldReturnNull) { ResultState result_state(/*scored_document_hits=*/{}, query_terms_map, CreateSearchSpec(TermMatchType::EXACT_ONLY), CreateScoringSpec(/*is_descending_order=*/true), - result_spec); + result_spec, document_store()); const SnippetContext& snippet_context = result_state.snippet_context(); EXPECT_THAT(snippet_context.query_terms, IsEmpty()); @@ -141,72 +196,375 @@ TEST(ResultStateTest, NoSnippetingShouldReturnNull) { EXPECT_THAT(snippet_context.match_type, TermMatchType::UNKNOWN); } -TEST(ResultStateTest, ShouldTruncateToNewSize) { +TEST_F(ResultStateTest, ShouldTruncateToNewSize) { + ScoredDocumentHit scored_hit_0 = AddScoredDocument(/*document_id=*/0); + ScoredDocumentHit scored_hit_1 = AddScoredDocument(/*document_id=*/1); + ScoredDocumentHit scored_hit_2 = AddScoredDocument(/*document_id=*/2); + ScoredDocumentHit scored_hit_3 = AddScoredDocument(/*document_id=*/3); + ScoredDocumentHit scored_hit_4 = AddScoredDocument(/*document_id=*/4); std::vector<ScoredDocumentHit> scored_document_hits = { - CreateScoredDocumentHit(/*document_id=*/2), - CreateScoredDocumentHit(/*document_id=*/1), - CreateScoredDocumentHit(/*document_id=*/3), - CreateScoredDocumentHit(/*document_id=*/5), - CreateScoredDocumentHit(/*document_id=*/4)}; + scored_hit_1, scored_hit_0, scored_hit_2, scored_hit_4, scored_hit_3}; // Creates a ResultState with 5 ScoredDocumentHits. ResultState result_state(scored_document_hits, /*query_terms=*/{}, CreateSearchSpec(TermMatchType::EXACT_ONLY), CreateScoringSpec(/*is_descending_order=*/true), - CreateResultSpec(/*num_per_page=*/5)); + CreateResultSpec(/*num_per_page=*/5), + document_store()); result_state.TruncateHitsTo(/*new_size=*/3); // The best 3 are left. EXPECT_THAT( - result_state.GetNextPage(), + result_state.GetNextPage(document_store()), ElementsAre( - EqualsScoredDocumentHit(CreateScoredDocumentHit(/*document_id=*/5)), EqualsScoredDocumentHit(CreateScoredDocumentHit(/*document_id=*/4)), - EqualsScoredDocumentHit(CreateScoredDocumentHit(/*document_id=*/3)))); + EqualsScoredDocumentHit(CreateScoredDocumentHit(/*document_id=*/3)), + EqualsScoredDocumentHit(CreateScoredDocumentHit(/*document_id=*/2)))); } -TEST(ResultStateTest, ShouldTruncateToZero) { +TEST_F(ResultStateTest, ShouldTruncateToZero) { + ScoredDocumentHit scored_hit_0 = AddScoredDocument(/*document_id=*/0); + ScoredDocumentHit scored_hit_1 = AddScoredDocument(/*document_id=*/1); + ScoredDocumentHit scored_hit_2 = AddScoredDocument(/*document_id=*/2); + ScoredDocumentHit scored_hit_3 = AddScoredDocument(/*document_id=*/3); + ScoredDocumentHit scored_hit_4 = AddScoredDocument(/*document_id=*/4); std::vector<ScoredDocumentHit> scored_document_hits = { - CreateScoredDocumentHit(/*document_id=*/2), - CreateScoredDocumentHit(/*document_id=*/1), - CreateScoredDocumentHit(/*document_id=*/3), - CreateScoredDocumentHit(/*document_id=*/5), - CreateScoredDocumentHit(/*document_id=*/4)}; + scored_hit_1, scored_hit_0, scored_hit_2, scored_hit_4, scored_hit_3}; // Creates a ResultState with 5 ScoredDocumentHits. ResultState result_state(scored_document_hits, /*query_terms=*/{}, CreateSearchSpec(TermMatchType::EXACT_ONLY), CreateScoringSpec(/*is_descending_order=*/true), - CreateResultSpec(/*num_per_page=*/5)); + CreateResultSpec(/*num_per_page=*/5), + document_store()); result_state.TruncateHitsTo(/*new_size=*/0); - EXPECT_THAT(result_state.GetNextPage(), IsEmpty()); + EXPECT_THAT(result_state.GetNextPage(document_store()), IsEmpty()); } -TEST(ResultStateTest, ShouldNotTruncateToNegative) { +TEST_F(ResultStateTest, ShouldNotTruncateToNegative) { + ScoredDocumentHit scored_hit_0 = AddScoredDocument(/*document_id=*/0); + ScoredDocumentHit scored_hit_1 = AddScoredDocument(/*document_id=*/1); + ScoredDocumentHit scored_hit_2 = AddScoredDocument(/*document_id=*/2); + ScoredDocumentHit scored_hit_3 = AddScoredDocument(/*document_id=*/3); + ScoredDocumentHit scored_hit_4 = AddScoredDocument(/*document_id=*/4); std::vector<ScoredDocumentHit> scored_document_hits = { - CreateScoredDocumentHit(/*document_id=*/2), - CreateScoredDocumentHit(/*document_id=*/1), - CreateScoredDocumentHit(/*document_id=*/3), - CreateScoredDocumentHit(/*document_id=*/5), - CreateScoredDocumentHit(/*document_id=*/4)}; + scored_hit_1, scored_hit_0, scored_hit_2, scored_hit_4, scored_hit_3}; // Creates a ResultState with 5 ScoredDocumentHits. ResultState result_state(scored_document_hits, /*query_terms=*/{}, CreateSearchSpec(TermMatchType::EXACT_ONLY), CreateScoringSpec(/*is_descending_order=*/true), - CreateResultSpec(/*num_per_page=*/5)); + CreateResultSpec(/*num_per_page=*/5), + document_store()); result_state.TruncateHitsTo(/*new_size=*/-1); // Results are not affected. EXPECT_THAT( - result_state.GetNextPage(), + result_state.GetNextPage(document_store()), ElementsAre( - EqualsScoredDocumentHit(CreateScoredDocumentHit(/*document_id=*/5)), EqualsScoredDocumentHit(CreateScoredDocumentHit(/*document_id=*/4)), EqualsScoredDocumentHit(CreateScoredDocumentHit(/*document_id=*/3)), EqualsScoredDocumentHit(CreateScoredDocumentHit(/*document_id=*/2)), - EqualsScoredDocumentHit(CreateScoredDocumentHit(/*document_id=*/1)))); + EqualsScoredDocumentHit(CreateScoredDocumentHit(/*document_id=*/1)), + EqualsScoredDocumentHit(CreateScoredDocumentHit(/*document_id=*/0)))); +} + +TEST_F(ResultStateTest, ResultGroupingShouldLimitResults) { + // Creates 2 documents and ensures the relationship in terms of document + // score is: document1 < document2 + DocumentProto document1 = DocumentBuilder() + .SetKey("namespace", "uri/1") + .SetSchema("Document") + .SetScore(1) + .Build(); + DocumentProto document2 = DocumentBuilder() + .SetKey("namespace", "uri/2") + .SetSchema("Document") + .SetScore(2) + .Build(); + + ICING_ASSERT_OK_AND_ASSIGN(DocumentId document_id1, + document_store().Put(document1)); + ScoredDocumentHit scored_hit_1(document_id1, kSectionIdMaskNone, + document1.score()); + ICING_ASSERT_OK_AND_ASSIGN(DocumentId document_id2, + document_store().Put(document2)); + ScoredDocumentHit scored_hit_2(document_id2, kSectionIdMaskNone, + document2.score()); + std::vector<ScoredDocumentHit> scored_document_hits = {scored_hit_2, + scored_hit_1}; + + // Create a ResultSpec that limits "namespace" to a single result. + ResultSpecProto result_spec; + result_spec.set_num_per_page(5); + ResultSpecProto::ResultGrouping* result_grouping = + result_spec.add_result_groupings(); + result_grouping->set_max_results(1); + result_grouping->add_namespaces("namespace"); + + // Creates a ResultState with 2 ScoredDocumentHits. + ResultState result_state(scored_document_hits, /*query_terms=*/{}, + CreateSearchSpec(TermMatchType::EXACT_ONLY), + CreateScoringSpec(/*is_descending_order=*/true), + result_spec, document_store()); + + // Only the top ranked document in "namespace" (document2), should be + // returned. + EXPECT_THAT(result_state.GetNextPage(document_store()), + ElementsAre(EqualsScoredDocumentHit(scored_hit_2))); +} + +TEST_F(ResultStateTest, ResultGroupingDoesNotLimitOtherNamespaceResults) { + // Creates 4 documents and ensures the relationship in terms of document + // score is: document1 < document2 < document3 < document4 + DocumentProto document1 = DocumentBuilder() + .SetKey("namespace1", "uri/1") + .SetSchema("Document") + .SetScore(1) + .Build(); + DocumentProto document2 = DocumentBuilder() + .SetKey("namespace1", "uri/2") + .SetSchema("Document") + .SetScore(2) + .Build(); + DocumentProto document3 = DocumentBuilder() + .SetKey("namespace2", "uri/3") + .SetSchema("Document") + .SetScore(3) + .Build(); + DocumentProto document4 = DocumentBuilder() + .SetKey("namespace2", "uri/4") + .SetSchema("Document") + .SetScore(4) + .Build(); + + ICING_ASSERT_OK_AND_ASSIGN(DocumentId document_id1, + document_store().Put(document1)); + ScoredDocumentHit scored_hit_1(document_id1, kSectionIdMaskNone, + document1.score()); + ICING_ASSERT_OK_AND_ASSIGN(DocumentId document_id2, + document_store().Put(document2)); + ScoredDocumentHit scored_hit_2(document_id2, kSectionIdMaskNone, + document2.score()); + ICING_ASSERT_OK_AND_ASSIGN(DocumentId document_id3, + document_store().Put(document3)); + ScoredDocumentHit scored_hit_3(document_id3, kSectionIdMaskNone, + document3.score()); + ICING_ASSERT_OK_AND_ASSIGN(DocumentId document_id4, + document_store().Put(document4)); + ScoredDocumentHit scored_hit_4(document_id4, kSectionIdMaskNone, + document4.score()); + std::vector<ScoredDocumentHit> scored_document_hits = { + scored_hit_4, scored_hit_3, scored_hit_2, scored_hit_1}; + + // Create a ResultSpec that limits "namespace1" to a single result, but + // doesn't limit "namespace2". + ResultSpecProto result_spec; + result_spec.set_num_per_page(5); + ResultSpecProto::ResultGrouping* result_grouping = + result_spec.add_result_groupings(); + result_grouping->set_max_results(1); + result_grouping->add_namespaces("namespace1"); + + // Creates a ResultState with 4 ScoredDocumentHits. + ResultState result_state(scored_document_hits, /*query_terms=*/{}, + CreateSearchSpec(TermMatchType::EXACT_ONLY), + CreateScoringSpec(/*is_descending_order=*/true), + result_spec, document_store()); + + // Only the top ranked document in "namespace" (document2), should be + // returned. + EXPECT_THAT(result_state.GetNextPage(document_store()), + ElementsAre(EqualsScoredDocumentHit(scored_hit_4), + EqualsScoredDocumentHit(scored_hit_3), + EqualsScoredDocumentHit(scored_hit_2))); +} + +TEST_F(ResultStateTest, ResultGroupingNonexistentNamespaceShouldBeIgnored) { + // Creates 2 documents and ensures the relationship in terms of document + // score is: document1 < document2 + DocumentProto document1 = DocumentBuilder() + .SetKey("namespace", "uri/1") + .SetSchema("Document") + .SetScore(1) + .Build(); + DocumentProto document2 = DocumentBuilder() + .SetKey("namespace", "uri/2") + .SetSchema("Document") + .SetScore(2) + .Build(); + + ICING_ASSERT_OK_AND_ASSIGN(DocumentId document_id1, + document_store().Put(document1)); + ScoredDocumentHit scored_hit_1(document_id1, kSectionIdMaskNone, + document1.score()); + ICING_ASSERT_OK_AND_ASSIGN(DocumentId document_id2, + document_store().Put(document2)); + ScoredDocumentHit scored_hit_2(document_id2, kSectionIdMaskNone, + document2.score()); + std::vector<ScoredDocumentHit> scored_document_hits = {scored_hit_2, + scored_hit_1}; + + // Create a ResultSpec that limits "namespace"+"nonExistentNamespace" to a + // single result. + ResultSpecProto result_spec; + result_spec.set_num_per_page(5); + ResultSpecProto::ResultGrouping* result_grouping = + result_spec.add_result_groupings(); + result_grouping->set_max_results(1); + result_grouping->add_namespaces("namespace"); + result_grouping->add_namespaces("nonexistentNamespace"); + + // Creates a ResultState with 2 ScoredDocumentHits. + ResultState result_state(scored_document_hits, /*query_terms=*/{}, + CreateSearchSpec(TermMatchType::EXACT_ONLY), + CreateScoringSpec(/*is_descending_order=*/true), + result_spec, document_store()); + + // Only the top ranked document in "namespace" (document2), should be + // returned. The presence of "nonexistentNamespace" in the same result + // grouping should have no effect. + EXPECT_THAT(result_state.GetNextPage(document_store()), + ElementsAre(EqualsScoredDocumentHit(scored_hit_2))); +} + +TEST_F(ResultStateTest, ResultGroupingMultiNamespaceGrouping) { + // Creates 6 documents and ensures the relationship in terms of document + // score is: document1 < document2 < document3 < document4 < document5 < + // document6 + DocumentProto document1 = DocumentBuilder() + .SetKey("namespace1", "uri/1") + .SetSchema("Document") + .SetScore(1) + .Build(); + DocumentProto document2 = DocumentBuilder() + .SetKey("namespace1", "uri/2") + .SetSchema("Document") + .SetScore(2) + .Build(); + DocumentProto document3 = DocumentBuilder() + .SetKey("namespace2", "uri/3") + .SetSchema("Document") + .SetScore(3) + .Build(); + DocumentProto document4 = DocumentBuilder() + .SetKey("namespace2", "uri/4") + .SetSchema("Document") + .SetScore(4) + .Build(); + DocumentProto document5 = DocumentBuilder() + .SetKey("namespace3", "uri/5") + .SetSchema("Document") + .SetScore(5) + .Build(); + DocumentProto document6 = DocumentBuilder() + .SetKey("namespace3", "uri/6") + .SetSchema("Document") + .SetScore(6) + .Build(); + + ICING_ASSERT_OK_AND_ASSIGN(DocumentId document_id1, + document_store().Put(document1)); + ScoredDocumentHit scored_hit_1(document_id1, kSectionIdMaskNone, + document1.score()); + ICING_ASSERT_OK_AND_ASSIGN(DocumentId document_id2, + document_store().Put(document2)); + ScoredDocumentHit scored_hit_2(document_id2, kSectionIdMaskNone, + document2.score()); + ICING_ASSERT_OK_AND_ASSIGN(DocumentId document_id3, + document_store().Put(document3)); + ScoredDocumentHit scored_hit_3(document_id3, kSectionIdMaskNone, + document3.score()); + ICING_ASSERT_OK_AND_ASSIGN(DocumentId document_id4, + document_store().Put(document4)); + ScoredDocumentHit scored_hit_4(document_id4, kSectionIdMaskNone, + document4.score()); + ICING_ASSERT_OK_AND_ASSIGN(DocumentId document_id5, + document_store().Put(document5)); + ScoredDocumentHit scored_hit_5(document_id5, kSectionIdMaskNone, + document5.score()); + ICING_ASSERT_OK_AND_ASSIGN(DocumentId document_id6, + document_store().Put(document6)); + ScoredDocumentHit scored_hit_6(document_id6, kSectionIdMaskNone, + document6.score()); + std::vector<ScoredDocumentHit> scored_document_hits = { + scored_hit_6, scored_hit_5, scored_hit_4, + scored_hit_3, scored_hit_2, scored_hit_1}; + + // Create a ResultSpec that limits "namespace1" to a single result and limits + // "namespace2"+"namespace3" to a total of two results. + ResultSpecProto result_spec; + result_spec.set_num_per_page(5); + ResultSpecProto::ResultGrouping* result_grouping = + result_spec.add_result_groupings(); + result_grouping->set_max_results(1); + result_grouping->add_namespaces("namespace1"); + result_grouping = result_spec.add_result_groupings(); + result_grouping->set_max_results(2); + result_grouping->add_namespaces("namespace2"); + result_grouping->add_namespaces("namespace3"); + + // Creates a ResultState with 4 ScoredDocumentHits. + ResultState result_state(scored_document_hits, /*query_terms=*/{}, + CreateSearchSpec(TermMatchType::EXACT_ONLY), + CreateScoringSpec(/*is_descending_order=*/true), + result_spec, document_store()); + + // Only the top-ranked result in "namespace1" (document2) should be returned. + // Only the top-ranked results across "namespace2" and "namespace3" + // (document6, document5) should be returned. + EXPECT_THAT(result_state.GetNextPage(document_store()), + ElementsAre(EqualsScoredDocumentHit(scored_hit_6), + EqualsScoredDocumentHit(scored_hit_5), + EqualsScoredDocumentHit(scored_hit_2))); +} + +TEST_F(ResultStateTest, ResultGroupingOnlyNonexistentNamespaces) { + // Creates 2 documents and ensures the relationship in terms of document + // score is: document1 < document2 + DocumentProto document1 = DocumentBuilder() + .SetKey("namespace", "uri/1") + .SetSchema("Document") + .SetScore(1) + .Build(); + DocumentProto document2 = DocumentBuilder() + .SetKey("namespace", "uri/2") + .SetSchema("Document") + .SetScore(2) + .Build(); + + ICING_ASSERT_OK_AND_ASSIGN(DocumentId document_id1, + document_store().Put(document1)); + ScoredDocumentHit scored_hit_1(document_id1, kSectionIdMaskNone, + document1.score()); + ICING_ASSERT_OK_AND_ASSIGN(DocumentId document_id2, + document_store().Put(document2)); + ScoredDocumentHit scored_hit_2(document_id2, kSectionIdMaskNone, + document2.score()); + std::vector<ScoredDocumentHit> scored_document_hits = {scored_hit_2, + scored_hit_1}; + + // Create a ResultSpec that limits "nonexistentNamespace" to a single result. + // but doesn't limit "namespace" + ResultSpecProto result_spec; + result_spec.set_num_per_page(5); + ResultSpecProto::ResultGrouping* result_grouping = + result_spec.add_result_groupings(); + result_grouping->set_max_results(1); + result_grouping->add_namespaces("nonexistentNamespace"); + + // Creates a ResultState with 2 ScoredDocumentHits. + ResultState result_state(scored_document_hits, /*query_terms=*/{}, + CreateSearchSpec(TermMatchType::EXACT_ONLY), + CreateScoringSpec(/*is_descending_order=*/true), + result_spec, document_store()); + + // All documents in "namespace" should be returned. The presence of + // "nonexistentNamespace" should have no effect. + EXPECT_THAT(result_state.GetNextPage(document_store()), + ElementsAre(EqualsScoredDocumentHit(scored_hit_2), + EqualsScoredDocumentHit(scored_hit_1))); } } // namespace diff --git a/icing/scoring/scorer_test.cc b/icing/scoring/scorer_test.cc index b114515..31bdd15 100644 --- a/icing/scoring/scorer_test.cc +++ b/icing/scoring/scorer_test.cc @@ -25,6 +25,7 @@ #include "icing/proto/document.pb.h" #include "icing/proto/schema.pb.h" #include "icing/proto/scoring.pb.h" +#include "icing/schema-builder.h" #include "icing/schema/schema-store.h" #include "icing/store/document-id.h" #include "icing/store/document-store.h" @@ -38,6 +39,12 @@ namespace lib { namespace { using ::testing::Eq; +constexpr PropertyConfigProto_DataType_Code TYPE_STRING = + PropertyConfigProto_DataType_Code_STRING; + +constexpr PropertyConfigProto_Cardinality_Code CARDINALITY_REQUIRED = + PropertyConfigProto_Cardinality_Code_REQUIRED; + class ScorerTest : public testing::Test { protected: ScorerTest() @@ -64,13 +71,14 @@ class ScorerTest : public testing::Test { document_store_ = std::move(create_result.document_store); // Creates a simple email schema - SchemaProto test_email_schema; - auto type_config = test_email_schema.add_types(); - type_config->set_schema_type("email"); - auto subject = type_config->add_properties(); - subject->set_property_name("subject"); - subject->set_data_type(PropertyConfigProto::DataType::STRING); - subject->set_cardinality(PropertyConfigProto::Cardinality::REQUIRED); + SchemaProto test_email_schema = + SchemaBuilder() + .AddType(SchemaTypeConfigBuilder().SetType("email").AddProperty( + PropertyConfigBuilder() + .SetName("subject") + .SetDataType(TYPE_STRING) + .SetCardinality(CARDINALITY_REQUIRED))) + .Build(); ICING_ASSERT_OK(schema_store_->SetSchema(test_email_schema)); } diff --git a/icing/scoring/scoring-processor_test.cc b/icing/scoring/scoring-processor_test.cc index 65eecd1..5e251eb 100644 --- a/icing/scoring/scoring-processor_test.cc +++ b/icing/scoring/scoring-processor_test.cc @@ -24,6 +24,7 @@ #include "icing/proto/document.pb.h" #include "icing/proto/schema.pb.h" #include "icing/proto/scoring.pb.h" +#include "icing/schema-builder.h" #include "icing/testing/common-matchers.h" #include "icing/testing/fake-clock.h" #include "icing/testing/tmp-directory.h" @@ -36,6 +37,12 @@ using ::testing::ElementsAre; using ::testing::IsEmpty; using ::testing::SizeIs; +constexpr PropertyConfigProto_DataType_Code TYPE_STRING = + PropertyConfigProto_DataType_Code_STRING; + +constexpr PropertyConfigProto_Cardinality_Code CARDINALITY_OPTIONAL = + PropertyConfigProto_Cardinality_Code_OPTIONAL; + class ScoringProcessorTest : public testing::Test { protected: ScoringProcessorTest() @@ -60,14 +67,14 @@ class ScoringProcessorTest : public testing::Test { document_store_ = std::move(create_result.document_store); // Creates a simple email schema - SchemaProto test_email_schema; - auto type_config = test_email_schema.add_types(); - type_config->set_schema_type("email"); - auto subject = type_config->add_properties(); - subject->set_property_name("subject"); - subject->set_data_type(PropertyConfigProto::DataType::STRING); - subject->set_cardinality(PropertyConfigProto::Cardinality::OPTIONAL); - + SchemaProto test_email_schema = + SchemaBuilder() + .AddType(SchemaTypeConfigBuilder().SetType("email").AddProperty( + PropertyConfigBuilder() + .SetName("subject") + .SetDataType(TYPE_STRING) + .SetCardinality(CARDINALITY_OPTIONAL))) + .Build(); ICING_ASSERT_OK(schema_store_->SetSchema(test_email_schema)); } diff --git a/icing/store/document-store.cc b/icing/store/document-store.cc index 59944fe..33c0d1a 100644 --- a/icing/store/document-store.cc +++ b/icing/store/document-store.cc @@ -19,6 +19,7 @@ #include <memory> #include <string> #include <string_view> +#include <unordered_map> #include <utility> #include <vector> @@ -36,6 +37,7 @@ #include "icing/proto/document.pb.h" #include "icing/proto/document_wrapper.pb.h" #include "icing/proto/logging.pb.h" +#include "icing/proto/storage.pb.h" #include "icing/schema/schema-store.h" #include "icing/store/corpus-associated-scoring-data.h" #include "icing/store/corpus-id.h" @@ -44,6 +46,7 @@ #include "icing/store/document-id.h" #include "icing/store/key-mapper.h" #include "icing/store/namespace-id.h" +#include "icing/store/usage-store.h" #include "icing/tokenization/language-segmenter.h" #include "icing/util/clock.h" #include "icing/util/crc32.h" @@ -189,17 +192,6 @@ int64_t CalculateExpirationTimestampMs(int64_t creation_timestamp_ms, return expiration_timestamp_ms; } -void IncrementDeletedOrExpired(FileBackedVector<int64_t>* document_id_mapper, - DocumentId document_id, int* num_deleted_out, - int* num_expired_out) { - auto location_or = document_id_mapper->Get(document_id); - if (location_or.ok() && *location_or.ValueOrDie() == kDocDeletedFlag) { - ++(*num_deleted_out); - } else { - ++(*num_expired_out); - } -} - } // namespace DocumentStore::DocumentStore(const Filesystem* filesystem, @@ -227,7 +219,7 @@ libtextclassifier3::StatusOr<DocumentId> DocumentStore::Put( DocumentStore::~DocumentStore() { if (initialized_) { - if (!PersistToDisk().ok()) { + if (!PersistToDisk(PersistType::FULL).ok()) { ICING_LOG(ERROR) << "Error persisting to disk in DocumentStore destructor"; } @@ -1405,7 +1397,12 @@ libtextclassifier3::StatusOr<int> DocumentStore::BatchDelete( return num_updated_documents; } -libtextclassifier3::Status DocumentStore::PersistToDisk() { +libtextclassifier3::Status DocumentStore::PersistToDisk( + PersistType::Code persist_type) { + if (persist_type == PersistType::LITE) { + // only persist the document log. + return document_log_->PersistToDisk(); + } ICING_RETURN_IF_ERROR(document_log_->PersistToDisk()); ICING_RETURN_IF_ERROR(document_key_mapper_->PersistToDisk()); ICING_RETURN_IF_ERROR(document_id_mapper_->PersistToDisk()); @@ -1451,21 +1448,104 @@ DocumentStorageInfoProto DocumentStore::GetMemberStorageInfo() const { DocumentStorageInfoProto DocumentStore::CalculateDocumentStatusCounts( DocumentStorageInfoProto storage_info) const { - int num_alive = 0; - int num_expired = 0; - int num_deleted = 0; + int total_num_alive = 0; + int total_num_expired = 0; + int total_num_deleted = 0; + std::unordered_map<NamespaceId, std::string> namespace_id_to_namespace = + namespace_mapper_->GetValuesToKeys(); + std::unordered_map<std::string, NamespaceStorageInfoProto> + namespace_to_storage_info; + for (DocumentId document_id = 0; document_id < document_id_mapper_->num_elements(); ++document_id) { + // Check if it's deleted first. + auto location_or = document_id_mapper_->Get(document_id); + if (!location_or.ok()) { + ICING_VLOG(1) << "Error trying to get document offsets for document " + "store storage info counts."; + continue; + } + if (*location_or.ValueOrDie() == kDocDeletedFlag) { + // We don't have the namespace id of hard deleted documents anymore, so we + // can't add to our namespace storage info. + ++total_num_deleted; + continue; + } + + // At this point, the document is either alive or expired, we can get + // namespace info for it. + auto filter_data_or = filter_cache_->Get(document_id); + if (!filter_data_or.ok()) { + ICING_VLOG(1) << "Error trying to get filter data for document store " + "storage info counts."; + continue; + } + const DocumentFilterData* filter_data = filter_data_or.ValueOrDie(); + auto itr = namespace_id_to_namespace.find(filter_data->namespace_id()); + if (itr == namespace_id_to_namespace.end()) { + ICING_VLOG(1) << "Error trying to find namespace for document store " + "storage info counts."; + continue; + } + const std::string& name_space = itr->second; + + // Always set the namespace, if the NamespaceStorageInfoProto didn't exist + // before, we'll get back a default instance of it. + NamespaceStorageInfoProto& namespace_storage_info = + namespace_to_storage_info[name_space]; + namespace_storage_info.set_namespace_(name_space); + + // Get usage scores + auto usage_scores_or = usage_store_->GetUsageScores(document_id); + if (!usage_scores_or.ok()) { + ICING_VLOG(1) << "Error trying to get usage scores for document store " + "storage info counts."; + continue; + } + UsageStore::UsageScores usage_scores = usage_scores_or.ValueOrDie(); + + // Update our stats if (DoesDocumentExist(document_id)) { - ++num_alive; + ++total_num_alive; + namespace_storage_info.set_num_alive_documents( + namespace_storage_info.num_alive_documents() + 1); + if (usage_scores.usage_type1_count > 0) { + namespace_storage_info.set_num_alive_documents_usage_type1( + namespace_storage_info.num_alive_documents_usage_type1() + 1); + } + if (usage_scores.usage_type2_count > 0) { + namespace_storage_info.set_num_alive_documents_usage_type2( + namespace_storage_info.num_alive_documents_usage_type2() + 1); + } + if (usage_scores.usage_type3_count > 0) { + namespace_storage_info.set_num_alive_documents_usage_type3( + namespace_storage_info.num_alive_documents_usage_type3() + 1); + } } else { - IncrementDeletedOrExpired(document_id_mapper_.get(), document_id, - &num_deleted, &num_expired); + ++total_num_expired; + namespace_storage_info.set_num_expired_documents( + namespace_storage_info.num_expired_documents() + 1); + if (usage_scores.usage_type1_count > 0) { + namespace_storage_info.set_num_expired_documents_usage_type1( + namespace_storage_info.num_expired_documents_usage_type1() + 1); + } + if (usage_scores.usage_type2_count > 0) { + namespace_storage_info.set_num_expired_documents_usage_type2( + namespace_storage_info.num_expired_documents_usage_type2() + 1); + } + if (usage_scores.usage_type3_count > 0) { + namespace_storage_info.set_num_expired_documents_usage_type3( + namespace_storage_info.num_expired_documents_usage_type3() + 1); + } } } - storage_info.set_num_alive_documents(num_alive); - storage_info.set_num_deleted_documents(num_deleted); - storage_info.set_num_expired_documents(num_expired); + + for (auto& itr : namespace_to_storage_info) { + storage_info.mutable_namespace_storage_info()->Add(std::move(itr.second)); + } + storage_info.set_num_alive_documents(total_num_alive); + storage_info.set_num_deleted_documents(total_num_deleted); + storage_info.set_num_expired_documents(total_num_expired); return storage_info; } @@ -1649,9 +1729,13 @@ libtextclassifier3::Status DocumentStore::OptimizeInto( for (DocumentId document_id = 0; document_id < size; document_id++) { auto document_or = Get(document_id, /*clear_internal_fields=*/false); if (absl_ports::IsNotFound(document_or.status())) { - // Skip nonexistent documents - IncrementDeletedOrExpired(document_id_mapper_.get(), document_id, - &num_deleted, &num_expired); + // Don't optimize nonexistent documents, but collect stats + auto location_or = document_id_mapper_->Get(document_id); + if (location_or.ok() && *location_or.ValueOrDie() == kDocDeletedFlag) { + ++num_deleted; + } else { + ++num_expired; + } continue; } else if (!document_or.ok()) { // Real error, pass up @@ -1701,7 +1785,7 @@ libtextclassifier3::Status DocumentStore::OptimizeInto( stats->set_num_deleted_documents(num_deleted); stats->set_num_expired_documents(num_expired); } - ICING_RETURN_IF_ERROR(new_doc_store->PersistToDisk()); + ICING_RETURN_IF_ERROR(new_doc_store->PersistToDisk(PersistType::FULL)); return libtextclassifier3::Status::OK; } diff --git a/icing/store/document-store.h b/icing/store/document-store.h index 3b8408d..533b240 100644 --- a/icing/store/document-store.h +++ b/icing/store/document-store.h @@ -30,6 +30,7 @@ #include "icing/proto/document_wrapper.pb.h" #include "icing/proto/logging.pb.h" #include "icing/proto/optimize.pb.h" +#include "icing/proto/persist.pb.h" #include "icing/proto/storage.pb.h" #include "icing/schema/schema-store.h" #include "icing/store/corpus-associated-scoring-data.h" @@ -351,7 +352,7 @@ class DocumentStore { // Returns: // OK on success // INTERNAL on I/O error - libtextclassifier3::Status PersistToDisk(); + libtextclassifier3::Status PersistToDisk(PersistType::Code persist_type); // Calculates the StorageInfo for the Document Store. // diff --git a/icing/store/document-store_test.cc b/icing/store/document-store_test.cc index 440b48f..02e5f1b 100644 --- a/icing/store/document-store_test.cc +++ b/icing/store/document-store_test.cc @@ -66,6 +66,21 @@ using ::testing::Not; using ::testing::Return; using ::testing::UnorderedElementsAre; +const NamespaceStorageInfoProto& GetNamespaceStorageInfo( + const DocumentStorageInfoProto& storage_info, + const std::string& name_space) { + for (const NamespaceStorageInfoProto& namespace_storage_info : + storage_info.namespace_storage_info()) { + if (namespace_storage_info.namespace_() == name_space) { + return namespace_storage_info; + } + } + // Didn't find our namespace, fail the test. + EXPECT_TRUE(false) << "Failed to find namespace '" << name_space + << "' in DocumentStorageInfoProto."; + return std::move(NamespaceStorageInfoProto()); +} + UsageReport CreateUsageReport(std::string name_space, std::string uri, int64 timestamp_ms, UsageReport::UsageType usage_type) { @@ -3498,6 +3513,35 @@ TEST_F(DocumentStoreTest, DocumentStoreStorageInfo) { document3.set_uri("uri3"); ICING_ASSERT_OK(doc_store->Put(document3)); + DocumentProto document4 = test_document1_; + document4.set_namespace_("namespace.2"); + document4.set_uri("uri1"); + ICING_ASSERT_OK(doc_store->Put(document4)); + + // Report usage with type 1 on document1 + UsageReport usage_report_type1 = CreateUsageReport( + /*name_space=*/"namespace.1", /*uri=*/"uri1", /*timestamp_ms=*/1000, + UsageReport::USAGE_TYPE1); + ICING_ASSERT_OK(doc_store->ReportUsage(usage_report_type1)); + + // Report usage with type 2 on document2 + UsageReport usage_report_type2 = CreateUsageReport( + /*name_space=*/"namespace.1", /*uri=*/"uri2", /*timestamp_ms=*/1000, + UsageReport::USAGE_TYPE2); + ICING_ASSERT_OK(doc_store->ReportUsage(usage_report_type2)); + + // Report usage with type 3 on document3 + UsageReport usage_report_type3 = CreateUsageReport( + /*name_space=*/"namespace.1", /*uri=*/"uri3", /*timestamp_ms=*/1000, + UsageReport::USAGE_TYPE3); + ICING_ASSERT_OK(doc_store->ReportUsage(usage_report_type3)); + + // Report usage with type 1 on document4 + usage_report_type1 = CreateUsageReport( + /*name_space=*/"namespace.2", /*uri=*/"uri1", /*timestamp_ms=*/1000, + UsageReport::USAGE_TYPE1); + ICING_ASSERT_OK(doc_store->ReportUsage(usage_report_type1)); + // Delete the first doc. ICING_ASSERT_OK(doc_store->Delete(document1.namespace_(), document1.uri())); @@ -3505,8 +3549,9 @@ TEST_F(DocumentStoreTest, DocumentStoreStorageInfo) { fake_clock_.SetSystemTimeMilliseconds(document2.creation_timestamp_ms() + document2.ttl_ms() + 1); + // Check high level info DocumentStorageInfoProto storage_info = doc_store->GetStorageInfo(); - EXPECT_THAT(storage_info.num_alive_documents(), Eq(1)); + EXPECT_THAT(storage_info.num_alive_documents(), Eq(2)); EXPECT_THAT(storage_info.num_deleted_documents(), Eq(1)); EXPECT_THAT(storage_info.num_expired_documents(), Eq(1)); EXPECT_THAT(storage_info.document_store_size(), Ge(0)); @@ -3518,7 +3563,37 @@ TEST_F(DocumentStoreTest, DocumentStoreStorageInfo) { EXPECT_THAT(storage_info.corpus_mapper_size(), Ge(0)); EXPECT_THAT(storage_info.corpus_score_cache_size(), Ge(0)); EXPECT_THAT(storage_info.namespace_id_mapper_size(), Ge(0)); - EXPECT_THAT(storage_info.num_namespaces(), Eq(1)); + EXPECT_THAT(storage_info.num_namespaces(), Eq(2)); + + // Check per-namespace info + EXPECT_THAT(storage_info.namespace_storage_info_size(), Eq(2)); + + NamespaceStorageInfoProto namespace_storage_info = + GetNamespaceStorageInfo(storage_info, "namespace.1"); + EXPECT_THAT(namespace_storage_info.num_alive_documents(), Eq(1)); + EXPECT_THAT(namespace_storage_info.num_expired_documents(), Eq(1)); + EXPECT_THAT(namespace_storage_info.num_alive_documents_usage_type1(), Eq(0)); + EXPECT_THAT(namespace_storage_info.num_alive_documents_usage_type2(), Eq(0)); + EXPECT_THAT(namespace_storage_info.num_alive_documents_usage_type3(), Eq(1)); + EXPECT_THAT(namespace_storage_info.num_expired_documents_usage_type1(), + Eq(0)); + EXPECT_THAT(namespace_storage_info.num_expired_documents_usage_type2(), + Eq(1)); + EXPECT_THAT(namespace_storage_info.num_expired_documents_usage_type3(), + Eq(0)); + + namespace_storage_info = GetNamespaceStorageInfo(storage_info, "namespace.2"); + EXPECT_THAT(namespace_storage_info.num_alive_documents(), Eq(1)); + EXPECT_THAT(namespace_storage_info.num_expired_documents(), Eq(0)); + EXPECT_THAT(namespace_storage_info.num_alive_documents_usage_type1(), Eq(1)); + EXPECT_THAT(namespace_storage_info.num_alive_documents_usage_type2(), Eq(0)); + EXPECT_THAT(namespace_storage_info.num_alive_documents_usage_type3(), Eq(0)); + EXPECT_THAT(namespace_storage_info.num_expired_documents_usage_type1(), + Eq(0)); + EXPECT_THAT(namespace_storage_info.num_expired_documents_usage_type2(), + Eq(0)); + EXPECT_THAT(namespace_storage_info.num_expired_documents_usage_type3(), + Eq(0)); } } // namespace diff --git a/icing/testing/common-matchers.h b/icing/testing/common-matchers.h index dcb8bf3..bbc8084 100644 --- a/icing/testing/common-matchers.h +++ b/icing/testing/common-matchers.h @@ -376,14 +376,22 @@ MATCHER_P2(ProtoStatusIs, status_code, error_matcher, "") { return ExplainMatchResult(error_matcher, arg.message(), result_listener); } -MATCHER_P(EqualsSearchResultIgnoreStats, expected, "") { +MATCHER_P(EqualsSearchResultIgnoreStatsAndScores, expected, "") { SearchResultProto actual_copy = arg; actual_copy.clear_query_stats(); actual_copy.clear_debug_info(); + for (SearchResultProto::ResultProto& result : + *actual_copy.mutable_results()) { + result.clear_score(); + } SearchResultProto expected_copy = expected; expected_copy.clear_query_stats(); expected_copy.clear_debug_info(); + for (SearchResultProto::ResultProto& result : + *expected_copy.mutable_results()) { + result.clear_score(); + } return ExplainMatchResult(testing::EqualsProto(expected_copy), actual_copy, result_listener); } |