Recognize well-known types

[mgravell]

for APIs that involve, say, DateTime (rather than custom message types) we should try to pick the most reasonable looking well-known-type and use that;

obvious candidates:

• DateTime / .google.protobuf.Timestamp (corelogic readily available in protobuf-net; BclHelpers.ReadTimeStamp / WriteTimestamp)
• TimeSpan / .google.protobuf.Duration (core logic readily available in protobuf-net; BclHelpers.ReadDuration / WriteDuration)
• void / .google.protobuf.Empty (already handled, but should migrate to the new Empty in protobuf-net)

the following probably already work fine (not sure about byte[]), as "assume a message with a single field with index 1" is the default behavior already:

• double / .google.protobuf.DoubleValue (wrappers.proto)
• float / .google.protobuf.FloatValue (wrappers.proto)
• long / .google.protobuf.Int64Value (wrappers.proto)
• ulong / .google.protobuf.UInt64Value (wrappers.proto)
• int / .google.protobuf.Int32Value (wrappers.proto)
• uint / .google.protobuf.UInt32Value (wrappers.proto)
• bool / .google.protobuf.BoolValue (wrappers.proto)
• string / .google.protobuf.StringValue (wrappers.proto)
• byte[] / .google.protobuf.BytesValue (wrappers.proto)

Note: with the above, we should probably hard remove the ability to opt in/out of data-contract types on the marshaller; it should be "nope, it is either something we recognize, or a data-contract type".

Note: we should also think about null in the above; since outer-most message don't include a length prefix, this could be awkward - since wrappers.proto is proto3, that means that there is no way of expressing a difference between 0 and null. Maybe we just don't support nulls ?

less obvious - the only ones I can see us even considering are Type and Api

• .google.protobuf.Any (any.proto) - mostly because I haven't seen this used to know the full intent
• .google.protobuf.Api (api.proto) - possibly maps to Type when service-contract?
• .google.protobuf.Type (type.proto) - possibly maps to Type when data-contract?
• .google.protobuf.FieldMask (field_mask.proto) - very bespoke; not implemented
• .google.protobuf.SourceContext (source_context.proto) - kinda like Assembly maybe?
• .google.protobuf.Struct - think Dictionary<string, .google.protobuf.Value>
• .google.protobuf.Value - a catch all for several other types; object maybe? or is that Any?

[jakubsuchybio]

Any updates on this? Thx especially on fieldMasks

@mgravell I'm really interested is support for google.protobuf.Struct. I saw you mentioned that you might consider adding support for struct.proto in this issue as well. Is it on your radar to implement any time soon?
P.S. I see you removed "help wanted" label from this one, but if there is any way I could contribute (probably will need some guidance) - let me know.

[codymullins]

Hey @mgravell I'd like to add support for field_mask.proto, even if just on a fork or an extension to this library. Could you give any guidance or advice on a high level of how you'd implement this? Would like to stay close to your vision in case it's something you'd want to pull in later.

[mgravell]

honestly, I can't think of any good ways of fitting this retrospectively; I had in mind trying to add this as part of the AOT work, because I genuinely think it needs a very different approach; my idea was (approximately):

abstract class FieldMask {
    public static FieldMask All {get;} = new OpenFieldMask(); 
    public abstract bool Include(string field, [NotNullWhen(true)] out FieldMask? children);

    sealed class FilteredFieldMask : FieldMask
    {
        private readonly Dictionary<string, FieldMask> fields = ...
        public override bool Include(string field, [NotNullWhen(true)] out FieldMask? children)
            => fields.TryGetValue(field, out children);
    }
    sealed class OpenFieldMask : FieldMask
    {
        public override bool Include(string field, [NotNullWhen(true)] out FieldMask? children)
        {
            children = All;
            return true;
        }
    }
}

with some kind of parse step that normalizes string[] (or similar) into the composed field maps, then changing "serialize" and "deserialize" to add a logical if (mask.Include("a", out var children)) { /* serialize, passing down children */}; I think that covers the core semantic requirements, but it is not trivial to add to the IL code, hence my idea of adding it in the AOT work.

[codymullins]

Is the AOT work branched anywhere? I might be missing it (but it also might not be started yet -- which is fine)

[mgravell]

This is ultimately protobuf-net thing, so mostly over in protobuf-net.BuildTools (although there is a recent branch here too), however: most of the existing work needs to be massively refactored - largely a reference pie e and a "now that we know all those gaps" thing. I need to carve a big chunk o' time. It isn't small.

The change you want is one I support, but I don't think it is one you can just squeeze in quickly - although I do support the enthusiasm.

[codymullins]

Understood; would love to help with this effort, so please let me know when and how I can contribute!

[codymullins]

Here are some examples of how we are using this "pattern" in our app - it's not fancy or pretty, but it's us shimming in support in a few places as we build out our APIs and background tasks that use the masking feature. Hopefully this helps add a real world scenario outside of what you already may be thinking.

Updating specific data on an artifact, this is on the server:

if (request.UpdateMask is not null)
{
    if (request.UpdateMask.Fields.Contains("ArtifactStage"))
    {
        artifact.ArtifactStage = request.ArtifactDetails.ArtifactStage;
    }

    if (request.UpdateMask.Fields.Contains("ImageSha"))
    {
        artifact.ImageSha = request.ArtifactDetails.ImageSha;
    }
}
else
{
    artifact.ArtifactStage = request.ArtifactDetails.ArtifactStage;
    artifact.ImageSha = request.ArtifactDetails.ImageSha;
}

Setting a deployment as failed, this is on the client/background task:

await hostingService.UpdateDeployment(
    new UpdateDeploymentRequest
    {
        DeploymentId = context.Data.DeploymentId.Value,
        DeployStatus = DeployStatus.Error,
        UpdateMask = ["DeployStatus"]
    }, context.CallContext());