Ensure UsageTracker services retreived on UI thread.

grokys · grokys · commit c493e1e3f17d · 2016-11-08T12:39:00.000+01:00
Required changing most of the UsageTracker methods to be async.
diff --git a/src/GitHub.App/Models/RepositoryHost.cs b/src/GitHub.App/Models/RepositoryHost.cs
@@ -17,6 +17,7 @@
 using System.Linq;
 using System.Reactive.Threading.Tasks;
 using System.Collections.Generic;
+using GitHub.Extensions;
 
 namespace GitHub.Models
 {
@@ -241,7 +242,7 @@ IObservable<AuthenticationResult> LoginWithApiUser(UserAndScopes userAndScopes)
                     if (result.IsSuccess())
                     {
                         var accountCacheItem = new AccountCacheItem(userAndScopes.User);
-                        usage.IncrementLoginCount();
+                        usage.IncrementLoginCount().Forget();
                         return ModelService.InsertUser(accountCacheItem).Select(_ => result);
                     }
 
diff --git a/src/GitHub.App/Services/PullRequestService.cs b/src/GitHub.App/Services/PullRequestService.cs
@@ -91,7 +91,7 @@ async Task<IPullRequestModel> PushAndCreatePR(IRepositoryHost host,
                 await Task.Delay(TimeSpan.FromSeconds(5));
 
             var ret = await host.ModelService.CreatePullRequest(sourceRepository, targetRepository, sourceBranch, targetBranch, title, body);
-            usageTracker.IncrementUpstreamPullRequestCount();
+            await usageTracker.IncrementUpstreamPullRequestCount();
             return ret;
         }
 
diff --git a/src/GitHub.App/ViewModels/GistCreationViewModel.cs b/src/GitHub.App/ViewModels/GistCreationViewModel.cs
@@ -78,7 +78,7 @@ IObservable<Gist> OnCreateGist(object unused)
             newGist.Files.Add(FileName, SelectedText);
 
             return gistPublishService.PublishGist(apiClient, newGist)
-                .Do(_ => usageTracker.IncrementCreateGistCount())
+                .Do(_ => usageTracker.IncrementCreateGistCount().Forget())
                 .Catch<Gist, Exception>(ex =>
                 {
                     if (!ex.IsCriticalException())
diff --git a/src/GitHub.App/ViewModels/RepositoryCloneViewModel.cs b/src/GitHub.App/ViewModels/RepositoryCloneViewModel.cs
@@ -199,7 +199,7 @@ IObservable<Unit> OnCloneRepository(object state)
                 return cloneService.CloneRepository(repository.CloneUrl, repository.Name, BaseRepositoryPath)
                     .ContinueAfter(() =>
                     {
-                        usageTracker.IncrementCloneCount();
+                        usageTracker.IncrementCloneCount().Forget();
                         return Observable.Return(Unit.Default);
                     });
             })
diff --git a/src/GitHub.App/ViewModels/RepositoryCreationViewModel.cs b/src/GitHub.App/ViewModels/RepositoryCreationViewModel.cs
@@ -268,7 +268,7 @@ IObservable<Unit> OnCreateRepository(object state)
                 SelectedAccount,
                 BaseRepositoryPath,
                 repositoryHost.ApiClient)
-                .Do(_ => usageTracker.IncrementCreateCount());
+                .Do(_ => usageTracker.IncrementCreateCount().Forget());
         }
 
         ReactiveCommand<Unit> InitializeCreateRepositoryCommand()
diff --git a/src/GitHub.App/ViewModels/RepositoryPublishViewModel.cs b/src/GitHub.App/ViewModels/RepositoryPublishViewModel.cs
@@ -153,7 +153,7 @@ IObservable<ProgressState> OnPublishRepository(object arg)
             var account = SelectedAccount;
 
             return repositoryPublishService.PublishRepository(newRepository, account, SelectedHost.ApiClient)
-                .Do(_ => usageTracker.IncrementPublishCount())
+                .Do(_ => usageTracker.IncrementPublishCount().Forget())
                 .Select(_ => ProgressState.Success)
                 .Catch<ProgressState, Exception>(ex =>
                 {
diff --git a/src/GitHub.App/ViewModels/StartPageCloneViewModel.cs b/src/GitHub.App/ViewModels/StartPageCloneViewModel.cs
@@ -105,7 +105,7 @@ IObservable<Unit> OnCloneRepository(object state)
                 return cloneService.CloneRepository(repository.CloneUrl, repository.Name, BaseRepositoryPath)
                     .ContinueAfter(() =>
                     {
-                        usageTracker.IncrementCloneCount();
+                        usageTracker.IncrementCloneCount().Forget();
                         return Observable.Return(Unit.Default);
                     });
             })
diff --git a/src/GitHub.Exports/Services/IUsageTracker.cs b/src/GitHub.Exports/Services/IUsageTracker.cs
@@ -1,19 +1,20 @@
 ﻿using GitHub.VisualStudio;
 using System.Runtime.InteropServices;
+using System.Threading.Tasks;
 
 namespace GitHub.Services
 {
     [Guid(Guids.UsageTrackerId)]
     public interface IUsageTracker
     {
-        void IncrementLaunchCount();
-        void IncrementCloneCount();
-        void IncrementCreateCount();
-        void IncrementPublishCount();
-        void IncrementOpenInGitHubCount();
-        void IncrementLinkToGitHubCount();
-        void IncrementCreateGistCount();
-        void IncrementUpstreamPullRequestCount();
-        void IncrementLoginCount();
+        Task IncrementLaunchCount();
+        Task IncrementCloneCount();
+        Task IncrementCreateCount();
+        Task IncrementPublishCount();
+        Task IncrementOpenInGitHubCount();
+        Task IncrementLinkToGitHubCount();
+        Task IncrementCreateGistCount();
+        Task IncrementUpstreamPullRequestCount();
+        Task IncrementLoginCount();
     }
 }
diff --git a/src/GitHub.VisualStudio/Menus/CopyLink.cs b/src/GitHub.VisualStudio/Menus/CopyLink.cs
@@ -31,7 +31,7 @@ public async void Activate([AllowNull]object data = null)
                 Clipboard.SetText(link);
                 var ns = ServiceProvider.TryGetService<IStatusBarNotificationService>();
                 ns?.ShowMessage(Resources.LinkCopiedToClipboardMessage);
-                UsageTracker.IncrementLinkToGitHubCount();
+                await UsageTracker.IncrementLinkToGitHubCount();
             }
             catch
             {
diff --git a/src/GitHub.VisualStudio/Menus/OpenLink.cs b/src/GitHub.VisualStudio/Menus/OpenLink.cs
@@ -26,7 +26,7 @@ public async void Activate([AllowNull]object data = null)
             var browser = ServiceProvider.TryGetService<IVisualStudioBrowser>();
             browser?.OpenUrl(link.ToUri());
 
-            UsageTracker.IncrementOpenInGitHubCount();
+            await UsageTracker.IncrementOpenInGitHubCount();
         }
 
         public bool CanShow()
diff --git a/src/GitHub.VisualStudio/Services/UsageTracker.cs b/src/GitHub.VisualStudio/Services/UsageTracker.cs
@@ -11,6 +11,7 @@
 using Microsoft.VisualStudio.Shell;
 using Task = System.Threading.Tasks.Task;
 using GitHub.Extensions;
+using System.Threading.Tasks;
 
 namespace GitHub.Services
 {
@@ -70,77 +71,79 @@ public UsageTracker(IUIProvider uiProvider)
             RunTimer();
         }
 
-        public void IncrementLaunchCount()
+        public async Task IncrementLaunchCount()
         {
-            var usage = LoadUsage();
+            var usage = await LoadUsage();
             ++usage.Model.NumberOfStartups;
             ++usage.Model.NumberOfStartupsWeek;
             ++usage.Model.NumberOfStartupsMonth;
             SaveUsage(usage);
         }
 
-        public void IncrementCloneCount()
+        public async Task IncrementCloneCount()
         {
-            var usage = LoadUsage();
+            var usage = await LoadUsage();
             ++usage.Model.NumberOfClones;
             SaveUsage(usage);
         }
 
-        public void IncrementCreateCount()
+        public async Task IncrementCreateCount()
         {
-            var usage = LoadUsage();
+            var usage = await LoadUsage();
             ++usage.Model.NumberOfReposCreated;
             SaveUsage(usage);
         }
 
-        public void IncrementPublishCount()
+        public async Task IncrementPublishCount()
         {
-            var usage = LoadUsage();
+            var usage = await LoadUsage();
             ++usage.Model.NumberOfReposPublished;
             SaveUsage(usage);
         }
 
-        public void IncrementOpenInGitHubCount()
+        public async Task IncrementOpenInGitHubCount()
         {
-            var usage = LoadUsage();
+            var usage = await LoadUsage();
             ++usage.Model.NumberOfOpenInGitHub;
             SaveUsage(usage);
         }
 
-        public void IncrementLinkToGitHubCount()
+        public async Task IncrementLinkToGitHubCount()
         {
-            var usage = LoadUsage();
+            var usage = await LoadUsage();
             ++usage.Model.NumberOfLinkToGitHub;
             SaveUsage(usage);
         }
 
-        public void IncrementCreateGistCount()
+        public async Task IncrementCreateGistCount()
         {
-            var usage = LoadUsage();
+            var usage = await LoadUsage();
             ++usage.Model.NumberOfGists;
             SaveUsage(usage);
         }
 
-        public void IncrementUpstreamPullRequestCount()
+        public async Task IncrementUpstreamPullRequestCount()
         {
-            var usage = LoadUsage();
+            var usage = await LoadUsage();
             ++usage.Model.NumberOfUpstreamPullRequests;
             SaveUsage(usage);
         }
 
-        public void IncrementLoginCount()
+        public async Task IncrementLoginCount()
         {
-            var usage = LoadUsage();
+            var usage = await LoadUsage();
             ++usage.Model.NumberOfLogins;
             SaveUsage(usage);
         }
 
-        void Initialize()
+        async Task Initialize()
         {
             // The services needed by the usage tracker are loaded when they are first needed to
             // improve the startup time of the extension.
             if (userSettings == null)
             {
+                await ThreadHelper.JoinableTaskFactory.SwitchToMainThreadAsync();
+
                 client = uiProvider.GetService<IMetricsService>();
                 connectionManager = uiProvider.GetService<IConnectionManager>();
                 userSettings = uiProvider.GetService<IPackageSettings>();
@@ -154,9 +157,9 @@ void Initialize()
             }
         }
 
-        UsageStore LoadUsage()
+        async Task<UsageStore> LoadUsage()
         {
-            Initialize();
+            await Initialize();
 
             var json = fileExists(storePath) ? readAllText(storePath, Encoding.UTF8) : null;
             UsageStore result = null;
@@ -204,33 +207,26 @@ void TimerTick(object sender, EventArgs e)
 
         async Task TimerTick()
         {
-            Initialize();
+            await Initialize();
 
             if (firstRun)
             {
-                IncrementLaunchCount();
+                await IncrementLaunchCount();
                 timer.Interval = TimeSpan.FromHours(8);
                 firstRun = false;
             }
 
-            if (userSettings == null)
+            if (client == null || !userSettings.CollectMetrics)
             {
-                await ThreadHelper.JoinableTaskFactory.SwitchToMainThreadAsync();
-                if (client == null)
-                {
-                    timer.Stop();
-                    return;
-                }
-            }
-
-            if (!userSettings.CollectMetrics)
+                timer.Stop();
                 return;
+            }
 
             // Every time we increment the launch count we increment both daily and weekly
             // launch count but we only submit (and clear) the weekly launch count when we've
             // transitioned into a new week. We've defined a week by the ISO8601 definition,
             // i.e. week starting on Monday and ending on Sunday.
-            var usage = LoadUsage();
+            var usage = await LoadUsage();
             var lastDate = usage.LastUpdated;
             var currentDate = DateTimeOffset.Now;
             var includeWeekly = GetIso8601WeekOfYear(lastDate) != GetIso8601WeekOfYear(currentDate);
diff --git a/src/GitHub.VisualStudio/Services/UsageTrackerDispatcher.cs b/src/GitHub.VisualStudio/Services/UsageTrackerDispatcher.cs
@@ -1,6 +1,7 @@
 ﻿using Microsoft.VisualStudio.Shell;
 using System;
 using System.ComponentModel.Composition;
+using Task = System.Threading.Tasks.Task;
 
 namespace GitHub.Services
 {
@@ -16,14 +17,14 @@ public UsageTrackerDispatcher([Import(typeof(SVsServiceProvider))] IServiceProvi
             inner = serviceProvider.GetService(typeof(IUsageTracker)) as IUsageTracker;
         }
 
-        public void IncrementCloneCount() => inner.IncrementCloneCount();
-        public void IncrementCreateCount() => inner.IncrementCreateCount();
-        public void IncrementCreateGistCount() => inner.IncrementCreateGistCount();
-        public void IncrementLaunchCount() => inner.IncrementLaunchCount();
-        public void IncrementLinkToGitHubCount() => inner.IncrementLinkToGitHubCount();
-        public void IncrementLoginCount() => inner.IncrementLoginCount();
-        public void IncrementOpenInGitHubCount() => inner.IncrementOpenInGitHubCount();
-        public void IncrementPublishCount() => inner.IncrementPublishCount();
-        public void IncrementUpstreamPullRequestCount() => inner.IncrementUpstreamPullRequestCount();
+        public Task IncrementCloneCount() => inner.IncrementCloneCount();
+        public Task IncrementCreateCount() => inner.IncrementCreateCount();
+        public Task IncrementCreateGistCount() => inner.IncrementCreateGistCount();
+        public Task IncrementLaunchCount() => inner.IncrementLaunchCount();
+        public Task IncrementLinkToGitHubCount() => inner.IncrementLinkToGitHubCount();
+        public Task IncrementLoginCount() => inner.IncrementLoginCount();
+        public Task IncrementOpenInGitHubCount() => inner.IncrementOpenInGitHubCount();
+        public Task IncrementPublishCount() => inner.IncrementPublishCount();
+        public Task IncrementUpstreamPullRequestCount() => inner.IncrementUpstreamPullRequestCount();
     }
 }
diff --git a/src/UnitTests/GitHub.App/ViewModels/RepositoryCloneViewModelTests.cs b/src/UnitTests/GitHub.App/ViewModels/RepositoryCloneViewModelTests.cs
@@ -13,6 +13,7 @@
 using GitHub.Api;
 using Akavache;
 using GitHub.Collections;
+using GitHub.Extensions;
 
 public class RepositoryCloneViewModelTests
 {
@@ -358,7 +359,7 @@ public async Task UpdatesMetricsWhenRepositoryCloned()
             vm.SelectedRepository = Substitute.For<IRepositoryModel>();
             await vm.CloneCommand.ExecuteAsync();
 
-            usageTracker.Received().IncrementCloneCount();
+            usageTracker.Received().IncrementCloneCount().Forget();
         }
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -17,6 +17,7 @@`
`17`	`17`	`using System.Linq;`
`18`	`18`	`using System.Reactive.Threading.Tasks;`
`19`	`19`	`using System.Collections.Generic;`
	`20`	`+using GitHub.Extensions;`
`20`	`21`
`21`	`22`	`namespace GitHub.Models`
`22`	`23`	`{`
`@@ -241,7 +242,7 @@ IObservable<AuthenticationResult> LoginWithApiUser(UserAndScopes userAndScopes)`
`241`	`242`	`if (result.IsSuccess())`
`242`	`243`	`{`
`243`	`244`	`var accountCacheItem = new AccountCacheItem(userAndScopes.User);`
`244`		`- usage.IncrementLoginCount();`
	`245`	`+ usage.IncrementLoginCount().Forget();`
`245`	`246`	`return ModelService.InsertUser(accountCacheItem).Select(_ => result);`
`246`	`247`	`}`
`247`	`248`
Original file line number	Diff line number	Diff line change
`@@ -91,7 +91,7 @@ async Task<IPullRequestModel> PushAndCreatePR(IRepositoryHost host,`
`91`	`91`	`await Task.Delay(TimeSpan.FromSeconds(5));`
`92`	`92`
`93`	`93`	`var ret = await host.ModelService.CreatePullRequest(sourceRepository, targetRepository, sourceBranch, targetBranch, title, body);`
`94`		`- usageTracker.IncrementUpstreamPullRequestCount();`
	`94`	`+ await usageTracker.IncrementUpstreamPullRequestCount();`
`95`	`95`	`return ret;`
`96`	`96`	`}`
`97`	`97`
Original file line number	Diff line number	Diff line change
`@@ -78,7 +78,7 @@ IObservable<Gist> OnCreateGist(object unused)`
`78`	`78`	`newGist.Files.Add(FileName, SelectedText);`
`79`	`79`
`80`	`80`	`return gistPublishService.PublishGist(apiClient, newGist)`
`81`		`- .Do(_ => usageTracker.IncrementCreateGistCount())`
	`81`	`+ .Do(_ => usageTracker.IncrementCreateGistCount().Forget())`
`82`	`82`	`.Catch<Gist, Exception>(ex =>`
`83`	`83`	`{`
`84`	`84`	`if (!ex.IsCriticalException())`
Original file line number	Diff line number	Diff line change
`@@ -199,7 +199,7 @@ IObservable<Unit> OnCloneRepository(object state)`
`199`	`199`	`return cloneService.CloneRepository(repository.CloneUrl, repository.Name, BaseRepositoryPath)`
`200`	`200`	`.ContinueAfter(() =>`
`201`	`201`	`{`
`202`		`- usageTracker.IncrementCloneCount();`
	`202`	`+ usageTracker.IncrementCloneCount().Forget();`
`203`	`203`	`return Observable.Return(Unit.Default);`
`204`	`204`	`});`
`205`	`205`	`})`
Original file line number	Diff line number	Diff line change
`@@ -268,7 +268,7 @@ IObservable<Unit> OnCreateRepository(object state)`
`268`	`268`	`SelectedAccount,`
`269`	`269`	`BaseRepositoryPath,`
`270`	`270`	`repositoryHost.ApiClient)`
`271`		`- .Do(_ => usageTracker.IncrementCreateCount());`
	`271`	`+ .Do(_ => usageTracker.IncrementCreateCount().Forget());`
`272`	`272`	`}`
`273`	`273`
`274`	`274`	`ReactiveCommand<Unit> InitializeCreateRepositoryCommand()`
Original file line number	Diff line number	Diff line change
`@@ -153,7 +153,7 @@ IObservable<ProgressState> OnPublishRepository(object arg)`
`153`	`153`	`var account = SelectedAccount;`
`154`	`154`
`155`	`155`	`return repositoryPublishService.PublishRepository(newRepository, account, SelectedHost.ApiClient)`
`156`		`- .Do(_ => usageTracker.IncrementPublishCount())`
	`156`	`+ .Do(_ => usageTracker.IncrementPublishCount().Forget())`
`157`	`157`	`.Select(_ => ProgressState.Success)`
`158`	`158`	`.Catch<ProgressState, Exception>(ex =>`
`159`	`159`	`{`
Original file line number	Diff line number	Diff line change
`@@ -105,7 +105,7 @@ IObservable<Unit> OnCloneRepository(object state)`
`105`	`105`	`return cloneService.CloneRepository(repository.CloneUrl, repository.Name, BaseRepositoryPath)`
`106`	`106`	`.ContinueAfter(() =>`
`107`	`107`	`{`
`108`		`- usageTracker.IncrementCloneCount();`
	`108`	`+ usageTracker.IncrementCloneCount().Forget();`
`109`	`109`	`return Observable.Return(Unit.Default);`
`110`	`110`	`});`
`111`	`111`	`})`
Original file line number	Diff line number	Diff line change
`@@ -31,7 +31,7 @@ public async void Activate([AllowNull]object data = null)`
`31`	`31`	`Clipboard.SetText(link);`
`32`	`32`	`var ns = ServiceProvider.TryGetService<IStatusBarNotificationService>();`
`33`	`33`	`ns?.ShowMessage(Resources.LinkCopiedToClipboardMessage);`
`34`		`- UsageTracker.IncrementLinkToGitHubCount();`
	`34`	`+ await UsageTracker.IncrementLinkToGitHubCount();`
`35`	`35`	`}`
`36`	`36`	`catch`
`37`	`37`	`{`
Original file line number	Diff line number	Diff line change
`@@ -26,7 +26,7 @@ public async void Activate([AllowNull]object data = null)`
`26`	`26`	`var browser = ServiceProvider.TryGetService<IVisualStudioBrowser>();`
`27`	`27`	`browser?.OpenUrl(link.ToUri());`
`28`	`28`
`29`		`- UsageTracker.IncrementOpenInGitHubCount();`
	`29`	`+ await UsageTracker.IncrementOpenInGitHubCount();`
`30`	`30`	`}`
`31`	`31`
`32`	`32`	`public bool CanShow()`