Use per-type table to lookup JSON name.

Different fields from different messages can map to the same JSON name
and the original global lookup table is only capable of mapping one of
such fields. This change converts the global table to per-type tables
so fields from different messages won't conflict.

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