HttpTransportType refactor (#1873)

- Rename file to HttpTransportType.cs
- Add HttpTransportType.None
- Move All to static readonly field
- Make TransportType on client and service HttpConnectionOptions consistent
- Move setting defaults into ctor

samples/JwtClientSample/Program.cs

@@ -34,7 +34,7 @@ namespace JwtClientSample
             var hubConnection = new HubConnectionBuilder()
                 .WithUrl(ServerUrl + "/broadcast", options =>
                 {
-                    options.Transport = transportType;
+                    options.Transports = transportType;
                     options.AccessTokenFactory = () => _tokens[userId];
                 })
                 .Build();

src/Microsoft.AspNetCore.Http.Connections.Client/DefaultTransportFactory.cs

@@ -20,11 +20,6 @@ namespace Microsoft.AspNetCore.Http.Connections.Client
 
         public DefaultTransportFactory(HttpTransportType requestedTransportType, ILoggerFactory loggerFactory, HttpClient httpClient, HttpOptions httpOptions)
         {
-            if (requestedTransportType <= 0 || requestedTransportType > HttpTransportType.All)
-            {
-                throw new ArgumentOutOfRangeException(nameof(requestedTransportType));
-            }
-
             if (httpClient == null && requestedTransportType != HttpTransportType.WebSockets)
             {
                 throw new ArgumentNullException(nameof(httpClient));

src/Microsoft.AspNetCore.Http.Connections.Client/HttpConnection.cs

@@ -37,7 +37,7 @@ namespace Microsoft.AspNetCore.Http.Connections.Client
         private ITransport _transport;
         private readonly ITransportFactory _transportFactory;
         private string _connectionId;
-        private readonly HttpTransportType _requestedTransportType = HttpTransportType.All;
+        private readonly HttpTransportType _requestedTransports;
         private readonly ConnectionLogScope _logScope;
         private readonly IDisposable _scopeDisposable;
         private readonly ILoggerFactory _loggerFactory;
@@ -60,25 +60,25 @@ namespace Microsoft.AspNetCore.Http.Connections.Client
         public IFeatureCollection Features { get; } = new FeatureCollection();
 
         public HttpConnection(Uri url)
-            : this(url, HttpTransportType.All)
+            : this(url, HttpTransports.All)
         { }
 
-        public HttpConnection(Uri url, HttpTransportType transportType)
+        public HttpConnection(Uri url, HttpTransportType transports)
-            : this(url, transportType, loggerFactory: null)
+            : this(url, transports, loggerFactory: null)
         {
         }
 
         public HttpConnection(Uri url, ILoggerFactory loggerFactory)
-            : this(url, HttpTransportType.All, loggerFactory, httpOptions: null)
+            : this(url, HttpTransports.All, loggerFactory, httpOptions: null)
         {
         }
 
-        public HttpConnection(Uri url, HttpTransportType transportType, ILoggerFactory loggerFactory)
+        public HttpConnection(Uri url, HttpTransportType transports, ILoggerFactory loggerFactory)
-            : this(url, transportType, loggerFactory, httpOptions: null)
+            : this(url, transports, loggerFactory, httpOptions: null)
         {
         }
 
-        public HttpConnection(Uri url, HttpTransportType transportType, ILoggerFactory loggerFactory, HttpOptions httpOptions)
+        public HttpConnection(Uri url, HttpTransportType transports, ILoggerFactory loggerFactory, HttpOptions httpOptions)
         {
             Url = url ?? throw new ArgumentNullException(nameof(url));
             _loggerFactory = loggerFactory ?? NullLoggerFactory.Instance;
@@ -86,13 +86,13 @@ namespace Microsoft.AspNetCore.Http.Connections.Client
             _logger = _loggerFactory.CreateLogger<HttpConnection>();
             _httpOptions = httpOptions;
 
-            _requestedTransportType = transportType;
+            _requestedTransports = transports;
-            if (_requestedTransportType != HttpTransportType.WebSockets)
+            if (_requestedTransports != HttpTransportType.WebSockets)
             {
                 _httpClient = CreateHttpClient();
             }
 
-            _transportFactory = new DefaultTransportFactory(transportType, _loggerFactory, _httpClient, httpOptions);
+            _transportFactory = new DefaultTransportFactory(transports, _loggerFactory, _httpClient, httpOptions);
             _logScope = new ConnectionLogScope();
             _scopeDisposable = _logger.BeginScope(_logScope);
         }
@@ -104,6 +104,7 @@ namespace Microsoft.AspNetCore.Http.Connections.Client
             _logger = _loggerFactory.CreateLogger<HttpConnection>();
             _httpOptions = httpOptions;
             _httpClient = CreateHttpClient();
+            _requestedTransports = HttpTransports.All;
             _transportFactory = transportFactory ?? throw new ArgumentNullException(nameof(transportFactory));
             _logScope = new ConnectionLogScope();
             _scopeDisposable = _logger.BeginScope(_logScope);
@@ -205,10 +206,10 @@ namespace Microsoft.AspNetCore.Http.Connections.Client
 
         private async Task SelectAndStartTransport(TransferFormat transferFormat)
         {
-            if (_requestedTransportType == HttpTransportType.WebSockets)
+            if (_requestedTransports == HttpTransportType.WebSockets)
             {
-                Log.StartingTransport(_logger, _requestedTransportType, Url);
+                Log.StartingTransport(_logger, _requestedTransports, Url);
-                await StartTransport(Url, _requestedTransportType, transferFormat);
+                await StartTransport(Url, _requestedTransports, transferFormat);
             }
             else
             {
@@ -238,7 +239,7 @@ namespace Microsoft.AspNetCore.Http.Connections.Client
 
                     try
                     {
-                        if ((transportType & _requestedTransportType) == 0)
+                        if ((transportType & _requestedTransports) == 0)
                         {
                             Log.TransportDisabledByClient(_logger, transportType);
                         }

src/Microsoft.AspNetCore.Http.Connections.Common/HttpTransportType.cs

@@ -8,9 +8,9 @@ namespace Microsoft.AspNetCore.Http.Connections
     [Flags]
     public enum HttpTransportType
     {
+        None = 0,
         WebSockets = 1,
         ServerSentEvents = 2,
         LongPolling = 4,
-        All = WebSockets | ServerSentEvents | LongPolling
     }
 }

src/Microsoft.AspNetCore.Http.Connections.Common/Internal/HttpTransports.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.
+
+namespace Microsoft.AspNetCore.Http.Connections.Internal
+{
+    public static class HttpTransports
+    {
+        // Note that this is static readonly instead of const so it is not baked into a DLL when referenced
+        // Updating package without recompiling will automatically pick up new transports added here
+        public static readonly HttpTransportType All = HttpTransportType.WebSockets | HttpTransportType.ServerSentEvents | HttpTransportType.LongPolling;
+    }
+}

src/Microsoft.AspNetCore.Http.Connections/HttpConnectionOptions.cs

@@ -3,21 +3,32 @@
 
 using System.Collections.Generic;
 using Microsoft.AspNetCore.Authorization;
+using Microsoft.AspNetCore.Http.Connections.Internal;
 
 namespace Microsoft.AspNetCore.Http.Connections
 {
     public class HttpConnectionOptions
     {
-        public IList<IAuthorizeData> AuthorizationData { get; } = new List<IAuthorizeData>();
+        public HttpConnectionOptions()
+        {
+            AuthorizationData = new List<IAuthorizeData>();
+            Transports = HttpTransports.All;
+            WebSockets = new WebSocketOptions();
+            LongPolling = new LongPollingOptions();
+            TransportMaxBufferSize = 0;
+            ApplicationMaxBufferSize = 0;
+        }
 
-        public HttpTransportType Transports { get; set; } = HttpTransportType.All;
+        public IList<IAuthorizeData> AuthorizationData { get; }
 
-        public WebSocketOptions WebSockets { get; } = new WebSocketOptions();
+        public HttpTransportType Transports { get; set; }
 
-        public LongPollingOptions LongPolling { get; } = new LongPollingOptions();
+        public WebSocketOptions WebSockets { get; }
 
-        public long TransportMaxBufferSize { get; set; } = 0;
+        public LongPollingOptions LongPolling { get; }
 
-        public long ApplicationMaxBufferSize { get; set; } = 0;
+        public long TransportMaxBufferSize { get; set; }
+
+        public long ApplicationMaxBufferSize { get; set; }
     }
 }

src/Microsoft.AspNetCore.SignalR.Client/HttpConnectionOptions.cs

@@ -8,6 +8,7 @@ using System.Net.Http;
 using System.Net.WebSockets;
 using System.Security.Cryptography.X509Certificates;
 using Microsoft.AspNetCore.Http.Connections;
+using Microsoft.AspNetCore.Http.Connections.Internal;
 
 namespace Microsoft.AspNetCore.SignalR.Client
 {
@@ -17,8 +18,13 @@ namespace Microsoft.AspNetCore.SignalR.Client
         internal IDictionary<string, string> _headers;
         internal CookieContainer _cookies;
 
+        public HttpConnectionOptions()
+        {
+            Transports = HttpTransports.All;
+        }
+
         public Uri Url { get; set; }
-        public HttpTransportType? Transport { get; set; }
+        public HttpTransportType Transports { get; set; }
         public Func<HttpMessageHandler, HttpMessageHandler> MessageHandlerFactory { get; set; }
         public bool? UseDefaultCredentials { get; set; }
         public ICredentials Credentials { get; set; }

src/Microsoft.AspNetCore.SignalR.Client/HubConnectionBuilderHttpExtensions.cs

@@ -6,6 +6,7 @@ using System.Collections.ObjectModel;
 using Microsoft.AspNetCore.Connections;
 using Microsoft.AspNetCore.Http.Connections;
 using Microsoft.AspNetCore.Http.Connections.Client;
+using Microsoft.AspNetCore.Http.Connections.Internal;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Options;
@@ -16,52 +17,61 @@ namespace Microsoft.AspNetCore.SignalR.Client
     {
         public static IHubConnectionBuilder WithUrl(this IHubConnectionBuilder hubConnectionBuilder, string url)
         {
-            hubConnectionBuilder.WithUrl(new Uri(url), null, _ => { });
+            hubConnectionBuilder.WithUrlCore(new Uri(url), null, _ => { });
             return hubConnectionBuilder;
         }
 
         public static IHubConnectionBuilder WithUrl(this IHubConnectionBuilder hubConnectionBuilder, string url, Action<HttpConnectionOptions> configureHttpConnection)
         {
-            hubConnectionBuilder.WithUrl(new Uri(url), null, configureHttpConnection);
+            hubConnectionBuilder.WithUrlCore(new Uri(url), null, configureHttpConnection);
             return hubConnectionBuilder;
         }
 
-        public static IHubConnectionBuilder WithUrl(this IHubConnectionBuilder hubConnectionBuilder, string url, HttpTransportType? transportType)
+        public static IHubConnectionBuilder WithUrl(this IHubConnectionBuilder hubConnectionBuilder, string url, HttpTransportType transports)
         {
-            hubConnectionBuilder.WithUrl(new Uri(url), transportType, _ => { });
+            hubConnectionBuilder.WithUrlCore(new Uri(url), transports, _ => { });
             return hubConnectionBuilder;
         }
 
-        public static IHubConnectionBuilder WithUrl(this IHubConnectionBuilder hubConnectionBuilder, string url, HttpTransportType? transportType, Action<HttpConnectionOptions> configureHttpConnection)
+        public static IHubConnectionBuilder WithUrl(this IHubConnectionBuilder hubConnectionBuilder, string url, HttpTransportType transports, Action<HttpConnectionOptions> configureHttpConnection)
         {
-            hubConnectionBuilder.WithUrl(new Uri(url), transportType, configureHttpConnection);
+            hubConnectionBuilder.WithUrlCore(new Uri(url), transports, configureHttpConnection);
             return hubConnectionBuilder;
         }
 
         public static IHubConnectionBuilder WithUrl(this IHubConnectionBuilder hubConnectionBuilder, Uri url)
         {
-            hubConnectionBuilder.WithUrl(url, null, _ => { });
+            hubConnectionBuilder.WithUrlCore(url, null, _ => { });
             return hubConnectionBuilder;
         }
 
         public static IHubConnectionBuilder WithUrl(this IHubConnectionBuilder hubConnectionBuilder, Uri url, Action<HttpConnectionOptions> configureHttpConnection)
         {
-            hubConnectionBuilder.WithUrl(url, null, configureHttpConnection);
+            hubConnectionBuilder.WithUrlCore(url, null, configureHttpConnection);
             return hubConnectionBuilder;
         }
 
-        public static IHubConnectionBuilder WithUrl(this IHubConnectionBuilder hubConnectionBuilder, Uri url, HttpTransportType? transportType)
+        public static IHubConnectionBuilder WithUrl(this IHubConnectionBuilder hubConnectionBuilder, Uri url, HttpTransportType transports)
         {
-            hubConnectionBuilder.WithUrl(url, null, _ => { });
+            hubConnectionBuilder.WithUrlCore(url, null, _ => { });
             return hubConnectionBuilder;
         }
 
-        public static IHubConnectionBuilder WithUrl(this IHubConnectionBuilder hubConnectionBuilder, Uri url, HttpTransportType? transportType, Action<HttpConnectionOptions> configureHttpConnection)
+        public static IHubConnectionBuilder WithUrl(this IHubConnectionBuilder hubConnectionBuilder, Uri url, HttpTransportType transports, Action<HttpConnectionOptions> configureHttpConnection)
+        {
+            hubConnectionBuilder.WithUrlCore(url, transports, _ => { });
+            return hubConnectionBuilder;
+        }
+
+        private static IHubConnectionBuilder WithUrlCore(this IHubConnectionBuilder hubConnectionBuilder, Uri url, HttpTransportType? transports, Action<HttpConnectionOptions> configureHttpConnection)
         {
             hubConnectionBuilder.Services.Configure<HttpConnectionOptions>(o =>
             {
                 o.Url = url;
-                o.Transport = transportType;
+                if (transports != null)
+                {
+                    o.Transports = transports.Value;
+                }
             });
 
             if (configureHttpConnection != null)
@@ -88,7 +98,7 @@ namespace Microsoft.AspNetCore.SignalR.Client
 
                 Func<IConnection> createConnection = () => new HttpConnection(
                     value.Url,
-                    value.Transport ?? HttpTransportType.All,
+                    value.Transports,
                     services.GetService<ILoggerFactory>(),
                     httpOptions);
 

test/Microsoft.AspNetCore.Http.Connections.Tests/HttpConnectionDispatcherTests.cs

@@ -147,8 +147,8 @@ namespace Microsoft.AspNetCore.Http.Connections.Tests
         }
 
         [Theory]
-        [InlineData(HttpTransportType.All)]
+        [InlineData(HttpTransportType.LongPolling | HttpTransportType.WebSockets | HttpTransportType.ServerSentEvents)]
-        [InlineData((HttpTransportType)0)]
+        [InlineData(HttpTransportType.None)]
         [InlineData(HttpTransportType.LongPolling | HttpTransportType.WebSockets)]
         public async Task NegotiateReturnsAvailableTransportsAfterFilteringByOptions(HttpTransportType transports)
         {
@@ -169,7 +169,7 @@ namespace Microsoft.AspNetCore.Http.Connections.Tests
                 await dispatcher.ExecuteNegotiateAsync(context, new HttpConnectionOptions { Transports = transports });
 
                 var negotiateResponse = JsonConvert.DeserializeObject<JObject>(Encoding.UTF8.GetString(ms.ToArray()));
-                var availableTransports = (HttpTransportType)0;
+                var availableTransports = HttpTransportType.None;
                 foreach (var transport in negotiateResponse["availableTransports"])
                 {
                     var transportType = (HttpTransportType)Enum.Parse(typeof(HttpTransportType), transport.Value<string>("transport"));

test/Microsoft.AspNetCore.SignalR.Client.FunctionalTests/HubConnectionTests.cs

@@ -52,7 +52,7 @@ namespace Microsoft.AspNetCore.SignalR.Client.FunctionalTests
             var hubConnectionBuilder = new HubConnectionBuilder();
             hubConnectionBuilder.WithHubProtocol(protocol);
             hubConnectionBuilder.WithLoggerFactory(loggerFactory);
-            hubConnectionBuilder.WithConnectionFactory(GetHttpConnectionFactory(loggerFactory, path, transportType ?? HttpTransportType.All));
+            hubConnectionBuilder.WithConnectionFactory(GetHttpConnectionFactory(loggerFactory, path, transportType ?? HttpTransportType.LongPolling | HttpTransportType.WebSockets | HttpTransportType.ServerSentEvents));
 
             return hubConnectionBuilder.Build();
         }

test/Microsoft.AspNetCore.SignalR.Client.Tests/HttpConnectionTests.Negotiate.cs

@@ -39,7 +39,7 @@ namespace Microsoft.AspNetCore.SignalR.Client.Tests
             }
 
             [Theory]
-            [InlineData((HttpTransportType)0)]
+            [InlineData(HttpTransportType.None)]
             [InlineData(HttpTransportType.ServerSentEvents)]
             public Task ConnectionCannotBeStartedIfNoCommonTransportsBetweenClientAndServer(HttpTransportType serverTransports)
             {

test/Microsoft.AspNetCore.SignalR.Client.Tests/HttpConnectionTests.cs

@@ -38,15 +38,6 @@ namespace Microsoft.AspNetCore.SignalR.Client.Tests
             Assert.Equal(connectionUrl, new HttpConnection(connectionUrl).Url);
         }
 
-        [Theory]
-        [InlineData((HttpTransportType)0)]
-        [InlineData(HttpTransportType.All + 1)]
-        public void CannotStartConnectionWithInvalidTransportType(HttpTransportType requestedTransportType)
-        {
-            Assert.Throws<ArgumentOutOfRangeException>(
-                () => new HttpConnection(new Uri("http://fakeuri.org/"), requestedTransportType));
-        }
-
         [Fact]
         public async Task HttpOptionsSetOntoHttpClientHandler()
         {

test/Microsoft.AspNetCore.SignalR.Client.Tests/HubConnectionBuilderExtensionsTests.cs

@@ -38,7 +38,7 @@ namespace Microsoft.AspNetCore.SignalR.Client.Tests
 
             var value = serviceProvider.GetService<IOptions<HttpConnectionOptions>>().Value;
 
-            Assert.Equal(HttpTransportType.LongPolling, value.Transport);
+            Assert.Equal(HttpTransportType.LongPolling, value.Transports);
         }
 
         [Fact]

test/Microsoft.AspNetCore.SignalR.Client.Tests/ResponseUtils.cs

@@ -7,6 +7,7 @@ using System.Net;
 using System.Net.Http;
 using Microsoft.AspNetCore.Connections;
 using Microsoft.AspNetCore.Http.Connections;
+using Microsoft.AspNetCore.Http.Connections.Internal;
 using Newtonsoft.Json;
 
 namespace Microsoft.AspNetCore.SignalR.Client.Tests
@@ -37,10 +38,11 @@ namespace Microsoft.AspNetCore.SignalR.Client.Tests
         }
 
         public static string CreateNegotiationContent(string connectionId = "00000000-0000-0000-0000-000000000000",
-            HttpTransportType transportTypes = HttpTransportType.All)
+            HttpTransportType? transportTypes = null)
         {
             var availableTransports = new List<object>();
 
+            transportTypes = transportTypes ?? HttpTransports.All;
             if ((transportTypes & HttpTransportType.WebSockets) != 0)
             {
                 availableTransports.Add(new

test/Microsoft.AspNetCore.SignalR.Tests/DefaultTransportFactoryTests.cs

@@ -14,17 +14,18 @@ namespace Microsoft.AspNetCore.SignalR.Tests
 {
     public class DefaultTransportFactoryTests
     {
+        private const HttpTransportType AllTransportTypes = HttpTransportType.WebSockets | HttpTransportType.ServerSentEvents | HttpTransportType.LongPolling;
+
         [Theory]
-        [InlineData((HttpTransportType)0)]
+        [InlineData(HttpTransportType.None)]
-        [InlineData(HttpTransportType.All + 1)]
+        [InlineData((HttpTransportType)int.MaxValue)]
-        public void DefaultTransportFactoryCannotBeCreatedWithInvalidTransportType(HttpTransportType transportType)
+        public void DefaultTransportFactoryCanBeCreatedWithNoOrUnknownTransportTypeFlags(HttpTransportType transportType)
         {
-            Assert.Throws<ArgumentOutOfRangeException>(
+            Assert.NotNull(new DefaultTransportFactory(transportType, new LoggerFactory(), new HttpClient(), httpOptions: null));
-                () => new DefaultTransportFactory(transportType, new LoggerFactory(), new HttpClient(), httpOptions: null));
         }
 
         [Theory]
-        [InlineData(HttpTransportType.All)]
+        [InlineData(AllTransportTypes)]
         [InlineData(HttpTransportType.LongPolling)]
         [InlineData(HttpTransportType.ServerSentEvents)]
         [InlineData(HttpTransportType.LongPolling | HttpTransportType.WebSockets)]
@@ -52,14 +53,14 @@ namespace Microsoft.AspNetCore.SignalR.Tests
         {
             var transportFactory = new DefaultTransportFactory(requestedTransport, loggerFactory: null, httpClient: new HttpClient(), httpOptions: null);
             Assert.IsType(expectedTransportType,
-                transportFactory.CreateTransport(HttpTransportType.All));
+                transportFactory.CreateTransport(AllTransportTypes));
         }
 
         [Theory]
         [InlineData(HttpTransportType.WebSockets)]
         [InlineData(HttpTransportType.ServerSentEvents)]
         [InlineData(HttpTransportType.LongPolling)]
-        [InlineData(HttpTransportType.All)]
+        [InlineData(AllTransportTypes)]
         public void DefaultTransportFactoryThrowsIfItCannotCreateRequestedTransport(HttpTransportType requestedTransport)
         {
             var transportFactory =
@@ -75,12 +76,12 @@ namespace Microsoft.AspNetCore.SignalR.Tests
         public void DefaultTransportFactoryCreatesWebSocketsTransportIfAvailable()
         {
             Assert.IsType<WebSocketsTransport>(
-                new DefaultTransportFactory(HttpTransportType.All, loggerFactory: null, httpClient: new HttpClient(), httpOptions: null)
+                new DefaultTransportFactory(AllTransportTypes, loggerFactory: null, httpClient: new HttpClient(), httpOptions: null)
-                    .CreateTransport(HttpTransportType.All));
+                    .CreateTransport(AllTransportTypes));
         }
 
         [Theory]
-        [InlineData(HttpTransportType.All, typeof(ServerSentEventsTransport))]
+        [InlineData(AllTransportTypes, typeof(ServerSentEventsTransport))]
         [InlineData(HttpTransportType.ServerSentEvents, typeof(ServerSentEventsTransport))]
         [InlineData(HttpTransportType.LongPolling, typeof(LongPollingTransport))]
         public void DefaultTransportFactoryCreatesRequestedTransportIfAvailable_Win7(HttpTransportType requestedTransport, Type expectedTransportType)
@@ -89,7 +90,7 @@ namespace Microsoft.AspNetCore.SignalR.Tests
             {
                 var transportFactory = new DefaultTransportFactory(requestedTransport, loggerFactory: null, httpClient: new HttpClient(), httpOptions: null);
                 Assert.IsType(expectedTransportType,
-                    transportFactory.CreateTransport(HttpTransportType.All));
+                    transportFactory.CreateTransport(AllTransportTypes));
             }
         }
 
@@ -102,7 +103,7 @@ namespace Microsoft.AspNetCore.SignalR.Tests
                 var transportFactory =
                     new DefaultTransportFactory(requestedTransport, loggerFactory: null, httpClient: new HttpClient(), httpOptions: null);
                 var ex = Assert.Throws<InvalidOperationException>(
-                    () => transportFactory.CreateTransport(HttpTransportType.All));
+                    () => transportFactory.CreateTransport(AllTransportTypes));
 
                 Assert.Equal("No requested transports available on the server.", ex.Message);
             }