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

Commit 7adf529

Browse files
shanashana
authored
Merge branch 'master' into refactor/new-metrics-format
2 parents 13452f3 + 9f431b5 commit 7adf529

File tree

19 files changed

+402
-78
lines changed

19 files changed

+402
-78
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/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>

0 commit comments

Comments
 (0)