Merge pull request Cysharp#187 from Cysharp/ImproveFilterApi

feat: Improve Filter APIs

README.md

@@ -304,18 +304,12 @@ MagicOnion filter is powerful feature to hook before-after invoke. It is useful
 // for StreamingHub methods, implement StreamingHubFilterAttribute instead.
 public class SampleFilterAttribute : MagicOnionFilterAttribute
 {
-    // constructor convention rule. requires Func<ServiceContext, Task> next.
-    public SampleFilterAttribute(Func<ServiceContext, Task> next) : base(next) { }
-
-    // other constructor, use base(null)
-    public SampleFilterAttribute() : base(null) { }
-
-    public override async ValueTask Invoke(ServiceContext context)
+    public override async ValueTask Invoke(ServiceContext context, Func<ServiceContext, Task> next)
     {
         try
         {
             /* on before */
-            await Next(context); // next
+            await next(context); // next
             /* on after */
         }
         catch
@@ -552,9 +546,9 @@ MagicOnionOption/Logging
 | --- | --- |
 | `IMagicOnionLogger` MagicOnionLogger | Set the diagnostics info logger. |
 | `bool` DisableEmbeddedService | Disable embedded service(ex:heartbeat), default is false. |
-| `MagicOnionFilterAttribute[]` GlobalFilters | Global MagicOnion filters. |
+| `IList<MagicOnionFilterDescriptor>` GlobalFilters | Global MagicOnion filters. |
 | `bool` EnableCurrentContext | Enable ServiceContext.Current option by AsyncLocal, default is false. |
-| `StreamingHubFilterAttribute[]` Global StreamingHub filters. | GlobalStreamingHubFilters |
+| `IList<StreamingHubFilterDescriptor>` Global StreamingHub filters. | GlobalStreamingHubFilters |
 | `IGroupRepositoryFactory` DefaultGroupRepositoryFactory | Default GroupRepository factory for StreamingHub, default is ``. |
 | `IServiceLocator` ServiceLocator | Add the extra typed option. |
 | `bool` IsReturnExceptionStackTraceInErrorDetail | If true, MagicOnion handles exception ownself and send to message. If false, propagate to gRPC engine. Default is false. |
@@ -1355,14 +1349,16 @@ await MagicOnionHost.CreateDefaultBuilder(useSimpleConsoleLogger: true)
         collection.AddSingleton<ITracer>(Tracing.Tracer);
         collection.AddSingleton<ISampler>(Samplers.AlwaysSample);
     })
-    .UseMagicOnion(
-        new MagicOnionOptions()
+    .UseMagicOnion()
+    .ConfigureServices((hostContext, services) =>
+    {
+        services.Configure<MagicOnionHostingOptions>(options =>
         {
-            GlobalFilters = new[] { new OpenTelemetryCollectorFilter(null) },
-            GlobalStreamingHubFilters = new[] { new OpenTelemetryHubCollectorFilter(null) },
-            MagicOnionLogger = new OpenTelemetryCollectorLogger(Stats.StatsRecorder, Tags.Tagger, null))
-        },
-        new ServerPort("localhost", 12345, ServerCredentials.Insecure))
+             options.Service.GlobalFilters.Add<OpenTelemetryCollectorFilter>();
+             options.Service.GlobalStreamingHubFilters.Add<OpenTelemetryHubCollectorFilter>();
+             options.Service.MagicOnionLogger = new OpenTelemetryCollectorLogger(Stats.StatsRecorder, Tags.Tagger, null);
+        });
+    })
     .RunConsoleAsync();
 ```
 

src/MagicOnion.OpenTelemetry/MagicOnionCollector.cs

@@ -283,18 +283,19 @@ public void WriteToStream(ServiceContext context, byte[] writeData, Type type)
     /// </summary>
     public class OpenTelemetryCollectorFilter : MagicOnionFilterAttribute
     {
-        public OpenTelemetryCollectorFilter(Func<ServiceContext, ValueTask> next) :
-            base(next)
+        readonly ITracer tracer;
+        readonly ISampler sampler;
+
+        public OpenTelemetryCollectorFilter(ITracer tracer, ISampler sampler)
         {
+            this.tracer = tracer;
+            this.sampler = sampler;
         }
 
-        public override async ValueTask Invoke(ServiceContext context)
+        public override async ValueTask Invoke(ServiceContext context, Func<ServiceContext, ValueTask> next)
         {
             // https://github.com/open-telemetry/opentelemetry-specification/blob/master/semantic-conventions.md#grpc
 
-            var tracer = context.ServiceLocator.GetService<ITracer>();
-            var sampler = context.ServiceLocator.GetService<ISampler>();
-
             // span name must be `$package.$service/$method` but MagicOnion has no $package.
             var spanBuilder = tracer.SpanBuilder(context.CallContext.Method, SpanKind.Server);
             if (sampler != null)
@@ -309,7 +310,7 @@ public override async ValueTask Invoke(ServiceContext context)
                     span.SetAttribute("component", "grpc");
                     //span.SetAttribute("request.size", context.GetRawRequest().LongLength);
 
-                    await Next(context);
+                    await next(context);
 
                     //span.SetAttribute("response.size", context.GetRawResponse().LongLength);
                     span.SetAttribute("status_code", (long)context.CallContext.Status.StatusCode);
@@ -332,17 +333,19 @@ public override async ValueTask Invoke(ServiceContext context)
     /// </summary>
     public class OpenTelemetryHubCollectorFilter : StreamingHubFilterAttribute
     {
-        public OpenTelemetryHubCollectorFilter(Func<StreamingHubContext, ValueTask> next) : base(next)
+        readonly ITracer tracer;
+        readonly ISampler sampler;
+
+        public OpenTelemetryHubCollectorFilter(ITracer tracer, ISampler sampler)
         {
+            this.tracer = tracer;
+            this.sampler = sampler;
         }
 
-        public override async ValueTask Invoke(StreamingHubContext context)
+        public override async ValueTask Invoke(StreamingHubContext context, Func<StreamingHubContext, ValueTask> next)
         {
             // https://github.com/open-telemetry/opentelemetry-specification/blob/master/semantic-conventions.md#grpc
 
-            var tracer = context.ServiceContext.ServiceLocator.GetService<ITracer>();
-            var sampler = context.ServiceContext.ServiceLocator.GetService<ISampler>();
-
             // span name must be `$package.$service/$method` but MagicOnion has no $package.
             var spanBuilder = tracer.SpanBuilder(context.ServiceContext.CallContext.Method, SpanKind.Server);
             if (sampler != null)
@@ -357,7 +360,7 @@ public override async ValueTask Invoke(StreamingHubContext context)
                     span.SetAttribute("component", "grpc");
                     //span.SetAttribute("request.size", context.GetRawRequest().LongLength);
 
-                    await Next(context);
+                    await next(context);
 
                     //span.SetAttribute("response.size", context.GetRawResponse().LongLength);
                     span.SetAttribute("status_code", (long)context.ServiceContext.CallContext.Status.StatusCode);

src/MagicOnion/Server/FromServiceFilterAttribute.cs

@@ -0,0 +1,49 @@
+using MagicOnion.Server.Hubs;
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Reflection;
+
+namespace MagicOnion.Server
+{
+    /// <summary>
+    /// A MagicOnion filter that provided another filter via <see cref="IServiceLocator"/>.
+    /// </summary>
+    [AttributeUsage(AttributeTargets.Class | AttributeTargets.Method, AllowMultiple = true)]
+    public class FromServiceFilterAttribute : Attribute,
+        IMagicOnionFilterFactory<MagicOnionFilterAttribute>,
+        IMagicOnionFilterFactory<StreamingHubFilterAttribute>
+    {
+        public Type Type { get; }
+
+        public int Order { get; set; }
+
+        public FromServiceFilterAttribute(Type type)
+        {
+            if (!typeof(MagicOnionFilterAttribute).IsAssignableFrom(type) &&
+                !typeof(StreamingHubFilterAttribute).IsAssignableFrom(type))
+            {
+                throw new ArgumentException($"{type.FullName} doesn't inherit from MagicOnionFilterAttribute or StreamingHubFilterAttribute.", nameof(type));
+            }
+
+            Type = type;
+        }
+
+        MagicOnionFilterAttribute IMagicOnionFilterFactory<MagicOnionFilterAttribute>.CreateInstance(IServiceLocator serviceLocator)
+        {
+            if (!typeof(MagicOnionFilterAttribute).IsAssignableFrom(Type)) throw new InvalidOperationException($"Type '{Type.FullName}' doesn't inherit from {nameof(MagicOnionFilterAttribute)}.");
+            return CreateInstance<MagicOnionFilterAttribute>(serviceLocator);
+        }
+
+        StreamingHubFilterAttribute IMagicOnionFilterFactory<StreamingHubFilterAttribute>.CreateInstance(IServiceLocator serviceLocator)
+        {
+            if (!typeof(StreamingHubFilterAttribute).IsAssignableFrom(Type)) throw new InvalidOperationException($"Type '{Type.FullName}' doesn't inherit from {nameof(StreamingHubFilterAttribute)}.");
+            return CreateInstance<StreamingHubFilterAttribute>(serviceLocator);
+        }
+
+        protected T CreateInstance<T>(IServiceLocator serviceLocator)
+        {
+            return (T)serviceLocator.GetService(Type);
+        }
+    }
+}

src/MagicOnion/Server/FromTypeFilterAttribute.cs

@@ -0,0 +1,61 @@
+using MagicOnion.Server.Hubs;
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Reflection;
+
+namespace MagicOnion.Server
+{
+    /// <summary>
+    /// A MagicOnion filter that creates another filter of type.
+    /// </summary>
+    [AttributeUsage(AttributeTargets.Class | AttributeTargets.Method, AllowMultiple = true)]
+    public class FromTypeFilterAttribute : Attribute,
+        IMagicOnionFilterFactory<MagicOnionFilterAttribute>,
+        IMagicOnionFilterFactory<StreamingHubFilterAttribute>
+    {
+        public Type Type { get; }
+
+        public int Order { get; set; }
+
+        public object[] Arguments { get; set; } = Array.Empty<object>();
+
+        public FromTypeFilterAttribute(Type type)
+        {
+            if (!typeof(MagicOnionFilterAttribute).IsAssignableFrom(type) &&
+                !typeof(StreamingHubFilterAttribute).IsAssignableFrom(type))
+            {
+                throw new ArgumentException($"{type.FullName} doesn't inherit from MagicOnionFilterAttribute or StreamingHubFilterAttribute.", nameof(type));
+            }
+
+            Type = type;
+        }
+
+        MagicOnionFilterAttribute IMagicOnionFilterFactory<MagicOnionFilterAttribute>.CreateInstance(IServiceLocator serviceLocator)
+        {
+            if (!typeof(MagicOnionFilterAttribute).IsAssignableFrom(Type)) throw new InvalidOperationException($"Type '{Type.FullName}' doesn't inherit from {nameof(MagicOnionFilterAttribute)}.");
+            return CreateInstance<MagicOnionFilterAttribute>(serviceLocator);
+        }
+
+        StreamingHubFilterAttribute IMagicOnionFilterFactory<StreamingHubFilterAttribute>.CreateInstance(IServiceLocator serviceLocator)
+        {
+            if (!typeof(StreamingHubFilterAttribute).IsAssignableFrom(Type)) throw new InvalidOperationException($"Type '{Type.FullName}' doesn't inherit from {nameof(StreamingHubFilterAttribute)}.");
+            return CreateInstance<StreamingHubFilterAttribute>(serviceLocator);
+        }
+
+        protected T CreateInstance<T>(IServiceLocator serviceLocator)
+        {
+            var filterType = Type;
+            var ctors = filterType.GetConstructors();
+            var ctor = ctors.Select(x => (Ctor: x, Parameters: x.GetParameters()))
+                .OrderByDescending(x => x.Parameters.Length)
+                .First();
+
+            var @params = ctor.Parameters
+                .Select((x, i) => (Arguments.Length > i) ? Arguments[i] : serviceLocator.GetService(x.ParameterType))
+                .ToArray();
+
+            return (T)Activator.CreateInstance(filterType, @params);
+        }
+    }
+}

src/MagicOnion/Server/Hubs/StreamingHubFilterAttribute.cs

@@ -13,16 +13,13 @@ public int Order
             set { order = value; }
         }
 
-        protected Func<StreamingHubContext, ValueTask> Next { get; private set; }
-
         /// <summary>
         /// This constructor used by MagicOnionEngine when register handler.
         /// </summary>
-        public StreamingHubFilterAttribute(Func<StreamingHubContext, ValueTask> next)
+        public StreamingHubFilterAttribute()
         {
-            this.Next = next;
         }
 
-        public abstract ValueTask Invoke(StreamingHubContext context);
+        public abstract ValueTask Invoke(StreamingHubContext context, Func<StreamingHubContext, ValueTask> next);
     }
 }

src/MagicOnion/Server/Hubs/StreamingHubHandler.cs

@@ -17,7 +17,9 @@ public class StreamingHubHandler : IEquatable<StreamingHubHandler>
 
         public ILookup<Type, Attribute> AttributeLookup { get; private set; }
 
-        readonly StreamingHubFilterAttribute[] filters;
+        readonly IServiceLocator serviceLocator;
+
+        readonly IMagicOnionFilterFactory<StreamingHubFilterAttribute>[] filters;
         internal readonly Type RequestType;
         readonly Type UnwrappedResponseType;
         internal readonly IFormatterResolver resolver;
@@ -62,11 +64,19 @@ public StreamingHubHandler(MagicOnionOptions options, Type classType, MethodInfo
                 .ToLookup(x => x.GetType());
 
             this.filters = options.GlobalStreamingHubFilters
-                .Concat(classType.GetCustomAttributes<StreamingHubFilterAttribute>(true))
-                .Concat(methodInfo.GetCustomAttributes<StreamingHubFilterAttribute>(true))
+                .OfType<IMagicOnionFilterFactory<StreamingHubFilterAttribute>>()
+                .Concat(classType.GetCustomAttributes<StreamingHubFilterAttribute>(true).Select(x => new StreamingHubFilterDescriptor(x, x.Order)))
+                .Concat(classType.GetCustomAttributes<FromTypeFilterAttribute>(true))
+                .Concat(classType.GetCustomAttributes<FromServiceFilterAttribute>(true))
+                .Concat(methodInfo.GetCustomAttributes<StreamingHubFilterAttribute>(true).Select(x => new StreamingHubFilterDescriptor(x, x.Order)))
+                .Concat(methodInfo.GetCustomAttributes<FromTypeFilterAttribute>(true))
+                .Concat(methodInfo.GetCustomAttributes<FromServiceFilterAttribute>(true))
                 .OrderBy(x => x.Order)
                 .ToArray();
 
+            // options
+            this.serviceLocator = options.ServiceLocator;
+
             // validation filter
             if (methodInfo.GetCustomAttribute<MagicOnionFilterAttribute>(true) != null)
             {
@@ -145,21 +155,13 @@ static Type UnwrapResponseType(MethodInfo methodInfo)
 
         Func<StreamingHubContext, ValueTask> BuildMethodBodyWithFilter(Func<StreamingHubContext, ValueTask> methodBody)
         {
-            var flags = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance;
             Func<StreamingHubContext, ValueTask> next = methodBody;
 
-            foreach (var filter in this.filters.Reverse())
+            foreach (var filterFactory in this.filters.Reverse())
             {
-                var fields = filter.GetType().GetFields(flags);
-
-                var newFilter = (StreamingHubFilterAttribute)Activator.CreateInstance(filter.GetType(), new object[] { next });
-                // copy all data.
-                foreach (var item in fields)
-                {
-                    item.SetValue(newFilter, item.GetValue(filter));
-                }
-
-                next = newFilter.Invoke;
+                var newFilter = filterFactory.CreateInstance(serviceLocator);
+                var next_ = next; // capture reference
+                next = (ctx) => newFilter.Invoke(ctx, next_);
             }
 
             return next;

src/MagicOnion/Server/Hubs/StreamingHubHandlerRepository.cs

@@ -49,7 +49,10 @@ public static UniqueHashDictionary<StreamingHubHandler> GetHandlers(MethodHandle
 
         public static void AddGroupRepository(MethodHandler parent, IGroupRepository repository)
         {
-            cacheGroup.Add(parent, repository);
+            lock (cacheGroup)
+            {
+                cacheGroup.Add(parent, repository);
+            }
         }
 
         public static IGroupRepository GetGroupRepository(MethodHandler parent)

src/MagicOnion/Server/IMagicOnionFilterFactory.cs

@@ -0,0 +1,8 @@
+namespace MagicOnion.Server
+{
+    public interface IMagicOnionFilterFactory<T>
+    {
+        T CreateInstance(IServiceLocator serviceLocator);
+        int Order { get; }
+    }
+}

src/MagicOnion/Server/IServiceLocator.cs

@@ -1,6 +1,7 @@
 using System;
 using System.Linq;
 using System.Linq.Expressions;
+using System.Reflection;
 
 namespace MagicOnion.Server
 {
@@ -49,6 +50,13 @@ static class Cache<T>
 
     internal static class ServiceLocatorHelper
     {
+        static readonly MethodInfo serviceLocatorGetServiceT = typeof(IServiceLocator).GetMethod("GetService");
+
+        internal static object GetService(this IServiceLocator serviceLocator, Type t)
+        {
+            return serviceLocatorGetServiceT.MakeGenericMethod(t).Invoke(serviceLocator, null);
+        }
+
         internal static TServiceBase CreateService<TServiceBase, TServiceInterface>(ServiceContext context)
             where TServiceBase : ServiceBase<TServiceInterface>
         {

src/MagicOnion/Server/MagicOnionFilterAttribute.cs

@@ -13,17 +13,14 @@ public int Order
             set { order = value; }
         }
 
-        protected Func<ServiceContext, ValueTask> Next { get; private set; }
-
         /// <summary>
         /// This constructor used by MagicOnionEngine when register handler.
         /// </summary>
-        public MagicOnionFilterAttribute(Func<ServiceContext, ValueTask> next)
+        public MagicOnionFilterAttribute()
         {
-            this.Next = next;
         }
 
-        public abstract ValueTask Invoke(ServiceContext context);
+        public abstract ValueTask Invoke(ServiceContext context, Func<ServiceContext, ValueTask> next);
 
         protected static void SetStatusCode(ServiceContext context, Grpc.Core.StatusCode statusCode, string detail)
         {