Finalizing 1.3.0 API changes and cleanup (#65)

* Making name server collection in `DnsQueryAndServerOptions` readonly
* Changing the way Options and Settings work slightly. Resolve name servers now happens in LookupClient's ctor instead of the options. See #33 
   * New property on `AutoResolveNameServers` which defaults to `True` if no name servers are passed in. Goes to `False` if there are (but can be set to true again...
* IDnsQuery API's with options simplified and I went back from using optional parameters, there are 2 overloads now instead, but not for all variants.

Author: MichaCo

DnsClientDotNet.sln

@@ -22,11 +22,11 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "MiniDig", "samples\MiniDig\
 EndProject
 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "DnsClient.Tests", "test\DnsClient.Tests\DnsClient.Tests.csproj", "{01902674-53A3-4E81-B08F-CFAA7CA62F56}"
 EndProject
-Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "ApiDesign", "test\ApiDesign\ApiDesign.csproj", "{2F789CAA-31E5-4394-96B9-1B06A1AA5EA0}"
+Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "ApiDesign", "test-other\ApiDesign\ApiDesign.csproj", "{2F789CAA-31E5-4394-96B9-1B06A1AA5EA0}"
 EndProject
 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "DnsClient", "src\DnsClient\DnsClient.csproj", "{493E6161-7F71-4A59-AB28-B4A5E45A5E27}"
 EndProject
-Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Benchmarks", "test\Benchmarks\Benchmarks.csproj", "{7461E2C7-008E-4E61-ABC3-AF85DE37084B}"
+Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Benchmarks", "test-other\Benchmarks\Benchmarks.csproj", "{7461E2C7-008E-4E61-ABC3-AF85DE37084B}"
 EndProject
 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Documentation", "Documentation", "{5352F4A6-D88E-43E8-B0D1-E90CBB162613}"
 	ProjectSection(SolutionItems) = preProject
@@ -37,11 +37,13 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Documentation", "Documentat
 		Documentation\toc.yml = Documentation\toc.yml
 	EndProjectSection
 EndProject
-Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "DnsClient.TestsCommon", "test\DnsClient.TestsCommon\DnsClient.TestsCommon.csproj", "{8422A76D-CC40-4B4B-AA03-C17C3D86A073}"
+Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "DnsClient.TestsCommon", "test-other\DnsClient.TestsCommon\DnsClient.TestsCommon.csproj", "{8422A76D-CC40-4B4B-AA03-C17C3D86A073}"
 EndProject
-Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "DnsClient.PerfTestHost", "test\DnsClient.PerfTestHost\DnsClient.PerfTestHost.csproj", "{58173158-2B2D-4C26-A666-495641477066}"
+Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "DnsClient.PerfTestHost", "test-other\DnsClient.PerfTestHost\DnsClient.PerfTestHost.csproj", "{58173158-2B2D-4C26-A666-495641477066}"
 EndProject
-Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "ApiDesign.OldReference", "test\ApiDesign.OldReference\ApiDesign.OldReference.csproj", "{77554E92-95AB-404F-8999-A82BE4978361}"
+Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "ApiDesign.OldReference", "test-other\ApiDesign.OldReference\ApiDesign.OldReference.csproj", "{77554E92-95AB-404F-8999-A82BE4978361}"
+EndProject
+Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "test-other", "test-other", "{28CFF2BF-88D0-481F-829E-43B51DE7FB1F}"
 EndProject
 Global
 	GlobalSection(SolutionConfigurationPlatforms) = preSolution
@@ -88,12 +90,12 @@ Global
 	GlobalSection(NestedProjects) = preSolution
 		{CEC9D7AD-8000-4FC6-8EB3-AC1417B9808A} = {A1BE93CF-0106-495B-9720-008046657391}
 		{01902674-53A3-4E81-B08F-CFAA7CA62F56} = {34E43CED-3DF4-49CE-8556-69225ABD84E6}
-		{2F789CAA-31E5-4394-96B9-1B06A1AA5EA0} = {34E43CED-3DF4-49CE-8556-69225ABD84E6}
+		{2F789CAA-31E5-4394-96B9-1B06A1AA5EA0} = {28CFF2BF-88D0-481F-829E-43B51DE7FB1F}
 		{493E6161-7F71-4A59-AB28-B4A5E45A5E27} = {3D3BE1AD-F551-420A-A81D-4CB88286C8CD}
-		{7461E2C7-008E-4E61-ABC3-AF85DE37084B} = {34E43CED-3DF4-49CE-8556-69225ABD84E6}
-		{8422A76D-CC40-4B4B-AA03-C17C3D86A073} = {34E43CED-3DF4-49CE-8556-69225ABD84E6}
-		{58173158-2B2D-4C26-A666-495641477066} = {34E43CED-3DF4-49CE-8556-69225ABD84E6}
-		{77554E92-95AB-404F-8999-A82BE4978361} = {34E43CED-3DF4-49CE-8556-69225ABD84E6}
+		{7461E2C7-008E-4E61-ABC3-AF85DE37084B} = {28CFF2BF-88D0-481F-829E-43B51DE7FB1F}
+		{8422A76D-CC40-4B4B-AA03-C17C3D86A073} = {28CFF2BF-88D0-481F-829E-43B51DE7FB1F}
+		{58173158-2B2D-4C26-A666-495641477066} = {28CFF2BF-88D0-481F-829E-43B51DE7FB1F}
+		{77554E92-95AB-404F-8999-A82BE4978361} = {28CFF2BF-88D0-481F-829E-43B51DE7FB1F}
 	EndGlobalSection
 	GlobalSection(ExtensibilityGlobals) = postSolution
 		SolutionGuid = {E26DC1C5-A82B-4984-88DB-107C9D66271F}

samples/MiniDig/DnsCommand.cs

@@ -87,9 +87,8 @@ public NameServer[] GetEndpointsValue()
 
         public LookupClientOptions GetLookupSettings()
         {
-            return new LookupClientOptions()
+            return new LookupClientOptions(GetEndpointsValue())
             {
-                NameServers = GetEndpointsValue(),
                 Recursion = GetUseRecursionValue(),
                 Retries = GetTriesValue(),
                 Timeout = TimeSpan.FromMilliseconds(GetTimeoutValue()),

samples/MiniDig/Program.cs

@@ -9,7 +9,7 @@ public class Program
     {
         public static async Task Main(string[] args)
         {
-            DnsClient.Tracing.Source.Switch.Level = SourceLevels.Verbose;
+            DnsClient.Tracing.Source.Switch.Level = SourceLevels.Information;
             DnsClient.Tracing.Source.Listeners.Add(new ConsoleTraceListener());
 
             var app = new CommandLineApplication(throwOnUnexpectedArg: true);

src/DnsClient/DnsQueryOptions.cs

@@ -129,10 +129,10 @@ public class DnsQueryOptions
         /// Default is <c>True</c>.
         /// </summary>
         /// <remarks>
-        /// The query is answered if there is at least one <see cref="DnsResourceRecord"/> in the answers section 
+        /// The query is answered if there is at least one <see cref="DnsResourceRecord"/> in the answers section
         /// matching the <see cref="DnsQuestion"/>'s <see cref="QueryType"/>.
         /// <para>
-        /// If there are zero answeres in the response, the query is not answered, independent of the <see cref="QueryType"/>.
+        /// If there are zero answers in the response, the query is not answered, independent of the <see cref="QueryType"/>.
         /// If there are answers in the response, the <see cref="QueryType"/> is used to find a matching record,
         /// query types <see cref="QueryType.ANY"/> and <see cref="QueryType.AXFR"/> will be ignored by this check.
         /// </para>
@@ -241,39 +241,38 @@ public static implicit operator DnsQuerySettings(DnsQueryOptions fromOptions)
     /// </summary>
     public class DnsQueryAndServerOptions : DnsQueryOptions
     {
-        // TODO: evalualte if defaulting resolveNameServers to false here and in the query plan, fall back to configured nameservers of the client
         /// <summary>
         /// Creates a new instance of <see cref="DnsQueryAndServerOptions"/> without name servers.
-        /// </summary>
-        /// <remarks>
         /// If no nameservers are configured, a query will fallback to the nameservers already configured on the <see cref="LookupClient"/> instance.
-        /// </remarks>
-        /// <param name="resolveNameServers">If set to <c>true</c>, <see cref="NameServer.ResolveNameServers(bool, bool)"/>
-        /// will be used to get a list of nameservers.</param>
-        public DnsQueryAndServerOptions(bool resolveNameServers = false)
-            : this(resolveNameServers ? NameServer.ResolveNameServers()?.ToArray() : null)
+        /// </summary>
+        public DnsQueryAndServerOptions()
         {
-            AutoResolvedNameServers = resolveNameServers;
         }
 
         /// <summary>
         /// Creates a new instance of <see cref="DnsQueryAndServerOptions"/>.
         /// </summary>
         /// <param name="nameServers">A collection of name servers.</param>
+        /// <exception cref="ArgumentNullException">If <paramref name="nameServers"/> is null.</exception>
         public DnsQueryAndServerOptions(params NameServer[] nameServers)
             : base()
         {
             if (nameServers != null && nameServers.Length > 0)
             {
                 NameServers = nameServers.ToList();
             }
+            else
+            {
+                throw new ArgumentNullException(nameof(nameServers));
+            }
         }
 
         // TODO: remove overloads in favor of implicit conversion to NameServer?
         /// <summary>
         /// Creates a new instance of <see cref="DnsQueryAndServerOptions"/>.
         /// </summary>
         /// <param name="nameServers">A collection of name servers.</param>
+        /// <exception cref="ArgumentNullException">If <paramref name="nameServers"/> is null.</exception>
         public DnsQueryAndServerOptions(params IPEndPoint[] nameServers)
            : this(nameServers?.Select(p => (NameServer)p).ToArray())
         {
@@ -283,20 +282,16 @@ public DnsQueryAndServerOptions(params IPEndPoint[] nameServers)
         /// Creates a new instance of <see cref="DnsQueryAndServerOptions"/>.
         /// </summary>
         /// <param name="nameServers">A collection of name servers.</param>
+        /// <exception cref="ArgumentNullException">If <paramref name="nameServers"/> is null.</exception>
         public DnsQueryAndServerOptions(params IPAddress[] nameServers)
             : this(nameServers?.Select(p => (NameServer)p).ToArray())
         {
         }
 
         /// <summary>
-        /// Gets a flag indicating whether the name server collection was manually defined or automatically resolved
-        /// </summary>
-        public bool AutoResolvedNameServers { get; }
-
-        /// <summary>
-        /// Gets or sets a list of name servers which should be used to query.
+        /// Gets a list of name servers which should be used to query.
         /// </summary>
-        public IList<NameServer> NameServers { get; set; } = new List<NameServer>();
+        public IReadOnlyList<NameServer> NameServers { get; } = new NameServer[0];
 
         /// <summary>
         /// Converts the query options into readonly settings.
@@ -327,40 +322,57 @@ public class LookupClientOptions : DnsQueryAndServerOptions
         private TimeSpan? _maximumCacheTimeout;
 
         /// <summary>
-        /// Creates a new instance of <see cref="LookupClientOptions"/> without name servers.
+        /// Creates a new instance of <see cref="LookupClientOptions"/> with default settings.
         /// </summary>
-        public LookupClientOptions(bool resolveNameServers = true)
-            : base(resolveNameServers)
+        public LookupClientOptions()
+            : base()
         {
         }
 
         /// <summary>
         /// Creates a new instance of <see cref="LookupClientOptions"/>.
         /// </summary>
         /// <param name="nameServers">A collection of name servers.</param>
+        /// <exception cref="ArgumentNullException">If <paramref name="nameServers"/> is null.</exception>
         public LookupClientOptions(params NameServer[] nameServers)
             : base(nameServers)
         {
+            AutoResolveNameServers = false;
         }
 
         /// <summary>
         /// Creates a new instance of <see cref="LookupClientOptions"/>.
         /// </summary>
         /// <param name="nameServers">A collection of name servers.</param>
+        /// <exception cref="ArgumentNullException">If <paramref name="nameServers"/> is null.</exception>
         public LookupClientOptions(params IPEndPoint[] nameServers)
             : base(nameServers)
         {
+            AutoResolveNameServers = false;
         }
 
         /// <summary>
         /// Creates a new instance of <see cref="LookupClientOptions"/>.
         /// </summary>
         /// <param name="nameServers">A collection of name servers.</param>
+        /// <exception cref="ArgumentNullException">If <paramref name="nameServers"/> is null.</exception>
         public LookupClientOptions(params IPAddress[] nameServers)
             : base(nameServers)
         {
+            AutoResolveNameServers = false;
         }
 
+        /// <summary>
+        /// Gets or sets a flag indicating whether the name server collection should be automatically resolved.
+        /// Default is <c>True</c>.
+        /// </summary>
+        /// <remarks>
+        /// If name servers are configured manually via the constructor, this flag is set to false.
+        /// If you want both, your manually configured servers and auto resolved name servers,
+        /// you can use both (ctor or) <see cref="DnsQueryAndServerOptions.NameServers"/> and <see cref="AutoResolveNameServers"/> set to <c>True</c>.
+        /// </remarks>
+        public bool AutoResolveNameServers { get; set; } = true;
+
         /// <summary>
         /// Gets or sets a <see cref="TimeSpan"/> which can override the TTL of a resource record in case the
         /// TTL of the record is lower than this minimum value.
@@ -427,20 +439,6 @@ public TimeSpan? MaximumCacheTimeout
                 }
             }
         }
-
-        /// <summary>
-        /// Converts the options into readonly settings.
-        /// </summary>
-        /// <param name="fromOptions">The options.</param>
-        public static implicit operator LookupClientSettings(LookupClientOptions fromOptions)
-        {
-            if (fromOptions == null)
-            {
-                return null;
-            }
-
-            return new LookupClientSettings(fromOptions);
-        }
     }
 
 #pragma warning disable CS0659 // Type overrides Object.Equals(object o) but does not override Object.GetHashCode() - intentionally, Equals is only used by unit tests
@@ -551,10 +549,10 @@ public class DnsQuerySettings : IEquatable<DnsQuerySettings>
         /// Default is <c>True</c>.
         /// </summary>
         /// <remarks>
-        /// The query is answered if there is at least one <see cref="DnsResourceRecord"/> in the answers section 
+        /// The query is answered if there is at least one <see cref="DnsResourceRecord"/> in the answers section
         /// matching the <see cref="DnsQuestion"/>'s <see cref="QueryType"/>.
         /// <para>
-        /// If there are zero answeres in the response, the query is not answered, independent of the <see cref="QueryType"/>.
+        /// If there are zero answers in the response, the query is not answered, independent of the <see cref="QueryType"/>.
         /// If there are answers in the response, the <see cref="QueryType"/> is used to find a matching record,
         /// query types <see cref="QueryType.ANY"/> and <see cref="QueryType.AXFR"/> will be ignored by this check.
         /// </para>
@@ -638,7 +636,7 @@ public DnsQuerySettings(DnsQueryOptions options)
             UseTcpFallback = options.UseTcpFallback;
             UseTcpOnly = options.UseTcpOnly;
             ExtendedDnsBufferSize = options.ExtendedDnsBufferSize;
-            RequestDnsSecRecords = options.RequestDnsSecRecords;            
+            RequestDnsSecRecords = options.RequestDnsSecRecords;
         }
 
         /// <inheritdocs />
@@ -699,10 +697,6 @@ public class DnsQueryAndServerSettings : DnsQuerySettings, IEquatable<DnsQueryAn
         /// </summary>
         public IReadOnlyList<NameServer> NameServers => _endpoints;
 
-        /// <summary>
-        /// Gets a flag indicating whether the name server collection was manually defined or automatically resolved
-        /// </summary>
-        public bool AutoResolvedNameServers { get; }
 
         /// <summary>
         /// Creates a new instance of <see cref="DnsQueryAndServerSettings"/>.
@@ -711,8 +705,6 @@ public DnsQueryAndServerSettings(DnsQueryAndServerOptions options)
             : base(options)
         {
             _endpoints = options.NameServers?.ToArray() ?? new NameServer[0];
-
-            AutoResolvedNameServers = options.AutoResolvedNameServers;
         }
 
         /// <summary>
@@ -754,7 +746,6 @@ public bool Equals(DnsQueryAndServerSettings other)
             }
 
             return NameServers.SequenceEqual(other.NameServers)
-                   && AutoResolvedNameServers == other.AutoResolvedNameServers
                    && base.Equals(other);
         }
 
@@ -794,6 +785,13 @@ public LookupClientSettings(LookupClientOptions options)
             MaximumCacheTimeout = options.MaximumCacheTimeout;
         }
 
+        internal LookupClientSettings(LookupClientOptions options, IReadOnlyCollection<NameServer> overrideServers)
+            : base(options, overrideServers)
+        {
+            MinimumCacheTimeout = options.MinimumCacheTimeout;
+            MaximumCacheTimeout = options.MaximumCacheTimeout;
+        }
+
         /// <summary>
         /// Gets a <see cref="TimeSpan"/> which can override the TTL of a resource record in case the
         /// TTL of the record is lower than this minimum value.
@@ -870,7 +868,7 @@ internal LookupClientSettings Copy(
             bool? useTcpOnly = null)
         {
             // auto resolved flag might get lost here. But this stuff gets deleted anyways.
-            return new LookupClientOptions(nameServers?.ToArray())
+            return new LookupClientSettings(new LookupClientOptions(nameServers?.ToArray())
             {
                 MinimumCacheTimeout = minimumCacheTimeout,
                 ContinueOnDnsError = continueOnDnsError ?? ContinueOnDnsError,
@@ -887,7 +885,7 @@ internal LookupClientSettings Copy(
                 ExtendedDnsBufferSize = ExtendedDnsBufferSize,
                 RequestDnsSecRecords = RequestDnsSecRecords,
                 ContinueOnEmptyResponse = ContinueOnEmptyResponse
-            };
+            });
         }
 
         // TODO: remove if LookupClient settings can be made readonly