ServiceProviderExtensions architecture

[grokys]

I've been trying to understand our service provider architecture and I've got some questions/suggestions regarding ServiceProviderExtensions.

Spot the difference

• What is the difference between GetService and TryGetService in ServiceProviderExtensions? Documentation says that TryGetService is a "Safe variant of GetService that doesn't throw exceptions if the service is not found." except AFAICS GetService just calls TryGetService, casts the result and doesn't throw anyway.
• What is the difference between TryGetService and GetExportedValue in ServiceProviderExtensions? They appear to be the same, even though they don't call each other (the code appears to be simply copy/pasted and modified slightly).

Trying to be too clever?

I think understand what the GetService, TryGetService etc methods on ServiceProviderExtensions are trying to do: if the service provider is an IUIProvider, then call GetService on it, otherwise get an instance of IUIProvider from the service provider and call GetService on that. However I wonder if this is more confusing than helpful:

• If you call provider.GetService(type(T)) then this mechanism will be bypassed, meaning that provider.GetService(type(T)) will give different results to provider.GetService<T>()
• It masks the fact that there are different service providers for different types of resources.

IMO it would be clearer to see e.g.:

var uiProvider = serviceProvider.GetService<IUIProvider>();
uiProvider.GetSerivce<IConnectionManager>();

Even if this is longer, it gives me some important information: it that tells me that serviceProvider at that point isn't an IUIProvider and that an IUIProvider is needed to get an IConnectionManager.

Wrong Place

The AddCommandHandler methods don't look like they should be in ServiceProviderExtensions to me.

[shana]

GetService() on the global IServiceProvider returns global services. These are services that are registered explicitely via AddService (like https://github.com/github/VisualStudio/blob/master/src/GitHub.VisualStudio/GitHubPackage.cs#L117) and available to either the local extension or globally. These are singleton instances.

When you [Import] something (or use an ImportingConstructor), MEF gives you whatever it can find from both services that have been explicitely registered via AddService and types that are marked with Export. The problem with this is that when you want to access a type outside of an importing constructor, you have to know how it was registered - if it's a "service", you have to call GetService(), if it's an [Export], you need to call GetExportedValue.

GetService has no generic counterpart, so the GetService<>() extension is that counterpart. TryGetService is an earlier addition I had because I never want to throw when attempting to get services, in general. The reason GetService<> doesn't throw is because all these extension methods came from other places and were aggregated here, and meh I never want to throw anyway.
GetExportedValue does have a generic implementation by default, and it even has one that doesn't throw, so the mess here is mostly due to the fact that the IServiceProvider itself is a complete mess when it comes to this API.

I don't want to ever have to know whether a type I'm asking for is found via an [Export] or AddService(), it is a useless distinction, especially since we can have singletons as exports. Furthermore, GetService is available in the service provider, but GetExportedValue is available only from the ExportProvider in the component model service provided by the service provider.

IUIProvider.GetService hides all the different registration mechanisms away by trying all the different combinations to find the instance you're looking for. Unfortunately, we cannot pass around IUIProvider as a MEF export in our importing constructors, since that will trigger MEF to load and scan everything, which causes dependencies to get loaded when we don't want them to be loaded. That is the reason why the code checks whether the service provider is an IUIProvider and calls GetService on it, otherwise it calls GetExportedValue

Also a lot of the reasons why this is the way it is is because the github windows code assumed everywhere that it has a IServiceProvider that it can call directly to get types, because the windows app has its own global IServiceProvider. We have to deal with VS's service provider and our own service provider. At some point we had one, see #137

[shana]

There is a PR floating somewhere that cleans these up, I just never got around to finishing it

[shana]

TL;DR

I want to call GetService everywhere, but the default implementation throws and it doesn't work if the value is not a manually registered service, so next best thing would be to call GetService<> everywhere and not throw.

We have to limit our usage of IUIProvider in importing constructors because it triggers a lot of other things to happen, and until 2 weeks ago, IUIProvider was not a manually registered service, so GetService<> has to be in IServiceProvider to work for the case where we're talking to VS's IServiceProvider.

Now that IUIProvider is a manually registered service, most of this caching code is not needed.

var uiProvider = serviceProvider.GetService<IUIProvider>();
uiProvider.GetSerivce<IConnectionManager>();

We can't do this in Team Explorer home and other areas, where instantiating IUIProvider causes MEF to scan and load everything, so we want to grab IConnectionManager from the export provider directly via GetExportedValue, but I really don't want to have a different call just because of that.

[shana]

The AddCommandHandler methods don't look like they should be in ServiceProviderExtensions to me.

They were in the package base class before as static methods, but then that got annoying. They could probably live in the helpers folder as a separate serviceprovider menu extension class of some sort.