Revert "Clean up protocol abstractions (#2381)" (#2382)

This reverts commit ddd0b4c260.

src/Kestrel.Core/Internal/ConnectionHandler.cs

@@ -28,25 +28,29 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal
 
         private IKestrelTrace Log => _serviceContext.Log;
 
-        public void OnConnection(TransportConnection connection)
+        public void OnConnection(IFeatureCollection features)
         {
+            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, connection.MemoryPool, connection.InputWriterScheduler);
+            var inputOptions = GetInputPipeOptions(_serviceContext, transportFeature.MemoryPool, transportFeature.InputWriterScheduler);
-            var outputOptions = GetOutputPipeOptions(_serviceContext, connection.MemoryPool, connection.OutputReaderScheduler);
+            var outputOptions = GetOutputPipeOptions(_serviceContext, transportFeature.MemoryPool, transportFeature.OutputReaderScheduler);
 
             var pair = DuplexPipe.CreateConnectionPair(inputOptions, outputOptions);
 
             // Set the transport and connection id
-            connection.ConnectionId = CorrelationIdGenerator.GetNextId();
+            connectionContext.ConnectionId = CorrelationIdGenerator.GetNextId();
-            connection.Transport = pair.Transport;
+            connectionContext.Transport = pair.Transport;
 
             // This *must* be set before returning from OnConnection
-            connection.Application = pair.Application;
+            transportFeature.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(new DefaultConnectionContext(connection));
+            _ = Execute(connectionContext);
         }
 
         private async Task Execute(ConnectionContext connectionContext)

src/Kestrel.Core/Internal/HttpConnectionMiddleware.cs

@@ -33,8 +33,7 @@ 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 applicationFeature = connectionContext.Features.Get<IApplicationTransportFeature>();
+            var transportFeature = connectionContext.Features.Get<IConnectionTransportFeature>();
-            var memoryPoolFeature = connectionContext.Features.Get<IMemoryPoolFeature>();
 
             var httpConnectionId = Interlocked.Increment(ref _lastHttpConnectionId);
 
@@ -45,10 +44,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal
                 Protocols = _protocols,
                 ServiceContext = _serviceContext,
                 ConnectionFeatures = connectionContext.Features,
-                MemoryPool = memoryPoolFeature.MemoryPool,
+                MemoryPool = transportFeature.MemoryPool,
                 ConnectionAdapters = _connectionAdapters,
                 Transport = connectionContext.Transport,
-                Application = applicationFeature.Application
+                Application = transportFeature.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(TransportConnection connection);
+        void OnConnection(IFeatureCollection features);
     }
 }

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

@@ -12,24 +12,15 @@ 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;
 
@@ -104,7 +95,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal
             set => LocalPort = value;
         }
 
-        MemoryPool<byte> IMemoryPoolFeature.MemoryPool => MemoryPool;
+        MemoryPool<byte> IConnectionTransportFeature.MemoryPool => MemoryPool;
 
         IDuplexPipe IConnectionTransportFeature.Transport
         {
@@ -112,15 +103,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal
             set => Transport = value;
         }
 
-        IDuplexPipe IApplicationTransportFeature.Application
+        IDuplexPipe IConnectionTransportFeature.Application
         {
             get => Application;
             set => Application = value;
         }
 
-        PipeScheduler ITransportSchedulerFeature.InputWriterScheduler => InputWriterScheduler;
-        PipeScheduler ITransportSchedulerFeature.OutputReaderScheduler => OutputReaderScheduler;
-
         object IFeatureCollection.this[Type key]
         {
             get
@@ -140,21 +128,6 @@ 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);
@@ -178,18 +151,6 @@ 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);
@@ -199,30 +160,18 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal
 
         TFeature IFeatureCollection.Get<TFeature>()
         {
-            if (typeof(TFeature) == IHttpConnectionFeatureType)
+            if (typeof(TFeature) == typeof(IHttpConnectionFeature))
             {
                 return (TFeature)_currentIHttpConnectionFeature;
             }
-            else if (typeof(TFeature) == IConnectionIdFeatureType)
+            else if (typeof(TFeature) == typeof(IConnectionIdFeature))
             {
                 return (TFeature)_currentIConnectionIdFeature;
             }
-            else if (typeof(TFeature) == IConnectionTransportFeatureType)
+            else if (typeof(TFeature) == typeof(IConnectionTransportFeature))
             {
                 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));
@@ -235,30 +184,18 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal
         {
             _featureRevision++;
 
-            if (typeof(TFeature) == IHttpConnectionFeatureType)
+            if (typeof(TFeature) == typeof(IHttpConnectionFeature))
             {
                 _currentIHttpConnectionFeature = instance;
             }
-            else if (typeof(TFeature) == IConnectionIdFeatureType)
+            else if (typeof(TFeature) == typeof(IConnectionIdFeature))
             {
                 _currentIConnectionIdFeature = instance;
             }
-            else if (typeof(TFeature) == IConnectionTransportFeatureType)
+            else if (typeof(TFeature) == typeof(IConnectionTransportFeature))
             {
                 _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);
@@ -286,21 +223,6 @@ 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,9 +12,6 @@ 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

@@ -1,34 +0,0 @@
-// 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,6 +14,10 @@ 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

@@ -1,44 +0,0 @@
-// 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

@@ -1,14 +0,0 @@
-// 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

@@ -1,12 +0,0 @@
-// 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,7 +1,4 @@
-// Copyright (c) .NET Foundation. All rights reserved.
+namespace Microsoft.AspNetCore.Protocols.Features
-// 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

@@ -1,24 +0,0 @@
-// 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

@@ -1,13 +0,0 @@
-// 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,7 +1,4 @@
-// Copyright (c) .NET Foundation. All rights reserved.
+using System.Buffers;
-// 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;
 
@@ -9,6 +6,14 @@ 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

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

src/Protocols.Abstractions/Features/IMemoryPoolFeature.cs

@@ -1,14 +0,0 @@
-// 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

@@ -1,16 +0,0 @@
-// 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,13 +45,24 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
             Assert.True(((TestKestrelTrace)serviceContext.Log).Logger.Scopes.IsEmpty);
         }
 
-        private class TestConnection : TransportConnection
+        private class TestConnection : FeatureCollection, IConnectionIdFeature, IConnectionTransportFeature
         {
-            public override MemoryPool<byte> MemoryPool { get; } = KestrelMemoryPool.Create();
+            public TestConnection()
+            {
+                Set<IConnectionIdFeature>(this);
+                Set<IConnectionTransportFeature>(this);
+            }
 
-            public override PipeScheduler InputWriterScheduler => PipeScheduler.ThreadPool;
+            public MemoryPool<byte> MemoryPool { get; } = KestrelMemoryPool.Create();
 
-            public override PipeScheduler OutputReaderScheduler => 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; }
         }
     }
 }

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

@@ -16,13 +16,17 @@ 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(TransportConnection connection)
+        public void OnConnection(IFeatureCollection features)
         {
-            Input = new Pipe(InputOptions(connection.MemoryPool));
+            var connectionContext = new DefaultConnectionContext(features);
-            Output = new Pipe(OutputOptions(connection.MemoryPool));
 
-            connection.Transport = new DuplexPipe(Input.Reader, Output.Writer);
+            var feature = connectionContext.Features.Get<IConnectionTransportFeature>();
-            connection.Application = new DuplexPipe(Output.Reader, Input.Writer);
+
+            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);
         }
 
         public Pipe Input { get; private set; }