On Tue, Nov 23, 2010 at 5:04 PM, Brenden Matthews <bren...@diddyinc.com>wrote:
> Greetings, > > One of the best features of protocol buffers is the notion of presence. > Another great feature is the ability to do 'message.PrintDebugString()'. > One problem, however, is that fields which are not present do not get > displayed when printing the debug string. > > Proposed solution: add an optional parameter to display optional fields and > their default values (if applicable) even when they aren't present. > > Attached is a patch which implements this. > > Please provide comments/suggestions, and let me know if this is the right > place for this. > I also rushed things a bit, and the first patch has a mistake. Here is a fixed version of it. -- You received this message because you are subscribed to the Google Groups "Protocol Buffers" group. To post to this group, send email to proto...@googlegroups.com. To unsubscribe from this group, send email to protobuf+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/protobuf?hl=en.
From edf13be13766caa1a496c9feeced8d9d24de0c10 Mon Sep 17 00:00:00 2001 From: Brenden Matthews <bren...@diddyinc.com> Date: Tue, 23 Nov 2010 17:01:01 -0800 Subject: [PATCH] Display non-present fields with TextFormat. This allows for the non-presence of optional fields to be represented in string output for a message. This greatly aids in debugging data-intensive applications which use protobuf. --- protobuf/src/google/protobuf/descriptor.cc | 2 +- .../protobuf/generated_message_reflection.cc | 9 +-- .../google/protobuf/generated_message_reflection.h | 4 +- protobuf/src/google/protobuf/message.h | 14 ++-- protobuf/src/google/protobuf/text_format.cc | 74 ++++++++++++-------- protobuf/src/google/protobuf/text_format.h | 26 ++++--- 6 files changed, 76 insertions(+), 53 deletions(-) diff --git a/protobuf/src/google/protobuf/descriptor.cc b/protobuf/src/google/protobuf/descriptor.cc index b6f276d..9e8aa51 100644 --- a/protobuf/src/google/protobuf/descriptor.cc +++ b/protobuf/src/google/protobuf/descriptor.cc @@ -1507,7 +1507,7 @@ bool RetrieveOptions(const Message &options, vector<string> *option_entries) { for (int j = 0; j < count; j++) { string fieldval; TextFormat::PrintFieldValueToString(options, fields[i], - repeated ? count : -1, &fieldval); + repeated ? count : -1, &fieldval, false); option_entries->push_back(fields[i]->name() + " = " + fieldval); } } diff --git a/protobuf/src/google/protobuf/generated_message_reflection.cc b/protobuf/src/google/protobuf/generated_message_reflection.cc index 8428b75..aa7e628 100644 --- a/protobuf/src/google/protobuf/generated_message_reflection.cc +++ b/protobuf/src/google/protobuf/generated_message_reflection.cc @@ -640,7 +640,8 @@ struct FieldNumberSorter { void GeneratedMessageReflection::ListFields( const Message& message, - vector<const FieldDescriptor*>* output) const { + vector<const FieldDescriptor*>* output, + const bool include_missing) const { output->clear(); // Optimization: The default instance never has any fields set. @@ -652,10 +653,8 @@ void GeneratedMessageReflection::ListFields( if (FieldSize(message, field) > 0) { output->push_back(field); } - } else { - if (HasBit(message, field)) { - output->push_back(field); - } + } else if (include_missing || HasBit(message, field)) { + output->push_back(field); } } diff --git a/protobuf/src/google/protobuf/generated_message_reflection.h b/protobuf/src/google/protobuf/generated_message_reflection.h index b545fa1..1475f9e 100644 --- a/protobuf/src/google/protobuf/generated_message_reflection.h +++ b/protobuf/src/google/protobuf/generated_message_reflection.h @@ -144,8 +144,8 @@ class LIBPROTOBUF_EXPORT GeneratedMessageReflection : public Reflection { void Swap(Message* message1, Message* message2) const; void SwapElements(Message* message, const FieldDescriptor* field, int index1, int index2) const; - void ListFields(const Message& message, - vector<const FieldDescriptor*>* output) const; + void ListFields(const Message& message, vector<const FieldDescriptor*>* + output, const bool include_missing = false) const; int32 GetInt32 (const Message& message, const FieldDescriptor* field) const; diff --git a/protobuf/src/google/protobuf/message.h b/protobuf/src/google/protobuf/message.h index c0062f9..988f50c 100644 --- a/protobuf/src/google/protobuf/message.h +++ b/protobuf/src/google/protobuf/message.h @@ -243,13 +243,13 @@ class LIBPROTOBUF_EXPORT Message : public MessageLite { // Generates a human readable form of this message, useful for debugging // and other purposes. - string DebugString() const; + string DebugString(const bool show_missing = false) const; // Like DebugString(), but with less whitespace. string ShortDebugString() const; // Like DebugString(), but do not escape UTF-8 byte sequences. - string Utf8DebugString() const; + string Utf8DebugString(const bool show_missing = false) const; // Convenience function useful in GDB. Prints DebugString() to stdout. - void PrintDebugString() const; + void PrintDebugString(const bool show_missing = false) const; // Heavy I/O ------------------------------------------------------- // Additional parsing and serialization methods not implemented by @@ -431,9 +431,11 @@ class LIBPROTOBUF_EXPORT Reflection { // extensions. Singular fields will only be listed if HasField(field) would // return true and repeated fields will only be listed if FieldSize(field) // would return non-zero. Fields (both normal fields and extension fields) - // will be listed ordered by field number. - virtual void ListFields(const Message& message, - vector<const FieldDescriptor*>* output) const = 0; + // will be listed ordered by field number. If the `include_missing' + // parameter is true, this also will return fields that aren't present. + virtual void ListFields(const Message& message, vector<const + FieldDescriptor*>* output, + const bool include_missing = false) const = 0; // Singular field getters ------------------------------------------ // These get the value of a non-repeated field. They return the default diff --git a/protobuf/src/google/protobuf/text_format.cc b/protobuf/src/google/protobuf/text_format.cc index 8c6f2e4..5aa68c2 100644 --- a/protobuf/src/google/protobuf/text_format.cc +++ b/protobuf/src/google/protobuf/text_format.cc @@ -52,10 +52,10 @@ namespace google { namespace protobuf { -string Message::DebugString() const { +string Message::DebugString(const bool show_missing) const { string debug_string; - TextFormat::PrintToString(*this, &debug_string); + TextFormat::PrintToString(*this, &debug_string, show_missing); return debug_string; } @@ -76,19 +76,19 @@ string Message::ShortDebugString() const { return debug_string; } -string Message::Utf8DebugString() const { +string Message::Utf8DebugString(const bool show_missing) const { string debug_string; TextFormat::Printer printer; printer.SetUseUtf8StringEscaping(true); - printer.PrintToString(*this, &debug_string); + printer.PrintToString(*this, &debug_string, show_missing); return debug_string; } -void Message::PrintDebugString() const { - printf("%s", DebugString().c_str()); +void Message::PrintDebugString(const bool show_missing) const { + printf("%s", DebugString(show_missing).c_str()); } @@ -853,13 +853,14 @@ TextFormat::Printer::Printer() TextFormat::Printer::~Printer() {} bool TextFormat::Printer::PrintToString(const Message& message, - string* output) { + string* output, + const bool show_missing) { GOOGLE_DCHECK(output) << "output specified is NULL"; output->clear(); io::StringOutputStream output_stream(output); - bool result = Print(message, &output_stream); + bool result = Print(message, &output_stream, show_missing); return result; } @@ -875,10 +876,10 @@ bool TextFormat::Printer::PrintUnknownFieldsToString( } bool TextFormat::Printer::Print(const Message& message, - io::ZeroCopyOutputStream* output) { + io::ZeroCopyOutputStream* output, const bool show_missing) { TextGenerator generator(output, initial_indent_level_); - Print(message, generator); + Print(message, generator, show_missing); // Output false if the generator failed internally. return !generator.failed(); @@ -896,12 +897,12 @@ bool TextFormat::Printer::PrintUnknownFields( } void TextFormat::Printer::Print(const Message& message, - TextGenerator& generator) { + TextGenerator& generator, const bool show_missing) { const Reflection* reflection = message.GetReflection(); vector<const FieldDescriptor*> fields; - reflection->ListFields(message, &fields); + reflection->ListFields(message, &fields, show_missing); for (unsigned int i = 0; i < fields.size(); i++) { - PrintField(message, reflection, fields[i], generator); + PrintField(message, reflection, fields[i], generator, show_missing); } PrintUnknownFields(reflection->GetUnknownFields(message), generator); } @@ -910,7 +911,7 @@ void TextFormat::Printer::PrintFieldValueToString( const Message& message, const FieldDescriptor* field, int index, - string* output) { + string* output, const bool show_missing) { GOOGLE_DCHECK(output) << "output specified is NULL"; @@ -918,18 +919,18 @@ void TextFormat::Printer::PrintFieldValueToString( io::StringOutputStream output_stream(output); TextGenerator generator(&output_stream, initial_indent_level_); - PrintFieldValue(message, message.GetReflection(), field, index, generator); + PrintFieldValue(message, message.GetReflection(), field, index, generator, show_missing); } void TextFormat::Printer::PrintField(const Message& message, const Reflection* reflection, const FieldDescriptor* field, - TextGenerator& generator) { + TextGenerator& generator, const bool show_missing) { if (use_short_repeated_primitives_ && field->is_repeated() && field->cpp_type() != FieldDescriptor::CPPTYPE_STRING && field->cpp_type() != FieldDescriptor::CPPTYPE_MESSAGE) { - PrintShortRepeatedField(message, reflection, field, generator); + PrintShortRepeatedField(message, reflection, field, generator, show_missing); return; } @@ -941,6 +942,22 @@ void TextFormat::Printer::PrintField(const Message& message, count = 1; } + if (!count && field->is_optional() && field->cpp_type() != FieldDescriptor::CPPTYPE_MESSAGE) { + PrintFieldName(message, reflection, field, generator); + if (field->has_default_value()) { + generator.Print(": "); + PrintFieldValue(message, reflection, field, -1, generator, show_missing); + } else { + generator.Print(": --"); + } + + if (single_line_mode_) { + generator.Print(" "); + } else { + generator.Print("\n"); + } + } + for (int j = 0; j < count; ++j) { PrintFieldName(message, reflection, field, generator); @@ -961,7 +978,7 @@ void TextFormat::Printer::PrintField(const Message& message, field_index = -1; } - PrintFieldValue(message, reflection, field, field_index, generator); + PrintFieldValue(message, reflection, field, field_index, generator, show_missing); if (field->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE) { if (single_line_mode_) { @@ -983,7 +1000,8 @@ void TextFormat::Printer::PrintField(const Message& message, void TextFormat::Printer::PrintShortRepeatedField(const Message& message, const Reflection* reflection, const FieldDescriptor* field, - TextGenerator& generator) { + TextGenerator& generator, + const bool show_missing) { // Print primitive repeated field in short form. PrintFieldName(message, reflection, field, generator); @@ -991,7 +1009,7 @@ void TextFormat::Printer::PrintShortRepeatedField(const Message& message, generator.Print(": ["); for (int i = 0; i < size; i++) { if (i > 0) generator.Print(", "); - PrintFieldValue(message, reflection, field, i, generator); + PrintFieldValue(message, reflection, field, i, generator, show_missing); } if (single_line_mode_) { generator.Print("] "); @@ -1031,7 +1049,7 @@ void TextFormat::Printer::PrintFieldValue( const Reflection* reflection, const FieldDescriptor* field, int index, - TextGenerator& generator) { + TextGenerator& generator, const bool show_missing) { GOOGLE_DCHECK(field->is_repeated() || (index == -1)) << "Index must be -1 for non-repeated fields"; @@ -1089,14 +1107,14 @@ void TextFormat::Printer::PrintFieldValue( Print(field->is_repeated() ? reflection->GetRepeatedMessage(message, field, index) : reflection->GetMessage(message, field), - generator); + generator, show_missing); break; } } /* static */ bool TextFormat::Print(const Message& message, - io::ZeroCopyOutputStream* output) { - return Printer().Print(message, output); + io::ZeroCopyOutputStream* output, const bool show_missing) { + return Printer().Print(message, output, show_missing); } /* static */ bool TextFormat::PrintUnknownFields( @@ -1106,8 +1124,8 @@ void TextFormat::Printer::PrintFieldValue( } /* static */ bool TextFormat::PrintToString( - const Message& message, string* output) { - return Printer().PrintToString(message, output); + const Message& message, string* output, const bool show_missing) { + return Printer().PrintToString(message, output, show_missing); } /* static */ bool TextFormat::PrintUnknownFieldsToString( @@ -1119,8 +1137,8 @@ void TextFormat::Printer::PrintFieldValue( const Message& message, const FieldDescriptor* field, int index, - string* output) { - return Printer().PrintFieldValueToString(message, field, index, output); + string* output, const bool show_missing) { + return Printer().PrintFieldValueToString(message, field, index, output, show_missing); } /* static */ bool TextFormat::ParseFieldValueFromString( diff --git a/protobuf/src/google/protobuf/text_format.h b/protobuf/src/google/protobuf/text_format.h index e78e104..d487e39 100644 --- a/protobuf/src/google/protobuf/text_format.h +++ b/protobuf/src/google/protobuf/text_format.h @@ -58,7 +58,7 @@ class LIBPROTOBUF_EXPORT TextFormat { public: // Outputs a textual representation of the given message to the given // output stream. - static bool Print(const Message& message, io::ZeroCopyOutputStream* output); + static bool Print(const Message& message, io::ZeroCopyOutputStream* output, const bool show_missing = false); // Print the fields in an UnknownFieldSet. They are printed by tag number // only. Embedded messages are heuristically identified by attempting to @@ -66,8 +66,10 @@ class LIBPROTOBUF_EXPORT TextFormat { static bool PrintUnknownFields(const UnknownFieldSet& unknown_fields, io::ZeroCopyOutputStream* output); - // Like Print(), but outputs directly to a string. - static bool PrintToString(const Message& message, string* output); + // Like Print(), but outputs directly to a string. If `show_missing' is + // true, missing fields will also be displayed. + static bool PrintToString(const Message& message, string* output, + const bool show_missing = false); // Like PrintUnknownFields(), but outputs directly to a string. static bool PrintUnknownFieldsToString(const UnknownFieldSet& unknown_fields, @@ -80,7 +82,7 @@ class LIBPROTOBUF_EXPORT TextFormat { static void PrintFieldValueToString(const Message& message, const FieldDescriptor* field, int index, - string* output); + string* output, const bool show_missing = false); // Class for those users which require more fine-grained control over how // a protobuffer message is printed out. @@ -90,12 +92,12 @@ class LIBPROTOBUF_EXPORT TextFormat { ~Printer(); // Like TextFormat::Print - bool Print(const Message& message, io::ZeroCopyOutputStream* output); + bool Print(const Message& message, io::ZeroCopyOutputStream* output, const bool show_missing = false); // Like TextFormat::PrintUnknownFields bool PrintUnknownFields(const UnknownFieldSet& unknown_fields, io::ZeroCopyOutputStream* output); // Like TextFormat::PrintToString - bool PrintToString(const Message& message, string* output); + bool PrintToString(const Message& message, string* output, const bool show_missing = false); // Like TextFormat::PrintUnknownFieldsToString bool PrintUnknownFieldsToString(const UnknownFieldSet& unknown_fields, string* output); @@ -103,7 +105,7 @@ class LIBPROTOBUF_EXPORT TextFormat { void PrintFieldValueToString(const Message& message, const FieldDescriptor* field, int index, - string* output); + string* output, const bool show_missing = false); // Adjust the initial indent level of all output. Each indent level is // equal to two spaces. @@ -143,19 +145,20 @@ class LIBPROTOBUF_EXPORT TextFormat { // Internal Print method, used for writing to the OutputStream via // the TextGenerator class. void Print(const Message& message, - TextGenerator& generator); + TextGenerator& generator, const bool show_missing = false); // Print a single field. void PrintField(const Message& message, const Reflection* reflection, const FieldDescriptor* field, - TextGenerator& generator); + TextGenerator& generator, const bool show_missing = false); // Print a repeated primitive field in short form. void PrintShortRepeatedField(const Message& message, const Reflection* reflection, const FieldDescriptor* field, - TextGenerator& generator); + TextGenerator& generator, + const bool show_missing = false); // Print the name of a field -- i.e. everything that comes before the // ':' for a single name/value pair. @@ -170,7 +173,8 @@ class LIBPROTOBUF_EXPORT TextFormat { const Reflection* reflection, const FieldDescriptor* field, int index, - TextGenerator& generator); + TextGenerator& generator, + const bool show_missing = false); // Print the fields in an UnknownFieldSet. They are printed by tag number // only. Embedded messages are heuristically identified by attempting to -- 1.7.2.3