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

Commit dec0a62

Browse files
jcansdalejcansdale
authored
Merge branch 'master' into enhancement/1591-status-bar-open-PRs
2 parents 211719b + b755fca commit dec0a62

File tree

17 files changed

+392
-72
lines changed

17 files changed

+392
-72
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.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.Logging/GitHub.Logging.csproj‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,10 @@
5151
<HintPath>..\..\packages\Serilog.2.5.0\lib\net46\Serilog.dll</HintPath>
5252
<Private>True</Private>
5353
</Reference>
54+
<Reference Include="Serilog.Enrichers.Process, Version=2.0.0.0, Culture=neutral, PublicKeyToken=24c2f752a8e58a10, processorArchitecture=MSIL">
55+
<HintPath>..\..\packages\Serilog.Enrichers.Process.2.0.1\lib\net45\Serilog.Enrichers.Process.dll</HintPath>
56+
<Private>True</Private>
57+
</Reference>
5458
<Reference Include="Serilog.Enrichers.Thread, Version=2.0.0.0, Culture=neutral, PublicKeyToken=24c2f752a8e58a10, processorArchitecture=MSIL">
5559
<HintPath>..\..\packages\Serilog.Enrichers.Thread.3.0.0\lib\net45\Serilog.Enrichers.Thread.dll</HintPath>
5660
<Private>True</Private>

‎src/GitHub.Logging/Logging/LogManager.cs‎

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,20 @@
44
using GitHub.Info;
55
using Serilog;
66
using Serilog.Core;
7+
using Serilog.Events;
78

89
namespace GitHub.Logging
910
{
1011
public static class LogManager
1112
{
13+
#if DEBUG
14+
private static LogEventLevel DefaultLoggingLevel = LogEventLevel.Debug;
15+
#else
16+
private static LogEventLevel DefaultLoggingLevel = LogEventLevel.Information;
17+
#endif
18+
19+
private static LoggingLevelSwitch LoggingLevelSwitch = new LoggingLevelSwitch(DefaultLoggingLevel);
20+
1221
static Logger CreateLogger()
1322
{
1423
var logPath = Path.Combine(
@@ -17,21 +26,29 @@ static Logger CreateLogger()
1726
"extension.log");
1827

1928
const string outputTemplate =
20-
"{Timestamp:yyyy-MM-dd HH:mm:ss.fff} {Level:u4} [{ThreadId:00}] {ShortSourceContext,-25} {Message:lj}{NewLine}{Exception}";
29+
"{Timestamp:yyyy-MM-dd HH:mm:ss.fff} [{ProcessId:00000}] {Level:u4} [{ThreadId:00}] {ShortSourceContext,-25} {Message:lj}{NewLine}{Exception}";
2130

2231
return new LoggerConfiguration()
32+
.Enrich.WithProcessId()
2333
.Enrich.WithThreadId()
24-
#if DEBUG
25-
.MinimumLevel.Debug()
26-
#else
27-
.MinimumLevel.Information()
28-
#endif
34+
.MinimumLevel.ControlledBy(LoggingLevelSwitch)
2935
.WriteTo.File(logPath,
3036
fileSizeLimitBytes: null,
31-
outputTemplate: outputTemplate)
37+
outputTemplate: outputTemplate,
38+
shared: true)
3239
.CreateLogger();
3340
}
3441

42+
public static void EnableTraceLogging(bool enable)
43+
{
44+
var logEventLevel = enable ? LogEventLevel.Verbose : DefaultLoggingLevel;
45+
if(LoggingLevelSwitch.MinimumLevel != logEventLevel)
46+
{
47+
ForContext(typeof(LogManager)).Information("Set Logging Level: {LogEventLevel}", logEventLevel);
48+
LoggingLevelSwitch.MinimumLevel = logEventLevel;
49+
}
50+
}
51+
3552
static Lazy<Logger> Logger { get; } = new Lazy<Logger>(CreateLogger);
3653

3754
[SuppressMessage("Microsoft.Design", "CA1004:GenericMethodsShouldProvideTypeParameter")]

‎src/GitHub.Logging/packages.config‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
<?xml version="1.0" encoding="utf-8"?>
22
<packages>
33
<package id="Serilog" version="2.5.0" targetFramework="net461" />
4+
<package id="Serilog.Enrichers.Process" version="2.0.1" targetFramework="net461" />
45
<package id="Serilog.Enrichers.Thread" version="3.0.0" targetFramework="net452" />
56
<package id="Serilog.Sinks.File" version="3.2.0" targetFramework="net452" />
67
<package id="SerilogAnalyzer" version="0.12.0.0" targetFramework="net461" />

‎src/GitHub.VisualStudio.UI/Resources.Designer.cs‎

Lines changed: 18 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/GitHub.VisualStudio.UI/Resources.resx‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -401,4 +401,10 @@
401401
<data name="TokenPrompt" xml:space="preserve">
402402
<value>Token</value>
403403
</data>
404+
<data name="Options_DebuggingTitle" xml:space="preserve">
405+
<value>Debugging</value>
406+
</data>
407+
<data name="Options_EnableTraceLoggingText" xml:space="preserve">
408+
<value>Enable Trace Logging</value>
409+
</data>
404410
</root>

‎src/GitHub.VisualStudio/GitHubPackage.cs‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,10 @@
1111
using GitHub.Exports;
1212
using GitHub.Logging;
1313
using GitHub.Services;
14+
using GitHub.Settings;
1415
using GitHub.Services.Vssdk.Commands;
1516
using GitHub.ViewModels.GitHubPane;
17+
using GitHub.VisualStudio.Settings;
1618
using GitHub.VisualStudio.UI;
1719
using Microsoft.VisualStudio;
1820
using Microsoft.VisualStudio.ComponentModelHost;
@@ -37,13 +39,31 @@ protected override async Task InitializeAsync(CancellationToken cancellationToke
3739
{
3840
LogVersionInformation();
3941
await base.InitializeAsync(cancellationToken, progress);
42+
43+
await InitializeLoggingAsync();
4044
await GetServiceAsync(typeof(IUsageTracker));
4145

4246
// Avoid delays when there is ongoing UI activity.
4347
// See: https://github.com/github/VisualStudio/issues/1537
4448
await JoinableTaskFactory.RunAsync(VsTaskRunContext.UIThreadNormalPriority, InitializeMenus);
4549
}
4650

51+
async Task InitializeLoggingAsync()
52+
{
53+
var packageSettings = await GetServiceAsync(typeof(IPackageSettings)) as IPackageSettings;
54+
LogManager.EnableTraceLogging(packageSettings?.EnableTraceLogging ?? false);
55+
if (packageSettings != null)
56+
{
57+
packageSettings.PropertyChanged += (sender, args) =>
58+
{
59+
if (args.PropertyName == "EnableTraceLogging")
60+
{
61+
LogManager.EnableTraceLogging(packageSettings.EnableTraceLogging);
62+
}
63+
};
64+
}
65+
}
66+
4767
void LogVersionInformation()
4868
{
4969
var packageVersion = ApplicationInfo.GetPackageVersion(this);
@@ -110,13 +130,17 @@ public ServiceProviderExports([Import(typeof(SVsServiceProvider))] IServiceProvi
110130
[ExportForProcess(typeof(IVSGitExt), ProcessName)]
111131
public IVSGitExt VSGitExt => GetService<IVSGitExt>();
112132

133+
[ExportForProcess(typeof(IPackageSettings), ProcessName)]
134+
public IPackageSettings PackageSettings => GetService<IPackageSettings>();
135+
113136
T GetService<T>() => (T)serviceProvider.GetService(typeof(T));
114137
}
115138

116139
[PackageRegistration(UseManagedResourcesOnly = true, AllowsBackgroundLoading = true)]
117140
[ProvideService(typeof(ILoginManager), IsAsyncQueryable = true)]
118141
[ProvideService(typeof(IGitHubServiceProvider), IsAsyncQueryable = true)]
119142
[ProvideService(typeof(IUsageTracker), IsAsyncQueryable = true)]
143+
[ProvideService(typeof(IPackageSettings), IsAsyncQueryable = true)]
120144
[ProvideService(typeof(IUsageService), IsAsyncQueryable = true)]
121145
[ProvideService(typeof(IVSGitExt), IsAsyncQueryable = true)]
122146
[ProvideService(typeof(IGitHubToolWindowManager))]
@@ -134,6 +158,7 @@ protected override Task InitializeAsync(CancellationToken cancellationToken, IPr
134158
AddService(typeof(IUsageService), CreateService, true);
135159
AddService(typeof(ILoginManager), CreateService, true);
136160
AddService(typeof(IGitHubToolWindowManager), CreateService, true);
161+
AddService(typeof(IPackageSettings), CreateService, true);
137162
return Task.CompletedTask;
138163
}
139164

@@ -240,6 +265,12 @@ async Task<object> CreateService(IAsyncServiceContainer container, CancellationT
240265
{
241266
return this;
242267
}
268+
else if (serviceType == typeof(IPackageSettings))
269+
{
270+
await ThreadHelper.JoinableTaskFactory.SwitchToMainThreadAsync();
271+
var sp = new ServiceProvider(Services.Dte as Microsoft.VisualStudio.OLE.Interop.IServiceProvider);
272+
return new PackageSettings(sp);
273+
}
243274
// go the mef route
244275
else
245276
{

0 commit comments

Comments
 (0)