Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix performance degradation on handling conflict fields #212

Merged
merged 5 commits into from
Feb 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 31 additions & 12 deletions src/graphql/validation/rules/overlapping_fields_can_be_merged.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,8 @@
FragmentDefinitionNode,
FragmentSpreadNode,
InlineFragmentNode,
ObjectFieldNode,
ObjectValueNode,
SelectionSetNode,
ValueNode,
print_ast,
)
from ...type import (
Expand Down Expand Up @@ -557,7 +556,7 @@ def find_conflict(
)

# Two field calls must have the same arguments.
if stringify_arguments(node1) != stringify_arguments(node2):
if not same_arguments(node1, node2):
return (response_name, "they have differing arguments"), [node1], [node2]

directives1 = node1.directives
Expand Down Expand Up @@ -597,14 +596,34 @@ def find_conflict(
return None # no conflict


def stringify_arguments(field_node: Union[FieldNode, DirectiveNode]) -> str:
input_object_with_args = ObjectValueNode(
fields=tuple(
ObjectFieldNode(name=arg_node.name, value=arg_node.value)
for arg_node in field_node.arguments
)
)
return print_ast(sort_value_node(input_object_with_args))
def same_arguments(
node1: Union[FieldNode, DirectiveNode], node2: Union[FieldNode, DirectiveNode]
) -> bool:
args1 = node1.arguments
args2 = node2.arguments

if args1 is None or len(args1) == 0:
return args2 is None or len(args2) == 0

if args2 is None or len(args2) == 0:
return False

if len(args1) != len(args2):
return False

values2 = {arg.name.value: arg.value for arg in args2}

for arg1 in args1:
value1 = arg1.value
value2 = values2.get(arg1.name.value)
if value2 is None or stringify_value(value1) != stringify_value(value2):
return False

return True


def stringify_value(value: ValueNode) -> str:
return print_ast(sort_value_node(value))


def get_stream_directive(
Expand All @@ -626,7 +645,7 @@ def same_streams(
return True
if stream1 and stream2:
# check if both fields have equivalent streams
return stringify_arguments(stream1) == stringify_arguments(stream2)
return same_arguments(stream1, stream2)
# fields have a mix of stream and no stream
return False

Expand Down
27 changes: 27 additions & 0 deletions tests/benchmarks/test_repeated_fields_benchmark.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
from graphql import (
GraphQLField,
GraphQLObjectType,
GraphQLSchema,
GraphQLString,
graphql_sync,
)


schema = GraphQLSchema(
query=GraphQLObjectType(
name="Query",
fields={
"hello": GraphQLField(
GraphQLString,
resolve=lambda obj, info: "world",
)
},
)
)
source = "query {{ {fields} }}".format(fields="hello " * 250)


def test_many_repeated_fields(benchmark):
print(source)
result = benchmark(lambda: graphql_sync(schema, source))
assert not result.errors
17 changes: 17 additions & 0 deletions tests/validation/test_overlapping_fields_can_be_merged.py
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,23 @@ def different_stream_directive_second_missing_args():
],
)

def different_stream_directive_extra_argument():
assert_errors(
"""
fragment conflictingArgs on Dog {
name @stream(label: "streamLabel", initialCount: 1)
name @stream(label: "streamLabel", initialCount: 1, extraArg: true)
}""",
[
{
"message": "Fields 'name' conflict because they have differing"
" stream directives. Use different aliases on the fields"
" to fetch both if this was intentional.",
"locations": [(3, 15), (4, 15)],
}
],
)

def mix_of_stream_and_no_stream():
assert_errors(
"""
Expand Down
Loading