Merge pull request #174 from evo-company/fix-keyerror-for-invalid-fra…

…gment-type-name Return proper error instead of KeyError for invalid fragment type name
evo-company · Nov 8, 2024 · 0e0a921 · 0e0a921
2 parents 2bfd8c3 + 7d50df0
commit 0e0a921
Show file tree

Hide file tree

Showing 3 changed files with 290 additions and 135 deletions.
diff --git 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
@@ -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,29 +215,30 @@ def test_named_fragments() -> None:
                 Field("apres"),
             ],
             [
-                Fragment("Goaded", 'Makai', Node([
-                    Field("doozie"),
-                    PinsLink
-                ])),
-            ]
+                Fragment("Goaded", "Makai", Node([Field("doozie"), PinsLink])),
+            ],
         ),
         options={"gire": "noatak"},
     )
 
     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},
     )