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

Suggestion: Alternative solution for injecting HttpRequestMessage #28

Closed
rexcfnghk opened this issue Sep 27, 2017 · 4 comments
Closed

Comments

@rexcfnghk
Copy link

rexcfnghk commented Sep 27, 2017

The current implementation of injecting HttpRequestMessage relies on updating the component registry

internal static void UpdateScopeWithHttpRequestMessage(HttpRequestMessage request)
{
    var scope = request.GetDependencyScope();
    var requestScope = scope.GetRequestLifetimeScope();
     if (requestScope == null) return;

     var registry = requestScope.ComponentRegistry;
     var builder = new ContainerBuilder();
     builder.Register(c => request).InstancePerRequest();
     builder.Update(registry);
}

https://github.com/autofac/Autofac.WebApi/blob/develop/src/Autofac.Integration.WebApi/CurrentRequestHandler.cs#L66

However this defeats the best practices in the documentation: the container should be considered immutable.

Also ContainerBuilder.Update is marked as obsolete in this commit autofac/Autofac@8a89e94

Should another approach be considered? Like the one Simple Injector is using, by capturing the HttpRequestMessage in a provider:
https://github.com/simpleinjector/SimpleInjector/blob/master/src/SimpleInjector.Integration.WebApi/SimpleInjectorWebApiExtensions.cs

I will be happy to help with a pull request if needed.

@alexmg
Copy link
Member

alexmg commented Sep 27, 2017

The container update certainly isn't ideal. I'm definitely open to a PR if you would like to put something together.

@rexcfnghk
Copy link
Author

If I create a PR that simply adapts to Simple Injector's way of injecting HttpRequestMessage, will it create any licensing issues?

@tillig
Copy link
Member

tillig commented Sep 27, 2017

Both projects are MIT license, so there's no problem. It appears SimpleInjector is using the same mechanism as the ASP.NET Core HttpContextAccessor anyway, so that's not a new pattern.

@rexcfnghk
Copy link
Author

I see. I will come up with a PR soon!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants