[JSON] generalize handling of RefCountedPtr<> (#33048)

Also remove a check in the weighted_target LB policy that I somehow
missed in #32932.

build_autogenerated.yaml

@@ -5885,6 +5885,7 @@ targets:
   - test/core/client_channel/client_channel_service_config_test.cc
   deps:
   - grpc_test_util
+  uses_polling: false
 - name: client_channel_stress_test
   gtest: true
   build: test
@@ -5916,6 +5917,7 @@ targets:
   - test/core/client_channel/client_channel_test.cc
   deps:
   - grpc_test_util
+  uses_polling: false
 - name: client_context_test_peer_test
   gtest: true
   build: test
@@ -10671,6 +10673,7 @@ targets:
   - test/core/client_channel/retry_service_config_test.cc
   deps:
   - grpc_test_util
+  uses_polling: false
 - name: retry_throttle_test
   gtest: true
   build: test

src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc

@@ -1892,7 +1892,7 @@ class GrpcLbFactory : public LoadBalancingPolicyFactory {
 
   absl::StatusOr<RefCountedPtr<LoadBalancingPolicy::Config>>
   ParseLoadBalancingConfig(const Json& json) const override {
-    return LoadRefCountedFromJson<GrpcLbConfig>(
+    return LoadFromJson<RefCountedPtr<GrpcLbConfig>>(
         json, JsonArgs(), "errors validating grpclb LB policy config");
   }
 };

src/core/ext/filters/client_channel/lb_policy/priority/priority.cc

@@ -903,7 +903,7 @@ class PriorityLbFactory : public LoadBalancingPolicyFactory {
 
   absl::StatusOr<RefCountedPtr<LoadBalancingPolicy::Config>>
   ParseLoadBalancingConfig(const Json& json) const override {
-    return LoadRefCountedFromJson<PriorityLbConfig>(
+    return LoadFromJson<RefCountedPtr<PriorityLbConfig>>(
         json, JsonArgs(), "errors validating priority LB policy config");
   }
 };

src/core/ext/filters/client_channel/lb_policy/rls/rls.cc

@@ -2500,7 +2500,7 @@ class RlsLbFactory : public LoadBalancingPolicyFactory {
 
   absl::StatusOr<RefCountedPtr<LoadBalancingPolicy::Config>>
   ParseLoadBalancingConfig(const Json& json) const override {
-    return LoadRefCountedFromJson<RlsLbConfig>(
+    return LoadFromJson<RefCountedPtr<RlsLbConfig>>(
         json, JsonArgs(), "errors validing RLS LB policy config");
   }
 };

src/core/ext/filters/client_channel/lb_policy/weighted_round_robin/weighted_round_robin.cc

@@ -989,7 +989,7 @@ class WeightedRoundRobinFactory : public LoadBalancingPolicyFactory {
 
   absl::StatusOr<RefCountedPtr<LoadBalancingPolicy::Config>>
   ParseLoadBalancingConfig(const Json& json) const override {
-    return LoadRefCountedFromJson<WeightedRoundRobinConfig>(
+    return LoadFromJson<RefCountedPtr<WeightedRoundRobinConfig>>(
         json, JsonArgs(),
         "errors validating weighted_round_robin LB policy config");
   }

src/core/ext/filters/client_channel/lb_policy/weighted_target/weighted_target.cc

@@ -766,15 +766,7 @@ class WeightedTargetLbFactory : public LoadBalancingPolicyFactory {
 
   absl::StatusOr<RefCountedPtr<LoadBalancingPolicy::Config>>
   ParseLoadBalancingConfig(const Json& json) const override {
-    if (json.type() == Json::Type::kNull) {
-      // weighted_target was mentioned as a policy in the deprecated
-      // loadBalancingPolicy field or in the client API.
-      return absl::InvalidArgumentError(
-          "field:loadBalancingPolicy error:weighted_target policy requires "
-          "configuration.  Please use loadBalancingConfig field of service "
-          "config instead.");
-    }
-    return LoadRefCountedFromJson<WeightedTargetLbConfig>(
+    return LoadFromJson<RefCountedPtr<WeightedTargetLbConfig>>(
         json, JsonArgs(), "errors validating weighted_target LB policy config");
   }
 };

src/core/ext/filters/client_channel/lb_policy/xds/cds.cc

@@ -755,7 +755,7 @@ class CdsLbFactory : public LoadBalancingPolicyFactory {
 
   absl::StatusOr<RefCountedPtr<LoadBalancingPolicy::Config>>
   ParseLoadBalancingConfig(const Json& json) const override {
-    return LoadRefCountedFromJson<CdsLbConfig>(
+    return LoadFromJson<RefCountedPtr<CdsLbConfig>>(
         json, JsonArgs(), "errors validating cds LB policy config");
   }
 };

src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_impl.cc

@@ -777,7 +777,7 @@ class XdsClusterImplLbFactory : public LoadBalancingPolicyFactory {
 
   absl::StatusOr<RefCountedPtr<LoadBalancingPolicy::Config>>
   ParseLoadBalancingConfig(const Json& json) const override {
-    return LoadRefCountedFromJson<XdsClusterImplLbConfig>(
+    return LoadFromJson<RefCountedPtr<XdsClusterImplLbConfig>>(
         json, JsonArgs(),
         "errors validating xds_cluster_impl LB policy config");
   }

src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_manager.cc

@@ -678,7 +678,7 @@ class XdsClusterManagerLbFactory : public LoadBalancingPolicyFactory {
 
   absl::StatusOr<RefCountedPtr<LoadBalancingPolicy::Config>>
   ParseLoadBalancingConfig(const Json& json) const override {
-    return LoadRefCountedFromJson<XdsClusterManagerLbConfig>(
+    return LoadFromJson<RefCountedPtr<XdsClusterManagerLbConfig>>(
         json, JsonArgs(),
         "errors validating xds_cluster_manager LB policy config");
   }

src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_resolver.cc

@@ -1150,7 +1150,7 @@ class XdsClusterResolverLbFactory : public LoadBalancingPolicyFactory {
 
   absl::StatusOr<RefCountedPtr<LoadBalancingPolicy::Config>>
   ParseLoadBalancingConfig(const Json& json) const override {
-    return LoadRefCountedFromJson<XdsClusterResolverLbConfig>(
+    return LoadFromJson<RefCountedPtr<XdsClusterResolverLbConfig>>(
         json, JsonArgs(),
         "errors validating xds_cluster_resolver LB policy config");
   }

src/core/ext/filters/client_channel/lb_policy/xds/xds_override_host.cc

@@ -743,7 +743,7 @@ class XdsOverrideHostLbFactory : public LoadBalancingPolicyFactory {
 
   absl::StatusOr<RefCountedPtr<LoadBalancingPolicy::Config>>
   ParseLoadBalancingConfig(const Json& json) const override {
-    return LoadRefCountedFromJson<XdsOverrideHostLbConfig>(
+    return LoadFromJson<RefCountedPtr<XdsOverrideHostLbConfig>>(
         json, JsonArgs(),
         "errors validating xds_override_host LB policy config");
   }

src/core/ext/filters/client_channel/lb_policy/xds/xds_wrr_locality.cc

@@ -347,7 +347,7 @@ class XdsWrrLocalityLbFactory : public LoadBalancingPolicyFactory {
 
   absl::StatusOr<RefCountedPtr<LoadBalancingPolicy::Config>>
   ParseLoadBalancingConfig(const Json& json) const override {
-    return LoadRefCountedFromJson<XdsWrrLocalityLbConfig>(
+    return LoadFromJson<RefCountedPtr<XdsWrrLocalityLbConfig>>(
         json, JsonArgs(),
         "errors validating xds_wrr_locality LB policy config");
   }

src/core/lib/json/json_object_loader.h

@@ -428,6 +428,26 @@ class AutoLoader<std::unique_ptr<T>> final : public LoadOptional {
   ~AutoLoader() = default;
 };
 
+// Specializations of AutoLoader for RefCountedPtr<>.
+template <typename T>
+class AutoLoader<RefCountedPtr<T>> final : public LoadOptional {
+ public:
+  void* Emplace(void* dst) const final {
+    auto& p = *static_cast<RefCountedPtr<T>*>(dst);
+    p = MakeRefCounted<T>();
+    return p.get();
+  }
+  void Reset(void* dst) const final {
+    static_cast<RefCountedPtr<T>*>(dst)->reset();
+  }
+  const LoaderInterface* ElementLoader() const final {
+    return LoaderForType<T>();
+  }
+
+ private:
+  ~AutoLoader() = default;
+};
+
 // Implementation of aforementioned LoaderForType.
 // Simply keeps a static AutoLoader<T> and returns a pointer to that.
 template <typename T>
@@ -596,19 +616,6 @@ absl::StatusOr<T> LoadFromJson(
   return std::move(result);
 }
 
-template <typename T>
-absl::StatusOr<RefCountedPtr<T>> LoadRefCountedFromJson(
-    const Json& json, const JsonArgs& args = JsonArgs(),
-    absl::string_view error_prefix = "errors validating JSON") {
-  ValidationErrors errors;
-  auto result = MakeRefCounted<T>();
-  json_detail::LoaderForType<T>()->LoadInto(json, args, result.get(), &errors);
-  if (!errors.ok()) {
-    return errors.status(absl::StatusCode::kInvalidArgument, error_prefix);
-  }
-  return std::move(result);
-}
-
 template <typename T>
 T LoadFromJson(const Json& json, const JsonArgs& args,
                ValidationErrors* errors) {

src/core/lib/load_balancing/lb_policy_registry.cc

@@ -84,7 +84,7 @@ bool LoadBalancingPolicyRegistry::LoadBalancingPolicyExists(
   if (factory == nullptr) return false;
   // If requested, check if the load balancing policy allows an empty config.
   if (requires_config != nullptr) {
-    auto config = factory->ParseLoadBalancingConfig(Json());
+    auto config = factory->ParseLoadBalancingConfig(Json::FromObject({}));
     *requires_config = !config.ok();
   }
   return true;

test/core/client_channel/BUILD

@@ -23,6 +23,8 @@ grpc_cc_test(
     srcs = ["client_channel_test.cc"],
     external_deps = ["gtest"],
     language = "C++",
+    uses_event_engine = False,
+    uses_polling = False,
     deps = [
         "//:grpc_client_channel",
         "//src/core:channel_args",
@@ -67,6 +69,8 @@ grpc_cc_test(
         "gtest",
     ],
     language = "C++",
+    uses_event_engine = False,
+    uses_polling = False,
     deps = [
         "//:gpr",
         "//:grpc",
@@ -82,6 +86,8 @@ grpc_cc_test(
         "gtest",
     ],
     language = "C++",
+    uses_event_engine = False,
+    uses_polling = False,
     deps = [
         "//:gpr",
         "//:grpc",

test/core/json/json_object_loader_test.cc

@@ -926,6 +926,34 @@ TEST(JsonObjectLoader, BareUniquePtr) {
   EXPECT_EQ(**parsed, 3);
 }
 
+TEST(JsonObjectLoader, BareRefCountedPtr) {
+  class RefCountedObject : public RefCounted<RefCountedObject> {
+   public:
+    RefCountedObject() = default;
+
+    int value() const { return value_; }
+
+    static const JsonLoaderInterface* JsonLoader(const JsonArgs&) {
+      static const auto* loader = JsonObjectLoader<RefCountedObject>()
+                                      .Field("value", &RefCountedObject::value_)
+                                      .Finish();
+      return loader;
+    }
+
+   private:
+    int value_ = -1;
+  };
+  auto parsed = Parse<RefCountedPtr<RefCountedObject>>("{\"value\": 3}");
+  ASSERT_TRUE(parsed.ok()) << parsed.status();
+  ASSERT_NE(*parsed, nullptr);
+  EXPECT_EQ((*parsed)->value(), 3);
+  parsed = Parse<RefCountedPtr<RefCountedObject>>("5");
+  EXPECT_EQ(parsed.status().code(), absl::StatusCode::kInvalidArgument);
+  EXPECT_EQ(parsed.status().message(),
+            "errors validating JSON: [field: error:is not an object]")
+      << parsed.status();
+}
+
 TEST(JsonObjectLoader, BareVector) {
   auto parsed = Parse<std::vector<int32_t>>("[1, 2, 3]");
   ASSERT_TRUE(parsed.ok()) << parsed.status();
@@ -1051,40 +1079,6 @@ TEST(JsonObjectLoader, CustomValidationInPostLoadHook) {
       << test_struct.status();
 }
 
-TEST(JsonObjectLoader, LoadRefCountedFromJson) {
-  struct TestStruct : public RefCounted<TestStruct> {
-    int32_t a = 0;
-
-    static const JsonLoaderInterface* JsonLoader(const JsonArgs&) {
-      static const auto* loader =
-          JsonObjectLoader<TestStruct>().Field("a", &TestStruct::a).Finish();
-      return loader;
-    }
-  };
-  // Valid.
-  {
-    absl::string_view json_str = "{\"a\":1}";
-    auto json = JsonParse(json_str);
-    ASSERT_TRUE(json.ok()) << json.status();
-    absl::StatusOr<RefCountedPtr<TestStruct>> test_struct =
-        LoadRefCountedFromJson<TestStruct>(*json, JsonArgs());
-    ASSERT_TRUE(test_struct.ok()) << test_struct.status();
-    EXPECT_EQ((*test_struct)->a, 1);
-  }
-  // Invalid.
-  {
-    absl::string_view json_str = "{\"a\":\"foo\"}";
-    auto json = JsonParse(json_str);
-    ASSERT_TRUE(json.ok()) << json.status();
-    absl::StatusOr<RefCountedPtr<TestStruct>> test_struct =
-        LoadRefCountedFromJson<TestStruct>(*json, JsonArgs());
-    EXPECT_EQ(test_struct.status().code(), absl::StatusCode::kInvalidArgument);
-    EXPECT_EQ(test_struct.status().message(),
-              "errors validating JSON: [field:a error:failed to parse number]")
-        << test_struct.status();
-  }
-}
-
 TEST(JsonObjectLoader, LoadFromJsonWithValidationErrors) {
   struct TestStruct {
     int32_t a = 0;

test/cpp/interop/rpc_behavior_lb_policy.cc

@@ -177,7 +177,7 @@ class RpcBehaviorLbPolicyFactory
 
   absl::StatusOr<RefCountedPtr<LoadBalancingPolicy::Config>>
   ParseLoadBalancingConfig(const Json& json) const override {
-    return grpc_core::LoadRefCountedFromJson<RpcBehaviorLbPolicyConfig>(
+    return grpc_core::LoadFromJson<RefCountedPtr<RpcBehaviorLbPolicyConfig>>(
         json, JsonArgs(), "errors validating LB policy config");
   }
 };

tools/run_tests/generated/tests.json

@@ -1819,7 +1819,7 @@
       "posix",
       "windows"
     ],
-    "uses_polling": true
+    "uses_polling": false
   },
   {
     "args": [],
@@ -1843,7 +1843,7 @@
       "posix",
       "windows"
     ],
-    "uses_polling": true
+    "uses_polling": false
   },
   {
     "args": [],
@@ -6151,7 +6151,7 @@
       "posix",
       "windows"
     ],
-    "uses_polling": true
+    "uses_polling": false
   },
   {
     "args": [],