Skip to content
This repository has been archived by the owner on Nov 20, 2018. It is now read-only.

Remove ApplicationServices from HttpContext? #466

Closed
davidfowl opened this issue Nov 6, 2015 · 14 comments
Closed

Remove ApplicationServices from HttpContext? #466

davidfowl opened this issue Nov 6, 2015 · 14 comments
Assignees
Milestone

Comments

@davidfowl
Copy link
Member

We got some feedback that is is confusing to have both on the HttpContext object. Do we need anything other than request services readily accessible?

/cc @lodejard @rynowak @Tratcher

@rynowak
Copy link
Member

rynowak commented Nov 6, 2015

My suggestion for a while has been just to have one SP on the context. If you're using request services middleware then it replaces the app services with the request services.

@lodejard
Copy link
Contributor

lodejard commented Nov 6, 2015

+1 - if there's one property just call it IServiceProvider Services and have it always track the currently effective scope like Ryan described.

It started as one property already, and changed in case a middleware needed to create a new child scope from the app scope instead of from the request scope. But it can just close over the app ISP at startup if that's the case. So even there this should be totally fine.

@Tratcher
Copy link
Member

Tratcher commented Nov 6, 2015

You can remove the HttpContext property but keep the IServiceProvidersFeature property for components that need access to the app services.

@davidfowl
Copy link
Member Author

Lets do this then.

@davidfowl davidfowl added this to the 1.0.0-rc2 milestone Nov 6, 2015
@Tratcher Tratcher assigned JunTaoLuo and unassigned Tratcher Nov 6, 2015
@davidfowl
Copy link
Member Author

/cc @DamianEdwards

@DamianEdwards
Copy link
Member

Sounds good. I'm OK with this in RC2.

@davidfowl
Copy link
Member Author

@Tratcher what component is that?

@Tratcher
Copy link
Member

Tratcher commented Nov 7, 2015

I don't know, why did you give access to them in the first place? Louis already pointed to one scenario.

@davidfowl
Copy link
Member Author

And also called out how why it's not required. Middleware can get ISP from the IApplicationBuilder or just taking it in the ctor. It doesn't need to be a property on the HttpContext or the feature interface

@rjperes
Copy link

rjperes commented Nov 9, 2015

Just for coherence, the Controller property is called Resolver... should we change both (that and HttpContext) for Services?

@davidfowl
Copy link
Member Author

I don't think so. The type was IDependencyResolver so Resolver made a bit more sense. I think we'll stick with Services here.

@rjperes
Copy link

rjperes commented Nov 10, 2015

was IDependencyResolver, now it is IServiceProvider! :-)
Don't you think, for coherence's sake, identical things should be called identically?
Not a big issue, though...

@davidfowl
Copy link
Member Author

In general yes but in this case I'd prefer not to break everything under the sun. I actually think we should just remove ApplicationServices and leave it as RequestServices.

@JunTaoLuo
Copy link
Contributor

@avanderhoorn You'll need to make changes to Glimpse after we made this change.

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

No branches or pull requests

7 participants