Merge pull request #88 from evo-company/requires-list

add support for requires as list of strings

docs/basics.rst

@@ -80,13 +80,13 @@ Then lets define our graph with one :py:class:`~hiku.graph.Node` and one
 
 ``character_data`` function :sup:`[8]` is used to resolve values for two fields
 in the ``Character`` node. As you can see, it returns basically a list of lists
-with values in the same order as it was requested in arguments (order of ids and
-fields should be preserved).
+with values in the same order as it was requested in arguments (**order of ids and
+fields should be preserved**).
 
 This function used twice in the graph :sup:`[20-21]` -- for two fields, this is
 how `Hiku` understands that both these fields can be loaded using this one
-function and one function call. `Hiku` groups fields by function, to load them
-together.
+function and one function call. `Hiku` **groups fields by function, to load them
+together.**
 
 This gives us ability to resolve many fields for many objects (ids) using just
 one simple function (when possible) to efficiently load data without introducing
@@ -131,11 +131,14 @@ Here is our extended graph definition:
     :emphasize-lines: 20,26,36,37,40-41,43,46-47
 
 Here ``actors`` :py:class:`~hiku.graph.Link` :sup:`[40-41]`, defined in the
-``Character`` node :sup:`[36]`, requires ``id`` field :sup:`[37]` to map
+``Character`` node :sup:`[36]`, ``requires='id'`` field :sup:`[37]` to map
 characters to actors. That's why ``id`` field :sup:`[37]` was added to the
 ``Character`` node :sup:`[36]`. The same work should be done in the ``Actor``
 node :sup:`[43]` to implement backward ``character`` link :sup:`[46-47]`.
 
+``requires`` argument can be specified as a list of fields, in this case
+``Hiku`` will resolve all of them and pass a ``list`` of ``dict`` to resolver.
+
 ``character_to_actors_link`` function :sup:`[20]` accepts ids of the characters
 and should return list of lists -- ids of the actors, in the same order, so
 every character id can be associated with a list of actor ids. This is how

docs/caching.rst

@@ -1,6 +1,8 @@
 Caching
 =======
 
+.. _caching-doc:
+
 Caching is experimental feature.
 
 For now only non-root link will be cached.

docs/changelog/changes_07.rst

@@ -29,6 +29,8 @@ Changes in 0.7
   - Added mypy and typings to codebase
   - Added checks for unhashable link results and extend errors. This must improve developer experience.
   - Added caching for parsing graphql query. It is optional and can be enabled by calling :py:func:`hiku.readers.graphql.setup_query_cache`.
+  - Added result cache - it is possible now to specify ``@cached`` directive to cache parts of the query. :ref:`Check cache documentation <caching-doc>`
+  - ``Link(requires=['a', 'b'])`` can be specified as a list of strings. It is useful when you want to require multiple fields at once. It will pass a list of dicts to the resolver.
 
 Backward-incompatible changes
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

hiku/edn.py

@@ -7,21 +7,7 @@
 from itertools import chain
 from json.encoder import encode_basestring, encode_basestring_ascii  # type: ignore # noqa: E501
 
-
-class ImmutableDict(dict):
-    _hash = None
-
-    def __hash__(self):
-        if self._hash is None:
-            self._hash = hash(frozenset(self.items()))
-        return self._hash
-
-    def _immutable(self):
-        raise TypeError("{} object is immutable"
-                        .format(self.__class__.__name__))
-
-    __delitem__ = __setitem__ = _immutable  # type: ignore
-    clear = pop = popitem = setdefault = update = _immutable  # type: ignore
+from hiku.utils import ImmutableDict
 
 
 class Symbol(str):

hiku/engine.py

@@ -59,6 +59,7 @@
     TaskSet,
     SubmitRes,
 )
+from .utils import ImmutableDict
 
 if TYPE_CHECKING:
     from .readers.graphql import Operation
@@ -150,7 +151,11 @@ def visit_link(self, obj: QueryLink) -> None:
         graph_obj = self._node.fields_map[obj.name]
         if isinstance(graph_obj, Link):
             if graph_obj.requires:
-                self.visit(QueryField(graph_obj.requires))
+                if isinstance(graph_obj.requires, list):
+                    for r in graph_obj.requires:
+                        self.visit(QueryField(r))
+                else:
+                    self.visit(QueryField(graph_obj.requires))
             self._links.append((graph_obj, obj))
         else:
             assert isinstance(graph_obj, Field), type(graph_obj)
@@ -191,7 +196,11 @@ def visit_field(self, obj: QueryField) -> None:
     def visit_link(self, obj: QueryLink) -> None:
         graph_obj = self._node.fields_map[obj.name]
         if graph_obj.requires:
-            self.visit(QueryField(graph_obj.requires))
+            if isinstance(graph_obj.requires, list):
+                for r in graph_obj.requires:
+                    self.visit(QueryField(r))
+            else:
+                self.visit(QueryField(graph_obj.requires))
         self._groups.append((graph_obj, obj))
         self._funcs.append(graph_obj.func)
         self._current_func = None
@@ -281,18 +290,32 @@ def store_fields(
     return None
 
 
+Req = TypeVar('Req')
+
+
 def link_reqs(
     index: Index,
     node: Node,
     link: Link,
     ids: Any
-) -> Any:
+) -> Union[List[ImmutableDict[str, Req]], List[Req], Req]:
     """For a given link, find link `requires` values by ids."""
     if node.name is not None:
         assert ids is not None
         node_idx = index[node.name]
+
+        if isinstance(link.requires, list):
+            reqs: List[ImmutableDict[str, Req]] = []
+            for i in ids:
+                req: ImmutableDict = ImmutableDict(
+                    (r, node_idx[i][r]) for r in link.requires
+                )
+                reqs.append(req)
+            return reqs
+
         return [node_idx[i][link.requires] for i in ids]
     else:
+        # TODO: add support for requires as list in root node
         return index.root[link.requires]
 
 
@@ -451,6 +474,9 @@ def link_result_to_ids(
     raise TypeError(repr([from_list, link_type]))
 
 
+Dep = Union[SubmitRes, TaskSet]
+
+
 class Query(Workflow):
 
     def __init__(
@@ -564,7 +590,7 @@ def process_node(
             from_func[func].append((graph_field, query_field))
 
         # schedule fields resolve
-        to_dep: Dict[Callable, Union[SubmitRes, TaskSet]] = {}
+        to_dep: Dict[Callable, Dep] = {}
         for func, func_fields in from_func.items():
             self._track(path)
             to_dep[func] = self._schedule_fields(
@@ -577,8 +603,22 @@ def process_node(
             schedule = partial(self._schedule_link, path, node,
                                graph_link, query_link, ids)
             if graph_link.requires:
-                dep = to_dep[to_func[graph_link.requires]]
-                self._queue.add_callback(dep, schedule)
+                if isinstance(graph_link.requires, list):
+                    done_deps = set()
+
+                    def add_done_dep_callback(dep: Dep, req: Any) -> None:
+                        def done_cb() -> None:
+                            done_deps.add(req)
+                            if done_deps == set(graph_link.requires):
+                                schedule()
+
+                        self._queue.add_callback(dep, done_cb)
+
+                    for req in graph_link.requires:
+                        add_done_dep_callback(to_dep[to_func[req]], req)
+                else:
+                    dep = to_dep[to_func[graph_link.requires]]
+                    self._queue.add_callback(dep, schedule)
             else:
                 schedule()
 
@@ -700,7 +740,7 @@ def _schedule_link(
         """
         args = []
         if graph_link.requires:
-            reqs = link_reqs(self._index, node, graph_link, ids)
+            reqs: Any = link_reqs(self._index, node, graph_link, ids)
 
             if 'cached' in query_link.directives_map and self._cache and not skip_cache:  # noqa: E501
                 return self._update_index_from_cache(
@@ -724,7 +764,7 @@ def callback() -> None:
         def store_link_cache() -> None:
             assert self._cache is not None
             cached = query_link.directives_map['cached']
-            reqs = link_reqs(self._index, node, graph_link, ids)
+            reqs: Any = link_reqs(self._index, node, graph_link, ids)
             to_cache = CacheVisitor(
                 self._cache, self._index, self._graph, node
             ).process(query_link, ids, reqs, self._ctx)

hiku/graph.py

@@ -377,7 +377,7 @@ def __init__(
         type_: t.Type[TypeRef],
         func: LinkOneFunc,
         *,
-        requires: t.Optional[str],
+        requires: t.Optional[t.Union[str, t.List[str]]],
         options: t.Optional[t.Sequence[Option]] = None,
         description: t.Optional[str] = None,
         directives: t.Optional[t.Sequence[DirectiveBase]] = None
@@ -391,7 +391,7 @@ def __init__(
         type_: t.Type[Optional],
         func: LinkMaybeFunc,
         *,
-        requires: t.Optional[str],
+        requires: t.Optional[t.Union[str, t.List[str]]],
         options: t.Optional[t.Sequence[Option]] = None,
         description: t.Optional[str] = None,
         directives: t.Optional[t.Sequence[DirectiveBase]] = None
@@ -405,7 +405,7 @@ def __init__(
         type_: t.Type[Sequence],
         func: LinkManyFunc,
         *,
-        requires: t.Optional[str],
+        requires: t.Optional[t.Union[str, t.List[str]]],
         options: t.Optional[t.Sequence[Option]] = None,
         description: t.Optional[str] = None,
         directives: t.Optional[t.Sequence[DirectiveBase]] = None

hiku/utils.py

@@ -8,6 +8,9 @@
     TypeVar,
     List,
     Iterator,
+    Dict,
+    Generic,
+    NoReturn,
 )
 
 from hiku.compat import ParamSpec
@@ -43,3 +46,23 @@ def listify(func: Callable[P, Iterator[T]]) -> Callable[P, List[T]]:
     def wrapper(*args: P.args, **kwargs: P.kwargs) -> List[T]:
         return list(func(*args, **kwargs))
     return wrapper
+
+
+K = TypeVar('K')
+V = TypeVar('V')
+
+
+class ImmutableDict(Dict, Generic[K, V]):
+    _hash = None
+
+    def __hash__(self) -> int:  # type: ignore
+        if self._hash is None:
+            self._hash = hash(frozenset(self.items()))
+        return self._hash
+
+    def _immutable(self) -> NoReturn:
+        raise TypeError("{} object is immutable"
+                        .format(self.__class__.__name__))
+
+    __delitem__ = __setitem__ = _immutable  # type: ignore
+    clear = pop = popitem = setdefault = update = _immutable  # type: ignore

hiku/validate/graph.py

@@ -133,11 +133,17 @@ def visit_link(self, obj: Link) -> None:
             )
 
         if obj.requires is not None:
-            if obj.requires not in self.ctx.fields_map:
-                self.errors.report(
-                    'Link "{}" requires missing field "{}" in the "{}" node'
-                    .format(obj.name, obj.requires, self._format_path())
-                )
+            requires = (
+                obj.requires if isinstance(obj.requires, list)
+                else [obj.requires]
+            )
+
+            for r in requires:
+                if r not in self.ctx.fields_map:
+                    self.errors.report(
+                        'Link "{}" requires missing field "{}" in the "{}" node'
+                        .format(obj.name, r, self._format_path())
+                    )
 
         self._validate_deprecated_duplicates(obj)
 

tests/test_cache.py

@@ -47,6 +47,7 @@
     CacheSettings,
     CacheInfo,
 )
+from tests.base import check_result
 
 
 class InMemoryCache(BaseCache):
@@ -591,7 +592,8 @@ def execute(q):
             }
         }
     }
-    assert execute(query) == expected_result
+
+    check_result(execute(query), expected_result)
 
     assert cache.get_many.call_count == 2
 
@@ -606,7 +608,8 @@ def execute(q):
 
     cache.reset_mock()
 
-    assert execute(query) == expected_result
+    check_result(execute(query), expected_result)
+
     cache.get_many.assert_has_calls([
         call([attributes_key]),
         call([company_key]),
@@ -770,7 +773,7 @@ def execute(q):
         }]
     }
 
-    assert execute(query) == expected_result
+    check_result(execute(query), expected_result)
 
     assert cache.get_many.call_count == 2
     calls = {
@@ -786,7 +789,7 @@ def execute(q):
 
     cache.reset_mock()
 
-    assert execute(query) == expected_result
+    check_result(execute(query), expected_result)
     assert set(*cache.get_many.mock_calls[0][1]) == {
         attributes11_12_key, attributes_none_key
     }