diff --git a/src/google/protobuf/util/message_differencer.cc b/src/google/protobuf/util/message_differencer.cc index 76e0267a7f..362db4723e 100644 --- a/src/google/protobuf/util/message_differencer.cc +++ b/src/google/protobuf/util/message_differencer.cc @@ -553,6 +553,8 @@ bool MessageDifferencer::FieldBefore(const FieldDescriptor* field1, bool MessageDifferencer::Compare(const Message& message1, const Message& message2) { std::vector parent_fields; + force_compare_no_presence_fields_.clear(); + force_compare_failure_triggering_fields_.clear(); bool result = false; // Setup the internal reporter if need be. @@ -580,6 +582,8 @@ bool MessageDifferencer::CompareWithFields( } std::vector parent_fields; + force_compare_no_presence_fields_.clear(); + force_compare_failure_triggering_fields_.clear(); bool result = false; @@ -757,12 +761,12 @@ FieldDescriptorArray MessageDifferencer::CombineFields( if (FieldBefore(field1, field2)) { if (fields1_scope == FULL) { - tmp_message_fields_.push_back(fields1[index1]); + tmp_message_fields_.push_back(field1); } ++index1; } else if (FieldBefore(field2, field1)) { if (fields2_scope == FULL) { - tmp_message_fields_.push_back(fields2[index2]); + tmp_message_fields_.push_back(field2); } else if (fields2_scope == PARTIAL && force_compare_no_presence_ && !field2->has_presence() && !field2->is_repeated()) { // In order to make MessageDifferencer play nicely with no-presence @@ -772,12 +776,12 @@ FieldDescriptorArray MessageDifferencer::CombineFields( // Those fields will appear in fields2 (since they have non default // value) but will not appear in fields1 (since they have the default // value or were never set). - force_compare_no_presence_fields_.insert(fields2[index2]); - tmp_message_fields_.push_back(fields2[index2]); + force_compare_no_presence_fields_.insert(field2); + tmp_message_fields_.push_back(field2); } ++index2; } else { - tmp_message_fields_.push_back(fields1[index1]); + tmp_message_fields_.push_back(field1); ++index1; ++index2; } @@ -865,6 +869,10 @@ bool MessageDifferencer::CompareWithFieldsInternal( ++field_index1; continue; } else if (FieldBefore(field2, field1)) { + if (force_compare_no_presence_fields_.contains(field2)) { + force_compare_failure_triggering_fields_.insert(field2->full_name()); + } + // Field 2 is not in the field list for message 1. if (IsIgnored(message1, message2, field2, *parent_fields)) { // We are ignoring field2. Report the ignore and move on to @@ -956,6 +964,10 @@ bool MessageDifferencer::CompareWithFieldsInternal( fieldDifferent = !CompareFieldValueUsingParentFields( message1, message2, unpacked_any, field1, -1, -1, parent_fields); + if (force_compare_no_presence_fields_.contains(field1)) { + force_compare_failure_triggering_fields_.insert(field1->full_name()); + } + if (reporter_ != nullptr) { SpecificField specific_field; specific_field.message1 = &message1; diff --git a/src/google/protobuf/util/message_differencer.h b/src/google/protobuf/util/message_differencer.h index d40430de2d..387b333503 100644 --- a/src/google/protobuf/util/message_differencer.h +++ b/src/google/protobuf/util/message_differencer.h @@ -655,6 +655,13 @@ class PROTOBUF_EXPORT MessageDifferencer { // differences to any previously set reporters or output strings. void ReportDifferencesTo(Reporter* reporter); + // Returns the list of fields which was automatically added to the list of + // compared fields by calling set_force_compare_no_presence and caused the + // last call to Compare to fail. + const absl::flat_hash_set& NoPresenceFieldsCausingFailure() { + return force_compare_failure_triggering_fields_; + } + private: // Class for processing Any deserialization. This logic is used by both the // MessageDifferencer and StreamReporter classes. @@ -947,6 +954,7 @@ class PROTOBUF_EXPORT MessageDifferencer { MessageFieldComparison message_field_comparison_; Scope scope_; absl::flat_hash_set force_compare_no_presence_fields_; + absl::flat_hash_set force_compare_failure_triggering_fields_; RepeatedFieldComparison repeated_field_comparison_; absl::flat_hash_map diff --git a/src/google/protobuf/util/message_differencer_unittest.cc b/src/google/protobuf/util/message_differencer_unittest.cc index da9daf1187..ee47a6f033 100644 --- a/src/google/protobuf/util/message_differencer_unittest.cc +++ b/src/google/protobuf/util/message_differencer_unittest.cc @@ -258,11 +258,18 @@ TEST(MessageDifferencerTest, // Clearing a no presence field inside a repeated field in a nested message. msg1.mutable_no_presence_repeated_nested(0)->clear_no_presence_bool(); EXPECT_FALSE(force_compare_differencer.Compare(msg1, msg2)); + EXPECT_THAT(force_compare_differencer.NoPresenceFieldsCausingFailure(), + testing::UnorderedElementsAre( + "proto3_unittest.TestNoPresenceField.no_presence_bool")); EXPECT_TRUE(default_differencer.Compare(msg1, msg2)); + EXPECT_TRUE(default_differencer.NoPresenceFieldsCausingFailure().empty()); force_compare_differencer.ReportDifferencesTo(nullptr); EXPECT_FALSE(force_compare_differencer.Compare(msg2, msg1)); + EXPECT_TRUE( + force_compare_differencer.NoPresenceFieldsCausingFailure().empty()); EXPECT_FALSE(default_differencer.Compare(msg2, msg1)); + EXPECT_TRUE(default_differencer.NoPresenceFieldsCausingFailure().empty()); } TEST(MessageDifferencerTest, @@ -284,7 +291,10 @@ TEST(MessageDifferencerTest, msg1.clear_no_presence_repeated_nested(); EXPECT_TRUE(force_compare_differencer.Compare(msg1, msg2)); + EXPECT_TRUE( + force_compare_differencer.NoPresenceFieldsCausingFailure().empty()); EXPECT_TRUE(default_differencer.Compare(msg1, msg2)); + EXPECT_TRUE(default_differencer.NoPresenceFieldsCausingFailure().empty()); EXPECT_FALSE(force_compare_differencer.Compare(msg2, msg1)); EXPECT_FALSE(default_differencer.Compare(msg2, msg1)); @@ -308,9 +318,14 @@ TEST(MessageDifferencerTest, msg1.mutable_no_presence_nested()->clear_no_presence_bool(); EXPECT_FALSE(force_compare_differencer.Compare(msg1, msg2)); + EXPECT_THAT(force_compare_differencer.NoPresenceFieldsCausingFailure(), + testing::UnorderedElementsAre( + "proto3_unittest.TestNoPresenceField.no_presence_bool")); EXPECT_TRUE(default_differencer.Compare(msg1, msg2)); EXPECT_FALSE(force_compare_differencer.Compare(msg2, msg1)); + EXPECT_TRUE( + force_compare_differencer.NoPresenceFieldsCausingFailure().empty()); EXPECT_FALSE(default_differencer.Compare(msg2, msg1)); } @@ -332,18 +347,27 @@ TEST(MessageDifferencerTest, msg1.clear_no_presence_nested(); EXPECT_TRUE(force_compare_differencer.Compare(msg1, msg2)); + EXPECT_TRUE( + force_compare_differencer.NoPresenceFieldsCausingFailure().empty()); EXPECT_TRUE(default_differencer.Compare(msg1, msg2)); EXPECT_FALSE(force_compare_differencer.Compare(msg2, msg1)); + EXPECT_TRUE( + force_compare_differencer.NoPresenceFieldsCausingFailure().empty()); EXPECT_FALSE(default_differencer.Compare(msg2, msg1)); // Creating an instance of the nested field will cause the comparison to fail // since it contains a no presence singualr field. msg1.mutable_no_presence_nested(); EXPECT_FALSE(force_compare_differencer.Compare(msg1, msg2)); + EXPECT_THAT(force_compare_differencer.NoPresenceFieldsCausingFailure(), + testing::UnorderedElementsAre( + "proto3_unittest.TestNoPresenceField.no_presence_bool")); EXPECT_TRUE(default_differencer.Compare(msg1, msg2)); EXPECT_FALSE(force_compare_differencer.Compare(msg2, msg1)); + EXPECT_TRUE( + force_compare_differencer.NoPresenceFieldsCausingFailure().empty()); EXPECT_FALSE(default_differencer.Compare(msg2, msg1)); } @@ -365,9 +389,13 @@ TEST(MessageDifferencerTest, msg1.clear_no_presence_bool(); EXPECT_FALSE(force_compare_differencer.Compare(msg1, msg2)); + EXPECT_TRUE( + !force_compare_differencer.NoPresenceFieldsCausingFailure().empty()); EXPECT_TRUE(default_differencer.Compare(msg1, msg2)); EXPECT_FALSE(force_compare_differencer.Compare(msg2, msg1)); + EXPECT_TRUE( + force_compare_differencer.NoPresenceFieldsCausingFailure().empty()); EXPECT_FALSE(default_differencer.Compare(msg2, msg1)); } @@ -392,6 +420,8 @@ TEST(MessageDifferencerTest, msg1.mutable_no_presence_repeated_nested(0)->clear_no_presence_bool(); EXPECT_FALSE(force_compare_differencer.Compare(msg1, msg2)); + EXPECT_TRUE( + !force_compare_differencer.NoPresenceFieldsCausingFailure().empty()); } EXPECT_EQ(output, "added: no_presence_repeated_nested[0].no_presence_bool (added for "