Merge pull request #63 from evo-company/check-result-hashable

check result of resolver is hashable

.python-version

@@ -1 +1 @@
-3.7
+3.7.12

Dockerfile

@@ -25,6 +25,8 @@ RUN python3 -m pip install \
     -r requirements-tests.txt
 RUN python3 -m pip install tox
 
+RUN python3 -m pip install tox
+
 FROM tests as examples
 
 RUN python3 -m pip install \

docker-compose.yaml

@@ -33,8 +33,8 @@ services:
 
   examples-aiohttp:
     <<: *examples-base
-    command: python3 examples/graphql_aiohttp.py
-
+    command: python3 examples/graphql_aiohttp.py  
+    
   test-base: &test-base
     <<: *base
     image: hiku-tests

hiku/engine.py

@@ -1,10 +1,12 @@
 import inspect
 import warnings
+import dataclasses
 
+from typing import Any
 from functools import partial
 from itertools import chain, repeat
 from collections import defaultdict
-from collections.abc import Sequence, Mapping
+from collections.abc import Sequence, Mapping, Hashable
 
 from . import query as hiku_query
 from .graph import Link, Maybe, One, Many, Nothing, Field
@@ -149,6 +151,18 @@ def _check_store_fields(node, fields, ids, result):
                             expected, result))
 
 
+def _is_hashable(obj: Any) -> bool:
+    if not isinstance(obj, Hashable):
+        return False
+
+    try:
+        hash(obj)
+    except TypeError:
+        return False
+
+    return True
+
+
 def store_fields(index, node, query_fields, ids, query_result):
     if inspect.isgenerator(query_result):
         warnings.warn('Data loading functions should not return generators',
@@ -200,12 +214,33 @@ def link_ref_many(graph_link, idents):
 }
 
 
+HASH_HINT = "\nHint: Consider adding __hash__ method or use hashable type."
+DATACLASS_HINT = "\nHint: Use @dataclass(frozen=True) to make object hashable."
+
+
+def _hashable_hint(obj: Any) -> str:
+    if isinstance(obj, Sequence):
+        return _hashable_hint(obj[0])
+
+    if (
+        dataclasses.is_dataclass(obj)
+        and not getattr(obj, dataclasses._PARAMS).frozen  # type: ignore[attr-defined]  # noqa: E501
+    ):
+        return DATACLASS_HINT
+
+    return HASH_HINT
+
+
 def _check_store_links(node, link, ids, result):
+    hint = ''
     if node.name is not None and link.requires is not None:
         assert ids is not None
         if link.type_enum is Maybe or link.type_enum is One:
             if isinstance(result, Sequence) and len(result) == len(ids):
-                return
+                if all(map(_is_hashable, result)):
+                    return
+                expected = 'list of hashable objects'
+                hint = _hashable_hint(result)
             else:
                 expected = 'list (len: {})'.format(len(ids))
         elif link.type_enum is Many:
@@ -214,25 +249,40 @@ def _check_store_links(node, link, ids, result):
                 and len(result) == len(ids)
                 and all(isinstance(r, Sequence) for r in result)
             ):
-                return
+                unhashable = False
+                for items in result:
+                    if not all(map(_is_hashable, items)):
+                        unhashable = True
+                        break
+
+                if not unhashable:
+                    return
+                expected = 'list (len: {}) of lists of hashable objects'.format(len(ids))  # noqa: E501
+                hint = _hashable_hint(result)
             else:
                 expected = 'list (len: {}) of lists'.format(len(ids))
         else:
             raise TypeError(link.type_enum)
     else:
         if link.type_enum is Maybe or link.type_enum is One:
-            return
+            if _is_hashable(result):
+                return
+            expected = 'hashable object'
+            hint = _hashable_hint(result)
         elif link.type_enum is Many:
             if isinstance(result, Sequence):
-                return
+                if all(map(_is_hashable, result)):
+                    return
+                expected = 'list of hashable objects'
+                hint = _hashable_hint(result)
             else:
                 expected = 'list'
         else:
             raise TypeError(link.type_enum)
     raise TypeError('Can\'t store link values, node: {!r}, link: {!r}, '
-                    'expected: {}, returned: {!r}'
+                    'expected: {}, returned: {!r}{}'
                     .format(node.name or '__root__', link.name,
-                            expected, result))
+                            expected, result, hint))
 
 
 def store_links(index, node, graph_link, query_link, ids, query_result):

tests/test_hashable.py

@@ -0,0 +1,228 @@
+import pytest
+
+from dataclasses import dataclass
+
+from hiku.builder import build, Q
+from hiku.engine import Engine
+from hiku.executors.sync import SyncExecutor
+from hiku.graph import Graph, Node, Field, Root, Link, Option
+from hiku.types import (
+    Any,
+    Integer,
+    TypeRef,
+    String,
+    Sequence,
+)
+
+
+def direct_link(ids):
+    return ids
+
+
+def execute(graph, query_, ctx=None):
+    engine = Engine(SyncExecutor())
+    return engine.execute(graph, query_, ctx=ctx)
+
+
+def test_link_requires_field_with_unhashable_data():
+    @dataclass(frozen=True)
+    class User:
+        id: int
+
+    @dataclass
+    class UserData:
+        age: int
+
+    def link_user():
+        return User(1)
+
+    def user_info_fields(fields, data):
+        return [
+            [getattr(d, f.name) for f in fields]
+            for d in data
+        ]
+
+    def user_fields(fields, ids):
+        def map_field(f, user):
+            if f.name == 'id':
+                return user.id
+            if f.name == '_data':
+                return UserData(20)
+
+        return [[map_field(f, user) for f in fields] for user in ids]
+
+    GRAPH = Graph([
+        Node('userInfo', [
+            Field('age', Integer, user_info_fields),
+        ]),
+        Node('user', [
+            Field('id', Integer, user_fields),
+            Field('_data', Any, user_fields),
+            Link('info', TypeRef['userInfo'], direct_link, requires='_data'),
+        ]),
+        Root([
+            Link('user', TypeRef['user'],  link_user,  requires=None),
+        ]),
+    ])
+
+    with pytest.raises(TypeError) as err:
+        execute(GRAPH, build([
+            Q.user[
+                Q.id,
+                Q.info[
+                    Q.age
+                ]
+            ]
+        ]))
+
+    err.match(
+        r"Can't store link values, node: 'user', link: 'info', "
+        r"expected: list of hashable objects, returned:(.*UserData)"
+    )
+
+
+def test_link_to_sequence():
+    @dataclass(frozen=True)
+    class User:
+        id: int
+
+    @dataclass
+    class Tag:
+        name: str
+
+    def link_user():
+        return User(1)
+
+    def tags_fields(fields, data):
+        return [[getattr(d, f.name) for f in fields] for d in data]
+
+    def user_fields(fields, users):
+        return [[getattr(user, f.name) for f in fields] for user in users]
+
+    def link_tags(ids):
+        return [[Tag('tag1')] for id_ in ids]
+
+    GRAPH = Graph([
+        Node('tags', [
+            Field('name', String, tags_fields),
+        ]),
+        Node('user', [
+            Field('id', Integer, user_fields),
+            Link('tags', Sequence[TypeRef['tags']], link_tags, requires='id'),
+        ]),
+        Root([
+            Link('user', TypeRef['user'],  link_user,  requires=None),
+        ]),
+    ])
+
+    with pytest.raises(TypeError) as err:
+        execute(GRAPH, build([
+            Q.user[
+                Q.id,
+                Q.tags[Q.name]
+            ]
+        ]))
+
+    err.match(
+        r"Can't store link values, node: 'user', link: 'tags', "
+        r"expected: list \(len: 1\) of lists of hashable objects, "
+        r"returned:(.*Tag)"
+    )
+
+
+def test_root_link_resolver_returns_unhashable_data():
+    @dataclass
+    class User:
+        id: int
+
+    def link_user():
+        return User(1)
+
+    def user_fields(fields, ids):
+        def map_field(f, user):
+            if f.name == 'id':
+                return user.id
+
+        return [[map_field(f, user) for f in fields] for user in ids]
+
+    GRAPH = Graph([
+        Node('user', [
+            Field('id', Integer, user_fields),
+        ]),
+        Root([
+            Link('user', TypeRef['user'],  link_user,  requires=None),
+        ]),
+    ])
+
+    with pytest.raises(TypeError) as err:
+        execute(GRAPH, build([Q.user[Q.id]]))
+
+    err.match(
+        r"Can't store link values, node: '__root__', link: 'user', "
+        r"expected: hashable object, returned:(.*User)"
+    )
+
+
+def test_root_link_requires_field_with_unhashable_data():
+    @dataclass
+    class User:
+        id: int
+
+    def user_data(fields):
+        return [User(1)]
+
+    def user_fields(fields, ids):
+        ...
+
+    GRAPH = Graph([
+        Node('user', [
+            Field('id', Integer, user_fields),
+        ]),
+        Root([
+            Field('_user', Any,  user_data),
+            Link('user', TypeRef['user'],  direct_link,  requires='_user'),
+        ]),
+    ])
+
+    with pytest.raises(TypeError) as err:
+        execute(GRAPH, build([Q.user[Q.id]]))
+
+    err.match(
+        r"Can't store link values, node: '__root__', link: 'user', "
+        r"expected: hashable object, returned:(.*User)"
+    )
+
+
+def test_root_link_options_unhashable_data():
+    @dataclass
+    class UserOpts:
+        id: int
+
+    def user_fields(fields, ids):
+        ...
+
+    def link_options(opts):
+        return UserOpts(opts['id'])
+
+    GRAPH = Graph([
+        Node('user', [
+            Field('id', Integer, user_fields),
+        ]),
+        Root([
+            Link(
+                'user',
+                TypeRef['user'],
+                link_options,
+                requires=None,
+                options=[Option('id', Integer)]
+            ),
+        ]),
+    ])
+
+    with pytest.raises(TypeError) as err:
+        execute(GRAPH, build([Q.user(id=1)[Q.id]]))
+
+    err.match(
+        r"Can't store link values, node: '__root__', link: 'user', "
+        r"expected: hashable object, returned:(.*User)"
+    )