diff options
3 files changed, 45 insertions, 10 deletions
diff --git a/src/google/protobuf/util/internal/protostream_objectwriter_test.cc b/src/google/protobuf/util/internal/protostream_objectwriter_test.cc index a9b15e68..97ef8fff 100644 --- a/src/google/protobuf/util/internal/protostream_objectwriter_test.cc +++ b/src/google/protobuf/util/internal/protostream_objectwriter_test.cc @@ -74,6 +74,8 @@ using google::protobuf::testing::Primitive; using google::protobuf::testing::Proto3Message; using google::protobuf::testing::Publisher; using google::protobuf::testing::StructType; +using google::protobuf::testing::TestJsonName1; +using google::protobuf::testing::TestJsonName2; using google::protobuf::testing::TimestampDuration; using google::protobuf::testing::ValueWrapper; using google::protobuf::testing::oneofs::OneOfsRequest; @@ -271,6 +273,26 @@ TEST_P(ProtoStreamObjectWriterTest, CustomJsonName) { CheckOutput(book); } +// Test that two messages can have different fields mapped to the same JSON +// name. See: https://github.com/google/protobuf/issues/1415 +TEST_P(ProtoStreamObjectWriterTest, ConflictingJsonName) { + ResetTypeInfo(TestJsonName1::descriptor()); + TestJsonName1 message1; + message1.set_one_value(12345); + ow_->StartObject("") + ->RenderInt32("value", 12345) + ->EndObject(); + CheckOutput(message1); + + ResetTypeInfo(TestJsonName2::descriptor()); + TestJsonName2 message2; + message2.set_another_value(12345); + ow_->StartObject("") + ->RenderInt32("value", 12345) + ->EndObject(); + CheckOutput(message2); +} + TEST_P(ProtoStreamObjectWriterTest, IntEnumValuesAreAccepted) { Book book; book.set_title("Some Book"); diff --git a/src/google/protobuf/util/internal/testdata/books.proto b/src/google/protobuf/util/internal/testdata/books.proto index 9fe4f7aa..869271f4 100644 --- a/src/google/protobuf/util/internal/testdata/books.proto +++ b/src/google/protobuf/util/internal/testdata/books.proto @@ -190,3 +190,12 @@ message Cyclic { repeated Author m_author = 5; optional Cyclic m_cyclic = 4; } + +// Test that two messages can have different fields mapped to the same JSON +// name. See: https://github.com/google/protobuf/issues/1415 +message TestJsonName1 { + optional int32 one_value = 1 [json_name = "value"]; +} +message TestJsonName2 { + optional int32 another_value = 1 [json_name = "value"]; +} diff --git a/src/google/protobuf/util/internal/type_info.cc b/src/google/protobuf/util/internal/type_info.cc index 17d58475..a5d903f1 100644 --- a/src/google/protobuf/util/internal/type_info.cc +++ b/src/google/protobuf/util/internal/type_info.cc @@ -107,12 +107,12 @@ class TypeInfoForTypeResolver : public TypeInfo { virtual const google::protobuf::Field* FindField( const google::protobuf::Type* type, StringPiece camel_case_name) const { - if (indexed_types_.find(type) == indexed_types_.end()) { - PopulateNameLookupTable(type); - indexed_types_.insert(type); - } + std::map<const google::protobuf::Type*, + CamelCaseNameTable>::const_iterator it = indexed_types_.find(type); + const CamelCaseNameTable& camel_case_name_table = (it == indexed_types_.end()) + ? PopulateNameLookupTable(type, &indexed_types_[type]) : it->second; StringPiece name = - FindWithDefault(camel_case_name_table_, camel_case_name, StringPiece()); + FindWithDefault(camel_case_name_table, camel_case_name, StringPiece()); if (name.empty()) { // Didn't find a mapping. Use whatever provided. name = camel_case_name; @@ -123,6 +123,7 @@ class TypeInfoForTypeResolver : public TypeInfo { private: typedef util::StatusOr<const google::protobuf::Type*> StatusOrType; typedef util::StatusOr<const google::protobuf::Enum*> StatusOrEnum; + typedef std::map<StringPiece, StringPiece> CamelCaseNameTable; template <typename T> static void DeleteCachedTypes(std::map<StringPiece, T>* cached_types) { @@ -134,32 +135,35 @@ class TypeInfoForTypeResolver : public TypeInfo { } } - void PopulateNameLookupTable(const google::protobuf::Type* type) const { + const CamelCaseNameTable& PopulateNameLookupTable( + const google::protobuf::Type* type, + CamelCaseNameTable* camel_case_name_table) const { for (int i = 0; i < type->fields_size(); ++i) { const google::protobuf::Field& field = type->fields(i); StringPiece name = field.name(); StringPiece camel_case_name = field.json_name(); const StringPiece* existing = InsertOrReturnExisting( - &camel_case_name_table_, camel_case_name, name); + camel_case_name_table, camel_case_name, name); if (existing && *existing != name) { GOOGLE_LOG(WARNING) << "Field '" << name << "' and '" << *existing << "' map to the same camel case name '" << camel_case_name << "'."; } } + return *camel_case_name_table; } TypeResolver* type_resolver_; // Stores string values that will be referenced by StringPieces in - // cached_types_, cached_enums_ and camel_case_name_table_. + // cached_types_, cached_enums_. mutable std::set<string> string_storage_; mutable std::map<StringPiece, StatusOrType> cached_types_; mutable std::map<StringPiece, StatusOrEnum> cached_enums_; - mutable std::set<const google::protobuf::Type*> indexed_types_; - mutable std::map<StringPiece, StringPiece> camel_case_name_table_; + mutable std::map<const google::protobuf::Type*, + CamelCaseNameTable> indexed_types_; }; } // namespace |