Skip to content

Commit d8edb04

Browse files
committed
Fix ValidationTests and possibleTypes logic
1 parent 3bea5f4 commit d8edb04

File tree

6 files changed

+85
-22
lines changed

6 files changed

+85
-22
lines changed

include/Validation.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,7 @@ class ValidateExecutableVisitor
250250
VariableSet _referencedVariables;
251251
FragmentSet _fragmentStack;
252252
size_t _fieldCount = 0;
253+
size_t _introspectionFieldCount = 0;
253254
TypeFields _typeFields;
254255
InputTypeFields _inputTypeFields;
255256
ValidateType _scopedType;

include/graphqlservice/internal/Schema.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ class InterfaceType : public BaseType
177177
GRAPHQLSERVICE_EXPORT static std::shared_ptr<InterfaceType> Make(
178178
std::string_view name, std::string_view description);
179179

180-
GRAPHQLSERVICE_EXPORT void AddPossibleType(std::weak_ptr<ObjectType> possibleType);
180+
GRAPHQLSERVICE_EXPORT void AddPossibleType(std::weak_ptr<BaseType> possibleType);
181181
GRAPHQLSERVICE_EXPORT void AddInterfaces(
182182
std::vector<std::shared_ptr<const InterfaceType>>&& interfaces);
183183
GRAPHQLSERVICE_EXPORT void AddFields(std::vector<std::shared_ptr<const Field>>&& fields);
@@ -188,6 +188,8 @@ class InterfaceType : public BaseType
188188
const noexcept final;
189189
GRAPHQLSERVICE_EXPORT const std::vector<std::weak_ptr<const BaseType>>& possibleTypes()
190190
const noexcept final;
191+
GRAPHQLSERVICE_EXPORT const std::vector<std::shared_ptr<const InterfaceType>>& interfaces()
192+
const noexcept final;
191193

192194
private:
193195
const std::string_view _name;

src/Introspection.cpp

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,12 @@
55

66
#include "graphqlservice/introspection/IntrospectionSchema.h"
77

8-
#include "graphqlservice/introspection/SchemaObject.h"
9-
#include "graphqlservice/introspection/TypeObject.h"
8+
#include "graphqlservice/introspection/DirectiveObject.h"
9+
#include "graphqlservice/introspection/EnumValueObject.h"
1010
#include "graphqlservice/introspection/FieldObject.h"
1111
#include "graphqlservice/introspection/InputValueObject.h"
12-
#include "graphqlservice/introspection/EnumValueObject.h"
13-
#include "graphqlservice/introspection/DirectiveObject.h"
12+
#include "graphqlservice/introspection/SchemaObject.h"
13+
#include "graphqlservice/introspection/TypeObject.h"
1414

1515
namespace graphql::introspection {
1616

@@ -168,9 +168,20 @@ std::optional<std::vector<std::shared_ptr<object::Type>>> Type::getPossibleTypes
168168
possibleTypes.end(),
169169
result->begin(),
170170
[](const auto& entry) {
171-
return std::make_shared<object::Type>(std::make_shared<Type>(entry.lock()));
171+
auto typeEntry = entry.lock();
172+
173+
return typeEntry && typeEntry->kind() == introspection::TypeKind::OBJECT
174+
? std::make_shared<object::Type>(std::make_shared<Type>(std::move(typeEntry)))
175+
: std::shared_ptr<object::Type> {};
172176
});
173177

178+
result->erase(std::remove_if(result->begin(),
179+
result->end(),
180+
[](const auto& entry) noexcept {
181+
return entry != nullptr;
182+
}),
183+
result->cend());
184+
174185
return result;
175186
}
176187

@@ -246,7 +257,8 @@ std::optional<std::string> Type::getSpecifiedByURL() const
246257
{
247258
const auto specifiedByURL = _type->specifiedByURL();
248259

249-
return { specifiedByURL.empty() ? std::nullopt : std::make_optional<std::string>(specifiedByURL) };
260+
return { specifiedByURL.empty() ? std::nullopt
261+
: std::make_optional<std::string>(specifiedByURL) };
250262
}
251263

252264
Field::Field(const std::shared_ptr<const schema::Field>& field)

src/Schema.cpp

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -235,8 +235,7 @@ void ObjectType::AddInterfaces(std::vector<std::shared_ptr<const InterfaceType>>
235235

236236
for (const auto& interface : _interfaces)
237237
{
238-
std::const_pointer_cast<InterfaceType>(interface)->AddPossibleType(
239-
std::static_pointer_cast<ObjectType>(shared_from_this()));
238+
std::const_pointer_cast<InterfaceType>(interface)->AddPossibleType(shared_from_this());
240239
}
241240
}
242241

@@ -278,14 +277,19 @@ InterfaceType::InterfaceType(init&& params)
278277
{
279278
}
280279

281-
void InterfaceType::AddPossibleType(std::weak_ptr<ObjectType> possibleType)
280+
void InterfaceType::AddPossibleType(std::weak_ptr<BaseType> possibleType)
282281
{
283282
_possibleTypes.push_back(possibleType);
284283
}
285284

286285
void InterfaceType::AddInterfaces(std::vector<std::shared_ptr<const InterfaceType>>&& interfaces)
287286
{
288287
_interfaces = std::move(interfaces);
288+
289+
for (const auto& interface : _interfaces)
290+
{
291+
std::const_pointer_cast<InterfaceType>(interface)->AddPossibleType(shared_from_this());
292+
}
289293
}
290294

291295
void InterfaceType::AddFields(std::vector<std::shared_ptr<const Field>>&& fields)
@@ -308,6 +312,11 @@ const std::vector<std::weak_ptr<const BaseType>>& InterfaceType::possibleTypes()
308312
return _possibleTypes;
309313
}
310314

315+
const std::vector<std::shared_ptr<const InterfaceType>>& InterfaceType::interfaces() const noexcept
316+
{
317+
return _interfaces;
318+
}
319+
311320
struct UnionType::init
312321
{
313322
std::string_view name;

src/Validation.cpp

Lines changed: 50 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -403,7 +403,15 @@ ValidateExecutableVisitor::ValidateExecutableVisitor(std::shared_ptr<schema::Sch
403403
{
404404
const auto& possibleTypes = entry.second->possibleTypes();
405405

406-
matchingTypes.reserve(possibleTypes.size());
406+
if (kind == introspection::TypeKind::INTERFACE)
407+
{
408+
matchingTypes.reserve(possibleTypes.size() + 1);
409+
matchingTypes.emplace(name);
410+
}
411+
else
412+
{
413+
matchingTypes.reserve(possibleTypes.size());
414+
}
407415

408416
for (const auto& possibleType : possibleTypes)
409417
{
@@ -773,26 +781,46 @@ void ValidateExecutableVisitor::visitOperationDefinition(const peg::ast_node& op
773781
}
774782

775783
_scopedType = itrType->second;
784+
_introspectionFieldCount = 0;
776785
_fieldCount = 0;
777786

778787
const auto& selection = *operationDefinition.children.back();
779788

780789
visitSelection(selection);
781790

782-
if (_fieldCount > 1 && operationType == strSubscription)
791+
if (operationType == strSubscription)
783792
{
784-
// https://spec.graphql.org/October2021/#sec-Single-root-field
785-
auto position = operationDefinition.begin();
786-
std::ostringstream error;
793+
if (_fieldCount > 1)
794+
{
795+
// https://spec.graphql.org/October2021/#sec-Single-root-field
796+
auto position = operationDefinition.begin();
797+
std::ostringstream error;
787798

788-
error << "Subscription with more than one root field";
799+
error << "Subscription with more than one root field";
789800

790-
if (!operationName.empty())
791-
{
792-
error << " name: " << operationName;
801+
if (!operationName.empty())
802+
{
803+
error << " name: " << operationName;
804+
}
805+
806+
_errors.push_back({ error.str(), { position.line, position.column } });
793807
}
794808

795-
_errors.push_back({ error.str(), { position.line, position.column } });
809+
if (_introspectionFieldCount != 0)
810+
{
811+
// https://spec.graphql.org/October2021/#sec-Single-root-field
812+
auto position = operationDefinition.begin();
813+
std::ostringstream error;
814+
815+
error << "Subscription with Introspection root field";
816+
817+
if (!operationName.empty())
818+
{
819+
error << " name: " << operationName;
820+
}
821+
822+
_errors.push_back({ error.str(), { position.line, position.column } });
823+
}
796824
}
797825

798826
_scopedType.reset();
@@ -1622,7 +1650,7 @@ void ValidateExecutableVisitor::visitField(const peg::ast_node& field)
16221650
std::list<std::string_view> argumentNames;
16231651

16241652
peg::on_first_child<peg::arguments>(field,
1625-
[this, &name, &validateArguments, &argumentLocations, &argumentNames](
1653+
[this, name, &validateArguments, &argumentLocations, &argumentNames](
16261654
const peg::ast_node& child) {
16271655
for (auto& argument : child.children)
16281656
{
@@ -1760,8 +1788,10 @@ void ValidateExecutableVisitor::visitField(const peg::ast_node& field)
17601788
auto outerType = std::move(_scopedType);
17611789
auto outerFields = std::move(_selectionFields);
17621790
auto outerFieldCount = _fieldCount;
1791+
auto outerIntrospectionFieldCount = _introspectionFieldCount;
17631792

17641793
_fieldCount = 0;
1794+
_introspectionFieldCount = 0;
17651795
_selectionFields.clear();
17661796
_scopedType = std::move(innerType);
17671797

@@ -1771,6 +1801,7 @@ void ValidateExecutableVisitor::visitField(const peg::ast_node& field)
17711801
_scopedType = std::move(outerType);
17721802
_selectionFields = std::move(outerFields);
17731803
subFieldCount = _fieldCount;
1804+
_introspectionFieldCount = outerIntrospectionFieldCount;
17741805
_fieldCount = outerFieldCount;
17751806
}
17761807

@@ -1787,6 +1818,14 @@ void ValidateExecutableVisitor::visitField(const peg::ast_node& field)
17871818
}
17881819

17891820
++_fieldCount;
1821+
1822+
constexpr auto c_introspectionFieldPrefix = R"gql(__)gql"sv;
1823+
1824+
if (name.size() >= c_introspectionFieldPrefix.size()
1825+
&& name.substr(0, c_introspectionFieldPrefix.size()) == c_introspectionFieldPrefix)
1826+
{
1827+
++_introspectionFieldCount;
1828+
}
17901829
}
17911830

17921831
void ValidateExecutableVisitor::visitFragmentSpread(const peg::ast_node& fragmentSpread)

test/ValidationTests.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,7 @@ TEST_F(ValidationExamplesCase, CounterExample112)
268268

269269
ASSERT_EQ(errors.size(), 1);
270270
EXPECT_EQ(
271-
R"js({"message":"Subscription with more than one root field name: sub","locations":[{"line":1,"column":1}]})js",
271+
R"js({"message":"Subscription with Introspection root field name: sub","locations":[{"line":1,"column":1}]})js",
272272
response::toJSON(std::move(errors[0])))
273273
<< "error should match";
274274
}

0 commit comments

Comments
 (0)