From 2637791568df844836cb7c0ebed92cc98ab4e10f Mon Sep 17 00:00:00 2001 From: Leszek Hanusz Date: Sun, 19 Nov 2023 22:32:12 +0100 Subject: [PATCH 1/8] Adding node_tree method in utilities --- gql/utilities/__init__.py | 2 + gql/utilities/node_tree.py | 80 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+) create mode 100644 gql/utilities/node_tree.py diff --git a/gql/utilities/__init__.py b/gql/utilities/__init__.py index 3d29dfe3..302c226a 100644 --- a/gql/utilities/__init__.py +++ b/gql/utilities/__init__.py @@ -1,5 +1,6 @@ from .build_client_schema import build_client_schema from .get_introspection_query_ast import get_introspection_query_ast +from .node_tree import node_tree from .parse_result import parse_result from .serialize_variable_values import serialize_value, serialize_variable_values from .update_schema_enum import update_schema_enum @@ -7,6 +8,7 @@ __all__ = [ "build_client_schema", + "node_tree", "parse_result", "get_introspection_query_ast", "serialize_variable_values", diff --git a/gql/utilities/node_tree.py b/gql/utilities/node_tree.py new file mode 100644 index 00000000..c5346faa --- /dev/null +++ b/gql/utilities/node_tree.py @@ -0,0 +1,80 @@ +from typing import Any, Iterable, List, Optional, Sized + +from graphql import Node + + +def _node_tree_recursive( + obj: Any, + *, + indent=0, + ignored_keys=None, +): + + if ignored_keys is None: + ignored_keys = [] + + results = [] + + if hasattr(obj, "__slots__"): + + results.append(" " * indent + f"{type(obj).__name__}") + + try: + keys = obj.keys + except AttributeError: + # If the object has no keys attribute, print its repr and return. + results.append(" " * (indent + 1) + repr(obj)) + else: + for key in keys: + if key in ignored_keys: + continue + attr_value = getattr(obj, key, None) + results.append(" " * (indent + 1) + f"{key}:") + if isinstance(attr_value, Iterable) and not isinstance( + attr_value, (str, bytes) + ): + if isinstance(attr_value, Sized) and len(attr_value) == 0: + results.append( + " " * (indent + 2) + f"empty {type(attr_value).__name__}" + ) + else: + for item in attr_value: + results.append( + _node_tree_recursive( + item, + indent=indent + 2, + ignored_keys=ignored_keys, + ) + ) + else: + results.append( + _node_tree_recursive( + attr_value, + indent=indent + 2, + ignored_keys=ignored_keys, + ) + ) + else: + results.append(" " * indent + repr(obj)) + + return "\n".join(results) + + +def node_tree( + obj: Node, *, ignore_loc: bool = True, ignored_keys: Optional[List] = None +): + """Method which returns a tree of Node elements as a String. + + Useful to debug deep DocumentNode instances created by gql or dsl_gql. + + WARNING: the output of this method is not guaranteed and may change without notice. + """ + + if ignored_keys is None: + ignored_keys = [] + + if ignore_loc: + # We are ignoring loc attributes by default + ignored_keys.append("loc") + + return _node_tree_recursive(obj, ignored_keys=ignored_keys) From 796b8f683a5f29aab1d6f77b36bf29e409144102 Mon Sep 17 00:00:00 2001 From: Leszek Hanusz Date: Sun, 19 Nov 2023 22:39:41 +0100 Subject: [PATCH 2/8] Adding failing test showing existing differences between DocumentNode instances produced by gql compared to dsl_gql. + Ignoring block attribute which can be None --- gql/utilities/node_tree.py | 10 +++++++++- .../test_dsl_directives.py | 19 +++++++++++++++++-- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/gql/utilities/node_tree.py b/gql/utilities/node_tree.py index c5346faa..923b9ecb 100644 --- a/gql/utilities/node_tree.py +++ b/gql/utilities/node_tree.py @@ -61,7 +61,11 @@ def _node_tree_recursive( def node_tree( - obj: Node, *, ignore_loc: bool = True, ignored_keys: Optional[List] = None + obj: Node, + *, + ignore_loc: bool = True, + ignore_block: bool = True, + ignored_keys: Optional[List] = None, ): """Method which returns a tree of Node elements as a String. @@ -77,4 +81,8 @@ def node_tree( # We are ignoring loc attributes by default ignored_keys.append("loc") + if ignore_block: + # We are ignoring block attributes by default (in StringValueNode) + ignored_keys.append("block") + return _node_tree_recursive(obj, ignored_keys=ignored_keys) diff --git a/tests/regressions/issue_447_dsl_missing_directives/test_dsl_directives.py b/tests/regressions/issue_447_dsl_missing_directives/test_dsl_directives.py index 61cc21e9..bf6ab33a 100644 --- a/tests/regressions/issue_447_dsl_missing_directives/test_dsl_directives.py +++ b/tests/regressions/issue_447_dsl_missing_directives/test_dsl_directives.py @@ -1,5 +1,6 @@ -from gql import Client -from gql.dsl import DSLFragment, DSLQuery, DSLSchema, dsl_gql +from gql import Client, gql +from gql.dsl import DSLFragment, DSLQuery, DSLSchema, dsl_gql, print_ast +from gql.utilities import node_tree schema_str = """ type MonsterForm { @@ -57,3 +58,17 @@ def test_issue_447(): q = dsl_gql(sprite, copy_of, DSLQuery(query)) client.validate(q) + + # Creating a tree from the DocumentNode created by dsl_gql + dsl_tree = node_tree(q) + + # Creating a tree from the DocumentNode created by dsl_gql + gql_tree = node_tree(gql(print_ast(q))) + + print("=======") + print(dsl_tree) + print("+++++++") + print(gql_tree) + print("=======") + + assert dsl_tree == gql_tree From 9cf41151ad9038d8229f2d3f3e555884455ca3ae Mon Sep 17 00:00:00 2001 From: Leszek Hanusz Date: Sun, 19 Nov 2023 23:44:12 +0100 Subject: [PATCH 3/8] Set variable_definitions to None for Fragments by default instead of empty tuple --- gql/dsl.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/gql/dsl.py b/gql/dsl.py index 0c834b33..1a7e3d89 100644 --- a/gql/dsl.py +++ b/gql/dsl.py @@ -1068,10 +1068,22 @@ def executable_ast(self) -> FragmentDefinitionNode: "Missing type condition. Please use .on(type_condition) method" ) + fragment_variable_definitions = self.variable_definitions.get_ast_definitions() + + if len(fragment_variable_definitions) == 0: + """Fragment variable definitions are obsolete and only supported on + graphql-core if the Parser is initialized with: + allow_legacy_fragment_variables=True. + + We will set variable_definitions to None instead of an empty tuple to be + coherent with how it works by default on graphql-core. + """ + fragment_variable_definitions = None + return FragmentDefinitionNode( type_condition=NamedTypeNode(name=NameNode(value=self._type.name)), selection_set=self.selection_set, - variable_definitions=self.variable_definitions.get_ast_definitions(), + variable_definitions=fragment_variable_definitions, name=NameNode(value=self.name), directives=(), ) From 9143a7de30a418792dcd59a48b26419779806ebd Mon Sep 17 00:00:00 2001 From: Leszek Hanusz Date: Sun, 19 Nov 2023 23:55:52 +0100 Subject: [PATCH 4/8] Adding more tests to ensure that the DocumentNode hierarchy produced by dsl_gql is the same than the one produced by the gql method. --- tests/starwars/test_dsl.py | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/tests/starwars/test_dsl.py b/tests/starwars/test_dsl.py index 9dc87910..91013cfa 100644 --- a/tests/starwars/test_dsl.py +++ b/tests/starwars/test_dsl.py @@ -35,7 +35,7 @@ ast_from_value, dsl_gql, ) -from gql.utilities import get_introspection_query_ast +from gql.utilities import get_introspection_query_ast, node_tree from .schema import StarWarsSchema @@ -151,6 +151,8 @@ def test_use_variable_definition_multiple_times(ds): }""" ) + assert node_tree(query) == node_tree(gql(print_ast(query))) + def test_add_variable_definitions(ds): var = DSLVariableDefinitions() @@ -172,6 +174,8 @@ def test_add_variable_definitions(ds): }""" ) + assert node_tree(query) == node_tree(gql(print_ast(query))) + def test_add_variable_definitions_with_default_value_enum(ds): var = DSLVariableDefinitions() @@ -216,6 +220,8 @@ def test_add_variable_definitions_with_default_value_input_object(ds): }""".strip() ) + assert node_tree(query) == node_tree(gql(print_ast(query))) + def test_add_variable_definitions_in_input_object(ds): var = DSLVariableDefinitions() @@ -241,6 +247,8 @@ def test_add_variable_definitions_in_input_object(ds): }""" ) + assert node_tree(query) == node_tree(gql(print_ast(query))) + def test_invalid_field_on_type_query(ds): with pytest.raises(AttributeError) as exc_info: @@ -402,6 +410,7 @@ def test_hero_name_query_result(ds, client): result = client.execute(query) expected = {"hero": {"name": "R2-D2"}} assert result == expected + assert node_tree(query) == node_tree(gql(print_ast(query))) def test_arg_serializer_list(ds, client): @@ -421,6 +430,7 @@ def test_arg_serializer_list(ds, client): ] } assert result == expected + assert node_tree(query) == node_tree(gql(print_ast(query))) def test_arg_serializer_enum(ds, client): @@ -428,6 +438,7 @@ def test_arg_serializer_enum(ds, client): result = client.execute(query) expected = {"hero": {"name": "Luke Skywalker"}} assert result == expected + assert node_tree(query) == node_tree(gql(print_ast(query))) def test_create_review_mutation_result(ds, client): @@ -442,6 +453,7 @@ def test_create_review_mutation_result(ds, client): result = client.execute(query) expected = {"createReview": {"stars": 5, "commentary": "This is a great movie!"}} assert result == expected + assert node_tree(query) == node_tree(gql(print_ast(query))) def test_subscription(ds): @@ -463,6 +475,8 @@ def test_subscription(ds): }""" ) + assert node_tree(query) == node_tree(gql(print_ast(query))) + def test_field_does_not_exit_in_type(ds): with pytest.raises( @@ -502,6 +516,7 @@ def test_multiple_root_fields(ds, client): "hero_of_episode_5": {"name": "Luke Skywalker"}, } assert result == expected + assert node_tree(query) == node_tree(gql(print_ast(query))) def test_root_fields_aliased(ds, client): @@ -517,6 +532,7 @@ def test_root_fields_aliased(ds, client): "hero_of_episode_5": {"name": "Luke Skywalker"}, } assert result == expected + assert node_tree(query) == node_tree(gql(print_ast(query))) def test_operation_name(ds): @@ -535,6 +551,8 @@ def test_operation_name(ds): }""" ) + assert node_tree(query) == node_tree(gql(print_ast(query))) + def test_multiple_operations(ds): query = dsl_gql( @@ -565,6 +583,8 @@ def test_multiple_operations(ds): }""" ) + assert node_tree(query) == node_tree(gql(print_ast(query))) + def test_inline_fragments(ds): query = """hero(episode: JEDI) { @@ -635,6 +655,7 @@ def test_fragments(ds): print(print_ast(document)) assert query == print_ast(document) + assert node_tree(document) == node_tree(gql(print_ast(document))) def test_fragment_without_type_condition_error(ds): @@ -731,6 +752,7 @@ def test_dsl_nested_query_with_fragment(ds): print(print_ast(document)) assert query == print_ast(document) + assert node_tree(document) == node_tree(gql(print_ast(document))) # Same thing, but incrementaly @@ -756,6 +778,7 @@ def test_dsl_nested_query_with_fragment(ds): print(print_ast(document)) assert query == print_ast(document) + assert node_tree(document) == node_tree(gql(print_ast(document))) def test_dsl_query_all_fields_should_be_instances_of_DSLField(): @@ -808,6 +831,8 @@ def test_dsl_root_type_not_default(): "Invalid field for : " ) in str(excinfo.value) + assert node_tree(query) == node_tree(gql(print_ast(query))) + def test_dsl_gql_all_arguments_should_be_operations_or_fragments(): with pytest.raises( @@ -967,6 +992,9 @@ def test_get_introspection_query_ast(option): ) assert print_ast(gql(introspection_query)) == print_ast(dsl_introspection_query) + assert node_tree(dsl_introspection_query) == node_tree( + gql(print_ast(dsl_introspection_query)) + ) def test_typename_aliased(ds): From 6b96315bb4da3a6df647b17579c267edc01ac549 Mon Sep 17 00:00:00 2001 From: Leszek Hanusz Date: Mon, 20 Nov 2023 00:19:37 +0100 Subject: [PATCH 5/8] Fix code coverage --- gql/utilities/node_tree.py | 9 ++-- tests/starwars/test_dsl.py | 89 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 4 deletions(-) diff --git a/gql/utilities/node_tree.py b/gql/utilities/node_tree.py index 923b9ecb..c307d937 100644 --- a/gql/utilities/node_tree.py +++ b/gql/utilities/node_tree.py @@ -6,12 +6,11 @@ def _node_tree_recursive( obj: Any, *, - indent=0, - ignored_keys=None, + indent: int = 0, + ignored_keys: List, ): - if ignored_keys is None: - ignored_keys = [] + assert ignored_keys is not None results = [] @@ -74,6 +73,8 @@ def node_tree( WARNING: the output of this method is not guaranteed and may change without notice. """ + assert isinstance(obj, Node) + if ignored_keys is None: ignored_keys = [] diff --git a/tests/starwars/test_dsl.py b/tests/starwars/test_dsl.py index 91013cfa..be9319b3 100644 --- a/tests/starwars/test_dsl.py +++ b/tests/starwars/test_dsl.py @@ -1014,3 +1014,92 @@ def test_typename_aliased(ds): ds.Character.name, DSLMetaField("__typename").alias("typenameField") ) assert query == str(query_dsl) + + +def test_node_tree_with_loc(ds): + query = """query GetHeroName { + hero { + name + } +}""".strip() + + document = gql(query) + + node_tree_result = """ +DocumentNode + loc: + Location + + definitions: + OperationDefinitionNode + loc: + Location + + name: + NameNode + loc: + Location + + value: + 'GetHeroName' + directives: + empty tuple + variable_definitions: + empty tuple + selection_set: + SelectionSetNode + loc: + Location + + selections: + FieldNode + loc: + Location + + directives: + empty tuple + alias: + None + name: + NameNode + loc: + Location + + value: + 'hero' + arguments: + empty tuple + nullability_assertion: + None + selection_set: + SelectionSetNode + loc: + Location + + selections: + FieldNode + loc: + Location + + directives: + empty tuple + alias: + None + name: + NameNode + loc: + Location + + value: + 'name' + arguments: + empty tuple + nullability_assertion: + None + selection_set: + None + operation: + +""".strip() + + assert node_tree(document, ignore_loc=False) == node_tree_result From 10e0f355f98c79a6d113347755ebd184657fe421 Mon Sep 17 00:00:00 2001 From: Leszek Hanusz Date: Mon, 20 Nov 2023 00:24:56 +0100 Subject: [PATCH 6/8] fix comment --- .../issue_447_dsl_missing_directives/test_dsl_directives.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/regressions/issue_447_dsl_missing_directives/test_dsl_directives.py b/tests/regressions/issue_447_dsl_missing_directives/test_dsl_directives.py index bf6ab33a..b31ade7f 100644 --- a/tests/regressions/issue_447_dsl_missing_directives/test_dsl_directives.py +++ b/tests/regressions/issue_447_dsl_missing_directives/test_dsl_directives.py @@ -62,7 +62,7 @@ def test_issue_447(): # Creating a tree from the DocumentNode created by dsl_gql dsl_tree = node_tree(q) - # Creating a tree from the DocumentNode created by dsl_gql + # Creating a tree from the DocumentNode created by gql gql_tree = node_tree(gql(print_ast(q))) print("=======") From 9d068ad224e0603be9e9b46ded910e8c466d2d6b Mon Sep 17 00:00:00 2001 From: Leszek Hanusz Date: Mon, 20 Nov 2023 00:33:14 +0100 Subject: [PATCH 7/8] Fix lint --- gql/dsl.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/gql/dsl.py b/gql/dsl.py index 1a7e3d89..536a8b8b 100644 --- a/gql/dsl.py +++ b/gql/dsl.py @@ -1075,15 +1075,19 @@ def executable_ast(self) -> FragmentDefinitionNode: graphql-core if the Parser is initialized with: allow_legacy_fragment_variables=True. - We will set variable_definitions to None instead of an empty tuple to be - coherent with how it works by default on graphql-core. + We will not provide variable_definitions instead of providing an empty + tuple to be coherent with how it works by default on graphql-core. """ - fragment_variable_definitions = None + variable_definition_kwargs = {} + else: + variable_definition_kwargs = { + "variable_definitions": fragment_variable_definitions + } return FragmentDefinitionNode( type_condition=NamedTypeNode(name=NameNode(value=self._type.name)), selection_set=self.selection_set, - variable_definitions=fragment_variable_definitions, + **variable_definition_kwargs, name=NameNode(value=self.name), directives=(), ) From 2b560703c254ab9e3fc372a13a3f051ba7478f8e Mon Sep 17 00:00:00 2001 From: Leszek Hanusz Date: Mon, 20 Nov 2023 13:54:04 +0100 Subject: [PATCH 8/8] Add test for legacy fragment with variables --- tests/starwars/test_dsl.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/starwars/test_dsl.py b/tests/starwars/test_dsl.py index be9319b3..4860e3a0 100644 --- a/tests/starwars/test_dsl.py +++ b/tests/starwars/test_dsl.py @@ -1103,3 +1103,30 @@ def test_node_tree_with_loc(ds): """.strip() assert node_tree(document, ignore_loc=False) == node_tree_result + + +def test_legacy_fragment_with_variables(ds): + var = DSLVariableDefinitions() + + hero_fragment = ( + DSLFragment("heroFragment") + .on(ds.Query) + .select( + ds.Query.hero.args(episode=var.episode).select(ds.Character.name), + ) + ) + + print(hero_fragment) + + hero_fragment.variable_definitions = var + + query = dsl_gql(hero_fragment) + + expected = """ +fragment heroFragment($episode: Episode) on Query { + hero(episode: $episode) { + name + } +} +""".strip() + assert print_ast(query) == expected