From f63dde8e8e7c0f7b0ec855f87085eba1f8ed23b5 Mon Sep 17 00:00:00 2001
From: Yash Tibrewal <yashkt@google.com>
Date: Mon, 20 May 2019 11:45:07 -0700
Subject: [PATCH] Make validation function a global function

---
 .../grpcpp/support/channel_arguments_impl.h   | 35 ++++++-------------
 src/cpp/common/channel_arguments.cc           | 27 +++++++-------
 .../end2end/service_config_end2end_test.cc    |  5 +--
 3 files changed, 26 insertions(+), 41 deletions(-)

diff --git a/include/grpcpp/support/channel_arguments_impl.h b/include/grpcpp/support/channel_arguments_impl.h
index 9a034078033..a137a5b7c7d 100644
--- a/include/grpcpp/support/channel_arguments_impl.h
+++ b/include/grpcpp/support/channel_arguments_impl.h
@@ -31,6 +31,15 @@ namespace grpc {
 namespace testing {
 class ChannelArgumentsTest;
 }  // namespace testing
+
+namespace experimental {
+/// Validates \a service_config_json. If valid, returns an empty string.
+/// Otherwise, returns the validation error.
+/// TODO(yashykt): Promote it to out of experimental once it is proved useful
+/// and gRFC is accepted.
+grpc::string ValidateServiceConfigJSON(const grpc::string& service_config_json);
+}  // namespace experimental
+
 }  // namespace grpc
 
 namespace grpc_impl {
@@ -40,27 +49,8 @@ class SecureChannelCredentials;
 /// Options for channel creation. The user can use generic setters to pass
 /// key value pairs down to C channel creation code. For gRPC related options,
 /// concrete setters are provided.
-/// This class derives from GrpcLibraryCodegen so that gRPC is initialized
-/// before ValidateAndSetServiceConfigJSON is used. (Service config validation
-/// methods are registered at initialization.)
-class ChannelArguments : private ::grpc::GrpcLibraryCodegen {
+class ChannelArguments {
  public:
-  /// NOTE: class experimental_type is not part of the public API of this class.
-  /// TODO(yashykt): Integrate into public API when this is no longer
-  /// experimental.
-  class experimental_type {
-   public:
-    explicit experimental_type(ChannelArguments* args) : args_(args) {}
-
-    /// Validates \a service_config_json. If valid, sets the service config and
-    /// returns an empty string. If invalid, returns the validation error.
-    grpc::string ValidateAndSetServiceConfigJSON(
-        const grpc::string& service_config_json);
-
-   private:
-    ChannelArguments* args_;
-  };
-
   ChannelArguments();
   ~ChannelArguments();
 
@@ -144,11 +134,6 @@ class ChannelArguments : private ::grpc::GrpcLibraryCodegen {
     return out;
   }
 
-  /// NOTE: The function experimental() is not stable public API. It is a view
-  /// to the experimental components of this class. It may be changed or removed
-  /// at any time.
-  experimental_type experimental() { return experimental_type(this); }
-
  private:
   friend class grpc_impl::SecureChannelCredentials;
   friend class grpc::testing::ChannelArgumentsTest;
diff --git a/src/cpp/common/channel_arguments.cc b/src/cpp/common/channel_arguments.cc
index 48dfe815042..2f32703814e 100644
--- a/src/cpp/common/channel_arguments.cc
+++ b/src/cpp/common/channel_arguments.cc
@@ -31,18 +31,13 @@
 
 namespace grpc_impl {
 
-namespace {
-::grpc::internal::GrpcLibraryInitializer g_gli_initializer;
-}  // namespace
-
 ChannelArguments::ChannelArguments() {
-  g_gli_initializer.summon();
   // This will be ignored if used on the server side.
   SetString(GRPC_ARG_PRIMARY_USER_AGENT_STRING, "grpc-c++/" + grpc::Version());
 }
 
 ChannelArguments::ChannelArguments(const ChannelArguments& other)
-    : ::grpc::GrpcLibraryCodegen(), strings_(other.strings_) {
+    : strings_(other.strings_) {
   args_.reserve(other.args_.size());
   auto list_it_dst = strings_.begin();
   auto list_it_src = other.strings_.begin();
@@ -222,18 +217,22 @@ void ChannelArguments::SetChannelArgs(grpc_channel_args* channel_args) const {
   }
 }
 
-grpc::string
-ChannelArguments::experimental_type::ValidateAndSetServiceConfigJSON(
+}  // namespace grpc_impl
+
+namespace grpc {
+namespace experimental {
+grpc::string ValidateServiceConfigJSON(
     const grpc::string& service_config_json) {
+  grpc_init();
   grpc_error* error = GRPC_ERROR_NONE;
   grpc_core::ServiceConfig::Create(service_config_json.c_str(), &error);
+  grpc::string return_value;
   if (error != GRPC_ERROR_NONE) {
-    grpc::string return_value = grpc_error_string(error);
+    return_value = grpc_error_string(error);
     GRPC_ERROR_UNREF(error);
-    return return_value;
   }
-  args_->SetServiceConfigJSON(service_config_json);
-  return "";
+  grpc_shutdown();
+  return return_value;
 }
-
-}  // namespace grpc_impl
+}  // namespace experimental
+}  // namespace grpc
diff --git a/test/cpp/end2end/service_config_end2end_test.cc b/test/cpp/end2end/service_config_end2end_test.cc
index 74a304f6b26..e5c3a8e3eff 100644
--- a/test/cpp/end2end/service_config_end2end_test.cc
+++ b/test/cpp/end2end/service_config_end2end_test.cc
@@ -231,9 +231,10 @@ class ServiceConfigEnd2endTest : public ::testing::Test {
 
   std::shared_ptr<Channel> BuildChannelWithDefaultServiceConfig() {
     ChannelArguments args;
-    EXPECT_THAT(args.experimental().ValidateAndSetServiceConfigJSON(
+    EXPECT_THAT(grpc::experimental::ValidateServiceConfigJSON(
                     ValidDefaultServiceConfig()),
                 ::testing::StrEq(""));
+    args.SetServiceConfigJSON(ValidDefaultServiceConfig());
     args.SetPointer(GRPC_ARG_FAKE_RESOLVER_RESPONSE_GENERATOR,
                     response_generator_.get());
     return ::grpc::CreateCustomChannel("fake:///", creds_, args);
@@ -242,7 +243,7 @@ class ServiceConfigEnd2endTest : public ::testing::Test {
   std::shared_ptr<Channel> BuildChannelWithInvalidDefaultServiceConfig() {
     ChannelArguments args;
     EXPECT_THAT(
-        args.experimental().ValidateAndSetServiceConfigJSON(
+        grpc::experimental::ValidateServiceConfigJSON(
             InvalidDefaultServiceConfig()),
         ::testing::HasSubstr("failed to parse JSON for service config"));
     args.SetServiceConfigJSON(InvalidDefaultServiceConfig());