diff --git a/BUILD b/BUILD index f2337092397..a6d2bf73d60 100644 --- a/BUILD +++ b/BUILD @@ -1214,6 +1214,22 @@ grpc_cc_library( ], ) +grpc_cc_library( + name = "validation_errors", + srcs = [ + "src/core/lib/gprpp/validation_errors.cc", + ], + hdrs = [ + "src/core/lib/gprpp/validation_errors.h", + ], + external_deps = [ + "absl/status", + "absl/strings", + ], + language = "c++", + deps = ["gpr_platform"], +) + # A library that vends only port_platform, so that libraries that don't need # anything else from gpr can still be portable! grpc_cc_library( @@ -4366,6 +4382,7 @@ grpc_cc_library( "time", "uri_parser", "useful", + "validation_errors", "work_serializer", ], ) @@ -4428,6 +4445,7 @@ grpc_cc_library( "subchannel_interface", "time", "uri_parser", + "validation_errors", "work_serializer", ], ) @@ -4929,6 +4947,7 @@ grpc_cc_library( "ref_counted_ptr", "server_address", "subchannel_interface", + "validation_errors", "work_serializer", "xds_client", ], @@ -4973,6 +4992,7 @@ grpc_cc_library( "ref_counted_ptr", "server_address", "subchannel_interface", + "validation_errors", "xds_client", ], ) @@ -5015,6 +5035,7 @@ grpc_cc_library( "server_address", "subchannel_interface", "time", + "validation_errors", "work_serializer", ], ) @@ -5139,6 +5160,7 @@ grpc_cc_library( "sockaddr_utils", "subchannel_interface", "unique_type_name", + "validation_errors", "work_serializer", ], ) @@ -5188,6 +5210,7 @@ grpc_cc_library( "json_args", "json_object_loader", "time", + "validation_errors", ], ) @@ -5231,6 +5254,7 @@ grpc_cc_library( "server_address", "sockaddr_utils", "subchannel_interface", + "validation_errors", "work_serializer", ], ) @@ -5274,6 +5298,7 @@ grpc_cc_library( "server_address", "subchannel_interface", "time", + "validation_errors", "work_serializer", ], ) @@ -5315,6 +5340,7 @@ grpc_cc_library( "server_address", "subchannel_interface", "time", + "validation_errors", "work_serializer", ], ) @@ -7777,6 +7803,7 @@ grpc_cc_library( "json_object_loader", "no_destruct", "time", + "validation_errors", ], ) @@ -7793,7 +7820,6 @@ grpc_cc_library( hdrs = ["src/core/lib/json/json_object_loader.h"], external_deps = [ "absl/meta:type_traits", - "absl/status", "absl/status:statusor", "absl/strings", "absl/types:optional", @@ -7805,6 +7831,7 @@ grpc_cc_library( "no_destruct", "ref_counted_ptr", "time", + "validation_errors", ], ) diff --git a/CMakeLists.txt b/CMakeLists.txt index cb5c4a3dad5..feaf0dae820 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1232,6 +1232,7 @@ if(gRPC_BUILD_TESTS) add_dependencies(buildtests_cxx unknown_frame_bad_client_test) add_dependencies(buildtests_cxx uri_parser_test) add_dependencies(buildtests_cxx useful_test) + add_dependencies(buildtests_cxx validation_errors_test) add_dependencies(buildtests_cxx varint_test) if(_gRPC_PLATFORM_LINUX OR _gRPC_PLATFORM_MAC OR _gRPC_PLATFORM_POSIX) add_dependencies(buildtests_cxx wakeup_fd_posix_test) @@ -2129,6 +2130,7 @@ add_library(grpc src/core/lib/gprpp/status_helper.cc src/core/lib/gprpp/time.cc src/core/lib/gprpp/time_averaged_stats.cc + src/core/lib/gprpp/validation_errors.cc src/core/lib/gprpp/work_serializer.cc src/core/lib/handshaker/proxy_mapper_registry.cc src/core/lib/http/format_request.cc @@ -2737,6 +2739,7 @@ add_library(grpc_unsecure src/core/lib/gprpp/status_helper.cc src/core/lib/gprpp/time.cc src/core/lib/gprpp/time_averaged_stats.cc + src/core/lib/gprpp/validation_errors.cc src/core/lib/gprpp/work_serializer.cc src/core/lib/handshaker/proxy_mapper_registry.cc src/core/lib/http/format_request.cc @@ -19377,6 +19380,41 @@ target_link_libraries(useful_test ) +endif() +if(gRPC_BUILD_TESTS) + +add_executable(validation_errors_test + test/core/gprpp/validation_errors_test.cc + third_party/googletest/googletest/src/gtest-all.cc + third_party/googletest/googlemock/src/gmock-all.cc +) + +target_include_directories(validation_errors_test + PRIVATE + ${CMAKE_CURRENT_SOURCE_DIR} + ${CMAKE_CURRENT_SOURCE_DIR}/include + ${_gRPC_ADDRESS_SORTING_INCLUDE_DIR} + ${_gRPC_RE2_INCLUDE_DIR} + ${_gRPC_SSL_INCLUDE_DIR} + ${_gRPC_UPB_GENERATED_DIR} + ${_gRPC_UPB_GRPC_GENERATED_DIR} + ${_gRPC_UPB_INCLUDE_DIR} + ${_gRPC_XXHASH_INCLUDE_DIR} + ${_gRPC_ZLIB_INCLUDE_DIR} + third_party/googletest/googletest/include + third_party/googletest/googletest + third_party/googletest/googlemock/include + third_party/googletest/googlemock + ${_gRPC_PROTO_GENS_DIR} +) + +target_link_libraries(validation_errors_test + ${_gRPC_PROTOBUF_LIBRARIES} + ${_gRPC_ALLTARGETS_LIBRARIES} + grpc_test_util +) + + endif() if(gRPC_BUILD_TESTS) diff --git a/Makefile b/Makefile index fd22b5e5e28..a14df20fa4f 100644 --- a/Makefile +++ b/Makefile @@ -1412,6 +1412,7 @@ LIBGRPC_SRC = \ src/core/lib/gprpp/status_helper.cc \ src/core/lib/gprpp/time.cc \ src/core/lib/gprpp/time_averaged_stats.cc \ + src/core/lib/gprpp/validation_errors.cc \ src/core/lib/gprpp/work_serializer.cc \ src/core/lib/handshaker/proxy_mapper_registry.cc \ src/core/lib/http/format_request.cc \ @@ -1884,6 +1885,7 @@ LIBGRPC_UNSECURE_SRC = \ src/core/lib/gprpp/status_helper.cc \ src/core/lib/gprpp/time.cc \ src/core/lib/gprpp/time_averaged_stats.cc \ + src/core/lib/gprpp/validation_errors.cc \ src/core/lib/gprpp/work_serializer.cc \ src/core/lib/handshaker/proxy_mapper_registry.cc \ src/core/lib/http/format_request.cc \ diff --git a/build_autogenerated.yaml b/build_autogenerated.yaml index ad0b36f446f..6eff7c6e487 100644 --- a/build_autogenerated.yaml +++ b/build_autogenerated.yaml @@ -775,6 +775,7 @@ libs: - src/core/lib/gprpp/time.h - src/core/lib/gprpp/time_averaged_stats.h - src/core/lib/gprpp/unique_type_name.h + - src/core/lib/gprpp/validation_errors.h - src/core/lib/gprpp/work_serializer.h - src/core/lib/handshaker/proxy_mapper.h - src/core/lib/handshaker/proxy_mapper_registry.h @@ -1467,6 +1468,7 @@ libs: - src/core/lib/gprpp/status_helper.cc - src/core/lib/gprpp/time.cc - src/core/lib/gprpp/time_averaged_stats.cc + - src/core/lib/gprpp/validation_errors.cc - src/core/lib/gprpp/work_serializer.cc - src/core/lib/handshaker/proxy_mapper_registry.cc - src/core/lib/http/format_request.cc @@ -1973,6 +1975,7 @@ libs: - src/core/lib/gprpp/time.h - src/core/lib/gprpp/time_averaged_stats.h - src/core/lib/gprpp/unique_type_name.h + - src/core/lib/gprpp/validation_errors.h - src/core/lib/gprpp/work_serializer.h - src/core/lib/handshaker/proxy_mapper.h - src/core/lib/handshaker/proxy_mapper_registry.h @@ -2306,6 +2309,7 @@ libs: - src/core/lib/gprpp/status_helper.cc - src/core/lib/gprpp/time.cc - src/core/lib/gprpp/time_averaged_stats.cc + - src/core/lib/gprpp/validation_errors.cc - src/core/lib/gprpp/work_serializer.cc - src/core/lib/handshaker/proxy_mapper_registry.cc - src/core/lib/http/format_request.cc @@ -10472,6 +10476,15 @@ targets: - absl/strings:strings - absl/types:variant uses_polling: false +- name: validation_errors_test + gtest: true + build: test + language: c++ + headers: [] + src: + - test/core/gprpp/validation_errors_test.cc + deps: + - grpc_test_util - name: varint_test gtest: true build: test diff --git a/config.m4 b/config.m4 index 959e92be8cb..01b3d25f836 100644 --- a/config.m4 +++ b/config.m4 @@ -535,6 +535,7 @@ if test "$PHP_GRPC" != "no"; then src/core/lib/gprpp/time.cc \ src/core/lib/gprpp/time_averaged_stats.cc \ src/core/lib/gprpp/time_util.cc \ + src/core/lib/gprpp/validation_errors.cc \ src/core/lib/gprpp/work_serializer.cc \ src/core/lib/handshaker/proxy_mapper_registry.cc \ src/core/lib/http/format_request.cc \ diff --git a/config.w32 b/config.w32 index e625fd78b91..cb5982a3612 100644 --- a/config.w32 +++ b/config.w32 @@ -501,6 +501,7 @@ if (PHP_GRPC != "no") { "src\\core\\lib\\gprpp\\time.cc " + "src\\core\\lib\\gprpp\\time_averaged_stats.cc " + "src\\core\\lib\\gprpp\\time_util.cc " + + "src\\core\\lib\\gprpp\\validation_errors.cc " + "src\\core\\lib\\gprpp\\work_serializer.cc " + "src\\core\\lib\\handshaker\\proxy_mapper_registry.cc " + "src\\core\\lib\\http\\format_request.cc " + diff --git a/gRPC-C++.podspec b/gRPC-C++.podspec index 1cc018f113b..0cadab78990 100644 --- a/gRPC-C++.podspec +++ b/gRPC-C++.podspec @@ -748,6 +748,7 @@ Pod::Spec.new do |s| 'src/core/lib/gprpp/time_averaged_stats.h', 'src/core/lib/gprpp/time_util.h', 'src/core/lib/gprpp/unique_type_name.h', + 'src/core/lib/gprpp/validation_errors.h', 'src/core/lib/gprpp/work_serializer.h', 'src/core/lib/handshaker/proxy_mapper.h', 'src/core/lib/handshaker/proxy_mapper_registry.h', @@ -1611,6 +1612,7 @@ Pod::Spec.new do |s| 'src/core/lib/gprpp/time_averaged_stats.h', 'src/core/lib/gprpp/time_util.h', 'src/core/lib/gprpp/unique_type_name.h', + 'src/core/lib/gprpp/validation_errors.h', 'src/core/lib/gprpp/work_serializer.h', 'src/core/lib/handshaker/proxy_mapper.h', 'src/core/lib/handshaker/proxy_mapper_registry.h', diff --git a/gRPC-Core.podspec b/gRPC-Core.podspec index cdcc70ce913..d45c4a2f9dd 100644 --- a/gRPC-Core.podspec +++ b/gRPC-Core.podspec @@ -1179,6 +1179,8 @@ Pod::Spec.new do |s| 'src/core/lib/gprpp/time_util.cc', 'src/core/lib/gprpp/time_util.h', 'src/core/lib/gprpp/unique_type_name.h', + 'src/core/lib/gprpp/validation_errors.cc', + 'src/core/lib/gprpp/validation_errors.h', 'src/core/lib/gprpp/work_serializer.cc', 'src/core/lib/gprpp/work_serializer.h', 'src/core/lib/handshaker/proxy_mapper.h', @@ -2237,6 +2239,7 @@ Pod::Spec.new do |s| 'src/core/lib/gprpp/time_averaged_stats.h', 'src/core/lib/gprpp/time_util.h', 'src/core/lib/gprpp/unique_type_name.h', + 'src/core/lib/gprpp/validation_errors.h', 'src/core/lib/gprpp/work_serializer.h', 'src/core/lib/handshaker/proxy_mapper.h', 'src/core/lib/handshaker/proxy_mapper_registry.h', diff --git a/grpc.gemspec b/grpc.gemspec index d21c0f99ae0..a57552c0b78 100644 --- a/grpc.gemspec +++ b/grpc.gemspec @@ -1092,6 +1092,8 @@ Gem::Specification.new do |s| s.files += %w( src/core/lib/gprpp/time_util.cc ) s.files += %w( src/core/lib/gprpp/time_util.h ) s.files += %w( src/core/lib/gprpp/unique_type_name.h ) + s.files += %w( src/core/lib/gprpp/validation_errors.cc ) + s.files += %w( src/core/lib/gprpp/validation_errors.h ) s.files += %w( src/core/lib/gprpp/work_serializer.cc ) s.files += %w( src/core/lib/gprpp/work_serializer.h ) s.files += %w( src/core/lib/handshaker/proxy_mapper.h ) diff --git a/grpc.gyp b/grpc.gyp index 31d27a6be64..26d0f93f2bd 100644 --- a/grpc.gyp +++ b/grpc.gyp @@ -825,6 +825,7 @@ 'src/core/lib/gprpp/status_helper.cc', 'src/core/lib/gprpp/time.cc', 'src/core/lib/gprpp/time_averaged_stats.cc', + 'src/core/lib/gprpp/validation_errors.cc', 'src/core/lib/gprpp/work_serializer.cc', 'src/core/lib/handshaker/proxy_mapper_registry.cc', 'src/core/lib/http/format_request.cc', @@ -1275,6 +1276,7 @@ 'src/core/lib/gprpp/status_helper.cc', 'src/core/lib/gprpp/time.cc', 'src/core/lib/gprpp/time_averaged_stats.cc', + 'src/core/lib/gprpp/validation_errors.cc', 'src/core/lib/gprpp/work_serializer.cc', 'src/core/lib/handshaker/proxy_mapper_registry.cc', 'src/core/lib/http/format_request.cc', diff --git a/package.xml b/package.xml index ad9ee75ee91..5f4aa3cc9ff 100644 --- a/package.xml +++ b/package.xml @@ -1074,6 +1074,8 @@ + + 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 17569105014..ae55f6beb56 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 @@ -114,6 +114,7 @@ #include "src/core/lib/gprpp/ref_counted.h" #include "src/core/lib/gprpp/ref_counted_ptr.h" #include "src/core/lib/gprpp/time.h" +#include "src/core/lib/gprpp/validation_errors.h" #include "src/core/lib/gprpp/work_serializer.h" #include "src/core/lib/iomgr/closure.h" #include "src/core/lib/iomgr/error.h" @@ -184,8 +185,9 @@ class GrpcLbConfig : public LoadBalancingPolicy::Config { return loader; } - void JsonPostLoad(const Json& json, const JsonArgs&, ErrorList* errors) { - ScopedField field(errors, ".childPolicy"); + void JsonPostLoad(const Json& json, const JsonArgs&, + ValidationErrors* errors) { + ValidationErrors::ScopedField field(errors, ".childPolicy"); Json child_policy_config_json_tmp; const Json* child_policy_config_json; auto it = json.object_value().find("childPolicy"); diff --git a/src/core/ext/filters/client_channel/lb_policy/outlier_detection/outlier_detection.cc b/src/core/ext/filters/client_channel/lb_policy/outlier_detection/outlier_detection.cc index 0f70b75456e..d94ffaf3d56 100644 --- a/src/core/ext/filters/client_channel/lb_policy/outlier_detection/outlier_detection.cc +++ b/src/core/ext/filters/client_channel/lb_policy/outlier_detection/outlier_detection.cc @@ -53,6 +53,7 @@ #include "src/core/lib/gprpp/orphanable.h" #include "src/core/lib/gprpp/ref_counted.h" #include "src/core/lib/gprpp/ref_counted_ptr.h" +#include "src/core/lib/gprpp/validation_errors.h" #include "src/core/lib/gprpp/work_serializer.h" #include "src/core/lib/iomgr/closure.h" #include "src/core/lib/iomgr/error.h" @@ -1032,16 +1033,17 @@ class OutlierDetectionLbFactory : public LoadBalancingPolicyFactory { "configuration. Please use loadBalancingConfig field of service " "config instead."); } - ErrorList errors; + ValidationErrors errors; OutlierDetectionConfig outlier_detection_config; RefCountedPtr child_policy; { - ScopedField field(&errors, "[\"outlier_detection_experimental\"]"); + ValidationErrors::ScopedField field( + &errors, "[\"outlier_detection_experimental\"]"); outlier_detection_config = LoadFromJson(json, JsonArgs(), &errors); // Parse childPolicy manually. { - ScopedField field(&errors, ".childPolicy"); + ValidationErrors::ScopedField field(&errors, ".childPolicy"); auto it = json.object_value().find("childPolicy"); if (it == json.object_value().end()) { errors.AddError("field not present"); @@ -1119,7 +1121,7 @@ const JsonLoaderInterface* OutlierDetectionConfig::JsonLoader(const JsonArgs&) { } void OutlierDetectionConfig::JsonPostLoad(const Json& json, const JsonArgs&, - ErrorList* /*errors*/) { + ValidationErrors* /*errors*/) { if (json.object_value().find("maxEjectionTime") == json.object_value().end()) { max_ejection_time = std::max(base_ejection_time, Duration::Seconds(300)); diff --git a/src/core/ext/filters/client_channel/lb_policy/outlier_detection/outlier_detection.h b/src/core/ext/filters/client_channel/lb_policy/outlier_detection/outlier_detection.h index 02f951236a5..b4d1494d205 100644 --- a/src/core/ext/filters/client_channel/lb_policy/outlier_detection/outlier_detection.h +++ b/src/core/ext/filters/client_channel/lb_policy/outlier_detection/outlier_detection.h @@ -24,6 +24,7 @@ #include "absl/types/optional.h" #include "src/core/lib/gprpp/time.h" +#include "src/core/lib/gprpp/validation_errors.h" #include "src/core/lib/json/json.h" #include "src/core/lib/json/json_args.h" #include "src/core/lib/json/json_object_loader.h" @@ -80,7 +81,8 @@ struct OutlierDetectionConfig { } static const JsonLoaderInterface* JsonLoader(const JsonArgs&); - void JsonPostLoad(const Json& json, const JsonArgs&, ErrorList* errors); + void JsonPostLoad(const Json& json, const JsonArgs&, + ValidationErrors* errors); }; } // namespace grpc_core diff --git a/src/core/ext/filters/client_channel/lb_policy/priority/priority.cc b/src/core/ext/filters/client_channel/lb_policy/priority/priority.cc index 61fb5c069d2..43445c66b95 100644 --- a/src/core/ext/filters/client_channel/lb_policy/priority/priority.cc +++ b/src/core/ext/filters/client_channel/lb_policy/priority/priority.cc @@ -48,6 +48,7 @@ #include "src/core/lib/gprpp/ref_counted.h" #include "src/core/lib/gprpp/ref_counted_ptr.h" #include "src/core/lib/gprpp/time.h" +#include "src/core/lib/gprpp/validation_errors.h" #include "src/core/lib/gprpp/work_serializer.h" #include "src/core/lib/iomgr/closure.h" #include "src/core/lib/iomgr/error.h" @@ -89,7 +90,8 @@ class PriorityLbConfig : public LoadBalancingPolicy::Config { bool ignore_reresolution_requests = false; static const JsonLoaderInterface* JsonLoader(const JsonArgs&); - void JsonPostLoad(const Json& json, const JsonArgs&, ErrorList* errors); + void JsonPostLoad(const Json& json, const JsonArgs&, + ValidationErrors* errors); }; PriorityLbConfig() = default; @@ -108,7 +110,8 @@ class PriorityLbConfig : public LoadBalancingPolicy::Config { const std::vector& priorities() const { return priorities_; } static const JsonLoaderInterface* JsonLoader(const JsonArgs&); - void JsonPostLoad(const Json& json, const JsonArgs&, ErrorList* errors); + void JsonPostLoad(const Json& json, const JsonArgs&, + ValidationErrors* errors); private: std::map children_; @@ -877,8 +880,8 @@ const JsonLoaderInterface* PriorityLbConfig::PriorityLbChild::JsonLoader( void PriorityLbConfig::PriorityLbChild::JsonPostLoad(const Json& json, const JsonArgs&, - ErrorList* errors) { - ScopedField field(errors, ".config"); + ValidationErrors* errors) { + ValidationErrors::ScopedField field(errors, ".config"); auto it = json.object_value().find("config"); if (it == json.object_value().end()) { errors->AddError("field not present"); @@ -904,7 +907,7 @@ const JsonLoaderInterface* PriorityLbConfig::JsonLoader(const JsonArgs&) { } void PriorityLbConfig::JsonPostLoad(const Json& /*json*/, const JsonArgs&, - ErrorList* errors) { + ValidationErrors* errors) { std::set unknown_priorities; for (const std::string& priority : priorities_) { if (children_.find(priority) == children_.end()) { diff --git a/src/core/ext/filters/client_channel/lb_policy/ring_hash/ring_hash.cc b/src/core/ext/filters/client_channel/lb_policy/ring_hash/ring_hash.cc index bfd2ba4594d..8a8356fa13f 100644 --- a/src/core/ext/filters/client_channel/lb_policy/ring_hash/ring_hash.cc +++ b/src/core/ext/filters/client_channel/lb_policy/ring_hash/ring_hash.cc @@ -90,16 +90,16 @@ const JsonLoaderInterface* RingHashConfig::JsonLoader(const JsonArgs&) { } void RingHashConfig::JsonPostLoad(const Json&, const JsonArgs&, - ErrorList* errors) { + ValidationErrors* errors) { { - ScopedField field(errors, ".min_ring_size"); + ValidationErrors::ScopedField field(errors, ".min_ring_size"); if (!errors->FieldHasErrors() && (min_ring_size == 0 || min_ring_size > 8388608)) { errors->AddError("must be in the range [1, 8388608]"); } } { - ScopedField field(errors, ".max_ring_size"); + ValidationErrors::ScopedField field(errors, ".max_ring_size"); if (!errors->FieldHasErrors() && (max_ring_size == 0 || max_ring_size > 8388608)) { errors->AddError("must be in the range [1, 8388608]"); diff --git a/src/core/ext/filters/client_channel/lb_policy/ring_hash/ring_hash.h b/src/core/ext/filters/client_channel/lb_policy/ring_hash/ring_hash.h index 9b498eb03f8..ecb19c9963a 100644 --- a/src/core/ext/filters/client_channel/lb_policy/ring_hash/ring_hash.h +++ b/src/core/ext/filters/client_channel/lb_policy/ring_hash/ring_hash.h @@ -22,6 +22,7 @@ #include #include "src/core/lib/gprpp/unique_type_name.h" +#include "src/core/lib/gprpp/validation_errors.h" #include "src/core/lib/json/json.h" #include "src/core/lib/json/json_args.h" #include "src/core/lib/json/json_object_loader.h" @@ -37,7 +38,8 @@ struct RingHashConfig { uint64_t max_ring_size = 8388608; static const JsonLoaderInterface* JsonLoader(const JsonArgs&); - void JsonPostLoad(const Json& json, const JsonArgs&, ErrorList* errors); + void JsonPostLoad(const Json& json, const JsonArgs&, + ValidationErrors* errors); }; } // namespace grpc_core diff --git a/src/core/ext/filters/client_channel/lb_policy/rls/rls.cc b/src/core/ext/filters/client_channel/lb_policy/rls/rls.cc index 999bdc62059..120a807c547 100644 --- a/src/core/ext/filters/client_channel/lb_policy/rls/rls.cc +++ b/src/core/ext/filters/client_channel/lb_policy/rls/rls.cc @@ -75,6 +75,7 @@ #include "src/core/lib/gprpp/ref_counted_ptr.h" #include "src/core/lib/gprpp/sync.h" #include "src/core/lib/gprpp/time.h" +#include "src/core/lib/gprpp/validation_errors.h" #include "src/core/lib/gprpp/work_serializer.h" #include "src/core/lib/iomgr/closure.h" #include "src/core/lib/iomgr/error.h" @@ -151,7 +152,7 @@ class RlsLbConfig : public LoadBalancingPolicy::Config { static const JsonLoaderInterface* JsonLoader(const JsonArgs&); void JsonPostLoad(const Json& json, const JsonArgs& args, - ErrorList* errors); + ValidationErrors* errors); }; RlsLbConfig() = default; @@ -194,7 +195,8 @@ class RlsLbConfig : public LoadBalancingPolicy::Config { } static const JsonLoaderInterface* JsonLoader(const JsonArgs&); - void JsonPostLoad(const Json& json, const JsonArgs&, ErrorList* errors); + void JsonPostLoad(const Json& json, const JsonArgs&, + ValidationErrors* errors); private: RouteLookupConfig route_lookup_config_; @@ -747,7 +749,7 @@ void RlsLb::ChildPolicyWrapper::Orphan() { bool InsertOrUpdateChildPolicyField(const std::string& field, const std::string& value, Json* config, - ErrorList* errors) { + ValidationErrors* errors) { if (config->type() != Json::Type::ARRAY) { errors->AddError("is not an array"); return false; @@ -755,7 +757,7 @@ bool InsertOrUpdateChildPolicyField(const std::string& field, bool success = true; for (size_t i = 0; i < config->array_value().size(); ++i) { Json& child_json = (*config->mutable_array())[i]; - ScopedField json_field(errors, absl::StrCat("[", i, "]")); + ValidationErrors::ScopedField json_field(errors, absl::StrCat("[", i, "]")); if (child_json.type() != Json::Type::OBJECT) { errors->AddError("is not an object"); success = false; @@ -765,7 +767,7 @@ bool InsertOrUpdateChildPolicyField(const std::string& field, errors->AddError("child policy object contains more than one field"); success = false; } else { - ScopedField json_field( + ValidationErrors::ScopedField json_field( errors, absl::StrCat("[\"", child.begin()->first, "\"]")); Json& child_config_json = child.begin()->second; if (child_config_json.type() != Json::Type::OBJECT) { @@ -783,7 +785,7 @@ bool InsertOrUpdateChildPolicyField(const std::string& field, void RlsLb::ChildPolicyWrapper::StartUpdate() { Json child_policy_config = lb_policy_->config_->child_policy_config(); - ErrorList errors; + ValidationErrors errors; GPR_ASSERT(InsertOrUpdateChildPolicyField( lb_policy_->config_->child_policy_config_target_field_name(), target_, &child_policy_config, &errors)); @@ -2176,23 +2178,24 @@ struct GrpcKeyBuilder { return loader; } - void JsonPostLoad(const Json&, const JsonArgs&, ErrorList* errors) { + void JsonPostLoad(const Json&, const JsonArgs&, ValidationErrors* errors) { // key must be non-empty. { - ScopedField field(errors, ".key"); + ValidationErrors::ScopedField field(errors, ".key"); if (!errors->FieldHasErrors() && key.empty()) { errors->AddError("must be non-empty"); } } // List of header names must be non-empty. { - ScopedField field(errors, ".names"); + ValidationErrors::ScopedField field(errors, ".names"); if (!errors->FieldHasErrors() && names.empty()) { errors->AddError("must be non-empty"); } // Individual header names must be non-empty. for (size_t i = 0; i < names.size(); ++i) { - ScopedField field(errors, absl::StrCat("[", i, "]")); + ValidationErrors::ScopedField field(errors, + absl::StrCat("[", i, "]")); if (!errors->FieldHasErrors() && names[i].empty()) { errors->AddError("must be non-empty"); } @@ -2200,7 +2203,7 @@ struct GrpcKeyBuilder { } // requiredMatch must not be present. { - ScopedField field(errors, ".requiredMatch"); + ValidationErrors::ScopedField field(errors, ".requiredMatch"); if (required_match.has_value()) { errors->AddError("must not be present"); } @@ -2223,10 +2226,11 @@ struct GrpcKeyBuilder { return loader; } - void JsonPostLoad(const Json&, const JsonArgs&, ErrorList* errors) { + void JsonPostLoad(const Json&, const JsonArgs&, ValidationErrors* errors) { auto check_field = [&](const std::string& field_name, absl::optional* struct_field) { - ScopedField field(errors, absl::StrCat(".", field_name)); + ValidationErrors::ScopedField field(errors, + absl::StrCat(".", field_name)); if (struct_field->has_value() && (*struct_field)->empty()) { errors->AddError("must be non-empty if set"); } @@ -2253,17 +2257,17 @@ struct GrpcKeyBuilder { return loader; } - void JsonPostLoad(const Json&, const JsonArgs&, ErrorList* errors) { + void JsonPostLoad(const Json&, const JsonArgs&, ValidationErrors* errors) { // The names field must be non-empty. { - ScopedField field(errors, ".names"); + ValidationErrors::ScopedField field(errors, ".names"); if (!errors->FieldHasErrors() && names.empty()) { errors->AddError("must be non-empty"); } } // Make sure no key in constantKeys is empty. if (constant_keys.find("") != constant_keys.end()) { - ScopedField field(errors, ".constantKeys[\"\"]"); + ValidationErrors::ScopedField field(errors, ".constantKeys[\"\"]"); errors->AddError("key must be non-empty"); } // Check for duplicate keys. @@ -2272,7 +2276,7 @@ struct GrpcKeyBuilder { const std::string& key, const std::string& field_name) { if (key.empty()) return; // Already generated an error about this. - ScopedField field(errors, field_name); + ValidationErrors::ScopedField field(errors, field_name); auto it = keys_seen.find(key); if (it != keys_seen.end()) { errors->AddError(absl::StrCat("duplicate key \"", key, "\"")); @@ -2320,14 +2324,14 @@ const JsonLoaderInterface* RlsLbConfig::RouteLookupConfig::JsonLoader( void RlsLbConfig::RouteLookupConfig::JsonPostLoad(const Json& json, const JsonArgs& args, - ErrorList* errors) { + ValidationErrors* errors) { // Parse grpcKeybuilders. auto grpc_keybuilders = LoadJsonObjectField>( json.object_value(), args, "grpcKeybuilders", errors); if (grpc_keybuilders.has_value()) { - ScopedField field(errors, ".grpcKeybuilders"); + ValidationErrors::ScopedField field(errors, ".grpcKeybuilders"); for (size_t i = 0; i < grpc_keybuilders->size(); ++i) { - ScopedField field(errors, absl::StrCat("[", i, "]")); + ValidationErrors::ScopedField field(errors, absl::StrCat("[", i, "]")); auto& grpc_keybuilder = (*grpc_keybuilders)[i]; // Construct KeyBuilder. RlsLbConfig::KeyBuilder key_builder; @@ -2358,7 +2362,7 @@ void RlsLbConfig::RouteLookupConfig::JsonPostLoad(const Json& json, } // Validate lookupService. { - ScopedField field(errors, ".lookupService"); + ValidationErrors::ScopedField field(errors, ".lookupService"); if (!errors->FieldHasErrors() && !CoreConfiguration::Get().resolver_registry().IsValidTarget( lookup_service)) { @@ -2370,14 +2374,14 @@ void RlsLbConfig::RouteLookupConfig::JsonPostLoad(const Json& json, // If staleAge is set, then maxAge must also be set. if (json.object_value().find("staleAge") != json.object_value().end() && json.object_value().find("maxAge") == json.object_value().end()) { - ScopedField field(errors, ".maxAge"); + ValidationErrors::ScopedField field(errors, ".maxAge"); errors->AddError("must be set if staleAge is set"); } // Ignore staleAge if greater than or equal to maxAge. if (stale_age >= max_age) stale_age = max_age; // Validate cacheSizeBytes. { - ScopedField field(errors, ".cacheSizeBytes"); + ValidationErrors::ScopedField field(errors, ".cacheSizeBytes"); if (!errors->FieldHasErrors() && cache_size_bytes <= 0) { errors->AddError("must be greater than 0"); } @@ -2388,7 +2392,7 @@ void RlsLbConfig::RouteLookupConfig::JsonPostLoad(const Json& json, } // Validate defaultTarget. { - ScopedField field(errors, ".defaultTarget"); + ValidationErrors::ScopedField field(errors, ".defaultTarget"); if (!errors->FieldHasErrors() && json.object_value().find("defaultTarget") != json.object_value().end() && @@ -2411,11 +2415,12 @@ const JsonLoaderInterface* RlsLbConfig::JsonLoader(const JsonArgs&) { } void RlsLbConfig::JsonPostLoad(const Json& json, const JsonArgs&, - ErrorList* errors) { + ValidationErrors* errors) { // Parse routeLookupChannelServiceConfig. auto it = json.object_value().find("routeLookupChannelServiceConfig"); if (it != json.object_value().end()) { - ScopedField field(errors, ".routeLookupChannelServiceConfig"); + ValidationErrors::ScopedField field(errors, + ".routeLookupChannelServiceConfig"); grpc_error_handle child_error = GRPC_ERROR_NONE; rls_channel_service_config_ = it->second.Dump(); auto service_config = MakeRefCounted( @@ -2427,7 +2432,8 @@ void RlsLbConfig::JsonPostLoad(const Json& json, const JsonArgs&, } // Validate childPolicyConfigTargetFieldName. { - ScopedField field(errors, ".childPolicyConfigTargetFieldName"); + ValidationErrors::ScopedField field(errors, + ".childPolicyConfigTargetFieldName"); if (!errors->FieldHasErrors() && child_policy_config_target_field_name_.empty()) { errors->AddError("must be non-empty"); @@ -2435,7 +2441,7 @@ void RlsLbConfig::JsonPostLoad(const Json& json, const JsonArgs&, } // Parse childPolicy. { - ScopedField field(errors, ".childPolicy"); + ValidationErrors::ScopedField field(errors, ".childPolicy"); auto it = json.object_value().find("childPolicy"); if (it == json.object_value().end()) { errors->AddError("field not present"); diff --git a/src/core/ext/filters/client_channel/lb_policy/weighted_target/weighted_target.cc b/src/core/ext/filters/client_channel/lb_policy/weighted_target/weighted_target.cc index 7336b7608a3..7f35acc476e 100644 --- a/src/core/ext/filters/client_channel/lb_policy/weighted_target/weighted_target.cc +++ b/src/core/ext/filters/client_channel/lb_policy/weighted_target/weighted_target.cc @@ -49,6 +49,7 @@ #include "src/core/lib/gprpp/ref_counted.h" #include "src/core/lib/gprpp/ref_counted_ptr.h" #include "src/core/lib/gprpp/time.h" +#include "src/core/lib/gprpp/validation_errors.h" #include "src/core/lib/gprpp/work_serializer.h" #include "src/core/lib/iomgr/exec_ctx.h" #include "src/core/lib/iomgr/pollset_set.h" @@ -87,7 +88,8 @@ class WeightedTargetLbConfig : public LoadBalancingPolicy::Config { RefCountedPtr config; static const JsonLoaderInterface* JsonLoader(const JsonArgs&); - void JsonPostLoad(const Json& json, const JsonArgs&, ErrorList* errors); + void JsonPostLoad(const Json& json, const JsonArgs&, + ValidationErrors* errors); }; using TargetMap = std::map; @@ -713,10 +715,9 @@ const JsonLoaderInterface* WeightedTargetLbConfig::ChildConfig::JsonLoader( return loader; } -void WeightedTargetLbConfig::ChildConfig::JsonPostLoad(const Json& json, - const JsonArgs&, - ErrorList* errors) { - ScopedField field(errors, ".childPolicy"); +void WeightedTargetLbConfig::ChildConfig::JsonPostLoad( + const Json& json, const JsonArgs&, ValidationErrors* errors) { + ValidationErrors::ScopedField field(errors, ".childPolicy"); auto it = json.object_value().find("childPolicy"); if (it == json.object_value().end()) { errors->AddError("field not present"); diff --git a/src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_impl.cc b/src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_impl.cc index 067471700ba..391f7f11f38 100644 --- a/src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_impl.cc +++ b/src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_impl.cc @@ -55,6 +55,7 @@ #include "src/core/lib/gprpp/ref_counted.h" #include "src/core/lib/gprpp/ref_counted_ptr.h" #include "src/core/lib/gprpp/sync.h" +#include "src/core/lib/gprpp/validation_errors.h" #include "src/core/lib/iomgr/pollset_set.h" #include "src/core/lib/json/json.h" #include "src/core/lib/json/json_args.h" @@ -169,7 +170,8 @@ class XdsClusterImplLbConfig : public LoadBalancingPolicy::Config { } static const JsonLoaderInterface* JsonLoader(const JsonArgs&); - void JsonPostLoad(const Json& json, const JsonArgs& args, ErrorList* errors); + void JsonPostLoad(const Json& json, const JsonArgs& args, + ValidationErrors* errors); private: RefCountedPtr child_policy_; @@ -722,11 +724,11 @@ const JsonLoaderInterface* XdsClusterImplLbConfig::JsonLoader(const JsonArgs&) { void XdsClusterImplLbConfig::JsonPostLoad(const Json& json, const JsonArgs& args, - ErrorList* errors) { + ValidationErrors* errors) { // LRS load reporting server name. auto it = json.object_value().find("lrsLoadReportingServer"); if (it != json.object_value().end()) { - ScopedField field(errors, ".lrsLoadReportingServer"); + ValidationErrors::ScopedField field(errors, ".lrsLoadReportingServer"); if (it->second.type() != Json::Type::OBJECT) { errors->AddError("is not an object"); } else { @@ -741,7 +743,7 @@ void XdsClusterImplLbConfig::JsonPostLoad(const Json& json, } // Parse "childPolicy" field. { - ScopedField field(errors, ".childPolicy"); + ValidationErrors::ScopedField field(errors, ".childPolicy"); auto it = json.object_value().find("childPolicy"); if (it == json.object_value().end()) { errors->AddError("field not present"); diff --git a/src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_manager.cc b/src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_manager.cc index 17fd238dae7..90e3c7aba7a 100644 --- a/src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_manager.cc +++ b/src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_manager.cc @@ -46,6 +46,7 @@ #include "src/core/lib/gprpp/ref_counted.h" #include "src/core/lib/gprpp/ref_counted_ptr.h" #include "src/core/lib/gprpp/time.h" +#include "src/core/lib/gprpp/validation_errors.h" #include "src/core/lib/gprpp/work_serializer.h" #include "src/core/lib/iomgr/closure.h" #include "src/core/lib/iomgr/error.h" @@ -80,7 +81,8 @@ class XdsClusterManagerLbConfig : public LoadBalancingPolicy::Config { RefCountedPtr config; static const JsonLoaderInterface* JsonLoader(const JsonArgs&); - void JsonPostLoad(const Json& json, const JsonArgs&, ErrorList* errors); + void JsonPostLoad(const Json& json, const JsonArgs&, + ValidationErrors* errors); }; XdsClusterManagerLbConfig() = default; @@ -100,7 +102,8 @@ class XdsClusterManagerLbConfig : public LoadBalancingPolicy::Config { } static const JsonLoaderInterface* JsonLoader(const JsonArgs&); - void JsonPostLoad(const Json& json, const JsonArgs&, ErrorList* errors); + void JsonPostLoad(const Json& json, const JsonArgs&, + ValidationErrors* errors); private: std::map cluster_map_; @@ -658,8 +661,8 @@ const JsonLoaderInterface* XdsClusterManagerLbConfig::Child::JsonLoader( void XdsClusterManagerLbConfig::Child::JsonPostLoad(const Json& json, const JsonArgs&, - ErrorList* errors) { - ScopedField field(errors, ".childPolicy"); + ValidationErrors* errors) { + ValidationErrors::ScopedField field(errors, ".childPolicy"); auto it = json.object_value().find("childPolicy"); if (it == json.object_value().end()) { errors->AddError("field not present"); @@ -685,9 +688,9 @@ const JsonLoaderInterface* XdsClusterManagerLbConfig::JsonLoader( } void XdsClusterManagerLbConfig::JsonPostLoad(const Json&, const JsonArgs&, - ErrorList* errors) { + ValidationErrors* errors) { if (cluster_map_.empty()) { - ScopedField field(errors, ".children"); + ValidationErrors::ScopedField field(errors, ".children"); if (!errors->FieldHasErrors()) { errors->AddError("no valid children configured"); } diff --git a/src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_resolver.cc b/src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_resolver.cc index e63fa58d8d5..ddc0feedadf 100644 --- a/src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_resolver.cc +++ b/src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_resolver.cc @@ -59,6 +59,7 @@ #include "src/core/lib/gprpp/debug_location.h" #include "src/core/lib/gprpp/orphanable.h" #include "src/core/lib/gprpp/ref_counted_ptr.h" +#include "src/core/lib/gprpp/validation_errors.h" #include "src/core/lib/gprpp/work_serializer.h" #include "src/core/lib/iomgr/pollset_set.h" #include "src/core/lib/json/json.h" @@ -121,7 +122,7 @@ class XdsClusterResolverLbConfig : public LoadBalancingPolicy::Config { static const JsonLoaderInterface* JsonLoader(const JsonArgs&); void JsonPostLoad(const Json& json, const JsonArgs& args, - ErrorList* errors); + ValidationErrors* errors); }; XdsClusterResolverLbConfig() = default; @@ -143,7 +144,8 @@ class XdsClusterResolverLbConfig : public LoadBalancingPolicy::Config { const Json& xds_lb_policy() const { return xds_lb_policy_; } static const JsonLoaderInterface* JsonLoader(const JsonArgs&); - void JsonPostLoad(const Json& json, const JsonArgs& args, ErrorList* errors); + void JsonPostLoad(const Json& json, const JsonArgs& args, + ValidationErrors* errors); private: std::vector discovery_mechanisms_; @@ -1096,11 +1098,11 @@ XdsClusterResolverLbConfig::DiscoveryMechanism::JsonLoader(const JsonArgs&) { } void XdsClusterResolverLbConfig::DiscoveryMechanism::JsonPostLoad( - const Json& json, const JsonArgs& args, ErrorList* errors) { + const Json& json, const JsonArgs& args, ValidationErrors* errors) { // LRS load reporting server name. auto it = json.object_value().find("lrsLoadReportingServer"); if (it != json.object_value().end()) { - ScopedField field(errors, ".lrsLoadReportingServer"); + ValidationErrors::ScopedField field(errors, ".lrsLoadReportingServer"); if (it->second.type() != Json::Type::OBJECT) { errors->AddError("is not an object"); } else { @@ -1123,7 +1125,7 @@ void XdsClusterResolverLbConfig::DiscoveryMechanism::JsonPostLoad( } else if (*type_field == "LOGICAL_DNS") { type = DiscoveryMechanismType::LOGICAL_DNS; } else { - ScopedField field(errors, ".type"); + ValidationErrors::ScopedField field(errors, ".type"); errors->AddError(absl::StrCat("unknown type \"", *type_field, "\"")); } } @@ -1157,17 +1159,17 @@ const JsonLoaderInterface* XdsClusterResolverLbConfig::JsonLoader( void XdsClusterResolverLbConfig::JsonPostLoad(const Json& json, const JsonArgs& args, - ErrorList* errors) { + ValidationErrors* errors) { // Validate discoveryMechanisms. { - ScopedField field(errors, ".discoveryMechanisms"); + ValidationErrors::ScopedField field(errors, ".discoveryMechanisms"); if (!errors->FieldHasErrors() && discovery_mechanisms_.empty()) { errors->AddError("must be non-empty"); } } // Parse "xdsLbPolicy". { - ScopedField field(errors, ".xdsLbPolicy"); + ValidationErrors::ScopedField field(errors, ".xdsLbPolicy"); auto it = json.object_value().find("xdsLbPolicy"); if (it != json.object_value().end()) { if (it->second.type() != Json::Type::ARRAY) { @@ -1175,7 +1177,8 @@ void XdsClusterResolverLbConfig::JsonPostLoad(const Json& json, } else { const Json::Array& array = it->second.array_value(); for (size_t i = 0; i < array.size(); ++i) { - ScopedField field(errors, absl::StrCat("[", i, "]")); + ValidationErrors::ScopedField field(errors, + absl::StrCat("[", i, "]")); if (array[i].type() != Json::Type::OBJECT) { errors->AddError("is not an object"); continue; @@ -1183,14 +1186,14 @@ void XdsClusterResolverLbConfig::JsonPostLoad(const Json& json, const Json::Object& policy = array[i].object_value(); auto policy_it = policy.find("ROUND_ROBIN"); if (policy_it != policy.end()) { - ScopedField field(errors, "[\"ROUND_ROBIN\"]"); + ValidationErrors::ScopedField field(errors, "[\"ROUND_ROBIN\"]"); if (policy_it->second.type() != Json::Type::OBJECT) { errors->AddError("is not an object"); } break; } { - ScopedField field(errors, "[\"RING_HASH\"]"); + ValidationErrors::ScopedField field(errors, "[\"RING_HASH\"]"); policy_it = policy.find("RING_HASH"); if (policy_it != policy.end()) { LoadFromJson(policy_it->second, args, errors); diff --git a/src/core/ext/xds/certificate_provider_store.cc b/src/core/ext/xds/certificate_provider_store.cc index b78129dcd9b..1e32ec3d67a 100644 --- a/src/core/ext/xds/certificate_provider_store.cc +++ b/src/core/ext/xds/certificate_provider_store.cc @@ -44,11 +44,11 @@ CertificateProviderStore::PluginDefinition::JsonLoader(const JsonArgs&) { } void CertificateProviderStore::PluginDefinition::JsonPostLoad( - const Json& json, const JsonArgs&, ErrorList* errors) { + const Json& json, const JsonArgs&, ValidationErrors* errors) { // Check that plugin is supported. CertificateProviderFactory* factory = nullptr; if (!plugin_name.empty()) { - ScopedField field(errors, ".plugin_name"); + ValidationErrors::ScopedField field(errors, ".plugin_name"); factory = CoreConfiguration::Get() .certificate_provider_registry() .LookupCertificateProviderFactory(plugin_name); @@ -59,7 +59,7 @@ void CertificateProviderStore::PluginDefinition::JsonPostLoad( } // Parse the config field. { - ScopedField field(errors, ".config"); + ValidationErrors::ScopedField field(errors, ".config"); auto it = json.object_value().find("config"); // The config field is optional; if not present, we use an empty JSON // object. diff --git a/src/core/ext/xds/certificate_provider_store.h b/src/core/ext/xds/certificate_provider_store.h index c945d6b9a55..8bcff00f3bc 100644 --- a/src/core/ext/xds/certificate_provider_store.h +++ b/src/core/ext/xds/certificate_provider_store.h @@ -35,6 +35,7 @@ #include "src/core/lib/gprpp/ref_counted_ptr.h" #include "src/core/lib/gprpp/sync.h" #include "src/core/lib/gprpp/unique_type_name.h" +#include "src/core/lib/gprpp/validation_errors.h" #include "src/core/lib/iomgr/iomgr_fwd.h" #include "src/core/lib/json/json.h" #include "src/core/lib/json/json_args.h" @@ -54,7 +55,8 @@ class CertificateProviderStore RefCountedPtr config; static const JsonLoaderInterface* JsonLoader(const JsonArgs&); - void JsonPostLoad(const Json& json, const JsonArgs&, ErrorList* errors); + void JsonPostLoad(const Json& json, const JsonArgs&, + ValidationErrors* errors); }; // Maps plugin instance (opaque) name to plugin defition. diff --git a/src/core/ext/xds/xds_bootstrap_grpc.cc b/src/core/ext/xds/xds_bootstrap_grpc.cc index 593306a1674..177fcade9b4 100644 --- a/src/core/ext/xds/xds_bootstrap_grpc.cc +++ b/src/core/ext/xds/xds_bootstrap_grpc.cc @@ -130,14 +130,14 @@ const JsonLoaderInterface* GrpcXdsBootstrap::GrpcXdsServer::JsonLoader( void GrpcXdsBootstrap::GrpcXdsServer::JsonPostLoad(const Json& json, const JsonArgs& args, - ErrorList* errors) { + ValidationErrors* errors) { // Parse "channel_creds". auto channel_creds_list = LoadJsonObjectField>( json.object_value(), args, "channel_creds", errors); if (channel_creds_list.has_value()) { - ScopedField field(errors, ".channel_creds"); + ValidationErrors::ScopedField field(errors, ".channel_creds"); for (size_t i = 0; i < channel_creds_list->size(); ++i) { - ScopedField field(errors, absl::StrCat("[", i, "]")); + ValidationErrors::ScopedField field(errors, absl::StrCat("[", i, "]")); auto& creds = (*channel_creds_list)[i]; // Select the first channel creds type that we support. if (channel_creds_.type.empty() && @@ -159,7 +159,7 @@ void GrpcXdsBootstrap::GrpcXdsServer::JsonPostLoad(const Json& json, } // Parse "server_features". { - ScopedField field(errors, ".server_features"); + ValidationErrors::ScopedField field(errors, ".server_features"); auto it = json.object_value().find("server_features"); if (it != json.object_value().end()) { if (it->second.type() != Json::Type::ARRAY) { @@ -260,16 +260,16 @@ const JsonLoaderInterface* GrpcXdsBootstrap::JsonLoader(const JsonArgs&) { void GrpcXdsBootstrap::JsonPostLoad(const Json& /*json*/, const JsonArgs& /*args*/, - ErrorList* errors) { + ValidationErrors* errors) { // Verify that each authority has the right prefix in the // client_listener_resource_name_template field. { - ScopedField field(errors, ".authorities"); + ValidationErrors::ScopedField field(errors, ".authorities"); for (const auto& p : authorities_) { const std::string& name = p.first; const GrpcAuthority& authority = static_cast(p.second); - ScopedField field( + ValidationErrors::ScopedField field( errors, absl::StrCat("[\"", name, "\"].client_listener_resource_name_template")); std::string expected_prefix = absl::StrCat("xdstp://", name, "/"); diff --git a/src/core/ext/xds/xds_bootstrap_grpc.h b/src/core/ext/xds/xds_bootstrap_grpc.h index d91f6a055f0..6ab7163b123 100644 --- a/src/core/ext/xds/xds_bootstrap_grpc.h +++ b/src/core/ext/xds/xds_bootstrap_grpc.h @@ -31,6 +31,7 @@ #include "src/core/ext/xds/certificate_provider_store.h" #include "src/core/ext/xds/xds_bootstrap.h" +#include "src/core/lib/gprpp/validation_errors.h" #include "src/core/lib/json/json.h" #include "src/core/lib/json/json_args.h" #include "src/core/lib/json/json_object_loader.h" @@ -87,7 +88,7 @@ class GrpcXdsBootstrap : public XdsBootstrap { static const JsonLoaderInterface* JsonLoader(const JsonArgs&); void JsonPostLoad(const Json& json, const JsonArgs& args, - ErrorList* errors); + ValidationErrors* errors); Json ToJson() const; @@ -126,7 +127,8 @@ class GrpcXdsBootstrap : public XdsBootstrap { absl::string_view json_string); static const JsonLoaderInterface* JsonLoader(const JsonArgs&); - void JsonPostLoad(const Json& json, const JsonArgs& args, ErrorList* errors); + void JsonPostLoad(const Json& json, const JsonArgs& args, + ValidationErrors* errors); std::string ToString() const override; diff --git a/src/core/lib/gprpp/validation_errors.cc b/src/core/lib/gprpp/validation_errors.cc new file mode 100644 index 00000000000..aee0aaacb9a --- /dev/null +++ b/src/core/lib/gprpp/validation_errors.cc @@ -0,0 +1,61 @@ +// Copyright 2020 gRPC authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include + +#include "src/core/lib/gprpp/validation_errors.h" + +#include +#include + +#include "absl/status/status.h" +#include "absl/strings/str_cat.h" +#include "absl/strings/str_join.h" +#include "absl/strings/strip.h" + +namespace grpc_core { + +void ValidationErrors::PushField(absl::string_view ext) { + // Skip leading '.' for top-level field names. + if (fields_.empty()) absl::ConsumePrefix(&ext, "."); + fields_.emplace_back(std::string(ext)); +} + +void ValidationErrors::PopField() { fields_.pop_back(); } + +void ValidationErrors::AddError(absl::string_view error) { + field_errors_[absl::StrJoin(fields_, "")].emplace_back(error); +} + +bool ValidationErrors::FieldHasErrors() const { + return field_errors_.find(absl::StrJoin(fields_, "")) != field_errors_.end(); +} + +absl::Status ValidationErrors::status(absl::string_view prefix) const { + if (field_errors_.empty()) return absl::OkStatus(); + std::vector errors; + for (const auto& p : field_errors_) { + if (p.second.size() > 1) { + errors.emplace_back(absl::StrCat("field:", p.first, " errors:[", + absl::StrJoin(p.second, "; "), "]")); + } else { + errors.emplace_back( + absl::StrCat("field:", p.first, " error:", p.second[0])); + } + } + return absl::InvalidArgumentError( + absl::StrCat(prefix, ": [", absl::StrJoin(errors, "; "), "]")); +} + +} // namespace grpc_core diff --git a/src/core/lib/gprpp/validation_errors.h b/src/core/lib/gprpp/validation_errors.h new file mode 100644 index 00000000000..e100b95b8e0 --- /dev/null +++ b/src/core/lib/gprpp/validation_errors.h @@ -0,0 +1,110 @@ +// Copyright 2020 gRPC authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef GRPC_CORE_LIB_GPRPP_VALIDATION_ERRORS_H +#define GRPC_CORE_LIB_GPRPP_VALIDATION_ERRORS_H + +#include + +#include + +#include +#include +#include + +#include "absl/status/status.h" +#include "absl/strings/string_view.h" + +namespace grpc_core { + +// Tracks errors that occur during validation of a data structure (e.g., +// a JSON object or protobuf message). Errors are tracked based on +// which field they are associated with. If at least one error occurs +// during validation, the validation failed. +// +// Example usage: +// +// absl::StatusOr GetFooBar(const Json::Object& json) { +// ValidationErrors errors; +// { +// ValidationErrors::ScopedField field("foo"); +// auto it = json.object_value().find("foo"); +// if (it == json.object_value().end()) { +// errors.AddError("field not present"); +// } else if (it->second.type() != Json::Type::OBJECT) { +// errors.AddError("must be a JSON object"); +// } else { +// const Json& foo = it->second; +// ValidationErrors::ScopedField field(".bar"); +// auto it = foo.object_value().find("bar"); +// if (it == json.object_value().end()) { +// errors.AddError("field not present"); +// } else if (it->second.type() != Json::Type::STRING) { +// errors.AddError("must be a JSON string"); +// } else { +// return it->second.string_value(); +// } +// } +// } +// return errors.status("errors validating foo.bar"); +// } +class ValidationErrors { + public: + // Pushes a field name onto the stack at construction and pops it off + // of the stack at destruction. + class ScopedField { + public: + ScopedField(ValidationErrors* errors, absl::string_view field_name) + : errors_(errors) { + errors_->PushField(field_name); + } + ~ScopedField() { errors_->PopField(); } + + private: + ValidationErrors* errors_; + }; + + // Records that we've encountered an error associated with the current + // field. + void AddError(absl::string_view error) GPR_ATTRIBUTE_NOINLINE; + + // Returns true if the current field has errors. + bool FieldHasErrors() const GPR_ATTRIBUTE_NOINLINE; + + // Returns the resulting status of parsing. + absl::Status status(absl::string_view prefix) const; + + // Returns true if there are no errors. + bool ok() const { return field_errors_.empty(); } + + size_t size() const { return field_errors_.size(); } + + private: + // Pushes a field name onto the stack. + void PushField(absl::string_view ext) GPR_ATTRIBUTE_NOINLINE; + // Pops a field name off of the stack. + void PopField() GPR_ATTRIBUTE_NOINLINE; + + // Errors that we have encountered so far, keyed by field name. + // TODO(roth): If we don't actually have any fields for which we + // report more than one error, simplify this data structure. + std::map> field_errors_; + // Stack of field names indicating the field that we are currently + // validating. + std::vector fields_; +}; + +} // namespace grpc_core + +#endif // GRPC_CORE_LIB_GPRPP_VALIDATION_ERRORS_H diff --git a/src/core/lib/json/json_object_loader.cc b/src/core/lib/json/json_object_loader.cc index 90f2ac6397c..654ac95a9d1 100644 --- a/src/core/lib/json/json_object_loader.cc +++ b/src/core/lib/json/json_object_loader.cc @@ -16,57 +16,17 @@ #include "src/core/lib/json/json_object_loader.h" -#include #include -#include "absl/status/status.h" #include "absl/strings/ascii.h" #include "absl/strings/str_cat.h" -#include "absl/strings/str_join.h" #include "absl/strings/strip.h" namespace grpc_core { - -void ErrorList::PushField(absl::string_view ext) { - // Skip leading '.' for top-level field names. - if (fields_.empty()) absl::ConsumePrefix(&ext, "."); - fields_.emplace_back(std::string(ext)); -} - -void ErrorList::PopField() { fields_.pop_back(); } - -void ErrorList::AddError(absl::string_view error) { - field_errors_[absl::StrJoin(fields_, "")].emplace_back(error); -} - -bool ErrorList::FieldHasErrors() const { - return field_errors_.find(absl::StrJoin(fields_, "")) != field_errors_.end(); -} - -absl::Status ErrorList::status() const { - return status("errors validating JSON"); -} - -absl::Status ErrorList::status(absl::string_view prefix) const { - if (field_errors_.empty()) return absl::OkStatus(); - std::vector errors; - for (const auto& p : field_errors_) { - if (p.second.size() > 1) { - errors.emplace_back(absl::StrCat("field:", p.first, " errors:[", - absl::StrJoin(p.second, "; "), "]")); - } else { - errors.emplace_back( - absl::StrCat("field:", p.first, " error:", p.second[0])); - } - } - return absl::InvalidArgumentError( - absl::StrCat(prefix, ": [", absl::StrJoin(errors, "; "), "]")); -} - namespace json_detail { void LoadScalar::LoadInto(const Json& json, const JsonArgs& /*args*/, void* dst, - ErrorList* errors) const { + ValidationErrors* errors) const { // We accept either STRING or NUMBER for numeric values, as per // https://developers.google.com/protocol-buffers/docs/proto3#json. if (json.type() != Json::Type::STRING && @@ -81,14 +41,14 @@ void LoadScalar::LoadInto(const Json& json, const JsonArgs& /*args*/, void* dst, bool LoadString::IsNumber() const { return false; } void LoadString::LoadInto(const std::string& value, void* dst, - ErrorList*) const { + ValidationErrors*) const { *static_cast(dst) = value; } bool LoadDuration::IsNumber() const { return false; } void LoadDuration::LoadInto(const std::string& value, void* dst, - ErrorList* errors) const { + ValidationErrors* errors) const { absl::string_view buf(value); if (!absl::ConsumeSuffix(&buf, "s")) { errors->AddError("Not a duration (no s suffix)"); @@ -125,7 +85,7 @@ void LoadDuration::LoadInto(const std::string& value, void* dst, bool LoadNumber::IsNumber() const { return true; } void LoadBool::LoadInto(const Json& json, const JsonArgs&, void* dst, - ErrorList* errors) const { + ValidationErrors* errors) const { if (json.type() == Json::Type::JSON_TRUE) { *static_cast(dst) = true; } else if (json.type() == Json::Type::JSON_FALSE) { @@ -136,7 +96,8 @@ void LoadBool::LoadInto(const Json& json, const JsonArgs&, void* dst, } void LoadUnprocessedJsonObject::LoadInto(const Json& json, const JsonArgs&, - void* dst, ErrorList* errors) const { + void* dst, + ValidationErrors* errors) const { if (json.type() != Json::Type::OBJECT) { errors->AddError("is not an object"); return; @@ -145,7 +106,7 @@ void LoadUnprocessedJsonObject::LoadInto(const Json& json, const JsonArgs&, } void LoadVector::LoadInto(const Json& json, const JsonArgs& args, void* dst, - ErrorList* errors) const { + ValidationErrors* errors) const { if (json.type() != Json::Type::ARRAY) { errors->AddError("is not an array"); return; @@ -153,7 +114,7 @@ void LoadVector::LoadInto(const Json& json, const JsonArgs& args, void* dst, const auto& array = json.array_value(); const LoaderInterface* element_loader = ElementLoader(); for (size_t i = 0; i < array.size(); ++i) { - ScopedField field(errors, absl::StrCat("[", i, "]")); + ValidationErrors::ScopedField field(errors, absl::StrCat("[", i, "]")); void* element = EmplaceBack(dst); element_loader->LoadInto(array[i], args, element, errors); } @@ -161,7 +122,7 @@ void LoadVector::LoadInto(const Json& json, const JsonArgs& args, void* dst, void AutoLoader>::LoadInto(const Json& json, const JsonArgs& args, void* dst, - ErrorList* errors) const { + ValidationErrors* errors) const { if (json.type() != Json::Type::ARRAY) { errors->AddError("is not an array"); return; @@ -170,7 +131,7 @@ void AutoLoader>::LoadInto(const Json& json, const LoaderInterface* element_loader = LoaderForType(); std::vector* vec = static_cast*>(dst); for (size_t i = 0; i < array.size(); ++i) { - ScopedField field(errors, absl::StrCat("[", i, "]")); + ValidationErrors::ScopedField field(errors, absl::StrCat("[", i, "]")); bool elem = false; element_loader->LoadInto(array[i], args, &elem, errors); vec->push_back(elem); @@ -178,21 +139,22 @@ void AutoLoader>::LoadInto(const Json& json, } void LoadMap::LoadInto(const Json& json, const JsonArgs& args, void* dst, - ErrorList* errors) const { + ValidationErrors* errors) const { if (json.type() != Json::Type::OBJECT) { errors->AddError("is not an object"); return; } const LoaderInterface* element_loader = ElementLoader(); for (const auto& pair : json.object_value()) { - ScopedField field(errors, absl::StrCat("[\"", pair.first, "\"]")); + ValidationErrors::ScopedField field(errors, + absl::StrCat("[\"", pair.first, "\"]")); void* element = Insert(pair.first, dst); element_loader->LoadInto(pair.second, args, element, errors); } } void LoadOptional::LoadInto(const Json& json, const JsonArgs& args, void* dst, - ErrorList* errors) const { + ValidationErrors* errors) const { if (json.type() == Json::Type::JSON_NULL) return; void* element = Emplace(dst); size_t starting_error_size = errors->size(); @@ -201,7 +163,7 @@ void LoadOptional::LoadInto(const Json& json, const JsonArgs& args, void* dst, } bool LoadObject(const Json& json, const JsonArgs& args, const Element* elements, - size_t num_elements, void* dst, ErrorList* errors) { + size_t num_elements, void* dst, ValidationErrors* errors) { if (json.type() != Json::Type::OBJECT) { errors->AddError("is not an object"); return false; @@ -211,7 +173,8 @@ bool LoadObject(const Json& json, const JsonArgs& args, const Element* elements, if (element.enable_key != nullptr && !args.IsEnabled(element.enable_key)) { continue; } - ScopedField field(errors, absl::StrCat(".", element.name)); + ValidationErrors::ScopedField field(errors, + absl::StrCat(".", element.name)); const auto& it = json.object_value().find(element.name); if (it == json.object_value().end()) { if (element.optional) continue; @@ -225,8 +188,8 @@ bool LoadObject(const Json& json, const JsonArgs& args, const Element* elements, } const Json* GetJsonObjectField(const Json::Object& json, - absl::string_view field, ErrorList* errors, - bool required) { + absl::string_view field, + ValidationErrors* errors, bool required) { auto it = json.find(std::string(field)); if (it == json.end()) { if (required) errors->AddError("field not present"); diff --git a/src/core/lib/json/json_object_loader.h b/src/core/lib/json/json_object_loader.h index 67ea4ac72ab..8825040cd6c 100644 --- a/src/core/lib/json/json_object_loader.h +++ b/src/core/lib/json/json_object_loader.h @@ -24,7 +24,6 @@ #include #include "absl/meta/type_traits.h" -#include "absl/status/status.h" #include "absl/status/statusor.h" #include "absl/strings/numbers.h" #include "absl/strings/str_cat.h" @@ -34,6 +33,7 @@ #include "src/core/lib/gprpp/no_destruct.h" #include "src/core/lib/gprpp/ref_counted_ptr.h" #include "src/core/lib/gprpp/time.h" +#include "src/core/lib/gprpp/validation_errors.h" #include "src/core/lib/json/json.h" #include "src/core/lib/json/json_args.h" @@ -62,7 +62,7 @@ // } // // Optional; omit if no post-processing needed. // void JsonPostLoad(const Json& source, const JsonArgs& args, -// ErrorList* errors) { +// ValidationErrors* errors) { // ++a; // } // }; @@ -70,58 +70,15 @@ // absl::StatusOr foo = LoadFromJson(json); namespace grpc_core { -// A list of errors that occurred during JSON parsing. -// If a non-empty list occurs during parsing, the parsing failed. -class ErrorList { - public: - // Record that we're reading some field. - void PushField(absl::string_view ext) GPR_ATTRIBUTE_NOINLINE; - // Record that we've finished reading that field. - void PopField() GPR_ATTRIBUTE_NOINLINE; - - // Record that we've encountered an error. - void AddError(absl::string_view error) GPR_ATTRIBUTE_NOINLINE; - // Returns true if the current field has errors. - bool FieldHasErrors() const GPR_ATTRIBUTE_NOINLINE; - - // Returns the resulting status of parsing. - absl::Status status() const; - absl::Status status(absl::string_view prefix) const; - - // Return true if there are no errors. - bool ok() const { return field_errors_.empty(); } - - size_t size() const { return field_errors_.size(); } - - private: - // TODO(roth): If we don't actually have any fields for which we - // report more than one error, simplify this data structure. - std::map> field_errors_; - std::vector fields_; -}; - -// Note that we're reading a field, and remove it at the end of the scope. -class ScopedField { - public: - ScopedField(ErrorList* error_list, absl::string_view field_name) - : error_list_(error_list) { - error_list_->PushField(field_name); - } - ~ScopedField() { error_list_->PopField(); } - - private: - ErrorList* error_list_; -}; - namespace json_detail { // An un-typed JSON loader. class LoaderInterface { public: // Convert json value to whatever type we're loading at dst. - // If errors occur, add them to error_list. + // If errors occur, add them to errors. virtual void LoadInto(const Json& json, const JsonArgs& args, void* dst, - ErrorList* errors) const = 0; + ValidationErrors* errors) const = 0; protected: ~LoaderInterface() = default; @@ -131,7 +88,7 @@ class LoaderInterface { class LoadScalar : public LoaderInterface { public: void LoadInto(const Json& json, const JsonArgs& args, void* dst, - ErrorList* errors) const override; + ValidationErrors* errors) const override; protected: ~LoadScalar() = default; @@ -143,7 +100,7 @@ class LoadScalar : public LoaderInterface { virtual bool IsNumber() const = 0; virtual void LoadInto(const std::string& json, void* dst, - ErrorList* errors) const = 0; + ValidationErrors* errors) const = 0; }; // Load a string. @@ -154,7 +111,7 @@ class LoadString : public LoadScalar { private: bool IsNumber() const override; void LoadInto(const std::string& value, void* dst, - ErrorList* errors) const override; + ValidationErrors* errors) const override; }; // Load a Duration. @@ -165,7 +122,7 @@ class LoadDuration : public LoadScalar { private: bool IsNumber() const override; void LoadInto(const std::string& value, void* dst, - ErrorList* errors) const override; + ValidationErrors* errors) const override; }; // Load a number. @@ -185,7 +142,7 @@ class TypedLoadSignedNumber : public LoadNumber { private: void LoadInto(const std::string& value, void* dst, - ErrorList* errors) const override { + ValidationErrors* errors) const override { if (!absl::SimpleAtoi(value, static_cast(dst))) { errors->AddError("failed to parse number"); } @@ -200,7 +157,7 @@ class TypedLoadUnsignedNumber : public LoadNumber { private: void LoadInto(const std::string& value, void* dst, - ErrorList* errors) const override { + ValidationErrors* errors) const override { if (!absl::SimpleAtoi(value, static_cast(dst))) { errors->AddError("failed to parse non-negative number"); } @@ -214,7 +171,7 @@ class LoadFloat : public LoadNumber { private: void LoadInto(const std::string& value, void* dst, - ErrorList* errors) const override { + ValidationErrors* errors) const override { if (!absl::SimpleAtof(value, static_cast(dst))) { errors->AddError("failed to parse floating-point number"); } @@ -228,7 +185,7 @@ class LoadDouble : public LoadNumber { private: void LoadInto(const std::string& value, void* dst, - ErrorList* errors) const override { + ValidationErrors* errors) const override { if (!absl::SimpleAtod(value, static_cast(dst))) { errors->AddError("failed to parse floating-point number"); } @@ -239,7 +196,7 @@ class LoadDouble : public LoadNumber { class LoadBool : public LoaderInterface { public: void LoadInto(const Json& json, const JsonArgs& /*args*/, void* dst, - ErrorList* errors) const override; + ValidationErrors* errors) const override; protected: ~LoadBool() = default; @@ -249,7 +206,7 @@ class LoadBool : public LoaderInterface { class LoadUnprocessedJsonObject : public LoaderInterface { public: void LoadInto(const Json& json, const JsonArgs& /*args*/, void* dst, - ErrorList* errors) const override; + ValidationErrors* errors) const override; protected: ~LoadUnprocessedJsonObject() = default; @@ -259,7 +216,7 @@ class LoadUnprocessedJsonObject : public LoaderInterface { class LoadVector : public LoaderInterface { public: void LoadInto(const Json& json, const JsonArgs& args, void* dst, - ErrorList* errors) const override; + ValidationErrors* errors) const override; protected: ~LoadVector() = default; @@ -273,7 +230,7 @@ class LoadVector : public LoaderInterface { class LoadMap : public LoaderInterface { public: void LoadInto(const Json& json, const JsonArgs& args, void* dst, - ErrorList* errors) const override; + ValidationErrors* errors) const override; protected: ~LoadMap() = default; @@ -287,7 +244,7 @@ class LoadMap : public LoaderInterface { class LoadOptional : public LoaderInterface { public: void LoadInto(const Json& json, const JsonArgs& args, void* dst, - ErrorList* errors) const override; + ValidationErrors* errors) const override; protected: ~LoadOptional() = default; @@ -310,7 +267,7 @@ template class AutoLoader final : public LoaderInterface { public: void LoadInto(const Json& json, const JsonArgs& args, void* dst, - ErrorList* errors) const override { + ValidationErrors* errors) const override { T::JsonLoader(args)->LoadInto(json, args, dst, errors); } @@ -393,7 +350,7 @@ template <> class AutoLoader> final : public LoaderInterface { public: void LoadInto(const Json& json, const JsonArgs& args, void* dst, - ErrorList* errors) const override; + ValidationErrors* errors) const override; private: ~AutoLoader() = default; @@ -494,7 +451,7 @@ class Vec { // the object from some parsed JSON. // Returns false if the JSON object was not of type Json::Type::OBJECT. bool LoadObject(const Json& json, const JsonArgs& args, const Element* elements, - size_t num_elements, void* dst, ErrorList* errors); + size_t num_elements, void* dst, ValidationErrors* errors); // Adaptor type - takes a compile time computed list of elements and // implements LoaderInterface by calling LoadObject. @@ -505,7 +462,7 @@ class FinishedJsonObjectLoader final : public LoaderInterface { : elements_(elements) {} void LoadInto(const Json& json, const JsonArgs& args, void* dst, - ErrorList* errors) const override { + ValidationErrors* errors) const override { LoadObject(json, args, elements_.data(), elements_.size(), dst, errors); } @@ -523,7 +480,7 @@ class FinishedJsonObjectLoader absl::StatusOr LoadFromJson( const Json& json, const JsonArgs& args = JsonArgs(), absl::string_view error_prefix = "errors validating JSON") { - ErrorList error_list; + ValidationErrors errors; T result{}; - json_detail::LoaderForType()->LoadInto(json, args, &result, &error_list); - if (!error_list.ok()) return error_list.status(error_prefix); + json_detail::LoaderForType()->LoadInto(json, args, &result, &errors); + if (!errors.ok()) return errors.status(error_prefix); return std::move(result); } @@ -604,18 +561,18 @@ template absl::StatusOr> LoadRefCountedFromJson( const Json& json, const JsonArgs& args = JsonArgs(), absl::string_view error_prefix = "errors validating JSON") { - ErrorList error_list; + ValidationErrors errors; auto result = MakeRefCounted(); - json_detail::LoaderForType()->LoadInto(json, args, result.get(), - &error_list); - if (!error_list.ok()) return error_list.status(error_prefix); + json_detail::LoaderForType()->LoadInto(json, args, result.get(), &errors); + if (!errors.ok()) return errors.status(error_prefix); return std::move(result); } template -T LoadFromJson(const Json& json, const JsonArgs& args, ErrorList* error_list) { +T LoadFromJson(const Json& json, const JsonArgs& args, + ValidationErrors* errors) { T result{}; - json_detail::LoaderForType()->LoadInto(json, args, &result, error_list); + json_detail::LoaderForType()->LoadInto(json, args, &result, errors); return result; } @@ -623,8 +580,9 @@ template absl::optional LoadJsonObjectField(const Json::Object& json, const JsonArgs& args, absl::string_view field, - ErrorList* errors, bool required = true) { - ScopedField error_field(errors, absl::StrCat(".", field)); + ValidationErrors* errors, + bool required = true) { + ValidationErrors::ScopedField error_field(errors, absl::StrCat(".", field)); const Json* field_json = json_detail::GetJsonObjectField(json, field, errors, required); if (field_json == nullptr) return absl::nullopt; diff --git a/src/core/lib/json/json_util.cc b/src/core/lib/json/json_util.cc index e6434149fa5..dd8d8e8d052 100644 --- a/src/core/lib/json/json_util.cc +++ b/src/core/lib/json/json_util.cc @@ -21,13 +21,14 @@ #include "src/core/lib/json/json_util.h" #include "src/core/lib/gprpp/no_destruct.h" +#include "src/core/lib/gprpp/validation_errors.h" #include "src/core/lib/json/json_args.h" #include "src/core/lib/json/json_object_loader.h" namespace grpc_core { bool ParseDurationFromJson(const Json& field, Duration* duration) { - ErrorList errors; + ValidationErrors errors; static_cast( NoDestructSingleton>::Get()) ->LoadInto(field, JsonArgs(), duration, &errors); diff --git a/src/python/grpcio/grpc_core_dependencies.py b/src/python/grpcio/grpc_core_dependencies.py index 82c8dc85726..3c250c33b10 100644 --- a/src/python/grpcio/grpc_core_dependencies.py +++ b/src/python/grpcio/grpc_core_dependencies.py @@ -510,6 +510,7 @@ CORE_SOURCE_FILES = [ 'src/core/lib/gprpp/time.cc', 'src/core/lib/gprpp/time_averaged_stats.cc', 'src/core/lib/gprpp/time_util.cc', + 'src/core/lib/gprpp/validation_errors.cc', 'src/core/lib/gprpp/work_serializer.cc', 'src/core/lib/handshaker/proxy_mapper_registry.cc', 'src/core/lib/http/format_request.cc', diff --git a/test/core/gprpp/BUILD b/test/core/gprpp/BUILD index 359ba9c2be5..dc51bb262b6 100644 --- a/test/core/gprpp/BUILD +++ b/test/core/gprpp/BUILD @@ -405,6 +405,19 @@ grpc_cc_test( ], ) +grpc_cc_test( + name = "validation_errors_test", + srcs = ["validation_errors_test.cc"], + external_deps = [ + "gtest", + ], + language = "C++", + deps = [ + "//:validation_errors", + "//test/core/util:grpc_test_util", + ], +) + grpc_cc_test( name = "notification_test", srcs = ["notification_test.cc"], diff --git a/test/core/gprpp/validation_errors_test.cc b/test/core/gprpp/validation_errors_test.cc new file mode 100644 index 00000000000..993838dec37 --- /dev/null +++ b/test/core/gprpp/validation_errors_test.cc @@ -0,0 +1,111 @@ +// +// Copyright 2022 gRPC authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +#include "src/core/lib/gprpp/validation_errors.h" + +#include +#include + +#include + +#include "test/core/util/test_config.h" + +namespace grpc_core { +namespace testing { +namespace { + +TEST(ValidationErrors, NoErrors) { + ValidationErrors errors; + EXPECT_TRUE(errors.ok()); + EXPECT_EQ(errors.size(), 0); + { + ValidationErrors::ScopedField field(&errors, "foo"); + { ValidationErrors::ScopedField field(&errors, ".bar"); } + } + EXPECT_TRUE(errors.ok()); + EXPECT_EQ(errors.size(), 0); + absl::Status status = errors.status("errors validating config"); + EXPECT_TRUE(status.ok()) << status; +} + +TEST(ValidationErrors, OneError) { + ValidationErrors errors; + { + ValidationErrors::ScopedField field(&errors, "foo"); + { + ValidationErrors::ScopedField field(&errors, ".bar"); + errors.AddError("value smells funny"); + } + } + EXPECT_FALSE(errors.ok()); + EXPECT_EQ(errors.size(), 1); + absl::Status status = errors.status("errors validating config"); + EXPECT_EQ(status.code(), absl::StatusCode::kInvalidArgument); + EXPECT_EQ( + status.message(), + "errors validating config: [field:foo.bar error:value smells funny]") + << status; +} + +TEST(ValidationErrors, MultipleErrorsForSameField) { + ValidationErrors errors; + { + ValidationErrors::ScopedField field(&errors, "foo"); + { + ValidationErrors::ScopedField field(&errors, ".bar"); + errors.AddError("value smells funny"); + errors.AddError("value is ugly"); + } + } + EXPECT_FALSE(errors.ok()); + EXPECT_EQ(errors.size(), 1); + absl::Status status = errors.status("errors validating config"); + EXPECT_EQ(status.code(), absl::StatusCode::kInvalidArgument); + EXPECT_EQ(status.message(), + "errors validating config: [field:foo.bar errors:[" + "value smells funny; value is ugly]]") + << status; +} + +TEST(ValidationErrors, ErrorsForMultipleFields) { + ValidationErrors errors; + { + ValidationErrors::ScopedField field(&errors, "foo"); + { + ValidationErrors::ScopedField field(&errors, ".bar"); + errors.AddError("value smells funny"); + } + errors.AddError("too hot"); + } + EXPECT_FALSE(errors.ok()); + EXPECT_EQ(errors.size(), 2); + absl::Status status = errors.status("errors validating config"); + EXPECT_EQ(status.code(), absl::StatusCode::kInvalidArgument); + EXPECT_EQ(status.message(), + "errors validating config: [" + "field:foo error:too hot; field:foo.bar error:value smells funny]") + << status; +} + +} // namespace +} // namespace testing +} // namespace grpc_core + +int main(int argc, char** argv) { + grpc::testing::TestEnvironment env(&argc, argv); + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} diff --git a/test/core/json/json_object_loader_test.cc b/test/core/json/json_object_loader_test.cc index 799e3ee1cde..7092e451aa4 100644 --- a/test/core/json/json_object_loader_test.cc +++ b/test/core/json/json_object_loader_test.cc @@ -844,7 +844,7 @@ TEST(JsonObjectLoader, PostLoadHook) { } void JsonPostLoad(const Json& /*source*/, const JsonArgs& /*args*/, - ErrorList* /*errors*/) { + ValidationErrors* /*errors*/) { ++a; } }; @@ -867,8 +867,8 @@ TEST(JsonObjectLoader, CustomValidationInPostLoadHook) { } void JsonPostLoad(const Json& /*source*/, const JsonArgs& /*args*/, - ErrorList* errors) { - ScopedField field(errors, ".a"); + ValidationErrors* errors) { + ValidationErrors::ScopedField field(errors, ".a"); if (!errors->FieldHasErrors() && a <= 0) { errors->AddError("must be greater than 0"); } @@ -927,7 +927,7 @@ TEST(JsonObjectLoader, LoadRefCountedFromJson) { } } -TEST(JsonObjectLoader, LoadFromJsonWithErrorList) { +TEST(JsonObjectLoader, LoadFromJsonWithValidationErrors) { struct TestStruct { int32_t a = 0; @@ -942,10 +942,10 @@ TEST(JsonObjectLoader, LoadFromJsonWithErrorList) { absl::string_view json_str = "{\"a\":1}"; auto json = Json::Parse(json_str); ASSERT_TRUE(json.ok()) << json.status(); - ErrorList errors; + ValidationErrors errors; TestStruct test_struct = LoadFromJson(*json, JsonArgs(), &errors); - ASSERT_TRUE(errors.ok()) << errors.status(); + ASSERT_TRUE(errors.ok()) << errors.status("unexpected errors"); EXPECT_EQ(test_struct.a, 1); } // Invalid. @@ -953,9 +953,9 @@ TEST(JsonObjectLoader, LoadFromJsonWithErrorList) { absl::string_view json_str = "{\"a\":\"foo\"}"; auto json = Json::Parse(json_str); ASSERT_TRUE(json.ok()) << json.status(); - ErrorList errors; + ValidationErrors errors; LoadFromJson(*json, JsonArgs(), &errors); - absl::Status status = errors.status(); + absl::Status status = errors.status("errors validating JSON"); EXPECT_EQ(status.code(), absl::StatusCode::kInvalidArgument); EXPECT_EQ(status.message(), "errors validating JSON: [field:a error:failed to parse number]") @@ -969,16 +969,16 @@ TEST(JsonObjectLoader, LoadJsonObjectField) { ASSERT_TRUE(json.ok()) << json.status(); // Load a valid field. { - ErrorList errors; + ValidationErrors errors; auto value = LoadJsonObjectField(json->object_value(), JsonArgs(), "int", &errors); - ASSERT_TRUE(value.has_value()) << errors.status(); + ASSERT_TRUE(value.has_value()) << errors.status("unexpected errors"); EXPECT_EQ(*value, 1); EXPECT_TRUE(errors.ok()); } // An optional field that is not present. { - ErrorList errors; + ValidationErrors errors; auto value = LoadJsonObjectField(json->object_value(), JsonArgs(), "not_present", &errors, /*required=*/false); @@ -987,11 +987,11 @@ TEST(JsonObjectLoader, LoadJsonObjectField) { } // A required field that is not present. { - ErrorList errors; + ValidationErrors errors; auto value = LoadJsonObjectField(json->object_value(), JsonArgs(), "not_present", &errors); EXPECT_FALSE(value.has_value()); - auto status = errors.status(); + auto status = errors.status("errors validating JSON"); EXPECT_THAT(status.code(), absl::StatusCode::kInvalidArgument); EXPECT_EQ(status.message(), "errors validating JSON: [" @@ -1000,11 +1000,11 @@ TEST(JsonObjectLoader, LoadJsonObjectField) { } // Value has the wrong type. { - ErrorList errors; + ValidationErrors errors; auto value = LoadJsonObjectField(json->object_value(), JsonArgs(), "int", &errors); EXPECT_FALSE(value.has_value()); - auto status = errors.status(); + auto status = errors.status("errors validating JSON"); EXPECT_THAT(status.code(), absl::StatusCode::kInvalidArgument); EXPECT_EQ(status.message(), "errors validating JSON: [field:int error:is not a string]") diff --git a/test/cpp/ext/gcp/observability_config_test.cc b/test/cpp/ext/gcp/observability_config_test.cc index 96144dc787a..bdfbd80638a 100644 --- a/test/cpp/ext/gcp/observability_config_test.cc +++ b/test/cpp/ext/gcp/observability_config_test.cc @@ -41,10 +41,10 @@ TEST(GcpObservabilityConfigJsonParsingTest, Basic) { })json"; auto json = grpc_core::Json::Parse(json_str); ASSERT_TRUE(json.ok()) << json.status(); - grpc_core::ErrorList errors; + grpc_core::ValidationErrors errors; auto config = grpc_core::LoadFromJson( *json, grpc_core::JsonArgs(), &errors); - ASSERT_TRUE(errors.ok()) << errors.status(); + ASSERT_TRUE(errors.ok()) << errors.status("unexpected errors"); EXPECT_TRUE(config.cloud_logging.has_value()); EXPECT_TRUE(config.cloud_monitoring.has_value()); EXPECT_TRUE(config.cloud_trace.has_value()); @@ -57,10 +57,10 @@ TEST(GcpObservabilityConfigJsonParsingTest, Defaults) { })json"; auto json = grpc_core::Json::Parse(json_str); ASSERT_TRUE(json.ok()) << json.status(); - grpc_core::ErrorList errors; + grpc_core::ValidationErrors errors; auto config = grpc_core::LoadFromJson( *json, grpc_core::JsonArgs(), &errors); - ASSERT_TRUE(errors.ok()) << errors.status(); + ASSERT_TRUE(errors.ok()) << errors.status("unexpected errors"); EXPECT_FALSE(config.cloud_logging.has_value()); EXPECT_FALSE(config.cloud_monitoring.has_value()); EXPECT_FALSE(config.cloud_trace.has_value()); @@ -75,10 +75,10 @@ TEST(GcpObservabilityConfigJsonParsingTest, SamplingRateDefaults) { })json"; auto json = grpc_core::Json::Parse(json_str); ASSERT_TRUE(json.ok()) << json.status(); - grpc_core::ErrorList errors; + grpc_core::ValidationErrors errors; auto config = grpc_core::LoadFromJson( *json, grpc_core::JsonArgs(), &errors); - ASSERT_TRUE(errors.ok()) << errors.status(); + ASSERT_TRUE(errors.ok()) << errors.status("unexpected errors"); ASSERT_TRUE(config.cloud_trace.has_value()); EXPECT_FLOAT_EQ(config.cloud_trace->sampling_rate, 0.05); } diff --git a/tools/doxygen/Doxyfile.c++.internal b/tools/doxygen/Doxyfile.c++.internal index 3643a512611..bfd99647c5a 100644 --- a/tools/doxygen/Doxyfile.c++.internal +++ b/tools/doxygen/Doxyfile.c++.internal @@ -2077,6 +2077,8 @@ src/core/lib/gprpp/time_averaged_stats.h \ src/core/lib/gprpp/time_util.cc \ src/core/lib/gprpp/time_util.h \ src/core/lib/gprpp/unique_type_name.h \ +src/core/lib/gprpp/validation_errors.cc \ +src/core/lib/gprpp/validation_errors.h \ src/core/lib/gprpp/work_serializer.cc \ src/core/lib/gprpp/work_serializer.h \ src/core/lib/handshaker/proxy_mapper.h \ diff --git a/tools/doxygen/Doxyfile.core.internal b/tools/doxygen/Doxyfile.core.internal index 66da0b3ddfa..4d55ee883b6 100644 --- a/tools/doxygen/Doxyfile.core.internal +++ b/tools/doxygen/Doxyfile.core.internal @@ -1867,6 +1867,8 @@ src/core/lib/gprpp/time_averaged_stats.h \ src/core/lib/gprpp/time_util.cc \ src/core/lib/gprpp/time_util.h \ src/core/lib/gprpp/unique_type_name.h \ +src/core/lib/gprpp/validation_errors.cc \ +src/core/lib/gprpp/validation_errors.h \ src/core/lib/gprpp/work_serializer.cc \ src/core/lib/gprpp/work_serializer.h \ src/core/lib/handshaker/proxy_mapper.h \ diff --git a/tools/run_tests/generated/tests.json b/tools/run_tests/generated/tests.json index 83fc8499c50..9562f7f6b08 100644 --- a/tools/run_tests/generated/tests.json +++ b/tools/run_tests/generated/tests.json @@ -7753,6 +7753,30 @@ ], "uses_polling": false }, + { + "args": [], + "benchmark": false, + "ci_platforms": [ + "linux", + "mac", + "posix", + "windows" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "gtest": true, + "language": "c++", + "name": "validation_errors_test", + "platforms": [ + "linux", + "mac", + "posix", + "windows" + ], + "uses_polling": true + }, { "args": [], "benchmark": false,