refactor grpc: disallow constructing SimpleClientComponent of non-generated clients

This was an accidentally allowed hack.

`samples/grpc_service` shows how to create custom gRPC client wrappers if needed.
commit_hash:eac2884a8429cf8672dbbcb3b129a195f470134e

Author: Anton3

otlp/src/otlp/logs/logger.cpp

@@ -214,7 +214,6 @@ Logger::Logger(
                                                 trace_client = std::move(trace_client)]() mutable {
         SendingLoop(consumer, log_client, trace_client);
     });
-    ;
 }
 
 Logger::~Logger() { Stop(); }

samples/grpc_service/src/call_greeter_client_test_handler.hpp

@@ -18,7 +18,7 @@ class CallGreeterClientTestHandler final : public server::handlers::HttpHandlerB
 
     CallGreeterClientTestHandler(const components::ComponentConfig& config, const components::ComponentContext& context)
         : HttpHandlerBase(config, context),
-          grpc_greeter_client_(context.FindComponent<GreeterClientComponent>().GetClient()) {}
+          grpc_greeter_client_(context.FindComponent<GreeterClientComponent>().GetClientWrapper()) {}
 
     std::string HandleRequest(server::http::HttpRequest& request, server::request::RequestContext&) const override {
         const auto& arg_case = request.GetArg("case");

samples/grpc_service/src/greeter_client.cpp

@@ -25,7 +25,7 @@ const USERVER_NAMESPACE::dynamic_config::Key<ugrpc::client::ClientQos> kGreeterC
 
 // A user-defined wrapper around api::GreeterServiceClient that provides
 // a simplified interface.
-GreeterClient::GreeterClient(api::GreeterServiceClient&& raw_client) : raw_client_(std::move(raw_client)) {}
+GreeterClient::GreeterClient(api::GreeterServiceClient& raw_client) : raw_client_(raw_client) {}
 
 /// [client]
 std::string GreeterClient::SayHello(std::string name) const {

samples/grpc_service/src/greeter_client.hpp

@@ -27,7 +27,7 @@ namespace samples {
 // microservice. Ignore that, it's just for the sake of example.
 class GreeterClient final {
 public:
-    explicit GreeterClient(api::GreeterServiceClient&& raw_client);
+    explicit GreeterClient(api::GreeterServiceClient& raw_client);
 
     std::string SayHello(std::string name) const;
 
@@ -37,12 +37,10 @@ class GreeterClient final {
 
     std::vector<std::string> SayHelloStreams(const std::vector<std::string_view>& names) const;
 
-    static std::optional<ugrpc::impl::StaticServiceMetadata> GetMetadata() { return std::nullopt; }
-
 private:
     static ugrpc::client::CallOptions MakeCallOptions();
 
-    api::GreeterServiceClient raw_client_;
+    api::GreeterServiceClient& raw_client_;
 };
 /// [client]
 
@@ -51,16 +49,21 @@ using Client = GreeterClient;
 /// [component]
 extern const USERVER_NAMESPACE::dynamic_config::Key<ugrpc::client::ClientQos> kGreeterClientQos;
 
-class GreeterClientComponent final : public ugrpc::client::SimpleClientComponent<Client> {
+class GreeterClientComponent final : public ugrpc::client::SimpleClientComponent<api::GreeterServiceClient> {
 public:
     static constexpr std::string_view kName = "greeter-client";
 
-    using Base = ugrpc::client::SimpleClientComponent<Client>;
+    using Base = ugrpc::client::SimpleClientComponent<api::GreeterServiceClient>;
 
     GreeterClientComponent(const ::components::ComponentConfig& config, const ::components::ComponentContext& context)
-        : Base(config, context, kGreeterClientQos) {}
+        : Base(config, context, kGreeterClientQos), client_wrapper_(GetClient()) {}
 
     using Base::GetClient;
+
+    GreeterClient& GetClientWrapper() noexcept { return client_wrapper_; }
+
+private:
+    GreeterClient client_wrapper_;
 };
 /// [component]
 

samples/grpc_service/unittests/greeter_service_test.cpp

@@ -17,27 +17,30 @@ class GreeterServiceTest : public ugrpc::tests::ServiceFixtureBase {
     GreeterServiceTest() : service_(prefix_) {
         RegisterService(service_);
         StartServer();
+        generated_client_ = MakeClient<samples::api::GreeterServiceClient>();
     }
 
     ~GreeterServiceTest() override { StopServer(); }
 
+    samples::api::GreeterServiceClient& GetGeneratedClient() { return generated_client_.value(); }
+
 private:
     const std::string prefix_{"Hello"};
     // We've made sure to separate the logic into samples::GreeterService that is
     // detached from the component system and only depends on things obtainable
     // in gtest tests.
     samples::GreeterService service_;
+
+    std::optional<samples::api::GreeterServiceClient> generated_client_;
 };
 /// [service fixture]
 }  // namespace
 
 /// [service tests]
 UTEST_F(GreeterServiceTest, SayHelloDirectCall) {
-    const auto client = MakeClient<samples::api::GreeterServiceClient>();
-
     samples::api::GreetingRequest request;
     request.set_name("gtest");
-    const auto response = client.SayHello(request);
+    const auto response = GetGeneratedClient().SayHello(request);
 
     EXPECT_EQ(response.greeting(), "Hello, gtest!");
 }
@@ -46,15 +49,15 @@ UTEST_F(GreeterServiceTest, SayHelloCustomClient) {
     // We've made sure to separate some logic into samples::GreeterClient that is
     // detached from the component system, it only needs the gRPC client, which we
     // can create in gtest tests.
-    const samples::GreeterClient client{MakeClient<samples::api::GreeterServiceClient>()};
+    const samples::GreeterClient client{GetGeneratedClient()};
     const auto response = client.SayHello("gtest");
     EXPECT_EQ(response, "Hello, gtest!");
 }
 /// [service tests]
 
 /// [service tests response stream]
 UTEST_F(GreeterServiceTest, SayHelloResponseStreamCustomClient) {
-    const samples::GreeterClient client{MakeClient<samples::api::GreeterServiceClient>()};
+    const samples::GreeterClient client{GetGeneratedClient()};
     const auto responses = client.SayHelloResponseStream("gtest");
     EXPECT_THAT(
         responses,
@@ -67,7 +70,7 @@ UTEST_F(GreeterServiceTest, SayHelloResponseStreamCustomClient) {
 
 /// [service tests request stream]
 UTEST_F(GreeterServiceTest, SayHelloRequestStreamCustomClient) {
-    const samples::GreeterClient client{MakeClient<samples::api::GreeterServiceClient>()};
+    const samples::GreeterClient client{GetGeneratedClient()};
 
     const std::vector<std::string_view> names = {"gtest", "!", "!", "!"};
     const auto response = client.SayHelloRequestStream(names);
@@ -78,7 +81,7 @@ UTEST_F(GreeterServiceTest, SayHelloRequestStreamCustomClient) {
 
 /// [service tests streams]
 UTEST_F(GreeterServiceTest, SayHelloStreamsCustomClient) {
-    const samples::GreeterClient client{MakeClient<samples::api::GreeterServiceClient>()};
+    const samples::GreeterClient client{GetGeneratedClient()};
 
     const std::vector<std::string_view> names = {"gtest", "!", "!", "!"};
     const auto responses = client.SayHelloStreams(names);
@@ -102,18 +105,26 @@ class GreeterMock final : public samples::api::GreeterServiceBase {
         ::samples::api::GreetingRequest&& request,
         SayHelloResponseStreamWriter& writer
     ) override;
+
     SayHelloRequestStreamResult SayHelloRequestStream(CallContext& context, SayHelloRequestStreamReader& reader)
         override;
+
     SayHelloStreamsResult SayHelloStreams(CallContext& context, SayHelloStreamsReaderWriter& stream) override;
 };
 
 // Default-constructs GreeterMock.
-using GreeterClientTest = ugrpc::tests::ServiceFixture<GreeterMock>;
+class GreeterClientTest : public ugrpc::tests::ServiceFixture<GreeterMock> {
+protected:
+    samples::api::GreeterServiceClient& GetGeneratedClient() { return generated_client_; }
+
+private:
+    samples::api::GreeterServiceClient generated_client_{MakeClient<samples::api::GreeterServiceClient>()};
+};
 
 }  // namespace
 
 UTEST_F(GreeterClientTest, SayHelloMockedServiceCustomClient) {
-    const samples::GreeterClient client{MakeClient<samples::api::GreeterServiceClient>()};
+    const samples::GreeterClient client{GetGeneratedClient()};
     const auto response = client.SayHello("gtest");
     EXPECT_EQ(response, "Mocked response");
 }
@@ -138,7 +149,7 @@ GreeterMock::SayHelloResponseStreamResult GreeterMock::SayHelloResponseStream(
 }
 
 UTEST_F(GreeterClientTest, SayHelloResponseStreamMockedServiceCustomClient) {
-    const samples::GreeterClient client{MakeClient<samples::api::GreeterServiceClient>()};
+    const samples::GreeterClient client{GetGeneratedClient()};
     const auto responses = client.SayHelloResponseStream("gtest");
     EXPECT_THAT(
         responses,
@@ -161,7 +172,7 @@ GreeterMock::SayHelloRequestStream(CallContext& /*context*/, SayHelloRequestStre
 }
 
 UTEST_F(GreeterClientTest, SayHelloRequestStreamMockedServiceCustomClient) {
-    const samples::GreeterClient client{MakeClient<samples::api::GreeterServiceClient>()};
+    const samples::GreeterClient client{GetGeneratedClient()};
     const std::vector<std::string_view> names = {"gtest", "!", "!", "!"};
     auto response = client.SayHelloRequestStream(names);
     EXPECT_EQ(response, "Mocked response!!!");
@@ -183,7 +194,7 @@ GreeterMock::SayHelloStreams(CallContext& /*context*/, SayHelloStreamsReaderWrit
 }
 
 UTEST_F(GreeterClientTest, SayHelloStreamsMockedServiceCustomClient) {
-    const samples::GreeterClient client{MakeClient<samples::api::GreeterServiceClient>()};
+    const samples::GreeterClient client{GetGeneratedClient()};
 
     const std::vector<std::string_view> names = {"gtest", "!", "!", "!"};
     const auto responses = client.SayHelloStreams(names);

scripts/grpc/templates/client.usrv.hpp.jinja

@@ -72,8 +72,9 @@ public:
 
     /// @cond
     // For internal use only
-    {{service.name}}Client(USERVER_NAMESPACE::ugrpc::client::impl::ClientInternals&& internals);
+    explicit {{service.name}}Client(USERVER_NAMESPACE::ugrpc::client::impl::ClientInternals&& internals);
 
+    // For internal use only
     static USERVER_NAMESPACE::ugrpc::impl::StaticServiceMetadata GetMetadata();
     /// @endcond
 
@@ -142,9 +143,11 @@ public:
 
     /// @cond
     // For internal use only
-    {{service.name}}Client(
-        USERVER_NAMESPACE::utils::SharedRef<{{namespace_path}}::{{service.name}}Client> protobuf_client);
+    explicit {{service.name}}Client(
+        USERVER_NAMESPACE::utils::SharedRef<{{namespace_path}}::{{service.name}}Client> protobuf_client
+    );
 
+    // For internal use only
     static USERVER_NAMESPACE::ugrpc::impl::StaticServiceMetadata GetMetadata();
     /// @endcond