Simplify splitting service response into data and errors

wravery · wravery · commit cef14e1d2aaf · 2021-05-27T16:52:01.000-07:00
diff --git a/include/graphqlservice/GraphQLClient.h b/include/graphqlservice/GraphQLClient.h
@@ -31,6 +31,36 @@
 
 namespace graphql::client {
 
+// Errors may specify the line number and column number where the error occurred.
+struct ErrorLocation
+{
+	response::IntType line {};
+	response::IntType column {};
+};
+
+// Errors may specify a path to the field which triggered the error. The path consists of
+// field names and the indices of elements in a list.
+using ErrorPathSegment = std::variant<response::StringType, response::IntType>;
+
+// Error returned from the service.
+struct Error
+{
+	std::string message;
+	std::vector<ErrorLocation> locations;
+	std::vector<ErrorPathSegment> path;
+};
+
+// Complete response from the service, split into the unparsed graphql::response::Value in
+// data and the (typically empty) collection of Errors in errors.
+struct ServiceResponse
+{
+	response::Value data;
+	std::vector<Error> errors;
+};
+
+// Split a service response into separate ServiceResponse data and errors members.
+GRAPHQLCLIENT_EXPORT ServiceResponse parseServiceResponse(response::Value&& response);
+
 // GraphQL types are nullable by default, but they may be wrapped with non-null or list types.
 // Since nullability is a more special case in C++, we invert the default and apply that modifier
 // instead when the non-null wrapper is not present in that part of the wrapper chain.
diff --git a/samples/client/BenchmarkClient.cpp b/samples/client/BenchmarkClient.cpp
@@ -5,8 +5,6 @@
 
 #include "BenchmarkClient.h"
 
-#include "graphqlservice/GraphQLClient.h"
-
 #include <algorithm>
 #include <array>
 #include <stdexcept>
diff --git a/samples/client/BenchmarkClient.h b/samples/client/BenchmarkClient.h
@@ -8,6 +8,7 @@
 #ifndef BENCHMARKCLIENT_H
 #define BENCHMARKCLIENT_H
 
+#include "graphqlservice/GraphQLClient.h"
 #include "graphqlservice/GraphQLParse.h"
 #include "graphqlservice/GraphQLResponse.h"
 
diff --git a/samples/client/MutateClient.cpp b/samples/client/MutateClient.cpp
@@ -5,8 +5,6 @@
 
 #include "MutateClient.h"
 
-#include "graphqlservice/GraphQLClient.h"
-
 #include <algorithm>
 #include <array>
 #include <stdexcept>
diff --git a/samples/client/MutateClient.h b/samples/client/MutateClient.h
@@ -8,6 +8,7 @@
 #ifndef MUTATECLIENT_H
 #define MUTATECLIENT_H
 
+#include "graphqlservice/GraphQLClient.h"
 #include "graphqlservice/GraphQLParse.h"
 #include "graphqlservice/GraphQLResponse.h"
 
diff --git a/samples/client/QueryClient.cpp b/samples/client/QueryClient.cpp
@@ -5,8 +5,6 @@
 
 #include "QueryClient.h"
 
-#include "graphqlservice/GraphQLClient.h"
-
 #include <algorithm>
 #include <array>
 #include <stdexcept>
diff --git a/samples/client/QueryClient.h b/samples/client/QueryClient.h
@@ -8,6 +8,7 @@
 #ifndef QUERYCLIENT_H
 #define QUERYCLIENT_H
 
+#include "graphqlservice/GraphQLClient.h"
 #include "graphqlservice/GraphQLParse.h"
 #include "graphqlservice/GraphQLResponse.h"
 
diff --git a/samples/client/SubscribeClient.cpp b/samples/client/SubscribeClient.cpp
@@ -5,8 +5,6 @@
 
 #include "SubscribeClient.h"
 
-#include "graphqlservice/GraphQLClient.h"
-
 #include <algorithm>
 #include <array>
 #include <stdexcept>
diff --git a/samples/client/SubscribeClient.h b/samples/client/SubscribeClient.h
@@ -8,6 +8,7 @@
 #ifndef SUBSCRIBECLIENT_H
 #define SUBSCRIBECLIENT_H
 
+#include "graphqlservice/GraphQLClient.h"
 #include "graphqlservice/GraphQLParse.h"
 #include "graphqlservice/GraphQLResponse.h"
 
diff --git a/samples/client/benchmark.cpp b/samples/client/benchmark.cpp
@@ -1,8 +1,8 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 
-#include "TodayMock.h"
 #include "BenchmarkClient.h"
+#include "TodayMock.h"
 
 #include <chrono>
 #include <iostream>
@@ -128,6 +128,7 @@ int main(int argc, char** argv)
 
 	auto service = buildService();
 	std::vector<std::chrono::steady_clock::duration> durationResolve(iterations);
+	std::vector<std::chrono::steady_clock::duration> durationParseServiceResponse(iterations);
 	std::vector<std::chrono::steady_clock::duration> durationParseResponse(iterations);
 	const auto startTime = std::chrono::steady_clock::now();
 
@@ -142,11 +143,14 @@ int main(int argc, char** argv)
 			const auto startResolve = std::chrono::steady_clock::now();
 			auto response =
 				service->resolve(nullptr, query, "", response::Value(response::Type::Map)).get();
+			const auto startParseServiceResponse = std::chrono::steady_clock::now();
+			auto serviceResponse = client::parseServiceResponse(std::move(response));
 			const auto startParseResponse = std::chrono::steady_clock::now();
-			const auto parsed = parseResponse(std::move(response));
+			const auto parsed = parseResponse(std::move(serviceResponse.data));
 			const auto endParseResponse = std::chrono::steady_clock::now();
 
-			durationResolve[i] = startParseResponse - startResolve;
+			durationResolve[i] = startParseServiceResponse - startResolve;
+			durationParseServiceResponse[i] = startParseResponse - startParseServiceResponse;
 			durationParseResponse[i] = endParseResponse - startParseResponse;
 		}
 	}
@@ -162,6 +166,7 @@ int main(int argc, char** argv)
 	outputOverview(iterations, totalDuration);
 
 	outputSegment("Resolve"sv, durationResolve);
+	outputSegment("ParseServiceResponse"sv, durationParseServiceResponse);
 	outputSegment("ParseResponse"sv, durationParseResponse);
 
 	return 0;
diff --git a/src/ClientGenerator.cpp b/src/ClientGenerator.cpp
@@ -190,7 +190,8 @@ bool Generator::outputHeader() const noexcept
 	std::ofstream headerFile(_headerPath, std::ios_base::trunc);
 	IncludeGuardScope includeGuard { headerFile, fs::path(_headerPath).filename().string() };
 
-	headerFile << R"cpp(#include "graphqlservice/GraphQLParse.h"
+	headerFile << R"cpp(#include "graphqlservice/GraphQLClient.h"
+#include "graphqlservice/GraphQLParse.h"
 #include "graphqlservice/GraphQLResponse.h"
 
 #include "graphqlservice/internal/Version.h"
@@ -449,8 +450,6 @@ bool Generator::outputSource() const noexcept
 #include ")cpp" << _schemaLoader.getFilenamePrefix()
 			   << R"cpp(Client.h"
 
-#include "graphqlservice/GraphQLClient.h"
-
 #include <algorithm>
 #include <array>
 #include <stdexcept>
diff --git a/src/GraphQLClient.cpp b/src/GraphQLClient.cpp
@@ -3,8 +3,166 @@
 
 #include "graphqlservice/GraphQLClient.h"
 
+using namespace std::literals;
+
 namespace graphql::client {
 
+ErrorLocation parseServiceErrorLocation(response::Value&& location)
+{
+	ErrorLocation result;
+
+	if (location.type() == response::Type::Map)
+	{
+		auto members = location.release<response::MapType>();
+
+		for (auto& member : members)
+		{
+			if (member.first == "line"sv)
+			{
+				if (member.second.type() == response::Type::Int)
+				{
+					result.line = static_cast<size_t>(member.second.get<response::IntType>());
+				}
+
+				continue;
+			}
+
+			if (member.first == "column"sv)
+			{
+				if (member.second.type() == response::Type::Int)
+				{
+					result.column = static_cast<size_t>(member.second.get<response::IntType>());
+				}
+
+				continue;
+			}
+		}
+	}
+
+	return result;
+}
+
+ErrorPathSegment parseServiceErrorPathSegment(response::Value&& segment)
+{
+	ErrorPathSegment result { response::IntType {} };
+
+	switch (segment.type())
+	{
+		case response::Type::Int:
+			result = segment.get<response::IntType>();
+			break;
+
+		case response::Type::String:
+			result = segment.release<response::StringType>();
+			break;
+	}
+
+	return result;
+}
+
+Error parseServiceError(response::Value&& error)
+{
+	Error result;
+
+	if (error.type() == response::Type::Map)
+	{
+		auto members = error.release<response::MapType>();
+
+		for (auto& member : members)
+		{
+			if (member.first == "message"sv)
+			{
+				if (member.second.type() == response::Type::String)
+				{
+					result.message = member.second.release<response::StringType>();
+				}
+
+				continue;
+			}
+
+			if (member.first == "locations"sv)
+			{
+				if (member.second.type() == response::Type::List)
+				{
+					auto locations = member.second.release<response::ListType>();
+
+					result.locations.reserve(locations.size());
+					std::transform(locations.begin(),
+						locations.end(),
+						std::back_inserter(result.locations),
+						[](response::Value& location)
+					{
+						return parseServiceErrorLocation(std::move(location));
+					});
+				}
+
+				continue;
+			}
+
+			if (member.first == "path"sv)
+			{
+				if (member.second.type() == response::Type::List)
+				{
+					auto segments = member.second.release<response::ListType>();
+
+					result.path.reserve(segments.size());
+					std::transform(segments.begin(),
+						segments.end(),
+						std::back_inserter(result.path),
+						[](response::Value& segment)
+					{
+						return parseServiceErrorPathSegment(std::move(segment));
+					});
+				}
+
+				continue;
+			}
+		}
+	}
+
+	return result;
+}
+
+ServiceResponse parseServiceResponse(response::Value&& response)
+{
+	ServiceResponse result;
+
+	if (response.type() == response::Type::Map)
+	{
+		auto members = response.release<response::MapType>();
+
+		for (auto& member : members)
+		{
+			if (member.first == "data"sv)
+			{
+				// The generated client code can parse this.
+				result.data = std::move(member.second);
+				continue;
+			}
+
+			if (member.first == "errors"sv)
+			{
+				if (member.second.type() == response::Type::List)
+				{
+					auto errors = member.second.release<response::ListType>();
+
+					result.errors.reserve(errors.size());
+					std::transform(errors.begin(),
+						errors.end(),
+						std::back_inserter(result.errors),
+						[](response::Value& error) {
+							return parseServiceError(std::move(error));
+						});
+				}
+
+				continue;
+			}
+		}
+	}
+
+	return result;
+}
+
 template <>
 response::Value ModifiedVariable<response::IntType>::serialize(response::IntType&& value)
 {
diff --git a/test/ClientTests.cpp b/test/ClientTests.cpp
@@ -127,13 +127,10 @@ TEST_F(ClientCase, QueryEverything)
 	try
 	{
 		ASSERT_TRUE(result.type() == response::Type::Map);
-		auto errorsItr = result.find("errors");
-		if (errorsItr != result.get<response::MapType>().cend())
-		{
-			FAIL() << "service returned errors";
-		}
+		auto serviceResponse = client::parseServiceResponse(std::move(result));
+		const auto response = parseResponse(std::move(serviceResponse.data));
 
-		const auto response = parseResponse(service::ScalarArgument::require("data", result));
+		EXPECT_EQ(0, serviceResponse.errors.size()) << "no errors expected";
 
 		ASSERT_TRUE(response.appointments.edges.has_value()) << "appointments should be set";
 		ASSERT_EQ(1, response.appointments.edges->size()) << "appointments should have 1 entry";
@@ -209,13 +206,10 @@ TEST_F(ClientCase, MutateCompleteTask)
 	try
 	{
 		ASSERT_TRUE(result.type() == response::Type::Map);
-		auto errorsItr = result.find("errors");
-		if (errorsItr != result.get<response::MapType>().cend())
-		{
-			FAIL() << "service returned errors";
-		}
+		auto serviceResponse = client::parseServiceResponse(std::move(result));
+		const auto response = parseResponse(std::move(serviceResponse.data));
 
-		const auto response = parseResponse(service::ScalarArgument::require("data", result));
+		EXPECT_EQ(0, serviceResponse.errors.size()) << "no errors expected";
 
 		const auto& completedTask = response.completedTask;
 		const auto& task = completedTask.completedTask;
@@ -257,13 +251,10 @@ TEST_F(ClientCase, SubscribeNextAppointmentChangeDefault)
 	try
 	{
 		ASSERT_TRUE(result.type() == response::Type::Map);
-		auto errorsItr = result.find("errors");
-		if (errorsItr != result.get<response::MapType>().cend())
-		{
-			FAIL() << "service returned errors";
-		}
+		auto serviceResponse = client::parseServiceResponse(std::move(result));
+		const auto response = parseResponse(std::move(serviceResponse.data));
 
-		const auto response = parseResponse(service::ScalarArgument::require("data", result));
+		EXPECT_EQ(0, serviceResponse.errors.size()) << "no errors expected";
 
 		const auto& appointmentNode = response.nextAppointment;
 		ASSERT_TRUE(appointmentNode.has_value()) << "should get back a task";
