diff options
author | Grace Zhao <gracezrx@google.com> | 2023-07-17 10:31:06 -0700 |
---|---|---|
committer | Grace Zhao <gracezrx@google.com> | 2023-07-17 10:31:06 -0700 |
commit | b83a3188c44e60dc20032b92fc08e1caa3427731 (patch) | |
tree | b4fdb2da75b0a1e3c692a4e28ef3f7620733a804 | |
parent | c6203a6b1b1eba6fe70bb0c7ece3cf14a47518c4 (diff) | |
parent | f8a8e6c2e2b55900e082d2657f3691ee6d9b2d27 (diff) | |
download | icing-b83a3188c44e60dc20032b92fc08e1caa3427731.tar.gz |
Merge remote-tracking branch 'aosp/upstream-master' into androidx-main
* aosp/upstream-master:
Update Icing from upstream.
Descriptions:
========================================================================
[Icing][adhoc] Add backup schema producer unit tests for properties using indexable nested properties list
========================================================================
Fix SetSchema bug where adding an indexable nested document property does not trigger index-rebuild
========================================================================
Add join test for SetSchemaNewIndexedDocumentPropertyTriggersIndexRestorationAndReturnsOk
========================================================================
Bug: 291019114
NO_IFTTT="Path is only valid in G3."
Change-Id: Ib0f38921783a9ea2a964523439f3547250853cb0
-rw-r--r-- | icing/icing-search-engine_schema_test.cc | 178 | ||||
-rw-r--r-- | icing/schema/backup-schema-producer_test.cc | 144 | ||||
-rw-r--r-- | icing/schema/schema-store_test.cc | 131 | ||||
-rw-r--r-- | icing/schema/schema-util.cc | 47 | ||||
-rw-r--r-- | icing/schema/schema-util.h | 3 | ||||
-rw-r--r-- | icing/schema/schema-util_test.cc | 432 | ||||
-rw-r--r-- | icing/testing/common-matchers.h | 31 | ||||
-rw-r--r-- | synced_AOSP_CL_number.txt | 2 |
8 files changed, 936 insertions, 32 deletions
diff --git a/icing/icing-search-engine_schema_test.cc b/icing/icing-search-engine_schema_test.cc index af9d0e2..5076d50 100644 --- a/icing/icing-search-engine_schema_test.cc +++ b/icing/icing-search-engine_schema_test.cc @@ -885,6 +885,184 @@ TEST_F(IcingSearchEngineSchemaTest, expected_search_result_proto)); } +TEST_F( + IcingSearchEngineSchemaTest, + SetSchemaNewIndexedDocumentPropertyTriggersIndexRestorationAndReturnsOk) { + IcingSearchEngine icing(GetDefaultIcingOptions(), GetTestJniCache()); + ASSERT_THAT(icing.Initialize().status(), ProtoIsOk()); + + // Create a schema with a nested document type: + // + // Section id assignment for 'Person': + // - "age": integer type, indexed. Section id = 0 + // - "name": string type, indexed. Section id = 1. + // - "worksFor.name": string type, (nested) indexed. Section id = 2. + SchemaProto schema_one = + SchemaBuilder() + .AddType(SchemaTypeConfigBuilder() + .SetType("Person") + .AddProperty(PropertyConfigBuilder() + .SetName("name") + .SetDataTypeString(TERM_MATCH_PREFIX, + TOKENIZER_PLAIN) + .SetCardinality(CARDINALITY_REQUIRED)) + .AddProperty(PropertyConfigBuilder() + .SetName("age") + .SetDataTypeInt64(NUMERIC_MATCH_RANGE) + .SetCardinality(CARDINALITY_OPTIONAL)) + .AddProperty(PropertyConfigBuilder() + .SetName("worksFor") + .SetDataTypeDocument( + "Organization", + /*index_nested_properties=*/true) + .SetCardinality(CARDINALITY_OPTIONAL))) + .AddType(SchemaTypeConfigBuilder() + .SetType("Organization") + .AddProperty(PropertyConfigBuilder() + .SetName("name") + .SetDataTypeString(TERM_MATCH_PREFIX, + TOKENIZER_PLAIN) + .SetCardinality(CARDINALITY_REQUIRED))) + .Build(); + + SetSchemaResultProto set_schema_result = icing.SetSchema(schema_one); + // Ignore latency numbers. They're covered elsewhere. + set_schema_result.clear_latency_ms(); + SetSchemaResultProto expected_set_schema_result; + expected_set_schema_result.mutable_status()->set_code(StatusProto::OK); + expected_set_schema_result.mutable_new_schema_types()->Add("Organization"); + expected_set_schema_result.mutable_new_schema_types()->Add("Person"); + EXPECT_THAT(set_schema_result, EqualsProto(expected_set_schema_result)); + + DocumentProto personDocument = + DocumentBuilder() + .SetKey("namespace", "person/2") + .SetSchema("Person") + .SetCreationTimestampMs(1000) + .AddStringProperty("name", "John") + .AddInt64Property("age", 20) + .AddDocumentProperty("worksFor", + DocumentBuilder() + .SetKey("namespace", "org/1") + .SetSchema("Organization") + .AddStringProperty("name", "Google") + .Build()) + .Build(); + EXPECT_THAT(icing.Put(personDocument).status(), ProtoIsOk()); + + SearchResultProto expected_search_result_proto; + expected_search_result_proto.mutable_status()->set_code(StatusProto::OK); + *expected_search_result_proto.mutable_results()->Add()->mutable_document() = + personDocument; + + SearchResultProto empty_result; + empty_result.mutable_status()->set_code(StatusProto::OK); + + // Verify term search + SearchSpecProto search_spec1; + search_spec1.set_query("worksFor.name:Google"); + search_spec1.set_term_match_type(TermMatchType::EXACT_ONLY); + + SearchResultProto actual_results = + icing.Search(search_spec1, GetDefaultScoringSpec(), + ResultSpecProto::default_instance()); + EXPECT_THAT(actual_results, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); + + // Verify numeric (integer) search + SearchSpecProto search_spec2; + search_spec2.set_query("age == 20"); + search_spec2.set_search_type( + SearchSpecProto::SearchType::EXPERIMENTAL_ICING_ADVANCED_QUERY); + search_spec2.add_enabled_features(std::string(kNumericSearchFeature)); + + actual_results = icing.Search(search_spec2, GetDefaultScoringSpec(), + ResultSpecProto::default_instance()); + EXPECT_THAT(actual_results, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); + + // Change the schema to add another nested document property to 'Person' + // + // New section id assignment for 'Person': + // - "age": integer type, indexed. Section id = 0 + // - "almaMater.name", string type, indexed. Section id = 1 + // - "name": string type, indexed. Section id = 2 + // - "worksFor.name": string type, (nested) indexed. Section id = 3 + SchemaProto schema_two = + SchemaBuilder() + .AddType(SchemaTypeConfigBuilder() + .SetType("Person") + .AddProperty(PropertyConfigBuilder() + .SetName("name") + .SetDataTypeString(TERM_MATCH_PREFIX, + TOKENIZER_PLAIN) + .SetCardinality(CARDINALITY_REQUIRED)) + .AddProperty(PropertyConfigBuilder() + .SetName("age") + .SetDataTypeInt64(NUMERIC_MATCH_RANGE) + .SetCardinality(CARDINALITY_OPTIONAL)) + .AddProperty(PropertyConfigBuilder() + .SetName("worksFor") + .SetDataTypeDocument( + "Organization", + /*index_nested_properties=*/true) + .SetCardinality(CARDINALITY_OPTIONAL)) + .AddProperty(PropertyConfigBuilder() + .SetName("almaMater") + .SetDataTypeDocument( + "Organization", + /*index_nested_properties=*/true) + .SetCardinality(CARDINALITY_OPTIONAL))) + .AddType(SchemaTypeConfigBuilder() + .SetType("Organization") + .AddProperty(PropertyConfigBuilder() + .SetName("name") + .SetDataTypeString(TERM_MATCH_PREFIX, + TOKENIZER_PLAIN) + .SetCardinality(CARDINALITY_REQUIRED))) + .Build(); + + // This schema change is compatible since the added 'almaMater' property has + // CARDINALITY_OPTIONAL. + // + // Index restoration should be triggered here because new schema requires more + // properties to be indexed. Also new section ids will be reassigned and index + // restoration should use new section ids to rebuild. + set_schema_result = icing.SetSchema(schema_two); + // Ignore latency numbers. They're covered elsewhere. + set_schema_result.clear_latency_ms(); + expected_set_schema_result = SetSchemaResultProto(); + expected_set_schema_result.mutable_status()->set_code(StatusProto::OK); + expected_set_schema_result.mutable_index_incompatible_changed_schema_types() + ->Add("Person"); + expected_set_schema_result.mutable_join_incompatible_changed_schema_types() + ->Add("Person"); + EXPECT_THAT(set_schema_result, EqualsProto(expected_set_schema_result)); + + // Verify term search: + // Searching for "worksFor.name:Google" should still match document + actual_results = icing.Search(search_spec1, GetDefaultScoringSpec(), + ResultSpecProto::default_instance()); + EXPECT_THAT(actual_results, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); + + // In new_schema the 'name' property is now indexed at section id 2. If + // searching for "name:Google" matched the document, this means that index + // rebuild was not triggered and Icing is still searching the old index, where + // 'worksFor.name' was indexed at section id 2. + search_spec1.set_query("name:Google"); + actual_results = icing.Search(search_spec1, GetDefaultScoringSpec(), + ResultSpecProto::default_instance()); + EXPECT_THAT(actual_results, + EqualsSearchResultIgnoreStatsAndScores(empty_result)); + + // Verify numeric (integer) search: should still match document + actual_results = icing.Search(search_spec2, GetDefaultScoringSpec(), + ResultSpecProto::default_instance()); + EXPECT_THAT(actual_results, EqualsSearchResultIgnoreStatsAndScores( + expected_search_result_proto)); +} + TEST_F(IcingSearchEngineSchemaTest, SetSchemaChangeNestedPropertiesTriggersIndexRestorationAndReturnsOk) { IcingSearchEngine icing(GetDefaultIcingOptions(), GetTestJniCache()); diff --git a/icing/schema/backup-schema-producer_test.cc b/icing/schema/backup-schema-producer_test.cc index b0e793c..dbd033f 100644 --- a/icing/schema/backup-schema-producer_test.cc +++ b/icing/schema/backup-schema-producer_test.cc @@ -36,6 +36,8 @@ namespace lib { namespace { using ::testing::Eq; +using ::testing::Pointee; +using ::testing::SizeIs; class BackupSchemaProducerTest : public ::testing::Test { protected: @@ -442,6 +444,96 @@ TEST_F(BackupSchemaProducerTest, MakeExtraDocumentIndexedPropertiesUnindexed) { EXPECT_THAT(backup, portable_equals_proto::EqualsProto(expected_backup)); } +TEST_F( + BackupSchemaProducerTest, + MakeExtraDocumentIndexedPropertiesWithIndexableNestedPropertiesListUnindexed) { + PropertyConfigBuilder indexed_string_property_builder = + PropertyConfigBuilder() + .SetCardinality(CARDINALITY_OPTIONAL) + .SetDataTypeString(TERM_MATCH_PREFIX, TOKENIZER_PLAIN); + PropertyConfigBuilder indexed_int_property_builder = + PropertyConfigBuilder() + .SetCardinality(CARDINALITY_OPTIONAL) + .SetDataTypeInt64(NUMERIC_MATCH_RANGE); + SchemaTypeConfigProto typeB = + SchemaTypeConfigBuilder() + .SetType("TypeB") + .AddProperty(indexed_string_property_builder.SetName("prop0")) + .AddProperty(indexed_int_property_builder.SetName("prop1")) + .AddProperty(indexed_string_property_builder.SetName("prop2")) + .AddProperty(indexed_int_property_builder.SetName("prop3")) + .AddProperty(indexed_string_property_builder.SetName("prop4")) + .AddProperty(indexed_int_property_builder.SetName("prop5")) + .AddProperty(indexed_string_property_builder.SetName("prop6")) + .AddProperty(indexed_int_property_builder.SetName("prop7")) + .AddProperty(indexed_string_property_builder.SetName("prop8")) + .AddProperty(indexed_int_property_builder.SetName("prop9")) + .Build(); + + // Create indexed document property by using indexable nested properties list. + PropertyConfigBuilder indexed_document_property_with_list_builder = + PropertyConfigBuilder() + .SetCardinality(CARDINALITY_OPTIONAL) + .SetDataTypeDocument( + "TypeB", /*indexable_nested_properties_list=*/{ + "prop0", "prop1", "prop2", "prop3", "prop4", "prop5", + "unknown1", "unknown2", "unknown3"}); + SchemaTypeConfigProto typeA = + SchemaTypeConfigBuilder() + .SetType("TypeA") + .AddProperty( + indexed_document_property_with_list_builder.SetName("propA")) + .AddProperty( + indexed_document_property_with_list_builder.SetName("propB")) + .Build(); + + SchemaProto schema = SchemaBuilder().AddType(typeA).AddType(typeB).Build(); + + SchemaUtil::TypeConfigMap type_config_map; + SchemaUtil::BuildTypeConfigMap(schema, &type_config_map); + ICING_ASSERT_OK_AND_ASSIGN( + std::unique_ptr<DynamicTrieKeyMapper<SchemaTypeId>> type_id_mapper, + DynamicTrieKeyMapper<SchemaTypeId>::Create(filesystem_, schema_store_dir_, + /*maximum_size_bytes=*/10000)); + ASSERT_THAT(type_id_mapper->Put("TypeA", 0), IsOk()); + ASSERT_THAT(type_id_mapper->Put("TypeB", 1), IsOk()); + + ICING_ASSERT_OK_AND_ASSIGN( + std::unique_ptr<SchemaTypeManager> schema_type_manager, + SchemaTypeManager::Create(type_config_map, type_id_mapper.get())); + ASSERT_THAT(schema_type_manager->section_manager().GetMetadataList("TypeA"), + IsOkAndHolds(Pointee(SizeIs(18)))); + + ICING_ASSERT_OK_AND_ASSIGN( + BackupSchemaProducer backup_producer, + BackupSchemaProducer::Create(schema, + schema_type_manager->section_manager())); + EXPECT_THAT(backup_producer.is_backup_necessary(), Eq(true)); + SchemaProto backup = std::move(backup_producer).Produce(); + + PropertyConfigProto unindexed_document_property = + PropertyConfigBuilder() + .SetCardinality(CARDINALITY_OPTIONAL) + .SetDataType(TYPE_DOCUMENT) + .Build(); + unindexed_document_property.set_schema_type("TypeB"); + PropertyConfigBuilder unindexed_document_property_builder( + unindexed_document_property); + + // "propA" and "propB" both have 9 sections respectively, so we have to drop + // "propB" indexing config to make total # of sections <= 16. + SchemaTypeConfigProto expected_typeA = + SchemaTypeConfigBuilder() + .SetType("TypeA") + .AddProperty( + indexed_document_property_with_list_builder.SetName("propA")) + .AddProperty(unindexed_document_property_builder.SetName("propB")) + .Build(); + SchemaProto expected_backup = + SchemaBuilder().AddType(expected_typeA).AddType(typeB).Build(); + EXPECT_THAT(backup, portable_equals_proto::EqualsProto(expected_backup)); +} + TEST_F(BackupSchemaProducerTest, MakeRfcPropertiesUnindexedFirst) { PropertyConfigBuilder indexed_string_property_builder = PropertyConfigBuilder() @@ -539,31 +631,33 @@ TEST_F(BackupSchemaProducerTest, MakeExtraPropertiesUnindexedMultipleTypes) { .AddProperty(indexed_string_property_builder.SetName("prop2")) .AddProperty(indexed_int_property_builder.SetName("prop3")) .AddProperty(indexed_string_property_builder.SetName("prop4")) - .AddProperty(indexed_int_property_builder.SetName("prop5")) - .AddProperty(indexed_string_property_builder.SetName("prop6")) - .AddProperty(indexed_int_property_builder.SetName("prop7")) - .AddProperty(indexed_string_property_builder.SetName("prop8")) - .AddProperty(indexed_int_property_builder.SetName("prop9")) .Build(); PropertyConfigBuilder indexed_document_property_builder = PropertyConfigBuilder() .SetCardinality(CARDINALITY_OPTIONAL) .SetDataTypeDocument("TypeB", /*index_nested_properties=*/true); + PropertyConfigBuilder indexed_document_property_with_list_builder = + PropertyConfigBuilder() + .SetCardinality(CARDINALITY_OPTIONAL) + .SetDataTypeDocument( + "TypeB", /*indexable_nested_properties_list=*/{ + "prop0", "prop4", "unknown1", "unknown2", "unknown3"}); SchemaTypeConfigProto typeA = SchemaTypeConfigBuilder() .SetType("TypeA") .AddProperty(indexed_string_property_builder.SetName("propA")) - .AddProperty(indexed_int_property_builder.SetName("propB")) + .AddProperty( + indexed_document_property_with_list_builder.SetName("propB")) .AddProperty(indexed_string_property_builder.SetName("propC")) - .AddProperty(indexed_int_property_builder.SetName("propD")) + .AddProperty(indexed_document_property_builder.SetName("propD")) .AddProperty(indexed_string_property_builder.SetName("propE")) .AddProperty(indexed_int_property_builder.SetName("propF")) - .AddProperty(indexed_string_property_builder.SetName("propG")) - .AddProperty(indexed_int_property_builder.SetName("propH")) - .AddProperty(indexed_document_property_builder.SetName("propI")) - .AddProperty(indexed_string_property_builder.SetName("propJ")) - .AddProperty(indexed_int_property_builder.SetName("propK")) + .AddProperty(indexed_document_property_builder.SetName("propG")) + .AddProperty(indexed_string_property_builder.SetName("propH")) + .AddProperty(indexed_int_property_builder.SetName("propI")) + .AddProperty( + indexed_document_property_with_list_builder.SetName("propJ")) .Build(); SchemaProto schema = SchemaBuilder().AddType(typeA).AddType(typeB).Build(); @@ -580,6 +674,8 @@ TEST_F(BackupSchemaProducerTest, MakeExtraPropertiesUnindexedMultipleTypes) { ICING_ASSERT_OK_AND_ASSIGN( std::unique_ptr<SchemaTypeManager> schema_type_manager, SchemaTypeManager::Create(type_config_map, type_id_mapper.get())); + ASSERT_THAT(schema_type_manager->section_manager().GetMetadataList("TypeA"), + IsOkAndHolds(Pointee(SizeIs(26)))); ICING_ASSERT_OK_AND_ASSIGN( BackupSchemaProducer backup_producer, @@ -605,20 +701,30 @@ TEST_F(BackupSchemaProducerTest, MakeExtraPropertiesUnindexedMultipleTypes) { PropertyConfigBuilder unindexed_document_property_builder( unindexed_document_property); + // On version 0 (Android T): + // - Only "propA", "propC", "propD.prop0", "propD.prop1", "propD.prop2", + // "propD.prop3", "propD.prop4", "propE", "propF" will be assigned sections. + // - Unlike version 2, "propB.prop0", "propB.prop4", "propB.unknown1", + // "propB.unknown2", "propB.unknown3" will be ignored because version 0 + // doesn't recognize indexable nested properties list. + // - So there will be only 9 sections on version 0. We still have potential to + // avoid dropping "propG", "propH", "propI" indexing configs on version 0 + // (in this case it will be 16 sections), but it is ok to make it simple as + // long as total # of sections <= 16. SchemaTypeConfigProto expected_typeA = SchemaTypeConfigBuilder() .SetType("TypeA") .AddProperty(indexed_string_property_builder.SetName("propA")) - .AddProperty(indexed_int_property_builder.SetName("propB")) + .AddProperty( + indexed_document_property_with_list_builder.SetName("propB")) .AddProperty(indexed_string_property_builder.SetName("propC")) - .AddProperty(indexed_int_property_builder.SetName("propD")) + .AddProperty(indexed_document_property_builder.SetName("propD")) .AddProperty(indexed_string_property_builder.SetName("propE")) .AddProperty(indexed_int_property_builder.SetName("propF")) - .AddProperty(indexed_string_property_builder.SetName("propG")) - .AddProperty(indexed_int_property_builder.SetName("propH")) - .AddProperty(unindexed_document_property_builder.SetName("propI")) - .AddProperty(unindexed_string_property_builder.SetName("propJ")) - .AddProperty(unindexed_int_property_builder.SetName("propK")) + .AddProperty(unindexed_document_property_builder.SetName("propG")) + .AddProperty(unindexed_string_property_builder.SetName("propH")) + .AddProperty(unindexed_int_property_builder.SetName("propI")) + .AddProperty(unindexed_document_property_builder.SetName("propJ")) .Build(); SchemaProto expected_backup = SchemaBuilder().AddType(expected_typeA).AddType(typeB).Build(); diff --git a/icing/schema/schema-store_test.cc b/icing/schema/schema-store_test.cc index 8fc51e7..8cc7008 100644 --- a/icing/schema/schema-store_test.cc +++ b/icing/schema/schema-store_test.cc @@ -1084,6 +1084,137 @@ TEST_F(SchemaStoreTest, SetSchemaWithCompatibleNestedTypesOk) { EXPECT_THAT(*actual_schema, EqualsProto(new_schema)); } +TEST_F(SchemaStoreTest, SetSchemaWithAddedIndexableNestedTypeOk) { + ICING_ASSERT_OK_AND_ASSIGN( + std::unique_ptr<SchemaStore> schema_store, + SchemaStore::Create(&filesystem_, schema_store_dir_, &fake_clock_)); + + // 1. Create a ContactPoint type with a optional property, and a type that + // references the ContactPoint type. + SchemaTypeConfigBuilder contact_point = + SchemaTypeConfigBuilder() + .SetType("ContactPoint") + .AddProperty( + PropertyConfigBuilder() + .SetName("label") + .SetDataTypeString(TERM_MATCH_PREFIX, TOKENIZER_PLAIN) + .SetCardinality(CARDINALITY_REPEATED)); + SchemaTypeConfigBuilder person = + SchemaTypeConfigBuilder().SetType("Person").AddProperty( + PropertyConfigBuilder() + .SetName("contactPoints") + .SetDataTypeDocument("ContactPoint", + /*index_nested_properties=*/true) + .SetCardinality(CARDINALITY_REPEATED)); + SchemaProto old_schema = + SchemaBuilder().AddType(contact_point).AddType(person).Build(); + ICING_EXPECT_OK(schema_store->SetSchema( + old_schema, /*ignore_errors_and_delete_documents=*/false, + /*allow_circular_schema_definitions=*/false)); + + // 2. Add another nested document property to "Person" that has type + // "ContactPoint" + SchemaTypeConfigBuilder new_person = + SchemaTypeConfigBuilder() + .SetType("Person") + .AddProperty( + PropertyConfigBuilder() + .SetName("contactPoints") + .SetDataTypeDocument("ContactPoint", + /*index_nested_properties=*/true) + .SetCardinality(CARDINALITY_REPEATED)) + .AddProperty( + PropertyConfigBuilder() + .SetName("anotherContactPoint") + .SetDataTypeDocument("ContactPoint", + /*index_nested_properties=*/true) + .SetCardinality(CARDINALITY_REPEATED)); + SchemaProto new_schema = + SchemaBuilder().AddType(contact_point).AddType(new_person).Build(); + + // 3. Set to new schema. "Person" should be index-incompatible since we need + // to index an additional property: 'anotherContactPoint.label'. + // - "Person" is also considered join-incompatible since the added nested + // document property could also contain a joinable property. + SchemaStore::SetSchemaResult expected_result; + expected_result.success = true; + expected_result.schema_types_index_incompatible_by_name.insert("Person"); + expected_result.schema_types_join_incompatible_by_name.insert("Person"); + + EXPECT_THAT(schema_store->SetSchema( + new_schema, /*ignore_errors_and_delete_documents=*/false, + /*allow_circular_schema_definitions=*/false), + IsOkAndHolds(EqualsSetSchemaResult(expected_result))); + ICING_ASSERT_OK_AND_ASSIGN(const SchemaProto* actual_schema, + schema_store->GetSchema()); + EXPECT_THAT(*actual_schema, EqualsProto(new_schema)); +} + +TEST_F(SchemaStoreTest, SetSchemaWithAddedJoinableNestedTypeOk) { + ICING_ASSERT_OK_AND_ASSIGN( + std::unique_ptr<SchemaStore> schema_store, + SchemaStore::Create(&filesystem_, schema_store_dir_, &fake_clock_)); + + // 1. Create a ContactPoint type with a optional property, and a type that + // references the ContactPoint type. + SchemaTypeConfigBuilder contact_point = + SchemaTypeConfigBuilder() + .SetType("ContactPoint") + .AddProperty( + PropertyConfigBuilder() + .SetName("label") + .SetDataTypeString(TERM_MATCH_PREFIX, TOKENIZER_PLAIN) + .SetJoinable(JOINABLE_VALUE_TYPE_QUALIFIED_ID, + /*propagate_delete=*/false) + .SetCardinality(CARDINALITY_REQUIRED)); + SchemaTypeConfigBuilder person = + SchemaTypeConfigBuilder().SetType("Person").AddProperty( + PropertyConfigBuilder() + .SetName("contactPoints") + .SetDataTypeDocument("ContactPoint", + /*index_nested_properties=*/true) + .SetCardinality(CARDINALITY_OPTIONAL)); + SchemaProto old_schema = + SchemaBuilder().AddType(contact_point).AddType(person).Build(); + ICING_EXPECT_OK(schema_store->SetSchema( + old_schema, /*ignore_errors_and_delete_documents=*/false, + /*allow_circular_schema_definitions=*/false)); + + // 2. Add another nested document property to "Person" that has type + // "ContactPoint", but make it non-indexable + SchemaTypeConfigBuilder new_person = + SchemaTypeConfigBuilder() + .SetType("Person") + .AddProperty( + PropertyConfigBuilder() + .SetName("contactPoints") + .SetDataTypeDocument("ContactPoint", + /*index_nested_properties=*/true) + .SetCardinality(CARDINALITY_OPTIONAL)) + .AddProperty( + PropertyConfigBuilder() + .SetName("anotherContactPoint") + .SetDataTypeDocument("ContactPoint", + /*index_nested_properties=*/false) + .SetCardinality(CARDINALITY_OPTIONAL)); + SchemaProto new_schema = + SchemaBuilder().AddType(contact_point).AddType(new_person).Build(); + + // 3. Set to new schema. "Person" should be join-incompatible but + // index-compatible. + SchemaStore::SetSchemaResult expected_result; + expected_result.success = true; + expected_result.schema_types_join_incompatible_by_name.insert("Person"); + + EXPECT_THAT(schema_store->SetSchema( + new_schema, /*ignore_errors_and_delete_documents=*/false, + /*allow_circular_schema_definitions=*/false), + IsOkAndHolds(EqualsSetSchemaResult(expected_result))); + ICING_ASSERT_OK_AND_ASSIGN(const SchemaProto* actual_schema, + schema_store->GetSchema()); + EXPECT_THAT(*actual_schema, EqualsProto(new_schema)); +} + TEST_F(SchemaStoreTest, GetSchemaTypeId) { ICING_ASSERT_OK_AND_ASSIGN( std::unique_ptr<SchemaStore> schema_store, diff --git a/icing/schema/schema-util.cc b/icing/schema/schema-util.cc index 6d63b10..4390b6a 100644 --- a/icing/schema/schema-util.cc +++ b/icing/schema/schema-util.cc @@ -791,7 +791,8 @@ libtextclassifier3::Status SchemaUtil::ValidateDocumentIndexingConfig( return absl_ports::InvalidArgumentError(absl_ports::StrCat( "DocumentIndexingConfig.index_nested_properties is required to be " "false when providing a non-empty indexable_nested_properties_list " - "for property '", schema_type, ".", property_name, "'")); + "for property '", + schema_type, ".", property_name, "'")); } return libtextclassifier3::Status::OK; } @@ -807,11 +808,19 @@ libtextclassifier3::Status SchemaUtil::ValidateDocumentIndexingConfig( case PropertyConfigProto::DataType::INT64: return property_config.integer_indexing_config().numeric_match_type() != IntegerIndexingConfig::NumericMatchType::UNKNOWN; + case PropertyConfigProto::DataType::DOCUMENT: + // A document property is considered indexed if it has + // index_nested_properties=true, or a non-empty + // indexable_nested_properties_list. + return property_config.document_indexing_config() + .index_nested_properties() || + !property_config.document_indexing_config() + .indexable_nested_properties_list() + .empty(); case PropertyConfigProto::DataType::UNKNOWN: case PropertyConfigProto::DataType::DOUBLE: case PropertyConfigProto::DataType::BOOLEAN: case PropertyConfigProto::DataType::BYTES: - case PropertyConfigProto::DataType::DOCUMENT: return false; } } @@ -944,6 +953,13 @@ SchemaUtil::ParsedPropertyConfigs SchemaUtil::ParsePropertyConfigs( JoinableConfig::ValueType::NONE) { ++parsed_property_configs.num_joinable_properties; } + + // Also keep track of how many nested document properties there are. Adding + // new nested document properties will result in join-index rebuild. + if (property_config.data_type() == + PropertyConfigProto::DataType::DOCUMENT) { + ++parsed_property_configs.num_nested_document_properties; + } } return parsed_property_configs; @@ -982,6 +998,7 @@ const SchemaUtil::SchemaDelta SchemaUtil::ComputeCompatibilityDelta( int32_t old_required_properties = 0; int32_t old_indexed_properties = 0; int32_t old_joinable_properties = 0; + int32_t old_nested_document_properties = 0; // If there is a different number of properties, then there must have been a // change. @@ -1011,6 +1028,14 @@ const SchemaUtil::SchemaDelta SchemaUtil::ComputeCompatibilityDelta( ++old_joinable_properties; } + // A nested-document property is a property of DataType::DOCUMENT. + bool is_nested_document_property = + old_property_config.data_type() == + PropertyConfigProto::DataType::DOCUMENT; + if (is_nested_document_property) { + ++old_nested_document_properties; + } + auto new_property_name_and_config = new_parsed_property_configs.property_config_map.find( old_property_config.property_name()); @@ -1024,7 +1049,8 @@ const SchemaUtil::SchemaDelta SchemaUtil::ComputeCompatibilityDelta( "' was not defined in new schema"); is_incompatible = true; is_index_incompatible |= is_indexed_property; - is_join_incompatible |= is_joinable_property; + is_join_incompatible |= + is_joinable_property || is_nested_document_property; continue; } @@ -1076,8 +1102,9 @@ const SchemaUtil::SchemaDelta SchemaUtil::ComputeCompatibilityDelta( is_incompatible = true; } - // If we've gained any new indexed properties, then the section ids may - // change. Since the section ids are stored in the index, we'll need to + // If we've gained any new indexed properties (this includes gaining new + // indexed nested document properties), then the section ids may change. + // Since the section ids are stored in the index, we'll need to // reindex everything. if (new_parsed_property_configs.num_indexed_properties > old_indexed_properties) { @@ -1089,9 +1116,15 @@ const SchemaUtil::SchemaDelta SchemaUtil::ComputeCompatibilityDelta( // If we've gained any new joinable properties, then the joinable property // ids may change. Since the joinable property ids are stored in the cache, - // we'll need to reconstruct joinable cache. + // we'll need to reconstruct join index. + // If we've gained any new nested document properties, we also rebuild the + // join index. This is because we index all nested joinable properties, so + // adding a nested document property will most probably result in having + // more joinable properties. if (new_parsed_property_configs.num_joinable_properties > - old_joinable_properties) { + old_joinable_properties || + new_parsed_property_configs.num_nested_document_properties > + old_nested_document_properties) { ICING_VLOG(1) << "Set of joinable properties in schema type '" << old_type_config.schema_type() << "' has changed, required reconstructing joinable cache."; diff --git a/icing/schema/schema-util.h b/icing/schema/schema-util.h index 0adc0ae..6d0ff73 100644 --- a/icing/schema/schema-util.h +++ b/icing/schema/schema-util.h @@ -121,6 +121,9 @@ class SchemaUtil { // Total number of properties that have joinable config int32_t num_joinable_properties = 0; + + // Total number of properties that have DataType::DOCUMENT + int32_t num_nested_document_properties = 0; }; // This function validates: diff --git a/icing/schema/schema-util_test.cc b/icing/schema/schema-util_test.cc index 6f9e420..564bbc0 100644 --- a/icing/schema/schema-util_test.cc +++ b/icing/schema/schema-util_test.cc @@ -2792,6 +2792,438 @@ TEST_P(SchemaUtilTest, IsEmpty()); } +TEST_P(SchemaUtilTest, + AddingNewIndexedDocumentPropertyMakesIndexAndJoinIncompatible) { + SchemaTypeConfigProto nested_schema = + SchemaTypeConfigBuilder() + .SetType(kEmailType) + .AddProperty(PropertyConfigBuilder() + .SetName("subject") + .SetDataTypeString(TERM_MATCH_EXACT, TOKENIZER_PLAIN) + .SetCardinality(CARDINALITY_OPTIONAL)) + .Build(); + + // Configure old schema + SchemaProto old_schema = + SchemaBuilder() + .AddType(nested_schema) + .AddType(SchemaTypeConfigBuilder() + .SetType(kPersonType) + .AddProperty(PropertyConfigBuilder() + .SetName("Property") + .SetDataTypeInt64(NUMERIC_MATCH_RANGE) + .SetCardinality(CARDINALITY_OPTIONAL))) + .Build(); + + // Configure new schema + SchemaProto new_schema = + SchemaBuilder() + .AddType(nested_schema) + .AddType(SchemaTypeConfigBuilder() + .SetType(kPersonType) + .AddProperty(PropertyConfigBuilder() + .SetName("Property") + .SetDataTypeInt64(NUMERIC_MATCH_RANGE) + .SetCardinality(CARDINALITY_OPTIONAL)) + .AddProperty( + PropertyConfigBuilder() + .SetName("NewEmailProperty") + .SetDataTypeDocument( + kEmailType, /*index_nested_properties=*/true) + .SetCardinality(CARDINALITY_OPTIONAL))) + .Build(); + + SchemaUtil::SchemaDelta schema_delta; + schema_delta.schema_types_index_incompatible.insert(kPersonType); + schema_delta.schema_types_join_incompatible.insert(kPersonType); + + SchemaUtil::DependentMap dependents_map = {{kEmailType, {{kPersonType, {}}}}}; + SchemaUtil::SchemaDelta result_schema_delta = + SchemaUtil::ComputeCompatibilityDelta(old_schema, new_schema, + dependents_map); + EXPECT_THAT(result_schema_delta, Eq(schema_delta)); +} + +TEST_P( + SchemaUtilTest, + AddingNewIndexedDocumentPropertyWithIndexableListMakesIndexAndJoinIncompatible) { + SchemaTypeConfigProto nested_schema = + SchemaTypeConfigBuilder() + .SetType(kEmailType) + .AddProperty(PropertyConfigBuilder() + .SetName("subject") + .SetDataTypeString(TERM_MATCH_EXACT, TOKENIZER_PLAIN) + .SetCardinality(CARDINALITY_OPTIONAL)) + .Build(); + + // Configure old schema + SchemaProto old_schema = + SchemaBuilder() + .AddType(nested_schema) + .AddType(SchemaTypeConfigBuilder() + .SetType(kPersonType) + .AddProperty(PropertyConfigBuilder() + .SetName("Property") + .SetDataTypeInt64(NUMERIC_MATCH_RANGE) + .SetCardinality(CARDINALITY_OPTIONAL))) + .Build(); + + // Configure new schema. The added nested document property is indexed, so + // this is both index and join incompatible + SchemaProto new_schema = + SchemaBuilder() + .AddType(nested_schema) + .AddType( + SchemaTypeConfigBuilder() + .SetType(kPersonType) + .AddProperty(PropertyConfigBuilder() + .SetName("Property") + .SetDataTypeInt64(NUMERIC_MATCH_RANGE) + .SetCardinality(CARDINALITY_OPTIONAL)) + .AddProperty( + PropertyConfigBuilder() + .SetName("NewEmailProperty") + .SetDataTypeDocument( + kEmailType, + /*indexable_nested_properties_list=*/ + std::initializer_list<std::string>{"subject"}) + .SetCardinality(CARDINALITY_OPTIONAL))) + .Build(); + + SchemaUtil::SchemaDelta schema_delta; + schema_delta.schema_types_index_incompatible.insert(kPersonType); + schema_delta.schema_types_join_incompatible.insert(kPersonType); + + SchemaUtil::DependentMap dependents_map = {{kEmailType, {{kPersonType, {}}}}}; + SchemaUtil::SchemaDelta result_schema_delta = + SchemaUtil::ComputeCompatibilityDelta(old_schema, new_schema, + dependents_map); + EXPECT_THAT(result_schema_delta, Eq(schema_delta)); +} + +TEST_P(SchemaUtilTest, + AddingNewNonIndexedDocumentPropertyMakesJoinIncompatible) { + SchemaTypeConfigProto nested_schema = + SchemaTypeConfigBuilder() + .SetType(kEmailType) + .AddProperty(PropertyConfigBuilder() + .SetName("subject") + .SetDataTypeString(TERM_MATCH_EXACT, TOKENIZER_PLAIN) + .SetCardinality(CARDINALITY_OPTIONAL)) + .Build(); + + // Configure old schema + SchemaProto old_schema = + SchemaBuilder() + .AddType(nested_schema) + .AddType(SchemaTypeConfigBuilder() + .SetType(kPersonType) + .AddProperty(PropertyConfigBuilder() + .SetName("Property") + .SetDataTypeInt64(NUMERIC_MATCH_RANGE) + .SetCardinality(CARDINALITY_OPTIONAL))) + .Build(); + + // Configure new schema. The added nested document property is not indexed, so + // this is index compatible, but join incompatible + SchemaProto new_schema = + SchemaBuilder() + .AddType(nested_schema) + .AddType(SchemaTypeConfigBuilder() + .SetType(kPersonType) + .AddProperty(PropertyConfigBuilder() + .SetName("Property") + .SetDataTypeInt64(NUMERIC_MATCH_RANGE) + .SetCardinality(CARDINALITY_OPTIONAL)) + .AddProperty(PropertyConfigBuilder() + .SetName("NewEmailProperty") + .SetDataTypeDocument( + kEmailType, + /*index_nested_properties=*/false) + .SetCardinality(CARDINALITY_OPTIONAL))) + .Build(); + + SchemaUtil::SchemaDelta schema_delta; + schema_delta.schema_types_join_incompatible.insert(kPersonType); + + SchemaUtil::DependentMap dependents_map = {{kEmailType, {{kPersonType, {}}}}}; + SchemaUtil::SchemaDelta result_schema_delta = + SchemaUtil::ComputeCompatibilityDelta(old_schema, new_schema, + dependents_map); + EXPECT_THAT(result_schema_delta, Eq(schema_delta)); +} + +TEST_P(SchemaUtilTest, DeletingIndexedDocumentPropertyIsIncompatible) { + SchemaTypeConfigProto nested_schema = + SchemaTypeConfigBuilder() + .SetType(kEmailType) + .AddProperty(PropertyConfigBuilder() + .SetName("subject") + .SetDataTypeString(TERM_MATCH_EXACT, TOKENIZER_PLAIN) + .SetCardinality(CARDINALITY_OPTIONAL)) + .Build(); + + // Configure old schemam with two nested document properties of the same type + SchemaProto old_schema = + SchemaBuilder() + .AddType(nested_schema) + .AddType(SchemaTypeConfigBuilder() + .SetType(kPersonType) + .AddProperty(PropertyConfigBuilder() + .SetName("Property") + .SetDataTypeInt64(NUMERIC_MATCH_RANGE) + .SetCardinality(CARDINALITY_OPTIONAL)) + .AddProperty( + PropertyConfigBuilder() + .SetName("EmailProperty") + .SetDataTypeDocument( + kEmailType, /*index_nested_properties=*/true) + .SetCardinality(CARDINALITY_OPTIONAL)) + .AddProperty( + PropertyConfigBuilder() + .SetName("AnotherEmailProperty") + .SetDataTypeDocument( + kEmailType, /*index_nested_properties=*/true) + .SetCardinality(CARDINALITY_OPTIONAL))) + .Build(); + + // Configure new schema and drop one of the nested document properties + SchemaProto new_schema = + SchemaBuilder() + .AddType(nested_schema) + .AddType(SchemaTypeConfigBuilder() + .SetType(kPersonType) + .AddProperty(PropertyConfigBuilder() + .SetName("Property") + .SetDataTypeInt64(NUMERIC_MATCH_RANGE) + .SetCardinality(CARDINALITY_OPTIONAL)) + .AddProperty( + PropertyConfigBuilder() + .SetName("EmailProperty") + .SetDataTypeDocument( + kEmailType, /*index_nested_properties=*/true) + .SetCardinality(CARDINALITY_OPTIONAL))) + .Build(); + + SchemaUtil::SchemaDelta schema_delta; + schema_delta.schema_types_incompatible.insert(kPersonType); + schema_delta.schema_types_index_incompatible.insert(kPersonType); + schema_delta.schema_types_join_incompatible.insert(kPersonType); + + SchemaUtil::DependentMap dependents_map = {{kEmailType, {{kPersonType, {}}}}}; + SchemaUtil::SchemaDelta result_schema_delta = + SchemaUtil::ComputeCompatibilityDelta(old_schema, new_schema, + dependents_map); + EXPECT_THAT(result_schema_delta, Eq(schema_delta)); +} + +TEST_P(SchemaUtilTest, + DeletingNonIndexedDocumentPropertyIsIncompatible) { + SchemaTypeConfigProto nested_schema = + SchemaTypeConfigBuilder() + .SetType(kEmailType) + .AddProperty(PropertyConfigBuilder() + .SetName("subject") + .SetDataTypeString(TERM_MATCH_EXACT, TOKENIZER_PLAIN) + .SetCardinality(CARDINALITY_OPTIONAL)) + .Build(); + + // Configure old schemam with two nested document properties of the same type + SchemaProto old_schema = + SchemaBuilder() + .AddType(nested_schema) + .AddType(SchemaTypeConfigBuilder() + .SetType(kPersonType) + .AddProperty(PropertyConfigBuilder() + .SetName("Property") + .SetDataTypeInt64(NUMERIC_MATCH_RANGE) + .SetCardinality(CARDINALITY_OPTIONAL)) + .AddProperty( + PropertyConfigBuilder() + .SetName("EmailProperty") + .SetDataTypeDocument( + kEmailType, /*index_nested_properties=*/true) + .SetCardinality(CARDINALITY_OPTIONAL)) + .AddProperty(PropertyConfigBuilder() + .SetName("AnotherEmailProperty") + .SetDataTypeDocument( + kEmailType, + /*index_nested_properties=*/false) + .SetCardinality(CARDINALITY_OPTIONAL))) + .Build(); + + // Configure new schema and drop the non-indexed nested document property + SchemaProto new_schema = + SchemaBuilder() + .AddType(nested_schema) + .AddType(SchemaTypeConfigBuilder() + .SetType(kPersonType) + .AddProperty(PropertyConfigBuilder() + .SetName("Property") + .SetDataTypeInt64(NUMERIC_MATCH_RANGE) + .SetCardinality(CARDINALITY_OPTIONAL)) + .AddProperty( + PropertyConfigBuilder() + .SetName("EmailProperty") + .SetDataTypeDocument( + kEmailType, /*index_nested_properties=*/true) + .SetCardinality(CARDINALITY_OPTIONAL))) + .Build(); + + SchemaUtil::SchemaDelta schema_delta; + schema_delta.schema_types_incompatible.insert(kPersonType); + schema_delta.schema_types_join_incompatible.insert(kPersonType); + + SchemaUtil::DependentMap dependents_map = {{kEmailType, {{kPersonType, {}}}}}; + SchemaUtil::SchemaDelta result_schema_delta = + SchemaUtil::ComputeCompatibilityDelta(old_schema, new_schema, + dependents_map); + EXPECT_THAT(result_schema_delta, Eq(schema_delta)); +} + +TEST_P(SchemaUtilTest, ChangingIndexedDocumentPropertyIsIncompatible) { + SchemaTypeConfigProto nested_schema = + SchemaTypeConfigBuilder() + .SetType(kEmailType) + .AddProperty(PropertyConfigBuilder() + .SetName("subject") + .SetDataTypeString(TERM_MATCH_EXACT, TOKENIZER_PLAIN) + .SetCardinality(CARDINALITY_OPTIONAL)) + .Build(); + + // Configure old schemam with two nested document properties of the same type + SchemaProto old_schema = + SchemaBuilder() + .AddType(nested_schema) + .AddType(SchemaTypeConfigBuilder() + .SetType(kPersonType) + .AddProperty(PropertyConfigBuilder() + .SetName("Property") + .SetDataTypeInt64(NUMERIC_MATCH_RANGE) + .SetCardinality(CARDINALITY_OPTIONAL)) + .AddProperty( + PropertyConfigBuilder() + .SetName("EmailProperty") + .SetDataTypeDocument( + kEmailType, /*index_nested_properties=*/true) + .SetCardinality(CARDINALITY_OPTIONAL)) + .AddProperty( + PropertyConfigBuilder() + .SetName("AnotherEmailProperty") + .SetDataTypeDocument( + kEmailType, /*index_nested_properties=*/true) + .SetCardinality(CARDINALITY_OPTIONAL))) + .Build(); + + // Configure new schema and change one of the nested document properties + // to a different name (this is the same as deleting a property and adding + // another) + SchemaProto new_schema = + SchemaBuilder() + .AddType(nested_schema) + .AddType(SchemaTypeConfigBuilder() + .SetType(kPersonType) + .AddProperty(PropertyConfigBuilder() + .SetName("Property") + .SetDataTypeInt64(NUMERIC_MATCH_RANGE) + .SetCardinality(CARDINALITY_OPTIONAL)) + .AddProperty( + PropertyConfigBuilder() + .SetName("EmailProperty") + .SetDataTypeDocument( + kEmailType, /*index_nested_properties=*/true) + .SetCardinality(CARDINALITY_OPTIONAL)) + .AddProperty( + PropertyConfigBuilder() + .SetName("DifferentEmailProperty") + .SetDataTypeDocument( + kEmailType, /*index_nested_properties=*/true) + .SetCardinality(CARDINALITY_OPTIONAL))) + .Build(); + + SchemaUtil::SchemaDelta schema_delta; + schema_delta.schema_types_incompatible.insert(kPersonType); + schema_delta.schema_types_index_incompatible.insert(kPersonType); + schema_delta.schema_types_join_incompatible.insert(kPersonType); + + SchemaUtil::DependentMap dependents_map = {{kEmailType, {{kPersonType, {}}}}}; + SchemaUtil::SchemaDelta result_schema_delta = + SchemaUtil::ComputeCompatibilityDelta(old_schema, new_schema, + dependents_map); + EXPECT_THAT(result_schema_delta, Eq(schema_delta)); +} + +TEST_P(SchemaUtilTest, ChangingNonIndexedDocumentPropertyIsIncompatible) { + SchemaTypeConfigProto nested_schema = + SchemaTypeConfigBuilder() + .SetType(kEmailType) + .AddProperty(PropertyConfigBuilder() + .SetName("subject") + .SetDataTypeString(TERM_MATCH_EXACT, TOKENIZER_PLAIN) + .SetCardinality(CARDINALITY_OPTIONAL)) + .Build(); + + // Configure old schemam with two nested document properties of the same type + SchemaProto old_schema = + SchemaBuilder() + .AddType(nested_schema) + .AddType(SchemaTypeConfigBuilder() + .SetType(kPersonType) + .AddProperty(PropertyConfigBuilder() + .SetName("Property") + .SetDataTypeInt64(NUMERIC_MATCH_RANGE) + .SetCardinality(CARDINALITY_OPTIONAL)) + .AddProperty( + PropertyConfigBuilder() + .SetName("EmailProperty") + .SetDataTypeDocument( + kEmailType, /*index_nested_properties=*/true) + .SetCardinality(CARDINALITY_OPTIONAL)) + .AddProperty(PropertyConfigBuilder() + .SetName("AnotherEmailProperty") + .SetDataTypeDocument( + kEmailType, + /*index_nested_properties=*/false) + .SetCardinality(CARDINALITY_OPTIONAL))) + .Build(); + + // Configure new schema and change the non-indexed nested document property to + // a different name (this is the same as deleting a property and adding + // another) + SchemaProto new_schema = + SchemaBuilder() + .AddType(nested_schema) + .AddType(SchemaTypeConfigBuilder() + .SetType(kPersonType) + .AddProperty(PropertyConfigBuilder() + .SetName("Property") + .SetDataTypeInt64(NUMERIC_MATCH_RANGE) + .SetCardinality(CARDINALITY_OPTIONAL)) + .AddProperty( + PropertyConfigBuilder() + .SetName("EmailProperty") + .SetDataTypeDocument( + kEmailType, /*index_nested_properties=*/true) + .SetCardinality(CARDINALITY_OPTIONAL)) + .AddProperty(PropertyConfigBuilder() + .SetName("DifferentEmailProperty") + .SetDataTypeDocument( + kEmailType, + /*index_nested_properties=*/false) + .SetCardinality(CARDINALITY_OPTIONAL))) + .Build(); + + SchemaUtil::SchemaDelta schema_delta; + schema_delta.schema_types_incompatible.insert(kPersonType); + schema_delta.schema_types_join_incompatible.insert(kPersonType); + + SchemaUtil::DependentMap dependents_map = {{kEmailType, {{kPersonType, {}}}}}; + SchemaUtil::SchemaDelta result_schema_delta = + SchemaUtil::ComputeCompatibilityDelta(old_schema, new_schema, + dependents_map); + EXPECT_THAT(result_schema_delta, Eq(schema_delta)); +} + TEST_P(SchemaUtilTest, ChangingJoinablePropertiesMakesJoinIncompatible) { // Configure old schema SchemaProto schema_with_joinable_property = diff --git a/icing/testing/common-matchers.h b/icing/testing/common-matchers.h index bbc1a59..c6500db 100644 --- a/icing/testing/common-matchers.h +++ b/icing/testing/common-matchers.h @@ -241,7 +241,9 @@ MATCHER_P(EqualsSetSchemaResult, expected, "") { actual.schema_types_changed_fully_compatible_by_name == expected.schema_types_changed_fully_compatible_by_name && actual.schema_types_index_incompatible_by_name == - expected.schema_types_index_incompatible_by_name) { + expected.schema_types_index_incompatible_by_name && + actual.schema_types_join_incompatible_by_name == + expected.schema_types_join_incompatible_by_name) { return true; } @@ -338,6 +340,21 @@ MATCHER_P(EqualsSetSchemaResult, expected, "") { ","), "]"); + // Format schema_types_join_incompatible_by_name + std::string actual_schema_types_join_incompatible_by_name = + absl_ports::StrCat( + "[", + absl_ports::StrJoin(actual.schema_types_join_incompatible_by_name, + ","), + "]"); + + std::string expected_schema_types_join_incompatible_by_name = + absl_ports::StrCat( + "[", + absl_ports::StrJoin(expected.schema_types_join_incompatible_by_name, + ","), + "]"); + *result_listener << IcingStringUtil::StringPrintf( "\nExpected {\n" "\tsuccess=%d,\n" @@ -347,8 +364,9 @@ MATCHER_P(EqualsSetSchemaResult, expected, "") { "\tschema_types_incompatible_by_name=%s,\n" "\tschema_types_incompatible_by_id=%s\n" "\tschema_types_new_by_name=%s,\n" - "\tschema_types_index_incompatible_by_name=%s,\n" "\tschema_types_changed_fully_compatible_by_name=%s\n" + "\tschema_types_index_incompatible_by_name=%s,\n" + "\tschema_types_join_incompatible_by_name=%s\n" "}\n" "Actual {\n" "\tsuccess=%d,\n" @@ -358,8 +376,9 @@ MATCHER_P(EqualsSetSchemaResult, expected, "") { "\tschema_types_incompatible_by_name=%s,\n" "\tschema_types_incompatible_by_id=%s\n" "\tschema_types_new_by_name=%s,\n" - "\tschema_types_index_incompatible_by_name=%s,\n" "\tschema_types_changed_fully_compatible_by_name=%s\n" + "\tschema_types_index_incompatible_by_name=%s,\n" + "\tschema_types_join_incompatible_by_name=%s\n" "}\n", expected.success, expected_old_schema_type_ids_changed.c_str(), expected_schema_types_deleted_by_name.c_str(), @@ -368,7 +387,8 @@ MATCHER_P(EqualsSetSchemaResult, expected, "") { expected_schema_types_incompatible_by_id.c_str(), expected_schema_types_new_by_name.c_str(), expected_schema_types_changed_fully_compatible_by_name.c_str(), - expected_schema_types_index_incompatible_by_name.c_str(), actual.success, + expected_schema_types_index_incompatible_by_name.c_str(), + expected_schema_types_join_incompatible_by_name.c_str(), actual.success, actual_old_schema_type_ids_changed.c_str(), actual_schema_types_deleted_by_name.c_str(), actual_schema_types_deleted_by_id.c_str(), @@ -376,7 +396,8 @@ MATCHER_P(EqualsSetSchemaResult, expected, "") { actual_schema_types_incompatible_by_id.c_str(), actual_schema_types_new_by_name.c_str(), actual_schema_types_changed_fully_compatible_by_name.c_str(), - actual_schema_types_index_incompatible_by_name.c_str()); + actual_schema_types_index_incompatible_by_name.c_str(), + actual_schema_types_join_incompatible_by_name.c_str()); return false; } diff --git a/synced_AOSP_CL_number.txt b/synced_AOSP_CL_number.txt index 09c1edd..58918d3 100644 --- a/synced_AOSP_CL_number.txt +++ b/synced_AOSP_CL_number.txt @@ -1 +1 @@ -set(synced_AOSP_CL_number=546391919) +set(synced_AOSP_CL_number=548180296) |