Skip to content

Commit fe822e3

Browse files
committed
fix(validate): no reusing root types
Replicates graphql/graphql-js@c008d0f
1 parent 1ed451a commit fe822e3

File tree

6 files changed

+182
-25
lines changed

6 files changed

+182
-25
lines changed

src/graphql/pyutils/__init__.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
unregister_description,
1818
)
1919
from .did_you_mean import did_you_mean
20+
from .format_list import or_list, and_list
2021
from .group_by import group_by
2122
from .identity_func import identity_func
2223
from .inspect import inspect
@@ -37,6 +38,8 @@
3738
"snake_to_camel",
3839
"cached_property",
3940
"did_you_mean",
41+
"or_list",
42+
"and_list",
4043
"Description",
4144
"group_by",
4245
"is_description",

src/graphql/pyutils/did_you_mean.py

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
from typing import Optional, Sequence
22

3+
from .format_list import or_list
4+
35

46
__all__ = ["did_you_mean"]
57

@@ -10,20 +12,11 @@ def did_you_mean(suggestions: Sequence[str], sub_message: Optional[str] = None)
1012
"""Given [ A, B, C ] return ' Did you mean A, B, or C?'"""
1113
if not suggestions or not MAX_LENGTH:
1214
return ""
13-
parts = [" Did you mean "]
15+
message = " Did you mean "
1416
if sub_message:
15-
parts.extend([sub_message, " "])
17+
message += sub_message + " "
1618
suggestions = suggestions[:MAX_LENGTH]
17-
n = len(suggestions)
18-
if n == 1:
19-
parts.append(f"'{suggestions[0]}'?")
20-
elif n == 2:
21-
parts.append(f"'{suggestions[0]}' or '{suggestions[1]}'?")
22-
else:
23-
parts.extend(
24-
[
25-
", ".join(f"'{s}'" for s in suggestions[:-1]),
26-
f", or '{suggestions[-1]}'?",
27-
]
28-
)
29-
return "".join(parts)
19+
suggestion_list = or_list(
20+
[f"'{suggestion}'" for suggestion in suggestions[:MAX_LENGTH]]
21+
)
22+
return message + suggestion_list + "?"

src/graphql/pyutils/format_list.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
from typing import Sequence
2+
3+
4+
__all__ = ["or_list", "and_list"]
5+
6+
7+
def or_list(items: Sequence[str]) -> str:
8+
"""Given [ A, B, C ] return 'A, B, or C'."""
9+
return format_list("or", items)
10+
11+
12+
def and_list(items: Sequence[str]) -> str:
13+
"""Given [ A, B, C ] return 'A, B, and C'."""
14+
return format_list("and", items)
15+
16+
17+
def format_list(conjunction: str, items: Sequence[str]) -> str:
18+
"""Given [ A, B, C ] return 'A, B, (conjunction) C'"""
19+
if not items:
20+
raise ValueError("Missing list items to be formatted.")
21+
22+
n = len(items)
23+
if n == 1:
24+
return items[0]
25+
if n == 2:
26+
return f"{items[0]} {conjunction} {items[1]}"
27+
28+
*all_but_last, last_item = items
29+
return f"{', '.join(all_but_last)}, {conjunction} {last_item}"

src/graphql/type/validate.py

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
from collections import defaultdict
12
from operator import attrgetter, itemgetter
23
from typing import Any, Collection, Dict, List, Optional, Set, Tuple, Union, cast
34

@@ -11,7 +12,7 @@
1112
SchemaDefinitionNode,
1213
SchemaExtensionNode,
1314
)
14-
from ..pyutils import inspect
15+
from ..pyutils import and_list, inspect
1516
from ..utilities.type_comparators import is_equal_type, is_type_sub_type_of
1617
from .definition import (
1718
GraphQLEnumType,
@@ -105,19 +106,37 @@ def validate_root_types(self) -> None:
105106
schema = self.schema
106107
if not schema.query_type:
107108
self.report_error("Query root type must be provided.", schema.ast_node)
109+
root_types_map: Dict[GraphQLObjectType, List[OperationType]] = defaultdict(list)
110+
108111
for operation_type in OperationType:
109112
root_type = schema.get_root_type(operation_type)
110-
if root_type and not is_object_type(root_type):
111-
operation_type_str = operation_type.value.capitalize()
112-
root_type_str = inspect(root_type)
113-
if_provided_str = (
114-
"" if operation_type == operation_type.QUERY else " if provided"
113+
if root_type:
114+
if is_object_type(root_type):
115+
root_types_map[root_type].append(operation_type)
116+
else:
117+
operation_type_str = operation_type.value.capitalize()
118+
root_type_str = inspect(root_type)
119+
if_provided_str = (
120+
"" if operation_type == operation_type.QUERY else " if provided"
121+
)
122+
self.report_error(
123+
f"{operation_type_str} root type must be Object type"
124+
f"{if_provided_str}, it cannot be {root_type_str}.",
125+
get_operation_type_node(schema, operation_type)
126+
or root_type.ast_node,
127+
)
128+
for root_type, operation_types in root_types_map.items():
129+
if len(operation_types) > 1:
130+
operation_list = and_list(
131+
[operation_type.value for operation_type in operation_types]
115132
)
116133
self.report_error(
117-
f"{operation_type_str} root type must be Object type"
118-
f"{if_provided_str}, it cannot be {root_type_str}.",
119-
get_operation_type_node(schema, operation_type)
120-
or root_type.ast_node,
134+
"All root types must be different,"
135+
f" '{root_type.name}' type is used as {operation_list} root types.",
136+
[
137+
get_operation_type_node(schema, operation_type)
138+
for operation_type in operation_types
139+
],
121140
)
122141

123142
def validate_directives(self) -> None:

tests/pyutils/test_format_list.py

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
from pytest import raises
2+
3+
from graphql.pyutils import and_list, or_list
4+
5+
6+
def describe_and_list():
7+
def does_not_accept_an_empty_list():
8+
with raises(ValueError):
9+
and_list([])
10+
11+
def handles_single_item():
12+
assert and_list(["A"]) == "A"
13+
14+
def handles_two_items():
15+
assert and_list(["A", "B"]) == "A and B"
16+
17+
def handles_three_items():
18+
assert and_list(["A", "B", "C"]) == "A, B, and C"
19+
20+
def handles_more_than_five_items():
21+
assert and_list(["A", "B", "C", "D", "E", "F"]) == "A, B, C, D, E, and F"
22+
23+
24+
def describe_or_list():
25+
def does_not_accept_an_empty_list():
26+
with raises(ValueError):
27+
or_list([])
28+
29+
def handles_single_item():
30+
assert or_list(["A"]) == "A"
31+
32+
def handles_two_items():
33+
assert or_list(["A", "B"]) == "A or B"
34+
35+
def handles_three_items():
36+
assert or_list(["A", "B", "C"]) == "A, B, or C"
37+
38+
def handles_more_than_five_items():
39+
assert or_list(["A", "B", "C", "D", "E", "F"]) == "A, B, C, D, E, or F"

tests/type/test_validation.py

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -454,6 +454,80 @@ def rejects_a_schema_whose_directives_are_incorrectly_typed():
454454
]
455455

456456

457+
def describe_type_system_root_types_must_all_be_different_if_provided():
458+
def accepts_a_schema_with_different_root_types():
459+
schema = build_schema(
460+
"""
461+
type SomeObject1 {
462+
field: String
463+
}
464+
465+
type SomeObject2 {
466+
field: String
467+
}
468+
469+
type SomeObject3 {
470+
field: String
471+
}
472+
473+
schema {
474+
query: SomeObject1
475+
mutation: SomeObject2
476+
subscription: SomeObject3
477+
}
478+
"""
479+
)
480+
assert validate_schema(schema) == []
481+
482+
def rejects_a_schema_where_the_same_type_is_used_for_multiple_root_types():
483+
schema = build_schema(
484+
"""
485+
type SomeObject {
486+
field: String
487+
}
488+
489+
type UniqueObject {
490+
field: String
491+
}
492+
493+
schema {
494+
query: SomeObject
495+
mutation: UniqueObject
496+
subscription: SomeObject
497+
}
498+
"""
499+
)
500+
assert validate_schema(schema) == [
501+
{
502+
"message": "All root types must be different, 'SomeObject' type"
503+
" is used as query and subscription root types.",
504+
"locations": [(11, 22), (13, 29)],
505+
}
506+
]
507+
508+
def rejects_a_schema_where_the_same_type_is_used_for_all_root_types():
509+
schema = build_schema(
510+
"""
511+
type SomeObject {
512+
field: String
513+
}
514+
515+
schema {
516+
query: SomeObject
517+
mutation: SomeObject
518+
subscription: SomeObject
519+
}
520+
"""
521+
)
522+
assert validate_schema(schema) == [
523+
{
524+
"message": "All root types must be different, 'SomeObject' type"
525+
" is used as query, mutation, and subscription root types.",
526+
"locations": [(7, 22), (8, 25), (9, 29)],
527+
}
528+
]
529+
530+
457531
def describe_type_system_objects_must_have_fields():
458532
def accepts_an_object_type_with_fields_object():
459533
schema = build_schema(

0 commit comments

Comments
 (0)