Skip to content

Commit

Permalink
Address comments, do more cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
@costin-zaharia-sonarsource
costin-zaharia-sonarsource committed Nov 22, 2022
1 parent b06b872 commit 50d9310
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 62 deletions.
2 changes: 1 addition & 1 deletion Tests/SonarScanner.MSBuild.PreProcessor.Test/SonarWebServiceTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -985,7 +985,7 @@ public Task<string> Download(Uri url, bool logPermissionDenied = false)
: throw new ArgumentException("Cannot find URL " + url);
}

public Task<Stream> DownloadStream(Uri url, bool logPermissionDenied = false) =>
public Task<Stream> DownloadStream(Uri url) =>
throw new NotImplementedException();

public Task<bool> TryDownloadFileIfExists(Uri url, string targetFilePath, bool logPermissionDenied = false)
Expand Down
91 changes: 41 additions & 50 deletions Tests/SonarScanner.MSBuild.PreProcessor.Test/WebClientDownloaderTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ namespace SonarScanner.MSBuild.PreProcessor.Test
[TestClass]
public class WebClientDownloaderTest
{
private const string TestContent = "test content";

private readonly Uri testUri = new("https://www.sonarsource.com/");

[TestMethod]
public void Credentials()
{
Expand Down Expand Up @@ -153,94 +157,76 @@ public void MultipleDisposeCallsNotFailing()
[TestMethod]
public async Task DownloadStream_Success()
{
const string testContent = "test content";
var logger = new TestLogger();
var httpClient = MockHttpClient(new HttpResponseMessage { StatusCode = HttpStatusCode.OK, Content = new StringContent(testContent) });
var sut = CreateSut(logger, HttpStatusCode.OK);

var sut = new WebClientDownloader("username", "password", logger, httpClient: httpClient);
var stream = await sut.DownloadStream(new Uri("https://test.com"));
var stream = await sut.DownloadStream(testUri);

var text = await new StreamReader(stream).ReadToEndAsync();
text.Should().Be(testContent);

logger.AssertDebugLogged("Downloading from https://test.com/...");
text.Should().Be(TestContent);
logger.AssertDebugLogged("Downloading from https://www.sonarsource.com/...");
}

[TestMethod]
public async Task DownloadStream_Fail_ForbiddenResponse_WithLog()
public async Task DownloadStream_Fail()
{
const string testContent = "test content";
var logger = new TestLogger();
var httpClient = MockHttpClient(new HttpResponseMessage { StatusCode = HttpStatusCode.Forbidden, Content = new StringContent(testContent) });
var sut = CreateSut(logger, HttpStatusCode.NotFound);

var sut = new WebClientDownloader("username", "password", logger, httpClient: httpClient);
await new Func<Task<Stream>>(async () => await sut.DownloadStream(new Uri("https://test.com"), true))
.Should().ThrowAsync<HttpRequestException>();
var stream = await sut.DownloadStream(testUri);

logger.AssertDebugLogged("Downloading from https://test.com/...");
logger.AssertWarningLogged("To analyze private projects make sure the scanner user has 'Browse' permission.");
stream.Should().BeNull();
logger.AssertDebugLogged("Downloading from https://www.sonarsource.com/...");
logger.AssertNoWarningsLogged();
}

[TestMethod]
public async Task DownloadStream_Fail_NotForbidden_WithLog()
public async Task Download_Success()
{
const string testContent = "test content";
var logger = new TestLogger();
var httpClient = MockHttpClient(new HttpResponseMessage { StatusCode = HttpStatusCode.NotFound, Content = new StringContent(testContent) });

var sut = new WebClientDownloader("username", "password", logger, httpClient: httpClient);
var stream = await sut.DownloadStream(new Uri("https://test.com"), true);
var sut = CreateSut(logger, HttpStatusCode.OK);

stream.Should().BeNull();
var text = await sut.Download(testUri);

logger.AssertDebugLogged("Downloading from https://test.com/...");
logger.AssertNoWarningsLogged();
text.Should().Be(TestContent);
logger.AssertDebugLogged("Downloading from https://www.sonarsource.com/...");
}

[TestMethod]
public async Task DownloadStream_Fail_NoLog()
public async Task Download_Fail_NoLog()
{
const string testContent = "test content";
var logger = new TestLogger();
var httpClient = MockHttpClient(new HttpResponseMessage { StatusCode = HttpStatusCode.Forbidden, Content = new StringContent(testContent) });
var sut = CreateSut(logger, HttpStatusCode.Forbidden);

var sut = new WebClientDownloader("username", "password", logger, httpClient: httpClient);
var stream = await sut.DownloadStream(new Uri("https://test.com"));
var text = await sut.Download(testUri);

stream.Should().BeNull();

logger.AssertDebugLogged("Downloading from https://test.com/...");
text.Should().BeNull();
logger.AssertDebugLogged("Downloading from https://www.sonarsource.com/...");
logger.AssertNoWarningsLogged();
}

[TestMethod]
public async Task Download_Success()
public async Task Download_Fail_Forbidden_WithLog()
{
const string testContent = "test content";
var logger = new TestLogger();
var httpClient = MockHttpClient(new HttpResponseMessage { StatusCode = HttpStatusCode.OK, Content = new StringContent(testContent) });

var sut = new WebClientDownloader("username", "password", logger, httpClient: httpClient);
var text = await sut.Download(new Uri("https://test.com"));
var sut = CreateSut(logger, HttpStatusCode.Forbidden);

text.Should().Be(testContent);
await new Func<Task<string>>(async () => await sut.Download(testUri, true)).Should().ThrowAsync<HttpRequestException>();

logger.AssertDebugLogged("Downloading from https://test.com/...");
logger.AssertDebugLogged("Downloading from https://www.sonarsource.com/...");
logger.AssertWarningLogged("To analyze private projects make sure the scanner user has 'Browse' permission.");
}

[TestMethod]
public async Task Download_Fail_NoLog()
public async Task Download_Fail_NotForbidden_WithLog()
{
const string testContent = "test content";
var logger = new TestLogger();
var httpClient = MockHttpClient(new HttpResponseMessage { StatusCode = HttpStatusCode.Forbidden, Content = new StringContent(testContent) });
var sut = CreateSut(logger, HttpStatusCode.NotFound);

var sut = new WebClientDownloader("username", "password", logger, httpClient: httpClient);
var text = await sut.Download(new Uri("https://test.com"));
var text = await sut.Download(testUri, true);

text.Should().BeNull();

logger.AssertDebugLogged("Downloading from https://test.com/...");
logger.AssertDebugLogged("Downloading from https://www.sonarsource.com/...");
logger.AssertNoWarningsLogged();
}

Expand All @@ -250,15 +236,20 @@ private static HttpClient MockHttpClient(HttpResponseMessage responseMessage)
httpMessageHandlerMock
.Protected()
.Setup<Task<HttpResponseMessage>>("SendAsync", ItExpr.IsAny<HttpRequestMessage>(), ItExpr.IsAny<CancellationToken>())
.ReturnsAsync(responseMessage)
.Verifiable();
.ReturnsAsync(responseMessage);

return new HttpClient(httpMessageHandlerMock.Object);
}

private static WebClientDownloader CreateSut(ILogger logger, HttpStatusCode statusCode)
{
var client = MockHttpClient(new HttpResponseMessage { StatusCode = statusCode, Content = new StringContent(TestContent) });
return new("username", "password", logger, client: client);
}

private sealed class TestDownloader : WebClientDownloader
{
public bool IsDisposedCalled { get; private set; } = false;
public bool IsDisposedCalled { get; private set; }

public TestDownloader(string userName, string password, ILogger logger, string clientCertPath = null, string clientCertPassword = null) : base(userName, password, logger, clientCertPath, clientCertPassword) { }

Expand Down
2 changes: 1 addition & 1 deletion src/SonarScanner.MSBuild.PreProcessor/Interfaces/IDownloader.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public interface IDownloader : IDisposable

Task<string> Download(Uri url, bool logPermissionDenied = false);

Task<Stream> DownloadStream(Uri url, bool logPermissionDenied = false);
Task<Stream> DownloadStream(Uri url);

Task<HttpResponseMessage> TryGetLicenseInformation(Uri url);
}
Expand Down
20 changes: 10 additions & 10 deletions src/SonarScanner.MSBuild.PreProcessor/WebClientDownloader.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,13 @@ public class WebClientDownloader : IDownloader
private readonly ILogger logger;
private readonly HttpClient client;

public WebClientDownloader(string userName, string password, ILogger logger, string clientCertPath = null, string clientCertPassword = null, HttpClient httpClient = null)
public WebClientDownloader(string userName, string password, ILogger logger, string clientCertPath = null, string clientCertPassword = null, HttpClient client = null)
{
this.logger = logger ?? throw new ArgumentNullException(nameof(logger));
password = password ?? string.Empty;

client = httpClient ?? CreateHttpClient(clientCertPath, clientCertPassword);
client.DefaultRequestHeaders.Add(HttpRequestHeader.UserAgent.ToString(), $"ScannerMSBuild/{Utilities.ScannerVersion}");
this.client = client ?? CreateHttpClient(clientCertPath, clientCertPassword);
this.client.DefaultRequestHeaders.Add(HttpRequestHeader.UserAgent.ToString(), $"ScannerMSBuild/{Utilities.ScannerVersion}");

if (userName != null)
{
Expand All @@ -56,22 +56,22 @@ public WebClientDownloader(string userName, string password, ILogger logger, str

var credentials = string.Format(System.Globalization.CultureInfo.InvariantCulture, "{0}:{1}", userName, password);
credentials = Convert.ToBase64String(Encoding.ASCII.GetBytes(credentials));
client.DefaultRequestHeaders.Add(HttpRequestHeader.Authorization.ToString(), "Basic " + credentials);
this.client.DefaultRequestHeaders.Add(HttpRequestHeader.Authorization.ToString(), "Basic " + credentials);
}
}

private static HttpClient CreateHttpClient(string clientCertPath, string clientCertPassword)
{
// password mandatory, as to use client cert in .jar it cannot be with empty password
if (clientCertPath == null || clientCertPassword == null)
if (clientCertPath is null || clientCertPassword is null)
{
return new HttpClient();
return new();
}
else
{
var clientHandler = new HttpClientHandler { ClientCertificateOptions = ClientCertificateOption.Manual };
clientHandler.ClientCertificates.Add(new X509Certificate2(clientCertPath, clientCertPassword));
return new HttpClient(clientHandler);
return new(clientHandler);
}
}

Expand Down Expand Up @@ -169,7 +169,7 @@ public async Task<string> Download(Uri url, bool logPermissionDenied = false)
return null;
}

public async Task<Stream> DownloadStream(Uri url, bool logPermissionDenied = false)
public async Task<Stream> DownloadStream(Uri url)
{
logger.LogDebug(Resources.MSG_Downloading, url);
var response = await client.GetAsync(url);
Expand All @@ -178,11 +178,11 @@ public async Task<Stream> DownloadStream(Uri url, bool logPermissionDenied = fal
return await response.Content.ReadAsStreamAsync();
}

CheckPermissions(response, logPermissionDenied);
CheckPermissions(response, true);
return null;
}

private void CheckPermissions(HttpResponseMessage response, bool logPermissionDenied = false)
private void CheckPermissions(HttpResponseMessage response, bool logPermissionDenied)
{
if (!logPermissionDenied || response.StatusCode != HttpStatusCode.Forbidden)
{
Expand Down

0 comments on commit 50d9310

Please sign in to comment.