DEV-266 - Return null for discovery mode or multiple hosts in connection string (#280)

w1am · web-flow · commit a0c283b20ca0 · 2024-02-26T13:41:17.000Z
* Refactor address resolution logic in connectivity settings

* Fix test

* Make DefaultAddress private
diff --git a/src/EventStore.Client/EventStoreClientConnectivitySettings.cs b/src/EventStore.Client/EventStoreClientConnectivitySettings.cs
@@ -6,27 +6,26 @@ namespace EventStore.Client {
 	/// A class used to describe how to connect to an instance of EventStoreDB.
 	/// </summary>
 	public class EventStoreClientConnectivitySettings {
-		private const int DefaultPort = 2113;
-		private bool _insecure;
-		private Uri? _address;
-		
+		private const int  DefaultPort = 2113;
+		private       bool _insecure;
+		private       Uri? _address;
+
 		/// <summary>
 		/// The <see cref="Uri"/> of the EventStoreDB. Use this when connecting to a single node.
 		/// </summary>
-		public Uri Address {
-			get { return _address != null && IsSingleNode ? _address : DefaultAddress; }
-			set { _address = value; }
+		public Uri? Address {
+			get => IsSingleNode ? _address : null;
+			set => _address = value;
 		}
 
-		private Uri DefaultAddress {
-			get {
-				return new UriBuilder {
-						Scheme = _insecure ? Uri.UriSchemeHttp : Uri.UriSchemeHttps,
-						Port = DefaultPort
-					}.Uri;
-			}
-		}
-		
+		internal Uri ResolvedAddressOrDefault => Address ?? DefaultAddress;
+
+		private Uri DefaultAddress =>
+			new UriBuilder {
+				Scheme = _insecure ? Uri.UriSchemeHttp : Uri.UriSchemeHttps,
+				Port   = DefaultPort
+			}.Uri;
+
 		/// <summary>
 		/// The maximum number of times to attempt <see cref="EndPoint"/> discovery.
 		/// </summary>
@@ -38,8 +37,8 @@ private Uri DefaultAddress {
 		public EndPoint[] GossipSeeds =>
 			((object?)DnsGossipSeeds ?? IpGossipSeeds) switch {
 				DnsEndPoint[] dns => Array.ConvertAll<DnsEndPoint, EndPoint>(dns, x => x),
-				IPEndPoint[] ip => Array.ConvertAll<IPEndPoint, EndPoint>(ip, x => x),
-				_ => Array.Empty<EndPoint>()
+				IPEndPoint[] ip   => Array.ConvertAll<IPEndPoint, EndPoint>(ip, x => x),
+				_                 => Array.Empty<EndPoint>()
 			};
 
 		/// <summary>
@@ -87,37 +86,31 @@ private Uri DefaultAddress {
 		/// True if pointing to a single EventStoreDB node.
 		/// </summary>
 		public bool IsSingleNode => GossipSeeds.Length == 0;
-		
+
 		/// <summary>
 		/// True if communicating over an insecure channel; otherwise false.
 		/// </summary>
 		public bool Insecure {
-			get {
-				return IsSingleNode
-					? string.Equals(Address.Scheme, Uri.UriSchemeHttp, StringComparison.OrdinalIgnoreCase)
-					: _insecure;
-			}
-			set {
-				_insecure = value;
-			}
+			get => IsSingleNode ? string.Equals(Address?.Scheme, Uri.UriSchemeHttp, StringComparison.OrdinalIgnoreCase) : _insecure;
+			set => _insecure = value;
 		}
 
 		/// <summary>
-		/// True if certificates will be validated; otherwise false. 
+		/// True if certificates will be validated; otherwise false.
 		/// </summary>
 		public bool TlsVerifyCert { get; set; } = true;
-		
+
 		/// <summary>
 		/// The default <see cref="EventStoreClientConnectivitySettings"/>.
 		/// </summary>
 		public static EventStoreClientConnectivitySettings Default => new EventStoreClientConnectivitySettings {
 			MaxDiscoverAttempts = 10,
-			GossipTimeout = TimeSpan.FromSeconds(5),
-			DiscoveryInterval = TimeSpan.FromMilliseconds(100),
-			NodePreference = NodePreference.Leader,
-			KeepAliveInterval = TimeSpan.FromSeconds(10),
-			KeepAliveTimeout =  TimeSpan.FromSeconds(10),
-			TlsVerifyCert = true,
+			GossipTimeout       = TimeSpan.FromSeconds(5),
+			DiscoveryInterval   = TimeSpan.FromMilliseconds(100),
+			NodePreference      = NodePreference.Leader,
+			KeepAliveInterval   = TimeSpan.FromSeconds(10),
+			KeepAliveTimeout    = TimeSpan.FromSeconds(10),
+			TlsVerifyCert       = true,
 		};
 	}
 }
diff --git a/src/EventStore.Client/EventStoreClientSettings.ConnectionString.cs b/src/EventStore.Client/EventStoreClientSettings.ConnectionString.cs
@@ -40,7 +40,7 @@ private static class ConnectionStringParser {
 			private const string UriSchemeDiscover = "esdb+discover";
 
 			private static readonly string[] Schemes = {"esdb", UriSchemeDiscover};
-			private static readonly int DefaultPort = EventStoreClientConnectivitySettings.Default.Address.Port;
+			private static readonly int DefaultPort = EventStoreClientConnectivitySettings.Default.ResolvedAddressOrDefault.Port;
 			private static readonly bool DefaultUseTls = true;
 
 			private static readonly Dictionary<string, Type> SettingsType =
diff --git a/src/EventStore.Client/HttpFallback.cs b/src/EventStore.Client/HttpFallback.cs
@@ -13,7 +13,7 @@ internal class HttpFallback : IDisposable {
 		private readonly string _addressScheme;
 
 		internal HttpFallback (EventStoreClientSettings settings) {
-			_addressScheme = settings.ConnectivitySettings.Address.Scheme;
+			_addressScheme = settings.ConnectivitySettings.ResolvedAddressOrDefault.Scheme;
             _defaultCredentials = settings.DefaultCredentials;
 			
 			var handler = new HttpClientHandler();
diff --git a/src/EventStore.Client/SingleNodeChannelSelector.cs b/src/EventStore.Client/SingleNodeChannelSelector.cs
@@ -20,7 +20,7 @@ public SingleNodeChannelSelector(
 
 			_channelCache = channelCache;
 			
-			var uri = settings.ConnectivitySettings.Address;
+			var uri = settings.ConnectivitySettings.ResolvedAddressOrDefault;
 			_endPoint = new DnsEndPoint(host: uri.Host, port: uri.Port);
 		}
 
diff --git a/src/EventStore.Client/SingleNodeHttpHandler.cs b/src/EventStore.Client/SingleNodeHttpHandler.cs
@@ -14,7 +14,7 @@ public SingleNodeHttpHandler(EventStoreClientSettings settings) {
 		protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request,
 			CancellationToken cancellationToken) {
 			request.RequestUri = new UriBuilder(request.RequestUri!) {
-				Scheme = _settings.ConnectivitySettings.Address.Scheme
+				Scheme = _settings.ConnectivitySettings.ResolvedAddressOrDefault.Scheme
 			}.Uri;
 			return base.SendAsync(request, cancellationToken);
 		}
diff --git a/test/EventStore.Client.Tests.Common/Fixtures/Base/EventStoreClientFixtureBase.cs b/test/EventStore.Client.Tests.Common/Fixtures/Base/EventStoreClientFixtureBase.cs
@@ -55,8 +55,8 @@ protected EventStoreClientFixtureBase(
 			TestServer = new EventStoreTestServerExternal();
 		else
 			TestServer = GlobalEnvironment.UseCluster
-				? new EventStoreTestServerCluster(hostCertificatePath, Settings.ConnectivitySettings.Address, env)
-				: new EventStoreTestServer(hostCertificatePath, Settings.ConnectivitySettings.Address, env);
+				? new EventStoreTestServerCluster(hostCertificatePath, Settings.ConnectivitySettings.ResolvedAddressOrDefault, env)
+				: new EventStoreTestServer(hostCertificatePath, Settings.ConnectivitySettings.ResolvedAddressOrDefault, env);
 	}
 
 	public    IEventStoreTestServer    TestServer { get; }
diff --git a/test/EventStore.Client.Tests.Common/Fixtures/EventStoreTestNode.cs b/test/EventStore.Client.Tests.Common/Fixtures/EventStoreTestNode.cs
@@ -57,7 +57,7 @@ public static EventStoreFixtureOptions DefaultOptions() {
 	protected override ContainerBuilder Configure() {
 		var env = Options.Environment.Select(pair => $"{pair.Key}={pair.Value}").ToArray();
 
-		var port      = Options.ClientSettings.ConnectivitySettings.Address.Port;
+		var port      = Options.ClientSettings.ConnectivitySettings.ResolvedAddressOrDefault.Port;
 		var certsPath = Path.Combine(Environment.CurrentDirectory, "certs");
 
 		var containerName = port == 2113
diff --git a/test/EventStore.Client.Tests/ConnectionStringTests.cs b/test/EventStore.Client.Tests/ConnectionStringTests.cs
@@ -27,8 +27,8 @@ public class ConnectionStringTests {
 			};
 
 			settings.ConnectivitySettings.Address =
-				new UriBuilder(EventStoreClientConnectivitySettings.Default.Address) {
-					Scheme = settings.ConnectivitySettings.Address.Scheme
+				new UriBuilder(EventStoreClientConnectivitySettings.Default.ResolvedAddressOrDefault) {
+					Scheme = settings.ConnectivitySettings.ResolvedAddressOrDefault.Scheme
 				}.Uri;
 
 			yield return new object?[] {
@@ -48,8 +48,8 @@ public class ConnectionStringTests {
 			};
 
 			ipGossipSettings.ConnectivitySettings.Address =
-				new UriBuilder(EventStoreClientConnectivitySettings.Default.Address) {
-					Scheme = ipGossipSettings.ConnectivitySettings.Address.Scheme
+				new UriBuilder(EventStoreClientConnectivitySettings.Default.ResolvedAddressOrDefault) {
+					Scheme = ipGossipSettings.ConnectivitySettings.ResolvedAddressOrDefault.Scheme
 				}.Uri;
 
 			ipGossipSettings.ConnectivitySettings.DnsGossipSeeds = null;
@@ -73,7 +73,7 @@ public class ConnectionStringTests {
 			singleNodeSettings.ConnectivitySettings.DnsGossipSeeds = null;
 			singleNodeSettings.ConnectivitySettings.IpGossipSeeds  = null;
 			singleNodeSettings.ConnectivitySettings.Address = new UriBuilder(fixture.Create<Uri>()) {
-				Scheme = singleNodeSettings.ConnectivitySettings.Address.Scheme
+				Scheme = singleNodeSettings.ConnectivitySettings.ResolvedAddressOrDefault.Scheme
 			}.Uri;
 
 			yield return new object?[] {
@@ -115,7 +115,7 @@ public void tls_verify_cert(bool tlsVerifyCert) {
 		var       result           = EventStoreClientSettings.Create(connectionString);
 		using var handler          = result.CreateHttpMessageHandler?.Invoke();
 #if NET
-		var       socketsHandler   = Assert.IsType<SocketsHttpHandler>(handler);
+		var socketsHandler = Assert.IsType<SocketsHttpHandler>(handler);
 		if (!tlsVerifyCert) {
 			Assert.NotNull(socketsHandler.SslOptions.RemoteCertificateValidationCallback);
 			Assert.True(
@@ -241,8 +241,8 @@ public void with_default_settings() {
 
 		Assert.Null(settings.ConnectionName);
 		Assert.Equal(
-			EventStoreClientConnectivitySettings.Default.Address.Scheme,
-			settings.ConnectivitySettings.Address.Scheme
+			EventStoreClientConnectivitySettings.Default.ResolvedAddressOrDefault.Scheme,
+			settings.ConnectivitySettings.ResolvedAddressOrDefault.Scheme
 		);
 
 		Assert.Equal(
@@ -301,7 +301,7 @@ public void use_tls(string connectionString, bool expectedUseTls) {
 		var result         = EventStoreClientSettings.Create(connectionString);
 		var expectedScheme = expectedUseTls ? "https" : "http";
 		Assert.NotEqual(expectedUseTls, result.ConnectivitySettings.Insecure);
-		Assert.Equal(expectedScheme, result.ConnectivitySettings.Address.Scheme);
+		Assert.Equal(expectedScheme, result.ConnectivitySettings.ResolvedAddressOrDefault.Scheme);
 	}
 
 	[Theory]
@@ -334,7 +334,20 @@ public void allow_tls_override_for_single_node(string connectionString, bool? in
 
 		var expectedScheme = expectedUseTls ? "https" : "http";
 		Assert.Equal(expectedUseTls, !settings.Insecure);
-		Assert.Equal(expectedScheme, result.ConnectivitySettings.Address.Scheme);
+		Assert.Equal(expectedScheme, result.ConnectivitySettings.ResolvedAddressOrDefault.Scheme);
+	}
+
+	[Theory]
+	[InlineData("esdb://localhost:1234", "localhost", 1234)]
+	[InlineData("esdb://localhost:1234,localhost:4567", null, null)]
+	[InlineData("esdb+discover://localhost:1234", null, null)]
+	[InlineData("esdb+discover://localhost:1234,localhost:4567", null, null)]
+	public void connection_string_with_custom_ports(string connectionString, string? expectedHost, int? expectedPort) {
+		var result               = EventStoreClientSettings.Create(connectionString);
+		var connectivitySettings = result.ConnectivitySettings;
+
+		Assert.Equal(expectedHost, connectivitySettings.Address?.Host);
+		Assert.Equal(expectedPort, connectivitySettings.Address?.Port);
 	}
 
 	static string GetConnectionString(
@@ -350,7 +363,7 @@ static string GetScheme(EventStoreClientSettings settings) =>
 
 	static string GetAuthority(EventStoreClientSettings settings) =>
 		settings.ConnectivitySettings.IsSingleNode
-			? $"{settings.ConnectivitySettings.Address.Host}:{settings.ConnectivitySettings.Address.Port}"
+			? $"{settings.ConnectivitySettings.ResolvedAddressOrDefault.Host}:{settings.ConnectivitySettings.ResolvedAddressOrDefault.Port}"
 			: string.Join(
 				",",
 				settings.ConnectivitySettings.GossipSeeds.Select(x => $"{x.GetHost()}:{x.GetPort()}")
@@ -447,7 +460,7 @@ public bool Equals(EventStoreClientConnectivitySettings? x, EventStoreClientConn
 			if (x.GetType() != y.GetType())
 				return false;
 
-			return (!x.IsSingleNode || x.Address.Equals(y.Address)) &&
+			return (!x.IsSingleNode || x.ResolvedAddressOrDefault.Equals(y.Address)) &&
 			       x.MaxDiscoverAttempts == y.MaxDiscoverAttempts &&
 			       x.GossipSeeds.SequenceEqual(y.GossipSeeds) &&
 			       x.GossipTimeout.Equals(y.GossipTimeout) &&
@@ -461,7 +474,7 @@ public bool Equals(EventStoreClientConnectivitySettings? x, EventStoreClientConn
 		public int GetHashCode(EventStoreClientConnectivitySettings obj) =>
 			obj.GossipSeeds.Aggregate(
 				HashCode.Hash
-					.Combine(obj.Address.GetHashCode())
+					.Combine(obj.ResolvedAddressOrDefault.GetHashCode())
 					.Combine(obj.MaxDiscoverAttempts)
 					.Combine(obj.GossipTimeout)
 					.Combine(obj.DiscoveryInterval)

Original file line number	Diff line number	Diff line change
`@@ -20,7 +20,7 @@ public SingleNodeChannelSelector(`
`20`	`20`
`21`	`21`	`_channelCache = channelCache;`
`22`	`22`
`23`		`- var uri = settings.ConnectivitySettings.Address;`
	`23`	`+ var uri = settings.ConnectivitySettings.ResolvedAddressOrDefault;`
`24`	`24`	`_endPoint = new DnsEndPoint(host: uri.Host, port: uri.Port);`
`25`	`25`	`}`
`26`	`26`
Original file line number	Diff line number	Diff line change
`@@ -14,7 +14,7 @@ public SingleNodeHttpHandler(EventStoreClientSettings settings) {`
`14`	`14`	`protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request,`
`15`	`15`	`CancellationToken cancellationToken) {`
`16`	`16`	`request.RequestUri = new UriBuilder(request.RequestUri!) {`
`17`		`- Scheme = _settings.ConnectivitySettings.Address.Scheme`
	`17`	`+ Scheme = _settings.ConnectivitySettings.ResolvedAddressOrDefault.Scheme`
`18`	`18`	`}.Uri;`
`19`	`19`	`return base.SendAsync(request, cancellationToken);`
`20`	`20`	`}`