From 7d50df0049b42c3af4c3cf681e0cad7c4a642848 Mon Sep 17 00:00:00 2001 From: "s.kovbasa" Date: Fri, 8 Nov 2024 02:58:27 +0200 Subject: [PATCH] Return proper error instead of KeyError for invalid fragment type name --- hiku/validate/query.py | 71 +++++----- tests/test_read_graphql.py | 99 ++++++++++---- tests/test_validate_query.py | 255 +++++++++++++++++++++++++---------- 3 files changed, 290 insertions(+), 135 deletions(-) diff --git a/hiku/validate/query.py b/hiku/validate/query.py index bc91fbb..e89a330 100644 --- a/hiku/validate/query.py +++ b/hiku/validate/query.py @@ -1,53 +1,47 @@ import typing as t - -from dataclasses import dataclass -from contextlib import contextmanager from collections import abc as collections_abc +from contextlib import contextmanager +from dataclasses import dataclass +from hiku.graph import ( + Field, + Graph, + GraphVisitor, + Interface, + Link, + LinkType, + Node, + Nothing, + Option, + Root, + Union, +) +from hiku.query import Field as QueryField +from hiku.query import FieldBase, Fragment +from hiku.query import Link as QueryLink +from hiku.query import Node as QueryNode +from hiku.query import QueryVisitor from hiku.scalar import ScalarMeta from ..types import ( AbstractTypeVisitor, - IDMeta, - Record, - OptionalMeta, - SequenceMeta, - RecordMeta, - TypeRefMeta, - EnumRefMeta, - GenericMeta, AnyMeta, BooleanMeta, - StringMeta, - IntegerMeta, + EnumRefMeta, FloatMeta, + GenericMeta, + IDMeta, + IntegerMeta, MappingMeta, -) - -from hiku.query import ( - Field as QueryField, - FieldBase, - Fragment, - Node as QueryNode, - Link as QueryLink, - QueryVisitor, -) -from hiku.graph import ( - Interface, - LinkType, - Node, - Field, - Link, - GraphVisitor, - Root, - Option, - Nothing, - Graph, - Union, + OptionalMeta, + Record, + RecordMeta, + SequenceMeta, + StringMeta, + TypeRefMeta, ) from .errors import Errors - _undefined = object() @@ -523,6 +517,11 @@ def visit_fragment(self, obj: Fragment) -> t.Any: self._type[-1].name is None and obj.type_name is None ): graph_node = self.graph.root + elif obj.type_name not in self.graph.nodes_map: + self.errors.report( + "Fragment on an unknown type '{}'".format(obj.type_name) + ) + return else: graph_node = self.graph.nodes_map[obj.type_name] diff --git a/tests/test_read_graphql.py b/tests/test_read_graphql.py index 803dc06..ce47e79 100644 --- a/tests/test_read_graphql.py +++ b/tests/test_read_graphql.py @@ -191,12 +191,18 @@ def test_named_fragments() -> None: ), ], [ - Fragment("Meer", 'Torsion', Node([ - Link( - "kilned", - Node([Field("rusk")]), + Fragment( + "Meer", + "Torsion", + Node( + [ + Link( + "kilned", + Node([Field("rusk")]), + ), + ] ), - ])), + ), ], ), ) @@ -209,11 +215,8 @@ def test_named_fragments() -> None: Field("apres"), ], [ - Fragment("Goaded", 'Makai', Node([ - Field("doozie"), - PinsLink - ])), - ] + Fragment("Goaded", "Makai", Node([Field("doozie"), PinsLink])), + ], ), options={"gire": "noatak"}, ) @@ -221,17 +224,21 @@ def test_named_fragments() -> None: GiltsLink = Link( "gilts", Node( + [SneezerLink], [ - SneezerLink - ], - [ - Fragment(None, "Valium", Node([ - Link( - "movies", - Node([Field("boree")]), + Fragment( + None, + "Valium", + Node( + [ + Link( + "movies", + Node([Field("boree")]), + ), + ] ), - ])), - ] + ), + ], ), ) @@ -371,11 +378,23 @@ def test_variables_in_fragment(): Node( [], [ - Fragment("Pujari", "Ashlee", Node([Field( - "fibbery", - options={"baps": None, "bankit": 123, "riuer": 234}, - )])) - ] + Fragment( + "Pujari", + "Ashlee", + Node( + [ + Field( + "fibbery", + options={ + "baps": None, + "bankit": 123, + "riuer": 234, + }, + ) + ] + ), + ) + ], ), {"popedom": 123}, ) @@ -455,7 +474,14 @@ def test_skip_fragment_spread(skip): bar } """, - Node([Field("foo")], ([] if skip else [Fragment("Fragment", "Thing", Node([Field("bar")]))])), + Node( + [Field("foo")], + ( + [] + if skip + else [Fragment("Fragment", "Thing", Node([Field("bar")]))] + ), + ), {"cond": skip}, ) @@ -471,7 +497,10 @@ def test_skip_inline_fragment(skip): } } """, - Node([Field("foo")], ([] if skip else [Fragment(None, "Thing", Node([Field("bar")]))])), + Node( + [Field("foo")], + ([] if skip else [Fragment(None, "Thing", Node([Field("bar")]))]), + ), {"cond": skip}, ) @@ -502,7 +531,14 @@ def test_include_fragment_spread(include): bar } """, - Node([Field("foo")], ([Fragment("Fragment", "Thing", Node([Field("bar")]))] if include else [])), + Node( + [Field("foo")], + ( + [Fragment("Fragment", "Thing", Node([Field("bar")]))] + if include + else [] + ), + ), {"cond": include}, ) @@ -518,7 +554,14 @@ def test_include_inline_fragment(include): } } """, - Node([Field("foo")], ([Fragment(None, "Thing", Node([Field("bar")]))] if include else [])), + Node( + [Field("foo")], + ( + [Fragment(None, "Thing", Node([Field("bar")]))] + if include + else [] + ), + ), {"cond": include}, ) diff --git a/tests/test_validate_query.py b/tests/test_validate_query.py index 652e146..e8827af 100644 --- a/tests/test_validate_query.py +++ b/tests/test_validate_query.py @@ -1,9 +1,19 @@ import pytest from hiku import query as q -from hiku.graph import Graph, Node, Field, Link, Option, Root -from hiku.types import Integer, Record, Sequence, Optional, TypeRef, Boolean -from hiku.types import String, Mapping, Any +from hiku.graph import Field, Graph, Link, Node, Option, Root +from hiku.readers.graphql import read +from hiku.types import ( + Any, + Boolean, + Integer, + Mapping, + Optional, + Record, + Sequence, + String, + TypeRef, +) from hiku.validate.query import validate @@ -153,9 +163,14 @@ def test_field_complex(field_name): def test_field_complex_with_typename(): check_errors( - q.Node([ - q.Link('val', q.Node([q.Field('__typename'), q.Field('attr')], [])) - ]), [] + q.Node( + [ + q.Link( + "val", q.Node([q.Field("__typename"), q.Field("attr")], []) + ) + ] + ), + [], ) @@ -609,75 +624,157 @@ def test_distinct_by_name_fields(): ] -@pytest.mark.parametrize("query", [ - pytest.param( - q.Node([q.Link("x", q.Node([ - q.Field("a"), - q.Field("a", options={"e": 1}) - ]))]), - id="only fields" - ), - pytest.param( - q.Node([q.Link("x", q.Node( - [], - [ - q.Fragment('Fragment', 'X', q.Node([ - q.Field("a"), - q.Field("a", options={"e": 1}), - ])) - ]))]), - id="only fields inside fragment" - ), - pytest.param( - q.Node([q.Link("x", q.Node( - [], - [ - q.Fragment('FragmentA', 'X', q.Node([ - q.Field("a"), - ])), - q.Fragment('FragmentB', 'X', q.Node([ - q.Field("a", options={"e": 1}), - ])), - ]))]), - id="fields inside neighbour fragment" - ), - pytest.param( - q.Node([q.Link("x", q.Node( - [ - q.Field("a"), - ], - [ - q.Fragment('Fragment', 'X', q.Node([ - q.Field("a", options={"e": 1}), - ])), - ]))]), - id="field + neighbour fragment" - ), - pytest.param( - q.Node([ - q.Link("x", q.Node( - [], +@pytest.mark.parametrize( + "query", + [ + pytest.param( + q.Node( + [ + q.Link( + "x", + q.Node([q.Field("a"), q.Field("a", options={"e": 1})]), + ) + ] + ), + id="only fields", + ), + pytest.param( + q.Node( + [ + q.Link( + "x", + q.Node( + [], + [ + q.Fragment( + "Fragment", + "X", + q.Node( + [ + q.Field("a"), + q.Field("a", options={"e": 1}), + ] + ), + ) + ], + ), + ) + ] + ), + id="only fields inside fragment", + ), + pytest.param( + q.Node( [ - q.Fragment('FragmentB', 'X', q.Node([ - q.Field("a", options={"e": 1}), - ])), + q.Link( + "x", + q.Node( + [], + [ + q.Fragment( + "FragmentA", + "X", + q.Node( + [ + q.Field("a"), + ] + ), + ), + q.Fragment( + "FragmentB", + "X", + q.Node( + [ + q.Field("a", options={"e": 1}), + ] + ), + ), + ], + ), + ) ] - )) - ], [ - q.Fragment('FragmentA', 'Query', q.Node([ - q.Link("x", q.Node( - [], + ), + id="fields inside neighbour fragment", + ), + pytest.param( + q.Node( [ - q.Fragment('FragmentA', 'X', q.Node([ - q.Field("a"), - ])), + q.Link( + "x", + q.Node( + [ + q.Field("a"), + ], + [ + q.Fragment( + "Fragment", + "X", + q.Node( + [ + q.Field("a", options={"e": 1}), + ] + ), + ), + ], + ), + ) ] - )) - ])), - ]), - id="different level fragment, same link" - ), -]) + ), + id="field + neighbour fragment", + ), + pytest.param( + q.Node( + [ + q.Link( + "x", + q.Node( + [], + [ + q.Fragment( + "FragmentB", + "X", + q.Node( + [ + q.Field("a", options={"e": 1}), + ] + ), + ), + ], + ), + ) + ], + [ + q.Fragment( + "FragmentA", + "Query", + q.Node( + [ + q.Link( + "x", + q.Node( + [], + [ + q.Fragment( + "FragmentA", + "X", + q.Node( + [ + q.Field("a"), + ] + ), + ), + ], + ), + ) + ] + ), + ), + ], + ), + id="different level fragment, same link", + ), + ], +) def test_distinct_by_options_fields(query): graph = Graph( [ @@ -858,3 +955,19 @@ def test_any_in_option(): 'Invalid value for option "root.get:foo", ' '"str" instead of Mapping[String, Any]' ] + + +def test_validate_query_fragment_on_unknown_type(): + query = """ + query { + amyls { + ... on Asdf { + id + } + } + } + """ + + errors = validate(GRAPH, read(query)) + + assert errors == ["Fragment on an unknown type 'Asdf'"]