Merge pull request #32 from wravery/enum_args

wravery · web-flow · commit ee264a60568d · 2019-01-26T20:55:46.000-08:00
Fix for issue #30
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -138,6 +138,9 @@ if(BUILD_TESTS OR UPDATE_SAMPLES)
     add_test(NAME PegtlCase
       COMMAND tests --gtest_filter=PegtlCase.*
       WORKING_DIRECTORY $<TARGET_FILE_DIR:tests>)
+    add_test(NAME ResponseCase
+      COMMAND tests --gtest_filter=ResponseCase.*
+      WORKING_DIRECTORY $<TARGET_FILE_DIR:tests>)
   endif()
 
   if(UPDATE_SAMPLES)
diff --git a/GraphQLResponse.cpp b/GraphQLResponse.cpp
@@ -44,6 +44,12 @@ Value::~Value()
 	// omitted, declare it explicitly and define it in graphqlservice.
 }
 
+Value::Value(const char* value)
+	: _type(Type::String)
+	, _string(new StringType(value))
+{
+}
+
 Value::Value(StringType&& value)
 	: _type(Type::String)
 	, _string(new StringType(std::move(value)))
@@ -74,12 +80,14 @@ Value::Value(Value&& other) noexcept
 	, _map(std::move(other._map))
 	, _list(std::move(other._list))
 	, _string(std::move(other._string))
+	, _from_json(other._from_json)
 	, _scalar(std::move(other._scalar))
 	, _boolean(other._boolean)
 	, _int(other._int)
 	, _float(other._float)
 {
 	const_cast<Type&>(other._type) = Type::Null;
+	other._from_json = false;
 	other._boolean = false;
 	other._int = 0;
 	other._float = 0.0;
@@ -125,6 +133,8 @@ Value& Value::operator=(Value&& rhs) noexcept
 	_map = std::move(rhs._map);
 	_list = std::move(rhs._list);
 	_string = std::move(rhs._string);
+	_from_json = rhs._from_json;
+	rhs._from_json = false;
 	_scalar = std::move(rhs._scalar);
 	_boolean = rhs._boolean;
 	rhs._boolean = false;
@@ -180,11 +190,24 @@ bool Value::operator!=(const Value& rhs) const noexcept
 	return !(*this == rhs);
 }
 
-Type Value::type() const
+Type Value::type() const  noexcept
 {
 	return _type;
 }
 
+Value&& Value::from_json() noexcept
+{
+	_from_json = true;
+
+	return std::move(*this);
+}
+
+bool Value::maybe_enum() const noexcept
+{
+	return _type == Type::EnumValue
+		|| (_from_json && _type == Type::String);
+}
+
 void Value::reserve(size_t count)
 {
 	switch (_type)
diff --git a/JSONResponse.cpp b/JSONResponse.cpp
@@ -185,7 +185,7 @@ struct ResponseHandler
 
 	bool String(const Ch* str, rapidjson::SizeType /*length*/, bool /*copy*/)
 	{
-		setValue(Value(std::string(str)));
+		setValue(Value(std::string(str)).from_json());
 		return true;
 	}
 
diff --git a/SchemaGenerator.cpp b/SchemaGenerator.cpp
@@ -1623,7 +1623,7 @@ template <>
 			sourceFile << R"cpp(
 	};
 
-	if (value.type() != response::Type::EnumValue)
+	if (!value.maybe_enum())
 	{
 		throw service::schema_exception({ "not a valid )cpp" << enumType.type << R"cpp( value" });
 	}
@@ -1719,33 +1719,7 @@ template <>
 
 			for (const auto& inputField : inputType.fields)
 			{
-				std::string fieldName(inputField.name);
-
-				fieldName[0] = std::toupper(fieldName[0]);
-				if (inputField.defaultValue.type() == response::Type::Null)
-				{
-					sourceFile << R"cpp(	auto value)cpp" << fieldName
-						<< R"cpp( = )cpp" << getArgumentAccessType(inputField)
-						<< R"cpp(::require)cpp" << getTypeModifiers(inputField.modifiers)
-						<< R"cpp((")cpp" << inputField.name
-						<< R"cpp(", value);
-)cpp";
-				}
-				else
-				{
-					sourceFile << R"cpp(	auto pair)cpp" << fieldName
-						<< R"cpp( = )cpp" << getArgumentAccessType(inputField)
-						<< R"cpp(::find)cpp" << getTypeModifiers(inputField.modifiers)
-						<< R"cpp((")cpp" << inputField.name
-						<< R"cpp(", value);
-	auto value)cpp" << fieldName << R"cpp( = (pair)cpp" << fieldName << R"cpp(.second
-		? std::move(pair)cpp" << fieldName << R"cpp(.first)
-		: )cpp" << getArgumentAccessType(inputField)
-						<< R"cpp(::require)cpp" << getTypeModifiers(inputField.modifiers)
-						<< R"cpp((")cpp" << inputField.name
-						<< R"cpp(", defaultValue));
-)cpp";
-				}
+				sourceFile << getArgumentDeclaration(inputField, "value", "value", "defaultValue");
 			}
 
 			if (!inputType.fields.empty())
@@ -1942,33 +1916,7 @@ std::future<response::Value> )cpp" << objectType.type
 
 					for (const auto& argument : outputField.arguments)
 					{
-						std::string argumentName(argument.name);
-
-						argumentName[0] = std::toupper(argumentName[0]);
-						if (argument.defaultValue.type() == response::Type::Null)
-						{
-							sourceFile << R"cpp(	auto arg)cpp" << argumentName
-								<< R"cpp( = )cpp" << getArgumentAccessType(argument)
-								<< R"cpp(::require)cpp" << getTypeModifiers(argument.modifiers)
-								<< R"cpp((")cpp" << argument.name
-								<< R"cpp(", params.arguments);
-)cpp";
-						}
-						else
-						{
-							sourceFile << R"cpp(	auto pair)cpp" << argumentName
-								<< R"cpp( = )cpp" << getArgumentAccessType(argument)
-								<< R"cpp(::find)cpp" << getTypeModifiers(argument.modifiers)
-								<< R"cpp((")cpp" << argument.name
-								<< R"cpp(", params.arguments);
-	auto arg)cpp" << argumentName << R"cpp( = (pair)cpp" << argumentName << R"cpp(.second
-		? std::move(pair)cpp" << argumentName << R"cpp(.first)
-		: )cpp" << getArgumentAccessType(argument)
-								<< R"cpp(::require)cpp" << getTypeModifiers(argument.modifiers)
-								<< R"cpp((")cpp" << argument.name
-								<< R"cpp(", defaultArguments));
-)cpp";
-						}
+						sourceFile << getArgumentDeclaration(argument, "arg", "params.arguments", "defaultArguments");
 					}
 				}
 
@@ -2733,6 +2681,43 @@ std::string Generator::getArgumentDefaultValue(size_t level, const response::Val
 	return argumentDefaultValue.str();
 }
 
+std::string Generator::getArgumentDeclaration(const InputField& argument, const char* prefixToken, const char* argumentsToken, const char* defaultToken) const noexcept
+{
+	std::ostringstream argumentDeclaration;
+	std::string argumentName(argument.name);
+
+	argumentName[0] = std::toupper(argumentName[0]);
+	if (argument.defaultValue.type() == response::Type::Null)
+	{
+		argumentDeclaration << R"cpp(	auto )cpp" << prefixToken << argumentName
+			<< R"cpp( = )cpp" << getArgumentAccessType(argument)
+			<< R"cpp(::require)cpp" << getTypeModifiers(argument.modifiers)
+			<< R"cpp((")cpp" << argument.name
+			<< R"cpp(", )cpp" << argumentsToken
+			<< R"cpp();
+)cpp";
+	}
+	else
+	{
+		argumentDeclaration << R"cpp(	auto pair)cpp" << argumentName
+			<< R"cpp( = )cpp" << getArgumentAccessType(argument)
+			<< R"cpp(::find)cpp" << getTypeModifiers(argument.modifiers)
+			<< R"cpp((")cpp" << argument.name
+			<< R"cpp(", )cpp" << argumentsToken
+			<< R"cpp();
+	auto )cpp" << prefixToken << argumentName << R"cpp( = (pair)cpp" << argumentName << R"cpp(.second
+		? std::move(pair)cpp" << argumentName << R"cpp(.first)
+		: )cpp" << getArgumentAccessType(argument)
+			<< R"cpp(::require)cpp" << getTypeModifiers(argument.modifiers)
+			<< R"cpp((")cpp" << argument.name
+			<< R"cpp(", )cpp" << defaultToken
+			<< R"cpp());
+)cpp";
+	}
+
+	return argumentDeclaration.str();
+}
+
 std::string Generator::getArgumentAccessType(const InputField& argument) const noexcept
 {
 	std::ostringstream argumentType;
diff --git a/include/SchemaGenerator.h b/include/SchemaGenerator.h
@@ -281,6 +281,7 @@ class Generator
 
 	bool outputSource() const noexcept;
 	std::string getArgumentDefaultValue(size_t level, const response::Value& defaultValue) const noexcept;
+	std::string getArgumentDeclaration(const InputField& argument, const char* prefixToken, const char* argumentsToken, const char* defaultToken) const noexcept;
 	std::string getArgumentAccessType(const InputField& argument) const noexcept;
 	std::string getResultAccessType(const OutputField& result) const noexcept;
 	std::string getTypeModifiers(const TypeModifierStack& modifiers) const noexcept;
diff --git a/include/graphqlservice/GraphQLResponse.h b/include/graphqlservice/GraphQLResponse.h
@@ -44,6 +44,7 @@ struct Value
 	Value(Type type = Type::Null);
 	~Value();
 
+	explicit Value(const char* value);
 	explicit Value(StringType&& value);
 	explicit Value(BooleanType value);
 	explicit Value(IntType value);
@@ -60,7 +61,12 @@ struct Value
 	bool operator!=(const Value& rhs) const noexcept;
 
 	// Check the Type
-	Type type() const;
+	Type type() const noexcept;
+
+	// JSON doesn't distinguish between Type::String and Type::EnumValue, so if this value comes
+	// from JSON and it's a string we need to track the fact that it can be interpreted as either.
+	Value&& from_json() noexcept;
+	bool maybe_enum() const noexcept;
 
 	// Valid for Type::Map or Type::List
 	void reserve(size_t count);
@@ -101,6 +107,7 @@ struct Value
 
 	// Type::String or Type::EnumValue
 	std::unique_ptr<StringType> _string;
+	bool _from_json = false;
 
 	// Type::Boolean
 	BooleanType _boolean = false;
diff --git a/samples/IntrospectionSchema.cpp b/samples/IntrospectionSchema.cpp
@@ -27,7 +27,7 @@ introspection::__TypeKind ModifiedArgument<introspection::__TypeKind>::convert(c
 		{ "NON_NULL", introspection::__TypeKind::NON_NULL }
 	};
 
-	if (value.type() != response::Type::EnumValue)
+	if (!value.maybe_enum())
 	{
 		throw service::schema_exception({ "not a valid __TypeKind value" });
 	}
@@ -89,7 +89,7 @@ introspection::__DirectiveLocation ModifiedArgument<introspection::__DirectiveLo
 		{ "INPUT_FIELD_DEFINITION", introspection::__DirectiveLocation::INPUT_FIELD_DEFINITION }
 	};
 
-	if (value.type() != response::Type::EnumValue)
+	if (!value.maybe_enum())
 	{
 		throw service::schema_exception({ "not a valid __DirectiveLocation value" });
 	}
diff --git a/samples/TodaySchema.cpp b/samples/TodaySchema.cpp
@@ -25,7 +25,7 @@ today::TaskState ModifiedArgument<today::TaskState>::convert(const response::Val
 		{ "Unassigned", today::TaskState::Unassigned }
 	};
 
-	if (value.type() != response::Type::EnumValue)
+	if (!value.maybe_enum())
 	{
 		throw service::schema_exception({ "not a valid TaskState value" });
 	}
diff --git a/tests.cpp b/tests.cpp
@@ -1242,6 +1242,50 @@ TEST(ArgumentsCase, TaskStateEnum)
 	EXPECT_EQ(today::TaskState::Started, actual) << "should parse the enum";
 }
 
+TEST(ArgumentsCase, TaskStateEnumFromString)
+{
+	response::Value response(response::Type::Map);
+	response::Value status("Started");
+	response.emplace_back("status", std::move(status));
+	today::TaskState actual = static_cast<today::TaskState>(-1);
+	bool caughtException = false;
+	std::string exceptionWhat;
+
+	try
+	{
+		actual = service::ModifiedArgument<today::TaskState>::require("status", response);
+	}
+	catch (const service::schema_exception& ex)
+	{
+		caughtException = true;
+		exceptionWhat = response::toJSON(response::Value(ex.getErrors()));
+	}
+
+	EXPECT_NE(today::TaskState::Started, actual) << "should not parse the enum from a known string value";
+	ASSERT_TRUE(caughtException);
+	EXPECT_EQ(R"js([{"message":"Invalid argument: status message: not a valid TaskState value"}])js", exceptionWhat) << "exception should match";
+}
+
+TEST(ArgumentsCase, TaskStateEnumFromJSONString)
+{
+	response::Value response(response::Type::Map);
+	response::Value status("Started");
+	response.emplace_back("status", status.from_json());
+	today::TaskState actual = static_cast<today::TaskState>(-1);
+
+
+	try
+	{
+		actual = service::ModifiedArgument<today::TaskState>::require("status", response);
+	}
+	catch (const service::schema_exception& ex)
+	{
+		FAIL() << response::toJSON(response::Value(ex.getErrors()));
+	}
+
+	EXPECT_EQ(today::TaskState::Started, actual) << "should parse the enum";
+}
+
 TEST(PegtlCase, ParseKitchenSinkQuery)
 {
 	memory_input<> input(R"gql(
@@ -1624,3 +1668,12 @@ TEST(PegtlCase, AnalyzeGrammar)
 {
 	ASSERT_EQ(0, analyze<document>(true)) << "there shuldn't be any infinite loops in the PEG version of the grammar";
 }
+
+TEST(ResponseCase, ValueConstructorFromStringLiteral)
+{
+	auto expected = "Test String";
+	auto actual = response::Value(expected);
+
+	ASSERT_TRUE(response::Type::String == actual.type());
+	ASSERT_EQ(expected, actual.release<response::StringType>());
+}

Original file line number	Diff line number	Diff line change
`@@ -185,7 +185,7 @@ struct ResponseHandler`
`185`	`185`
`186`	`186`	`bool String(const Ch* str, rapidjson::SizeType /length/, bool /copy/)`
`187`	`187`	`{`
`188`		`- setValue(Value(std::string(str)));`
	`188`	`+ setValue(Value(std::string(str)).from_json());`
`189`	`189`	`return true;`
`190`	`190`	`}`
`191`	`191`
Original file line number	Diff line number	Diff line change
`@@ -27,7 +27,7 @@ introspection::__TypeKind ModifiedArgument<introspection::__TypeKind>::convert(c`
`27`	`27`	`{ "NON_NULL", introspection::__TypeKind::NON_NULL }`
`28`	`28`	`};`
`29`	`29`
`30`		`- if (value.type() != response::Type::EnumValue)`
	`30`	`+ if (!value.maybe_enum())`
`31`	`31`	`{`
`32`	`32`	`throw service::schema_exception({ "not a valid __TypeKind value" });`
`33`	`33`	`}`
`@@ -89,7 +89,7 @@ introspection::__DirectiveLocation ModifiedArgument<introspection::__DirectiveLo`
`89`	`89`	`{ "INPUT_FIELD_DEFINITION", introspection::__DirectiveLocation::INPUT_FIELD_DEFINITION }`
`90`	`90`	`};`
`91`	`91`
`92`		`- if (value.type() != response::Type::EnumValue)`
	`92`	`+ if (!value.maybe_enum())`
`93`	`93`	`{`
`94`	`94`	`throw service::schema_exception({ "not a valid __DirectiveLocation value" });`
`95`	`95`	`}`
Original file line number	Diff line number	Diff line change
`@@ -25,7 +25,7 @@ today::TaskState ModifiedArgument<today::TaskState>::convert(const response::Val`
`25`	`25`	`{ "Unassigned", today::TaskState::Unassigned }`
`26`	`26`	`};`
`27`	`27`
`28`		`- if (value.type() != response::Type::EnumValue)`
	`28`	`+ if (!value.maybe_enum())`
`29`	`29`	`{`
`30`	`30`	`throw service::schema_exception({ "not a valid TaskState value" });`
`31`	`31`	`}`