Relax ctor selection rules and improve the error message

sdk/extensions/Microsoft.Extensions.Azure/CHANGELOG.md

@@ -10,6 +10,8 @@
 
 ### Fixed
 
+- Improved the diagnostic message when a constructor can't be selected.
+- The issue where aggressive parameter validation caused constructor not to be selected.
 
 ## 1.1.0 (2021-06-09)
 

sdk/extensions/Microsoft.Extensions.Azure/src/Internal/ClientFactory.cs

@@ -80,7 +80,7 @@ public static object CreateClient(Type clientType, Type optionsType, object opti
                 return constructor.Invoke(arguments.ToArray());
             }
 
-            throw new InvalidOperationException(BuildErrorMessage(clientType, optionsType));
+            throw new InvalidOperationException(BuildErrorMessage(configuration, clientType, optionsType));
         }
 
         internal static TokenCredential CreateCredential(IConfiguration configuration, TokenCredentialOptions identityClientOptions = null)
@@ -217,10 +217,20 @@ private static bool IsOptionsParameter(ParameterInfo parameter, Type optionsType
                    parameter.Position == ((ConstructorInfo)parameter.Member).GetParameters().Length - 1;
         }
 
-        private static string BuildErrorMessage(Type clientType, Type optionsType)
+        private static string BuildErrorMessage(IConfiguration configuration, Type clientType, Type optionsType)
         {
             var builder = new StringBuilder();
-            builder.AppendLine("Unable to find matching constructor. Define one of the follow sets of configuration parameters:");
+
+            void TrimTrailingDelimiter()
+            {
+                while (builder[builder.Length - 1] is '/' or ',' or ' ')
+                {
+                    builder.Length--;
+                }
+            }
+
+            builder.Append("Unable to find matching constructor while trying to create an instance of ").Append(clientType.Name).AppendLine(".");
+            builder.AppendLine("Expected one of the follow sets of configuration parameters:");
 
             int counter = 1;
 
@@ -233,31 +243,58 @@ private static string BuildErrorMessage(Type clientType, Type optionsType)
 
                 builder.Append(counter).Append(". ");
 
-                bool first = true;
-
                 foreach (var parameter in constructor.GetParameters())
                 {
                     if (IsOptionsParameter(parameter, optionsType))
                     {
                         break;
                     }
 
-                    if (first)
+                    if (parameter.ParameterType is { IsClass: true } parameterType &&
+                        parameterType != typeof(string) &&
+                        parameterType != typeof(Uri))
                     {
-                        first = false;
+                        foreach (var parameterConstructor in GetApplicableParameterConstructors(parameterType))
+                        {
+                            foreach (var parameterConstructorParameter in parameterConstructor.GetParameters())
+                            {
+                                builder
+                                    .Append(parameter.Name)
+                                    .Append(':')
+                                    .Append(parameterConstructorParameter.Name)
+                                    .Append(", ");
+                            }
+
+                            TrimTrailingDelimiter();
+                            builder.Append('/');
+                        }
+
+                        TrimTrailingDelimiter();
                     }
                     else
                     {
-                        builder.Append(", ");
+                        builder.Append(parameter.Name);
                     }
 
-                    builder.Append(parameter.Name);
+                    builder.Append(", ");
                 }
 
+                TrimTrailingDelimiter();
+
                 builder.AppendLine();
                 counter++;
             }
 
+            builder.AppendLine();
+            builder.Append("Found the following configuration keys: ");
+
+            foreach (var child in configuration.AsEnumerable(true))
+            {
+                builder.Append(child.Key).Append(", ");
+            }
+
+            TrimTrailingDelimiter();
+
             return builder.ToString();
         }
 
@@ -282,11 +319,6 @@ private static bool TryConvertArgument(IConfiguration configuration, string para
                 return TryConvertFromString(configuration, parameterName, s => new Uri(s), out value);
             }
 
-            if (configuration[parameterName] != null)
-            {
-                throw new InvalidOperationException($"Unable to convert value '{configuration[parameterName]}' to parameter type {parameterType.FullName}");
-            }
-
             return TryCreateObject(parameterType, configuration.GetSection(parameterName), out value);
         }
 
@@ -312,7 +344,7 @@ internal static bool TryCreateObject(Type type, IConfigurationSection configurat
             }
 
             List<object> arguments = new List<object>();
-            foreach (var constructor in type.GetConstructors().OrderByDescending(c => c.GetParameters().Length))
+            foreach (var constructor in GetApplicableParameterConstructors(type))
             {
                 arguments.Clear();
 
@@ -337,7 +369,13 @@ internal static bool TryCreateObject(Type type, IConfigurationSection configurat
                 return true;
             }
 
-            throw new InvalidOperationException($"Unable to convert section '{configuration.Path}' to parameter type '{type}', unable to find matching constructor.");
+            value = null;
+            return false;
+        }
+
+        private static IOrderedEnumerable<ConstructorInfo> GetApplicableParameterConstructors(Type type)
+        {
+            return type.GetConstructors(BindingFlags.Public | BindingFlags.Instance).OrderByDescending(c => c.GetParameters().Length);
         }
     }
 }

sdk/extensions/Microsoft.Extensions.Azure/tests/ClientFactoryTests.cs

@@ -81,29 +81,24 @@ public void UsesLongestConstructor()
             Assert.AreSame(clientOptions, client.Options);
         }
 
-        [Test]
-        public void ThrowsWhenUnableToReadCompositeObject()
-        {
-            IConfiguration configuration = GetConfiguration(
-                new KeyValuePair<string, string>("composite:non-existent", "_"));
-
-            var clientOptions = new TestClientOptions();
-            var exception = Assert.Throws<InvalidOperationException>(() => ClientFactory.CreateClient(typeof(TestClient), typeof(TestClientOptions), clientOptions, configuration, null));
-            Assert.AreEqual("Unable to convert section 'composite' to parameter type 'Azure.Core.Extensions.Tests.TestClient+CompositeObject', unable to find matching constructor.", exception.Message);
-        }
-
         [Test]
         public void ThrowsExceptionWithInformationAboutArguments()
         {
-            IConfiguration configuration = GetConfiguration();
+            IConfiguration configuration = GetConfiguration(
+                new KeyValuePair<string, string>("some section:a", "a"),
+                new KeyValuePair<string, string>("some section:b:c", "b")
+                ).GetSection("some section");
 
             var clientOptions = new TestClientOptions();
-            var exception = Assert.Throws<InvalidOperationException>(() => ClientFactory.CreateClient(typeof(TestClient), typeof(TestClientOptions), clientOptions, configuration, null));
-            Assert.AreEqual("Unable to find matching constructor. Define one of the follow sets of configuration parameters:" + Environment.NewLine +
-                "1. connectionString" + Environment.NewLine +
-                "2. uri" + Environment.NewLine +
-                "3. uri, composite" + Environment.NewLine +
-                "4. composite" + Environment.NewLine,
+            var exception = Assert.Throws<InvalidOperationException>(() => ClientFactory.CreateClient(typeof(TestClientWithCredentials), typeof(TestClientOptions), clientOptions, configuration, null));
+            Assert.AreEqual("Unable to find matching constructor while trying to create an instance of TestClientWithCredentials." + Environment.NewLine +
+                "Expected one of the follow sets of configuration parameters:" + Environment.NewLine +
+                "1. uri" + Environment.NewLine +
+                "2. uri, credential:key" + Environment.NewLine +
+                "3. uri, credential:signature" + Environment.NewLine +
+                "4. uri" + Environment.NewLine +
+                "" + Environment.NewLine +
+                "Found the following configuration keys: b, b:c, a",
                 exception.Message);
         }
 
@@ -205,6 +200,57 @@ public void IgnoresConstructorWhenCredentialsNull()
             Assert.AreSame(clientOptions, client.Options);
         }
 
+        [Test]
+        public void IgnoresNonTokenCredentialConstructors()
+        {
+            IConfiguration configuration = GetConfiguration(
+                new KeyValuePair<string, string>("uri", "http://localhost"),
+                new KeyValuePair<string, string>("credential", "managedidentity"),
+                new KeyValuePair<string, string>("clientId", "secret")
+            );
+
+            var clientOptions = new TestClientOptions();
+            var client = (TestClientWithCredentials)ClientFactory.CreateClient(typeof(TestClientWithCredentials), typeof(TestClientOptions), clientOptions, configuration, new DefaultAzureCredential());
+
+            Assert.AreEqual("http://localhost/", client.Uri.ToString());
+            Assert.AreSame(clientOptions, client.Options);
+            Assert.NotNull(client.Credential);
+        }
+
+        [Test]
+        public void CanUseAzureKeyCredential()
+        {
+            IConfiguration configuration = GetConfiguration(
+                new KeyValuePair<string, string>("uri", "http://localhost"),
+                new KeyValuePair<string, string>("credential:key", "key"),
+                new KeyValuePair<string, string>("clientId", "secret")
+            );
+
+            var clientOptions = new TestClientOptions();
+            var client = (TestClientWithCredentials)ClientFactory.CreateClient(typeof(TestClientWithCredentials), typeof(TestClientOptions), clientOptions, configuration, new DefaultAzureCredential());
+
+            Assert.AreEqual("http://localhost/", client.Uri.ToString());
+            Assert.AreSame(clientOptions, client.Options);
+            Assert.AreEqual("key", client.AzureKeyCredential.Key);
+        }
+
+        [Test]
+        public void CanUseAzureSasCredential()
+        {
+            IConfiguration configuration = GetConfiguration(
+                new KeyValuePair<string, string>("uri", "http://localhost"),
+                new KeyValuePair<string, string>("credential:signature", "key"),
+                new KeyValuePair<string, string>("clientId", "secret")
+            );
+
+            var clientOptions = new TestClientOptions();
+            var client = (TestClientWithCredentials)ClientFactory.CreateClient(typeof(TestClientWithCredentials), typeof(TestClientOptions), clientOptions, configuration, new DefaultAzureCredential());
+
+            Assert.AreEqual("http://localhost/", client.Uri.ToString());
+            Assert.AreSame(clientOptions, client.Options);
+            Assert.AreEqual("key", client.AzureSasCredential.Signature);
+        }
+
         private IConfiguration GetConfiguration(params KeyValuePair<string, string>[] items)
         {
             return new ConfigurationBuilder().AddInMemoryCollection(items).Build();

sdk/extensions/Microsoft.Extensions.Azure/tests/TestClientWithCredentials.cs

@@ -8,11 +8,25 @@ namespace Azure.Core.Extensions.Tests
     internal class TestClientWithCredentials : TestClient
     {
         public TokenCredential Credential { get; }
+        public AzureKeyCredential AzureKeyCredential { get; }
+        public AzureSasCredential AzureSasCredential { get; }
 
         public TestClientWithCredentials(Uri uri, TestClientOptions options) : base(uri, options)
         {
         }
 
+        public TestClientWithCredentials(Uri uri, AzureKeyCredential credential, TestClientOptions options) : base(uri, options)
+        {
+            if (credential == null) throw new ArgumentNullException(nameof(credential));
+            AzureKeyCredential = credential;
+        }
+
+        public TestClientWithCredentials(Uri uri, AzureSasCredential credential, TestClientOptions options) : base(uri, options)
+        {
+            if (credential == null) throw new ArgumentNullException(nameof(credential));
+            AzureSasCredential = credential;
+        }
+
         public TestClientWithCredentials(Uri uri, TokenCredential credential, TestClientOptions options) : base(uri, options)
         {
             if (credential == null) throw new ArgumentNullException(nameof(credential));