Commit 61e87f3d authored by Feng Xiao's avatar Feng Xiao

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.
parent cad6a51a
...@@ -74,6 +74,8 @@ using google::protobuf::testing::Primitive; ...@@ -74,6 +74,8 @@ using google::protobuf::testing::Primitive;
using google::protobuf::testing::Proto3Message; using google::protobuf::testing::Proto3Message;
using google::protobuf::testing::Publisher; using google::protobuf::testing::Publisher;
using google::protobuf::testing::StructType; using google::protobuf::testing::StructType;
using google::protobuf::testing::TestJsonName1;
using google::protobuf::testing::TestJsonName2;
using google::protobuf::testing::TimestampDuration; using google::protobuf::testing::TimestampDuration;
using google::protobuf::testing::ValueWrapper; using google::protobuf::testing::ValueWrapper;
using google::protobuf::testing::oneofs::OneOfsRequest; using google::protobuf::testing::oneofs::OneOfsRequest;
...@@ -271,6 +273,26 @@ TEST_P(ProtoStreamObjectWriterTest, CustomJsonName) { ...@@ -271,6 +273,26 @@ TEST_P(ProtoStreamObjectWriterTest, CustomJsonName) {
CheckOutput(book); 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) { TEST_P(ProtoStreamObjectWriterTest, IntEnumValuesAreAccepted) {
Book book; Book book;
book.set_title("Some Book"); book.set_title("Some Book");
......
...@@ -190,3 +190,12 @@ message Cyclic { ...@@ -190,3 +190,12 @@ message Cyclic {
repeated Author m_author = 5; repeated Author m_author = 5;
optional Cyclic m_cyclic = 4; 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"];
}
...@@ -107,12 +107,12 @@ class TypeInfoForTypeResolver : public TypeInfo { ...@@ -107,12 +107,12 @@ class TypeInfoForTypeResolver : public TypeInfo {
virtual const google::protobuf::Field* FindField( virtual const google::protobuf::Field* FindField(
const google::protobuf::Type* type, StringPiece camel_case_name) const { const google::protobuf::Type* type, StringPiece camel_case_name) const {
if (indexed_types_.find(type) == indexed_types_.end()) { std::map<const google::protobuf::Type*,
PopulateNameLookupTable(type); CamelCaseNameTable>::const_iterator it = indexed_types_.find(type);
indexed_types_.insert(type); const CamelCaseNameTable& camel_case_name_table = (it == indexed_types_.end())
} ? PopulateNameLookupTable(type, &indexed_types_[type]) : it->second;
StringPiece name = StringPiece name =
FindWithDefault(camel_case_name_table_, camel_case_name, StringPiece()); FindWithDefault(camel_case_name_table, camel_case_name, StringPiece());
if (name.empty()) { if (name.empty()) {
// Didn't find a mapping. Use whatever provided. // Didn't find a mapping. Use whatever provided.
name = camel_case_name; name = camel_case_name;
...@@ -123,6 +123,7 @@ class TypeInfoForTypeResolver : public TypeInfo { ...@@ -123,6 +123,7 @@ class TypeInfoForTypeResolver : public TypeInfo {
private: private:
typedef util::StatusOr<const google::protobuf::Type*> StatusOrType; typedef util::StatusOr<const google::protobuf::Type*> StatusOrType;
typedef util::StatusOr<const google::protobuf::Enum*> StatusOrEnum; typedef util::StatusOr<const google::protobuf::Enum*> StatusOrEnum;
typedef std::map<StringPiece, StringPiece> CamelCaseNameTable;
template <typename T> template <typename T>
static void DeleteCachedTypes(std::map<StringPiece, T>* cached_types) { static void DeleteCachedTypes(std::map<StringPiece, T>* cached_types) {
...@@ -134,32 +135,35 @@ class TypeInfoForTypeResolver : public TypeInfo { ...@@ -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) { for (int i = 0; i < type->fields_size(); ++i) {
const google::protobuf::Field& field = type->fields(i); const google::protobuf::Field& field = type->fields(i);
StringPiece name = field.name(); StringPiece name = field.name();
StringPiece camel_case_name = field.json_name(); StringPiece camel_case_name = field.json_name();
const StringPiece* existing = InsertOrReturnExisting( const StringPiece* existing = InsertOrReturnExisting(
&camel_case_name_table_, camel_case_name, name); camel_case_name_table, camel_case_name, name);
if (existing && *existing != name) { if (existing && *existing != name) {
GOOGLE_LOG(WARNING) << "Field '" << name << "' and '" << *existing GOOGLE_LOG(WARNING) << "Field '" << name << "' and '" << *existing
<< "' map to the same camel case name '" << camel_case_name << "' map to the same camel case name '" << camel_case_name
<< "'."; << "'.";
} }
} }
return *camel_case_name_table;
} }
TypeResolver* type_resolver_; TypeResolver* type_resolver_;
// Stores string values that will be referenced by StringPieces in // 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::set<string> string_storage_;
mutable std::map<StringPiece, StatusOrType> cached_types_; mutable std::map<StringPiece, StatusOrType> cached_types_;
mutable std::map<StringPiece, StatusOrEnum> cached_enums_; mutable std::map<StringPiece, StatusOrEnum> cached_enums_;
mutable std::set<const google::protobuf::Type*> indexed_types_; mutable std::map<const google::protobuf::Type*,
mutable std::map<StringPiece, StringPiece> camel_case_name_table_; CamelCaseNameTable> indexed_types_;
}; };
} // namespace } // namespace
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment