Skip to content
This repository was archived by the owner on Jun 21, 2023. It is now read-only.

Commit 617be8e

Browse files
grokysgrokys
authored
Merge branch 'master' into update-authenticating-to-github
2 parents 19bdb37 + ee77977 commit 617be8e

File tree

7 files changed

+110
-31
lines changed

7 files changed

+110
-31
lines changed

‎src/GitHub.App/Services/EnterpriseCapabilitiesService.cs‎

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -30,22 +30,29 @@ public EnterpriseCapabilitiesService(
3030

3131
public async Task<EnterpriseLoginMethods> ProbeLoginMethods(Uri enterpriseBaseUrl)
3232
{
33-
// It's important that we don't use our cached credentials on this connection, as they
34-
// may be wrong - we're trying to log in after all.
35-
var hostAddress = HostAddress.Create(enterpriseBaseUrl);
36-
var connection = new Octokit.Connection(program.ProductHeader, hostAddress.ApiUri);
37-
var meta = await GetMetadata(connection).ConfigureAwait(false);
38-
var result = EnterpriseLoginMethods.Token;
33+
try
34+
{
35+
// It's important that we don't use our cached credentials on this connection, as they
36+
// may be wrong - we're trying to log in after all.
37+
var hostAddress = HostAddress.Create(enterpriseBaseUrl);
38+
var connection = new Octokit.Connection(program.ProductHeader, hostAddress.ApiUri);
39+
var meta = await GetMetadata(connection).ConfigureAwait(false);
40+
var result = EnterpriseLoginMethods.Token;
41+
42+
if (meta.VerifiablePasswordAuthentication != false) result |= EnterpriseLoginMethods.UsernameAndPassword;
3943

40-
if (meta.VerifiablePasswordAuthentication) result |= EnterpriseLoginMethods.UsernameAndPassword;
44+
if (meta.InstalledVersion != null)
45+
{
46+
var version = new Version(meta.InstalledVersion);
47+
if (version >= MinimumOAuthVersion) result |= EnterpriseLoginMethods.OAuth;
48+
}
4149

42-
if (meta.InstalledVersion != null)
50+
return result;
51+
}
52+
catch
4353
{
44-
var version = new Version(meta.InstalledVersion);
45-
if (version >= MinimumOAuthVersion) result |= EnterpriseLoginMethods.OAuth;
54+
return EnterpriseLoginMethods.Token | EnterpriseLoginMethods.UsernameAndPassword;
4655
}
47-
48-
return result;
4956
}
5057

5158
private async Task<EnterpriseMeta> GetMetadata(IConnection connection)
@@ -56,9 +63,10 @@ private async Task<EnterpriseMeta> GetMetadata(IConnection connection)
5663
}
5764

5865
[SuppressMessage("Microsoft.Performance", "CA1812:AvoidUninstantiatedInternalClasses", Justification = "Created via Octokit reflection")]
59-
class EnterpriseMeta : Meta
66+
class EnterpriseMeta
6067
{
6168
public string InstalledVersion { get; private set; }
69+
public bool? VerifiablePasswordAuthentication { get; private set; }
6270
}
6371
}
6472
}

‎src/GitHub.App/Services/TeamExplorerContext.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ void Refresh()
7676

7777
if (newRepositoryPath != repositoryPath)
7878
{
79-
log.Debug("Fire PropertyChanged event for ActiveRepository");
79+
log.Debug("ActiveRepository changed to {CloneUrl} @ {Path}", repo?.CloneUrl, newRepositoryPath);
8080
ActiveRepository = repo;
8181
}
8282
else if (newBranchName != branchName)

‎src/GitHub.App/ViewModels/Dialog/LoginToGitHubForEnterpriseViewModel.cs‎

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -140,22 +140,14 @@ async void EnterpriseUrlChanged(string url, bool valid)
140140

141141
var enterpriseInstance = false;
142142
var loginMethods = (EnterpriseLoginMethods?)null;
143+
var uri = new UriBuilder(url).Uri;
143144

144-
try
145-
{
146-
var uri = new UriBuilder(url).Uri;
147-
ProbeStatus = EnterpriseProbeStatus.Checking;
145+
ProbeStatus = EnterpriseProbeStatus.Checking;
148146

149-
if (await enterpriseCapabilities.Probe(uri) == EnterpriseProbeResult.Ok)
150-
{
151-
loginMethods = await enterpriseCapabilities.ProbeLoginMethods(uri);
152-
enterpriseInstance = true;
153-
}
154-
}
155-
catch
147+
if (await enterpriseCapabilities.Probe(uri) == EnterpriseProbeResult.Ok)
156148
{
157-
ProbeStatus = EnterpriseProbeStatus.Invalid;
158-
loginMethods = null;
149+
loginMethods = await enterpriseCapabilities.ProbeLoginMethods(uri);
150+
enterpriseInstance = true;
159151
}
160152

161153
if (url == EnterpriseUrl)

‎src/GitHub.App/ViewModels/GitHubPane/GitHubPaneViewModel.cs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,13 @@
1111
using GitHub.Extensions;
1212
using GitHub.Factories;
1313
using GitHub.Info;
14+
using GitHub.Logging;
1415
using GitHub.Models;
1516
using GitHub.Primitives;
1617
using GitHub.Services;
1718
using GitHub.VisualStudio;
1819
using ReactiveUI;
20+
using Serilog;
1921
using OleMenuCommand = Microsoft.VisualStudio.Shell.OleMenuCommand;
2022

2123
namespace GitHub.ViewModels.GitHubPane
@@ -27,6 +29,7 @@ namespace GitHub.ViewModels.GitHubPane
2729
[PartCreationPolicy(CreationPolicy.NonShared)]
2830
public sealed class GitHubPaneViewModel : ViewModelBase, IGitHubPaneViewModel, IDisposable
2931
{
32+
static readonly ILogger log = LogManager.ForContext<GitHubPaneViewModel>();
3033
static readonly Regex pullUri = CreateRoute("/:owner/:repo/pull/:number");
3134

3235
readonly IViewViewModelFactory viewModelFactory;
@@ -360,18 +363,22 @@ async Task NavigateTo<TViewModel>(Func<TViewModel, Task> initialize, Func<TViewM
360363

361364
async Task UpdateContent(ILocalRepositoryModel repository)
362365
{
366+
log.Debug("UpdateContent called with {CloneUrl}", repository?.CloneUrl);
367+
363368
LocalRepository = repository;
364369
Connection = null;
365370
Content = null;
366371
navigator.Clear();
367372

368373
if (repository == null)
369374
{
375+
log.Debug("Not a git repository: {CloneUrl}", repository?.CloneUrl);
370376
Content = notAGitRepository;
371377
return;
372378
}
373379
else if (string.IsNullOrWhiteSpace(repository.CloneUrl))
374380
{
381+
log.Debug("Not a GitHub repository: {CloneUrl}", repository?.CloneUrl);
375382
Content = notAGitHubRepository;
376383
return;
377384
}
@@ -389,16 +396,19 @@ async Task UpdateContent(ILocalRepositoryModel repository)
389396

390397
if (Connection?.IsLoggedIn == true)
391398
{
399+
log.Debug("Found a GitHub repository: {CloneUrl}", repository?.CloneUrl);
392400
Content = navigator;
393401
await ShowDefaultPage();
394402
}
395403
else
396404
{
405+
log.Debug("Found a a GitHub repository but not logged in: {CloneUrl}", repository?.CloneUrl);
397406
Content = loggedOut;
398407
}
399408
}
400409
else
401410
{
411+
log.Debug("Not a GitHub repository: {CloneUrl}", repository?.CloneUrl);
402412
Content = notAGitHubRepository;
403413
}
404414
}

‎src/GitHub.TeamFoundation.14/Services/VSGitExt.cs‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ public class VSGitExt : IVSGitExt
2424
readonly IGitHubServiceProvider serviceProvider;
2525
readonly IVSUIContext context;
2626
readonly ILocalRepositoryModelFactory repositoryFactory;
27+
readonly object refreshLock = new object();
2728

2829
IGitExt gitService;
2930
IReadOnlyList<ILocalRepositoryModel> activeRepositories;
@@ -98,7 +99,15 @@ void RefreshActiveRepositories()
9899
{
99100
try
100101
{
101-
ActiveRepositories = gitService?.ActiveRepositories.Select(x => repositoryFactory.Create(x.RepositoryPath)).ToList();
102+
lock (refreshLock)
103+
{
104+
log.Debug(
105+
"IGitExt.ActiveRepositories (#{Id}) returned {Repositories}",
106+
gitService.GetHashCode(),
107+
gitService?.ActiveRepositories.Select(x => x.RepositoryPath));
108+
109+
ActiveRepositories = gitService?.ActiveRepositories.Select(x => repositoryFactory.Create(x.RepositoryPath)).ToList();
110+
}
102111
}
103112
catch (Exception e)
104113
{
@@ -118,6 +127,7 @@ private set
118127
{
119128
if (value != activeRepositories)
120129
{
130+
log.Debug("ActiveRepositories changed to {Repositories}", value?.Select(x => x.CloneUrl));
121131
activeRepositories = value;
122132
ActiveRepositoriesChanged?.Invoke();
123133
}

‎src/GitHub.VisualStudio/source.extension.vsixmanifest‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
<PackageId>GitHub.VisualStudio</PackageId>
88
<MoreInfo>https://visualstudio.github.com</MoreInfo>
99
<License>LICENSE.txt</License>
10+
<ReleaseNotes>https://visualstudio.github.com/release-notes.html</ReleaseNotes>
1011
<Icon>Resources\[email protected]</Icon>
1112
<PreviewImage>Resources\preview_200x200.png</PreviewImage>
1213
<Tags>GitHub;git;open source;source control;branch;pull request;team explorer;commit;publish</Tags>

‎test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs‎

Lines changed: 61 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@
99
using NUnit.Framework;
1010
using NSubstitute;
1111
using Microsoft.VisualStudio.TeamFoundation.Git.Extensibility;
12+
using System.Threading.Tasks;
13+
using System.Linq;
14+
using GitHub.TeamFoundation.Services;
1215

1316
public class VSGitExtTests
1417
{
@@ -171,9 +174,26 @@ public void ExceptionRefreshingRepositories_ReturnsEmptyList()
171174
repoFactory.Received(1).Create(repoPath);
172175
Assert.That(activeRepositories.Count, Is.EqualTo(0));
173176
}
177+
178+
[Test]
179+
public async Task ActiveRepositoriesChangedOrderingShouldBeCorrectAcrossThreads()
180+
{
181+
var gitExt = new MockGitExt();
182+
var repoFactory = new MockRepositoryFactory();
183+
var target = CreateVSGitExt(gitExt: gitExt, repoFactory: repoFactory);
184+
var activeRepositories1 = CreateActiveRepositories("repo1");
185+
var activeRepositories2 = CreateActiveRepositories("repo2");
186+
var task1 = Task.Run(() => gitExt.ActiveRepositories = activeRepositories1);
187+
await Task.Delay(1);
188+
var task2 = Task.Run(() => gitExt.ActiveRepositories = activeRepositories2);
189+
190+
await Task.WhenAll(task1, task2);
191+
192+
Assert.That(target.ActiveRepositories.Single().LocalPath, Is.EqualTo("repo2"));
193+
}
174194
}
175195

176-
static IReadOnlyList<IGitRepositoryInfo> CreateActiveRepositories(IList<string> repositoryPaths)
196+
static IReadOnlyList<IGitRepositoryInfo> CreateActiveRepositories(params string[] repositoryPaths)
177197
{
178198
var repositories = new List<IGitRepositoryInfo>();
179199
foreach (var repositoryPath in repositoryPaths)
@@ -203,9 +223,8 @@ static VSGitExt CreateVSGitExt(IVSUIContext context = null, IGitExt gitExt = nul
203223
return vsGitExt;
204224
}
205225

206-
static IGitExt CreateGitExt(IList<string> repositoryPaths = null)
226+
static IGitExt CreateGitExt(params string[] repositoryPaths)
207227
{
208-
repositoryPaths = repositoryPaths ?? Array.Empty<string>();
209228
var gitExt = Substitute.For<IGitExt>();
210229
var repoList = CreateActiveRepositories(repositoryPaths);
211230
gitExt.ActiveRepositories.Returns(repoList);
@@ -218,4 +237,43 @@ static IVSUIContext CreateVSUIContext(bool isActive)
218237
context.IsActive.Returns(isActive);
219238
return context;
220239
}
240+
241+
class MockGitExt : IGitExt
242+
{
243+
IReadOnlyList<IGitRepositoryInfo> activeRepositories = new IGitRepositoryInfo[0];
244+
245+
public IReadOnlyList<IGitRepositoryInfo> ActiveRepositories
246+
{
247+
get { return activeRepositories; }
248+
set
249+
{
250+
if (activeRepositories != value)
251+
{
252+
activeRepositories = value;
253+
PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(ActiveRepositories)));
254+
}
255+
}
256+
}
257+
258+
public event PropertyChangedEventHandler PropertyChanged;
259+
}
260+
261+
class MockRepositoryFactory : ILocalRepositoryModelFactory
262+
{
263+
public ILocalRepositoryModel Create(string localPath)
264+
{
265+
var result = Substitute.For<ILocalRepositoryModel>();
266+
result.LocalPath.Returns(localPath);
267+
268+
if (localPath == "repo1")
269+
{
270+
// Trying to force #1493 here by introducing a a delay on the first
271+
// ActiveRepositories changed notification so that the second completes
272+
// first.
273+
Thread.Sleep(10);
274+
}
275+
276+
return result;
277+
}
278+
}
221279
}

0 commit comments

Comments
 (0)