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

Commit 60354ce

Browse files
jcansdalejcansdale
authored
Merge branch 'master' into fixes/1576-open-pr-on-github
2 parents acec87b + b755fca commit 60354ce

File tree

26 files changed

+517
-167
lines changed

26 files changed

+517
-167
lines changed

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

Lines changed: 68 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,14 @@
11
using System;
22
using System.ComponentModel.Composition;
3-
using System.Diagnostics;
43
using System.Globalization;
54
using System.Net;
65
using System.Net.Http;
76
using System.Reactive.Linq;
87
using System.Reactive.Threading.Tasks;
8+
using System.Runtime.Serialization;
9+
using System.Diagnostics.CodeAnalysis;
10+
using System.Collections.Generic;
11+
using System.Threading.Tasks;
912
using GitHub.Logging;
1013
using Octokit;
1114
using Octokit.Internal;
@@ -17,14 +20,61 @@ namespace GitHub.Services
1720
public class ImageDownloader : IImageDownloader
1821
{
1922
readonly Lazy<IHttpClient> httpClient;
23+
readonly IDictionary<string, NonImageContentException> exceptionCache;
2024

2125
[ImportingConstructor]
2226
public ImageDownloader(Lazy<IHttpClient> httpClient)
2327
{
2428
this.httpClient = httpClient;
29+
exceptionCache = new Dictionary<string, NonImageContentException>();
2530
}
2631

32+
public static string CachedExceptionMessage(string host) =>
33+
string.Format(CultureInfo.InvariantCulture, "Host '{0}' previously returned a non-image content type", host);
34+
public static string CouldNotDownloadExceptionMessage(Uri imageUri) =>
35+
string.Format(CultureInfo.InvariantCulture, "Could not download image from '{0}'", imageUri);
36+
public static string NonImageContentExceptionMessage(string contentType) =>
37+
string.Format(CultureInfo.InvariantCulture, "Server responded with a non-image content type '{0}'", contentType);
38+
39+
/// <summary>
40+
/// Get the bytes for a given image URI.
41+
/// </summary>
42+
/// <remarks>
43+
/// If a host returns a non-image content type, this will be remembered and subsequent download requests
44+
/// to the same host will automatically throw a <see cref="NonImageContentException"/>. This prevents a
45+
/// barrage of download requests when authentication is required (but not currently supported).
46+
/// </remarks>
47+
/// <param name="imageUri">The URI of an image.</param>
48+
/// <returns>The bytes for a given image URI.</returns>
49+
/// <exception cref="HttpRequestException">When the URI returns a status code that isn't OK/200.</exception>
50+
/// <exception cref="NonImageContentException">When the URI returns a non-image content type.</exception>
2751
public IObservable<byte[]> DownloadImageBytes(Uri imageUri)
52+
{
53+
return ExceptionCachingDownloadImageBytesAsync(imageUri).ToObservable();
54+
}
55+
56+
async Task<byte[]> ExceptionCachingDownloadImageBytesAsync(Uri imageUri)
57+
{
58+
var host = imageUri.Host;
59+
60+
NonImageContentException exception;
61+
if (exceptionCache.TryGetValue(host, out exception))
62+
{
63+
throw new NonImageContentException(CachedExceptionMessage(host), exception);
64+
}
65+
66+
try
67+
{
68+
return await DownloadImageBytesAsync(imageUri);
69+
}
70+
catch (NonImageContentException e)
71+
{
72+
exceptionCache[host] = e;
73+
throw;
74+
}
75+
}
76+
77+
async Task<byte[]> DownloadImageBytesAsync(Uri imageUri)
2878
{
2979
var request = new Request
3080
{
@@ -33,9 +83,8 @@ public IObservable<byte[]> DownloadImageBytes(Uri imageUri)
3383
Method = HttpMethod.Get,
3484
};
3585

36-
return HttpClient.Send(request)
37-
.ToObservable()
38-
.Select(response => GetSuccessfulBytes(imageUri, response));
86+
var response = await HttpClient.Send(request);
87+
return GetSuccessfulBytes(imageUri, response);
3988
}
4089

4190
static byte[] GetSuccessfulBytes(Uri imageUri, IResponse response)
@@ -45,19 +94,29 @@ static byte[] GetSuccessfulBytes(Uri imageUri, IResponse response)
4594

4695
if (response.StatusCode != HttpStatusCode.OK)
4796
{
48-
throw new HttpRequestException(string.Format(CultureInfo.InvariantCulture, "Could not download image from {0}", imageUri));
97+
throw new HttpRequestException(CouldNotDownloadExceptionMessage(imageUri));
4998
}
5099

51100
if (response.ContentType == null || !response.ContentType.StartsWith("image/", StringComparison.OrdinalIgnoreCase))
52101
{
53-
throw new HttpRequestException(
54-
string.Format(CultureInfo.InvariantCulture,
55-
"Server responded with a non-image content type: {0}", response.ContentType));
102+
throw new NonImageContentException(NonImageContentExceptionMessage(response.ContentType));
56103
}
57104

58105
return response.Body as byte[];
59106
}
60107

61108
IHttpClient HttpClient { get { return httpClient.Value; } }
62109
}
63-
}
110+
111+
[Serializable]
112+
public class NonImageContentException : HttpRequestException
113+
{
114+
public NonImageContentException() { }
115+
public NonImageContentException(string message) : base(message) { }
116+
public NonImageContentException(string message, Exception inner) : base(message, inner) { }
117+
118+
[SuppressMessage("Microsoft.Usage", "CA2236:CallBaseClassMethodsOnISerializableTypes",
119+
Justification = "HttpRequestException doesn't have required constructor")]
120+
protected NonImageContentException(SerializationInfo info, StreamingContext context) { }
121+
}
122+
}

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

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -326,15 +326,15 @@ public async Task InitializeAsync(
326326
string repo,
327327
int number)
328328
{
329-
if (repo != localRepository.Name)
330-
{
331-
throw new NotSupportedException();
332-
}
333-
334329
IsLoading = true;
335330

336331
try
337332
{
333+
if (!string.Equals(repo, localRepository.Name, StringComparison.OrdinalIgnoreCase))
334+
{
335+
throw new NotSupportedException("Showing pull requests from other repositories not yet supported.");
336+
}
337+
338338
LocalRepository = localRepository;
339339
RemoteRepositoryOwner = owner;
340340
Number = number;
@@ -344,6 +344,10 @@ public async Task InitializeAsync(
344344
await Refresh();
345345
teamExplorerContext.StatusChanged += RefreshIfActive;
346346
}
347+
catch (Exception ex)
348+
{
349+
Error = ex;
350+
}
347351
finally
348352
{
349353
IsLoading = false;

‎src/GitHub.Exports/Settings/Guids.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ public static class Guids
1313
public const string CodeContainerProviderId = "6CE146CB-EF57-4F2C-A93F-5BA685317660";
1414
public const string InlineReviewsPackageId = "248325BE-4A2D-4111-B122-E7D59BF73A35";
1515
public const string PullRequestStatusPackageId = "5121BEC6-1088-4553-8453-0DDC7C8E2238";
16+
public const string GitHubPanePackageId = "0A40459D-6B6D-4110-B6CE-EC83C0BC6A09";
1617
public const string TeamExplorerWelcomeMessage = "C529627F-8AA6-4FDB-82EB-4BFB7DB753C3";
1718
public const string LoginManagerId = "7BA2071A-790A-4F95-BE4A-0EEAA5928AAF";
1819

‎src/GitHub.Exports/Settings/generated/IPackageSettings.cs‎

Lines changed: 25 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,34 @@
11
// This is an automatically generated file, based on settings.json and PackageSettingsGen.tt
22
/* settings.json content:
33
{
4-
"settings": [
4+
"settings": [
55
{
6-
"name": "CollectMetrics",
7-
"type": "bool",
8-
"default": 'true'
6+
"name": "CollectMetrics",
7+
"type": "bool",
8+
"default": 'true'
99
},
1010
{
11-
"name": "UIState",
12-
"type": "object",
13-
"typename": "UIState",
14-
"default": 'null'
11+
"name": "EditorComments",
12+
"type": "bool",
13+
"default": "false"
1514
},
16-
{
17-
"name": "HideTeamExplorerWelcomeMessage",
18-
"type": "bool",
19-
"default": 'false'
20-
}
21-
]
15+
{
16+
"name": "UIState",
17+
"type": "object",
18+
"typename": "UIState",
19+
"default": "null"
20+
},
21+
{
22+
"name": "HideTeamExplorerWelcomeMessage",
23+
"type": "bool",
24+
"default": "false"
25+
},
26+
{
27+
"name": "EnableTraceLogging",
28+
"type": "bool",
29+
"default": "false"
30+
}
31+
]
2232
}
2333
*/
2434

@@ -33,5 +43,6 @@ public interface IPackageSettings : INotifyPropertyChanged
3343
bool EditorComments { get; set; }
3444
UIState UIState { get; set; }
3545
bool HideTeamExplorerWelcomeMessage { get; set; }
46+
bool EnableTraceLogging { get; set; }
3647
}
3748
}

‎src/GitHub.InlineReviews/InlineReviewsPackage.cs‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,14 +35,17 @@ protected override async Task InitializeAsync(
3535

3636
async Task InitializeMenus()
3737
{
38-
var menuService = (IMenuCommandService)(await GetServiceAsync(typeof(IMenuCommandService)));
3938
var componentModel = (IComponentModel)(await GetServiceAsync(typeof(SComponentModel)));
4039
var exports = componentModel.DefaultExportProvider;
40+
var commands = new IVsCommandBase[]
41+
{
42+
exports.GetExportedValue<INextInlineCommentCommand>(),
43+
exports.GetExportedValue<IPreviousInlineCommentCommand>()
44+
};
4145

4246
await JoinableTaskFactory.SwitchToMainThreadAsync();
43-
menuService.AddCommands(
44-
exports.GetExportedValue<INextInlineCommentCommand>(),
45-
exports.GetExportedValue<IPreviousInlineCommentCommand>());
47+
var menuService = (IMenuCommandService)(await GetServiceAsync(typeof(IMenuCommandService)));
48+
menuService.AddCommands(commands);
4649
}
4750
}
4851
}

‎src/GitHub.InlineReviews/PullRequestStatusBarPackage.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
using System;
22
using System.Threading;
33
using System.Runtime.InteropServices;
4-
using GitHub.Services;
54
using GitHub.VisualStudio;
65
using GitHub.InlineReviews.Services;
76
using Microsoft.VisualStudio.Shell;
87
using Task = System.Threading.Tasks.Task;
98
using Microsoft.VisualStudio.Threading;
9+
using Microsoft.VisualStudio.ComponentModelHost;
1010

1111
namespace GitHub.InlineReviews
1212
{
@@ -27,9 +27,9 @@ protected override async Task InitializeAsync(CancellationToken cancellationToke
2727

2828
async Task InitializeStatusBar()
2929
{
30-
var usageTracker = (IUsageTracker)await GetServiceAsync(typeof(IUsageTracker));
31-
var serviceProvider = (IGitHubServiceProvider)await GetServiceAsync(typeof(IGitHubServiceProvider));
32-
var barManager = new PullRequestStatusBarManager(usageTracker, serviceProvider);
30+
var componentModel = (IComponentModel)(await GetServiceAsync(typeof(SComponentModel)));
31+
var exports = componentModel.DefaultExportProvider;
32+
var barManager = exports.GetExportedValue<PullRequestStatusBarManager>();
3333

3434
await JoinableTaskFactory.SwitchToMainThreadAsync();
3535
barManager.StartShowingStatus();

‎src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs‎

Lines changed: 14 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,14 @@
11
using System;
22
using System.Windows;
3-
using System.Windows.Input;
43
using System.Windows.Controls;
54
using System.Windows.Controls.Primitives;
65
using System.ComponentModel.Composition;
6+
using GitHub.Commands;
77
using GitHub.InlineReviews.Views;
88
using GitHub.InlineReviews.ViewModels;
99
using GitHub.Services;
10-
using GitHub.VisualStudio;
1110
using GitHub.Models;
1211
using GitHub.Logging;
13-
using GitHub.Extensions;
1412
using Serilog;
1513
using ReactiveUI;
1614

@@ -19,21 +17,25 @@ namespace GitHub.InlineReviews.Services
1917
/// <summary>
2018
/// Manage the UI that shows the PR for the current branch.
2119
/// </summary>
20+
[Export(typeof(PullRequestStatusBarManager))]
2221
public class PullRequestStatusBarManager
2322
{
2423
static readonly ILogger log = LogManager.ForContext<PullRequestStatusBarManager>();
2524
const string StatusBarPartName = "PART_SccStatusBarHost";
2625

27-
readonly IUsageTracker usageTracker;
28-
readonly IGitHubServiceProvider serviceProvider;
26+
readonly IShowCurrentPullRequestCommand showCurrentPullRequestCommand;
2927

30-
IPullRequestSessionManager pullRequestSessionManager;
28+
// At the moment this must be constructed on the main thread.
29+
// TeamExplorerContext needs to retrieve DTE using GetService.
30+
readonly Lazy<IPullRequestSessionManager> pullRequestSessionManager;
3131

3232
[ImportingConstructor]
33-
public PullRequestStatusBarManager(IUsageTracker usageTracker, IGitHubServiceProvider serviceProvider)
33+
public PullRequestStatusBarManager(
34+
IShowCurrentPullRequestCommand showCurrentPullRequestCommand,
35+
Lazy<IPullRequestSessionManager> pullRequestSessionManager)
3436
{
35-
this.usageTracker = usageTracker;
36-
this.serviceProvider = serviceProvider;
37+
this.showCurrentPullRequestCommand = showCurrentPullRequestCommand;
38+
this.pullRequestSessionManager = pullRequestSessionManager;
3739
}
3840

3941
/// <summary>
@@ -46,8 +48,7 @@ public void StartShowingStatus()
4648
{
4749
try
4850
{
49-
pullRequestSessionManager = serviceProvider.GetService<IPullRequestSessionManager>();
50-
pullRequestSessionManager.WhenAnyValue(x => x.CurrentSession)
51+
pullRequestSessionManager.Value.WhenAnyValue(x => x.CurrentSession)
5152
.Subscribe(x => RefreshCurrentSession());
5253
}
5354
catch (Exception e)
@@ -58,16 +59,14 @@ public void StartShowingStatus()
5859

5960
void RefreshCurrentSession()
6061
{
61-
var pullRequest = pullRequestSessionManager.CurrentSession?.PullRequest;
62+
var pullRequest = pullRequestSessionManager.Value.CurrentSession?.PullRequest;
6263
var viewModel = pullRequest != null ? CreatePullRequestStatusViewModel(pullRequest) : null;
6364
ShowStatus(viewModel);
6465
}
6566

6667
PullRequestStatusViewModel CreatePullRequestStatusViewModel(IPullRequestModel pullRequest)
6768
{
68-
var dte = serviceProvider.TryGetService<EnvDTE.DTE>();
69-
var command = new RaisePullRequestCommand(dte, usageTracker);
70-
var pullRequestStatusViewModel = new PullRequestStatusViewModel(command);
69+
var pullRequestStatusViewModel = new PullRequestStatusViewModel(showCurrentPullRequestCommand);
7170
pullRequestStatusViewModel.Number = pullRequest.Number;
7271
pullRequestStatusViewModel.Title = pullRequest.Title;
7372
return pullRequestStatusViewModel;
@@ -111,44 +110,5 @@ StatusBar FindSccStatusBar(Window mainWindow)
111110
var contentControl = mainWindow?.Template?.FindName(StatusBarPartName, mainWindow) as ContentControl;
112111
return contentControl?.Content as StatusBar;
113112
}
114-
115-
class RaisePullRequestCommand : ICommand
116-
{
117-
readonly string guid = Guids.guidGitHubCmdSetString;
118-
readonly int id = PkgCmdIDList.showCurrentPullRequestCommand;
119-
120-
readonly EnvDTE.DTE dte;
121-
readonly IUsageTracker usageTracker;
122-
123-
internal RaisePullRequestCommand(EnvDTE.DTE dte, IUsageTracker usageTracker)
124-
{
125-
this.dte = dte;
126-
this.usageTracker = usageTracker;
127-
}
128-
129-
public bool CanExecute(object parameter) => true;
130-
131-
public void Execute(object parameter)
132-
{
133-
try
134-
{
135-
object customIn = null;
136-
object customOut = null;
137-
dte?.Commands.Raise(guid, id, ref customIn, ref customOut);
138-
}
139-
catch (Exception e)
140-
{
141-
log.Error(e, "Couldn't raise {Guid}:{ID}", guid, id);
142-
}
143-
144-
usageTracker.IncrementCounter(x => x.NumberOfShowCurrentPullRequest).Forget();
145-
}
146-
147-
public event EventHandler CanExecuteChanged
148-
{
149-
add { }
150-
remove { }
151-
}
152-
}
153113
}
154114
}

0 commit comments

Comments
 (0)