Clean up protocol abstractions (#2381)

- This change aims to clean up the feature interfaces
 used by kestrel and exposed by protocol absractions. It splits out the
 IConnectionTransportFeature into smaller features that may or may
 not be implemented on the connection.
- Added all of the features from Socket.Abstractions
in an attempt to make it go away completely. As a result
the helper methods and extensions have all been added here.
- Change IConnectionHandler to take TransportConnection. This cleans up the interface and makes it more explicit what features are required by Kestrel

src/Kestrel.Core/Internal/ConnectionHandler.cs

@@ -28,29 +28,25 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal
 
         private IKestrelTrace Log => _serviceContext.Log;
 
-        public void OnConnection(IFeatureCollection features)
+        public void OnConnection(TransportConnection connection)
         {
-            var connectionContext = new DefaultConnectionContext(features);
-
-            var transportFeature = connectionContext.Features.Get<IConnectionTransportFeature>();
-
             // REVIEW: Unfortunately, we still need to use the service context to create the pipes since the settings
             // for the scheduler and limits are specified here
-            var inputOptions = GetInputPipeOptions(_serviceContext, transportFeature.MemoryPool, transportFeature.InputWriterScheduler);
-            var outputOptions = GetOutputPipeOptions(_serviceContext, transportFeature.MemoryPool, transportFeature.OutputReaderScheduler);
+            var inputOptions = GetInputPipeOptions(_serviceContext, connection.MemoryPool, connection.InputWriterScheduler);
+            var outputOptions = GetOutputPipeOptions(_serviceContext, connection.MemoryPool, connection.OutputReaderScheduler);
 
             var pair = DuplexPipe.CreateConnectionPair(inputOptions, outputOptions);
 
             // Set the transport and connection id
-            connectionContext.ConnectionId = CorrelationIdGenerator.GetNextId();
-            connectionContext.Transport = pair.Transport;
+            connection.ConnectionId = CorrelationIdGenerator.GetNextId();
+            connection.Transport = pair.Transport;
 
             // This *must* be set before returning from OnConnection
-            transportFeature.Application = pair.Application;
+            connection.Application = pair.Application;
 
             // REVIEW: This task should be tracked by the server for graceful shutdown
             // Today it's handled specifically for http but not for aribitrary middleware
-            _ = Execute(connectionContext);
+            _ = Execute(new DefaultConnectionContext(connection));
         }
 
         private async Task Execute(ConnectionContext connectionContext)

src/Kestrel.Core/Internal/HttpConnectionMiddleware.cs

@@ -33,7 +33,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal
         {
             // We need the transport feature so that we can cancel the output reader that the transport is using
             // This is a bit of a hack but it preserves the existing semantics
-            var transportFeature = connectionContext.Features.Get<IConnectionTransportFeature>();
+            var applicationFeature = connectionContext.Features.Get<IApplicationTransportFeature>();
+            var memoryPoolFeature = connectionContext.Features.Get<IMemoryPoolFeature>();
 
             var httpConnectionId = Interlocked.Increment(ref _lastHttpConnectionId);
 
@@ -44,10 +45,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal
                 Protocols = _protocols,
                 ServiceContext = _serviceContext,
                 ConnectionFeatures = connectionContext.Features,
-                MemoryPool = transportFeature.MemoryPool,
+                MemoryPool = memoryPoolFeature.MemoryPool,
                 ConnectionAdapters = _connectionAdapters,
                 Transport = connectionContext.Transport,
-                Application = transportFeature.Application
+                Application = applicationFeature.Application
             };
 
             var connectionFeature = connectionContext.Features.Get<IHttpConnectionFeature>();

src/Kestrel.Transport.Abstractions/Internal/IConnectionHandler.cs

@@ -7,6 +7,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal
 {
     public interface IConnectionHandler
     {
-        void OnConnection(IFeatureCollection features);
+        void OnConnection(TransportConnection connection);
     }
 }

src/Kestrel.Transport.Abstractions/Internal/TransportConnection.Features.cs

@@ -12,15 +12,24 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal
     public partial class TransportConnection : IFeatureCollection,
                                                IHttpConnectionFeature,
                                                IConnectionIdFeature,
-                                               IConnectionTransportFeature
+                                               IConnectionTransportFeature,
+                                               IMemoryPoolFeature,
+                                               IApplicationTransportFeature,
+                                               ITransportSchedulerFeature
     {
         private static readonly Type IHttpConnectionFeatureType = typeof(IHttpConnectionFeature);
         private static readonly Type IConnectionIdFeatureType = typeof(IConnectionIdFeature);
         private static readonly Type IConnectionTransportFeatureType = typeof(IConnectionTransportFeature);
+        private static readonly Type IMemoryPoolFeatureType = typeof(IMemoryPoolFeature);
+        private static readonly Type IApplicationTransportFeatureType = typeof(IApplicationTransportFeature);
+        private static readonly Type ITransportSchedulerFeatureType = typeof(ITransportSchedulerFeature);
 
         private object _currentIHttpConnectionFeature;
         private object _currentIConnectionIdFeature;
         private object _currentIConnectionTransportFeature;
+        private object _currentIMemoryPoolFeature;
+        private object _currentIApplicationTransportFeature;
+        private object _currentITransportSchedulerFeature;
 
         private int _featureRevision;
 
@@ -95,7 +104,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal
             set => LocalPort = value;
         }
 
-        MemoryPool<byte> IConnectionTransportFeature.MemoryPool => MemoryPool;
+        MemoryPool<byte> IMemoryPoolFeature.MemoryPool => MemoryPool;
 
         IDuplexPipe IConnectionTransportFeature.Transport
         {
@@ -103,12 +112,15 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal
             set => Transport = value;
         }
 
-        IDuplexPipe IConnectionTransportFeature.Application
+        IDuplexPipe IApplicationTransportFeature.Application
         {
             get => Application;
             set => Application = value;
         }
 
+        PipeScheduler ITransportSchedulerFeature.InputWriterScheduler => InputWriterScheduler;
+        PipeScheduler ITransportSchedulerFeature.OutputReaderScheduler => OutputReaderScheduler;
+
         object IFeatureCollection.this[Type key]
         {
             get
@@ -128,6 +140,21 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal
                     return _currentIConnectionTransportFeature;
                 }
 
+                if (key == IMemoryPoolFeatureType)
+                {
+                    return _currentIMemoryPoolFeature;
+                }
+
+                if (key == IApplicationTransportFeatureType)
+                {
+                    return _currentIApplicationTransportFeature;
+                }
+
+                if (key == ITransportSchedulerFeatureType)
+                {
+                    return _currentITransportSchedulerFeature;
+                }
+
                 if (MaybeExtra != null)
                 {
                     return ExtraFeatureGet(key);
@@ -151,6 +178,18 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal
                 {
                     _currentIConnectionTransportFeature = value;
                 }
+                else if (key == IMemoryPoolFeatureType)
+                {
+                    _currentIMemoryPoolFeature = value;
+                }
+                else if (key == IApplicationTransportFeatureType)
+                {
+                    _currentIApplicationTransportFeature = value;
+                }
+                else if (key == ITransportSchedulerFeatureType)
+                {
+                    _currentITransportSchedulerFeature = value;
+                }
                 else
                 {
                     ExtraFeatureSet(key, value);
@@ -160,18 +199,30 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal
 
         TFeature IFeatureCollection.Get<TFeature>()
         {
-            if (typeof(TFeature) == typeof(IHttpConnectionFeature))
+            if (typeof(TFeature) == IHttpConnectionFeatureType)
             {
                 return (TFeature)_currentIHttpConnectionFeature;
             }
-            else if (typeof(TFeature) == typeof(IConnectionIdFeature))
+            else if (typeof(TFeature) == IConnectionIdFeatureType)
             {
                 return (TFeature)_currentIConnectionIdFeature;
             }
-            else if (typeof(TFeature) == typeof(IConnectionTransportFeature))
+            else if (typeof(TFeature) == IConnectionTransportFeatureType)
             {
                 return (TFeature)_currentIConnectionTransportFeature;
             }
+            else if (typeof(TFeature) == IMemoryPoolFeatureType)
+            {
+                return (TFeature)_currentIMemoryPoolFeature;
+            }
+            else if (typeof(TFeature) == IApplicationTransportFeatureType)
+            {
+                return (TFeature)_currentIApplicationTransportFeature;
+            }
+            else if (typeof(TFeature) == ITransportSchedulerFeatureType)
+            {
+                return (TFeature)_currentITransportSchedulerFeature;
+            }
             else if (MaybeExtra != null)
             {
                 return (TFeature)ExtraFeatureGet(typeof(TFeature));
@@ -184,18 +235,30 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal
         {
             _featureRevision++;
 
-            if (typeof(TFeature) == typeof(IHttpConnectionFeature))
+            if (typeof(TFeature) == IHttpConnectionFeatureType)
             {
                 _currentIHttpConnectionFeature = instance;
             }
-            else if (typeof(TFeature) == typeof(IConnectionIdFeature))
+            else if (typeof(TFeature) == IConnectionIdFeatureType)
             {
                 _currentIConnectionIdFeature = instance;
             }
-            else if (typeof(TFeature) == typeof(IConnectionTransportFeature))
+            else if (typeof(TFeature) == IConnectionTransportFeatureType)
             {
                 _currentIConnectionTransportFeature = instance;
             }
+            else if (typeof(TFeature) == IMemoryPoolFeatureType)
+            {
+                _currentIMemoryPoolFeature = instance;
+            }
+            else if (typeof(TFeature) == IApplicationTransportFeatureType)
+            {
+                _currentIApplicationTransportFeature = instance;
+            }
+            else if (typeof(TFeature) == ITransportSchedulerFeatureType)
+            {
+                _currentITransportSchedulerFeature = instance;
+            }
             else
             {
                 ExtraFeatureSet(typeof(TFeature), instance);
@@ -223,6 +286,21 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal
                 yield return new KeyValuePair<Type, object>(IConnectionTransportFeatureType, _currentIConnectionTransportFeature);
             }
 
+            if (_currentIMemoryPoolFeature != null)
+            {
+                yield return new KeyValuePair<Type, object>(IMemoryPoolFeatureType, _currentIMemoryPoolFeature);
+            }
+
+            if (_currentIApplicationTransportFeature != null)
+            {
+                yield return new KeyValuePair<Type, object>(IApplicationTransportFeatureType, _currentIApplicationTransportFeature);
+            }
+
+            if (_currentITransportSchedulerFeature != null)
+            {
+                yield return new KeyValuePair<Type, object>(ITransportSchedulerFeatureType, _currentITransportSchedulerFeature);
+            }
+
             if (MaybeExtra != null)
             {
                 foreach (var item in MaybeExtra)

src/Kestrel.Transport.Abstractions/Internal/TransportConnection.cs

@@ -12,6 +12,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal
             _currentIConnectionIdFeature = this;
             _currentIConnectionTransportFeature = this;
             _currentIHttpConnectionFeature = this;
+            _currentIApplicationTransportFeature = this;
+            _currentIMemoryPoolFeature = this;
+            _currentITransportSchedulerFeature = this;
         }
 
         public IPAddress RemoteAddress { get; set; }

src/Protocols.Abstractions/ConnectionBuilderExtensions.cs

@@ -0,0 +1,34 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System;
+using System.Threading.Tasks;
+
+namespace Microsoft.AspNetCore.Protocols
+{
+    public static class ConnectionBuilderExtensions
+    {
+        public static IConnectionBuilder Use(this IConnectionBuilder connectionBuilder, Func<ConnectionContext, Func<Task>, Task> middleware)
+        {
+            return connectionBuilder.Use(next =>
+            {
+                return context =>
+                {
+                    Func<Task> simpleNext = () => next(context);
+                    return middleware(context, simpleNext);
+                };
+            });
+        }
+
+        public static IConnectionBuilder Run(this IConnectionBuilder connectionBuilder, Func<ConnectionContext, Task> middleware)
+        {
+            return connectionBuilder.Use(next =>
+            {
+                return context =>
+                {
+                    return middleware(context);
+                };
+            });
+        }
+    }
+}

src/Protocols.Abstractions/DuplexPipe.cs

@@ -14,10 +14,6 @@ namespace System.IO.Pipelines
 
         public PipeWriter Output { get; }
 
-        public void Dispose()
-        {
-        }
-
         public static DuplexPipePair CreateConnectionPair(PipeOptions inputOptions, PipeOptions outputOptions)
         {
             var input = new Pipe(inputOptions);

src/Protocols.Abstractions/Features/ConnectionBuilder.cs

@@ -0,0 +1,44 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+
+namespace Microsoft.AspNetCore.Protocols
+{
+    public class ConnectionBuilder : IConnectionBuilder
+    {
+        private readonly IList<Func<ConnectionDelegate, ConnectionDelegate>> _components = new List<Func<ConnectionDelegate, ConnectionDelegate>>();
+
+        public IServiceProvider ApplicationServices { get; }
+
+        public ConnectionBuilder(IServiceProvider applicationServices)
+        {
+            ApplicationServices = applicationServices;
+        }
+
+        public IConnectionBuilder Use(Func<ConnectionDelegate, ConnectionDelegate> middleware)
+        {
+            _components.Add(middleware);
+            return this;
+        }
+
+        public ConnectionDelegate Build()
+        {
+            ConnectionDelegate app = features =>
+            {
+                return Task.CompletedTask;
+            };
+
+            foreach (var component in _components.Reverse())
+            {
+                app = component(app);
+            }
+
+            return app;
+        }
+    }
+}

src/Protocols.Abstractions/Features/IApplicationTransportFeature.cs

@@ -0,0 +1,14 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System.Buffers;
+using System.IO.Pipelines;
+using System.Threading;
+
+namespace Microsoft.AspNetCore.Protocols.Features
+{
+    public interface IApplicationTransportFeature
+    {
+        IDuplexPipe Application { get; set; }
+    }
+}

src/Protocols.Abstractions/Features/IConnectionHeartbeatFeature.cs

@@ -0,0 +1,12 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System;
+
+namespace Microsoft.AspNetCore.Protocols.Features
+{
+    public interface IConnectionHeartbeatFeature
+    {
+        void OnHeartbeat(Action<object> action, object state);
+    }
+}

src/Protocols.Abstractions/Features/IConnectionIdFeature.cs

@@ -1,4 +1,7 @@
-namespace Microsoft.AspNetCore.Protocols.Features
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+namespace Microsoft.AspNetCore.Protocols.Features
 {
     public interface IConnectionIdFeature
     {

src/Protocols.Abstractions/Features/IConnectionInherentKeepAliveFeature.cs

@@ -0,0 +1,24 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System;
+using System.Collections.Generic;
+using System.Text;
+
+namespace Microsoft.AspNetCore.Protocols.Features
+{
+    /// <summary>
+    /// Indicates if the connection transport has an "inherent keep-alive", which means that the transport will automatically
+    /// inform the client that it is still present.
+    /// </summary>
+    /// <remarks>
+    /// The most common example of this feature is the Long Polling HTTP transport, which must (due to HTTP limitations) terminate
+    /// each poll within a particular interval and return a signal indicating "the server is still here, but there is no data yet".
+    /// This feature allows applications to add keep-alive functionality, but limit it only to transports that don't have some kind
+    /// of inherent keep-alive.
+    /// </remarks>
+    public interface IConnectionInherentKeepAliveFeature
+    {
+        TimeSpan KeepAliveInterval { get; }
+    }
+}

src/Protocols.Abstractions/Features/IConnectionMetadataFeature.cs

@@ -0,0 +1,13 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System;
+using System.Collections.Generic;
+
+namespace Microsoft.AspNetCore.Protocols.Features
+{
+    public interface IConnectionMetadataFeature
+    {
+        IDictionary<object, object> Metadata { get; set; }
+    }
+}

src/Protocols.Abstractions/Features/IConnectionTransportFeature.cs

@@ -1,4 +1,7 @@
-using System.Buffers;
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System.Buffers;
 using System.IO.Pipelines;
 using System.Threading;
 
@@ -6,14 +9,6 @@ namespace Microsoft.AspNetCore.Protocols.Features
 {
     public interface IConnectionTransportFeature
     {
-        MemoryPool<byte> MemoryPool { get; }
-
         IDuplexPipe Transport { get; set; }
-
-        IDuplexPipe Application { get; set; }
-
-        PipeScheduler InputWriterScheduler { get; }
-
-        PipeScheduler OutputReaderScheduler { get; }
     }
 }

src/Protocols.Abstractions/Features/IConnectionUserFeature.cs

@@ -0,0 +1,9 @@
+using System.Security.Claims;
+
+namespace Microsoft.AspNetCore.Protocols.Features
+{
+    public interface IConnectionUserFeature
+    {
+        ClaimsPrincipal User { get; set; }
+    }
+}

src/Protocols.Abstractions/Features/IMemoryPoolFeature.cs

@@ -0,0 +1,14 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System.Buffers;
+using System.IO.Pipelines;
+using System.Threading;
+
+namespace Microsoft.AspNetCore.Protocols.Features
+{
+    public interface IMemoryPoolFeature
+    {
+        MemoryPool<byte> MemoryPool { get; }
+    }
+}

src/Protocols.Abstractions/Features/ITransportSchedulerFeature.cs

@@ -0,0 +1,16 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System.Buffers;
+using System.IO.Pipelines;
+using System.Threading;
+
+namespace Microsoft.AspNetCore.Protocols.Features
+{
+    public interface ITransportSchedulerFeature
+    {
+        PipeScheduler InputWriterScheduler { get; }
+
+        PipeScheduler OutputReaderScheduler { get; }
+    }
+}

test/Kestrel.Core.Tests/ConnectionHandlerTests.cs

@@ -45,24 +45,13 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
             Assert.True(((TestKestrelTrace)serviceContext.Log).Logger.Scopes.IsEmpty);
         }
 
-        private class TestConnection : FeatureCollection, IConnectionIdFeature, IConnectionTransportFeature
+        private class TestConnection : TransportConnection
         {
-            public TestConnection()
-            {
-                Set<IConnectionIdFeature>(this);
-                Set<IConnectionTransportFeature>(this);
-            }
+            public override MemoryPool<byte> MemoryPool { get; } = KestrelMemoryPool.Create();
 
-            public MemoryPool<byte> MemoryPool { get; } = KestrelMemoryPool.Create();
+            public override PipeScheduler InputWriterScheduler => PipeScheduler.ThreadPool;
 
-            public IDuplexPipe Transport { get; set; }
-            public IDuplexPipe Application { get; set; }
-
-            public PipeScheduler InputWriterScheduler => PipeScheduler.ThreadPool;
-
-            public PipeScheduler OutputReaderScheduler => PipeScheduler.ThreadPool;
-
-            public string ConnectionId { get; set; }
+            public override PipeScheduler OutputReaderScheduler => PipeScheduler.ThreadPool;
         }
     }
 }

test/Kestrel.Transport.Libuv.Tests/TestHelpers/MockConnectionHandler.cs

@@ -16,17 +16,13 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Tests.TestHelpers
         public Func<MemoryPool<byte>, PipeOptions> InputOptions { get; set; } = pool => new PipeOptions(pool, readerScheduler: PipeScheduler.Inline, writerScheduler: PipeScheduler.Inline, useSynchronizationContext: false);
         public Func<MemoryPool<byte>, PipeOptions> OutputOptions { get; set; } = pool => new PipeOptions(pool, readerScheduler: PipeScheduler.Inline, writerScheduler: PipeScheduler.Inline, useSynchronizationContext: false);
 
-        public void OnConnection(IFeatureCollection features)
+        public void OnConnection(TransportConnection connection)
         {
-            var connectionContext = new DefaultConnectionContext(features);
+            Input = new Pipe(InputOptions(connection.MemoryPool));
+            Output = new Pipe(OutputOptions(connection.MemoryPool));
 
-            var feature = connectionContext.Features.Get<IConnectionTransportFeature>();
-
-            Input = new Pipe(InputOptions(feature.MemoryPool));
-            Output = new Pipe(OutputOptions(feature.MemoryPool));
-
-            connectionContext.Transport = new DuplexPipe(Input.Reader, Output.Writer);
-            feature.Application = new DuplexPipe(Output.Reader, Input.Writer);
+            connection.Transport = new DuplexPipe(Input.Reader, Output.Writer);
+            connection.Application = new DuplexPipe(Output.Reader, Input.Writer);
         }
 
         public Pipe Input { get; private set; }