Merge branch 'meirkr-meirk/reflection_inherited_and_generics_methods'

Author: mgravell

src/protobuf-net.Grpc.Reflection/SchemaGenerator.cs

@@ -4,6 +4,7 @@
 using ProtoBuf.Meta;
 using System;
 using System.Collections.Generic;
+using System.Linq;
 using System.Reflection;
 
 namespace ProtoBuf.Grpc.Reflection
@@ -78,7 +79,8 @@ public string GetSchema(params Type[] contractTypes)
                 {
                     Name = name
                 };
-                var ops = contractType.GetMethods(BindingFlags.Public | BindingFlags.Instance);
+            
+                var ops = GetMethodsRecursively(binder, contractType);
                 foreach (var method in ops)
                 {
                     if (method.DeclaringType == typeof(object))
@@ -134,5 +136,16 @@ static Type ApplySubstitutes(Type type)
                 return type;
             }
         }
+
+        private static MethodInfo[] GetMethodsRecursively(ServiceBinder serviceBinder, Type contractType)
+        {
+            var includingInheritedInterfaces = ContractOperation.ExpandWithInterfacesMarkedAsSubService(serviceBinder, contractType);
+
+            var inheritedMethods = includingInheritedInterfaces
+                .SelectMany(t => t.GetMethods(BindingFlags.Public | BindingFlags.Instance))
+                .ToArray();
+            
+            return inheritedMethods;
+        }
     }
 }

src/protobuf-net.Grpc/Configuration/ServerBinder.cs

@@ -24,57 +24,77 @@ are observed and respected
         /// <summary>
         /// Initiate a bind operation, causing all service methods to be crawled for the provided type
         /// </summary>
-        public int Bind<TService>(object state, BinderConfiguration? binderConfiguration = null, TService? service = null)
+        public int Bind<TService>(object state, BinderConfiguration? binderConfiguration = null,
+            TService? service = null)
             where TService : class
             => Bind(state, typeof(TService), binderConfiguration, service);
 
         /// <summary>
         /// Initiate a bind operation, causing all service methods to be crawled for the provided type
         /// </summary>
-        public int Bind(object state, Type serviceType, BinderConfiguration? binderConfiguration = null, object? service = null)
+        public int Bind(object state, Type serviceType, BinderConfiguration? binderConfiguration = null,
+            object? service = null)
         {
             int totalCount = 0;
             object?[]? argsBuffer = null;
             Type[] typesBuffer = Array.Empty<Type>();
-            string? serviceName;
-            if (binderConfiguration == null) binderConfiguration = BinderConfiguration.Default;
-            var serviceContracts = typeof(IGrpcService).IsAssignableFrom(serviceType)
-                ? new HashSet<Type> { serviceType }
+            binderConfiguration ??= BinderConfiguration.Default;
+            var potentialServiceContracts = typeof(IGrpcService).IsAssignableFrom(serviceType)
+                ? new HashSet<Type> {serviceType}
                 : ContractOperation.ExpandInterfaces(serviceType);
 
             bool serviceImplSimplifiedExceptions = serviceType.IsDefined(typeof(SimpleRpcExceptionsAttribute));
-            foreach (var serviceContract in serviceContracts)
+            foreach (var potentialServiceContract in potentialServiceContracts)
             {
-                if (!binderConfiguration.Binder.IsServiceContract(serviceContract, out serviceName)) continue;
+                if (!binderConfiguration.Binder.IsServiceContract(potentialServiceContract, out var serviceName)) continue;
+
+                // now that we know it is a service contract type for sure
+                var serviceContract = potentialServiceContract;
+                
+                var typesToBeIncludedInMethodsBinding =
+                    ContractOperation.ExpandWithInterfacesMarkedAsSubService(binderConfiguration.Binder, serviceContract);
+
+                // Per service contract, we will collect all the methods of sub-service interfaces
+                // and bind them as they were defined in the service contract itself.
+                // (their binding key will be based on the service contract and not based on the base-interfaces).
 
-                var serviceContractSimplifiedExceptions = serviceImplSimplifiedExceptions || serviceContract.IsDefined(typeof(SimpleRpcExceptionsAttribute));
                 int svcOpCount = 0;
-                var bindCtx = new ServiceBindContext(serviceContract, serviceType, state, binderConfiguration.Binder);
-                foreach (var op in ContractOperation.FindOperations(binderConfiguration, serviceContract, this))
+                foreach (var typeToBindItsMethods in typesToBeIncludedInMethodsBinding)
                 {
-                    if (ServerInvokerLookup.TryGetValue(op.MethodType, op.Context, op.Arg, op.Result, op.Void, out var invoker)
-                        && AddMethod(op.From, op.To, op.Name, op.Method, op.MethodType, invoker, bindCtx,
-                        serviceContractSimplifiedExceptions || op.Method.IsDefined(typeof(SimpleRpcExceptionsAttribute))
-                        ))
+                    var serviceContractSimplifiedExceptions = serviceImplSimplifiedExceptions ||
+                                                              typeToBindItsMethods.IsDefined(
+                                                                  typeof(SimpleRpcExceptionsAttribute));
+                    var bindCtx = new ServiceBindContext(serviceContract, serviceType, state, binderConfiguration.Binder);
+                    foreach (var op in ContractOperation.FindOperations(binderConfiguration, typeToBindItsMethods, this))
                     {
-                        // yay!
-                        totalCount++;
-                        svcOpCount++;
+                        if (ServerInvokerLookup.TryGetValue(op.MethodType, op.Context, op.Arg, op.Result, op.Void, out var invoker)
+                            && AddMethod(serviceName, op.From, op.To, op.Name, op.Method, op.MethodType, invoker, bindCtx,
+                                serviceContractSimplifiedExceptions || op.Method.IsDefined(typeof(SimpleRpcExceptionsAttribute))
+                            ))
+                        {
+                            // yay!
+                            totalCount++;
+                            svcOpCount++;
+                        }
                     }
                 }
+
                 OnServiceBound(state, serviceName!, serviceType, serviceContract, svcOpCount);
             }
+
             return totalCount;
 
-            bool AddMethod(Type @in, Type @out, string on, MethodInfo m, MethodType t,
-                Func<MethodInfo, ParameterExpression[], Expression>? invoker, ServiceBindContext bindContext, bool simplifiedExceptionHandling)
+            bool AddMethod(string? serviceName, Type @in, Type @out, string on, MethodInfo m, MethodType t,
+                Func<MethodInfo, ParameterExpression[], Expression>? invoker, ServiceBindContext bindContext,
+                bool simplifiedExceptionHandling)
             {
                 try
                 {
                     if (typesBuffer.Length == 0)
                     {
-                        typesBuffer = new Type[] { serviceType, typeof(void), typeof(void) };
+                        typesBuffer = new Type[] {serviceType, typeof(void), typeof(void)};
                     }
+
                     typesBuffer[1] = @in;
                     typesBuffer[2] = @out;
 
@@ -84,6 +104,7 @@ bool AddMethod(Type @in, Type @out, string on, MethodInfo m, MethodType t,
                         argsBuffer[6] = binderConfiguration!.MarshallerCache;
                         argsBuffer[7] = service is null ? null : Expression.Constant(service, serviceType);
                     }
+
                     argsBuffer[0] = serviceName;
                     argsBuffer[1] = on;
                     argsBuffer[2] = m;
@@ -93,7 +114,7 @@ bool AddMethod(Type @in, Type @out, string on, MethodInfo m, MethodType t,
                     // 6, 7 set during array initialization
                     argsBuffer[8] = simplifiedExceptionHandling;
 
-                    return (bool)s_addMethod.MakeGenericMethod(typesBuffer).Invoke(this, argsBuffer)!;
+                    return (bool) s_addMethod.MakeGenericMethod(typesBuffer).Invoke(this, argsBuffer)!;
                 }
                 catch (Exception fail)
                 {
@@ -128,7 +149,8 @@ protected readonly struct MethodStub<TService>
             /// </summary>
             public MethodInfo Method { get; }
 
-            internal MethodStub(Func<MethodInfo, Expression[], Expression>? invoker, MethodInfo method, ConstantExpression? service, bool simpleExceptionHandling)
+            internal MethodStub(Func<MethodInfo, Expression[], Expression>? invoker, MethodInfo method,
+                ConstantExpression? service, bool simpleExceptionHandling)
             {
                 _simpleExceptionHandling = simpleExceptionHandling;
                 _invoker = invoker;
@@ -158,7 +180,7 @@ public TDelegate CreateDelegate<TDelegate>()
                     else
                     {
                         // basic - direct call
-                        return (TDelegate)Delegate.CreateDelegate(typeof(TDelegate), _service, Method);
+                        return (TDelegate) Delegate.CreateDelegate(typeof(TDelegate), _service, Method);
                     }
                 }
                 else
@@ -167,7 +189,8 @@ public TDelegate CreateDelegate<TDelegate>()
 
                     Expression[] mapArgs;
                     if (_service is null)
-                    {   // if no service object, then the service is part of the signature, i.e. (svc, req) => svc.Blah();
+                    {
+                        // if no service object, then the service is part of the signature, i.e. (svc, req) => svc.Blah();
                         mapArgs = lambdaArgs;
                     }
                     else
@@ -184,6 +207,7 @@ public TDelegate CreateDelegate<TDelegate>()
                     {
                         body = ApplySimpleExceptionHandling(body);
                     }
+
                     var lambda = Expression.Lambda<TDelegate>(body, lambdaArgs);
 
                     return lambda.Compile();
@@ -199,17 +223,19 @@ static Expression ApplySimpleExceptionHandling(Expression body)
                 }
                 else if (type.IsGenericType && type.GetGenericTypeDefinition() == typeof(Task<>))
                 {
-                    body = Expression.Call(s_ReshapeWithSimpleExceptionHandling[1].MakeGenericMethod(type.GetGenericArguments()), body);
+                    body = Expression.Call(
+                        s_ReshapeWithSimpleExceptionHandling[1].MakeGenericMethod(type.GetGenericArguments()), body);
                 }
+
                 return body;
             }
         }
 
 #pragma warning disable CS0618
         private static readonly Dictionary<int, MethodInfo> s_ReshapeWithSimpleExceptionHandling =
             (from method in typeof(Reshape).GetMethods(BindingFlags.Public | BindingFlags.Static)
-             where method.Name is nameof(Reshape.WithSimpleExceptionHandling)
-             select method)
+                where method.Name is nameof(Reshape.WithSimpleExceptionHandling)
+                select method)
             .ToDictionary(method => method.IsGenericMethodDefinition ? method.GetGenericArguments().Length : 0);
 #pragma warning restore CS0618
 
@@ -222,7 +248,8 @@ private bool AddMethod<TService, TRequest, TResponse>(
             where TRequest : class
             where TResponse : class
         {
-            var grpcMethod = new Method<TRequest, TResponse>(methodType, serviceName, operationName, marshallerCache.GetMarshaller<TRequest>(), marshallerCache.GetMarshaller<TResponse>());
+            var grpcMethod = new Method<TRequest, TResponse>(methodType, serviceName, operationName,
+                marshallerCache.GetMarshaller<TRequest>(), marshallerCache.GetMarshaller<TResponse>());
             var stub = new MethodStub<TService>(invoker, method, service, simplfiedExceptionHandling);
             try
             {
@@ -235,13 +262,11 @@ private bool AddMethod<TService, TRequest, TResponse>(
                 OnError(ex.Message);
                 return false;
             }
-
         }
 
         void IBindContext.LogWarning(string message, object?[]? args) => OnWarn(message, args);
         void IBindContext.LogError(string message, object?[]? args) => OnError(message, args);
 
-        /// <summary>
         /// Describes the relationship between a service contract and a service definition
         /// </summary>
         protected internal sealed class ServiceBindContext
@@ -283,4 +308,4 @@ public IList<object> GetMetadata(MethodInfo method)
                 => ServiceBinder.GetMetadata(method, ContractType, ServiceType);
         }
     }
-}
+}

src/protobuf-net.Grpc/Configuration/ServiceBinder.cs

@@ -222,5 +222,28 @@ public virtual IList<object> GetMetadata(MethodInfo method, Type contractType, T
             }
             return null;
         }
+
+        /// <summary>
+        /// Attempt to resolve a sub-service
+        /// </summary>
+        /// <param name="type"></param>
+        /// <param name="typesToSearchIn"></param>
+        /// <param name="serviceName"></param>
+        /// <returns></returns>
+        internal bool TryFindInheritedService(Type type, IEnumerable<Type> typesToSearchIn, out string? serviceName)
+        {
+            foreach (var potentialServiceContract in typesToSearchIn)
+            {
+                if (potentialServiceContract != type
+                    && type.IsAssignableFrom(potentialServiceContract)
+                    && IsServiceContract(potentialServiceContract, out serviceName))
+                {
+                    return true;
+                }
+            }
+
+            serviceName = null;
+            return false;
+        }
     }
 }

src/protobuf-net.Grpc/Configuration/SubServiceAttribute.cs

@@ -0,0 +1,16 @@
+using System;
+using System.ComponentModel;
+
+namespace ProtoBuf.Grpc.Configuration
+{
+    /// <summary>
+    /// Indicates that this interface can be inherited by a gRPC service.
+    /// All methods of this interface will be routed based on the top-level service name.
+    /// </summary>
+    /// <remarks>This is particularly useful for genenric service APIs.</remarks>
+    [AttributeUsage(AttributeTargets.Interface, AllowMultiple = false, Inherited = false)]
+    [ImmutableObject(true)]
+    public sealed class SubServiceAttribute : Attribute
+    {
+    }
+}

src/protobuf-net.Grpc/Internal/ContractOperation.cs

@@ -403,6 +403,47 @@ internal static ISet<Type> ExpandInterfaces(Type type)
             if (type.IsInterface) set.Add(type);
             return set;
         }
+
+        /// <summary>
+        /// Collect all the types to be used for extracting methods for a specific Service Contract
+        /// </summary>
+        /// <param name="serviceBinder"></param>
+        /// <param name="serviceContract">Must be a service contract</param>
+        /// <returns>types to be used for extracting methods</returns>
+        internal static ISet<Type> ExpandWithInterfacesMarkedAsSubService(ServiceBinder serviceBinder,
+            Type serviceContract)
+        {
+            var set = new HashSet<Type>();
+            
+            // first add the service contract by itself 
+            set.Add(serviceContract); 
+
+            // now add all inherited interfaces which are marked as sub-services
+            foreach (var t in serviceContract.GetInterfaces())
+            {
+                if (t.IsDefined(typeof(SubServiceAttribute)))
+                {
+                    set.Add(t);
+                }
+            }
+
+            ValidateServiceContracts(serviceBinder, set);
+            return set;
+        }
+
+        private static void ValidateServiceContracts(ServiceBinder serviceBinder, HashSet<Type> set)
+        {
+            foreach (var item in set)
+            {
+                if (item.IsDefined(typeof(SubServiceAttribute)))
+                {
+                    if (serviceBinder.IsServiceContract(item, out var serviceName))
+                        throw new ArgumentException(
+                            $"Bad definition for service {serviceName}: " +
+                            $"A service contract cannot be marked as a sub-service as well");
+                }
+            }
+        }
     }
 
     internal enum ContextKind

src/protobuf-net.Grpc/Internal/ProxyEmitter.cs

@@ -185,7 +185,9 @@ FieldBuilder Marshaller(Type forType)
                 }
 
                 int fieldIndex = 0;
-                foreach (var iType in ContractOperation.ExpandInterfaces(typeof(TService)))
+                var contractExpandInterfaces = ContractOperation.ExpandInterfaces(typeof(TService))
+                    .ToArray();
+                foreach (var iType in contractExpandInterfaces)
                 {
                     bool isService = binderConfig.Binder.IsServiceContract(iType, out var serviceName);
 
@@ -203,11 +205,26 @@ FieldBuilder Marshaller(Type forType)
                         type.DefineMethodOverride(impl, iMethod);
 
                         var il = impl.GetILGenerator();
-                        if (!(isService && ContractOperation.TryIdentifySignature(iMethod, binderConfig, out var op, null)))
+                        
+                        // check whether the method belongs to [ServiceInherited] interface
+                        var isMethodInherited =
+                            iMethod.DeclaringType?.IsDefined(typeof(SubServiceAttribute)) ?? false;
+                        var shallMethodBeImplemented = isService || isMethodInherited;
+                        if (!(shallMethodBeImplemented && ContractOperation.TryIdentifySignature(iMethod, binderConfig, out var op, null)))
                         {
                             il.ThrowException(typeof(NotSupportedException));
                             continue;
                         }
+                        
+                        // in case method belongs to an sub-service interface, we have to find the service contract name inheriting it
+                        if (isMethodInherited)
+                        {
+                            if (!binderConfig.Binder.TryFindInheritedService(iType, contractExpandInterfaces, out serviceName))
+                            {
+                                il.ThrowException(typeof(NotSupportedException));
+                                continue;
+                            }
+                        }
 
                         Type[] fromTo = new Type[] { op.From, op.To };
                         // private static Method<from, to> s_{i}