Default JsonFormatter.diagnosticFormatter to show fields with default values

[smclewin]

trafficstars

In the .net Protobuf library I want diagnostic output from the JsonFormatter that can both show any custom output I've added with an ICustomDiagnosticMessage as well as showing fields that have default values (0, null, etc).

While I can create my own JsonFormatter object and pass in settings

// Create a JsonFormatter that will show fields even when they have a default value.
var formatter = new JsonFormatter(new JsonFormatter.Settings(true));

When I create a JsonFormatter class myself it will not call any ICustomDiagnosticMessage implementations. The implementation of the JsonFormatter.WriteMessage() method will only call for custom diagnostic messages when the instance of the JsonFormatteris the one held in the diagnosticFormattermember of the JsonFormatterclass.

I cannot find any path where I can alter the JsonFormatter.Settings.Default object to set JsonFormatter.Settings.FormatDefaultValues to true.

I think I'm forced to choose between having my ICustomDiagnosticMessageformatted output, or having fields with default values present in the output, but I cannot have both at the same time.

I tried pulling a copy of JsonFormatterinto my project to create a custom implementation, but it depends on members/methods that are marked as internal to the Google.Protobuf library (such as message.Descriptor.IsWellKnownType).

I would like to either be able to set JsonFormatter.settings.FormatDefaultValuesto true for my overall application, or to be able to control the behavior of whether default values are printed when calling ToDiagnosticString()

[anandolee]

"I think I'm forced to choose between having my ICustomDiagnosticMessageformatted output, or having fields with default values present in the output, but I cannot have both at the same time."

Can you add a property in your ICustomDiagnosticMessage to set if you want to format the default values or not?

[smclewin]

Based on my reading of the JsonFormatter code, I don't believe so @anandolee. But I'm open to seeing an example of what you are thinking about. I may have missed a path to work around the behavior as I've explained it in the initial issue post.

[anandolee]

I may misunderstood your requirement, thought you want to have the ability to set if you want to format default in your ICustomDiagnosticMessage

public partial class MyMessage : ICustomDiagnosticMessage {
    public bool formatDefault { get; set; }
    public string ToDiagnosticString() {
        //format set fields.
        ...
        if formatDefault {
          //format default fields.
        }
    }
}

If your ICustomDiagnosticMessage is a sub message field, there's no way to call your formatted output and also print default fields. Is it supported by other languages? If c++ or jave has such support we should add it to C# too. Otherwise, I don't think C# should support this usage

[smclewin]

@anandolee I left out what I suspect is a useful use case nuance in my original write-up. We have a custom type, and the custom type has an ICustomDiagnosticMessage, and then the custom type is used within several other messages. I don't have control over how the parent messages are displayed, and I do want all the values to show even if they are a 0|null|""|false default value under some scenarios.

But based on my reading of the JsonFormatter class, this does not change the problem, which is why I left out that level of complexity in my original issue. If I create my own instance of the JsonFormatter class so that it can have whatever Settings I want on it (in my case, display of default fields), then I do not believe the instance I create will ever call to get my custom diagnostic message.

I may not be understanding a nuance here, so please tell me where I've gone wrong.

The WriteMessage method has a check to only write the output of diagnostic message if DiagnosticOnly is true.

private void WriteMessage(TextWriter writer, IMessage message)
        {
            if (message == null)
            {
                WriteNull(writer);
                return;
            }
            if (DiagnosticOnly)
            {
                ICustomDiagnosticMessage customDiagnosticMessage = message as ICustomDiagnosticMessage;
                if (customDiagnosticMessage != null)
                {
                    writer.Write(customDiagnosticMessage.ToDiagnosticString());
                    return;
                }
            }
            writer.Write("{ ");
            bool writtenFields = WriteMessageFields(writer, message, false);
            writer.Write(writtenFields ? " }" : "}");
}

There is a static instance of the JsonFormatter within the class definition. It is created with default settings, which include the setting to suppress display of fields with default values. I see no method offered to change settings on a JsonFormatter once constructed. The Settings class is readonly.

private static readonly JsonFormatter diagnosticFormatter = new JsonFormatter(Settings.Default);

private readonly Settings settings;

The static ToDiagnosticString method expressly calls on the static formatter instance. There is no mechanism offered to change the diagnosticFormatter to reference another instance.

public static string ToDiagnosticString(IMessage message)
        {
            ProtoPreconditions.CheckNotNull(message, nameof(message));
            return diagnosticFormatter.Format(message);
}

And finally, even if there was a method offered to change diagnosticFormatter to reference an instance that had settings where default values were not suppressed in the display, the definition of DiagnosticOnly would prevent that from getting used, since it is defined to be true only when the default instance of the JsonFormatter class is the one attempting to write the message. This is the default instance where I believe the Settings class cannot be altered by a user of the library.

private bool DiagnosticOnly => ReferenceEquals(this, diagnosticFormatter);

I appreciate guidance in seeing what I've overlooked.

Thanks

[anandolee]

Your understanding is right, there's no way to print default fields as well if you want to use ICustomDiagnosticMessage. It is originally designed for ToString(): https://github.com/google/protobuf/issues/933

@jskeet , do you think we need to support FormatDefaultValues (and other settings in Json) for Diagnostic?

[smclewin]

Thanks @anandolee. I do use it for ToString(), in most cases for diagnostic output. There are a few cases where I do want to write out valid JSON that can get parsed back into a POCO through the ProtoBuf parser. Making that choice explicit at the time of the call would be a useful behavior.

[anandolee]

Ping @jskeet JsonFormatter is unable to set the settings for diagnostic. Is it by purpose?

[jskeet]

There was no obvious use case at the time, and it was simpler not to support it. It should be feasible to support it via an extra setting (using the pattern we've now established). I don't think it's worth the risk of trying to get it into 3.3 at this stage though. (So let's try to get it in 3.4, but not cherry pick onto the 3.3 branch.)

[smclewin]

@jskeet I'm still interested in this change. If it made it into 3.4 or later then I'd simply like some guidance on how to make use of the new feature.

[jskeet]

I don't see any sign of this having been done - I'm pretty sure I didn't do it, I'm afraid.

[pditterline]

I'm interested in this as well, for the same reasons smclewin mentioned--I need to print several messages that contain subtypes which have ICustomDiagnosticMessage output defined for human-readable json, and I need to output default values in the message as well. I have to choose between outputting less-readable json with defaults, or readable json without defaults.

[AimeSoleil]

I am using protobuf 3.5.1 csharp version. Also interested in this as well-I generate code from proto file and see no way to override the ToString() or change the flag-settings.FormatDefaultValues . Any update for this enhancement?

[jskeet]

Just revisiting this a little: @AimeSoleil to set FormatDefaultValues you'd use something like:

var settings = JsonFormatter.Settings.Default.WithFormatDefaultValues(true);
var formatter = new JsonFormatter(settings);
var json = formatter.Format(message);

But there's no way of changing what the default JsonFormatter does, nor would I want to add one.

I haven't looked at this issue fully though - it's entirely possible there's more to it than that, e.g. a setting we could add for "emit the diagnostic values". Will look again when I have more time.