Reflection - allow generating multi services with their shared classes defined once. (#209)

meirkr · Meir Kriheli · web-flow · commit 51482788f5e2 · 2021-12-15T13:16:47.000Z
* allow generating multo services with their shared classes defined once.
Added more unit tests to cover this feature.
Added an addition unit test of exception in case schema gets a non service contract interface/class.

* code review - don't break backward (runtime) compatibility.
Add back the existing GetSchema with singele type as argument, which actually calls to the new form of params types.

* add attention remarks no to delete the method although it looks as covered by its following overloading method.

Co-authored-by: Meir Kriheli &lt;meir-kr@orbotech.com&gt;
diff --git a/src/protobuf-net.Grpc.Reflection/SchemaGenerator.cs b/src/protobuf-net.Grpc.Reflection/SchemaGenerator.cs
@@ -3,6 +3,7 @@
 using ProtoBuf.Grpc.Internal;
 using ProtoBuf.Meta;
 using System;
+using System.Collections.Generic;
 using System.Reflection;
 
 namespace ProtoBuf.Grpc.Reflection
@@ -25,67 +26,100 @@ public sealed class SchemaGenerator
         /// <summary>
         /// Get the .proto schema associated with a service contract
         /// </summary>
+        /// <typeparam name="TService">The service type to generate schema for.</typeparam>
         /// <remarks>This API is considered experimental and may change slightly</remarks>
         public string GetSchema<TService>()
             => GetSchema(typeof(TService));
 
         /// <summary>
         /// Get the .proto schema associated with a service contract
         /// </summary>
-        /// <remarks>This API is considered experimental and may change slightly</remarks>
+        /// <param name="contractType">The service type to generate schema for.</param>
+        /// <remarks>This API is considered experimental and may change slightly.
+        /// ATTENTION! although the 'GetSchema(params Type[] contractTypes)' covers also a case of 'GetSchema(Type contractType)',
+        /// this method need to remain for backward compatibility for client which will get this updated version, without recompilation.
+        /// Thus, this method mustn't be deleted.</remarks>
         public string GetSchema(Type contractType)
+            => GetSchema(new [] {contractType});
+
+        /// <summary>
+        /// Get the .proto schema associated with multiple service contracts
+        /// </summary>
+        /// <param name="contractTypes">Array (or params syntax) of service types to generate schema for.</param>
+        /// <remarks>This API is considered experimental and may change slightly
+        /// All types will be generated into single schema.
+        /// All the shared classes the services use will be generated only once for all of them.</remarks>
+        public string GetSchema(params Type[] contractTypes)
         {
+            string globalPackage = "";
+            List<Service> services = new List<Service>();  
             var binderConfiguration = BinderConfiguration ?? BinderConfiguration.Default;
             var binder = binderConfiguration.Binder;
-            if (!binder.IsServiceContract(contractType, out var name))
+            foreach (var contractType in contractTypes)
             {
-                throw new ArgumentException($"Type '{contractType.Name}' is not a service contract", nameof(contractType));
-            }
+                if (!binder.IsServiceContract(contractType, out var name))
+                {
+                    throw new ArgumentException($"Type '{contractType.Name}' is not a service contract",
+                        nameof(contractTypes));
+                }
 
-            name = ServiceBinder.GetNameParts(name, contractType, out var package);
-            var service = new Service
-            {
-                Name = name
-            };
-            var ops = contractType.GetMethods(BindingFlags.Public | BindingFlags.Instance);
-            foreach (var method in ops)
-            {
-                if (method.DeclaringType == typeof(object))
-                { /* skip */ }
-                else if (ContractOperation.TryIdentifySignature(method, binderConfiguration, out var op, null))
+                name = ServiceBinder.GetNameParts(name, contractType, out var package);
+                // currently we allow only services from same package, to be output to single proto file
+                if (!string.IsNullOrEmpty(globalPackage)
+                    && package != globalPackage)
                 {
-                    service.Methods.Add(
-                        new ServiceMethod
-                        {
-                            Name = op.Name,
-                            InputType = ApplySubstitutes(op.From),
-                            OutputType = ApplySubstitutes(op.To),
-                            ClientStreaming = op.MethodType switch
-                            {
-                                MethodType.ClientStreaming => true,
-                                MethodType.DuplexStreaming => true,
-                                _ => false,
-                            },
-                            ServerStreaming = op.MethodType switch
+                    throw new ArgumentException(
+                        $"All services must be of the same package! '{contractType.Name}' is from package '{package}' while previous package: {globalPackage}",
+                        nameof(contractTypes));
+                }
+                globalPackage = package;
+                
+                var service = new Service
+                {
+                    Name = name
+                };
+                var ops = contractType.GetMethods(BindingFlags.Public | BindingFlags.Instance);
+                foreach (var method in ops)
+                {
+                    if (method.DeclaringType == typeof(object))
+                    {
+                        /* skip */
+                    }
+                    else if (ContractOperation.TryIdentifySignature(method, binderConfiguration, out var op, null))
+                    {
+                        service.Methods.Add(
+                            new ServiceMethod
                             {
-                                MethodType.ServerStreaming => true,
-                                MethodType.DuplexStreaming => true,
-                                _ => false,
-                            },
-                        }
-                    );
+                                Name = op.Name,
+                                InputType = ApplySubstitutes(op.From),
+                                OutputType = ApplySubstitutes(op.To),
+                                ClientStreaming = op.MethodType switch
+                                {
+                                    MethodType.ClientStreaming => true,
+                                    MethodType.DuplexStreaming => true,
+                                    _ => false,
+                                },
+                                ServerStreaming = op.MethodType switch
+                                {
+                                    MethodType.ServerStreaming => true,
+                                    MethodType.DuplexStreaming => true,
+                                    _ => false,
+                                },
+                            }
+                        );
+                    }
                 }
+
+                service.Methods.Sort((x, y) => string.Compare(x.Name, y.Name)); // make it predictable
+                services.Add(service);
             }
-            service.Methods.Sort((x, y) => string.Compare(x.Name, y.Name)); // make it predictable
+
             var options = new SchemaGenerationOptions
             {
                 Syntax = ProtoSyntax,
-                Package = package,
-                Services =
-                {
-                    service
-                }
+                Package = globalPackage,
             };
+            options.Services.AddRange(services);
 
             var model = binderConfiguration.MarshallerCache.TryGetFactory<ProtoBufMarshallerFactory>()?.Model ?? RuntimeTypeModel.Default;
             return model.GetSchema(options);
diff --git a/tests/protobuf-net.Grpc.Reflection.Test/SchemaGeneration.cs b/tests/protobuf-net.Grpc.Reflection.Test/SchemaGeneration.cs
@@ -5,8 +5,8 @@
 using System;
 using System.Collections.Generic;
 using System.Runtime.Serialization;
-using System.ServiceModel;
 using System.Threading.Tasks;
+using protobuf_net.Grpc.Reflection.Test;
 using Xunit;
 using Xunit.Abstractions;
 
@@ -151,5 +151,119 @@ service ConferencesService {
 }
 ", proto, ignoreLineEndingDifferences: true);
         }
+
+        public interface INotAService
+        {
+            ValueTask<MyResponse> SomeMethod1(MyRequest request, CallContext callContext = default);
+        }        
+        [Fact]
+        public void WhenInterfaceIsNotServiceContract_Throw()
+        {
+            var generator = new SchemaGenerator();
+            Action activation = () => generator.GetSchema<INotAService>();
+            Assert.Throws<ArgumentException>(activation.Invoke);
+        }
+        
+        // ReSharper disable once ClassNeverInstantiated.Global
+        public class NotAService
+        {
+            public Task<MyResponse> SomeMethod1(MyRequest request, CallContext callContext = default) 
+                => Task.FromResult(new MyResponse());
+        }        
+        [Fact]
+        public void WhenClassIsNotServiceContract_Throw()
+        {
+            var generator = new SchemaGenerator();
+            Action activation = () => generator.GetSchema<NotAService>();
+            Assert.Throws<ArgumentException>(activation.Invoke);
+        }
+
+        [Service]
+        public interface ISimpleService1
+        {
+            ValueTask<MyResponse> SomeMethod1(MyRequest request, CallContext callContext = default);
+        }
+        [Service]
+        public interface ISimpleService2
+        {
+            ValueTask<MyResponse> SomeMethod2(MyRequest request, CallContext callContext = default);
+        }
+        
+        /// <summary>
+        /// When we have multiple services which share same classes,
+        /// we would like to have a schema which defines those  services
+        /// while having their shared classes defined only once - in that schema.
+        /// The proto schema consumer will generate code (in any language) while the classes are common to the several services
+        /// and can be reused towards multiple services, the same way the code-first uses those shared classes.. 
+        /// </summary>
+        [Fact]
+        public void MultiServicesInSameSchema_ServicesAreFromSameNamespace_Success()
+        {
+            var generator = new SchemaGenerator();
+            
+            var proto =  generator.GetSchema(typeof(ISimpleService1), typeof(ISimpleService2));
+            
+            Assert.Equal(@"syntax = ""proto3"";
+package protobuf_net.Grpc.Reflection.Test;
+import ""google/protobuf/timestamp.proto"";
+
+enum Category {
+   Default = 0;
+   Foo = 1;
+   Bar = 2;
+}
+message MyRequest {
+   int32 Id = 1;
+   .google.protobuf.Timestamp When = 2;
+}
+message MyResponse {
+   string Value = 1;
+   Category Category = 2;
+   string RefId = 3; // default value could not be applied: 00000000-0000-0000-0000-000000000000
+}
+service SimpleService1 {
+   rpc SomeMethod1 (MyRequest) returns (MyResponse);
+}
+service SimpleService2 {
+   rpc SomeMethod2 (MyRequest) returns (MyResponse);
+}
+", proto, ignoreLineEndingDifferences: true);            
+        }
+        
+        
+        /// <summary>
+        /// When we have multiple services but with different namespaces,
+        /// since schema should export a single package, this situation is unsupported.
+        /// We expect for an exception.
+        /// </summary>
+        [Fact]
+        public void MultiServicesInSameSchema_ServicesAreFromDifferentNamespaces_Throw()
+        {
+            var generator = new SchemaGenerator();
+            
+            Action activation = () => generator.GetSchema(typeof(DifferentNamespace1.IServiceInNamespace1), typeof(DifferentNamespace2.IServiceInNamespace2));
+            Assert.Throws<ArgumentException>(activation.Invoke);                  
+        }
+    }
+}
+
+
+namespace DifferentNamespace1
+{
+
+    [Service]
+    public interface IServiceInNamespace1
+    {
+        ValueTask<SchemaGeneration.MyResponse> SomeMethod1(SchemaGeneration.MyRequest request, CallContext callContext = default);
     }
 }
+namespace DifferentNamespace2
+{
+    [Service]
+    public interface IServiceInNamespace2
+    {
+        ValueTask<SchemaGeneration.MyResponse> SomeMethod2(SchemaGeneration.MyRequest request, CallContext callContext = default);
+    }
+
+}
+    
