Skip to content

Allow nullable with IFeatureCollection generic Get/Set #28809

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Dec 26, 2020
Merged

Allow nullable with IFeatureCollection generic Get/Set #28809

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions src/Hosting/TestHost/src/ClientHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ protected override async Task<HttpResponseMessage> SendAsync(
// Copy trailers to the response message when the response stream is complete
contextBuilder.RegisterResponseReadCompleteCallback(context =>
{
var responseTrailersFeature = context.Features.Get<IHttpResponseTrailersFeature>();
var responseTrailersFeature = context.Features.Get<IHttpResponseTrailersFeature>()!;

foreach (var trailer in responseTrailersFeature.Trailers)
{
Expand All @@ -196,7 +196,7 @@ protected override async Task<HttpResponseMessage> SendAsync(
var httpContext = await contextBuilder.SendAsync(cancellationToken);

response.StatusCode = (HttpStatusCode)httpContext.Response.StatusCode;
response.ReasonPhrase = httpContext.Features.Get<IHttpResponseFeature>().ReasonPhrase;
response.ReasonPhrase = httpContext.Features.Get<IHttpResponseFeature>()!.ReasonPhrase;
response.RequestMessage = request;
response.Version = request.Version;

Expand Down
2 changes: 1 addition & 1 deletion src/Hosting/TestHost/src/HttpContextBuilder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ internal async Task ReturnResponseMessageAsync()
{
newFeatures[pair.Key] = pair.Value;
}
var serverResponseFeature = _httpContext.Features.Get<IHttpResponseFeature>();
var serverResponseFeature = _httpContext.Features.Get<IHttpResponseFeature>()!;
// The client gets a deep copy of this so they can interact with the body stream independently of the server.
var clientResponseFeature = new HttpResponseFeature()
{
Expand Down
2 changes: 1 addition & 1 deletion src/Http/Http.Extensions/src/ResponseExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public static void Clear(this HttpResponse response)
throw new InvalidOperationException("The response cannot be cleared, it has already started sending.");
}
response.StatusCode = 200;
response.HttpContext.Features.Get<IHttpResponseFeature>().ReasonPhrase = null;
response.HttpContext.Features.Get<IHttpResponseFeature>()!.ReasonPhrase = null;
response.Headers.Clear();
if (response.Body.CanSeek)
{
Expand Down
2 changes: 1 addition & 1 deletion src/Http/Http.Extensions/src/SendFileResponseExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ private static async Task SendFileAsyncCore(HttpResponse response, string fileNa
{
var useRequestAborted = !cancellationToken.CanBeCanceled;
var localCancel = useRequestAborted ? response.HttpContext.RequestAborted : cancellationToken;
var sendFile = response.HttpContext.Features.Get<IHttpResponseBodyFeature>();
var sendFile = response.HttpContext.Features.Get<IHttpResponseBodyFeature>()!;

try
{
Expand Down
2 changes: 1 addition & 1 deletion src/Http/Http.Features/src/FeatureCollection.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ public IEnumerator<KeyValuePair<Type, object>> GetEnumerator()
return (TFeature?)this[typeof(TFeature)];
}

public void Set<TFeature>(TFeature instance)
public void Set<TFeature>(TFeature? instance)
{
this[typeof(TFeature)] = instance;
}
Expand Down
4 changes: 2 additions & 2 deletions src/Http/Http.Features/src/IFeatureCollection.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,13 @@ public interface IFeatureCollection : IEnumerable<KeyValuePair<Type, object>>
/// </summary>
/// <typeparam name="TFeature">The feature key.</typeparam>
/// <returns>The requested feature, or null if it is not present.</returns>
TFeature Get<TFeature>();
TFeature? Get<TFeature>();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think we need a GetRequiredFeature to avoid the use of null forgiveness?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. It's not that commonly used API. But it could be added later.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, GetRequiredFeature would be a good idea. There are several features we always assume are present like IHttpRequestFeature.


/// <summary>
/// Sets the given feature in the collection.
/// </summary>
/// <typeparam name="TFeature">The feature key.</typeparam>
/// <param name="instance">The feature value.</param>
void Set<TFeature>(TFeature instance);
void Set<TFeature>(TFeature? instance);
}
}
6 changes: 3 additions & 3 deletions src/Http/Http.Features/src/PublicAPI.Shipped.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ Microsoft.AspNetCore.Http.Features.FeatureCollection.FeatureCollection(Microsoft
Microsoft.AspNetCore.Http.Features.FeatureCollection.Get<TFeature>() -> TFeature?
Microsoft.AspNetCore.Http.Features.FeatureCollection.GetEnumerator() -> System.Collections.Generic.IEnumerator<System.Collections.Generic.KeyValuePair<System.Type!, object!>>!
Microsoft.AspNetCore.Http.Features.FeatureCollection.IsReadOnly.get -> bool
Microsoft.AspNetCore.Http.Features.FeatureCollection.Set<TFeature>(TFeature instance) -> void
Microsoft.AspNetCore.Http.Features.FeatureCollection.Set<TFeature>(TFeature? instance) -> void
Microsoft.AspNetCore.Http.Features.FeatureCollection.this[System.Type! key].get -> object?
Microsoft.AspNetCore.Http.Features.FeatureCollection.this[System.Type! key].set -> void
Microsoft.AspNetCore.Http.Features.FeatureReference<T>
Expand All @@ -46,10 +46,10 @@ Microsoft.AspNetCore.Http.Features.HttpsCompressionMode.Compress = 2 -> Microsof
Microsoft.AspNetCore.Http.Features.HttpsCompressionMode.Default = 0 -> Microsoft.AspNetCore.Http.Features.HttpsCompressionMode
Microsoft.AspNetCore.Http.Features.HttpsCompressionMode.DoNotCompress = 1 -> Microsoft.AspNetCore.Http.Features.HttpsCompressionMode
Microsoft.AspNetCore.Http.Features.IFeatureCollection
Microsoft.AspNetCore.Http.Features.IFeatureCollection.Get<TFeature>() -> TFeature
Microsoft.AspNetCore.Http.Features.IFeatureCollection.Get<TFeature>() -> TFeature?
Microsoft.AspNetCore.Http.Features.IFeatureCollection.IsReadOnly.get -> bool
Microsoft.AspNetCore.Http.Features.IFeatureCollection.Revision.get -> int
Microsoft.AspNetCore.Http.Features.IFeatureCollection.Set<TFeature>(TFeature instance) -> void
Microsoft.AspNetCore.Http.Features.IFeatureCollection.Set<TFeature>(TFeature? instance) -> void
Microsoft.AspNetCore.Http.Features.IFeatureCollection.this[System.Type! key].get -> object?
Microsoft.AspNetCore.Http.Features.IFeatureCollection.this[System.Type! key].set -> void
Microsoft.AspNetCore.Http.Features.IFormFeature
Expand Down
2 changes: 1 addition & 1 deletion src/Http/Http/src/Internal/DefaultHttpResponse.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public override Stream Body
get { return HttpResponseBodyFeature.Stream; }
set
{
var otherFeature = _features.Collection.Get<IHttpResponseBodyFeature>();
var otherFeature = _features.Collection.Get<IHttpResponseBodyFeature>()!;

if (otherFeature is StreamResponseBodyFeature streamFeature
&& streamFeature.PriorFeature != null
Expand Down
2 changes: 1 addition & 1 deletion src/Http/Http/src/Internal/ResponseCookies.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ internal class ResponseCookies : IResponseCookies
internal ResponseCookies(IFeatureCollection features)
{
_features = features;
Headers = _features.Get<IHttpResponseFeature>().Headers;
Headers = _features.Get<IHttpResponseFeature>()!.Headers;
}

private IHeaderDictionary Headers { get; set; }
Expand Down
3 changes: 3 additions & 0 deletions src/Middleware/ResponseCompression/src/ResponseCompressionMiddleware.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Diagnostics;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Http.Features;
Expand Down Expand Up @@ -54,6 +55,8 @@ public async Task Invoke(HttpContext context)
var originalBodyFeature = context.Features.Get<IHttpResponseBodyFeature>();
var originalCompressionFeature = context.Features.Get<IHttpsCompressionFeature>();

Debug.Assert(originalBodyFeature != null);

var compressionBody = new ResponseCompressionBody(context, _provider, originalBodyFeature);
context.Features.Set<IHttpResponseBodyFeature>(compressionBody);
context.Features.Set<IHttpsCompressionFeature>(compressionBody);
Expand Down
2 changes: 1 addition & 1 deletion src/Middleware/Rewrite/src/UrlActions/CustomResponseAction.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public override void ApplyAction(RewriteContext context, BackReferenceCollection

if (!string.IsNullOrEmpty(StatusReason))
{
context.HttpContext.Features.Get<IHttpResponseFeature>().ReasonPhrase = StatusReason;
context.HttpContext.Features.Get<IHttpResponseFeature>()!.ReasonPhrase = StatusReason;
}

if (!string.IsNullOrEmpty(StatusDescription))
Expand Down
10 changes: 5 additions & 5 deletions src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.Generated.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -388,12 +388,12 @@ private void ExtraFeatureSet(Type key, object? value)
}
else
{
ExtraFeatureSet(key, value!); // TODO: What happens if you set an extra feature with a null value?
ExtraFeatureSet(key, value);
}
}
}

TFeature IFeatureCollection.Get<TFeature>()
TFeature? IFeatureCollection.Get<TFeature>() where TFeature : default
{
TFeature? feature = default;
if (typeof(TFeature) == typeof(IHttpRequestFeature))
Expand Down Expand Up @@ -518,10 +518,10 @@ TFeature IFeatureCollection.Get<TFeature>()
feature = ConnectionFeatures.Get<TFeature>();
}

return feature!;
return feature;
}

void IFeatureCollection.Set<TFeature>(TFeature feature)
void IFeatureCollection.Set<TFeature>(TFeature? feature) where TFeature : default
{
_featureRevision++;
if (typeof(TFeature) == typeof(IHttpRequestFeature))
Expand Down Expand Up @@ -638,7 +638,7 @@ void IFeatureCollection.Set<TFeature>(TFeature feature)
}
else
{
ExtraFeatureSet(typeof(TFeature), feature!); // TODO: What happens if you set an extra feature with a null value?
ExtraFeatureSet(typeof(TFeature), feature);
}
}

Expand Down
1 change: 1 addition & 0 deletions src/Servers/Kestrel/Core/src/Internal/Http3/Http3Connection.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,7 @@ internal async Task InnerProcessRequestsAsync<TContext>(IHttpApplication<TContex
var streamIdFeature = streamContext.Features.Get<IStreamIdFeature>();

Debug.Assert(quicStreamFeature != null);
Debug.Assert(streamIdFeature != null);

var httpConnectionContext = new Http3StreamContext(
streamContext.ConnectionId,
Expand Down
4 changes: 2 additions & 2 deletions src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ public Http3Stream(Http3Connection http3Connection, Http3StreamContext context)
_http3Connection = http3Connection;
_context = context;

_errorCodeFeature = _context.ConnectionFeatures.Get<IProtocolErrorCodeFeature>();
_streamIdFeature = _context.ConnectionFeatures.Get<IStreamIdFeature>();
_errorCodeFeature = _context.ConnectionFeatures.Get<IProtocolErrorCodeFeature>()!;
_streamIdFeature = _context.ConnectionFeatures.Get<IStreamIdFeature>()!;

_frameWriter = new Http3FrameWriter(
context.Transport.Output,
Expand Down
2 changes: 1 addition & 1 deletion src/Servers/Kestrel/Transport.Quic/src/Internal/QuicConnectionContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ public override ValueTask<ConnectionContext> ConnectAsync(IFeatureCollection? fe

if (features != null)
{
var streamDirectionFeature = features.Get<IStreamDirectionFeature>();
var streamDirectionFeature = features.Get<IStreamDirectionFeature>()!;
if (streamDirectionFeature.CanRead)
{
quicStream = _connection.OpenBidirectionalStream();
Expand Down
10 changes: 5 additions & 5 deletions src/Servers/Kestrel/shared/TransportConnection.Generated.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,12 +157,12 @@ private void ExtraFeatureSet(Type key, object? value)
}
else
{
ExtraFeatureSet(key, value!); // TODO: What happens if you set an extra feature with a null value?
ExtraFeatureSet(key, value);
}
}
}

TFeature IFeatureCollection.Get<TFeature>()
TFeature? IFeatureCollection.Get<TFeature>() where TFeature : default
{
TFeature? feature = default;
if (typeof(TFeature) == typeof(IConnectionIdFeature))
Expand Down Expand Up @@ -190,10 +190,10 @@ TFeature IFeatureCollection.Get<TFeature>()
feature = (TFeature?)(ExtraFeatureGet(typeof(TFeature)));
}

return feature!;
return feature;
}

void IFeatureCollection.Set<TFeature>(TFeature feature)
void IFeatureCollection.Set<TFeature>(TFeature? feature) where TFeature : default
{
_featureRevision++;
if (typeof(TFeature) == typeof(IConnectionIdFeature))
Expand All @@ -218,7 +218,7 @@ void IFeatureCollection.Set<TFeature>(TFeature feature)
}
else
{
ExtraFeatureSet(typeof(TFeature), feature!); // TODO: What happens if you set an extra feature with a null value?
ExtraFeatureSet(typeof(TFeature), feature);
}
}

Expand Down
10 changes: 5 additions & 5 deletions src/Servers/Kestrel/shared/TransportMultiplexedConnection.Generated.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,12 +157,12 @@ private void ExtraFeatureSet(Type key, object? value)
}
else
{
ExtraFeatureSet(key, value!); // TODO: What happens if you set an extra feature with a null value?
ExtraFeatureSet(key, value);
}
}
}

TFeature IFeatureCollection.Get<TFeature>()
TFeature? IFeatureCollection.Get<TFeature>() where TFeature : default
{
TFeature? feature = default;
if (typeof(TFeature) == typeof(IConnectionIdFeature))
Expand Down Expand Up @@ -190,10 +190,10 @@ TFeature IFeatureCollection.Get<TFeature>()
feature = (TFeature?)(ExtraFeatureGet(typeof(TFeature)));
}

return feature!;
return feature;
}

void IFeatureCollection.Set<TFeature>(TFeature feature)
void IFeatureCollection.Set<TFeature>(TFeature? feature) where TFeature : default
{
_featureRevision++;
if (typeof(TFeature) == typeof(IConnectionIdFeature))
Expand All @@ -218,7 +218,7 @@ void IFeatureCollection.Set<TFeature>(TFeature feature)
}
else
{
ExtraFeatureSet(typeof(TFeature), feature!); // TODO: What happens if you set an extra feature with a null value?
ExtraFeatureSet(typeof(TFeature), feature);
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/Servers/Kestrel/tools/CodeGenerator/FeatureCollectionGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ private void ExtraFeatureSet(Type key, object? value)
}}
}}

TFeature IFeatureCollection.Get<TFeature>()
TFeature? IFeatureCollection.Get<TFeature>() where TFeature : default
{{
TFeature? feature = default;{Each(features, feature => $@"
{(feature.Index != 0 ? "else " : "")}if (typeof(TFeature) == typeof({feature.Name}))
Expand All @@ -159,10 +159,10 @@ TFeature IFeatureCollection.Get<TFeature>()
feature = {fallbackFeatures}.Get<TFeature>();
}}")}

return feature!;
return feature;
}}

void IFeatureCollection.Set<TFeature>(TFeature feature)
void IFeatureCollection.Set<TFeature>(TFeature? feature) where TFeature : default
{{
_featureRevision++;{Each(features, feature => $@"
{(feature.Index != 0 ? "else " : "")}if (typeof(TFeature) == typeof({feature.Name}))
Expand Down
2 changes: 1 addition & 1 deletion ...ignalR/common/Http.Connections/src/Internal/Transports/ServerSentEventsServerTransport.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public async Task ProcessRequestAsync(HttpContext context, CancellationToken tok
context.Response.Headers[HeaderNames.Pragma] = "no-cache";

// Make sure we disable all response buffering for SSE
var bufferingFeature = context.Features.Get<IHttpResponseBodyFeature>();
var bufferingFeature = context.Features.Get<IHttpResponseBodyFeature>()!;
bufferingFeature.DisableBuffering();

context.Response.Headers[HeaderNames.ContentEncoding] = "identity";
Expand Down