Skip to content

Commit cef14e1

Browse files
wraverywravery
committed
Simplify splitting service response into data and errors
1 parent 063dd54 commit cef14e1

13 files changed

+211
-32
lines changed

‎include/graphqlservice/GraphQLClient.h

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,36 @@
3131

3232
namespace graphql::client {
3333

34+
// Errors may specify the line number and column number where the error occurred.
35+
struct ErrorLocation
36+
{
37+
response::IntType line {};
38+
response::IntType column {};
39+
};
40+
41+
// Errors may specify a path to the field which triggered the error. The path consists of
42+
// field names and the indices of elements in a list.
43+
using ErrorPathSegment = std::variant<response::StringType, response::IntType>;
44+
45+
// Error returned from the service.
46+
struct Error
47+
{
48+
std::string message;
49+
std::vector<ErrorLocation> locations;
50+
std::vector<ErrorPathSegment> path;
51+
};
52+
53+
// Complete response from the service, split into the unparsed graphql::response::Value in
54+
// data and the (typically empty) collection of Errors in errors.
55+
struct ServiceResponse
56+
{
57+
response::Value data;
58+
std::vector<Error> errors;
59+
};
60+
61+
// Split a service response into separate ServiceResponse data and errors members.
62+
GRAPHQLCLIENT_EXPORT ServiceResponse parseServiceResponse(response::Value&& response);
63+
3464
// GraphQL types are nullable by default, but they may be wrapped with non-null or list types.
3565
// Since nullability is a more special case in C++, we invert the default and apply that modifier
3666
// instead when the non-null wrapper is not present in that part of the wrapper chain.

‎samples/client/BenchmarkClient.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@
55

66
#include "BenchmarkClient.h"
77

8-
#include "graphqlservice/GraphQLClient.h"
9-
108
#include <algorithm>
119
#include <array>
1210
#include <stdexcept>

‎samples/client/BenchmarkClient.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#ifndef BENCHMARKCLIENT_H
99
#define BENCHMARKCLIENT_H
1010

11+
#include "graphqlservice/GraphQLClient.h"
1112
#include "graphqlservice/GraphQLParse.h"
1213
#include "graphqlservice/GraphQLResponse.h"
1314

‎samples/client/MutateClient.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@
55

66
#include "MutateClient.h"
77

8-
#include "graphqlservice/GraphQLClient.h"
9-
108
#include <algorithm>
119
#include <array>
1210
#include <stdexcept>

‎samples/client/MutateClient.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#ifndef MUTATECLIENT_H
99
#define MUTATECLIENT_H
1010

11+
#include "graphqlservice/GraphQLClient.h"
1112
#include "graphqlservice/GraphQLParse.h"
1213
#include "graphqlservice/GraphQLResponse.h"
1314

‎samples/client/QueryClient.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@
55

66
#include "QueryClient.h"
77

8-
#include "graphqlservice/GraphQLClient.h"
9-
108
#include <algorithm>
119
#include <array>
1210
#include <stdexcept>

‎samples/client/QueryClient.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#ifndef QUERYCLIENT_H
99
#define QUERYCLIENT_H
1010

11+
#include "graphqlservice/GraphQLClient.h"
1112
#include "graphqlservice/GraphQLParse.h"
1213
#include "graphqlservice/GraphQLResponse.h"
1314

‎samples/client/SubscribeClient.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@
55

66
#include "SubscribeClient.h"
77

8-
#include "graphqlservice/GraphQLClient.h"
9-
108
#include <algorithm>
119
#include <array>
1210
#include <stdexcept>

‎samples/client/SubscribeClient.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#ifndef SUBSCRIBECLIENT_H
99
#define SUBSCRIBECLIENT_H
1010

11+
#include "graphqlservice/GraphQLClient.h"
1112
#include "graphqlservice/GraphQLParse.h"
1213
#include "graphqlservice/GraphQLResponse.h"
1314

‎samples/client/benchmark.cpp

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved.
22
// Licensed under the MIT License.
33

4-
#include "TodayMock.h"
54
#include "BenchmarkClient.h"
5+
#include "TodayMock.h"
66

77
#include <chrono>
88
#include <iostream>
@@ -128,6 +128,7 @@ int main(int argc, char** argv)
128128

129129
auto service = buildService();
130130
std::vector<std::chrono::steady_clock::duration> durationResolve(iterations);
131+
std::vector<std::chrono::steady_clock::duration> durationParseServiceResponse(iterations);
131132
std::vector<std::chrono::steady_clock::duration> durationParseResponse(iterations);
132133
const auto startTime = std::chrono::steady_clock::now();
133134

@@ -142,11 +143,14 @@ int main(int argc, char** argv)
142143
const auto startResolve = std::chrono::steady_clock::now();
143144
auto response =
144145
service->resolve(nullptr, query, "", response::Value(response::Type::Map)).get();
146+
const auto startParseServiceResponse = std::chrono::steady_clock::now();
147+
auto serviceResponse = client::parseServiceResponse(std::move(response));
145148
const auto startParseResponse = std::chrono::steady_clock::now();
146-
const auto parsed = parseResponse(std::move(response));
149+
const auto parsed = parseResponse(std::move(serviceResponse.data));
147150
const auto endParseResponse = std::chrono::steady_clock::now();
148151

149-
durationResolve[i] = startParseResponse - startResolve;
152+
durationResolve[i] = startParseServiceResponse - startResolve;
153+
durationParseServiceResponse[i] = startParseResponse - startParseServiceResponse;
150154
durationParseResponse[i] = endParseResponse - startParseResponse;
151155
}
152156
}
@@ -162,6 +166,7 @@ int main(int argc, char** argv)
162166
outputOverview(iterations, totalDuration);
163167

164168
outputSegment("Resolve"sv, durationResolve);
169+
outputSegment("ParseServiceResponse"sv, durationParseServiceResponse);
165170
outputSegment("ParseResponse"sv, durationParseResponse);
166171

167172
return 0;

‎src/ClientGenerator.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,8 @@ bool Generator::outputHeader() const noexcept
190190
std::ofstream headerFile(_headerPath, std::ios_base::trunc);
191191
IncludeGuardScope includeGuard { headerFile, fs::path(_headerPath).filename().string() };
192192

193-
headerFile << R"cpp(#include "graphqlservice/GraphQLParse.h"
193+
headerFile << R"cpp(#include "graphqlservice/GraphQLClient.h"
194+
#include "graphqlservice/GraphQLParse.h"
194195
#include "graphqlservice/GraphQLResponse.h"
195196
196197
#include "graphqlservice/internal/Version.h"
@@ -449,8 +450,6 @@ bool Generator::outputSource() const noexcept
449450
#include ")cpp" << _schemaLoader.getFilenamePrefix()
450451
<< R"cpp(Client.h"
451452
452-
#include "graphqlservice/GraphQLClient.h"
453-
454453
#include <algorithm>
455454
#include <array>
456455
#include <stdexcept>

‎src/GraphQLClient.cpp

Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,166 @@
33

44
#include "graphqlservice/GraphQLClient.h"
55

6+
using namespace std::literals;
7+
68
namespace graphql::client {
79

10+
ErrorLocation parseServiceErrorLocation(response::Value&& location)
11+
{
12+
ErrorLocation result;
13+
14+
if (location.type() == response::Type::Map)
15+
{
16+
auto members = location.release<response::MapType>();
17+
18+
for (auto& member : members)
19+
{
20+
if (member.first == "line"sv)
21+
{
22+
if (member.second.type() == response::Type::Int)
23+
{
24+
result.line = static_cast<size_t>(member.second.get<response::IntType>());
25+
}
26+
27+
continue;
28+
}
29+
30+
if (member.first == "column"sv)
31+
{
32+
if (member.second.type() == response::Type::Int)
33+
{
34+
result.column = static_cast<size_t>(member.second.get<response::IntType>());
35+
}
36+
37+
continue;
38+
}
39+
}
40+
}
41+
42+
return result;
43+
}
44+
45+
ErrorPathSegment parseServiceErrorPathSegment(response::Value&& segment)
46+
{
47+
ErrorPathSegment result { response::IntType {} };
48+
49+
switch (segment.type())
50+
{
51+
case response::Type::Int:
52+
result = segment.get<response::IntType>();
53+
break;
54+
55+
case response::Type::String:
56+
result = segment.release<response::StringType>();
57+
break;
58+
}
59+
60+
return result;
61+
}
62+
63+
Error parseServiceError(response::Value&& error)
64+
{
65+
Error result;
66+
67+
if (error.type() == response::Type::Map)
68+
{
69+
auto members = error.release<response::MapType>();
70+
71+
for (auto& member : members)
72+
{
73+
if (member.first == "message"sv)
74+
{
75+
if (member.second.type() == response::Type::String)
76+
{
77+
result.message = member.second.release<response::StringType>();
78+
}
79+
80+
continue;
81+
}
82+
83+
if (member.first == "locations"sv)
84+
{
85+
if (member.second.type() == response::Type::List)
86+
{
87+
auto locations = member.second.release<response::ListType>();
88+
89+
result.locations.reserve(locations.size());
90+
std::transform(locations.begin(),
91+
locations.end(),
92+
std::back_inserter(result.locations),
93+
[](response::Value& location)
94+
{
95+
return parseServiceErrorLocation(std::move(location));
96+
});
97+
}
98+
99+
continue;
100+
}
101+
102+
if (member.first == "path"sv)
103+
{
104+
if (member.second.type() == response::Type::List)
105+
{
106+
auto segments = member.second.release<response::ListType>();
107+
108+
result.path.reserve(segments.size());
109+
std::transform(segments.begin(),
110+
segments.end(),
111+
std::back_inserter(result.path),
112+
[](response::Value& segment)
113+
{
114+
return parseServiceErrorPathSegment(std::move(segment));
115+
});
116+
}
117+
118+
continue;
119+
}
120+
}
121+
}
122+
123+
return result;
124+
}
125+
126+
ServiceResponse parseServiceResponse(response::Value&& response)
127+
{
128+
ServiceResponse result;
129+
130+
if (response.type() == response::Type::Map)
131+
{
132+
auto members = response.release<response::MapType>();
133+
134+
for (auto& member : members)
135+
{
136+
if (member.first == "data"sv)
137+
{
138+
// The generated client code can parse this.
139+
result.data = std::move(member.second);
140+
continue;
141+
}
142+
143+
if (member.first == "errors"sv)
144+
{
145+
if (member.second.type() == response::Type::List)
146+
{
147+
auto errors = member.second.release<response::ListType>();
148+
149+
result.errors.reserve(errors.size());
150+
std::transform(errors.begin(),
151+
errors.end(),
152+
std::back_inserter(result.errors),
153+
[](response::Value& error) {
154+
return parseServiceError(std::move(error));
155+
});
156+
}
157+
158+
continue;
159+
}
160+
}
161+
}
162+
163+
return result;
164+
}
165+
8166
template <>
9167
response::Value ModifiedVariable<response::IntType>::serialize(response::IntType&& value)
10168
{

‎test/ClientTests.cpp

Lines changed: 9 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -127,13 +127,10 @@ TEST_F(ClientCase, QueryEverything)
127127
try
128128
{
129129
ASSERT_TRUE(result.type() == response::Type::Map);
130-
auto errorsItr = result.find("errors");
131-
if (errorsItr != result.get<response::MapType>().cend())
132-
{
133-
FAIL() << "service returned errors";
134-
}
130+
auto serviceResponse = client::parseServiceResponse(std::move(result));
131+
const auto response = parseResponse(std::move(serviceResponse.data));
135132

136-
const auto response = parseResponse(service::ScalarArgument::require("data", result));
133+
EXPECT_EQ(0, serviceResponse.errors.size()) << "no errors expected";
137134

138135
ASSERT_TRUE(response.appointments.edges.has_value()) << "appointments should be set";
139136
ASSERT_EQ(1, response.appointments.edges->size()) << "appointments should have 1 entry";
@@ -209,13 +206,10 @@ TEST_F(ClientCase, MutateCompleteTask)
209206
try
210207
{
211208
ASSERT_TRUE(result.type() == response::Type::Map);
212-
auto errorsItr = result.find("errors");
213-
if (errorsItr != result.get<response::MapType>().cend())
214-
{
215-
FAIL() << "service returned errors";
216-
}
209+
auto serviceResponse = client::parseServiceResponse(std::move(result));
210+
const auto response = parseResponse(std::move(serviceResponse.data));
217211

218-
const auto response = parseResponse(service::ScalarArgument::require("data", result));
212+
EXPECT_EQ(0, serviceResponse.errors.size()) << "no errors expected";
219213

220214
const auto& completedTask = response.completedTask;
221215
const auto& task = completedTask.completedTask;
@@ -257,13 +251,10 @@ TEST_F(ClientCase, SubscribeNextAppointmentChangeDefault)
257251
try
258252
{
259253
ASSERT_TRUE(result.type() == response::Type::Map);
260-
auto errorsItr = result.find("errors");
261-
if (errorsItr != result.get<response::MapType>().cend())
262-
{
263-
FAIL() << "service returned errors";
264-
}
254+
auto serviceResponse = client::parseServiceResponse(std::move(result));
255+
const auto response = parseResponse(std::move(serviceResponse.data));
265256

266-
const auto response = parseResponse(service::ScalarArgument::require("data", result));
257+
EXPECT_EQ(0, serviceResponse.errors.size()) << "no errors expected";
267258

268259
const auto& appointmentNode = response.nextAppointment;
269260
ASSERT_TRUE(appointmentNode.has_value()) << "should get back a task";

0 commit comments

Comments
 (0)