From c2f2800647217feb5e4f2b8bf9c02269b0ab9b01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Hibner?= Date: Mon, 28 Sep 2026 08:57:41 +0000 Subject: [PATCH] Fix defaults for omitted nullable input fields --- include/graphqlservice/GraphQLService.h | 24 +-- samples/validation/schema/QueryObject.cpp | 18 +- samples/validation/schema/QueryObject.h | 30 +++ .../validation/schema/ValidationSchema.cpp | 13 ++ .../schema/ValidationSharedTypes.cpp | 187 ++++++++++++++++++ .../validation/schema/ValidationSharedTypes.h | 38 ++++ .../schema/ValidationSharedTypes.ixx | 2 + .../schema/schema.validation.graphql | 16 ++ test/ValidationTests.cpp | 150 ++++++++++++++ 9 files changed, 463 insertions(+), 15 deletions(-) diff --git a/include/graphqlservice/GraphQLService.h b/include/graphqlservice/GraphQLService.h index 5c5f11f6..11563f31 100644 --- a/include/graphqlservice/GraphQLService.h +++ b/include/graphqlservice/GraphQLService.h @@ -662,18 +662,16 @@ struct ModifiedArgument } } - // Wrap require in a try/catch block. + // Return false only for an absent argument; supplied values must pass conversion. [[nodiscard("unnecessary call")]] static std::pair find( - const std::string& name, const response::Value& arguments) noexcept + const std::string& name, const response::Value& arguments) { - try - { - return { require(name, arguments), true }; - } - catch (const std::exception&) + if (arguments.find(name) == arguments.get().cend()) { return { Type {}, false }; } + + return { require(name, arguments), true }; } // Peel off the none modifier. If it's included, it should always be last in the list. @@ -738,20 +736,18 @@ struct ModifiedArgument return result; } - // Wrap require with modifiers in a try/catch block. + // Preserve absence separately from nullable values and conversion errors. template [[nodiscard("unnecessary call")]] static std::pair< typename ArgumentTraits::type, bool> - find(std::string_view name, const response::Value& arguments) noexcept + find(std::string_view name, const response::Value& arguments) { - try - { - return { require(name, arguments), true }; - } - catch (const std::exception&) + if (arguments.find(name) == arguments.get().cend()) { return { typename ArgumentTraits::type {}, false }; } + + return { require(name, arguments), true }; } // Peel off the none modifier. If it's included, it should always be last in the list. diff --git a/samples/validation/schema/QueryObject.cpp b/samples/validation/schema/QueryObject.cpp index c15a5f0b..fc8dd387 100644 --- a/samples/validation/schema/QueryObject.cpp +++ b/samples/validation/schema/QueryObject.cpp @@ -49,7 +49,8 @@ service::ResolverMap Query::getResolvers() const noexcept { R"gql(resource)gql"sv, [this](service::ResolverParams&& params) { return resolveResource(std::move(params)); } }, { R"gql(arguments)gql"sv, [this](service::ResolverParams&& params) { return resolveArguments(std::move(params)); } }, { R"gql(__typename)gql"sv, [this](service::ResolverParams&& params) { return resolve_typename(std::move(params)); } }, - { R"gql(booleanList)gql"sv, [this](service::ResolverParams&& params) { return resolveBooleanList(std::move(params)); } } + { R"gql(booleanList)gql"sv, [this](service::ResolverParams&& params) { return resolveBooleanList(std::move(params)); } }, + { R"gql(inputDefaults)gql"sv, [this](service::ResolverParams&& params) { return resolveInputDefaults(std::move(params)); } } }; } @@ -153,6 +154,18 @@ service::AwaitableResolver Query::resolveBooleanList(service::ResolverParams&& p return service::ModifiedResult::convert(std::move(result), std::move(params)); } +service::AwaitableResolver Query::resolveInputDefaults(service::ResolverParams&& params) const +{ + auto argInput = service::ModifiedArgument::require("input", params.arguments); + std::unique_lock resolverLock(_resolverMutex); + service::SelectionSetParams selectionSetParams { static_cast(params) }; + auto directives = std::move(params.fieldDirectives); + auto result = _pimpl->getInputDefaults(service::FieldParams { std::move(selectionSetParams), std::move(directives) }, std::move(argInput)); + resolverLock.unlock(); + + return service::ModifiedResult::convert(std::move(result), std::move(params)); +} + service::AwaitableResolver Query::resolve_typename(service::ResolverParams&& params) const { return service::Result::convert(std::string{ R"gql(Query)gql" }, std::move(params)); @@ -174,6 +187,9 @@ void AddQueryDetails(const std::shared_ptr& typeQuery, const }), schema::Field::Make(R"gql(booleanList)gql"sv, R"md()md"sv, std::nullopt, schema->LookupType(R"gql(Boolean)gql"sv), { schema::InputValue::Make(R"gql(booleanListArg)gql"sv, R"md()md"sv, schema->WrapType(introspection::TypeKind::LIST, schema->WrapType(introspection::TypeKind::NON_NULL, schema->LookupType(R"gql(Boolean)gql"sv))), R"gql()gql"sv) + }), + schema::Field::Make(R"gql(inputDefaults)gql"sv, R"md()md"sv, std::nullopt, schema->WrapType(introspection::TypeKind::NON_NULL, schema->LookupType(R"gql(String)gql"sv)), { + schema::InputValue::Make(R"gql(input)gql"sv, R"md()md"sv, schema->WrapType(introspection::TypeKind::NON_NULL, schema->LookupType(R"gql(DefaultInput)gql"sv)), R"gql()gql"sv) }) }); } diff --git a/samples/validation/schema/QueryObject.h b/samples/validation/schema/QueryObject.h index 50b145f4..c7bcfd68 100644 --- a/samples/validation/schema/QueryObject.h +++ b/samples/validation/schema/QueryObject.h @@ -109,6 +109,18 @@ concept getBooleanList = requires (TImpl impl, std::optional> { service::AwaitableScalar> { impl.getBooleanList(std::move(booleanListArgArg)) } }; }; +template +concept getInputDefaultsWithParams = requires (TImpl impl, service::FieldParams params, DefaultInput inputArg) +{ + { service::AwaitableScalar { impl.getInputDefaults(std::move(params), std::move(inputArg)) } }; +}; + +template +concept getInputDefaults = requires (TImpl impl, DefaultInput inputArg) +{ + { service::AwaitableScalar { impl.getInputDefaults(std::move(inputArg)) } }; +}; + template concept beginSelectionSet = requires (TImpl impl, const service::SelectionSetParams params) { @@ -135,6 +147,7 @@ class [[nodiscard("unnecessary construction")]] Query final [[nodiscard("unnecessary call")]] service::AwaitableResolver resolveResource(service::ResolverParams&& params) const; [[nodiscard("unnecessary call")]] service::AwaitableResolver resolveFindDog(service::ResolverParams&& params) const; [[nodiscard("unnecessary call")]] service::AwaitableResolver resolveBooleanList(service::ResolverParams&& params) const; + [[nodiscard("unnecessary call")]] service::AwaitableResolver resolveInputDefaults(service::ResolverParams&& params) const; [[nodiscard("unnecessary call")]] service::AwaitableResolver resolve_typename(service::ResolverParams&& params) const; @@ -153,6 +166,7 @@ class [[nodiscard("unnecessary construction")]] Query final [[nodiscard("unnecessary call")]] virtual service::AwaitableObject> getResource(service::FieldParams&& params) const = 0; [[nodiscard("unnecessary call")]] virtual service::AwaitableObject> getFindDog(service::FieldParams&& params, std::unique_ptr&& complexArg) const = 0; [[nodiscard("unnecessary call")]] virtual service::AwaitableScalar> getBooleanList(service::FieldParams&& params, std::optional>&& booleanListArgArg) const = 0; + [[nodiscard("unnecessary call")]] virtual service::AwaitableScalar getInputDefaults(service::FieldParams&& params, DefaultInput&& inputArg) const = 0; }; template @@ -292,6 +306,22 @@ class [[nodiscard("unnecessary construction")]] Query final } } + [[nodiscard("unnecessary call")]] service::AwaitableScalar getInputDefaults(service::FieldParams&& params, DefaultInput&& inputArg) const override + { + if constexpr (methods::QueryHas::getInputDefaultsWithParams) + { + return { _pimpl->getInputDefaults(std::move(params), std::move(inputArg)) }; + } + else if constexpr (methods::QueryHas::getInputDefaults) + { + return { _pimpl->getInputDefaults(std::move(inputArg)) }; + } + else + { + throw service::unimplemented_method(R"ex(Query::getInputDefaults)ex"); + } + } + void beginSelectionSet(const service::SelectionSetParams& params) const override { if constexpr (methods::QueryHas::beginSelectionSet) diff --git a/samples/validation/schema/ValidationSchema.cpp b/samples/validation/schema/ValidationSchema.cpp index 4a99aec5..8eed5675 100644 --- a/samples/validation/schema/ValidationSchema.cpp +++ b/samples/validation/schema/ValidationSchema.cpp @@ -44,6 +44,10 @@ void AddTypesToSchema(const std::shared_ptr& schema) schema->AddType(R"gql(CatCommand)gql"sv, typeCatCommand); auto typeComplexInput = schema::InputObjectType::Make(R"gql(ComplexInput)gql"sv, R"md()md"sv); schema->AddType(R"gql(ComplexInput)gql"sv, typeComplexInput); + auto typeDefaultInput = schema::InputObjectType::Make(R"gql(DefaultInput)gql"sv, R"md()md"sv); + schema->AddType(R"gql(DefaultInput)gql"sv, typeDefaultInput); + auto typeNestedDefaultInput = schema::InputObjectType::Make(R"gql(NestedDefaultInput)gql"sv, R"md()md"sv); + schema->AddType(R"gql(NestedDefaultInput)gql"sv, typeNestedDefaultInput); auto typeSentient = schema::InterfaceType::Make(R"gql(Sentient)gql"sv, R"md()md"sv); schema->AddType(R"gql(Sentient)gql"sv, typeSentient); auto typePet = schema::InterfaceType::Make(R"gql(Pet)gql"sv, R"md()md"sv); @@ -94,6 +98,15 @@ void AddTypesToSchema(const std::shared_ptr& schema) schema::InputValue::Make(R"gql(name)gql"sv, R"md()md"sv, schema->LookupType(R"gql(String)gql"sv), R"gql()gql"sv), schema::InputValue::Make(R"gql(owner)gql"sv, R"md()md"sv, schema->LookupType(R"gql(String)gql"sv), R"gql()gql"sv) }); + typeDefaultInput->AddInputValues({ + schema::InputValue::Make(R"gql(values)gql"sv, R"md()md"sv, schema->WrapType(introspection::TypeKind::LIST, schema->WrapType(introspection::TypeKind::NON_NULL, schema->LookupType(R"gql(Int)gql"sv))), R"gql([])gql"sv), + schema::InputValue::Make(R"gql(count)gql"sv, R"md()md"sv, schema->LookupType(R"gql(Int)gql"sv), R"gql(42)gql"sv), + schema::InputValue::Make(R"gql(child)gql"sv, R"md()md"sv, schema->LookupType(R"gql(NestedDefaultInput)gql"sv), R"gql({})gql"sv), + schema::InputValue::Make(R"gql(required)gql"sv, R"md()md"sv, schema->WrapType(introspection::TypeKind::NON_NULL, schema->LookupType(R"gql(Int)gql"sv)), R"gql(9)gql"sv) + }); + typeNestedDefaultInput->AddInputValues({ + schema::InputValue::Make(R"gql(count)gql"sv, R"md()md"sv, schema->LookupType(R"gql(Int)gql"sv), R"gql(7)gql"sv) + }); AddSentientDetails(typeSentient, schema); AddPetDetails(typePet, schema); diff --git a/samples/validation/schema/ValidationSharedTypes.cpp b/samples/validation/schema/ValidationSharedTypes.cpp index aa67bdbb..39594ca4 100644 --- a/samples/validation/schema/ValidationSharedTypes.cpp +++ b/samples/validation/schema/ValidationSharedTypes.cpp @@ -151,6 +151,87 @@ validation::ComplexInput Argument::convert(const respo }; } +template <> +validation::DefaultInput Argument::convert(const response::Value& value) +{ + const auto defaultValue = []() + { + response::Value values(response::Type::Map); + response::Value entry; + + entry = []() + { + response::Value elements(response::Type::List); + response::Value entry; + + return elements; + }(); + values.emplace_back("values", std::move(entry)); + entry = response::Value(static_cast(42)); + values.emplace_back("count", std::move(entry)); + entry = []() + { + response::Value members(response::Type::Map); + response::Value entry; + + return members; + }(); + values.emplace_back("child", std::move(entry)); + entry = response::Value(static_cast(9)); + values.emplace_back("required", std::move(entry)); + + return values; + }(); + + auto pairValues = service::ModifiedArgument::find("values", value); + auto valueValues = (pairValues.second + ? std::move(pairValues.first) + : service::ModifiedArgument::require("values", defaultValue)); + auto pairCount = service::ModifiedArgument::find("count", value); + auto valueCount = (pairCount.second + ? std::move(pairCount.first) + : service::ModifiedArgument::require("count", defaultValue)); + auto pairChild = service::ModifiedArgument::find("child", value); + auto valueChild = (pairChild.second + ? std::move(pairChild.first) + : service::ModifiedArgument::require("child", defaultValue)); + auto pairRequired = service::ModifiedArgument::find("required", value); + auto valueRequired = (pairRequired.second + ? pairRequired.first + : service::ModifiedArgument::require("required", defaultValue)); + + return validation::DefaultInput { + std::move(valueValues), + std::move(valueCount), + std::move(valueChild), + valueRequired + }; +} + +template <> +validation::NestedDefaultInput Argument::convert(const response::Value& value) +{ + const auto defaultValue = []() + { + response::Value values(response::Type::Map); + response::Value entry; + + entry = response::Value(static_cast(7)); + values.emplace_back("count", std::move(entry)); + + return values; + }(); + + auto pairCount = service::ModifiedArgument::find("count", value); + auto valueCount = (pairCount.second + ? std::move(pairCount.first) + : service::ModifiedArgument::require("count", defaultValue)); + + return validation::NestedDefaultInput { + std::move(valueCount) + }; +} + } // namespace service namespace validation { @@ -204,5 +285,111 @@ ComplexInput& ComplexInput::operator=(ComplexInput&& other) noexcept return *this; } + +DefaultInput::DefaultInput() noexcept + : values {} + , count {} + , child {} + , required {} +{ + // Explicit definition to prevent ODR violations when LTO is enabled. +} + +DefaultInput::DefaultInput( + std::optional> valuesArg, + std::optional countArg, + std::unique_ptr childArg, + int requiredArg) noexcept + : values { std::move(valuesArg) } + , count { std::move(countArg) } + , child { std::move(childArg) } + , required { std::move(requiredArg) } +{ +} + +DefaultInput::DefaultInput(const DefaultInput& other) + : values { service::ModifiedArgument::duplicate(other.values) } + , count { service::ModifiedArgument::duplicate(other.count) } + , child { service::ModifiedArgument::duplicate(other.child) } + , required { service::ModifiedArgument::duplicate(other.required) } +{ +} + +DefaultInput::DefaultInput(DefaultInput&& other) noexcept + : values { std::move(other.values) } + , count { std::move(other.count) } + , child { std::move(other.child) } + , required { std::move(other.required) } +{ +} + +DefaultInput::~DefaultInput() +{ + // Explicit definition to prevent ODR violations when LTO is enabled. +} + +DefaultInput& DefaultInput::operator=(const DefaultInput& other) +{ + DefaultInput value { other }; + + std::swap(*this, value); + + return *this; +} + +DefaultInput& DefaultInput::operator=(DefaultInput&& other) noexcept +{ + values = std::move(other.values); + count = std::move(other.count); + child = std::move(other.child); + required = std::move(other.required); + + return *this; +} + + +NestedDefaultInput::NestedDefaultInput() noexcept + : count {} +{ + // Explicit definition to prevent ODR violations when LTO is enabled. +} + +NestedDefaultInput::NestedDefaultInput( + std::optional countArg) noexcept + : count { std::move(countArg) } +{ +} + +NestedDefaultInput::NestedDefaultInput(const NestedDefaultInput& other) + : count { service::ModifiedArgument::duplicate(other.count) } +{ +} + +NestedDefaultInput::NestedDefaultInput(NestedDefaultInput&& other) noexcept + : count { std::move(other.count) } +{ +} + +NestedDefaultInput::~NestedDefaultInput() +{ + // Explicit definition to prevent ODR violations when LTO is enabled. +} + +NestedDefaultInput& NestedDefaultInput::operator=(const NestedDefaultInput& other) +{ + NestedDefaultInput value { other }; + + std::swap(*this, value); + + return *this; +} + +NestedDefaultInput& NestedDefaultInput::operator=(NestedDefaultInput&& other) noexcept +{ + count = std::move(other.count); + + return *this; +} + } // namespace validation } // namespace graphql diff --git a/samples/validation/schema/ValidationSharedTypes.h b/samples/validation/schema/ValidationSharedTypes.h index 7b963d0a..e0b4b9c6 100644 --- a/samples/validation/schema/ValidationSharedTypes.h +++ b/samples/validation/schema/ValidationSharedTypes.h @@ -95,6 +95,44 @@ struct [[nodiscard("unnecessary construction")]] ComplexInput std::optional owner; }; +struct NestedDefaultInput; + +struct [[nodiscard("unnecessary construction")]] DefaultInput +{ + explicit DefaultInput() noexcept; + explicit DefaultInput( + std::optional> valuesArg, + std::optional countArg, + std::unique_ptr childArg, + int requiredArg) noexcept; + DefaultInput(const DefaultInput& other); + DefaultInput(DefaultInput&& other) noexcept; + ~DefaultInput(); + + DefaultInput& operator=(const DefaultInput& other); + DefaultInput& operator=(DefaultInput&& other) noexcept; + + std::optional> values; + std::optional count; + std::unique_ptr child; + int required; +}; + +struct [[nodiscard("unnecessary construction")]] NestedDefaultInput +{ + explicit NestedDefaultInput() noexcept; + explicit NestedDefaultInput( + std::optional countArg) noexcept; + NestedDefaultInput(const NestedDefaultInput& other); + NestedDefaultInput(NestedDefaultInput&& other) noexcept; + ~NestedDefaultInput(); + + NestedDefaultInput& operator=(const NestedDefaultInput& other); + NestedDefaultInput& operator=(NestedDefaultInput&& other) noexcept; + + std::optional count; +}; + } // namespace validation } // namespace graphql diff --git a/samples/validation/schema/ValidationSharedTypes.ixx b/samples/validation/schema/ValidationSharedTypes.ixx index 9dbcfe15..ffa52de7 100644 --- a/samples/validation/schema/ValidationSharedTypes.ixx +++ b/samples/validation/schema/ValidationSharedTypes.ixx @@ -20,5 +20,7 @@ using validation::getCatCommandNames; using validation::getCatCommandValues; using validation::ComplexInput; +using validation::DefaultInput; +using validation::NestedDefaultInput; } // namespace graphql::validation diff --git a/samples/validation/schema/schema.validation.graphql b/samples/validation/schema/schema.validation.graphql index e1515c39..fd2325c2 100644 --- a/samples/validation/schema/schema.validation.graphql +++ b/samples/validation/schema/schema.validation.graphql @@ -130,3 +130,19 @@ extend type Query { "[Example 167](https://spec.graphql.org/October2021/#example-ce150)" booleanList(booleanListArg: [Boolean!]): Boolean } + +"Input-field defaults apply to omitted fields while preserving explicit nulls." +input DefaultInput { + values: [Int!] = [] + count: Int = 42 + child: NestedDefaultInput = {} + required: Int! = 9 +} + +input NestedDefaultInput { + count: Int = 7 +} + +extend type Query { + inputDefaults(input: DefaultInput!): String! +} diff --git a/test/ValidationTests.cpp b/test/ValidationTests.cpp index de1cb550..534f7daa 100644 --- a/test/ValidationTests.cpp +++ b/test/ValidationTests.cpp @@ -2042,3 +2042,153 @@ TEST_F(ValidationExamplesCase, Example187) ASSERT_TRUE(errors.empty()); } + +namespace { + +struct InputCase +{ + const char* json; + const char* literal; + std::optional> values; + std::optional count; + bool child; + std::optional childCount; +}; + +const InputCase cases[] { + { "{}", "{}", std::vector {}, 42, true, 7 }, + { R"({"values":null,"count":null,"child":null})", + "{values:null,count:null,child:null}", + std::nullopt, + std::nullopt, + false, + std::nullopt }, + { R"({"values":[]})", "{values:[]}", std::vector {}, 42, true, 7 }, + { R"({"values":[1,2],"count":3,"child":{"count":4}})", + "{values:[1,2],count:3,child:{count:4}}", + std::vector { 1, 2 }, + 3, + true, + 4 }, + { R"({"child":{}})", "{child:{}}", std::vector {}, 42, true, 7 }, + { R"({"child":{"count":null}})", + "{child:{count:null}}", + std::vector {}, + 42, + true, + std::nullopt }, +}; + +void checkInput(const validation::DefaultInput& input, const InputCase& expected) +{ + EXPECT_EQ(input.values, expected.values); + EXPECT_EQ(input.count, expected.count); + ASSERT_EQ(static_cast(input.child), expected.child); + if (input.child) + { + EXPECT_EQ(input.child->count, expected.childCount); + } + EXPECT_EQ(input.required, 9); +} + +struct InputDefaultsQuery +{ + std::optional received; + + std::string getInputDefaults(validation::DefaultInput input) + { + received = std::move(input); + return "ok"; + } +}; + +const char* invalidInputs[] { + R"({"values":[null]})", + R"({"values":["bad"]})", + R"({"values":{}})", + R"({"count":"bad"})", + R"({"child":{"count":"bad"}})", + R"({"child":1})", + R"({"required":null})", + R"({"required":"bad"})", +}; + +} // namespace + +TEST(InputDefaultsCase, GeneratedConversion) +{ + for (const auto& expected : cases) + { + SCOPED_TRACE(expected.json); + auto input = + service::Argument::convert(response::parseJSON(expected.json)); + checkInput(input, expected); + } +} + +TEST(InputDefaultsCase, InvalidGeneratedConversion) +{ + for (const auto* json : invalidInputs) + { + SCOPED_TRACE(json); + EXPECT_ANY_THROW(static_cast( + service::Argument::convert(response::parseJSON(json)))); + } +} + +TEST(InputDefaultsCase, RequestExecution) +{ + for (const auto& expected : cases) + { + for (bool variables : { false, true }) + { + SCOPED_TRACE(expected.json); + SCOPED_TRACE(variables); + auto impl = std::make_shared(); + validation::Operations service(impl, std::make_shared()); + auto query = peg::parseString(variables + ? "query($input:DefaultInput!){inputDefaults(input:$input)}" + : std::string("{inputDefaults(input:") + expected.literal + ")}"); + auto args = response::parseJSON(std::string("{\"input\":") + expected.json + "}"); + auto result = service.resolve({ query, {}, std::move(args) }).get(); + EXPECT_EQ(response::toJSON(std::move(result)), R"({"data":{"inputDefaults":"ok"}})"); + ASSERT_TRUE(impl->received); + checkInput(*impl->received, expected); + } + } +} + +TEST(InputDefaultsCase, InvalidRequestExecution) +{ + for (const auto* json : invalidInputs) + { + SCOPED_TRACE(json); + auto impl = std::make_shared(); + validation::Operations service(impl, std::make_shared()); + auto query = "query($input:DefaultInput!){inputDefaults(input:$input)}"_graphql; + auto args = response::parseJSON(std::string("{\"input\":") + json + "}"); + auto result = service.resolve({ query, {}, std::move(args) }).get(); + EXPECT_FALSE(impl->received); + ASSERT_NE(result.find("errors"), result.get().cend()); + EXPECT_GT(result["errors"].size(), 0U); + } +} + +TEST(InputDefaultsCase, FindDistinguishesAbsenceAndInvalidValues) +{ + const std::string name = "value"; + auto absent = response::parseJSON("{}"); + EXPECT_FALSE(service::IntArgument::find(name, absent).second); + EXPECT_FALSE(service::IntArgument::find<>(name, absent).second); + EXPECT_FALSE(service::IntArgument::find(name, absent).second); + for (const auto* json : { R"({"value":null})", R"({"value":"bad"})" }) + { + auto invalid = response::parseJSON(json); + EXPECT_ANY_THROW(static_cast(service::IntArgument::find(name, invalid))); + EXPECT_ANY_THROW(static_cast(service::IntArgument::find<>(name, invalid))); + } + auto null = response::parseJSON(R"({"value":null})"); + auto found = service::IntArgument::find(name, null); + EXPECT_TRUE(found.second); + EXPECT_FALSE(found.first); +}