Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Improve Filter APIs #187

Merged
merged 9 commits into from
Sep 9, 2019
Merged

feat: Improve Filter APIs #187

merged 9 commits into from
Sep 9, 2019

Conversation

mayuki
Copy link
Member

@mayuki mayuki commented Aug 28, 2019

Motivation

Currently, MagicOnion's Filter doesn't support constructor injection at handler build time. I think Filters should support DI and constructor injection as same as Service and StreamingHub. It will introduce avoiding direct use of ServiceLocator and improves testability.

Changes in this pull request

  • MagicOnionOptions.GlobalFilters and MagicOnionOptions.GlobalStreamingHubFilters changed from filter attribute arrays to List that accepts filter type and instance. (breaking changes)
  • At building a handler body, the builder applies constructor injection for instantiating filters.
  • To reuse a filter, so the next filter is now passed by parameter.
  • Add FromServiceFilterAttribute (a filter will be provided by IServiceLocator) and FromTypeFilterAttribute (a filter will be instantiated from Type). These are similar to ASP.NET Core's ServiceFilterAttribute and TypeFilterAttribute.

Code samples

Register filters on startup

static async Task Main(string[] args)
{
    MagicOnionHost.CreateDefaultBuilder()
        .UseMagicOnion()
        .ConfigureServices((hostContext, services) =>
        {
            services.Configure<MagicOnionHostingOptions>(options =>
            {
                options.Service.GlobalStreamingHubFilters.Add<MyStreamingHubFilterAttribute>();
                // options.Service.GlobalStreamingHubFilters.Add(new MyStreamingHubFilterAttribute(logger));

                options.Service.GlobalFilters.Add<MyFilterAttribute>();
                // options.Service.GlobalFilters.Add(new MyFilterAttribute(logger));
            });
        })
        .RunConsoleAsync();
}public class MyStreamingHubFilterAttribute : StreamingHubFilterAttribute
{
    private readonly ILogger _logger;

    // the `logger` parameter will be injected at instantiating.
    public MyStreamingHubFilterAttribute(ILogger<MyStreamingHubFilterAttribute> logger)
    {
        _logger = logger;
    }

    public override async ValueTask Invoke(StreamingHubContext context, Func<StreamingHubContext, ValueTask> next)
    {
        _logger.LogInformation($"MyStreamingHubFilter Begin: {context.Path}");
        await next(context);
        _logger.LogInformation($"MyStreamingHubFilter End: {context.Path}");
    }
}

Register filters using attributes with constructor injection.

[FromTypeFilter(typeof(MyFilterAttribute))]
public class MyService : ServiceBase<IMyService>, IMyService
{
    // The filter will instantiate from type.
    [FromTypeFilter(typeof(MySecondFilterAttribute))]
    public UnaryResult<int> Foo()
    {
        return UnaryResult(0);
    }

    // The filter will instantiate from type with some arguments. if the arguments are missing, it will be obtained from `IServiceLocator` 
    [FromTypeFilter(typeof(MyThirdFilterAttribute), Arguments = new object[] { "foo", 987654 })]
    public UnaryResult<int> Bar()
    {
        return UnaryResult(0);
    }

    // The filter instance will be provided via `IServiceLocator`.
    [FromServiceFilter(typeof(MyFourthFilterAttribute))]
    public UnaryResult<int> Baz()
    {
        return UnaryResult(0);
    }
}

@mayuki mayuki requested a review from neuecc August 28, 2019 15:46
@neuecc neuecc merged commit e156fd7 into master Sep 9, 2019
@neuecc neuecc deleted the ImproveFilterApi branch September 9, 2019 11:26
AntonC9018 pushed a commit to AntonC9018/MagicOnion that referenced this pull request Sep 13, 2022
AntonC9018 pushed a commit to AntonC9018/MagicOnion that referenced this pull request Sep 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants