Drop MultiplexedConnectionMarkerService

amcasey · amcasey · commit 8041dd111a5e · 2023-03-15T17:54:03.000-07:00
diff --git a/src/Servers/Connections.Abstractions/src/MultiplexedConnectionMarkerService.cs b/src/Servers/Connections.Abstractions/src/MultiplexedConnectionMarkerService.cs
diff --git a/src/Servers/Connections.Abstractions/src/PublicAPI/net462/PublicAPI.Unshipped.txt b/src/Servers/Connections.Abstractions/src/PublicAPI/net462/PublicAPI.Unshipped.txt
@@ -3,13 +3,11 @@ Microsoft.AspNetCore.Connections.Features.IConnectionNamedPipeFeature
 Microsoft.AspNetCore.Connections.Features.IConnectionNamedPipeFeature.NamedPipe.get -> System.IO.Pipes.NamedPipeServerStream!
 Microsoft.AspNetCore.Connections.IConnectionListenerFactorySelector
 Microsoft.AspNetCore.Connections.IConnectionListenerFactorySelector.CanBind(System.Net.EndPoint! endpoint) -> bool
-Microsoft.AspNetCore.Connections.MultiplexedConnectionMarkerService
-Microsoft.AspNetCore.Connections.MultiplexedConnectionMarkerService.MultiplexedConnectionMarkerService() -> void
 Microsoft.AspNetCore.Connections.NamedPipeEndPoint
 Microsoft.AspNetCore.Connections.NamedPipeEndPoint.NamedPipeEndPoint(string! pipeName) -> void
 Microsoft.AspNetCore.Connections.NamedPipeEndPoint.NamedPipeEndPoint(string! pipeName, string! serverName) -> void
 Microsoft.AspNetCore.Connections.NamedPipeEndPoint.PipeName.get -> string!
 Microsoft.AspNetCore.Connections.NamedPipeEndPoint.ServerName.get -> string!
 override Microsoft.AspNetCore.Connections.NamedPipeEndPoint.Equals(object? obj) -> bool
 override Microsoft.AspNetCore.Connections.NamedPipeEndPoint.GetHashCode() -> int
-override Microsoft.AspNetCore.Connections.NamedPipeEndPoint.ToString() -> string!
+override Microsoft.AspNetCore.Connections.NamedPipeEndPoint.ToString() -> string!
diff --git a/src/Servers/Connections.Abstractions/src/PublicAPI/net8.0/PublicAPI.Unshipped.txt b/src/Servers/Connections.Abstractions/src/PublicAPI/net8.0/PublicAPI.Unshipped.txt
@@ -4,8 +4,6 @@ Microsoft.AspNetCore.Connections.Features.IConnectionNamedPipeFeature.NamedPipe.
 Microsoft.AspNetCore.Connections.Features.ITlsHandshakeFeature.NegotiatedCipherSuite.get -> System.Net.Security.TlsCipherSuite?
 Microsoft.AspNetCore.Connections.IConnectionListenerFactorySelector
 Microsoft.AspNetCore.Connections.IConnectionListenerFactorySelector.CanBind(System.Net.EndPoint! endpoint) -> bool
-Microsoft.AspNetCore.Connections.MultiplexedConnectionMarkerService
-Microsoft.AspNetCore.Connections.MultiplexedConnectionMarkerService.MultiplexedConnectionMarkerService() -> void
 Microsoft.AspNetCore.Connections.NamedPipeEndPoint
 Microsoft.AspNetCore.Connections.NamedPipeEndPoint.NamedPipeEndPoint(string! pipeName) -> void
 Microsoft.AspNetCore.Connections.NamedPipeEndPoint.NamedPipeEndPoint(string! pipeName, string! serverName) -> void
diff --git a/src/Servers/Connections.Abstractions/src/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt b/src/Servers/Connections.Abstractions/src/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt
@@ -3,13 +3,11 @@ Microsoft.AspNetCore.Connections.Features.IConnectionNamedPipeFeature
 Microsoft.AspNetCore.Connections.Features.IConnectionNamedPipeFeature.NamedPipe.get -> System.IO.Pipes.NamedPipeServerStream!
 Microsoft.AspNetCore.Connections.IConnectionListenerFactorySelector
 Microsoft.AspNetCore.Connections.IConnectionListenerFactorySelector.CanBind(System.Net.EndPoint! endpoint) -> bool
-Microsoft.AspNetCore.Connections.MultiplexedConnectionMarkerService
-Microsoft.AspNetCore.Connections.MultiplexedConnectionMarkerService.MultiplexedConnectionMarkerService() -> void
 Microsoft.AspNetCore.Connections.NamedPipeEndPoint
 Microsoft.AspNetCore.Connections.NamedPipeEndPoint.NamedPipeEndPoint(string! pipeName) -> void
 Microsoft.AspNetCore.Connections.NamedPipeEndPoint.NamedPipeEndPoint(string! pipeName, string! serverName) -> void
 Microsoft.AspNetCore.Connections.NamedPipeEndPoint.PipeName.get -> string!
 Microsoft.AspNetCore.Connections.NamedPipeEndPoint.ServerName.get -> string!
 override Microsoft.AspNetCore.Connections.NamedPipeEndPoint.Equals(object? obj) -> bool
 override Microsoft.AspNetCore.Connections.NamedPipeEndPoint.GetHashCode() -> int
-override Microsoft.AspNetCore.Connections.NamedPipeEndPoint.ToString() -> string!
+override Microsoft.AspNetCore.Connections.NamedPipeEndPoint.ToString() -> string!
diff --git a/src/Servers/Connections.Abstractions/src/PublicAPI/netstandard2.1/PublicAPI.Unshipped.txt b/src/Servers/Connections.Abstractions/src/PublicAPI/netstandard2.1/PublicAPI.Unshipped.txt
@@ -3,13 +3,11 @@ Microsoft.AspNetCore.Connections.Features.IConnectionNamedPipeFeature
 Microsoft.AspNetCore.Connections.Features.IConnectionNamedPipeFeature.NamedPipe.get -> System.IO.Pipes.NamedPipeServerStream!
 Microsoft.AspNetCore.Connections.IConnectionListenerFactorySelector
 Microsoft.AspNetCore.Connections.IConnectionListenerFactorySelector.CanBind(System.Net.EndPoint! endpoint) -> bool
-Microsoft.AspNetCore.Connections.MultiplexedConnectionMarkerService
-Microsoft.AspNetCore.Connections.MultiplexedConnectionMarkerService.MultiplexedConnectionMarkerService() -> void
 Microsoft.AspNetCore.Connections.NamedPipeEndPoint
 Microsoft.AspNetCore.Connections.NamedPipeEndPoint.NamedPipeEndPoint(string! pipeName) -> void
 Microsoft.AspNetCore.Connections.NamedPipeEndPoint.NamedPipeEndPoint(string! pipeName, string! serverName) -> void
 Microsoft.AspNetCore.Connections.NamedPipeEndPoint.PipeName.get -> string!
 Microsoft.AspNetCore.Connections.NamedPipeEndPoint.ServerName.get -> string!
 override Microsoft.AspNetCore.Connections.NamedPipeEndPoint.Equals(object? obj) -> bool
 override Microsoft.AspNetCore.Connections.NamedPipeEndPoint.GetHashCode() -> int
-override Microsoft.AspNetCore.Connections.NamedPipeEndPoint.ToString() -> string!
+override Microsoft.AspNetCore.Connections.NamedPipeEndPoint.ToString() -> string!
diff --git a/src/Servers/Kestrel/Core/src/Internal/KestrelServerImpl.cs b/src/Servers/Kestrel/Core/src/Internal/KestrelServerImpl.cs
@@ -125,7 +125,8 @@ async Task OnBind(ListenOptions options, CancellationToken onBindCancellationTok
 
                 if (hasHttp3 && !haveMultiplexedFactories)
                 {
-                    if (Options.ApplicationServices.GetService(typeof(MultiplexedConnectionMarkerService)) is null)
+                    // There will be a IMultiplexedConnectionListenerFactory iff UseQuic was called
+                    if (options.ProtocolsSetExplicitly && Options.ApplicationServices.GetService(typeof(IMultiplexedConnectionListenerFactory)) is null)
                     {
                         throw new InvalidOperationException("You need to call UseQuic"); // TODO (acasey): message
                     }
diff --git a/src/Servers/Kestrel/Core/test/KestrelServerTests.cs b/src/Servers/Kestrel/Core/test/KestrelServerTests.cs
@@ -31,7 +31,6 @@ private KestrelServerOptions CreateServerOptions()
         var serverOptions = new KestrelServerOptions();
         serverOptions.ApplicationServices = new ServiceCollection()
             .AddLogging()
-            .AddSingleton<MultiplexedConnectionMarkerService>()
             .BuildServiceProvider();
         return serverOptions;
     }
diff --git a/src/Servers/Kestrel/Transport.Quic/src/QuicTransportFactory.cs b/src/Servers/Kestrel/Transport.Quic/src/QuicTransportFactory.cs
@@ -1,7 +1,9 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.Diagnostics;
 using System.Net;
+using System.Net.Quic;
 using Microsoft.AspNetCore.Connections;
 using Microsoft.AspNetCore.Http.Features;
 using Microsoft.AspNetCore.Server.Kestrel.Transport.Quic.Internal;
@@ -38,6 +40,7 @@ public QuicTransportFactory(ILoggerFactory loggerFactory, IOptions<QuicTransport
     public async ValueTask<IMultiplexedConnectionListener> BindAsync(EndPoint endpoint, IFeatureCollection? features = null, CancellationToken cancellationToken = default)
     {
         ArgumentNullException.ThrowIfNull(endpoint);
+        Debug.Assert(CanBind(endpoint));
 
         var tlsConnectionOptions = features?.Get<TlsConnectionCallbackOptions>();
 
@@ -58,6 +61,6 @@ public async ValueTask<IMultiplexedConnectionListener> BindAsync(EndPoint endpoi
 
     public bool CanBind(EndPoint endpoint)
     {
-        return endpoint is IPEndPoint;
+        return endpoint is IPEndPoint && QuicListener.IsSupported;
     }
 }
diff --git a/src/Servers/Kestrel/Transport.Quic/src/WebHostBuilderQuicExtensions.cs b/src/Servers/Kestrel/Transport.Quic/src/WebHostBuilderQuicExtensions.cs
@@ -1,7 +1,6 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
-using System.Net.Quic;
 using Microsoft.AspNetCore.Connections;
 using Microsoft.AspNetCore.Server.Kestrel.Transport.Quic;
 using Microsoft.Extensions.DependencyInjection;
@@ -20,22 +19,11 @@ public static class WebHostBuilderQuicExtensions
     /// <returns>The <see cref="IWebHostBuilder"/>.</returns>
     public static IWebHostBuilder UseQuic(this IWebHostBuilder hostBuilder)
     {
-        // In order to be able to provide useful error messages in slim scenarios, we have to be able
-        // to distinguish between QUIC-was-not-requested and QUIC-is-not-available.
-        hostBuilder.ConfigureServices(services =>
+        return hostBuilder.ConfigureServices(services =>
         {
-            services.AddSingleton<MultiplexedConnectionMarkerService>();
+            // CanBind will return false if QuicListener.IsSupported is false
+            services.AddSingleton<IMultiplexedConnectionListenerFactory, QuicTransportFactory>();
         });
-
-        if (QuicListener.IsSupported)
-        {
-            return hostBuilder.ConfigureServices(services =>
-            {
-                services.AddSingleton<IMultiplexedConnectionListenerFactory, QuicTransportFactory>();
-            });
-        }
-
-        return hostBuilder;
     }
 
     /// <summary>
diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/TestTransport/TestServer.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/TestTransport/TestServer.cs
@@ -88,9 +88,6 @@ public TestServer(RequestDelegate app, TestServiceContext context, Action<Kestre
                 services.AddSingleton<IStartup>(this);
                 services.AddSingleton(context.LoggerFactory);
 
-                // TODO (acasey): this feels like a hack and will likely make it hard to test the real feature
-                services.AddSingleton<MultiplexedConnectionMarkerService>();
-
                 services.AddSingleton<IServer>(sp =>
                 {
                     context.ServerOptions.ApplicationServices = sp;

Original file line number	Diff line number	Diff line change
`@@ -125,7 +125,8 @@ async Task OnBind(ListenOptions options, CancellationToken onBindCancellationTok`
`125`	`125`
`126`	`126`	`if (hasHttp3 && !haveMultiplexedFactories)`
`127`	`127`	`{`
`128`		`- if (Options.ApplicationServices.GetService(typeof(MultiplexedConnectionMarkerService)) is null)`
	`128`	`+ // There will be a IMultiplexedConnectionListenerFactory iff UseQuic was called`
	`129`	`+ if (options.ProtocolsSetExplicitly && Options.ApplicationServices.GetService(typeof(IMultiplexedConnectionListenerFactory)) is null)`
`129`	`130`	`{`
`130`	`131`	`throw new InvalidOperationException("You need to call UseQuic"); // TODO (acasey): message`
`131`	`132`	`}`
Original file line number	Diff line number	Diff line change
`@@ -31,7 +31,6 @@ private KestrelServerOptions CreateServerOptions()`
`31`	`31`	`var serverOptions = new KestrelServerOptions();`
`32`	`32`	`serverOptions.ApplicationServices = new ServiceCollection()`
`33`	`33`	`.AddLogging()`
`34`		`- .AddSingleton<MultiplexedConnectionMarkerService>()`
`35`	`34`	`.BuildServiceProvider();`
`36`	`35`	`return serverOptions;`
`37`	`36`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,7 +1,9 @@`
`1`	`1`	`// Licensed to the .NET Foundation under one or more agreements.`
`2`	`2`	`// The .NET Foundation licenses this file to you under the MIT license.`
`3`	`3`
	`4`	`+using System.Diagnostics;`
`4`	`5`	`using System.Net;`
	`6`	`+using System.Net.Quic;`
`5`	`7`	`using Microsoft.AspNetCore.Connections;`
`6`	`8`	`using Microsoft.AspNetCore.Http.Features;`
`7`	`9`	`using Microsoft.AspNetCore.Server.Kestrel.Transport.Quic.Internal;`
`@@ -38,6 +40,7 @@ public QuicTransportFactory(ILoggerFactory loggerFactory, IOptions<QuicTransport`
`38`	`40`	`public async ValueTask<IMultiplexedConnectionListener> BindAsync(EndPoint endpoint, IFeatureCollection? features = null, CancellationToken cancellationToken = default)`
`39`	`41`	`{`
`40`	`42`	`ArgumentNullException.ThrowIfNull(endpoint);`
	`43`	`+ Debug.Assert(CanBind(endpoint));`
`41`	`44`
`42`	`45`	`var tlsConnectionOptions = features?.Get<TlsConnectionCallbackOptions>();`
`43`	`46`
`@@ -58,6 +61,6 @@ public async ValueTask<IMultiplexedConnectionListener> BindAsync(EndPoint endpoi`
`58`	`61`
`59`	`62`	`public bool CanBind(EndPoint endpoint)`
`60`	`63`	`{`
`61`		`- return endpoint is IPEndPoint;`
	`64`	`+ return endpoint is IPEndPoint && QuicListener.IsSupported;`
`62`	`65`	`}`
`63`	`66`	`}`
Original file line number	Diff line number	Diff line change
`@@ -88,9 +88,6 @@ public TestServer(RequestDelegate app, TestServiceContext context, Action<Kestre`
`88`	`88`	`services.AddSingleton<IStartup>(this);`
`89`	`89`	`services.AddSingleton(context.LoggerFactory);`
`90`	`90`
`91`		`- // TODO (acasey): this feels like a hack and will likely make it hard to test the real feature`
`92`		`- services.AddSingleton<MultiplexedConnectionMarkerService>();`
`93`		`-`
`94`	`91`	`services.AddSingleton<IServer>(sp =>`
`95`	`92`	`{`
`96`	`93`	`context.ServerOptions.ApplicationServices = sp;`