Merge pull request #17889 from jtattermusch/server_service_definition_move

Refactor ServerServiceDefinition and move it to Grpc.Core.Api nuget.

src/csharp/Grpc.Core.Api/ServerServiceDefinition.cs

@@ -18,33 +18,31 @@
 
 using System;
 using System.Collections.Generic;
-using System.Collections.ObjectModel;
-using System.Linq;
-using Grpc.Core.Interceptors;
-using Grpc.Core.Internal;
-using Grpc.Core.Utils;
 
 namespace Grpc.Core
 {
     /// <summary>
-    /// Mapping of method names to server call handlers.
+    /// Stores mapping of methods to server call handlers.
     /// Normally, the <c>ServerServiceDefinition</c> objects will be created by the <c>BindService</c> factory method 
     /// that is part of the autogenerated code for a protocol buffers service definition.
     /// </summary>
     public class ServerServiceDefinition
     {
-        readonly ReadOnlyDictionary<string, IServerCallHandler> callHandlers;
+        readonly IReadOnlyList<Action<ServiceBinderBase>> addMethodActions;
 
-        internal ServerServiceDefinition(Dictionary<string, IServerCallHandler> callHandlers)
+        internal ServerServiceDefinition(List<Action<ServiceBinderBase>> addMethodActions)
         {
-            this.callHandlers = new ReadOnlyDictionary<string, IServerCallHandler>(callHandlers);
+            this.addMethodActions = addMethodActions.AsReadOnly();
         }
 
-        internal IDictionary<string, IServerCallHandler> CallHandlers
+        /// <summary>
+        /// Forwards all the previously stored <c>AddMethod</c> calls to the service binder.
+        /// </summary>
+        internal void BindService(ServiceBinderBase serviceBinder)
         {
-            get
+            foreach (var addMethodAction in addMethodActions)
             {
-                return this.callHandlers;
+                addMethodAction(serviceBinder);
             }
         }
 
@@ -62,7 +60,10 @@ namespace Grpc.Core
         /// </summary>
         public class Builder
         {
-            readonly Dictionary<string, IServerCallHandler> callHandlers = new Dictionary<string, IServerCallHandler>();
+            // to maintain legacy behavior, we need to detect duplicate keys and throw the same exception as before
+            readonly Dictionary<string, object> duplicateDetector = new Dictionary<string, object>();
+            // for each AddMethod call, we store an action that will later register the method and handler with ServiceBinderBase
+            readonly List<Action<ServiceBinderBase>> addMethodActions = new List<Action<ServiceBinderBase>>();
 
             /// <summary>
             /// Creates a new instance of builder.
@@ -85,7 +86,8 @@ namespace Grpc.Core
                     where TRequest : class
                     where TResponse : class
             {
-                callHandlers.Add(method.FullName, ServerCalls.UnaryCall(method, handler));
+                duplicateDetector.Add(method.FullName, null);
+                addMethodActions.Add((serviceBinder) => serviceBinder.AddMethod(method, handler));
                 return this;
             }
 
@@ -103,7 +105,8 @@ namespace Grpc.Core
                     where TRequest : class
                     where TResponse : class
             {
-                callHandlers.Add(method.FullName, ServerCalls.ClientStreamingCall(method, handler));
+                duplicateDetector.Add(method.FullName, null);
+                addMethodActions.Add((serviceBinder) => serviceBinder.AddMethod(method, handler));
                 return this;
             }
 
@@ -121,7 +124,8 @@ namespace Grpc.Core
                     where TRequest : class
                     where TResponse : class
             {
-                callHandlers.Add(method.FullName, ServerCalls.ServerStreamingCall(method, handler));
+                duplicateDetector.Add(method.FullName, null);
+                addMethodActions.Add((serviceBinder) => serviceBinder.AddMethod(method, handler));
                 return this;
             }
 
@@ -139,7 +143,8 @@ namespace Grpc.Core
                     where TRequest : class
                     where TResponse : class
             {
-                callHandlers.Add(method.FullName, ServerCalls.DuplexStreamingCall(method, handler));
+                duplicateDetector.Add(method.FullName, null);
+                addMethodActions.Add((serviceBinder) => serviceBinder.AddMethod(method, handler));
                 return this;
             }
 
@@ -149,7 +154,7 @@ namespace Grpc.Core
             /// <returns>The <c>ServerServiceDefinition</c> object.</returns>
             public ServerServiceDefinition Build()
             {
-                return new ServerServiceDefinition(callHandlers);
+                return new ServerServiceDefinition(addMethodActions);
             }
         }
     }

src/csharp/Grpc.Core.Api/ServiceBinderBase.cs

@@ -20,8 +20,6 @@ using System;
 using System.Collections.Generic;
 using System.Collections.ObjectModel;
 using System.Linq;
-using Grpc.Core.Interceptors;
-using Grpc.Core.Internal;
 using Grpc.Core.Utils;
 
 namespace Grpc.Core
@@ -30,7 +28,6 @@ namespace Grpc.Core
     /// Allows binding server-side method implementations in alternative serving stacks.
     /// Instances of this class are usually populated by the <c>BindService</c> method
     /// that is part of the autogenerated code for a protocol buffers service definition.
-    /// <seealso cref="ServerServiceDefinition"/>
     /// </summary>
     public class ServiceBinderBase
     {

src/csharp/Grpc.Core/ForwardedTypes.cs

@@ -25,7 +25,6 @@ using Grpc.Core.Utils;
 // https://docs.microsoft.com/en-us/dotnet/framework/app-domains/type-forwarding-in-the-common-language-runtime
 
 // TODO(jtattermusch): move types needed for implementing a client
-// TODO(jtattermusch): ServerServiceDefinition depends on IServerCallHandler (which depends on other stuff)
 
 [assembly:TypeForwardedToAttribute(typeof(ILogger))]
 [assembly:TypeForwardedToAttribute(typeof(LogLevel))]
@@ -50,6 +49,8 @@ using Grpc.Core.Utils;
 [assembly:TypeForwardedToAttribute(typeof(ClientStreamingServerMethod<,>))]
 [assembly:TypeForwardedToAttribute(typeof(ServerStreamingServerMethod<,>))]
 [assembly:TypeForwardedToAttribute(typeof(DuplexStreamingServerMethod<,>))]
+[assembly:TypeForwardedToAttribute(typeof(ServerServiceDefinition))]
+[assembly:TypeForwardedToAttribute(typeof(ServiceBinderBase))]
 [assembly:TypeForwardedToAttribute(typeof(Status))]
 [assembly:TypeForwardedToAttribute(typeof(StatusCode))]
 [assembly:TypeForwardedToAttribute(typeof(WriteOptions))]

src/csharp/Grpc.Core/Interceptors/ServerServiceDefinitionExtensions.cs

@@ -44,7 +44,10 @@ namespace Grpc.Core.Interceptors
         {
             GrpcPreconditions.CheckNotNull(serverServiceDefinition, nameof(serverServiceDefinition));
             GrpcPreconditions.CheckNotNull(interceptor, nameof(interceptor));
-            return new ServerServiceDefinition(serverServiceDefinition.CallHandlers.ToDictionary(x => x.Key, x => x.Value.Intercept(interceptor)));
+
+            var binder = new InterceptingServiceBinder(interceptor);
+            serverServiceDefinition.BindService(binder);
+            return binder.GetInterceptedServerServiceDefinition();
         }
 
         /// <summary>
@@ -75,5 +78,52 @@ namespace Grpc.Core.Interceptors
 
             return serverServiceDefinition;
         }
+
+        /// <summary>
+        /// Helper for creating <c>ServerServiceDefinition</c> with intercepted handlers.
+        /// </summary>
+        private class InterceptingServiceBinder : ServiceBinderBase
+        {
+            readonly ServerServiceDefinition.Builder builder = ServerServiceDefinition.CreateBuilder();
+            readonly Interceptor interceptor;
+
+            public InterceptingServiceBinder(Interceptor interceptor)
+            {
+                this.interceptor = GrpcPreconditions.CheckNotNull(interceptor, nameof(interceptor));
+            }
+
+            internal ServerServiceDefinition GetInterceptedServerServiceDefinition()
+            {
+                return builder.Build();
+            }
+
+            public override void AddMethod<TRequest, TResponse>(
+                Method<TRequest, TResponse> method,
+                UnaryServerMethod<TRequest, TResponse> handler)
+            {
+                builder.AddMethod(method, (request, context) => interceptor.UnaryServerHandler(request, context, handler));
+            }
+
+            public override void AddMethod<TRequest, TResponse>(
+                Method<TRequest, TResponse> method,
+                ClientStreamingServerMethod<TRequest, TResponse> handler)
+            {
+                builder.AddMethod(method, (requestStream, context) => interceptor.ClientStreamingServerHandler(requestStream, context, handler));
+            }
+
+            public override void AddMethod<TRequest, TResponse>(
+                Method<TRequest, TResponse> method,
+                ServerStreamingServerMethod<TRequest, TResponse> handler)
+            {
+                builder.AddMethod(method, (request, responseStream, context) => interceptor.ServerStreamingServerHandler(request, responseStream, context, handler));
+            }
+
+            public override void AddMethod<TRequest, TResponse>(
+                Method<TRequest, TResponse> method,
+                DuplexStreamingServerMethod<TRequest, TResponse> handler)
+            {
+                builder.AddMethod(method, (requestStream, responseStream, context) => interceptor.DuplexStreamingServerHandler(requestStream, responseStream, context, handler));
+            }
+        }
     }
 }

src/csharp/Grpc.Core/Internal/ServerCallHandler.cs

@@ -31,7 +31,6 @@ namespace Grpc.Core.Internal
     internal interface IServerCallHandler
     {
         Task HandleCall(ServerRpcNew newRpc, CompletionQueueSafeHandle cq);
-        IServerCallHandler Intercept(Interceptor interceptor);
     }
 
     internal class UnaryServerCallHandler<TRequest, TResponse> : IServerCallHandler
@@ -91,11 +90,6 @@ namespace Grpc.Core.Internal
             }
             await finishedTask.ConfigureAwait(false);
         }
-
-        public IServerCallHandler Intercept(Interceptor interceptor)
-        {
-            return new UnaryServerCallHandler<TRequest, TResponse>(method, (request, context) => interceptor.UnaryServerHandler(request, context, handler));
-        }
     }
 
     internal class ServerStreamingServerCallHandler<TRequest, TResponse> : IServerCallHandler
@@ -154,11 +148,6 @@ namespace Grpc.Core.Internal
             }
             await finishedTask.ConfigureAwait(false);
         }
-
-        public IServerCallHandler Intercept(Interceptor interceptor)
-        {
-            return new ServerStreamingServerCallHandler<TRequest, TResponse>(method, (request, responseStream, context) => interceptor.ServerStreamingServerHandler(request, responseStream, context, handler));
-        }
     }
 
     internal class ClientStreamingServerCallHandler<TRequest, TResponse> : IServerCallHandler
@@ -217,11 +206,6 @@ namespace Grpc.Core.Internal
             }
             await finishedTask.ConfigureAwait(false);
         }
-
-        public IServerCallHandler Intercept(Interceptor interceptor)
-        {
-            return new ClientStreamingServerCallHandler<TRequest, TResponse>(method, (requestStream, context) => interceptor.ClientStreamingServerHandler(requestStream, context, handler));
-        }
     }
 
     internal class DuplexStreamingServerCallHandler<TRequest, TResponse> : IServerCallHandler
@@ -277,11 +261,6 @@ namespace Grpc.Core.Internal
             }
             await finishedTask.ConfigureAwait(false);
         }
-
-        public IServerCallHandler Intercept(Interceptor interceptor)
-        {
-            return new DuplexStreamingServerCallHandler<TRequest, TResponse>(method, (requestStream, responseStream, context) => interceptor.DuplexStreamingServerHandler(requestStream, responseStream, context, handler));
-        }
     }
 
     internal class UnimplementedMethodCallHandler : IServerCallHandler
@@ -310,11 +289,6 @@ namespace Grpc.Core.Internal
         {
             return callHandlerImpl.HandleCall(newRpc, cq);
         }
-
-        public IServerCallHandler Intercept(Interceptor interceptor)
-        {
-            return this;  // Do not intercept unimplemented methods.
-        }
     }
 
     internal static class HandlerUtils

src/csharp/Grpc.Core/Internal/ServerServiceDefinitionExtensions.cs

@@ -0,0 +1,78 @@
+#region Copyright notice and license
+
+// Copyright 2019 The gRPC Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#endregion
+
+using System.Collections.Generic;
+using System.Collections.ObjectModel;
+using Grpc.Core.Internal;
+
+namespace Grpc.Core.Internal
+{
+    internal static class ServerServiceDefinitionExtensions
+    {
+        /// <summary>
+        /// Maps methods from <c>ServerServiceDefinition</c> to server call handlers.
+        /// </summary>
+        internal static ReadOnlyDictionary<string, IServerCallHandler> GetCallHandlers(this ServerServiceDefinition serviceDefinition)
+        {
+            var binder = new DefaultServiceBinder();
+            serviceDefinition.BindService(binder);
+            return binder.GetCallHandlers();
+        }
+
+        /// <summary>
+        /// Helper for converting <c>ServerServiceDefinition</c> to server call handlers.
+        /// </summary>
+        private class DefaultServiceBinder : ServiceBinderBase
+        {
+            readonly Dictionary<string, IServerCallHandler> callHandlers = new Dictionary<string, IServerCallHandler>();
+
+            internal ReadOnlyDictionary<string, IServerCallHandler> GetCallHandlers()
+            {
+                return new ReadOnlyDictionary<string, IServerCallHandler>(this.callHandlers);
+            }
+
+            public override void AddMethod<TRequest, TResponse>(
+                Method<TRequest, TResponse> method,
+                UnaryServerMethod<TRequest, TResponse> handler)
+            {
+                callHandlers.Add(method.FullName, ServerCalls.UnaryCall(method, handler));
+            }
+
+            public override void AddMethod<TRequest, TResponse>(
+                Method<TRequest, TResponse> method,
+                ClientStreamingServerMethod<TRequest, TResponse> handler)
+            {
+                callHandlers.Add(method.FullName, ServerCalls.ClientStreamingCall(method, handler));
+            }
+
+            public override void AddMethod<TRequest, TResponse>(
+                Method<TRequest, TResponse> method,
+                ServerStreamingServerMethod<TRequest, TResponse> handler)
+            {
+                callHandlers.Add(method.FullName, ServerCalls.ServerStreamingCall(method, handler));
+            }
+
+            public override void AddMethod<TRequest, TResponse>(
+                Method<TRequest, TResponse> method,
+                DuplexStreamingServerMethod<TRequest, TResponse> handler)
+            {
+                callHandlers.Add(method.FullName, ServerCalls.DuplexStreamingCall(method, handler));
+            }
+        }
+    }
+}

src/csharp/Grpc.Core/Server.cs

@@ -257,7 +257,7 @@ namespace Grpc.Core
             lock (myLock)
             {
                 GrpcPreconditions.CheckState(!startRequested);
-                foreach (var entry in serviceDefinition.CallHandlers)
+                foreach (var entry in serviceDefinition.GetCallHandlers())
                 {
                     callHandlers.Add(entry.Key, entry.Value);
                 }