Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: address some static code analysis issues, exposed by the latest analyzer #87

Merged
merged 5 commits into from
Nov 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions src/MockHttp.Server/Server/HttpResponseMessageExtensions.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Net.Http.Headers;
using System.Runtime.CompilerServices;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Http.Features;
using Microsoft.Extensions.Primitives;
Expand All @@ -22,11 +23,12 @@ internal static async Task MapToFeatureAsync
if (response.Content is not null)
{
CopyHeaders(response.Content.Headers, responseFeature.Headers);
Stream contentStream = await response.Content.ReadAsStreamAsync(
#if NET6_0_OR_GREATER
await using Stream contentStream = await response.Content.ReadAsStreamAsync(cancellationToken).ConfigureAwait(false);
#else
await using Stream contentStream = await response.Content.ReadAsStreamAsync().ConfigureAwait(false);
cancellationToken
#endif
).ConfigureAwait(false);
await using ConfiguredAsyncDisposable _ = contentStream.ConfigureAwait(false);
await contentStream.CopyToAsync(responseBodyFeature.Writer.AsStream(), 4096, cancellationToken).ConfigureAwait(false);
}
}
Expand Down
10 changes: 8 additions & 2 deletions src/MockHttp/Http/HttpHeaderEqualityComparer.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using MockHttp.Matchers.Patterns;
using System.ComponentModel;
using MockHttp.Matchers.Patterns;

namespace MockHttp.Http;

Expand All @@ -25,6 +26,11 @@

public HttpHeaderEqualityComparer(HttpHeaderMatchType matchType)
{
if (!Enum.IsDefined(typeof(HttpHeaderMatchType), matchType))
{
throw new InvalidEnumArgumentException(nameof(matchType), (int)matchType, typeof(HttpHeaderMatchType));
}

_matchType = matchType;
}

Expand Down Expand Up @@ -57,7 +63,7 @@
{
string[] headerValues = HttpHeadersCollection.ParseHttpHeaderValue(yValue).ToArray();
return _valuePatternMatcher is null && headerValues.Contains(xValue)
|| (_valuePatternMatcher is not null && headerValues.Any(_valuePatternMatcher.IsMatch));

Check warning on line 66 in src/MockHttp/Http/HttpHeaderEqualityComparer.cs

View workflow job for this annotation

GitHub Actions / analysis

Collection-specific "Exists" method should be used instead of the "Any" extension. (https://rules.sonarsource.com/csharp/RSPEC-6605)

Check warning on line 66 in src/MockHttp/Http/HttpHeaderEqualityComparer.cs

View workflow job for this annotation

GitHub Actions / analysis

Collection-specific "Exists" method should be used instead of the "Any" extension. (https://rules.sonarsource.com/csharp/RSPEC-6605)
})
))
{
Expand All @@ -67,7 +73,7 @@
return !x.Value.Any();
}
default:
throw new ArgumentOutOfRangeException();
return false;
}
}

Expand Down
3 changes: 3 additions & 0 deletions src/MockHttp/HttpMockException.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@

/// <inheritdoc />
[ExcludeFromCodeCoverage]
#if NET8_0_OR_GREATER
[Obsolete(DiagnosticId = "SYSLIB0051")]

Check warning on line 39 in src/MockHttp/HttpMockException.cs

View workflow job for this annotation

GitHub Actions / Analyze (csharp)

Provide a message for the ObsoleteAttribute that marks .ctor as Obsolete (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1041)

Check warning on line 39 in src/MockHttp/HttpMockException.cs

View workflow job for this annotation

GitHub Actions / build (v4.2.1-pr87.8)

Provide a message for the ObsoleteAttribute that marks .ctor as Obsolete (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1041)
#endif
protected HttpMockException(SerializationInfo info, StreamingContext context)
: base(info, context)
{
Expand Down
26 changes: 5 additions & 21 deletions src/MockHttp/Responses/HttpHeaderBehavior.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
using System.Net.Http.Headers;
using MockHttp.Http;
using MockHttp.Http;

namespace MockHttp.Responses;

Expand Down Expand Up @@ -57,38 +56,23 @@ private static void Add(KeyValuePair<string, IEnumerable<string?>> header, HttpR
// Special case handling of headers which only allow single values.
if (HeadersWithSingleValueOnly.Contains(header.Key))
{
// ReSharper disable once ConditionalAccessQualifierIsNonNullableAccordingToAPIContract
if (responseMessage.Content?.Headers.TryGetValues(header.Key, out _) == true)
{
responseMessage.Content.Headers.Remove(header.Key);
}

if (responseMessage.Headers.TryGetValues(header.Key, out _))
{
responseMessage.Headers.Remove(header.Key);
}
}

// Try to add as message header first, if that fails, add as content header.
if (!TryAdd(responseMessage.Headers, header.Key, header.Value))
// Let it throw if not supported.
if (!responseMessage.Headers.TryAddWithoutValidation(header.Key, header.Value))
{
responseMessage.Content?.Headers.Add(header.Key, header.Value);
}
}

private static bool TryAdd(HttpHeaders? headers, string name, IEnumerable<string?> values)
{
if (headers is null)
{
return false;
}

try
{
headers.Add(name, values);
return true;
}
catch (Exception)
{
return false;
}
}
}
13 changes: 13 additions & 0 deletions test/MockHttp.Json.Tests/ArgAny.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
namespace MockHttp.Json;

/// <summary>
/// To deal with runtime API differences around mocking with nullable.
/// </summary>
internal static class ArgAny
{
#if NETCOREAPP3_1_OR_GREATER
public static ref string? String() => ref Arg.Any<string?>();
#else
public static ref string String() => ref Arg.Any<string>();
#endif
}
15 changes: 8 additions & 7 deletions test/MockHttp.Json.Tests/JsonContentMatcherTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public JsonContentMatcherTests()

_equalityComparerMock = Substitute.For<IEqualityComparer<string>>();
_equalityComparerMock
.Equals(Arg.Any<string?>(), Arg.Any<string?>())
.Equals(ArgAny.String(), ArgAny.String())
.Returns(true);

_requestMessage = new HttpRequestMessage();
Expand All @@ -43,7 +43,7 @@ public async Task Given_that_adapter_is_provided_to_ctor_when_matching_it_should
// Assert
_adapterMock.Received(1).Serialize(jsonContentAsObject);
globalAdapterMock.DidNotReceiveWithAnyArgs().Serialize(Arg.Any<object?>());
_ = _equalityComparerMock.Received(1).Equals(Arg.Any<string?>(), serializedJson);
_ = _equalityComparerMock.Received(1).Equals(ArgAny.String(), serializedJson);
}

[Fact]
Expand All @@ -65,7 +65,7 @@ public async Task Given_that_adapter_is_not_provided_to_ctor_when_matching_it_sh

// Assert
globalAdapterMock.Received(1).Serialize(jsonContentAsObject);
_ = _equalityComparerMock.Received(1).Equals(Arg.Any<string?>(), serializedJson);
_ = _equalityComparerMock.Received(1).Equals(ArgAny.String(), serializedJson);
}

[Fact]
Expand All @@ -80,7 +80,7 @@ public async Task Given_that_adapter_is_not_provided_to_ctor_and_no_global_adapt
await sut.IsMatchAsync(_requestContext);

// Assert
_ = _equalityComparerMock.Received(1).Equals(Arg.Any<string?>(), serializedJson);
_ = _equalityComparerMock.Received(1).Equals(ArgAny.String(), serializedJson);
}

[Fact]
Expand All @@ -104,14 +104,14 @@ public async Task When_matching_it_should_return_the_results_of_the_comparer(boo
var sut = new JsonContentMatcher("something to compare with", _adapterMock, _equalityComparerMock);

_equalityComparerMock
.Equals(Arg.Any<string?>(), Arg.Any<string?>())
.Equals(ArgAny.String(), ArgAny.String())
.Returns(equals);

// Act
bool actual = await sut.IsMatchAsync(_requestContext);

// Assert
_ =_equalityComparerMock.Received(1).Equals(Arg.Any<string?>(), Arg.Any<string?>());
_ =_equalityComparerMock.Received(1).Equals(ArgAny.String(), ArgAny.String());
actual.Should().Be(equals);
}

Expand All @@ -131,7 +131,7 @@ HttpContent content
await sut.IsMatchAsync(_requestContext);

// Assert
_ = _equalityComparerMock.Received(1).Equals(string.Empty, Arg.Any<string?>());
_ = _equalityComparerMock.Received(1).Equals(string.Empty, ArgAny.String());
}
}

Expand Down Expand Up @@ -169,6 +169,7 @@ public static IEnumerable<object[]> JsonMatchTestCases()

public void Dispose()
{
// ReSharper disable once ConditionalAccessQualifierIsNonNullableAccordingToAPIContract
_requestMessage?.Dispose();
}
}
12 changes: 4 additions & 8 deletions test/MockHttp.Tests/Language/Flow/Response/HeaderSpec.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,8 @@ namespace MockHttp.Language.Flow.Response;

public class HeaderSpec : ResponseSpec
{
private readonly DateTimeOffset _utcNow;
private readonly DateTime _now;

public HeaderSpec()
{
_utcNow = DateTimeOffset.UtcNow;
_now = DateTime.Now;
}
private readonly DateTimeOffset _utcNow = DateTimeOffset.UtcNow;
private readonly DateTime _now = DateTime.Now;

protected override void Given(IResponseBuilder with)
{
Expand Down Expand Up @@ -42,6 +36,8 @@ protected override Task Should(HttpResponseMessage response)
.And.HaveHeader("X-Date", new[] { _now.AddYears(-1).ToString("R"), _now.ToString("R") })
.And.HaveHeader("X-Empty", string.Empty)
.And.HaveHeader("X-Null", string.Empty);
response.Headers.Count().Should().Be(5);
response.Content.Headers.Count().Should().Be(2);
return Task.CompletedTask;
}
}
Loading