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

[Group 4] Enable nullable annotations for Microsoft.Extensions.Http #66891

Merged
merged 4 commits into from
Mar 29, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ public abstract partial class HttpMessageHandlerBuilder
{
protected HttpMessageHandlerBuilder() { }
public abstract System.Collections.Generic.IList<System.Net.Http.DelegatingHandler> AdditionalHandlers { get; }
public abstract string Name { get; set; }
[System.Diagnostics.CodeAnalysis.DisallowNull]
public abstract string? Name { get; set; }
public abstract System.Net.Http.HttpMessageHandler PrimaryHandler { get; set; }
public virtual System.IServiceProvider Services { [System.Runtime.CompilerServices.CompilerGeneratedAttribute] get { throw null; } }
public abstract System.Net.Http.HttpMessageHandler Build();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFrameworks>$(NetCoreAppCurrent);$(NetCoreAppMinimum);netstandard2.0;$(NetFrameworkMinimum)</TargetFrameworks>
<Nullable>enable</Nullable>
</PropertyGroup>

<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,16 @@ namespace Microsoft.Extensions.Http
// for the 'expiry' pool simplifies the threading requirements significantly.
internal sealed class ActiveHandlerTrackingEntry
{
private static readonly TimerCallback _timerCallback = (s) => ((ActiveHandlerTrackingEntry)s).Timer_Tick();
private static readonly TimerCallback _timerCallback = (s) => ((ActiveHandlerTrackingEntry)s!).Timer_Tick();
private readonly object _lock;
private bool _timerInitialized;
private Timer _timer;
private TimerCallback _callback;
private Timer? _timer;
private TimerCallback? _callback;

public ActiveHandlerTrackingEntry(
string name,
LifetimeTrackingHttpMessageHandler handler,
IServiceScope scope,
IServiceScope? scope,
TimeSpan lifetime)
{
Name = name;
Expand All @@ -39,7 +39,7 @@ public ActiveHandlerTrackingEntry(

public string Name { get; }

public IServiceScope Scope { get; }
public IServiceScope? Scope { get; }

public void StartExpiryTimer(TimerCallback callback)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ namespace Microsoft.Extensions.Http
{
internal class DefaultHttpClientFactory : IHttpClientFactory, IHttpMessageHandlerFactory
{
private static readonly TimerCallback _cleanupCallback = (s) => ((DefaultHttpClientFactory)s).CleanupTimer_Tick();
private static readonly TimerCallback _cleanupCallback = (s) => ((DefaultHttpClientFactory)s!).CleanupTimer_Tick();
private readonly ILogger _logger;
private readonly IServiceProvider _services;
private readonly IServiceScopeFactory _scopeFactory;
Expand All @@ -37,7 +37,7 @@ internal class DefaultHttpClientFactory : IHttpClientFactory, IHttpMessageHandle
//
// There's no need for the factory itself to be disposable. If you stop using it, eventually everything will
// get reclaimed.
private Timer _cleanupTimer;
private Timer? _cleanupTimer;
private readonly object _cleanupTimerLock;
private readonly object _cleanupActiveLock;

Expand Down Expand Up @@ -116,7 +116,7 @@ public HttpMessageHandler CreateHandler(string name!!)
internal ActiveHandlerTrackingEntry CreateHandlerEntry(string name)
{
IServiceProvider services = _services;
var scope = (IServiceScope)null;
var scope = (IServiceScope?)null;

HttpClientFactoryOptions options = _optionsMonitor.Get(name);
if (!options.SuppressHandlerScope)
Expand Down Expand Up @@ -169,15 +169,15 @@ void Configure(HttpMessageHandlerBuilder b)
}

// Internal for tests
internal void ExpiryTimer_Tick(object state)
internal void ExpiryTimer_Tick(object? state)
{
var active = (ActiveHandlerTrackingEntry)state;
var active = (ActiveHandlerTrackingEntry)state!;

// The timer callback should be the only one removing from the active collection. If we can't find
// our entry in the collection, then this is a bug.
bool removed = _activeHandlers.TryRemove(active.Name, out Lazy<ActiveHandlerTrackingEntry> found);
bool removed = _activeHandlers.TryRemove(active.Name, out Lazy<ActiveHandlerTrackingEntry>? found);
Debug.Assert(removed, "Entry not found. We should always be able to remove the entry");
Debug.Assert(object.ReferenceEquals(active, found.Value), "Different entry found. The entry should not have been replaced");
Debug.Assert(object.ReferenceEquals(active, found!.Value), "Different entry found. The entry should not have been replaced");

// At this point the handler is no longer 'active' and will not be handed out to any new clients.
// However we haven't dropped our strong reference to the handler, so we can't yet determine if
Expand Down Expand Up @@ -216,7 +216,7 @@ internal virtual void StopCleanupTimer()
{
lock (_cleanupTimerLock)
{
_cleanupTimer.Dispose();
_cleanupTimer!.Dispose();
_cleanupTimer = null;
}
}
Expand Down Expand Up @@ -257,14 +257,14 @@ internal void CleanupTimer_Tick()
for (int i = 0; i < initialCount; i++)
{
// Since we're the only one removing from _expired, TryDequeue must always succeed.
_expiredHandlers.TryDequeue(out ExpiredHandlerTrackingEntry entry);
_expiredHandlers.TryDequeue(out ExpiredHandlerTrackingEntry? entry);
Debug.Assert(entry != null, "Entry was null, we should always get an entry back from TryDequeue");

if (entry.CanDispose)
{
try
{
entry.InnerHandler.Dispose();
entry.InnerHandler!.Dispose();
entry.Scope?.Dispose();
disposedCount++;
}
Expand Down Expand Up @@ -305,12 +305,12 @@ public static class EventIds
public static readonly EventId HandlerExpired = new EventId(103, "HandlerExpired");
}

private static readonly Action<ILogger, int, Exception> _cleanupCycleStart = LoggerMessage.Define<int>(
private static readonly Action<ILogger, int, Exception?> _cleanupCycleStart = LoggerMessage.Define<int>(
LogLevel.Debug,
EventIds.CleanupCycleStart,
"Starting HttpMessageHandler cleanup cycle with {InitialCount} items");

private static readonly Action<ILogger, double, int, int, Exception> _cleanupCycleEnd = LoggerMessage.Define<double, int, int>(
private static readonly Action<ILogger, double, int, int, Exception?> _cleanupCycleEnd = LoggerMessage.Define<double, int, int>(
LogLevel.Debug,
EventIds.CleanupCycleEnd,
"Ending HttpMessageHandler cleanup cycle after {ElapsedMilliseconds}ms - processed: {DisposedCount} items - remaining: {RemainingItems} items");
Expand All @@ -320,7 +320,7 @@ public static class EventIds
EventIds.CleanupItemFailed,
"HttpMessageHandler.Dispose() threw an unhandled exception for client: '{ClientName}'");

private static readonly Action<ILogger, double, string, Exception> _handlerExpired = LoggerMessage.Define<double, string>(
private static readonly Action<ILogger, double, string, Exception?> _handlerExpired = LoggerMessage.Define<double, string>(
LogLevel.Debug,
EventIds.HandlerExpired,
"HttpMessageHandler expired after {HandlerLifetime}ms for client '{ClientName}'");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Net.Http;

namespace Microsoft.Extensions.Http
Expand All @@ -14,9 +15,10 @@ public DefaultHttpMessageHandlerBuilder(IServiceProvider services)
Services = services;
}

private string _name;
private string? _name;

public override string Name
[DisallowNull]
public override string? Name
{
get => _name;
set
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,15 @@ public class Cache
{
private static readonly Func<ObjectFactory> _createActivator = () => ActivatorUtilities.CreateFactory(typeof(TClient), new Type[] { typeof(HttpClient), });

private ObjectFactory _activator;
private ObjectFactory? _activator;
private bool _initialized;
private object _lock;
private object? _lock;

public ObjectFactory Activator => LazyInitializer.EnsureInitialized(
ref _activator,
ref _initialized,
ref _lock,
_createActivator);
_createActivator)!;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -488,11 +488,11 @@ public static IHttpClientBuilder SetHandlerLifetime(this IHttpClientBuilder buil
// See comments on HttpClientMappingRegistry.
private static void ReserveClient(IHttpClientBuilder builder, Type type, string name, bool validateSingleType)
{
var registry = (HttpClientMappingRegistry)builder.Services.Single(sd => sd.ServiceType == typeof(HttpClientMappingRegistry)).ImplementationInstance;
var registry = (HttpClientMappingRegistry?)builder.Services.Single(sd => sd.ServiceType == typeof(HttpClientMappingRegistry)).ImplementationInstance;
Debug.Assert(registry != null);

// Check for same name registered to two types. This won't work because we rely on named options for the configuration.
if (registry.NamedClientRegistrations.TryGetValue(name, out Type otherType) &&
if (registry.NamedClientRegistrations.TryGetValue(name, out Type? otherType) &&

// Allow using the same name with multiple types in some cases (see callers).
validateSingleType &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -509,7 +509,7 @@ public static IHttpClientBuilder AddHttpClient(this IServiceCollection services!

var builder = new DefaultHttpClientBuilder(services, name);
builder.ConfigureHttpClient(configureClient);
builder.AddTypedClientCore<TClient>(validateSingleType: false); // name was explictly provided
builder.AddTypedClientCore<TClient>(validateSingleType: false); // name was explicitly provided
return builder;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@ public ExpiredHandlerTrackingEntry(ActiveHandlerTrackingEntry other)

public bool CanDispose => !_livenessTracker.IsAlive;

public HttpMessageHandler InnerHandler { get; }
maxkoshevoi marked this conversation as resolved.
Show resolved Hide resolved
public HttpMessageHandler? InnerHandler { get; }

public string Name { get; }

public IServiceScope Scope { get; }
public IServiceScope? Scope { get; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using System.Net.Http;

Expand All @@ -26,7 +27,8 @@ public abstract class HttpMessageHandlerBuilder
/// and is public for unit testing purposes only. Setting the <see cref="Name"/> outside of
/// testing scenarios may have unpredictable results.
/// </remarks>
public abstract string Name { get; set; }
[DisallowNull]
public abstract string? Name { get; set; }

/// <summary>
/// Gets or sets the primary <see cref="HttpMessageHandler"/>.
Expand All @@ -50,7 +52,7 @@ public abstract class HttpMessageHandlerBuilder
/// (default) this will be a reference to a scoped service provider that has the same
/// lifetime as the handler being created.
/// </remarks>
public virtual IServiceProvider Services { get; }
public virtual IServiceProvider Services { get; } = null!;

/// <summary>
/// Creates an <see cref="HttpMessageHandler"/>.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ internal sealed class HttpHeadersLogValue : IReadOnlyList<KeyValuePair<string, o
private readonly Kind _kind;
private readonly Func<string, bool> _shouldRedactHeaderValue;

private string _formatted;
private List<KeyValuePair<string, object>> _values;
private string? _formatted;
private List<KeyValuePair<string, object>>? _values;

public HttpHeadersLogValue(Kind kind, HttpHeaders headers, HttpHeaders contentHeaders, Func<string, bool> shouldRedactHeaderValue)
public HttpHeadersLogValue(Kind kind, HttpHeaders headers, HttpHeaders? contentHeaders, Func<string, bool> shouldRedactHeaderValue)
{
_kind = kind;
_shouldRedactHeaderValue = shouldRedactHeaderValue;
Expand All @@ -28,7 +28,7 @@ public HttpHeadersLogValue(Kind kind, HttpHeaders headers, HttpHeaders contentHe

public HttpHeaders Headers { get; }

public HttpHeaders ContentHeaders { get; }
public HttpHeaders? ContentHeaders { get; }

private List<KeyValuePair<string, object>> Values
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ namespace Microsoft.Extensions.Http.Logging
public class LoggingHttpMessageHandler : DelegatingHandler
{
private ILogger _logger;
private readonly HttpClientFactoryOptions _options;
private readonly HttpClientFactoryOptions? _options;

private static readonly Func<string, bool> _shouldNotRedactHeaderValue = (header) => false;

Expand Down Expand Up @@ -72,13 +72,13 @@ public static class EventIds

private static readonly LogDefineOptions _skipEnabledCheckLogDefineOptions = new LogDefineOptions() { SkipEnabledCheck = true };

private static readonly Action<ILogger, HttpMethod, string, Exception> _requestStart = LoggerMessage.Define<HttpMethod, string>(
private static readonly Action<ILogger, HttpMethod, string?, Exception?> _requestStart = LoggerMessage.Define<HttpMethod, string?>(
LogLevel.Information,
EventIds.RequestStart,
"Sending HTTP request {HttpMethod} {Uri}",
_skipEnabledCheckLogDefineOptions);

private static readonly Action<ILogger, double, int, Exception> _requestEnd = LoggerMessage.Define<double, int>(
private static readonly Action<ILogger, double, int, Exception?> _requestEnd = LoggerMessage.Define<double, int>(
LogLevel.Information,
EventIds.RequestEnd,
"Received HTTP response headers after {ElapsedMilliseconds}ms - {StatusCode}");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ namespace Microsoft.Extensions.Http.Logging
public class LoggingScopeHttpMessageHandler : DelegatingHandler
{
private ILogger _logger;
private readonly HttpClientFactoryOptions _options;
private readonly HttpClientFactoryOptions? _options;

private static readonly Func<string, bool> _shouldNotRedactHeaderValue = (header) => false;

Expand Down Expand Up @@ -72,14 +72,14 @@ public static class EventIds
public static readonly EventId ResponseHeader = new EventId(103, "RequestPipelineResponseHeader");
}

private static readonly Func<ILogger, HttpMethod, string, IDisposable> _beginRequestPipelineScope = LoggerMessage.DefineScope<HttpMethod, string>("HTTP {HttpMethod} {Uri}");
private static readonly Func<ILogger, HttpMethod, string?, IDisposable> _beginRequestPipelineScope = LoggerMessage.DefineScope<HttpMethod, string?>("HTTP {HttpMethod} {Uri}");

private static readonly Action<ILogger, HttpMethod, string, Exception> _requestPipelineStart = LoggerMessage.Define<HttpMethod, string>(
private static readonly Action<ILogger, HttpMethod, string?, Exception?> _requestPipelineStart = LoggerMessage.Define<HttpMethod, string?>(
LogLevel.Information,
EventIds.PipelineStart,
"Start processing HTTP request {HttpMethod} {Uri}");

private static readonly Action<ILogger, double, int, Exception> _requestPipelineEnd = LoggerMessage.Define<double, int>(
private static readonly Action<ILogger, double, int, Exception?> _requestPipelineEnd = LoggerMessage.Define<double, int>(
LogLevel.Information,
EventIds.PipelineEnd,
"End processing HTTP request after {ElapsedMilliseconds}ms - {StatusCode}");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@

<PropertyGroup>
<TargetFrameworks>$(NetCoreAppCurrent);$(NetCoreAppMinimum);netstandard2.0;$(NetFrameworkMinimum)</TargetFrameworks>
<Nullable>enable</Nullable>
<EnableDefaultItems>true</EnableDefaultItems>
<!-- Use targeting pack references instead of granular ones in the project file. -->
<DisableImplicitAssemblyReferences>false</DisableImplicitAssemblyReferences>
<IsPackable>true</IsPackable>
<Nullable>Annotations</Nullable>
<PackageDescription>The HttpClient factory is a pattern for configuring and retrieving named HttpClients in a composable way. The HttpClient factory provides extensibility to plug in DelegatingHandlers that address cross-cutting concerns such as service location, load balancing, and reliability. The default HttpClient factory provides built-in diagnostics and logging and manages the lifetimes of connections in a performant way.

Commonly Used Types:
Expand Down