Skip to content

Commit

Permalink
fix: address some static code analysis issues, exposed by the latest …
Browse files Browse the repository at this point in the history
…analyzer (#87)

* fix(CS8604): in older runtimes, IEqualityComparer<> did not use nullable annotation. Fix with string polyfill for Arg.Any.
* fix(CA2007): use `ConfigureAwait` on awaitable async stream
* refactor(CA1031): use `TryAddWithoutValidation` to avoid exceptions and to skip adding header if not supported.
* fix(SYSLIB0051): mark serializable/binary formatter support as obsolete
* refactor(CA2208): rather, catch the invalid enum value in ctor.
  • Loading branch information
skwasjer authored Nov 18, 2023
1 parent 6029dc2 commit 6e2fdb9
Show file tree
Hide file tree
Showing 7 changed files with 46 additions and 41 deletions.
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 @@ internal sealed class HttpHeaderEqualityComparer : IEqualityComparer<KeyValuePai

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 @@ -67,7 +73,7 @@ public bool Equals(KeyValuePair<string, IEnumerable<string>> x, KeyValuePair<str
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 @@ internal HttpMockException(string message, Exception innerException) : base(mess

/// <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 / build (v4.2.1-ci.1+3)

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;
}
}

0 comments on commit 6e2fdb9

Please sign in to comment.