Fix #239

wravery · wravery · commit c78c31b2889b · 2022-05-03T14:08:30.000-07:00
diff --git a/include/graphqlservice/GraphQLService.h b/include/graphqlservice/GraphQLService.h
@@ -1236,6 +1236,9 @@ struct [[nodiscard]] RequestSubscribeParams
 
 	// Optional sub-class of RequestState which will be passed to each resolver and field accessor.
 	std::shared_ptr<RequestState> state {};
+
+	// Optional override for the default Subscription operation object.
+	std::shared_ptr<const Object> subscriptionObject {};
 };
 
 struct [[nodiscard]] RequestUnsubscribeParams
@@ -1245,6 +1248,9 @@ struct [[nodiscard]] RequestUnsubscribeParams
 
 	// Optional async execution awaitable.
 	await_async launch {};
+
+	// Optional override for the default Subscription operation object.
+	std::shared_ptr<const Object> subscriptionObject {};
 };
 
 using SubscriptionArguments = std::map<std::string_view, response::Value>;
@@ -1321,6 +1327,13 @@ struct [[nodiscard]] SubscriptionData : std::enable_shared_from_this<Subscriptio
 	const peg::ast_node& selection;
 };
 
+// Placeholder for an empty subscription object.
+class SubscriptionPlaceholder
+{
+	// Private constructor, since it should never actually be instantiated.
+	SubscriptionPlaceholder() noexcept = default;
+};
+
 // Forward declare just the class type so we can reference it in the Request::_validation member.
 class ValidateExecutableVisitor;
 
diff --git a/samples/learn/schema/StarWarsSchema.cpp b/samples/learn/schema/StarWarsSchema.cpp
@@ -98,8 +98,8 @@ namespace learn {
 
 Operations::Operations(std::shared_ptr<object::Query> query, std::shared_ptr<object::Mutation> mutation)
 	: service::Request({
-		{ "query", query },
-		{ "mutation", mutation }
+		{ service::strQuery, query },
+		{ service::strMutation, mutation }
 	}, GetSchema())
 	, _query(std::move(query))
 	, _mutation(std::move(mutation))
diff --git a/samples/learn/schema/StarWarsSchema.h b/samples/learn/schema/StarWarsSchema.h
@@ -77,7 +77,10 @@ class [[nodiscard]] Operations final
 
 	template <class TQuery, class TMutation>
 	explicit Operations(std::shared_ptr<TQuery> query, std::shared_ptr<TMutation> mutation)
-		: Operations { std::make_shared<object::Query>(std::move(query)), std::make_shared<object::Mutation>(std::move(mutation)) }
+		: Operations {
+			std::make_shared<object::Query>(std::move(query)),
+			std::make_shared<object::Mutation>(std::move(mutation))
+		}
 	{
 	}
 
diff --git a/samples/today/TodayMock.h b/samples/today/TodayMock.h
@@ -547,7 +547,7 @@ class NextAppointmentChange
 
 	std::shared_ptr<object::Node> getNodeChange(const response::IdType&) const
 	{
-		return {};
+		throw std::runtime_error("Unexpected call to getNodeChange");
 	}
 
 private:
@@ -561,8 +561,9 @@ class NextAppointmentChange
 class NodeChange
 {
 public:
-	using nodeChange = std::function<std::shared_ptr<object::Node>(
-		const std::shared_ptr<service::RequestState>&, response::IdType&&)>;
+	using nodeChange =
+		std::function<std::shared_ptr<object::Node>(service::ResolverContext resolverContext,
+			const std::shared_ptr<service::RequestState>&, response::IdType&&)>;
 
 	explicit NodeChange(nodeChange&& changeNode)
 		: _changeNode(std::move(changeNode))
@@ -577,7 +578,7 @@ class NodeChange
 	std::shared_ptr<object::Node> getNodeChange(
 		const service::FieldParams& params, response::IdType&& idArg) const
 	{
-		return _changeNode(params.state, std::move(idArg));
+		return _changeNode(params.resolverContext, params.state, std::move(idArg));
 	}
 
 private:
diff --git a/samples/today/nointrospection/TodaySchema.cpp b/samples/today/nointrospection/TodaySchema.cpp
@@ -227,9 +227,9 @@ namespace today {
 
 Operations::Operations(std::shared_ptr<object::Query> query, std::shared_ptr<object::Mutation> mutation, std::shared_ptr<object::Subscription> subscription)
 	: service::Request({
-		{ "query", query },
-		{ "mutation", mutation },
-		{ "subscription", subscription }
+		{ service::strQuery, query },
+		{ service::strMutation, mutation },
+		{ service::strSubscription, subscription }
 	}, GetSchema())
 	, _query(std::move(query))
 	, _mutation(std::move(mutation))
diff --git a/samples/today/nointrospection/TodaySchema.h b/samples/today/nointrospection/TodaySchema.h
@@ -148,9 +148,13 @@ class [[nodiscard]] Operations final
 public:
 	explicit Operations(std::shared_ptr<object::Query> query, std::shared_ptr<object::Mutation> mutation, std::shared_ptr<object::Subscription> subscription);
 
-	template <class TQuery, class TMutation, class TSubscription>
-	explicit Operations(std::shared_ptr<TQuery> query, std::shared_ptr<TMutation> mutation, std::shared_ptr<TSubscription> subscription)
-		: Operations { std::make_shared<object::Query>(std::move(query)), std::make_shared<object::Mutation>(std::move(mutation)), std::make_shared<object::Subscription>(std::move(subscription)) }
+	template <class TQuery, class TMutation, class TSubscription = service::SubscriptionPlaceholder>
+	explicit Operations(std::shared_ptr<TQuery> query, std::shared_ptr<TMutation> mutation, std::shared_ptr<TSubscription> subscription = {})
+		: Operations {
+			std::make_shared<object::Query>(std::move(query)),
+			std::make_shared<object::Mutation>(std::move(mutation)),
+			subscription ? std::make_shared<object::Subscription>(std::move(subscription)) : std::shared_ptr<object::Subscription> {}
+		}
 	{
 	}
 
diff --git a/samples/today/schema/TodaySchema.cpp b/samples/today/schema/TodaySchema.cpp
@@ -227,9 +227,9 @@ namespace today {
 
 Operations::Operations(std::shared_ptr<object::Query> query, std::shared_ptr<object::Mutation> mutation, std::shared_ptr<object::Subscription> subscription)
 	: service::Request({
-		{ "query", query },
-		{ "mutation", mutation },
-		{ "subscription", subscription }
+		{ service::strQuery, query },
+		{ service::strMutation, mutation },
+		{ service::strSubscription, subscription }
 	}, GetSchema())
 	, _query(std::move(query))
 	, _mutation(std::move(mutation))
diff --git a/samples/today/schema/TodaySchema.h b/samples/today/schema/TodaySchema.h
@@ -148,9 +148,13 @@ class [[nodiscard]] Operations final
 public:
 	explicit Operations(std::shared_ptr<object::Query> query, std::shared_ptr<object::Mutation> mutation, std::shared_ptr<object::Subscription> subscription);
 
-	template <class TQuery, class TMutation, class TSubscription>
-	explicit Operations(std::shared_ptr<TQuery> query, std::shared_ptr<TMutation> mutation, std::shared_ptr<TSubscription> subscription)
-		: Operations { std::make_shared<object::Query>(std::move(query)), std::make_shared<object::Mutation>(std::move(mutation)), std::make_shared<object::Subscription>(std::move(subscription)) }
+	template <class TQuery, class TMutation, class TSubscription = service::SubscriptionPlaceholder>
+	explicit Operations(std::shared_ptr<TQuery> query, std::shared_ptr<TMutation> mutation, std::shared_ptr<TSubscription> subscription = {})
+		: Operations {
+			std::make_shared<object::Query>(std::move(query)),
+			std::make_shared<object::Mutation>(std::move(mutation)),
+			subscription ? std::make_shared<object::Subscription>(std::move(subscription)) : std::shared_ptr<object::Subscription> {}
+		}
 	{
 	}
 
diff --git a/samples/validation/ValidationMock.h b/samples/validation/ValidationMock.h
@@ -26,12 +26,6 @@ class Mutation
 	explicit Mutation() = default;
 };
 
-class Subscription
-{
-public:
-	explicit Subscription() = default;
-};
-
 } // namespace graphql::validation
 
 #endif // VALIDATIONMOCK_H
diff --git a/samples/validation/schema/ValidationSchema.cpp b/samples/validation/schema/ValidationSchema.cpp
@@ -155,9 +155,9 @@ namespace validation {
 
 Operations::Operations(std::shared_ptr<object::Query> query, std::shared_ptr<object::Mutation> mutation, std::shared_ptr<object::Subscription> subscription)
 	: service::Request({
-		{ "query", query },
-		{ "mutation", mutation },
-		{ "subscription", subscription }
+		{ service::strQuery, query },
+		{ service::strMutation, mutation },
+		{ service::strSubscription, subscription }
 	}, GetSchema())
 	, _query(std::move(query))
 	, _mutation(std::move(mutation))
diff --git a/samples/validation/schema/ValidationSchema.h b/samples/validation/schema/ValidationSchema.h
@@ -110,9 +110,13 @@ class [[nodiscard]] Operations final
 public:
 	explicit Operations(std::shared_ptr<object::Query> query, std::shared_ptr<object::Mutation> mutation, std::shared_ptr<object::Subscription> subscription);
 
-	template <class TQuery, class TMutation, class TSubscription>
-	explicit Operations(std::shared_ptr<TQuery> query, std::shared_ptr<TMutation> mutation, std::shared_ptr<TSubscription> subscription)
-		: Operations { std::make_shared<object::Query>(std::move(query)), std::make_shared<object::Mutation>(std::move(mutation)), std::make_shared<object::Subscription>(std::move(subscription)) }
+	template <class TQuery, class TMutation, class TSubscription = service::SubscriptionPlaceholder>
+	explicit Operations(std::shared_ptr<TQuery> query, std::shared_ptr<TMutation> mutation, std::shared_ptr<TSubscription> subscription = {})
+		: Operations {
+			std::make_shared<object::Query>(std::move(query)),
+			std::make_shared<object::Mutation>(std::move(mutation)),
+			subscription ? std::make_shared<object::Subscription>(std::move(subscription)) : std::shared_ptr<object::Subscription> {}
+		}
 	{
 	}
 
diff --git a/src/GraphQLService.cpp b/src/GraphQLService.cpp
@@ -1827,12 +1827,21 @@ AwaitableSubscribe Request::subscribe(RequestSubscribeParams params)
 	const auto spThis = shared_from_this();
 	const auto launch = std::move(params.launch);
 	std::unique_lock lock { spThis->_subscriptionMutex };
-	const auto key = spThis->addSubscription(std::move(params));
 	const auto itrOperation = spThis->_operations.find(strSubscription);
 
-	if (itrOperation != spThis->_operations.end())
+	if (itrOperation == _operations.end())
+	{
+		// There may be an empty entry in the operations map, but if it's completely missing then
+		// that means the schema doesn't support subscriptions at all.
+		throw std::logic_error("Subscriptions not supported");
+	}
+
+	const auto optionalOrDefaultSubscription =
+		params.subscriptionObject ? std::move(params.subscriptionObject) : itrOperation->second;
+	const auto key = spThis->addSubscription(std::move(params));
+
+	if (optionalOrDefaultSubscription)
 	{
-		const auto operation = itrOperation->second;
 		const auto registration = spThis->_subscriptions.at(key);
 		const SelectionSetParams selectionSetParams {
 			ResolverContext::NotifySubscribe,
@@ -1851,24 +1860,25 @@ AwaitableSubscribe Request::subscribe(RequestSubscribeParams params)
 		{
 			co_await launch;
 
-			auto errors = std::move((co_await operation->resolve(selectionSetParams,
-										 registration->selection,
-										 registration->data->fragments,
-										 registration->data->variables))
-										.errors);
+			auto errors =
+				std::move((co_await optionalOrDefaultSubscription->resolve(selectionSetParams,
+							   registration->selection,
+							   registration->data->fragments,
+							   registration->data->variables))
+							  .errors);
 
 			if (!errors.empty())
 			{
 				throw schema_exception { std::move(errors) };
 			}
 		}
-		catch (const std::exception& ex)
+		catch (...)
 		{
 			lock.lock();
 
 			// Rethrow the exception, but don't leave it subscribed if the resolver failed.
 			spThis->removeSubscription(key);
-			throw ex;
+			throw;
 		}
 	}
 
@@ -1880,11 +1890,20 @@ AwaitableUnsubscribe Request::unsubscribe(RequestUnsubscribeParams params)
 	const auto spThis = shared_from_this();
 	std::unique_lock lock { spThis->_subscriptionMutex };
 	const auto itrOperation = spThis->_operations.find(strSubscription);
+
+	if (itrOperation == _operations.end())
+	{
+		// There may be an empty entry in the operations map, but if it's completely missing then
+		// that means the schema doesn't support subscriptions at all.
+		throw std::logic_error("Subscriptions not supported");
+	}
+
+	const auto optionalOrDefaultSubscription =
+		params.subscriptionObject ? std::move(params.subscriptionObject) : itrOperation->second;
 	std::list<schema_error> errors {};
 
-	if (itrOperation != spThis->_operations.end())
+	if (optionalOrDefaultSubscription)
 	{
-		const auto operation = itrOperation->second;
 		const auto registration = spThis->_subscriptions.at(params.key);
 		const SelectionSetParams selectionSetParams {
 			ResolverContext::NotifyUnsubscribe,
@@ -1900,7 +1919,7 @@ AwaitableUnsubscribe Request::unsubscribe(RequestUnsubscribeParams params)
 		lock.unlock();
 
 		co_await params.launch;
-		errors = std::move((co_await operation->resolve(selectionSetParams,
+		errors = std::move((co_await optionalOrDefaultSubscription->resolve(selectionSetParams,
 								registration->selection,
 								registration->data->fragments,
 								registration->data->variables))
diff --git a/src/SchemaGenerator.cpp b/src/SchemaGenerator.cpp
@@ -372,6 +372,7 @@ static_assert(graphql::internal::MinorVersion == )cpp"
 
 	if (!_loader.isIntrospection())
 	{
+		bool hasSubscription = false;
 		bool firstOperation = true;
 
 		headerFile << R"cpp(class [[nodiscard]] Operations final
@@ -382,6 +383,8 @@ static_assert(graphql::internal::MinorVersion == )cpp"
 
 		for (const auto& operation : _loader.getOperationTypes())
 		{
+			hasSubscription = hasSubscription || operation.operation == service::strSubscription;
+
 			if (!firstOperation)
 			{
 				headerFile << R"cpp(, )cpp";
@@ -410,6 +413,11 @@ static_assert(graphql::internal::MinorVersion == )cpp"
 
 				firstOperation = false;
 				headerFile << R"cpp(class T)cpp" << operation.cppType;
+
+				if (hasSubscription && operation.operation == service::strSubscription)
+				{
+					headerFile << R"cpp( = service::SubscriptionPlaceholder)cpp";
+				}
 			}
 
 			headerFile << R"cpp(>
@@ -427,6 +435,11 @@ static_assert(graphql::internal::MinorVersion == )cpp"
 				firstOperation = false;
 				headerFile << R"cpp(std::shared_ptr<T)cpp" << operation.cppType << R"cpp(> )cpp"
 						   << operation.operation;
+
+				if (hasSubscription && operation.operation == service::strSubscription)
+				{
+					headerFile << R"cpp( = {})cpp";
+				}
 			}
 
 			headerFile << R"cpp()
@@ -442,11 +455,27 @@ static_assert(graphql::internal::MinorVersion == )cpp"
 				}
 
 				firstOperation = false;
-				headerFile << R"cpp( std::make_shared<object::)cpp" << operation.cppType
-						   << R"cpp(>(std::move()cpp" << operation.operation << R"cpp()))cpp";
+
+				if (hasSubscription && operation.operation == service::strSubscription)
+				{
+					headerFile << R"cpp(
+			)cpp" << operation.operation
+							   << R"cpp( ? std::make_shared<object::)cpp" << operation.cppType
+							   << R"cpp(>(std::move()cpp" << operation.operation
+							   << R"cpp()) : std::shared_ptr<object::)cpp" << operation.cppType
+							   << R"cpp(> {})cpp";
+				}
+				else
+				{
+					headerFile << R"cpp(
+			std::make_shared<object::)cpp"
+							   << operation.cppType << R"cpp(>(std::move()cpp"
+							   << operation.operation << R"cpp()))cpp";
+				}
 			}
 
-			headerFile << R"cpp( }
+			headerFile << R"cpp(
+		}
 	{
 	}
 )cpp";
@@ -1448,7 +1477,12 @@ Operations::Operations()cpp";
 			}
 
 			firstOperation = false;
-			sourceFile << R"cpp(		{ ")cpp" << operation.operation << R"cpp(", )cpp"
+
+			std::string operationName(operation.operation);
+
+			operationName[0] =
+				static_cast<char>(std::toupper(static_cast<unsigned char>(operationName[0])));
+			sourceFile << R"cpp(		{ service::str)cpp" << operationName << R"cpp(, )cpp"
 					   << operation.operation << R"cpp( })cpp";
 		}
 
diff --git a/test/TodayTests.cpp b/test/TodayTests.cpp
diff --git a/test/ValidationTests.cpp b/test/ValidationTests.cpp

Original file line number	Diff line number	Diff line change
`@@ -77,7 +77,10 @@ class [[nodiscard]] Operations final`
`77`	`77`
`78`	`78`	`template <class TQuery, class TMutation>`
`79`	`79`	`explicit Operations(std::shared_ptr<TQuery> query, std::shared_ptr<TMutation> mutation)`
`80`		`- : Operations { std::make_shared<object::Query>(std::move(query)), std::make_shared<object::Mutation>(std::move(mutation)) }`
	`80`	`+ : Operations {`
	`81`	`+ std::make_shared<object::Query>(std::move(query)),`
	`82`	`+ std::make_shared<object::Mutation>(std::move(mutation))`
	`83`	`+ }`
`81`	`84`	`{`
`82`	`85`	`}`
`83`	`86`
Original file line number	Diff line number	Diff line change
`@@ -547,7 +547,7 @@ class NextAppointmentChange`
`547`	`547`
`548`	`548`	`std::shared_ptr<object::Node> getNodeChange(const response::IdType&) const`
`549`	`549`	`{`
`550`		`- return {};`
	`550`	`+ throw std::runtime_error("Unexpected call to getNodeChange");`
`551`	`551`	`}`
`552`	`552`
`553`	`553`	`private:`
`@@ -561,8 +561,9 @@ class NextAppointmentChange`
`561`	`561`	`class NodeChange`
`562`	`562`	`{`
`563`	`563`	`public:`
`564`		`- using nodeChange = std::function<std::shared_ptr<object::Node>(`
`565`		`- const std::shared_ptr<service::RequestState>&, response::IdType&&)>;`
	`564`	`+ using nodeChange =`
	`565`	`+ std::function<std::shared_ptr<object::Node>(service::ResolverContext resolverContext,`
	`566`	`+ const std::shared_ptr<service::RequestState>&, response::IdType&&)>;`
`566`	`567`
`567`	`568`	`explicit NodeChange(nodeChange&& changeNode)`
`568`	`569`	`: _changeNode(std::move(changeNode))`
`@@ -577,7 +578,7 @@ class NodeChange`
`577`	`578`	`std::shared_ptr<object::Node> getNodeChange(`
`578`	`579`	`const service::FieldParams& params, response::IdType&& idArg) const`
`579`	`580`	`{`
`580`		`- return _changeNode(params.state, std::move(idArg));`
	`581`	`+ return _changeNode(params.resolverContext, params.state, std::move(idArg));`
`581`	`582`	`}`
`582`	`583`
`583`	`584`	`private:`