Fix client factory extension method validation issues (#2159)

grpc · Jun 14, 2023 · 73c726b · 73c726b
1 parent 697f349
commit 73c726b
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 20 deletions.
diff --git a/src/Grpc.Net.ClientFactory/GrpcClientServiceExtensions.cs b/src/Grpc.Net.ClientFactory/GrpcClientServiceExtensions.cs
@@ -1,4 +1,4 @@
-#region Copyright notice and license
+#region Copyright notice and license
 
 // Copyright 2019 The gRPC Authors
 //
@@ -65,9 +65,7 @@ public static IHttpClientBuilder AddGrpcClient<
             throw new ArgumentNullException(nameof(services));
         }
 
-        var name = TypeNameHelper.GetTypeDisplayName(typeof(TClient), fullName: false);
-
-        return services.AddGrpcClientCore<TClient>(name);
+        return services.AddGrpcClient<TClient>(o => { });
     }
 
     /// <summary>
@@ -183,7 +181,7 @@ public static IHttpClientBuilder AddGrpcClient<
             throw new ArgumentNullException(nameof(name));
         }
 
-        return services.AddGrpcClientCore<TClient>(name);
+        return services.AddGrpcClient<TClient>(name, o => { });
     }
 
     /// <summary>

diff --git a/src/Grpc.Net.ClientFactory/GrpcHttpClientBuilderExtensions.cs b/src/Grpc.Net.ClientFactory/GrpcHttpClientBuilderExtensions.cs
@@ -1,4 +1,4 @@
-#region Copyright notice and license
+#region Copyright notice and license
 
 // Copyright 2019 The gRPC Authors
 //
@@ -47,7 +47,7 @@ public static IHttpClientBuilder ConfigureChannel(this IHttpClientBuilder builde
             throw new ArgumentNullException(nameof(configureChannel));
         }
 
-        ValidateGrpcClient(builder);
+        ValidateGrpcClient(builder, nameof(ConfigureChannel));
 
         builder.Services.AddTransient<IConfigureOptions<GrpcClientFactoryOptions>>(services =>
         {
@@ -78,7 +78,7 @@ public static IHttpClientBuilder ConfigureChannel(this IHttpClientBuilder builde
             throw new ArgumentNullException(nameof(configureChannel));
         }
 
-        ValidateGrpcClient(builder);
+        ValidateGrpcClient(builder, nameof(ConfigureChannel));
 
         builder.Services.Configure<GrpcClientFactoryOptions>(builder.Name, options =>
         {
@@ -119,7 +119,7 @@ public static IHttpClientBuilder AddInterceptor(this IHttpClientBuilder builder,
             throw new ArgumentNullException(nameof(configureInvoker));
         }
 
-        ValidateGrpcClient(builder);
+        ValidateGrpcClient(builder, nameof(AddInterceptor));
 
         builder.Services.Configure<GrpcClientFactoryOptions>(builder.Name, options =>
         {
@@ -147,7 +147,7 @@ public static IHttpClientBuilder AddCallCredentials(this IHttpClientBuilder buil
             throw new ArgumentNullException(nameof(authInterceptor));
         }
 
-        ValidateGrpcClient(builder);
+        ValidateGrpcClient(builder, nameof(AddCallCredentials));
 
         builder.Services.Configure<GrpcClientFactoryOptions>(builder.Name, options =>
         {
@@ -181,7 +181,7 @@ public static IHttpClientBuilder AddCallCredentials(this IHttpClientBuilder buil
             throw new ArgumentNullException(nameof(authInterceptor));
         }
 
-        ValidateGrpcClient(builder);
+        ValidateGrpcClient(builder, nameof(AddCallCredentials));
 
         builder.Services.Configure<GrpcClientFactoryOptions>(builder.Name, options =>
         {
@@ -215,7 +215,7 @@ public static IHttpClientBuilder AddCallCredentials(this IHttpClientBuilder buil
             throw new ArgumentNullException(nameof(credentials));
         }
 
-        ValidateGrpcClient(builder);
+        ValidateGrpcClient(builder, nameof(AddCallCredentials));
 
         builder.Services.Configure<GrpcClientFactoryOptions>(builder.Name, options =>
         {
@@ -270,7 +270,7 @@ public static IHttpClientBuilder AddInterceptor(this IHttpClientBuilder builder,
             throw new ArgumentNullException(nameof(configureInvoker));
         }
 
-        ValidateGrpcClient(builder);
+        ValidateGrpcClient(builder, nameof(AddInterceptor));
 
         builder.Services.Configure<GrpcClientFactoryOptions>(builder.Name, options =>
         {
@@ -308,7 +308,7 @@ public static IHttpClientBuilder AddInterceptor<TInterceptor>(this IHttpClientBu
             throw new ArgumentNullException(nameof(builder));
         }
 
-        ValidateGrpcClient(builder);
+        ValidateGrpcClient(builder, nameof(AddInterceptor));
 
         builder.AddInterceptor(scope, serviceProvider =>
         {
@@ -337,7 +337,7 @@ public static IHttpClientBuilder ConfigureGrpcClientCreator(this IHttpClientBuil
             throw new ArgumentNullException(nameof(configureCreator));
         }
 
-        ValidateGrpcClient(builder);
+        ValidateGrpcClient(builder, nameof(ConfigureGrpcClientCreator));
 
         builder.Services.AddTransient<IConfigureOptions<GrpcClientFactoryOptions>>(services =>
         {
@@ -369,7 +369,7 @@ public static IHttpClientBuilder ConfigureGrpcClientCreator(this IHttpClientBuil
             throw new ArgumentNullException(nameof(configureCreator));
         }
 
-        ValidateGrpcClient(builder);
+        ValidateGrpcClient(builder, nameof(ConfigureGrpcClientCreator));
 
         builder.Services.Configure<GrpcClientFactoryOptions>(builder.Name, options =>
         {
@@ -379,7 +379,7 @@ public static IHttpClientBuilder ConfigureGrpcClientCreator(this IHttpClientBuil
         return builder;
     }
 
-    private static void ValidateGrpcClient(IHttpClientBuilder builder)
+    private static void ValidateGrpcClient(IHttpClientBuilder builder, string caller)
     {
         // Validate the builder is for a gRPC client
         foreach (var service in builder.Services)
@@ -395,6 +395,6 @@ private static void ValidateGrpcClient(IHttpClientBuilder builder)
             }
         }
 
-        throw new InvalidOperationException($"{nameof(AddInterceptor)} must be used with a gRPC client.");
+        throw new InvalidOperationException($"{caller} must be used with a gRPC client.");
     }
 }
diff --git a/test/Grpc.Net.ClientFactory.Tests/GrpcHttpClientBuilderExtensionsTests.cs b/test/Grpc.Net.ClientFactory.Tests/GrpcHttpClientBuilderExtensionsTests.cs
@@ -1,4 +1,4 @@
-#region Copyright notice and license
+#region Copyright notice and license
 
 // Copyright 2019 The gRPC Authors
 //
@@ -137,7 +137,6 @@ public void AddInterceptor_NotFromGrpcClientFactoryAndExistingGrpcClient_ThrowEr
     {
         // Arrange
         var services = new ServiceCollection();
-        services.AddGrpcClient<Greeter.GreeterClient>();
         var client = services.AddHttpClient("TestClient");
 
         var ex = Assert.Throws<InvalidOperationException>(() => client.AddInterceptor(() => new CallbackInterceptor(o => { })))!;
@@ -147,6 +146,28 @@ public void AddInterceptor_NotFromGrpcClientFactoryAndExistingGrpcClient_ThrowEr
         Assert.AreEqual("AddInterceptor must be used with a gRPC client.", ex.Message);
     }
 
+    [Test]
+    public void AddInterceptor_AddGrpcClientWithoutConfig_NoError()
+    {
+        // Arrange
+        var services = new ServiceCollection();
+        var client = services.AddGrpcClient<Greeter.GreeterClient>();
+
+        // Act
+        client.AddInterceptor(() => new CallbackInterceptor(o => { }));
+    }
+
+    [Test]
+    public void AddInterceptor_AddGrpcClientWithNameAndWithoutConfig_NoError()
+    {
+        // Arrange
+        var services = new ServiceCollection();
+        var client = services.AddGrpcClient<Greeter.GreeterClient>(nameof(Greeter.GreeterClient));
+
+        // Act
+        client.AddInterceptor(() => new CallbackInterceptor(o => { }));
+    }
+
     [Test]
     public void AddInterceptor_NotFromGrpcClientFactory_ThrowError()
     {