Issues with MapField type

[codymullins]

Following the documentation here, I added support for the MapField type on a gPRC endpoint.

Using the Dynamic GRPC Nuget package, the MapField type is not supported. I assume the CLI has the same limitation due to the code being used is the same / very close to the same:

Source

await foreach (var result in client.AsyncDynamicCall(service, method, ToAsync(input)))
{
    jsonResponse = ToJson(result)!.ToJsonString();
}

Example

message Equipment {
	string Id = 1;
	string name = 2;
	EquipmentModel model = 3;

	repeated Attributes fields = 4;
}

message Attributes {
	map<string, string> values = 1;
}

Expected

The client should send the request.

Actual

The below error is thrown:

Status(StatusCode="Internal", Detail="Error starting gRPC call. KeyNotFoundException: The given key 'hull_color' was not present in the dictionary.", DebugException="System.Collections.Generic.KeyNotFoundException: The given key 'hull_color' was not present in the dictionary.
   at DynamicGrpc.DynamicMessageSerializer.ComputeSize(IDictionary`2 value, DynamicGrpcClientContext context)
   at DynamicGrpc.DynamicMessageSerializer.DynamicMessage.CalculateSize()
   at Google.Protobuf.CodedOutputStream.ComputeMessageSize(IMessage value)
   at Google.Protobuf.FieldCodec.<>c__32`1.<ForMessage>b__32_4(T message)
   at Google.Protobuf.Collections.RepeatedField`1.CalculateSize(FieldCodec`1 codec)
   at DynamicGrpc.DynamicMessageSerializer.ComputeFieldSize(UInt32 tag, MessageDescriptor parentDescriptor, FieldDescriptor fieldDescriptor, String key, Object value, DynamicGrpcClientContext context)
   at DynamicGrpc.DynamicMessageSerializer.ComputeSize(IDictionary`2 value, DynamicGrpcClientContext context)
   at DynamicGrpc.DynamicMessageSerializer.WriteFieldValue(UInt32 tag, MessageDescriptor parentDescriptor, FieldDescriptor fieldDescriptor, String keyName, Object value, WriteContext& output, DynamicGrpcClientContext context)
   at DynamicGrpc.DynamicMessageSerializer.WriteTo(IDictionary`2 value, WriteContext& output, DynamicGrpcClientContext context)
   at Google.Protobuf.MessageExtensions.WriteTo(IMessage message, IBufferWriter`1 output)
   at DynamicGrpc.DynamicMessageSerializer.<>c__DisplayClass11_0.<GetMarshaller>b__2(IDictionary`2 value, SerializationContext ctx)
   at Grpc.Net.Client.StreamExtensions.WriteMessageAsync[TMessage](Stream stream, GrpcCall call, TMessage message, Action`2 serializer, CallOptions callOptions)
   at Grpc.Net.Client.Internal.PushUnaryContent`2.WriteMessageCore(ValueTask writeMessageTask)
   at System.Net.Http.Http2Connection.Http2Stream.SendRequestBodyAsync(CancellationToken cancellationToken)
   at System.Net.Http.Http2Connection.SendAsync(HttpRequestMessage request, Boolean async, CancellationToken cancellationToken)
   at System.Net.Http.HttpConnectionPool.SendWithVersionDetectionAndRetryAsync(HttpRequestMessage request, Boolean async, Boolean doRequestAuth, CancellationToken cancellationToken)
   at System.Net.Http.RedirectHandler.SendAsync(HttpRequestMessage request, Boolean async, CancellationToken cancellationToken)
   at Grpc.Net.Client.Internal.GrpcCall`2.RunCall(HttpRequestMessage request, Nullable`1 timeout)")

Example Request

{
    "equipment": {
        "name": "test equipment",
        "fields": [
            {
                "hull_color": "white"
            },
            {
                "beam": "29"
            }
        ]
    }
}

[codymullins]

After further testing, I have discovered this only throws this error when combining the map with the repeated above. When removing the repeated and only using a map, it does appear to work correctly EXCEPT the deserialization is returned as an array of [null null] instead of a dictionary with the values.

[codymullins]

It appears the reason for null, null is due to the switch statement in the ToJson(...) method.

Since there is no mapping, the default case is triggered. This is fixed locally by adding a case for KeyValuePair<object, object>.

Example PR

case KeyValuePair<object, object> pair:
    return JsonValue.Create(pair);
default: // don't know what to do here
    return null;

That said, this does not fix the fact the fields come back as an IEnumerable instead of an IDictionary. Still looking in to this aspect, so any direction would be helpful.

PS - sorry for all the updates on this ticket as I investigate!

[xoofx]

Just wondering, but shouldn't the request be:

{
    "equipment": {
        "name": "test equipment",
        "fields": [
            {  
                "values": { 
                    "hull_color": "white"
                }
            },
            {
                "values": { 
                    "beam": "29"
                }
            }
        ]
    }
}

(values is a dictionary within the repeated fields, so it should be present)

[codymullins]

It's been a few weeks since I looked at this, I'll take another look to sanity check...

[codymullins]

Alright, so I'm no longer utilizing the repeated map<string, string>, but this issue is still relevant for map<string, string> and using the Attributes wrapping the map type.

message Equipment {
	string Id = 1;
	string name = 2;
	EquipmentModel model = 3;

	// is affected by the null/null issue
	map<string, string> fields = 4;

	// not currently using, but appears to throw an error
	repeated .equipment.Attributes attributes = 5;

	// is affected by the null,null issue
	.equipment.Attributes attributes2 = 6;
}

Without the addition to the switch handling KeyValuePair in the Spacetime API client:

Adding handling for KeyValuePair:

I also just confirmed the same issue is present in the cli:

C:\Users\Cody>grpc-curl --json -d "{""equipment"":{""name"":""seas the day"",""fields"":{""hull_color"":""white"",""beam"":""29""},""attributes2"":{""values"":{""hull_color"":""white"",""beam"":""29""}}}}" http://localhost:5287 equipment.EquipmentService/AddEquipment
{
  "equipment": {
    "Id": "2b813d84-540b-4da3-84be-732b4d97c012",
    "name": "seas the day",
    "fields": [
      null,
      null
    ],
    "attributes2": {
      "values": [
        null,
        null
      ]
    }
  }
}

[xoofx]

hm, thanks for this test... I still don't understand why KeyValuePair is involved... but definitely looks like map is not handled correctly.