Skip to content

Use async service provider in ServiceLocator #2898

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.

Sign up for GitHub

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

Merged
nkolev92 merged 4 commits into from
Jun 27, 2019
Merged

Use async service provider in ServiceLocator #2898

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d9f7a88
work
nkolev92 Jun 24, 2019
40320fb
try to fix badly
nkolev92 Jun 24, 2019
fe6f8cd
work
nkolev92 Jun 24, 2019
f6df873
address feedback
nkolev92 Jun 24, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions src/NuGet.Clients/NuGet.Tools/NuGetPackage.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,6 @@ public sealed class NuGetPackage : AsyncPackage, IVsPackageExtensionProvider, IV
// It is displayed in the Help - About box of Visual Studio
public const string ProductVersion = "5.2.0";
private const string F1KeywordValuePmUI = "VS.NuGet.PackageManager.UI";
private static readonly object _credentialsPromptLock = new object();
private readonly HashSet<Uri> _credentialRequested = new HashSet<Uri>();

private AsyncLazy<IVsMonitorSelection> _vsMonitorSelection;
private IVsMonitorSelection VsMonitorSelection => ThreadHelper.JoinableTaskFactory.Run(_vsMonitorSelection.GetValueAsync);
Expand Down
34 changes: 20 additions & 14 deletions src/NuGet.Clients/NuGet.VisualStudio.Common/ServiceLocator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using Microsoft.VisualStudio.Shell;
using Microsoft.VisualStudio.Shell.Interop;
using Task = System.Threading.Tasks.Task;
using IAsyncServiceProvider = Microsoft.VisualStudio.Shell.IAsyncServiceProvider;
using VsServiceProvider = Microsoft.VisualStudio.OLE.Interop.IServiceProvider;

namespace NuGet.VisualStudio
Expand All @@ -22,17 +23,12 @@ namespace NuGet.VisualStudio
// REVIEW: Make this internal
public static class ServiceLocator
{
public static void InitializePackageServiceProvider(IServiceProvider provider)
public static void InitializePackageServiceProvider(IAsyncServiceProvider provider)
{
if (provider == null)
{
throw new ArgumentNullException(nameof(provider));
}

PackageServiceProvider = provider;
PackageServiceProvider = provider ?? throw new ArgumentNullException(nameof(provider));
}

public static IServiceProvider PackageServiceProvider { get; private set; }
public static IAsyncServiceProvider PackageServiceProvider { get; private set; }

public static TService GetInstanceSafe<TService>() where TService : class
{
Expand Down Expand Up @@ -96,23 +92,33 @@ public static async Task<TInterface> GetGlobalServiceAsync<TService, TInterface>
// and so this method can RPC into main thread. Switch to main thread explictly, since method has STA requirement
await NuGetUIThreadHelper.JoinableTaskFactory.SwitchToMainThreadAsync();

Comment thread
nkolev92 marked this conversation as resolved.
Outdated
return await GetGlobalServiceFreeThreadedAsync<TService, TInterface>();
if (PackageServiceProvider != null)
{
var result = await PackageServiceProvider.GetServiceAsync(typeof(TService));
var service = result as TInterface;
if (service != null)
{
return service;
}
}

return Package.GetGlobalService(typeof(TService)) as TInterface;

@dtivel dtivel Jun 27, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add telemetry for when this or this is called?

@nkolev92 nkolev92 Jun 27, 2019

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would be trying to learn?
It would just tell us that the service in question is not async.

Maybe something else?

@nkolev92 nkolev92 Jun 27, 2019

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dtivel

I'll merge this now, so it can runt through the nightly validations, but i'll be hpapy to discuss further improvements to this.

}

public static Task<TInterface> GetGlobalServiceFreeThreadedAsync<TService, TInterface>() where TInterface : class
public static async Task<TInterface> GetGlobalServiceFreeThreadedAsync<TService, TInterface>() where TInterface : class
{
if (PackageServiceProvider != null)
{
var result = PackageServiceProvider.GetService(typeof(TService));
var result = await PackageServiceProvider.GetServiceAsync(typeof(TService));
var service = result as TInterface;

if (service != null)
{
return Task.FromResult(service);
return service;
}
}

return Task.FromResult(Package.GetGlobalService(typeof(TService)) as TInterface);
return Package.GetGlobalService(typeof(TService)) as TInterface;
}

private static async Task<TService> GetDTEServiceAsync<TService>() where TService : class
Expand All @@ -123,7 +129,7 @@ private static async Task<TService> GetDTEServiceAsync<TService>() where TServic

private static async Task<TService> GetComponentModelServiceAsync<TService>() where TService : class
{
IComponentModel componentModel = await GetGlobalServiceAsync<SComponentModel, IComponentModel>();
IComponentModel componentModel = await GetGlobalServiceFreeThreadedAsync<SComponentModel, IComponentModel>();
return componentModel?.GetService<TService>();
}

Expand Down