Merge pull request #2824 from xfxyjwf/i1415

Use per-type table to lookup JSON name.

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");

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"];
+}

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