From 283574e3b7335d243fd5f4ec2334833d571c6b04 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Tue, 4 Feb 2020 07:42:31 -0800 Subject: [PATCH] Convert service config code to use new JSON API --- .../filters/client_channel/client_channel.cc | 6 +- .../client_channel/lb_policy/grpclb/grpclb.cc | 44 +- .../lb_policy/pick_first/pick_first.cc | 10 +- .../lb_policy/round_robin/round_robin.cc | 10 +- .../client_channel/lb_policy/xds/cds.cc | 106 ++-- .../client_channel/lb_policy/xds/xds.cc | 116 ++-- .../client_channel/lb_policy_factory.h | 6 +- .../client_channel/lb_policy_registry.cc | 118 ++-- .../client_channel/lb_policy_registry.h | 2 +- .../resolver/dns/c_ares/dns_resolver_ares.cc | 125 ++--- .../client_channel/resolver_result_parsing.cc | 502 ++++++++---------- .../client_channel/resolver_result_parsing.h | 4 +- .../filters/client_channel/service_config.cc | 240 +++------ .../filters/client_channel/service_config.h | 35 +- .../message_size/message_size_filter.cc | 61 +-- .../message_size/message_size_filter.h | 2 +- src/core/lib/gprpp/inlined_vector.h | 6 + src/core/lib/iomgr/error.h | 8 +- src/core/lib/json/json_reader_new.cc | 53 +- .../client_channel/service_config_test.cc | 136 +++-- .../core/client_channel/xds_bootstrap_test.cc | 4 +- test/core/util/test_lb_policies.cc | 2 +- .../end2end/service_config_end2end_test.cc | 7 +- test/cpp/naming/resolver_component_test.cc | 2 +- .../naming/resolver_component_tests_runner.py | 6 +- .../naming/resolver_test_record_groups.yaml | 8 +- 26 files changed, 701 insertions(+), 918 deletions(-) diff --git a/src/core/ext/filters/client_channel/client_channel.cc b/src/core/ext/filters/client_channel/client_channel.cc index 958be7250a0..4987ae8207b 100644 --- a/src/core/ext/filters/client_channel/client_channel.cc +++ b/src/core/ext/filters/client_channel/client_channel.cc @@ -1729,11 +1729,11 @@ bool ChannelData::ProcessResolverResultLocked( ((service_config == nullptr) != (chand->saved_service_config_ == nullptr)) || (service_config != nullptr && - strcmp(service_config->service_config_json(), - chand->saved_service_config_->service_config_json()) != 0); + service_config->json_string() != + chand->saved_service_config_->json_string()); if (service_config_changed) { service_config_json.reset(gpr_strdup( - service_config != nullptr ? service_config->service_config_json() + service_config != nullptr ? service_config->json_string().c_str() : "")); if (GRPC_TRACE_FLAG_ENABLED(grpc_client_channel_routing_trace)) { gpr_log(GPR_INFO, diff --git a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc index 00bc77d7e5d..8035008d036 100644 --- a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc +++ b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc @@ -121,10 +121,9 @@ namespace { constexpr char kGrpclb[] = "grpclb"; -class ParsedGrpcLbConfig : public LoadBalancingPolicy::Config { +class GrpcLbConfig : public LoadBalancingPolicy::Config { public: - explicit ParsedGrpcLbConfig( - RefCountedPtr child_policy) + explicit GrpcLbConfig(RefCountedPtr child_policy) : child_policy_(std::move(child_policy)) {} const char* name() const override { return kGrpclb; } @@ -1447,8 +1446,7 @@ void GrpcLb::ResetBackoffLocked() { void GrpcLb::UpdateLocked(UpdateArgs args) { const bool is_initial_update = lb_channel_ == nullptr; - auto* grpclb_config = - static_cast(args.config.get()); + auto* grpclb_config = static_cast(args.config.get()); if (grpclb_config != nullptr) { child_policy_config_ = grpclb_config->child_policy(); } else { @@ -1885,33 +1883,27 @@ class GrpcLbFactory : public LoadBalancingPolicyFactory { const char* name() const override { return kGrpclb; } RefCountedPtr ParseLoadBalancingConfig( - const grpc_json* json, grpc_error** error) const override { + const Json& json, grpc_error** error) const override { GPR_DEBUG_ASSERT(error != nullptr && *error == GRPC_ERROR_NONE); - if (json == nullptr) { - return RefCountedPtr( - new ParsedGrpcLbConfig(nullptr)); + if (json.type() == Json::Type::JSON_NULL) { + return MakeRefCounted(nullptr); } - InlinedVector error_list; + std::vector error_list; RefCountedPtr child_policy; - for (const grpc_json* field = json->child; field != nullptr; - field = field->next) { - if (field->key == nullptr) continue; - if (strcmp(field->key, "childPolicy") == 0) { - if (child_policy != nullptr) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:childPolicy error:Duplicate entry")); - } - grpc_error* parse_error = GRPC_ERROR_NONE; - child_policy = LoadBalancingPolicyRegistry::ParseLoadBalancingConfig( - field, &parse_error); - if (parse_error != GRPC_ERROR_NONE) { - error_list.push_back(parse_error); - } + auto it = json.object_value().find("childPolicy"); + if (it != json.object_value().end()) { + grpc_error* parse_error = GRPC_ERROR_NONE; + child_policy = LoadBalancingPolicyRegistry::ParseLoadBalancingConfig( + it->second, &parse_error); + if (parse_error != GRPC_ERROR_NONE) { + std::vector child_errors; + child_errors.push_back(parse_error); + error_list.push_back( + GRPC_ERROR_CREATE_FROM_VECTOR("field:childPolicy", &child_errors)); } } if (error_list.empty()) { - return RefCountedPtr( - new ParsedGrpcLbConfig(std::move(child_policy))); + return MakeRefCounted(std::move(child_policy)); } else { *error = GRPC_ERROR_CREATE_FROM_VECTOR("GrpcLb Parser", &error_list); return nullptr; diff --git a/src/core/ext/filters/client_channel/lb_policy/pick_first/pick_first.cc b/src/core/ext/filters/client_channel/lb_policy/pick_first/pick_first.cc index 7627092ad0d..278af3fde75 100644 --- a/src/core/ext/filters/client_channel/lb_policy/pick_first/pick_first.cc +++ b/src/core/ext/filters/client_channel/lb_policy/pick_first/pick_first.cc @@ -472,7 +472,7 @@ void PickFirst::PickFirstSubchannelData:: } } -class ParsedPickFirstConfig : public LoadBalancingPolicy::Config { +class PickFirstConfig : public LoadBalancingPolicy::Config { public: const char* name() const override { return kPickFirst; } }; @@ -491,12 +491,8 @@ class PickFirstFactory : public LoadBalancingPolicyFactory { const char* name() const override { return kPickFirst; } RefCountedPtr ParseLoadBalancingConfig( - const grpc_json* json, grpc_error** /*error*/) const override { - if (json != nullptr) { - GPR_DEBUG_ASSERT(strcmp(json->key, name()) == 0); - } - return RefCountedPtr( - new ParsedPickFirstConfig()); + const Json& json, grpc_error** /*error*/) const override { + return MakeRefCounted(); } }; diff --git a/src/core/ext/filters/client_channel/lb_policy/round_robin/round_robin.cc b/src/core/ext/filters/client_channel/lb_policy/round_robin/round_robin.cc index e7933980d2e..ecb46388ed5 100644 --- a/src/core/ext/filters/client_channel/lb_policy/round_robin/round_robin.cc +++ b/src/core/ext/filters/client_channel/lb_policy/round_robin/round_robin.cc @@ -467,7 +467,7 @@ void RoundRobin::UpdateLocked(UpdateArgs args) { } } -class ParsedRoundRobinConfig : public LoadBalancingPolicy::Config { +class RoundRobinConfig : public LoadBalancingPolicy::Config { public: const char* name() const override { return kRoundRobin; } }; @@ -486,12 +486,8 @@ class RoundRobinFactory : public LoadBalancingPolicyFactory { const char* name() const override { return kRoundRobin; } RefCountedPtr ParseLoadBalancingConfig( - const grpc_json* json, grpc_error** /*error*/) const override { - if (json != nullptr) { - GPR_DEBUG_ASSERT(strcmp(json->key, name()) == 0); - } - return RefCountedPtr( - new ParsedRoundRobinConfig()); + const Json& json, grpc_error** /*error*/) const override { + return MakeRefCounted(); } }; diff --git a/src/core/ext/filters/client_channel/lb_policy/xds/cds.cc b/src/core/ext/filters/client_channel/lb_policy/xds/cds.cc index d76dbab6269..4b39d88bc9d 100644 --- a/src/core/ext/filters/client_channel/lb_policy/xds/cds.cc +++ b/src/core/ext/filters/client_channel/lb_policy/xds/cds.cc @@ -36,12 +36,11 @@ namespace { constexpr char kCds[] = "cds_experimental"; -// Parsed config for this LB policy. -class ParsedCdsConfig : public LoadBalancingPolicy::Config { +// Config for this LB policy. +class CdsConfig : public LoadBalancingPolicy::Config { public: - explicit ParsedCdsConfig(std::string cluster) - : cluster_(std::move(cluster)) {} - const char* cluster() const { return cluster_.c_str(); } + explicit CdsConfig(std::string cluster) : cluster_(std::move(cluster)) {} + const std::string& cluster() const { return cluster_; } const char* name() const override { return kCds; } private: @@ -90,7 +89,7 @@ class CdsLb : public LoadBalancingPolicy { void ShutdownLocked() override; - RefCountedPtr config_; + RefCountedPtr config_; // Current channel args from the resolver. const grpc_channel_args* args_ = nullptr; @@ -118,41 +117,28 @@ void CdsLb::ClusterWatcher::OnClusterChanged(CdsUpdate cluster_data) { parent_.get()); } // Construct config for child policy. - char* lrs_str = nullptr; + Json::Object child_config = { + {"edsServiceName", + (cluster_data.eds_service_name.empty() ? parent_->config_->cluster() + : cluster_data.eds_service_name)}, + }; if (cluster_data.lrs_load_reporting_server_name.has_value()) { - gpr_asprintf(&lrs_str, " \"lrsLoadReportingServerName\": \"%s\",\n", - cluster_data.lrs_load_reporting_server_name.value().c_str()); + child_config["lrsLoadReportingServerName"] = + cluster_data.lrs_load_reporting_server_name.value(); } - char* json_str; - gpr_asprintf(&json_str, - "[{\n" - " \"xds_experimental\": {\n" - "%s" - " \"edsServiceName\": \"%s\"\n" - " }\n" - "}]", - (lrs_str == nullptr ? "" : lrs_str), - (cluster_data.eds_service_name.empty() - ? parent_->config_->cluster() - : cluster_data.eds_service_name.c_str())); - gpr_free(lrs_str); - grpc_core::UniquePtr json_str_deleter(json_str); + Json json = Json::Array{ + Json::Object{ + {"xds_experimental", std::move(child_config)}, + }, + }; if (GRPC_TRACE_FLAG_ENABLED(grpc_cds_lb_trace)) { + std::string json_str = json.Dump(); gpr_log(GPR_INFO, "[cdslb %p] generated config for child policy: %s", - parent_.get(), json_str); - } - grpc_json* json = grpc_json_parse_string(json_str); - if (json == nullptr) { - char* msg; - gpr_asprintf(&msg, "Could not parse LB config: %s", json_str); - OnError(GRPC_ERROR_CREATE_FROM_COPIED_STRING(msg)); - gpr_free(msg); - return; + parent_.get(), json_str.c_str()); } grpc_error* error = GRPC_ERROR_NONE; RefCountedPtr config = LoadBalancingPolicyRegistry::ParseLoadBalancingConfig(json, &error); - grpc_json_destroy(json); if (error != GRPC_ERROR_NONE) { OnError(error); return; @@ -179,7 +165,8 @@ void CdsLb::ClusterWatcher::OnClusterChanged(CdsUpdate cluster_data) { void CdsLb::ClusterWatcher::OnError(grpc_error* error) { gpr_log(GPR_ERROR, "[cdslb %p] xds error obtaining data for cluster %s: %s", - parent_.get(), parent_->config_->cluster(), grpc_error_string(error)); + parent_.get(), parent_->config_->cluster().c_str(), + grpc_error_string(error)); // Go into TRANSIENT_FAILURE if we have not yet created the child // policy (i.e., we have not yet received data from xds). Otherwise, // we keep running with the data we had previously. @@ -258,8 +245,8 @@ void CdsLb::ShutdownLocked() { } if (xds_client_ != nullptr) { if (cluster_watcher_ != nullptr) { - xds_client_->CancelClusterDataWatch(StringView(config_->cluster()), - cluster_watcher_); + xds_client_->CancelClusterDataWatch( + StringView(config_->cluster().c_str()), cluster_watcher_); } xds_client_.reset(); } @@ -281,15 +268,14 @@ void CdsLb::UpdateLocked(UpdateArgs args) { args_ = args.args; args.args = nullptr; // If cluster name changed, cancel watcher and restart. - if (old_config == nullptr || - strcmp(old_config->cluster(), config_->cluster()) != 0) { + if (old_config == nullptr || old_config->cluster() != config_->cluster()) { if (old_config != nullptr) { - xds_client_->CancelClusterDataWatch(StringView(old_config->cluster()), - cluster_watcher_); + xds_client_->CancelClusterDataWatch( + StringView(old_config->cluster().c_str()), cluster_watcher_); } auto watcher = grpc_core::MakeUnique(Ref()); cluster_watcher_ = watcher.get(); - xds_client_->WatchClusterData(StringView(config_->cluster()), + xds_client_->WatchClusterData(StringView(config_->cluster().c_str()), std::move(watcher)); } } @@ -308,9 +294,9 @@ class CdsFactory : public LoadBalancingPolicyFactory { const char* name() const override { return kCds; } RefCountedPtr ParseLoadBalancingConfig( - const grpc_json* json, grpc_error** error) const override { + const Json& json, grpc_error** error) const override { GPR_DEBUG_ASSERT(error != nullptr && *error == GRPC_ERROR_NONE); - if (json == nullptr) { + if (json.type() == Json::Type::JSON_NULL) { // xds was mentioned as a policy in the deprecated loadBalancingPolicy // field or in the client API. *error = GRPC_ERROR_CREATE_FROM_STATIC_STRING( @@ -318,35 +304,23 @@ class CdsFactory : public LoadBalancingPolicyFactory { "Please use loadBalancingConfig field of service config instead."); return nullptr; } - GPR_DEBUG_ASSERT(strcmp(json->key, name()) == 0); - InlinedVector error_list; - const char* cluster = nullptr; - for (const grpc_json* field = json->child; field != nullptr; - field = field->next) { - if (field->key == nullptr) continue; - if (strcmp(field->key, "cluster") == 0) { - if (cluster != nullptr) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:cluster error:Duplicate entry")); - } - if (field->type != GRPC_JSON_STRING) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:cluster error:type should be string")); - continue; - } - cluster = field->value; - } - } - if (cluster == nullptr) { + std::vector error_list; + std::string cluster; + auto it = json.object_value().find("cluster"); + if (it == json.object_value().end()) { error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( "required field 'cluster' not present")); - } - if (error_list.empty()) { - return MakeRefCounted(cluster); + } else if (it->second.type() != Json::Type::STRING) { + error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "field:cluster error:type should be string")); } else { + cluster = it->second.string_value(); + } + if (!error_list.empty()) { *error = GRPC_ERROR_CREATE_FROM_VECTOR("Cds Parser", &error_list); return nullptr; } + return MakeRefCounted(std::move(cluster)); } }; diff --git a/src/core/ext/filters/client_channel/lb_policy/xds/xds.cc b/src/core/ext/filters/client_channel/lb_policy/xds/xds.cc index 953e1a4bc3d..1c8c46eaace 100644 --- a/src/core/ext/filters/client_channel/lb_policy/xds/xds.cc +++ b/src/core/ext/filters/client_channel/lb_policy/xds/xds.cc @@ -74,12 +74,12 @@ namespace { constexpr char kXds[] = "xds_experimental"; -class ParsedXdsConfig : public LoadBalancingPolicy::Config { +class XdsConfig : public LoadBalancingPolicy::Config { public: - ParsedXdsConfig(RefCountedPtr child_policy, - RefCountedPtr fallback_policy, - std::string eds_service_name, - Optional lrs_load_reporting_server_name) + XdsConfig(RefCountedPtr child_policy, + RefCountedPtr fallback_policy, + std::string eds_service_name, + Optional lrs_load_reporting_server_name) : child_policy_(std::move(child_policy)), fallback_policy_(std::move(fallback_policy)), eds_service_name_(std::move(eds_service_name)), @@ -387,7 +387,7 @@ class XdsLb : public LoadBalancingPolicy { // Current channel args and config from the resolver. const grpc_channel_args* args_ = nullptr; - RefCountedPtr config_; + RefCountedPtr config_; // Internal state. bool shutting_down_ = false; @@ -1777,9 +1777,9 @@ class XdsFactory : public LoadBalancingPolicyFactory { const char* name() const override { return kXds; } RefCountedPtr ParseLoadBalancingConfig( - const grpc_json* json, grpc_error** error) const override { + const Json& json, grpc_error** error) const override { GPR_DEBUG_ASSERT(error != nullptr && *error == GRPC_ERROR_NONE); - if (json == nullptr) { + if (json.type() == Json::Type::JSON_NULL) { // xds was mentioned as a policy in the deprecated loadBalancingPolicy // field or in the client API. *error = GRPC_ERROR_CREATE_FROM_STATIC_STRING( @@ -1787,61 +1787,57 @@ class XdsFactory : public LoadBalancingPolicyFactory { "Please use loadBalancingConfig field of service config instead."); return nullptr; } - GPR_DEBUG_ASSERT(strcmp(json->key, name()) == 0); - InlinedVector error_list; + std::vector error_list; + // Child policy. RefCountedPtr child_policy; + auto it = json.object_value().find("childPolicy"); + if (it != json.object_value().end()) { + grpc_error* parse_error = GRPC_ERROR_NONE; + child_policy = LoadBalancingPolicyRegistry::ParseLoadBalancingConfig( + it->second, &parse_error); + if (child_policy == nullptr) { + GPR_DEBUG_ASSERT(parse_error != GRPC_ERROR_NONE); + std::vector child_errors; + child_errors.push_back(parse_error); + error_list.push_back( + GRPC_ERROR_CREATE_FROM_VECTOR("field:childPolicy", &child_errors)); + } + } + // Fallback policy. RefCountedPtr fallback_policy; + it = json.object_value().find("fallbackPolicy"); + if (it != json.object_value().end()) { + grpc_error* parse_error = GRPC_ERROR_NONE; + fallback_policy = LoadBalancingPolicyRegistry::ParseLoadBalancingConfig( + it->second, &parse_error); + if (fallback_policy == nullptr) { + GPR_DEBUG_ASSERT(parse_error != GRPC_ERROR_NONE); + std::vector child_errors; + child_errors.push_back(parse_error); + error_list.push_back(GRPC_ERROR_CREATE_FROM_VECTOR( + "field:fallbackPolicy", &child_errors)); + } + } + // EDS service name. const char* eds_service_name = nullptr; + it = json.object_value().find("edsServiceName"); + if (it != json.object_value().end()) { + if (it->second.type() != Json::Type::STRING) { + error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "field:edsServiceName error:type should be string")); + } else { + eds_service_name = it->second.string_value().c_str(); + } + } + // LRS load reporting server name. const char* lrs_load_reporting_server_name = nullptr; - for (const grpc_json* field = json->child; field != nullptr; - field = field->next) { - if (field->key == nullptr) continue; - if (strcmp(field->key, "childPolicy") == 0) { - if (child_policy != nullptr) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:childPolicy error:Duplicate entry")); - } - grpc_error* parse_error = GRPC_ERROR_NONE; - child_policy = LoadBalancingPolicyRegistry::ParseLoadBalancingConfig( - field, &parse_error); - if (child_policy == nullptr) { - GPR_DEBUG_ASSERT(parse_error != GRPC_ERROR_NONE); - error_list.push_back(parse_error); - } - } else if (strcmp(field->key, "fallbackPolicy") == 0) { - if (fallback_policy != nullptr) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:fallbackPolicy error:Duplicate entry")); - } - grpc_error* parse_error = GRPC_ERROR_NONE; - fallback_policy = LoadBalancingPolicyRegistry::ParseLoadBalancingConfig( - field, &parse_error); - if (fallback_policy == nullptr) { - GPR_DEBUG_ASSERT(parse_error != GRPC_ERROR_NONE); - error_list.push_back(parse_error); - } - } else if (strcmp(field->key, "edsServiceName") == 0) { - if (eds_service_name != nullptr) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:edsServiceName error:Duplicate entry")); - } - if (field->type != GRPC_JSON_STRING) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:edsServiceName error:type should be string")); - continue; - } - eds_service_name = field->value; - } else if (strcmp(field->key, "lrsLoadReportingServerName") == 0) { - if (lrs_load_reporting_server_name != nullptr) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:lrsLoadReportingServerName error:Duplicate entry")); - } - if (field->type != GRPC_JSON_STRING) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:lrsLoadReportingServerName error:type should be string")); - continue; - } - lrs_load_reporting_server_name = field->value; + it = json.object_value().find("lrsLoadReportingServerName"); + if (it != json.object_value().end()) { + if (it->second.type() != Json::Type::STRING) { + error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "field:lrsLoadReportingServerName error:type should be string")); + } else { + lrs_load_reporting_server_name = it->second.string_value().c_str(); } } if (error_list.empty()) { @@ -1850,7 +1846,7 @@ class XdsFactory : public LoadBalancingPolicyFactory { optional_lrs_load_reporting_server_name.emplace( std::string(lrs_load_reporting_server_name)); } - return MakeRefCounted( + return MakeRefCounted( std::move(child_policy), std::move(fallback_policy), eds_service_name == nullptr ? "" : eds_service_name, std::move(optional_lrs_load_reporting_server_name)); diff --git a/src/core/ext/filters/client_channel/lb_policy_factory.h b/src/core/ext/filters/client_channel/lb_policy_factory.h index 3cd6d456714..9e4425fa447 100644 --- a/src/core/ext/filters/client_channel/lb_policy_factory.h +++ b/src/core/ext/filters/client_channel/lb_policy_factory.h @@ -28,6 +28,8 @@ namespace grpc_core { class LoadBalancingPolicyFactory { public: + virtual ~LoadBalancingPolicyFactory() {} + /// Returns a new LB policy instance. virtual OrphanablePtr CreateLoadBalancingPolicy( LoadBalancingPolicy::Args) const = 0; @@ -37,9 +39,7 @@ class LoadBalancingPolicyFactory { virtual const char* name() const = 0; virtual RefCountedPtr ParseLoadBalancingConfig( - const grpc_json* json, grpc_error** error) const = 0; - - virtual ~LoadBalancingPolicyFactory() {} + const Json& json, grpc_error** error) const = 0; }; } // namespace grpc_core diff --git a/src/core/ext/filters/client_channel/lb_policy_registry.cc b/src/core/ext/filters/client_channel/lb_policy_registry.cc index 857522a4491..79e5f2f4926 100644 --- a/src/core/ext/filters/client_channel/lb_policy_registry.cc +++ b/src/core/ext/filters/client_channel/lb_policy_registry.cc @@ -105,106 +105,74 @@ bool LoadBalancingPolicyRegistry::LoadBalancingPolicyExists( grpc_error* error = GRPC_ERROR_NONE; // Check if the load balancing policy allows an empty config *requires_config = - factory->ParseLoadBalancingConfig(nullptr, &error) == nullptr; + factory->ParseLoadBalancingConfig(Json(), &error) == nullptr; GRPC_ERROR_UNREF(error); } return true; } namespace { + // Returns the JSON node of policy (with both policy name and config content) // given the JSON node of a LoadBalancingConfig array. -grpc_json* ParseLoadBalancingConfigHelper(const grpc_json* lb_config_array, - grpc_error** error) { - GPR_DEBUG_ASSERT(error != nullptr && *error == GRPC_ERROR_NONE); - if (lb_config_array == nullptr) { - *error = - GRPC_ERROR_CREATE_FROM_STATIC_STRING("LB config JSON tree is null"); - return nullptr; +grpc_error* ParseLoadBalancingConfigHelper( + const Json& lb_config_array, Json::Object::const_iterator* result) { + if (lb_config_array.type() != Json::Type::ARRAY) { + return GRPC_ERROR_CREATE_FROM_STATIC_STRING("type should be array"); } - char* error_msg; - if (lb_config_array->type != GRPC_JSON_ARRAY) { - gpr_asprintf(&error_msg, "field:%s error:type should be array", - lb_config_array->key); - *error = GRPC_ERROR_CREATE_FROM_COPIED_STRING(error_msg); - gpr_free(error_msg); - return nullptr; - } - const char* field_name = lb_config_array->key; // Find the first LB policy that this client supports. - for (const grpc_json* lb_config = lb_config_array->child; - lb_config != nullptr; lb_config = lb_config->next) { - if (lb_config->type != GRPC_JSON_OBJECT) { - gpr_asprintf(&error_msg, - "field:%s error:child entry should be of type object", - field_name); - *error = GRPC_ERROR_CREATE_FROM_COPIED_STRING(error_msg); - gpr_free(error_msg); - return nullptr; + for (const Json& lb_config : lb_config_array.array_value()) { + if (lb_config.type() != Json::Type::OBJECT) { + return GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "child entry should be of type object"); } - grpc_json* policy = nullptr; - for (grpc_json* field = lb_config->child; field != nullptr; - field = field->next) { - if (field->key == nullptr || field->type != GRPC_JSON_OBJECT) { - gpr_asprintf(&error_msg, - "field:%s error:child entry should be of type object", - field_name); - *error = GRPC_ERROR_CREATE_FROM_COPIED_STRING(error_msg); - gpr_free(error_msg); - return nullptr; - } - if (policy != nullptr) { - gpr_asprintf(&error_msg, "field:%s error:oneOf violation", field_name); - *error = GRPC_ERROR_CREATE_FROM_COPIED_STRING(error_msg); - gpr_free(error_msg); - return nullptr; - } // Violate "oneof" type. - policy = field; + if (lb_config.object_value().empty()) { + return GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "no policy found in child entry"); + } + if (lb_config.object_value().size() > 1) { + return GRPC_ERROR_CREATE_FROM_STATIC_STRING("oneOf violation"); } - if (policy == nullptr) { - gpr_asprintf(&error_msg, "field:%s error:no policy found in child entry", - field_name); - *error = GRPC_ERROR_CREATE_FROM_COPIED_STRING(error_msg); - gpr_free(error_msg); - return nullptr; + auto it = lb_config.object_value().begin(); + if (it->second.type() != Json::Type::OBJECT) { + return GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "child entry should be of type object"); } // If we support this policy, then select it. - if (LoadBalancingPolicyRegistry::LoadBalancingPolicyExists(policy->key, - nullptr)) { - return policy; + if (LoadBalancingPolicyRegistry::LoadBalancingPolicyExists( + it->first.c_str(), nullptr)) { + *result = it; + return GRPC_ERROR_NONE; } } - gpr_asprintf(&error_msg, "field:%s error:No known policy", field_name); - *error = GRPC_ERROR_CREATE_FROM_COPIED_STRING(error_msg); - gpr_free(error_msg); - return nullptr; + return GRPC_ERROR_CREATE_FROM_STATIC_STRING("No known policy"); } + } // namespace RefCountedPtr -LoadBalancingPolicyRegistry::ParseLoadBalancingConfig(const grpc_json* json, +LoadBalancingPolicyRegistry::ParseLoadBalancingConfig(const Json& json, grpc_error** error) { GPR_DEBUG_ASSERT(error != nullptr && *error == GRPC_ERROR_NONE); GPR_ASSERT(g_state != nullptr); - const grpc_json* policy = ParseLoadBalancingConfigHelper(json, error); - if (policy == nullptr) { + Json::Object::const_iterator policy; + *error = ParseLoadBalancingConfigHelper(json, &policy); + if (*error != GRPC_ERROR_NONE) { + return nullptr; + } + // Find factory. + LoadBalancingPolicyFactory* factory = + g_state->GetLoadBalancingPolicyFactory(policy->first.c_str()); + if (factory == nullptr) { + char* msg; + gpr_asprintf(&msg, "Factory not found for policy \"%s\"", + policy->first.c_str()); + *error = GRPC_ERROR_CREATE_FROM_COPIED_STRING(msg); + gpr_free(msg); return nullptr; - } else { - GPR_DEBUG_ASSERT(*error == GRPC_ERROR_NONE && json != nullptr); - // Find factory. - LoadBalancingPolicyFactory* factory = - g_state->GetLoadBalancingPolicyFactory(policy->key); - if (factory == nullptr) { - char* msg; - gpr_asprintf(&msg, "field:%s error:Factory not found to create policy", - json->key); - *error = GRPC_ERROR_CREATE_FROM_COPIED_STRING(msg); - gpr_free(msg); - return nullptr; - } - // Parse load balancing config via factory. - return factory->ParseLoadBalancingConfig(policy, error); } + // Parse load balancing config via factory. + return factory->ParseLoadBalancingConfig(policy->second, error); } } // namespace grpc_core diff --git a/src/core/ext/filters/client_channel/lb_policy_registry.h b/src/core/ext/filters/client_channel/lb_policy_registry.h index d6d9bc305fd..8d2e3e395d5 100644 --- a/src/core/ext/filters/client_channel/lb_policy_registry.h +++ b/src/core/ext/filters/client_channel/lb_policy_registry.h @@ -57,7 +57,7 @@ class LoadBalancingPolicyRegistry { /// Returns a parsed object of the load balancing policy to be used from a /// LoadBalancingConfig array \a json. static RefCountedPtr ParseLoadBalancingConfig( - const grpc_json* json, grpc_error** error); + const Json& json, grpc_error** error); }; } // namespace grpc_core diff --git a/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc b/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc index 3f281537e2d..7f8ccafc0f2 100644 --- a/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc +++ b/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc @@ -223,104 +223,92 @@ void AresDnsResolver::OnNextResolutionLocked(void* arg, grpc_error* error) { r->Unref(DEBUG_LOCATION, "next_resolution_timer"); } -bool ValueInJsonArray(grpc_json* array, const char* value) { - for (grpc_json* entry = array->child; entry != nullptr; entry = entry->next) { - if (entry->type == GRPC_JSON_STRING && strcmp(entry->value, value) == 0) { +bool ValueInJsonArray(const Json::Array& array, const char* value) { + for (const Json& entry : array) { + if (entry.type() == Json::Type::STRING && entry.string_value() == value) { return true; } } return false; } -char* ChooseServiceConfig(char* service_config_choice_json, - grpc_error** error) { - grpc_json* choices_json = grpc_json_parse_string(service_config_choice_json); - if (choices_json == nullptr) { - *error = GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "Service Config JSON Parsing, error: could not parse"); - return nullptr; - } - if (choices_json->type != GRPC_JSON_ARRAY) { +std::string ChooseServiceConfig(char* service_config_choice_json, + grpc_error** error) { + Json json = Json::Parse(service_config_choice_json, error); + if (*error != GRPC_ERROR_NONE) return ""; + if (json.type() != Json::Type::ARRAY) { *error = GRPC_ERROR_CREATE_FROM_STATIC_STRING( "Service Config Choices, error: should be of type array"); - return nullptr; + return ""; } - char* service_config = nullptr; + const Json* service_config = nullptr; InlinedVector error_list; - bool found_choice = false; // have we found a choice? - for (grpc_json* choice = choices_json->child; choice != nullptr; - choice = choice->next) { - if (choice->type != GRPC_JSON_OBJECT) { + for (const Json& choice : json.array_value()) { + if (choice.type() != Json::Type::OBJECT) { error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( "Service Config Choice, error: should be of type object")); continue; } - grpc_json* service_config_json = nullptr; - bool selected = true; // has this choice been rejected? - for (grpc_json* field = choice->child; field != nullptr; - field = field->next) { - // Check client language, if specified. - if (strcmp(field->key, "clientLanguage") == 0) { - if (field->type != GRPC_JSON_ARRAY) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:clientLanguage error:should be of type array")); - } else if (!ValueInJsonArray(field, "c++")) { - selected = false; - } + // Check client language, if specified. + auto it = choice.object_value().find("clientLanguage"); + if (it != choice.object_value().end()) { + if (it->second.type() != Json::Type::ARRAY) { + error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "field:clientLanguage error:should be of type array")); + } else if (!ValueInJsonArray(it->second.array_value(), "c++")) { + continue; } - // Check client hostname, if specified. - if (strcmp(field->key, "clientHostname") == 0) { - if (field->type != GRPC_JSON_ARRAY) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:clientHostname error:should be of type array")); - continue; - } + } + // Check client hostname, if specified. + it = choice.object_value().find("clientHostname"); + if (it != choice.object_value().end()) { + if (it->second.type() != Json::Type::ARRAY) { + error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "field:clientHostname error:should be of type array")); + } else { char* hostname = grpc_gethostname(); - if (hostname == nullptr || !ValueInJsonArray(field, hostname)) { - selected = false; - } - } - // Check percentage, if specified. - if (strcmp(field->key, "percentage") == 0) { - if (field->type != GRPC_JSON_NUMBER) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:percentage error:should be of type number")); + if (hostname == nullptr || + !ValueInJsonArray(it->second.array_value(), hostname)) { continue; } + } + } + // Check percentage, if specified. + it = choice.object_value().find("percentage"); + if (it != choice.object_value().end()) { + if (it->second.type() != Json::Type::NUMBER) { + error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "field:percentage error:should be of type number")); + } else { int random_pct = rand() % 100; int percentage; - if (sscanf(field->value, "%d", &percentage) != 1) { + if (sscanf(it->second.string_value().c_str(), "%d", &percentage) != 1) { error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( "field:percentage error:should be of type integer")); + } else if (random_pct > percentage || percentage == 0) { continue; } - if (random_pct > percentage || percentage == 0) { - selected = false; - } - } - // Save service config. - if (strcmp(field->key, "serviceConfig") == 0) { - if (field->type == GRPC_JSON_OBJECT) { - service_config_json = field; - } else { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:serviceConfig error:should be of type object")); - } } } - if (!found_choice && selected && service_config_json != nullptr) { - service_config = grpc_json_dump_to_string(service_config_json, 0); - found_choice = true; + // Found service config. + it = choice.object_value().find("serviceConfig"); + if (it == choice.object_value().end()) { + error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "field:serviceConfig error:required field missing")); + } else if (it->second.type() != Json::Type::OBJECT) { + error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "field:serviceConfig error:should be of type object")); + } else if (service_config == nullptr) { + service_config = &it->second; } } - grpc_json_destroy(choices_json); if (!error_list.empty()) { - gpr_free(service_config); service_config = nullptr; *error = GRPC_ERROR_CREATE_FROM_VECTOR("Service Config Choices Parser", &error_list); } - return service_config; + if (service_config == nullptr) return ""; + return service_config->Dump(); } void AresDnsResolver::OnResolved(void* arg, grpc_error* error) { @@ -344,17 +332,16 @@ void AresDnsResolver::OnResolvedLocked(void* arg, grpc_error* error) { Result result; result.addresses = std::move(*r->addresses_); if (r->service_config_json_ != nullptr) { - char* service_config_string = ChooseServiceConfig( + std::string service_config_string = ChooseServiceConfig( r->service_config_json_, &result.service_config_error); gpr_free(r->service_config_json_); if (result.service_config_error == GRPC_ERROR_NONE && - service_config_string != nullptr) { + !service_config_string.empty()) { GRPC_CARES_TRACE_LOG("resolver:%p selected service config choice: %s", - r, service_config_string); + r, service_config_string.c_str()); result.service_config = ServiceConfig::Create( service_config_string, &result.service_config_error); } - gpr_free(service_config_string); } result.args = grpc_channel_args_copy(r->channel_args_); r->result_handler()->ReturnResult(std::move(result)); diff --git a/src/core/ext/filters/client_channel/resolver_result_parsing.cc b/src/core/ext/filters/client_channel/resolver_result_parsing.cc index 92b60a2e1f9..8b9f9ec7c74 100644 --- a/src/core/ext/filters/client_channel/resolver_result_parsing.cc +++ b/src/core/ext/filters/client_channel/resolver_result_parsing.cc @@ -62,11 +62,11 @@ namespace { // Parses a JSON field of the form generated for a google.proto.Duration // proto message, as per: // https://developers.google.com/protocol-buffers/docs/proto3#json -bool ParseDuration(grpc_json* field, grpc_millis* duration) { - if (field->type != GRPC_JSON_STRING) return false; - size_t len = strlen(field->value); - if (field->value[len - 1] != 's') return false; - grpc_core::UniquePtr buf(gpr_strdup(field->value)); +bool ParseDuration(const Json& field, grpc_millis* duration) { + if (field.type() != Json::Type::STRING) return false; + size_t len = field.string_value().size(); + if (field.string_value()[len - 1] != 's') return false; + grpc_core::UniquePtr buf(gpr_strdup(field.string_value().c_str())); *(buf.get() + len - 1) = '\0'; // Remove trailing 's'. char* decimal_point = strchr(buf.get(), '.'); int nanos = 0; @@ -92,109 +92,92 @@ bool ParseDuration(grpc_json* field, grpc_millis* duration) { } std::unique_ptr ParseRetryPolicy( - grpc_json* field, grpc_error** error) { + const Json& json, grpc_error** error) { GPR_DEBUG_ASSERT(error != nullptr && *error == GRPC_ERROR_NONE); auto retry_policy = grpc_core::MakeUnique(); - if (field->type != GRPC_JSON_OBJECT) { + if (json.type() != Json::Type::OBJECT) { *error = GRPC_ERROR_CREATE_FROM_STATIC_STRING( "field:retryPolicy error:should be of type object"); return nullptr; } - InlinedVector error_list; - for (grpc_json* sub_field = field->child; sub_field != nullptr; - sub_field = sub_field->next) { - if (sub_field->key == nullptr) continue; - if (strcmp(sub_field->key, "maxAttempts") == 0) { - if (retry_policy->max_attempts != 0) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:maxAttempts error:Duplicate entry")); - } // Duplicate. Continue Parsing - if (sub_field->type != GRPC_JSON_NUMBER) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:maxAttempts error:should be of type number")); - continue; - } - retry_policy->max_attempts = gpr_parse_nonnegative_int(sub_field->value); + std::vector error_list; + // Parse maxAttempts. + auto it = json.object_value().find("maxAttempts"); + if (it != json.object_value().end()) { + if (it->second.type() != Json::Type::NUMBER) { + error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "field:maxAttempts error:should be of type number")); + } else { + retry_policy->max_attempts = + gpr_parse_nonnegative_int(it->second.string_value().c_str()); if (retry_policy->max_attempts <= 1) { error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( "field:maxAttempts error:should be at least 2")); - continue; - } - if (retry_policy->max_attempts > MAX_MAX_RETRY_ATTEMPTS) { + } else if (retry_policy->max_attempts > MAX_MAX_RETRY_ATTEMPTS) { gpr_log(GPR_ERROR, "service config: clamped retryPolicy.maxAttempts at %d", MAX_MAX_RETRY_ATTEMPTS); retry_policy->max_attempts = MAX_MAX_RETRY_ATTEMPTS; } - } else if (strcmp(sub_field->key, "initialBackoff") == 0) { - if (retry_policy->initial_backoff > 0) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:initialBackoff error:Duplicate entry")); - } // Duplicate, continue parsing. - if (!ParseDuration(sub_field, &retry_policy->initial_backoff)) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:initialBackoff error:Failed to parse")); - continue; - } - if (retry_policy->initial_backoff == 0) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:initialBackoff error:must be greater than 0")); - } - } else if (strcmp(sub_field->key, "maxBackoff") == 0) { - if (retry_policy->max_backoff > 0) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:maxBackoff error:Duplicate entry")); - } // Duplicate, continue parsing. - if (!ParseDuration(sub_field, &retry_policy->max_backoff)) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:maxBackoff error:failed to parse")); - continue; - } - if (retry_policy->max_backoff == 0) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:maxBackoff error:should be greater than 0")); - } - } else if (strcmp(sub_field->key, "backoffMultiplier") == 0) { - if (retry_policy->backoff_multiplier != 0) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:backoffMultiplier error:Duplicate entry")); - } // Duplicate, continue parsing. - if (sub_field->type != GRPC_JSON_NUMBER) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:backoffMultiplier error:should be of type number")); - continue; - } - if (sscanf(sub_field->value, "%f", &retry_policy->backoff_multiplier) != - 1) { + } + } + // Parse initialBackoff. + it = json.object_value().find("initialBackoff"); + if (it != json.object_value().end()) { + if (!ParseDuration(it->second, &retry_policy->initial_backoff)) { + error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "field:initialBackoff error:Failed to parse")); + } else if (retry_policy->initial_backoff == 0) { + error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "field:initialBackoff error:must be greater than 0")); + } + } + // Parse maxBackoff. + it = json.object_value().find("maxBackoff"); + if (it != json.object_value().end()) { + if (!ParseDuration(it->second, &retry_policy->max_backoff)) { + error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "field:maxBackoff error:failed to parse")); + } else if (retry_policy->max_backoff == 0) { + error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "field:maxBackoff error:should be greater than 0")); + } + } + // Parse backoffMultiplier. + it = json.object_value().find("backoffMultiplier"); + if (it != json.object_value().end()) { + if (it->second.type() != Json::Type::NUMBER) { + error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "field:backoffMultiplier error:should be of type number")); + } else { + if (sscanf(it->second.string_value().c_str(), "%f", + &retry_policy->backoff_multiplier) != 1) { error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( "field:backoffMultiplier error:failed to parse")); - continue; - } - if (retry_policy->backoff_multiplier <= 0) { + } else if (retry_policy->backoff_multiplier <= 0) { error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( "field:backoffMultiplier error:should be greater than 0")); } - } else if (strcmp(sub_field->key, "retryableStatusCodes") == 0) { - if (!retry_policy->retryable_status_codes.Empty()) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:retryableStatusCodes error:Duplicate entry")); - } // Duplicate, continue parsing. - if (sub_field->type != GRPC_JSON_ARRAY) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:retryableStatusCodes error:should be of type array")); - continue; - } - for (grpc_json* element = sub_field->child; element != nullptr; - element = element->next) { - if (element->type != GRPC_JSON_STRING) { + } + } + // Parse retryableStatusCodes. + it = json.object_value().find("retryableStatusCodes"); + if (it != json.object_value().end()) { + if (it->second.type() != Json::Type::ARRAY) { + error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "field:retryableStatusCodes error:should be of type array")); + } else { + for (const Json& element : it->second.array_value()) { + if (element.type() != Json::Type::STRING) { error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( "field:retryableStatusCodes error:status codes should be of type " "string")); continue; } grpc_status_code status; - if (!grpc_status_code_from_string(element->value, &status)) { + if (!grpc_status_code_from_string(element.string_value().c_str(), + &status)) { error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( "field:retryableStatusCodes error:failed to parse status code")); continue; @@ -222,34 +205,100 @@ std::unique_ptr ParseRetryPolicy( return *error == GRPC_ERROR_NONE ? std::move(retry_policy) : nullptr; } -const char* ParseHealthCheckConfig(const grpc_json* field, grpc_error** error) { +grpc_error* ParseRetryThrottling( + const Json& json, + ClientChannelGlobalParsedConfig::RetryThrottling* retry_throttling) { + if (json.type() != Json::Type::OBJECT) { + return GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "field:retryThrottling error:Type should be object"); + } + std::vector error_list; + // Parse maxTokens. + auto it = json.object_value().find("maxTokens"); + if (it == json.object_value().end()) { + error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "field:retryThrottling field:maxTokens error:Not found")); + } else if (it->second.type() != Json::Type::NUMBER) { + error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "field:retryThrottling field:maxTokens error:Type should be " + "number")); + } else { + retry_throttling->max_milli_tokens = + gpr_parse_nonnegative_int(it->second.string_value().c_str()) * 1000; + if (retry_throttling->max_milli_tokens <= 0) { + error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "field:retryThrottling field:maxTokens error:should be " + "greater than zero")); + } + } + // Parse tokenRatio. + it = json.object_value().find("tokenRatio"); + if (it == json.object_value().end()) { + error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "field:retryThrottling field:tokenRatio error:Not found")); + } else if (it->second.type() != Json::Type::NUMBER) { + error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "field:retryThrottling field:tokenRatio error:type should be " + "number")); + } else { + // We support up to 3 decimal digits. + size_t whole_len = it->second.string_value().size(); + const char* value = it->second.string_value().c_str(); + uint32_t multiplier = 1; + uint32_t decimal_value = 0; + const char* decimal_point = strchr(value, '.'); + if (decimal_point != nullptr) { + whole_len = static_cast(decimal_point - value); + multiplier = 1000; + size_t decimal_len = strlen(decimal_point + 1); + if (decimal_len > 3) decimal_len = 3; + if (!gpr_parse_bytes_to_uint32(decimal_point + 1, decimal_len, + &decimal_value)) { + error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "field:retryThrottling field:tokenRatio error:Failed " + "parsing")); + return GRPC_ERROR_CREATE_FROM_VECTOR("retryPolicy", &error_list); + } + uint32_t decimal_multiplier = 1; + for (size_t i = 0; i < (3 - decimal_len); ++i) { + decimal_multiplier *= 10; + } + decimal_value *= decimal_multiplier; + } + uint32_t whole_value; + if (!gpr_parse_bytes_to_uint32(value, whole_len, &whole_value)) { + error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "field:retryThrottling field:tokenRatio error:Failed " + "parsing")); + return GRPC_ERROR_CREATE_FROM_VECTOR("retryPolicy", &error_list); + } + retry_throttling->milli_token_ratio = + static_cast((whole_value * multiplier) + decimal_value); + if (retry_throttling->milli_token_ratio <= 0) { + error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "field:retryThrottling field:tokenRatio error:value should " + "be greater than 0")); + } + } + return GRPC_ERROR_CREATE_FROM_VECTOR("retryPolicy", &error_list); +} + +const char* ParseHealthCheckConfig(const Json& field, grpc_error** error) { GPR_DEBUG_ASSERT(error != nullptr && *error == GRPC_ERROR_NONE); const char* service_name = nullptr; - GPR_DEBUG_ASSERT(strcmp(field->key, "healthCheckConfig") == 0); - if (field->type != GRPC_JSON_OBJECT) { + if (field.type() != Json::Type::OBJECT) { *error = GRPC_ERROR_CREATE_FROM_STATIC_STRING( "field:healthCheckConfig error:should be of type object"); return nullptr; } - InlinedVector error_list; - for (grpc_json* sub_field = field->child; sub_field != nullptr; - sub_field = sub_field->next) { - if (sub_field->key == nullptr) { - GPR_DEBUG_ASSERT(false); - continue; - } - if (strcmp(sub_field->key, "serviceName") == 0) { - if (service_name != nullptr) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:serviceName error:Duplicate " - "entry")); - } // Duplicate. Continue parsing - if (sub_field->type != GRPC_JSON_STRING) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:serviceName error:should be of type string")); - continue; - } - service_name = sub_field->value; + std::vector error_list; + auto it = field.object_value().find("serviceName"); + if (it != field.object_value().end()) { + if (it->second.type() != Json::Type::STRING) { + error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "field:serviceName error:should be of type string")); + } else { + service_name = it->second.string_value().c_str(); } } if (!error_list.empty()) { @@ -263,43 +312,35 @@ const char* ParseHealthCheckConfig(const grpc_json* field, grpc_error** error) { } // namespace std::unique_ptr -ClientChannelServiceConfigParser::ParseGlobalParams(const grpc_json* json, +ClientChannelServiceConfigParser::ParseGlobalParams(const Json& json, grpc_error** error) { GPR_DEBUG_ASSERT(error != nullptr && *error == GRPC_ERROR_NONE); - InlinedVector error_list; + std::vector error_list; RefCountedPtr parsed_lb_config; grpc_core::UniquePtr lb_policy_name; Optional retry_throttling; const char* health_check_service_name = nullptr; - for (grpc_json* field = json->child; field != nullptr; field = field->next) { - if (field->key == nullptr) { - continue; // Not the LB config global parameter + // Parse LB config. + auto it = json.object_value().find("loadBalancingConfig"); + if (it != json.object_value().end()) { + grpc_error* parse_error = GRPC_ERROR_NONE; + parsed_lb_config = LoadBalancingPolicyRegistry::ParseLoadBalancingConfig( + it->second, &parse_error); + if (parsed_lb_config == nullptr) { + std::vector lb_errors; + lb_errors.push_back(parse_error); + error_list.push_back(GRPC_ERROR_CREATE_FROM_VECTOR( + "field:loadBalancingConfig", &lb_errors)); } - // Parsed Load balancing config - if (strcmp(field->key, "loadBalancingConfig") == 0) { - if (parsed_lb_config != nullptr) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:loadBalancingConfig error:Duplicate entry")); - } // Duplicate, continue parsing. - grpc_error* parse_error = GRPC_ERROR_NONE; - parsed_lb_config = LoadBalancingPolicyRegistry::ParseLoadBalancingConfig( - field, &parse_error); - if (parsed_lb_config == nullptr) { - error_list.push_back(parse_error); - } - } - // Parse deprecated loadBalancingPolicy - if (strcmp(field->key, "loadBalancingPolicy") == 0) { - if (lb_policy_name != nullptr) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:loadBalancingPolicy error:Duplicate entry")); - } // Duplicate, continue parsing. - if (field->type != GRPC_JSON_STRING) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:loadBalancingPolicy error:type should be string")); - continue; - } - lb_policy_name.reset(gpr_strdup(field->value)); + } + // Parse deprecated LB policy. + it = json.object_value().find("loadBalancingPolicy"); + if (it != json.object_value().end()) { + if (it->second.type() != Json::Type::STRING) { + error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "field:loadBalancingPolicy error:type should be string")); + } else { + lb_policy_name.reset(gpr_strdup(it->second.string_value().c_str())); char* lb_policy = lb_policy_name.get(); if (lb_policy != nullptr) { for (size_t i = 0; i < strlen(lb_policy); ++i) { @@ -321,118 +362,26 @@ ClientChannelServiceConfigParser::ParseGlobalParams(const grpc_json* json, gpr_free(error_msg); } } - // Parse retry throttling - if (strcmp(field->key, "retryThrottling") == 0) { - if (retry_throttling.has_value()) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:retryThrottling error:Duplicate entry")); - } // Duplicate, continue parsing. - if (field->type != GRPC_JSON_OBJECT) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:retryThrottling error:Type should be object")); - continue; - } - Optional max_milli_tokens; - Optional milli_token_ratio; - for (grpc_json* sub_field = field->child; sub_field != nullptr; - sub_field = sub_field->next) { - if (sub_field->key == nullptr) continue; - if (strcmp(sub_field->key, "maxTokens") == 0) { - if (max_milli_tokens.has_value()) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:retryThrottling field:maxTokens error:Duplicate " - "entry")); - } // Duplicate, continue parsing. - if (sub_field->type != GRPC_JSON_NUMBER) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:retryThrottling field:maxTokens error:Type should be " - "number")); - } else { - max_milli_tokens.emplace( - gpr_parse_nonnegative_int(sub_field->value) * 1000); - if (max_milli_tokens.value() <= 0) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:retryThrottling field:maxTokens error:should be " - "greater than zero")); - } - } - } else if (strcmp(sub_field->key, "tokenRatio") == 0) { - if (milli_token_ratio.has_value()) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:retryThrottling field:tokenRatio error:Duplicate " - "entry")); - } // Duplicate, continue parsing. - if (sub_field->type != GRPC_JSON_NUMBER) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:retryThrottling field:tokenRatio error:type should be " - "number")); - } else { - // We support up to 3 decimal digits. - size_t whole_len = strlen(sub_field->value); - uint32_t multiplier = 1; - uint32_t decimal_value = 0; - const char* decimal_point = strchr(sub_field->value, '.'); - if (decimal_point != nullptr) { - whole_len = static_cast(decimal_point - sub_field->value); - multiplier = 1000; - size_t decimal_len = strlen(decimal_point + 1); - if (decimal_len > 3) decimal_len = 3; - if (!gpr_parse_bytes_to_uint32(decimal_point + 1, decimal_len, - &decimal_value)) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:retryThrottling field:tokenRatio error:Failed " - "parsing")); - continue; - } - uint32_t decimal_multiplier = 1; - for (size_t i = 0; i < (3 - decimal_len); ++i) { - decimal_multiplier *= 10; - } - decimal_value *= decimal_multiplier; - } - uint32_t whole_value; - if (!gpr_parse_bytes_to_uint32(sub_field->value, whole_len, - &whole_value)) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:retryThrottling field:tokenRatio error:Failed " - "parsing")); - continue; - } - milli_token_ratio.emplace( - static_cast((whole_value * multiplier) + decimal_value)); - if (milli_token_ratio.value() <= 0) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:retryThrottling field:tokenRatio error:value should " - "be greater than 0")); - } - } - } - } - ClientChannelGlobalParsedConfig::RetryThrottling data; - if (!max_milli_tokens.has_value()) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:retryThrottling field:maxTokens error:Not found")); - } else { - data.max_milli_tokens = max_milli_tokens.value(); - } - if (!milli_token_ratio.has_value()) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:retryThrottling field:tokenRatio error:Not found")); - } else { - data.milli_token_ratio = milli_token_ratio.value(); - } + } + // Parse retry throttling. + it = json.object_value().find("retryThrottling"); + if (it != json.object_value().end()) { + ClientChannelGlobalParsedConfig::RetryThrottling data; + grpc_error* parsing_error = ParseRetryThrottling(it->second, &data); + if (parsing_error != GRPC_ERROR_NONE) { + error_list.push_back(parsing_error); + } else { retry_throttling.emplace(data); } - if (strcmp(field->key, "healthCheckConfig") == 0) { - if (health_check_service_name != nullptr) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:healthCheckConfig error:Duplicate entry")); - } // Duplicate continue parsing - grpc_error* parsing_error = GRPC_ERROR_NONE; - health_check_service_name = ParseHealthCheckConfig(field, &parsing_error); - if (parsing_error != GRPC_ERROR_NONE) { - error_list.push_back(parsing_error); - } + } + // Parse health check config. + it = json.object_value().find("healthCheckConfig"); + if (it != json.object_value().end()) { + grpc_error* parsing_error = GRPC_ERROR_NONE; + health_check_service_name = + ParseHealthCheckConfig(it->second, &parsing_error); + if (parsing_error != GRPC_ERROR_NONE) { + error_list.push_back(parsing_error); } } *error = GRPC_ERROR_CREATE_FROM_VECTOR("Client channel global parser", @@ -446,47 +395,40 @@ ClientChannelServiceConfigParser::ParseGlobalParams(const grpc_json* json, } std::unique_ptr -ClientChannelServiceConfigParser::ParsePerMethodParams(const grpc_json* json, +ClientChannelServiceConfigParser::ParsePerMethodParams(const Json& json, grpc_error** error) { GPR_DEBUG_ASSERT(error != nullptr && *error == GRPC_ERROR_NONE); - InlinedVector error_list; + std::vector error_list; Optional wait_for_ready; grpc_millis timeout = 0; std::unique_ptr retry_policy; - for (grpc_json* field = json->child; field != nullptr; field = field->next) { - if (field->key == nullptr) continue; - if (strcmp(field->key, "waitForReady") == 0) { - if (wait_for_ready.has_value()) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:waitForReady error:Duplicate entry")); - } // Duplicate, continue parsing. - if (field->type == GRPC_JSON_TRUE) { - wait_for_ready.emplace(true); - } else if (field->type == GRPC_JSON_FALSE) { - wait_for_ready.emplace(false); - } else { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:waitForReady error:Type should be true/false")); - } - } else if (strcmp(field->key, "timeout") == 0) { - if (timeout > 0) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:timeout error:Duplicate entry")); - } // Duplicate, continue parsing. - if (!ParseDuration(field, &timeout)) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:timeout error:Failed parsing")); - }; - } else if (strcmp(field->key, "retryPolicy") == 0) { - if (retry_policy != nullptr) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:retryPolicy error:Duplicate entry")); - } // Duplicate, continue parsing. - grpc_error* error = GRPC_ERROR_NONE; - retry_policy = ParseRetryPolicy(field, &error); - if (retry_policy == nullptr) { - error_list.push_back(error); - } + // Parse waitForReady. + auto it = json.object_value().find("waitForReady"); + if (it != json.object_value().end()) { + if (it->second.type() == Json::Type::JSON_TRUE) { + wait_for_ready.emplace(true); + } else if (it->second.type() == Json::Type::JSON_FALSE) { + wait_for_ready.emplace(false); + } else { + error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "field:waitForReady error:Type should be true/false")); + } + } + // Parse timeout. + it = json.object_value().find("timeout"); + if (it != json.object_value().end()) { + if (!ParseDuration(it->second, &timeout)) { + error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "field:timeout error:Failed parsing")); + }; + } + // Parse retry policy. + it = json.object_value().find("retryPolicy"); + if (it != json.object_value().end()) { + grpc_error* error = GRPC_ERROR_NONE; + retry_policy = ParseRetryPolicy(it->second, &error); + if (retry_policy == nullptr) { + error_list.push_back(error); } } *error = GRPC_ERROR_CREATE_FROM_VECTOR("Client channel parser", &error_list); diff --git a/src/core/ext/filters/client_channel/resolver_result_parsing.h b/src/core/ext/filters/client_channel/resolver_result_parsing.h index cc68866897b..27d7f6f1396 100644 --- a/src/core/ext/filters/client_channel/resolver_result_parsing.h +++ b/src/core/ext/filters/client_channel/resolver_result_parsing.h @@ -109,10 +109,10 @@ class ClientChannelMethodParsedConfig : public ServiceConfig::ParsedConfig { class ClientChannelServiceConfigParser : public ServiceConfig::Parser { public: std::unique_ptr ParseGlobalParams( - const grpc_json* json, grpc_error** error) override; + const Json& json, grpc_error** error) override; std::unique_ptr ParsePerMethodParams( - const grpc_json* json, grpc_error** error) override; + const Json& json, grpc_error** error) override; static size_t ParserIndex(); static void Register(); diff --git a/src/core/ext/filters/client_channel/service_config.cc b/src/core/ext/filters/client_channel/service_config.cc index 69e9d0a1870..8f240bfeb6b 100644 --- a/src/core/ext/filters/client_channel/service_config.cc +++ b/src/core/ext/filters/client_channel/service_config.cc @@ -40,37 +40,29 @@ typedef InlinedVector, ServiceConfigParserList* g_registered_parsers; } // namespace -RefCountedPtr ServiceConfig::Create(const char* json, +RefCountedPtr ServiceConfig::Create(StringView json_string, grpc_error** error) { - grpc_core::UniquePtr service_config_json(gpr_strdup(json)); - grpc_core::UniquePtr json_string(gpr_strdup(json)); GPR_DEBUG_ASSERT(error != nullptr); - grpc_json* json_tree = grpc_json_parse_string(json_string.get()); - if (json_tree == nullptr) { - *error = GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "failed to parse JSON for service config"); - return nullptr; - } + Json json = Json::Parse(json_string, error); + if (*error != GRPC_ERROR_NONE) return nullptr; return MakeRefCounted( - std::move(service_config_json), std::move(json_string), json_tree, error); + std::string(json_string.data(), json_string.size()), std::move(json), + error); } -ServiceConfig::ServiceConfig(grpc_core::UniquePtr service_config_json, - grpc_core::UniquePtr json_string, - grpc_json* json_tree, grpc_error** error) - : service_config_json_(std::move(service_config_json)), - json_string_(std::move(json_string)), - json_tree_(json_tree) { +ServiceConfig::ServiceConfig(std::string json_string, Json json, + grpc_error** error) + : json_string_(std::move(json_string)), json_(std::move(json)) { GPR_DEBUG_ASSERT(error != nullptr); - if (json_tree->type != GRPC_JSON_OBJECT || json_tree->key != nullptr) { - *error = GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "Malformed service Config JSON object"); + if (json_.type() != Json::Type::OBJECT) { + *error = + GRPC_ERROR_CREATE_FROM_STATIC_STRING("JSON value is not an object"); return; } grpc_error* error_list[2]; int error_count = 0; - grpc_error* global_error = ParseGlobalParams(json_tree); - grpc_error* local_error = ParsePerMethodParams(json_tree); + grpc_error* global_error = ParseGlobalParams(); + grpc_error* local_error = ParsePerMethodParams(); if (global_error != GRPC_ERROR_NONE) { error_list[error_count++] = global_error; } @@ -85,14 +77,12 @@ ServiceConfig::ServiceConfig(grpc_core::UniquePtr service_config_json, } } -grpc_error* ServiceConfig::ParseGlobalParams(const grpc_json* json_tree) { - GPR_DEBUG_ASSERT(json_tree_->type == GRPC_JSON_OBJECT); - GPR_DEBUG_ASSERT(json_tree_->key == nullptr); - InlinedVector error_list; +grpc_error* ServiceConfig::ParseGlobalParams() { + std::vector error_list; for (size_t i = 0; i < g_registered_parsers->size(); i++) { grpc_error* parser_error = GRPC_ERROR_NONE; auto parsed_obj = - (*g_registered_parsers)[i]->ParseGlobalParams(json_tree, &parser_error); + (*g_registered_parsers)[i]->ParseGlobalParams(json_, &parser_error); if (parser_error != GRPC_ERROR_NONE) { error_list.push_back(parser_error); } @@ -102,8 +92,9 @@ grpc_error* ServiceConfig::ParseGlobalParams(const grpc_json* json_tree) { } grpc_error* ServiceConfig::ParseJsonMethodConfigToServiceConfigVectorTable( - const grpc_json* json, - SliceHashTable::Entry* entries, size_t* idx) { + const Json& json, + InlinedVector::Entry, 10>* + entries) { auto objs_vector = grpc_core::MakeUnique(); InlinedVector error_list; for (size_t i = 0; i < g_registered_parsers->size(); i++) { @@ -116,30 +107,25 @@ grpc_error* ServiceConfig::ParseJsonMethodConfigToServiceConfigVectorTable( objs_vector->push_back(std::move(parsed_obj)); } parsed_method_config_vectors_storage_.push_back(std::move(objs_vector)); - const auto* vector_ptr = - parsed_method_config_vectors_storage_ - [parsed_method_config_vectors_storage_.size() - 1] - .get(); + const auto* vector_ptr = parsed_method_config_vectors_storage_.back().get(); // Construct list of paths. - InlinedVector, 10> paths; - for (grpc_json* child = json->child; child != nullptr; child = child->next) { - if (child->key == nullptr) continue; - if (strcmp(child->key, "name") == 0) { - if (child->type != GRPC_JSON_ARRAY) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:name error:not of type Array")); - goto wrap_error; - } - for (grpc_json* name = child->child; name != nullptr; name = name->next) { - grpc_error* parse_error = GRPC_ERROR_NONE; - grpc_core::UniquePtr path = - ParseJsonMethodName(name, &parse_error); - if (path == nullptr) { - error_list.push_back(parse_error); - } else { - GPR_DEBUG_ASSERT(parse_error == GRPC_ERROR_NONE); - paths.push_back(std::move(path)); - } + InlinedVector, 10> paths; + auto it = json.object_value().find("name"); + if (it != json.object_value().end()) { + if (it->second.type() != Json::Type::ARRAY) { + error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "field:name error:not of type Array")); + return GRPC_ERROR_CREATE_FROM_VECTOR("methodConfig", &error_list); + } + const Json::Array& name_array = it->second.array_value(); + for (const Json& name : name_array) { + grpc_error* parse_error = GRPC_ERROR_NONE; + UniquePtr path = ParseJsonMethodName(name, &parse_error); + if (path == nullptr) { + error_list.push_back(parse_error); + } else { + GPR_DEBUG_ASSERT(parse_error == GRPC_ERROR_NONE); + paths.push_back(std::move(path)); } } } @@ -149,136 +135,82 @@ grpc_error* ServiceConfig::ParseJsonMethodConfigToServiceConfigVectorTable( } // Add entry for each path. for (size_t i = 0; i < paths.size(); ++i) { - entries[*idx].key = grpc_slice_from_copied_string(paths[i].get()); - entries[*idx].value = vector_ptr; - ++*idx; + entries->push_back( + {grpc_slice_from_copied_string(paths[i].get()), vector_ptr}); } -wrap_error: return GRPC_ERROR_CREATE_FROM_VECTOR("methodConfig", &error_list); } -grpc_error* ServiceConfig::ParsePerMethodParams(const grpc_json* json_tree) { - GPR_DEBUG_ASSERT(json_tree_->type == GRPC_JSON_OBJECT); - GPR_DEBUG_ASSERT(json_tree_->key == nullptr); - SliceHashTable::Entry* entries = nullptr; - size_t num_entries = 0; - InlinedVector error_list; - for (grpc_json* field = json_tree->child; field != nullptr; - field = field->next) { - if (field->key == nullptr) { +grpc_error* ServiceConfig::ParsePerMethodParams() { + InlinedVector::Entry, 10> entries; + std::vector error_list; + auto it = json_.object_value().find("methodConfig"); + if (it != json_.object_value().end()) { + if (it->second.type() != Json::Type::ARRAY) { error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "error:Illegal key value - NULL")); - continue; + "field:methodConfig error:not of type Array")); } - if (strcmp(field->key, "methodConfig") == 0) { - if (entries != nullptr) { - GPR_ASSERT(false); - } - if (field->type != GRPC_JSON_ARRAY) { + for (const Json& method_config : it->second.array_value()) { + if (method_config.type() != Json::Type::OBJECT) { error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:methodConfig error:not of type Array")); - } - for (grpc_json* method = field->child; method != nullptr; - method = method->next) { - int count = CountNamesInMethodConfig(method); - if (count <= 0) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:methodConfig error:No names found")); - } - num_entries += static_cast(count); + "field:methodConfig error:not of type Object")); + continue; } - entries = static_cast::Entry*>( - gpr_zalloc(num_entries * - sizeof(SliceHashTable::Entry))); - size_t idx = 0; - for (grpc_json* method = field->child; method != nullptr; - method = method->next) { - grpc_error* error = ParseJsonMethodConfigToServiceConfigVectorTable( - method, entries, &idx); - if (error != GRPC_ERROR_NONE) { - error_list.push_back(error); - } + grpc_error* error = ParseJsonMethodConfigToServiceConfigVectorTable( + method_config, &entries); + if (error != GRPC_ERROR_NONE) { + error_list.push_back(error); } - // idx might not be equal to num_entries due to parsing errors - num_entries = idx; - break; } } - if (entries != nullptr) { + if (!entries.empty()) { parsed_method_configs_table_ = - SliceHashTable::Create(num_entries, entries, - nullptr); - gpr_free(entries); + SliceHashTable::Create( + entries.size(), entries.data(), nullptr); } return GRPC_ERROR_CREATE_FROM_VECTOR("Method Params", &error_list); } -ServiceConfig::~ServiceConfig() { grpc_json_destroy(json_tree_); } - -int ServiceConfig::CountNamesInMethodConfig(grpc_json* json) { - int num_names = 0; - for (grpc_json* field = json->child; field != nullptr; field = field->next) { - if (field->key != nullptr && strcmp(field->key, "name") == 0) { - if (field->type != GRPC_JSON_ARRAY) return -1; - for (grpc_json* name = field->child; name != nullptr; name = name->next) { - if (name->type != GRPC_JSON_OBJECT) return -1; - ++num_names; - } - } - } - return num_names; -} - -grpc_core::UniquePtr ServiceConfig::ParseJsonMethodName( - grpc_json* json, grpc_error** error) { - if (json->type != GRPC_JSON_OBJECT) { +UniquePtr ServiceConfig::ParseJsonMethodName(const Json& json, + grpc_error** error) { + if (json.type() != Json::Type::OBJECT) { *error = GRPC_ERROR_CREATE_FROM_STATIC_STRING( "field:name error:type is not object"); return nullptr; } - const char* service_name = nullptr; + // Find service name. + auto it = json.object_value().find("service"); + if (it == json.object_value().end()) { + *error = GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "field:name error: field:service error:not found"); + return nullptr; // Required field. + } + if (it->second.type() != Json::Type::STRING) { + *error = GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "field:name error: field:service error:not of type string"); + return nullptr; + } + if (it->second.string_value().empty()) { + *error = GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "field:name error: field:service error:empty value"); + return nullptr; + } + const char* service_name = it->second.string_value().c_str(); const char* method_name = nullptr; - for (grpc_json* child = json->child; child != nullptr; child = child->next) { - if (child->key == nullptr) { + // Find method name. + it = json.object_value().find("method"); + if (it != json.object_value().end()) { + if (it->second.type() != Json::Type::STRING) { *error = GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:name error:Child entry with no key"); + "field:name error: field:method error:not of type string"); return nullptr; } - if (child->type != GRPC_JSON_STRING) { + if (it->second.string_value().empty()) { *error = GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:name error:Child entry not of type string"); + "field:name error: field:method error:empty value"); return nullptr; } - if (strcmp(child->key, "service") == 0) { - if (service_name != nullptr) { - *error = GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:name error: field:service error:Multiple entries"); - return nullptr; // Duplicate. - } - if (child->value == nullptr) { - *error = GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:name error: field:service error:empty value"); - return nullptr; - } - service_name = child->value; - } else if (strcmp(child->key, "method") == 0) { - if (method_name != nullptr) { - *error = GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:name error: field:method error:multiple entries"); - return nullptr; // Duplicate. - } - if (child->value == nullptr) { - *error = GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:name error: field:method error:empty value"); - return nullptr; - } - method_name = child->value; - } - } - if (service_name == nullptr) { - *error = GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:name error: field:service error:not found"); - return nullptr; // Required field. + method_name = it->second.string_value().c_str(); } char* path; gpr_asprintf(&path, "/%s/%s", service_name, diff --git a/src/core/ext/filters/client_channel/service_config.h b/src/core/ext/filters/client_channel/service_config.h index 206671fd41f..6e9ead52af9 100644 --- a/src/core/ext/filters/client_channel/service_config.h +++ b/src/core/ext/filters/client_channel/service_config.h @@ -70,7 +70,7 @@ class ServiceConfig : public RefCounted { virtual ~Parser() = default; virtual std::unique_ptr ParseGlobalParams( - const grpc_json* /* json */, grpc_error** error) { + const Json& /* json */, grpc_error** error) { // Avoid unused parameter warning on debug-only parameter (void)error; GPR_DEBUG_ASSERT(error != nullptr); @@ -78,7 +78,7 @@ class ServiceConfig : public RefCounted { } virtual std::unique_ptr ParsePerMethodParams( - const grpc_json* /* json */, grpc_error** error) { + const Json& /* json */, grpc_error** error) { // Avoid unused parameter warning on debug-only parameter (void)error; GPR_DEBUG_ASSERT(error != nullptr); @@ -125,16 +125,12 @@ class ServiceConfig : public RefCounted { /// Creates a new service config from parsing \a json_string. /// Returns null on parse error. - static RefCountedPtr Create(const char* json, + static RefCountedPtr Create(StringView json_string, grpc_error** error); - // Takes ownership of \a json_tree. - ServiceConfig(grpc_core::UniquePtr service_config_json, - grpc_core::UniquePtr json_string, grpc_json* json_tree, - grpc_error** error); - ~ServiceConfig(); + ServiceConfig(std::string json_string, Json json, grpc_error** error); - const char* service_config_json() const { return service_config_json_.get(); } + const std::string& json_string() const { return json_string_; } /// Retrieves the global parsed config at index \a index. The /// lifetime of the returned object is tied to the lifetime of the @@ -163,24 +159,21 @@ class ServiceConfig : public RefCounted { private: // Helper functions to parse the service config - grpc_error* ParseGlobalParams(const grpc_json* json_tree); - grpc_error* ParsePerMethodParams(const grpc_json* json_tree); - - // Returns the number of names specified in the method config \a json. - static int CountNamesInMethodConfig(grpc_json* json); + grpc_error* ParseGlobalParams(); + grpc_error* ParsePerMethodParams(); // Returns a path string for the JSON name object specified by \a json. // Returns null on error, and stores error in \a error. - static grpc_core::UniquePtr ParseJsonMethodName(grpc_json* json, - grpc_error** error); + static UniquePtr ParseJsonMethodName(const Json& json, + grpc_error** error); grpc_error* ParseJsonMethodConfigToServiceConfigVectorTable( - const grpc_json* json, - SliceHashTable::Entry* entries, size_t* idx); + const Json& json, + InlinedVector::Entry, 10>* + entries); - grpc_core::UniquePtr service_config_json_; - grpc_core::UniquePtr json_string_; // Underlying storage for json_tree. - grpc_json* json_tree_; + std::string json_string_; + Json json_; InlinedVector, kNumPreallocatedParsers> parsed_global_configs_; diff --git a/src/core/ext/filters/message_size/message_size_filter.cc b/src/core/ext/filters/message_size/message_size_filter.cc index 1ec34e24f66..6cecb943156 100644 --- a/src/core/ext/filters/message_size/message_size_filter.cc +++ b/src/core/ext/filters/message_size/message_size_filter.cc @@ -45,43 +45,40 @@ size_t g_message_size_parser_index; } // namespace std::unique_ptr -MessageSizeParser::ParsePerMethodParams(const grpc_json* json, - grpc_error** error) { +MessageSizeParser::ParsePerMethodParams(const Json& json, grpc_error** error) { GPR_DEBUG_ASSERT(error != nullptr && *error == GRPC_ERROR_NONE); + std::vector error_list; + // Max request size. int max_request_message_bytes = -1; - int max_response_message_bytes = -1; - InlinedVector error_list; - for (grpc_json* field = json->child; field != nullptr; field = field->next) { - if (field->key == nullptr) continue; - if (strcmp(field->key, "maxRequestMessageBytes") == 0) { - if (max_request_message_bytes >= 0) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:maxRequestMessageBytes error:Duplicate entry")); - } // Duplicate, continue parsing. - if (field->type != GRPC_JSON_STRING && field->type != GRPC_JSON_NUMBER) { + auto it = json.object_value().find("maxRequestMessageBytes"); + if (it != json.object_value().end()) { + if (it->second.type() != Json::Type::STRING && + it->second.type() != Json::Type::NUMBER) { + error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "field:maxRequestMessageBytes error:should be of type number")); + } else { + max_request_message_bytes = + gpr_parse_nonnegative_int(it->second.string_value().c_str()); + if (max_request_message_bytes == -1) { error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:maxRequestMessageBytes error:should be of type number")); - } else { - max_request_message_bytes = gpr_parse_nonnegative_int(field->value); - if (max_request_message_bytes == -1) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:maxRequestMessageBytes error:should be non-negative")); - } + "field:maxRequestMessageBytes error:should be non-negative")); } - } else if (strcmp(field->key, "maxResponseMessageBytes") == 0) { - if (max_response_message_bytes >= 0) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:maxResponseMessageBytes error:Duplicate entry")); - } // Duplicate, continue parsing - if (field->type != GRPC_JSON_STRING && field->type != GRPC_JSON_NUMBER) { + } + } + // Max response size. + int max_response_message_bytes = -1; + it = json.object_value().find("maxResponseMessageBytes"); + if (it != json.object_value().end()) { + if (it->second.type() != Json::Type::STRING && + it->second.type() != Json::Type::NUMBER) { + error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "field:maxResponseMessageBytes error:should be of type number")); + } else { + max_response_message_bytes = + gpr_parse_nonnegative_int(it->second.string_value().c_str()); + if (max_response_message_bytes == -1) { error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:maxResponseMessageBytes error:should be of type number")); - } else { - max_response_message_bytes = gpr_parse_nonnegative_int(field->value); - if (max_response_message_bytes == -1) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:maxResponseMessageBytes error:should be non-negative")); - } + "field:maxResponseMessageBytes error:should be non-negative")); } } } diff --git a/src/core/ext/filters/message_size/message_size_filter.h b/src/core/ext/filters/message_size/message_size_filter.h index b54acd538f2..58c1fac0916 100644 --- a/src/core/ext/filters/message_size/message_size_filter.h +++ b/src/core/ext/filters/message_size/message_size_filter.h @@ -47,7 +47,7 @@ class MessageSizeParsedConfig : public ServiceConfig::ParsedConfig { class MessageSizeParser : public ServiceConfig::Parser { public: std::unique_ptr ParsePerMethodParams( - const grpc_json* json, grpc_error** error) override; + const Json& json, grpc_error** error) override; static void Register(); diff --git a/src/core/lib/gprpp/inlined_vector.h b/src/core/lib/gprpp/inlined_vector.h index e05a0a855f9..449bb0b339a 100644 --- a/src/core/lib/gprpp/inlined_vector.h +++ b/src/core/lib/gprpp/inlined_vector.h @@ -162,6 +162,12 @@ class InlinedVector { size_--; } + T& front() { return data()[0]; } + const T& front() const { return data()[0]; } + + T& back() { return data()[size_ - 1]; } + const T& back() const { return data()[size_ - 1]; } + size_t size() const { return size_; } bool empty() const { return size_ == 0; } diff --git a/src/core/lib/iomgr/error.h b/src/core/lib/iomgr/error.h index 8c22357b951..5c79aa78ae7 100644 --- a/src/core/lib/iomgr/error.h +++ b/src/core/lib/iomgr/error.h @@ -201,10 +201,10 @@ inline void grpc_error_unref(grpc_error* err) { // Consumes all the errors in the vector and forms a referencing error from // them. If the vector is empty, return GRPC_ERROR_NONE. -template -static grpc_error* grpc_error_create_from_vector( - const char* file, int line, const char* desc, - grpc_core::InlinedVector* error_list) { +template +static grpc_error* grpc_error_create_from_vector(const char* file, int line, + const char* desc, + VectorType* error_list) { grpc_error* error = GRPC_ERROR_NONE; if (error_list->size() != 0) { error = grpc_error_create(file, line, grpc_slice_from_static_string(desc), diff --git a/src/core/lib/json/json_reader_new.cc b/src/core/lib/json/json_reader_new.cc index acc8685efe9..d316dec98ad 100644 --- a/src/core/lib/json/json_reader_new.cc +++ b/src/core/lib/json/json_reader_new.cc @@ -21,6 +21,7 @@ #include #include +#include #include "src/core/lib/json/json.h" @@ -30,15 +31,15 @@ namespace { class JsonReader { public: + static grpc_error* Parse(StringView input, Json* output); + + private: enum class Status { GRPC_JSON_DONE, /* The parser finished successfully. */ GRPC_JSON_PARSE_ERROR, /* The parser found an error in the json stream. */ GRPC_JSON_INTERNAL_ERROR /* The parser got an internal error. */ }; - static Status Parse(StringView input, Json* output); - - private: enum class State { GRPC_JSON_STATE_OBJECT_KEY_BEGIN, GRPC_JSON_STATE_OBJECT_KEY_STRING, @@ -77,13 +78,16 @@ class JsonReader { static constexpr uint32_t GRPC_JSON_READ_CHAR_EOF = 0x7ffffff0; explicit JsonReader(StringView input) - : input_(reinterpret_cast(input.data())), + : original_input_(reinterpret_cast(input.data())), + input_(original_input_), remaining_input_(input.size()) {} Status Run(); uint32_t ReadChar(); bool IsComplete(); + size_t CurrentIndex() const { return input_ - original_input_ - 1; } + void StringAddChar(uint32_t c); void StringAddUtf32(uint32_t c); @@ -97,6 +101,7 @@ class JsonReader { void SetFalse(); void SetNull(); + const uint8_t* original_input_; const uint8_t* input_; size_t remaining_input_; @@ -105,7 +110,7 @@ class JsonReader { bool container_just_begun_ = false; uint16_t unicode_char_ = 0; uint16_t unicode_high_surrogate_ = 0; - bool duplicate_key_found_ = false; + std::vector errors_; Json root_value_; std::vector stack_; @@ -164,7 +169,11 @@ Json* JsonReader::CreateAndLinkValue() { Json* parent = stack_.back(); if (parent->type() == Json::Type::OBJECT) { if (parent->object_value().find(key_) != parent->object_value().end()) { - duplicate_key_found_ = true; + char* msg; + gpr_asprintf(&msg, "duplicate key \"%s\" at index %" PRIuPTR, + key_.c_str(), CurrentIndex()); + errors_.push_back(GRPC_ERROR_CREATE_FROM_COPIED_STRING(msg)); + gpr_free(msg); } value = &(*parent->mutable_object())[std::move(key_)]; } else { @@ -781,27 +790,35 @@ JsonReader::Status JsonReader::Run() { GPR_UNREACHABLE_CODE(return Status::GRPC_JSON_INTERNAL_ERROR); } -JsonReader::Status JsonReader::Parse(StringView input, Json* output) { +grpc_error* JsonReader::Parse(StringView input, Json* output) { JsonReader reader(input); Status status = reader.Run(); - if (reader.duplicate_key_found_) status = Status::GRPC_JSON_PARSE_ERROR; - if (status == Status::GRPC_JSON_DONE) { - *output = std::move(reader.root_value_); + if (status == Status::GRPC_JSON_INTERNAL_ERROR) { + char* msg; + gpr_asprintf(&msg, "internal error in JSON parser at index %" PRIuPTR, + reader.CurrentIndex()); + reader.errors_.push_back(GRPC_ERROR_CREATE_FROM_COPIED_STRING(msg)); + gpr_free(msg); + } else if (status == Status::GRPC_JSON_PARSE_ERROR) { + char* msg; + gpr_asprintf(&msg, "JSON parse error at index %" PRIuPTR, + reader.CurrentIndex()); + reader.errors_.push_back(GRPC_ERROR_CREATE_FROM_COPIED_STRING(msg)); + gpr_free(msg); + } + if (!reader.errors_.empty()) { + return GRPC_ERROR_CREATE_FROM_VECTOR("JSON parsing failed", + &reader.errors_); } - return status; + *output = std::move(reader.root_value_); + return GRPC_ERROR_NONE; } } // namespace Json Json::Parse(StringView json_str, grpc_error** error) { Json value; - JsonReader::Status status = JsonReader::Parse(json_str, &value); - if (status == JsonReader::Status::GRPC_JSON_PARSE_ERROR) { - *error = GRPC_ERROR_CREATE_FROM_STATIC_STRING("JSON parse error"); - } else if (status == JsonReader::Status::GRPC_JSON_INTERNAL_ERROR) { - *error = - GRPC_ERROR_CREATE_FROM_STATIC_STRING("internal error in JSON parser"); - } + *error = JsonReader::Parse(json_str, &value); return value; } diff --git a/test/core/client_channel/service_config_test.cc b/test/core/client_channel/service_config_test.cc index 0cb23bda44f..27ea0580e69 100644 --- a/test/core/client_channel/service_config_test.cc +++ b/test/core/client_channel/service_config_test.cc @@ -44,24 +44,22 @@ class TestParsedConfig1 : public ServiceConfig::ParsedConfig { class TestParser1 : public ServiceConfig::Parser { public: std::unique_ptr ParseGlobalParams( - const grpc_json* json, grpc_error** error) override { + const Json& json, grpc_error** error) override { GPR_DEBUG_ASSERT(error != nullptr); - for (grpc_json* field = json->child; field != nullptr; - field = field->next) { - if (strcmp(field->key, "global_param") == 0) { - if (field->type != GRPC_JSON_NUMBER) { - *error = - GRPC_ERROR_CREATE_FROM_STATIC_STRING(InvalidTypeErrorMessage()); - return nullptr; - } - int value = gpr_parse_nonnegative_int(field->value); - if (value == -1) { - *error = - GRPC_ERROR_CREATE_FROM_STATIC_STRING(InvalidValueErrorMessage()); - return nullptr; - } - return grpc_core::MakeUnique(value); + auto it = json.object_value().find("global_param"); + if (it != json.object_value().end()) { + if (it->second.type() != Json::Type::NUMBER) { + *error = + GRPC_ERROR_CREATE_FROM_STATIC_STRING(InvalidTypeErrorMessage()); + return nullptr; } + int value = gpr_parse_nonnegative_int(it->second.string_value().c_str()); + if (value == -1) { + *error = + GRPC_ERROR_CREATE_FROM_STATIC_STRING(InvalidValueErrorMessage()); + return nullptr; + } + return grpc_core::MakeUnique(value); } return nullptr; } @@ -78,27 +76,22 @@ class TestParser1 : public ServiceConfig::Parser { class TestParser2 : public ServiceConfig::Parser { public: std::unique_ptr ParsePerMethodParams( - const grpc_json* json, grpc_error** error) override { + const Json& json, grpc_error** error) override { GPR_DEBUG_ASSERT(error != nullptr); - for (grpc_json* field = json->child; field != nullptr; - field = field->next) { - if (field->key == nullptr || strcmp(field->key, "name") == 0) { - continue; + auto it = json.object_value().find("method_param"); + if (it != json.object_value().end()) { + if (it->second.type() != Json::Type::NUMBER) { + *error = + GRPC_ERROR_CREATE_FROM_STATIC_STRING(InvalidTypeErrorMessage()); + return nullptr; } - if (strcmp(field->key, "method_param") == 0) { - if (field->type != GRPC_JSON_NUMBER) { - *error = - GRPC_ERROR_CREATE_FROM_STATIC_STRING(InvalidTypeErrorMessage()); - return nullptr; - } - int value = gpr_parse_nonnegative_int(field->value); - if (value == -1) { - *error = - GRPC_ERROR_CREATE_FROM_STATIC_STRING(InvalidValueErrorMessage()); - return nullptr; - } - return grpc_core::MakeUnique(value); + int value = gpr_parse_nonnegative_int(it->second.string_value().c_str()); + if (value == -1) { + *error = + GRPC_ERROR_CREATE_FROM_STATIC_STRING(InvalidValueErrorMessage()); + return nullptr; } + return grpc_core::MakeUnique(value); } return nullptr; } @@ -116,14 +109,14 @@ class TestParser2 : public ServiceConfig::Parser { class ErrorParser : public ServiceConfig::Parser { public: std::unique_ptr ParsePerMethodParams( - const grpc_json* /*json*/, grpc_error** error) override { + const Json& /*json*/, grpc_error** error) override { GPR_DEBUG_ASSERT(error != nullptr); *error = GRPC_ERROR_CREATE_FROM_STATIC_STRING(MethodError()); return nullptr; } std::unique_ptr ParseGlobalParams( - const grpc_json* /*json*/, grpc_error** error) override { + const Json& /*json*/, grpc_error** error) override { GPR_DEBUG_ASSERT(error != nullptr); *error = GRPC_ERROR_CREATE_FROM_STATIC_STRING(GlobalError()); return nullptr; @@ -159,7 +152,7 @@ TEST_F(ServiceConfigTest, ErrorCheck1) { auto svc_cfg = ServiceConfig::Create(test_json, &error); gpr_log(GPR_ERROR, "%s", grpc_error_string(error)); ASSERT_TRUE(error != GRPC_ERROR_NONE); - std::regex e(std::string("failed to parse JSON for service config")); + std::regex e(std::string("JSON parse error")); VerifyRegexMatch(error, e); } @@ -177,11 +170,10 @@ TEST_F(ServiceConfigTest, ErrorNoNames) { gpr_log(GPR_ERROR, "%s", grpc_error_string(error)); ASSERT_TRUE(error != GRPC_ERROR_NONE); std::regex e( - std::string("(Service config parsing " - "error)(.*)(referenced_errors)(.*)(Method " - "Params)(.*)(referenced_errors)(.*)(No names " - "found)(.*)(methodConfig)(.*)(referenced_errors)(.*)(No " - "names specified)")); + std::string("(Service config parsing error)(.*)(referenced_errors)" + "(.*)(Method Params)(.*)(referenced_errors)" + "(.*)(methodConfig)(.*)(referenced_errors)" + "(.*)(No names specified)")); VerifyRegexMatch(error, e); } @@ -193,11 +185,10 @@ TEST_F(ServiceConfigTest, ErrorNoNamesWithMultipleMethodConfigs) { gpr_log(GPR_ERROR, "%s", grpc_error_string(error)); ASSERT_TRUE(error != GRPC_ERROR_NONE); std::regex e( - std::string("(Service config parsing " - "error)(.*)(referenced_errors)(.*)(Method " - "Params)(.*)(referenced_errors)(.*)(No names " - "found)(.*)(methodConfig)(.*)(referenced_errors)(.*)(No " - "names specified)")); + std::string("(Service config parsing error)(.*)(referenced_errors)" + "(.*)(Method Params)(.*)(referenced_errors)" + "(.*)(methodConfig)(.*)(referenced_errors)" + "(.*)(No names specified)")); VerifyRegexMatch(error, e); } @@ -337,18 +328,16 @@ TEST_F(ErroredParsersScopingTest, MethodParams) { auto svc_cfg = ServiceConfig::Create(test_json, &error); ASSERT_TRUE(error != GRPC_ERROR_NONE); gpr_log(GPR_ERROR, "%s", grpc_error_string(error)); - std::regex e( - std::string("(Service config parsing " - "error)(.*)(referenced_errors\":\\[)(.*)(Global " - "Params)(.*)(referenced_errors)()(.*)") + - ErrorParser::GlobalError() + std::string("(.*)") + - ErrorParser::GlobalError() + - std::string("(.*)(Method " - "Params)(.*)(referenced_errors)(.*)(field:methodConfig " - "error:No names " - "found)(.*)(methodConfig)(.*)(referenced_errors)(.*)") + - ErrorParser::MethodError() + std::string("(.*)") + - ErrorParser::MethodError() + std::string("(.*)(No names specified)")); + std::regex e(std::string("(Service config parsing " + "error)(.*)(referenced_errors\":\\[)(.*)(Global " + "Params)(.*)(referenced_errors)()(.*)") + + ErrorParser::GlobalError() + std::string("(.*)") + + ErrorParser::GlobalError() + + std::string("(.*)(Method Params)(.*)(referenced_errors)" + "(.*)(methodConfig)(.*)(referenced_errors)(.*)") + + ErrorParser::MethodError() + std::string("(.*)") + + ErrorParser::MethodError() + + std::string("(.*)(No names specified)")); VerifyRegexMatch(error, e); } @@ -428,11 +417,11 @@ TEST_F(ClientChannelParserTest, UnknownLoadBalancingConfig) { gpr_log(GPR_ERROR, "%s", grpc_error_string(error)); ASSERT_TRUE(error != GRPC_ERROR_NONE); std::regex e( - std::string("(Service config parsing " - "error)(.*)(referenced_errors)(.*)(Global " - "Params)(.*)(referenced_errors)(.*)(Client channel global " - "parser)(.*)(referenced_errors)(.*)(field:" - "loadBalancingConfig error:No known policy)")); + std::string("(Service config parsing error)(.*)(referenced_errors)" + "(.*)(Global Params)(.*)(referenced_errors)" + "(.*)(Client channel global parser)(.*)(referenced_errors)" + "(.*)(field:loadBalancingConfig)(.*)(referenced_errors)" + "(.*)(No known policy)")); VerifyRegexMatch(error, e); } @@ -445,12 +434,13 @@ TEST_F(ClientChannelParserTest, InvalidGrpclbLoadBalancingConfig) { gpr_log(GPR_ERROR, "%s", grpc_error_string(error)); ASSERT_TRUE(error != GRPC_ERROR_NONE); std::regex e( - std::string("(Service config parsing " - "error)(.*)(referenced_errors)(.*)(Global " - "Params)(.*)(referenced_errors)(.*)(Client channel global " - "parser)(.*)(referenced_errors)(.*)(GrpcLb " - "Parser)(.*)(referenced_errors)(.*)(field:childPolicy " - "error:No known policy)")); + std::string("(Service config parsing error)(.*)(referenced_errors)" + "(.*)(Global Params)(.*)(referenced_errors)" + "(.*)(Client channel global parser)(.*)(referenced_errors)" + "(.*)(field:loadBalancingConfig)(.*)(referenced_errors)" + "(.*)(GrpcLb Parser)(.*)(referenced_errors)" + "(.*)(field:childPolicy)(.*)(referenced_errors)" + "(.*)(No known policy)")); VerifyRegexMatch(error, e); } @@ -914,10 +904,8 @@ TEST_F(ClientChannelParserTest, InvalidHealthCheckMultipleEntries) { gpr_log(GPR_ERROR, "%s", grpc_error_string(error)); ASSERT_TRUE(error != GRPC_ERROR_NONE); std::regex e( - std::string("(Service config parsing " - "error)(.*)(referenced_errors)(.*)(Global " - "Params)(.*)(referenced_errors)(.*)(field:healthCheckConfig " - "error:Duplicate entry)")); + std::string("(JSON parsing failed)(.*)(referenced_errors)" + "(.*)(duplicate key \"healthCheckConfig\" at index 104)")); VerifyRegexMatch(error, e); } diff --git a/test/core/client_channel/xds_bootstrap_test.cc b/test/core/client_channel/xds_bootstrap_test.cc index 071e519a31d..ddafe1382bb 100644 --- a/test/core/client_channel/xds_bootstrap_test.cc +++ b/test/core/client_channel/xds_bootstrap_test.cc @@ -83,7 +83,7 @@ TEST(XdsBootstrapTest, Basic) { grpc_core::XdsBootstrap bootstrap(std::move(json), &error); EXPECT_EQ(error, GRPC_ERROR_NONE) << grpc_error_string(error); EXPECT_EQ(bootstrap.server().server_uri, "fake:///lb"); - ASSERT_EQ(bootstrap.server().channel_creds.size(), 1); + ASSERT_EQ(bootstrap.server().channel_creds.size(), 1UL); EXPECT_EQ(bootstrap.server().channel_creds[0].type, "fake"); EXPECT_EQ(bootstrap.server().channel_creds[0].config.type(), Json::Type::JSON_NULL); @@ -123,7 +123,7 @@ TEST(XdsBootstrapTest, ValidWithoutChannelCredsAndNode) { grpc_core::XdsBootstrap bootstrap(std::move(json), &error); EXPECT_EQ(error, GRPC_ERROR_NONE) << grpc_error_string(error); EXPECT_EQ(bootstrap.server().server_uri, "fake:///lb"); - EXPECT_EQ(bootstrap.server().channel_creds.size(), 0); + EXPECT_EQ(bootstrap.server().channel_creds.size(), 0UL); EXPECT_EQ(bootstrap.node(), nullptr); } diff --git a/test/core/util/test_lb_policies.cc b/test/core/util/test_lb_policies.cc index 150a196ae45..bc30add288e 100644 --- a/test/core/util/test_lb_policies.cc +++ b/test/core/util/test_lb_policies.cc @@ -226,7 +226,7 @@ class InterceptTrailingFactory : public LoadBalancingPolicyFactory { } RefCountedPtr ParseLoadBalancingConfig( - const grpc_json* /*json*/, grpc_error** /*error*/) const override { + const Json& /*json*/, grpc_error** /*error*/) const override { return nullptr; } diff --git a/test/cpp/end2end/service_config_end2end_test.cc b/test/cpp/end2end/service_config_end2end_test.cc index a00a45ecbb7..7466cf2a563 100644 --- a/test/cpp/end2end/service_config_end2end_test.cc +++ b/test/cpp/end2end/service_config_end2end_test.cc @@ -245,10 +245,9 @@ class ServiceConfigEnd2endTest : public ::testing::Test { std::shared_ptr BuildChannelWithInvalidDefaultServiceConfig() { ChannelArguments args; - EXPECT_THAT( - grpc::experimental::ValidateServiceConfigJSON( - InvalidDefaultServiceConfig()), - ::testing::HasSubstr("failed to parse JSON for service config")); + EXPECT_THAT(grpc::experimental::ValidateServiceConfigJSON( + InvalidDefaultServiceConfig()), + ::testing::HasSubstr("JSON parse error")); args.SetServiceConfigJSON(InvalidDefaultServiceConfig()); args.SetPointer(GRPC_ARG_FAKE_RESOLVER_RESPONSE_GENERATOR, response_generator_.get()); diff --git a/test/cpp/naming/resolver_component_test.cc b/test/cpp/naming/resolver_component_test.cc index e74f5666b81..b03901b92ee 100644 --- a/test/cpp/naming/resolver_component_test.cc +++ b/test/cpp/naming/resolver_component_test.cc @@ -488,7 +488,7 @@ class CheckingResultHandler : public ResultHandler { const char* service_config_json = result.service_config == nullptr ? nullptr - : result.service_config->service_config_json(); + : result.service_config->json_string().c_str(); CheckServiceConfigResultLocked( service_config_json, GRPC_ERROR_REF(result.service_config_error), args); if (args->expected_service_config_string == "") { diff --git a/test/cpp/naming/resolver_component_tests_runner.py b/test/cpp/naming/resolver_component_tests_runner.py index e767e4cb5b8..53d9364a35b 100755 --- a/test/cpp/naming/resolver_component_tests_runner.py +++ b/test/cpp/naming/resolver_component_tests_runner.py @@ -476,7 +476,7 @@ current_test_subprocess = subprocess.Popen([ '--target_name', 'ipv4-svc_cfg_bad_json.resolver-tests-version-4.grpctestingexp.', '--expected_addrs', '1.2.3.4:443,False', '--expected_chosen_service_config', '', - '--expected_service_config_error', 'could not parse', + '--expected_service_config_error', 'JSON parse error', '--expected_lb_policy', '', '--enable_srv_queries', 'True', '--enable_txt_queries', 'True', @@ -555,8 +555,8 @@ current_test_subprocess = subprocess.Popen([ args.test_bin_path, '--target_name', 'ipv4-config-causing-fallback-to-tcp-inject-broken-nameservers.resolver-tests-version-4.grpctestingexp.', '--expected_addrs', '1.2.3.4:443,False', - '--expected_chosen_service_config', '{"loadBalancingPolicy":"round_robin","methodConfig":[{"name":[{"method":"Foo","service":"SimpleService","waitForReady":true}]},{"name":[{"method":"FooTwo","service":"SimpleService","waitForReady":true}]},{"name":[{"method":"FooThree","service":"SimpleService","waitForReady":true}]},{"name":[{"method":"FooFour","service":"SimpleService","waitForReady":true}]},{"name":[{"method":"FooFive","service":"SimpleService","waitForReady":true}]},{"name":[{"method":"FooSix","service":"SimpleService","waitForReady":true}]},{"name":[{"method":"FooSeven","service":"SimpleService","waitForReady":true}]},{"name":[{"method":"FooEight","service":"SimpleService","waitForReady":true}]},{"name":[{"method":"FooNine","service":"SimpleService","waitForReady":true}]},{"name":[{"method":"FooTen","service":"SimpleService","waitForReady":true}]},{"name":[{"method":"FooEleven","service":"SimpleService","waitForReady":true}]},{"name":[{"method":"FooTwelve","service":"SimpleService","waitForReady":true}]},{"name":[{"method":"FooTwelve","service":"SimpleService","waitForReady":true}]},{"name":[{"method":"FooTwelve","service":"SimpleService","waitForReady":true}]},{"name":[{"method":"FooTwelve","service":"SimpleService","waitForReady":true}]}]}', - '--expected_service_config_error', 'Service config parsing error', + '--expected_chosen_service_config', '{"loadBalancingPolicy":["round_robin"]}', + '--expected_service_config_error', 'field:loadBalancingPolicy error:type should be string', '--expected_lb_policy', '', '--enable_srv_queries', 'True', '--enable_txt_queries', 'True', diff --git a/test/cpp/naming/resolver_test_record_groups.yaml b/test/cpp/naming/resolver_test_record_groups.yaml index f72a7eaf572..a1d932ae2b7 100644 --- a/test/cpp/naming/resolver_test_record_groups.yaml +++ b/test/cpp/naming/resolver_test_record_groups.yaml @@ -364,7 +364,7 @@ resolver_component_tests: - expected_addrs: - {address: '1.2.3.4:443', is_balancer: false} expected_chosen_service_config: null - expected_service_config_error: 'could not parse' + expected_service_config_error: 'JSON parse error' expected_lb_policy: null enable_srv_queries: true enable_txt_queries: true @@ -436,8 +436,8 @@ resolver_component_tests: - {TTL: '2100', data: 5.5.5.5, type: A} - expected_addrs: - {address: '1.2.3.4:443', is_balancer: false} - expected_chosen_service_config: '{"loadBalancingPolicy":"round_robin","methodConfig":[{"name":[{"method":"Foo","service":"SimpleService","waitForReady":true}]},{"name":[{"method":"FooTwo","service":"SimpleService","waitForReady":true}]},{"name":[{"method":"FooThree","service":"SimpleService","waitForReady":true}]},{"name":[{"method":"FooFour","service":"SimpleService","waitForReady":true}]},{"name":[{"method":"FooFive","service":"SimpleService","waitForReady":true}]},{"name":[{"method":"FooSix","service":"SimpleService","waitForReady":true}]},{"name":[{"method":"FooSeven","service":"SimpleService","waitForReady":true}]},{"name":[{"method":"FooEight","service":"SimpleService","waitForReady":true}]},{"name":[{"method":"FooNine","service":"SimpleService","waitForReady":true}]},{"name":[{"method":"FooTen","service":"SimpleService","waitForReady":true}]},{"name":[{"method":"FooEleven","service":"SimpleService","waitForReady":true}]},{"name":[{"method":"FooTwelve","service":"SimpleService","waitForReady":true}]},{"name":[{"method":"FooTwelve","service":"SimpleService","waitForReady":true}]},{"name":[{"method":"FooTwelve","service":"SimpleService","waitForReady":true}]},{"name":[{"method":"FooTwelve","service":"SimpleService","waitForReady":true}]}]}' - expected_service_config_error: 'Service config parsing error' + expected_chosen_service_config: '{"loadBalancingPolicy":["round_robin"]}' + expected_service_config_error: 'field:loadBalancingPolicy error:type should be string' expected_lb_policy: null enable_srv_queries: true enable_txt_queries: true @@ -447,5 +447,5 @@ resolver_component_tests: ipv4-config-causing-fallback-to-tcp-inject-broken-nameservers: - {TTL: '2100', data: 1.2.3.4, type: A} _grpc_config.ipv4-config-causing-fallback-to-tcp-inject-broken-nameservers: - - {TTL: '2100', data: 'grpc_config=[{"serviceConfig":{"loadBalancingPolicy":"round_robin","methodConfig":[{"name":[{"method":"Foo","service":"SimpleService","waitForReady":true}]},{"name":[{"method":"FooTwo","service":"SimpleService","waitForReady":true}]},{"name":[{"method":"FooThree","service":"SimpleService","waitForReady":true}]},{"name":[{"method":"FooFour","service":"SimpleService","waitForReady":true}]},{"name":[{"method":"FooFive","service":"SimpleService","waitForReady":true}]},{"name":[{"method":"FooSix","service":"SimpleService","waitForReady":true}]},{"name":[{"method":"FooSeven","service":"SimpleService","waitForReady":true}]},{"name":[{"method":"FooEight","service":"SimpleService","waitForReady":true}]},{"name":[{"method":"FooNine","service":"SimpleService","waitForReady":true}]},{"name":[{"method":"FooTen","service":"SimpleService","waitForReady":true}]},{"name":[{"method":"FooEleven","service":"SimpleService","waitForReady":true}]},{"name":[{"method":"FooTwelve","service":"SimpleService","waitForReady":true}]},{"name":[{"method":"FooTwelve","service":"SimpleService","waitForReady":true}]},{"name":[{"method":"FooTwelve","service":"SimpleService","waitForReady":true}]},{"name":[{"method":"FooTwelve","service":"SimpleService","waitForReady":true}]}]}}]', + - {TTL: '2100', data: 'grpc_config=[{"serviceConfig":{"loadBalancingPolicy":["round_robin"]}}]', type: TXT}