protobuf-net / protobuf-net.Grpc

GRPC bindings for protobuf-net and grpc-dotnet
Other
857 stars 109 forks source link

Generic method realizations with different types throw AmbiguousMatchException #152

Open RebelionTheGrey opened 3 years ago

RebelionTheGrey commented 3 years ago

Hello.

I have this class implementation

    public interface IReadRepository<T> where T: class
    {
        Task<ServiceObject> Get(ExpressionDTO<T> request);
    }

    public sealed class RouteCRUD : IReadRepository<TradeRoute>, IReadRepository<BettingProfile>, IGrpcService
    {
        public RouteCRUD()
        {
        }

        public async Task<ServiceObject> Get(ExpressionDTO<TradeRoute> request)
        {
               ...
        }

        public async Task<ServiceObject> Get(ExpressionDTO<BettingProfile> request)
        {
               ...
        }
    }

But on method 'Get' calling I get an error like this:

  info: Microsoft.AspNetCore.Hosting.Diagnostics[1]
  Request starting HTTP/2 POST https://localhost:5028/RouteCRUD/Get application/grpc -
  fail: Microsoft.AspNetCore.Server.Kestrel[13]
  Connection id "0HM5NAMO0UNB4", Request id "0HM5NAMO0UNB4:00000005": An unhandled exception was thrown by the application.
  Microsoft.AspNetCore.Routing.Matching.AmbiguousMatchException: The request matched multiple endpoints. Matches:

  gRPC - /RouteCRUD/Get
  gRPC - /RouteCRUD/Get                                                                                                   
  gRPC - Unimplemented method for RouteCRUD
  gRPC - Unimplemented service                                                                                               
     at Microsoft.AspNetCore.Routing.Matching.DefaultEndpointSelector.ReportAmbiguity(CandidateState[] candidateState)
     at Microsoft.AspNetCore.Routing.Matching.DefaultEndpointSelector.ProcessFinalCandidates(HttpContext httpContext, CandidateState[] candidateState)
     at Microsoft.AspNetCore.Routing.Matching.DefaultEndpointSelector.Select(HttpContext httpContext, CandidateState[] candidateState)
     at Microsoft.AspNetCore.Routing.Matching.DfaMatcher.MatchAsync(HttpContext httpContext)
     at Microsoft.AspNetCore.Routing.Matching.DataSourceDependentMatcher.MatchAsync(HttpContext httpContext)
     at Microsoft.AspNetCore.Routing.EndpointRoutingMiddleware.Invoke(HttpContext httpContext)
     at Microsoft.AspNetCore.HostFiltering.HostFilteringMiddleware.Invoke(HttpContext context)
     at Microsoft.AspNetCore.Hosting.HostingApplication.ProcessRequestAsync(Context context)
     at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpProtocol.ProcessRequests[TContext](IHttpApplication`1 application)

info: Microsoft.AspNetCore.Hosting.Diagnostics[2] Request finished HTTP/2 POST https://localhost:5028/RouteCRUD/Get application/grpc - - 500 0 - 55.2336ms

Is it possible to realize generic methods like this or not? And if it possible, so, what should I do for this?

Thank you.

.NET Core - .NET 5.0.1

mgravell commented 3 years ago

No, that isn't really possible right now, as they'd all resolve to the same route. Perhaps what we should do, however, is make better use of the generic args in the routing map. This is, annoyingly, a breaking change - so if someone right now only uses a single T is probably works fine. I need to think of smart ways of making this happen.

On Thu, 14 Jan 2021, 06:39 RebelionTheGrey, notifications@github.com wrote:

                         Hello.

I have this class implementation

public interface IReadRepository<T> where T: class
{
    Task<ServiceObject> Get(ExpressionDTO<T> request);
}

public sealed class RouteCRUD : IReadRepository<TradeRoute>, IReadRepository<BettingProfile>, IGrpcService
{
    public RouteCRUD()
    {
    }

    public async Task<ServiceObject> Get(ExpressionDTO<TradeRoute> request)
    {
           ...
    }

    public async Task<ServiceObject> Get(ExpressionDTO<BettingProfile> request)
    {
           ...
    }
}

But on method 'Get' calling I get an error like this:

info: Microsoft.AspNetCore.Hosting.Diagnostics[1] Request starting HTTP/2 POST https://localhost:5028/RouteCRUD/Get application/grpc - fail: Microsoft.AspNetCore.Server.Kestrel[13] Connection id "0HM5NAMO0UNB4", Request id "0HM5NAMO0UNB4:00000005": An unhandled exception was thrown by the application. Microsoft.AspNetCore.Routing.Matching.AmbiguousMatchException: The request matched multiple endpoints. Matches:

gRPC - /RouteCRUD/Get gRPC - /RouteCRUD/Get gRPC - Unimplemented method for RouteCRUD gRPC - Unimplemented service at Microsoft.AspNetCore.Routing.Matching.DefaultEndpointSelector.ReportAmbiguity(CandidateState[] candidateState) at Microsoft.AspNetCore.Routing.Matching.DefaultEndpointSelector.ProcessFinalCandidates(HttpContext httpContext, CandidateState[] candidateState) at Microsoft.AspNetCore.Routing.Matching.DefaultEndpointSelector.Select(HttpContext httpContext, CandidateState[] candidateState) at Microsoft.AspNetCore.Routing.Matching.DfaMatcher.MatchAsync(HttpContext httpContext) at Microsoft.AspNetCore.Routing.Matching.DataSourceDependentMatcher.MatchAsync(HttpContext httpContext) at Microsoft.AspNetCore.Routing.EndpointRoutingMiddleware.Invoke(HttpContext httpContext) at Microsoft.AspNetCore.HostFiltering.HostFilteringMiddleware.Invoke(HttpContext context) at Microsoft.AspNetCore.Hosting.HostingApplication.ProcessRequestAsync(Context context) at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpProtocol.ProcessRequests[TContext](IHttpApplication`1 application)

info: Microsoft.AspNetCore.Hosting.Diagnostics[2] Request finished HTTP/2 POST https://localhost:5028/RouteCRUD/Get application/grpc - - 500 0 - 55.2336ms

Is it possible to realize generic methods like this or not? And if it possible, so, what should I do for this?

Thank you.

.NET Core - .NET 5.0.1

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/protobuf-net/protobuf-net.Grpc/issues/152, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAEHMHG3RH626FFOWMUQI3SZ2GRVANCNFSM4WB6OIBQ .

RebelionTheGrey commented 3 years ago

Hello, Marc. I've found a possible way to resolve this situation: I need to realize EndpointSelector and put my realization to the DI container. https://docs.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.routing.matching.endpointselector?view=aspnetcore-5.0#definition But it's not so easy, there are too few information about this class or the same interface.(((

mgravell commented 3 years ago

I'm not sure that is a good idea here, as that isn't really how the bindings work for gRPC. What is want to do is generate the route based on the T. Maybe something as simple as

[Service("MyCorp/{0}Repository")] public interface IRepository {...}

would be enough, where the {N} maps to the generic type arguments of the service?

On Thu, 14 Jan 2021, 07:14 RebelionTheGrey, notifications@github.com wrote:

Hello, Mark. I've found a possible way to resolve this situation: I need to realize EndpointSelector and put my realization to the DI container. https://docs.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.routing.matching.endpointselector?view=aspnetcore-5.0#definition But it's not so easy, there are too few information about this class or the same interface.(((

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/protobuf-net/protobuf-net.Grpc/issues/152#issuecomment-759976100, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAEHMD3CSOV4AAZOLT42LDSZ2KU5ANCNFSM4WB6OIBQ .

RebelionTheGrey commented 3 years ago
[Service("MyCorp/{0}Repository")]
public interface IRepository<T> {...}

I'll try this today's evening after come back home.))) Thank you!

RebelionTheGrey commented 3 years ago

I'm not sure that is a good idea here, as that isn't really how the bindings work for gRPC. What is want to do is generate the route based on the T. Maybe something as simple as [Service("MyCorp/{0}Repository")] public interface IRepository {...} would be enough, where the {N} maps to the generic type arguments of the service? On Thu, 14 Jan 2021, 07:14 RebelionTheGrey, @.***> wrote: Hello, Mark. I've found a possible way to resolve this situation: I need to realize EndpointSelector and put my realization to the DI container. https://docs.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.routing.matching.endpointselector?view=aspnetcore-5.0#definition But it's not so easy, there are too few information about this class or the same interface.((( — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#152 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAEHMD3CSOV4AAZOLT42LDSZ2KU5ANCNFSM4WB6OIBQ .

Marc, I've tried to implement the code You wrote to me, but still no effect. I still have the same error. I think I should rewrite EndpointSelector and put it in the DI container... (((

mgravell commented 3 years ago

I still don't think EndpointSelector has any relevance here. In gRPC the routes are bound explicitly to services. What we need to do is influence that code.

On Sat, 16 Jan 2021, 13:56 RebelionTheGrey, notifications@github.com wrote:

I'm not sure that is a good idea here, as that isn't really how the bindings work for gRPC. What is want to do is generate the route based on the T. Maybe something as simple as [Service("MyCorp/{0}Repository")] public interface IRepository {...} would be enough, where the {N} maps to the generic type arguments of the service? … <#m-2564723470629941518> On Thu, 14 Jan 2021, 07:14 RebelionTheGrey, @.***> wrote: Hello, Mark. I've found a possible way to resolve this situation: I need to realize EndpointSelector and put my realization to the DI container. https://docs.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.routing.matching.endpointselector?view=aspnetcore-5.0#definition But it's not so easy, there are too few information about this class or the same interface.((( — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#152 (comment) https://github.com/protobuf-net/protobuf-net.Grpc/issues/152#issuecomment-759976100>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAEHMD3CSOV4AAZOLT42LDSZ2KU5ANCNFSM4WB6OIBQ .

Marc, I've tried to implement the code You wrote to me, but still no effect. I still have the same error. I think I should rewrite EndpointSelector and put it in the DI container... (((

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/protobuf-net/protobuf-net.Grpc/issues/152#issuecomment-761567307, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAEHMALIDFFADZP7D66NS3S2GLKLANCNFSM4WB6OIBQ .

RebelionTheGrey commented 3 years ago

I still don't think EndpointSelector has any relevance here. In gRPC the routes are bound explicitly to services. What we need to do is influence that code.

Oh, that's the real problem... (((

RebelionTheGrey commented 3 years ago

Marc, sorry, but maybe you know any way to identify the method and parameters of the method called through unary call grpc request? I see that I can get RequestMarshaller for each CandidateState from CandidateSet. But I can't understand the way to identify method's parameters. I can't use JsonConvert.Deserialize for request body and use GetType(), because content-type is the application/gprc type, not an application/json type.

mgravell commented 3 years ago

Usually this all works the other way around. The binder defines the route and the marshaller, which means it is defining the parameter type.

It would really help if you took a step back here and gave more info as to what you're trying to achieve, rather than how hour trying to achieve it.

On Sun, 17 Jan 2021, 11:04 RebelionTheGrey, notifications@github.com wrote:

Mark, sorry, but maybe you know any way to identify the method and parameters of the method called through unary call grpc request? I see that I can get RequestMarshaller for each CandidateState from CandidateSet. But I can't understand the way to identify method's parameters. I can't use JsonConvert.Deserialize for request body and use GetType(), because content-type is the application/gprc type, not an application/json type.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/protobuf-net/protobuf-net.Grpc/issues/152#issuecomment-761772277, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAEHMD74BBJ4XZRLREK4KLS2K75NANCNFSM4WB6OIBQ .

RebelionTheGrey commented 3 years ago

Marc, I try to realize my own generic repository.

For example, I have MyDbContext with 2 tables: Table1 and Table2.

public class Table1
{
     public string Field11 { get; set; }
}

public class Table2
{
     public string Field21 { get; set; }
}

public class MyDbContext : DbContext
{
    public DbSet<Table1> Table1Set { get; set;}
    public DbSet<Table2> Table2Set { get; set; }
}

I created an interface:

    public interface IReadRepository<TTable> where TTable : class
    {
        Task<ServiceObject> Get(ExpressionDTO<TTable> request);
    }

and

public class ConcreteGetRealizationClass : IReadRepository<Table1>, IReadRepository<Table2>, IGrpcService
{
      public async Task<ServiceObject> Get(ExpressionDTO<Table1> request) { ... }
      public async Task<ServiceObject> Get(ExpressionDTO<Table2> request) { ... }
}

Here I use Expression serialization for predicates and selectables.

So, I want to create Service which realizes Get method for each Table type in DbContext (I want to realize DbContext parsing and autogeneration for IReadRepository.Get method realization). Also, I want to put in the DI container each IReadRepository<TTable> realization as services.AddScoped<IReadRepository<TTable>, ConcreteGetRealizationClass>().

mgravell commented 3 years ago

OK. Let me play with that scenario and consider options.

RebelionTheGrey commented 3 years ago

Marc, If it interesting for you, I created my own EndpointSelector class realization

    public  sealed class MyDefaultEndpointSelector : EndpointSelector
    {
        public override Task SelectAsync(HttpContext httpContext, CandidateSet candidateSet)
        {
            int CheckCandidate(HttpContext httpContext, CandidateSet candidateSet, int index)
            {
                if (candidateSet.IsValidCandidate(index))
                {
                    httpContext.SetEndpoint(candidateSet[index].Endpoint);
                    httpContext.Request.RouteValues = candidateSet[index].Values!;

                    return 1;
                }

                return 0;
            }

            if (httpContext == null) { throw new ArgumentNullException(nameof(httpContext)); }
            if (candidateSet == null) { throw new ArgumentNullException(nameof(candidateSet)); }

            TypeInfo requestTypeInfo = null, responseTypeInfo = null;

            foreach(var elem in httpContext.Request.Headers)
            {
                if (elem.Key.Equals("requesttype")) //ToDo: make static external name for this field
                {
                    requestTypeInfo = JsonConvert.DeserializeObject<TypeInfo>(elem.Value[0]);
                    Console.WriteLine(requestTypeInfo.ToString());
                }

                if (elem.Key.Equals("responsetype"))  //ToDo: make static external name for this field
                {
                    responseTypeInfo = JsonConvert.DeserializeObject<TypeInfo>(elem.Value[0]);
                    Console.WriteLine(responseTypeInfo.ToString());
                }
            }

            int counter = 0;
            for (int i = 0; i < candidateSet.Count; i++)
            {
                foreach (var obj in candidateSet[i].Endpoint.Metadata)
                {
                    if (obj?.GetType().GetTypeInfo().Equals(typeof(Grpc.AspNetCore.Server.GrpcMethodMetadata)) ?? false)
                    {
                        var grpcMethodMetadata = (Grpc.AspNetCore.Server.GrpcMethodMetadata)obj;

                        if (grpcMethodMetadata.Method.GetType().GetGenericTypeDefinition() == typeof(Grpc.Core.Method<,>))
                        {
                            var genericArguments = grpcMethodMetadata.Method.GetType().GetGenericArguments();
                            var requestType = genericArguments[0];
                            var responseType = genericArguments[1];

                            if ((requestType.GetTypeInfo() == requestTypeInfo || requestTypeInfo.Equals(typeof(object))) && responseType.GetTypeInfo() == responseTypeInfo)
                            {
                                counter += CheckCandidate(httpContext, candidateSet, i);

                                    //break;
                            }
                        }
                    }
                }
            }

            if (counter > 1) { throw new AmbiguousMatchException(string.Format("Found {0} matched methods", counter)); }

            return Task.CompletedTask;
        }
    }   

That works now, but I need to realize a little "hack" on the client side:

        public async Task<TResponse> Execute<TRequest, TResponse>(TRequest request, string serviceName, string methodName) where TRequest  : class
                                                                                                                           where TResponse : class
        {
            object method;

            if (!methods.ContainsKey((typeof(TRequest), typeof(TResponse), serviceName, methodName)))
            {
                var requestMarshaller = marshallerFactory.CreateMarshaller<TRequest>();
                var responseMarshaller = marshallerFactory.CreateMarshaller<TResponse>();

                method = new Method<TRequest, TResponse>(MethodType.Unary, serviceName, methodName,
                                                             requestMarshaller, responseMarshaller);
                methods.TryAdd((typeof(TRequest), typeof(TResponse), serviceName, methodName), method);
            }
            else 
            {
                method = methods.GetValueOrDefault((typeof(TRequest), typeof(TResponse), serviceName, methodName));
            }

            var castedMethod = method as Method<TRequest, TResponse>;

            var metadata = new Metadata
            {
                { "RequestType", JsonConvert.SerializeObject(typeof(TRequest).GetTypeInfo()) },
                { "ResponseType", JsonConvert.SerializeObject(typeof(TResponse).GetTypeInfo()) }
            };

            var typedCallOptions = callOptions.WithHeaders(metadata);

            //using var auc = invoker.AsyncUnaryCall(castedMethod, null, callOptions, request);
            using var auc = invoker.AsyncUnaryCall(castedMethod, null, typedCallOptions, request);

            return await auc.ResponseAsync;
        }

where invoker is private readonly CallInvoker invoker; (this is part of your code with the simplest transformations by me).

Of course, it also needs to register this selector in DI container by services.AddSingleton<EndpointSelector, MyDefaultEndpointSelector>();

Maybe it will be interesting, but of course, I'm waiting for your recommendations.