Some clean-up in middleware implementation

Cito · Cito · commit 2865994b356e · 2018-09-16T00:17:19.000+02:00
diff --git a/README.md b/README.md
@@ -13,7 +13,7 @@ a query language for APIs created by Facebook.
 
 The current version 1.0.1 of GraphQL-core-next is up-to-date with GraphQL.js
 version 14.0.2. All parts of the API are covered by an extensive test suite of
-currently 1606 unit tests.
+currently 1609 unit tests.
 
 
 ## Documentation
diff --git a/graphql/execution/__init__.py b/graphql/execution/__init__.py
@@ -5,21 +5,13 @@
 """
 
 from .execute import (
-    execute,
-    default_field_resolver,
-    response_path_as_list,
-    ExecutionContext,
-    ExecutionResult,
-)
+    execute, default_field_resolver, response_path_as_list,
+    ExecutionContext, ExecutionResult, Middleware)
 from .middleware import MiddlewareManager
 from .values import get_directive_values
 
 __all__ = [
-    "execute",
-    "default_field_resolver",
-    "response_path_as_list",
-    "ExecutionContext",
-    "ExecutionResult",
-    "MiddlewareManager",
-    "get_directive_values",
-]
+    'execute', 'default_field_resolver', 'response_path_as_list',
+    'ExecutionContext', 'ExecutionResult',
+    'Middleware', 'MiddlewareManager',
+    'get_directive_values']
diff --git a/graphql/execution/execute.py b/graphql/execution/execute.py
@@ -8,6 +8,7 @@
     DocumentNode, FieldNode, FragmentDefinitionNode,
     FragmentSpreadNode, InlineFragmentNode, OperationDefinitionNode,
     OperationType, SelectionSetNode)
+from .middleware import MiddlewareManager
 from ..pyutils import is_invalid, is_nullish, MaybeAwaitable
 from ..utilities import get_operation_root_type, type_from_ast
 from ..type import (
@@ -20,13 +21,11 @@
     is_non_null_type, is_object_type)
 from .values import (
     get_argument_values, get_directive_values, get_variable_values)
-from .middleware import MiddlewareManager
-
 
 __all__ = [
     'add_path', 'assert_valid_execution_arguments', 'default_field_resolver',
     'execute', 'get_field_def', 'response_path_as_list',
-    'ExecutionResult', 'ExecutionContext']
+    'ExecutionResult', 'ExecutionContext', 'Middleware']
 
 
 # Terminology
@@ -61,13 +60,16 @@ class ExecutionResult(NamedTuple):
 
 ExecutionResult.__new__.__defaults__ = (None, None)  # type: ignore
 
+Middleware = Optional[Union[Iterable[Any], MiddlewareManager]]
+
 
 def execute(
         schema: GraphQLSchema, document: DocumentNode,
         root_value: Any=None, context_value: Any=None,
         variable_values: Dict[str, Any]=None,
-        operation_name: str=None, field_resolver: GraphQLFieldResolver=None,
-        middleware: Optional[Union[Iterable[Any], MiddlewareManager]]=None
+        operation_name: str=None,
+        field_resolver: GraphQLFieldResolver=None,
+        middleware: Middleware=None
         ) -> MaybeAwaitable[ExecutionResult]:
     """Execute a GraphQL operation.
 
@@ -151,7 +153,7 @@ def build(
             raw_variable_values: Dict[str, Any]=None,
             operation_name: str=None,
             field_resolver: GraphQLFieldResolver=None,
-            middleware: Optional[Union[Iterable[Any], MiddlewareManager]]=None
+            middleware: Middleware=None
             ) -> Union[List[GraphQLError], 'ExecutionContext']:
         """Build an execution context
 
@@ -165,16 +167,16 @@ def build(
         has_multiple_assumed_operations = False
         fragments: Dict[str, FragmentDefinitionNode] = {}
         middleware_manager: Optional[MiddlewareManager] = None
-        if middleware:
+        if middleware is not None:
             if isinstance(middleware, Iterable):
                 middleware_manager = MiddlewareManager(*middleware)
             elif isinstance(middleware, MiddlewareManager):
                 middleware_manager = middleware
             else:
                 raise TypeError(
-                    f"middlewares have to be an instance"
-                    "of MiddlewareManager. Received \"{middleware}\""
-                )
+                    "Middleware must be passed as a sequence of functions"
+                    " or objects, or as a single MiddlewareManager object."
+                    f" Got {middleware!r} instead.")
 
         for definition in document.definitions:
             if isinstance(definition, OperationDefinitionNode):
diff --git a/graphql/execution/middleware.py b/graphql/execution/middleware.py
@@ -1,76 +1,74 @@
-from typing import Callable, Iterator, Dict, Tuple, Any, Iterable, Optional, cast
-
-from inspect import isfunction
 from functools import partial
+from inspect import isfunction
 from itertools import chain
 
+from typing import (
+    Callable, Iterator, Dict, Tuple, Any, Iterable, Optional, cast)
 
-from ..type import GraphQLFieldResolver
-
+__all__ = ['MiddlewareManager']
 
-__all__ = ["MiddlewareManager", "middlewares"]
-
-# If the provided middleware is a class, this is the attribute we will look at
-MIDDLEWARE_RESOLVER_FUNCTION = "resolve"
+GraphQLFieldResolver = Callable[..., Any]
 
 
 class MiddlewareManager:
-    """MiddlewareManager helps to chain resolver functions with the provided
-    middleware functions and classes
+    """Manager for the middleware chain.
+
+    This class helps to wrap resolver functions with the provided middleware
+    functions and/or objects. The functions take the next middleware function
+    as first argument. If middleware is provided as an object, it must provide
+    a method 'resolve' that is used as the middleware function.
     """
 
-    __slots__ = ("middlewares", "_middleware_resolvers", "_cached_resolvers")
+    __slots__ = 'middlewares', '_middleware_resolvers', '_cached_resolvers'
 
     _cached_resolvers: Dict[GraphQLFieldResolver, GraphQLFieldResolver]
-    _middleware_resolvers: Optional[Tuple[Callable, ...]]
+    _middleware_resolvers: Optional[Iterator[Callable]]
 
     def __init__(self, *middlewares: Any) -> None:
         self.middlewares = middlewares
-        if middlewares:
-            self._middleware_resolvers = tuple(get_middleware_resolvers(middlewares))
-        else:
-            self.__middleware_resolvers = None
+        self._middleware_resolvers = get_middleware_resolvers(
+            middlewares) if middlewares else None
         self._cached_resolvers = {}
 
     def get_field_resolver(
-        self, field_resolver: GraphQLFieldResolver
-    ) -> GraphQLFieldResolver:
-        """Wraps the provided resolver returning a function that
-        executes chains the middleware functions with the resolver function"""
+            self, field_resolver: GraphQLFieldResolver
+            ) -> GraphQLFieldResolver:
+        """Wrap the provided resolver with the middleware.
+
+        Returns a function that chains the middleware functions with the
+        provided resolver function
+        """
         if self._middleware_resolvers is None:
             return field_resolver
         if field_resolver not in self._cached_resolvers:
             self._cached_resolvers[field_resolver] = middleware_chain(
-                field_resolver, self._middleware_resolvers
-            )
-
+                field_resolver, self._middleware_resolvers)
         return self._cached_resolvers[field_resolver]
 
 
-middlewares = MiddlewareManager
-
-
-def get_middleware_resolvers(middlewares: Tuple[Any, ...]) -> Iterator[Callable]:
-    """Returns the functions related to the middleware classes or functions"""
+def get_middleware_resolvers(
+        middlewares: Tuple[Any, ...]) -> Iterator[Callable]:
+    """Get a list of resolver functions from a list of classes or functions."""
     for middleware in middlewares:
-        # If the middleware is a function instead of a class
         if isfunction(middleware):
             yield middleware
-        resolver_func = getattr(middleware, MIDDLEWARE_RESOLVER_FUNCTION, None)
-        if resolver_func is not None:
-            yield resolver_func
+        else:  # middleware provided as object with 'resolve' method
+            resolver_func = getattr(middleware, 'resolve', None)
+            if resolver_func is not None:
+                yield resolver_func
 
 
 def middleware_chain(
-    func: GraphQLFieldResolver, middlewares: Iterable[Callable]
-) -> GraphQLFieldResolver:
-    """Reduces the current function with the provided middlewares,
-    returning a new resolver function"""
+        func: GraphQLFieldResolver, middlewares: Iterable[Callable]
+        ) -> GraphQLFieldResolver:
+    """Chain the given function with the provided middlewares.
+
+    Returns a new resolver function that is the chain of both.
+    """
     if not middlewares:
         return func
     middlewares = chain((func,), middlewares)
     last_func: Optional[GraphQLFieldResolver] = None
     for middleware in middlewares:
         last_func = partial(middleware, last_func) if last_func else middleware
-
     return cast(GraphQLFieldResolver, last_func)
diff --git a/graphql/graphql.py b/graphql/graphql.py
@@ -1,27 +1,26 @@
 from asyncio import ensure_future
 from inspect import isawaitable
-from typing import Any, Awaitable, Callable, Dict, Union, Optional, Iterable, cast
+from typing import Any, Awaitable, Callable, Dict, Union, cast
 
 from .error import GraphQLError
-from .execution import execute
+from .execution import execute, ExecutionResult, Middleware
 from .language import parse, Source
 from .pyutils import MaybeAwaitable
 from .type import GraphQLSchema, validate_schema
-from .execution import ExecutionResult, MiddlewareManager
 
-__all__ = ["graphql", "graphql_sync"]
+__all__ = ['graphql', 'graphql_sync']
 
 
 async def graphql(
-    schema: GraphQLSchema,
-    source: Union[str, Source],
-    root_value: Any = None,
-    context_value: Any = None,
-    variable_values: Dict[str, Any] = None,
-    operation_name: str = None,
-    field_resolver: Callable = None,
-    middleware: Optional[Union[Iterable[Any], MiddlewareManager]] = None,
-) -> ExecutionResult:
+        schema: GraphQLSchema,
+        source: Union[str, Source],
+        root_value: Any=None,
+        context_value: Any=None,
+        variable_values: Dict[str, Any]=None,
+        operation_name: str=None,
+        field_resolver: Callable=None,
+        middleware: Middleware=None
+        ) -> ExecutionResult:
     """Execute a GraphQL operation asynchronously.
 
     This is the primary entry point function for fulfilling GraphQL operations
@@ -70,8 +69,7 @@ async def graphql(
         variable_values,
         operation_name,
         field_resolver,
-        middleware,
-    )
+        middleware)
 
     if isawaitable(result):
         return await cast(Awaitable[ExecutionResult], result)
@@ -80,15 +78,15 @@ async def graphql(
 
 
 def graphql_sync(
-    schema: GraphQLSchema,
-    source: Union[str, Source],
-    root_value: Any = None,
-    context_value: Any = None,
-    variable_values: Dict[str, Any] = None,
-    operation_name: str = None,
-    field_resolver: Callable = None,
-    middleware: Optional[Union[Iterable[Any], MiddlewareManager]] = None,
-) -> ExecutionResult:
+        schema: GraphQLSchema,
+        source: Union[str, Source],
+        root_value: Any=None,
+        context_value: Any=None,
+        variable_values: Dict[str, Any]=None,
+        operation_name: str=None,
+        field_resolver: Callable=None,
+        middleware: Middleware=None
+        ) -> ExecutionResult:
     """Execute a GraphQL operation synchronously.
 
     The graphql_sync function also fulfills GraphQL operations by parsing,
@@ -104,27 +102,26 @@ def graphql_sync(
         variable_values,
         operation_name,
         field_resolver,
-        middleware,
-    )
+        middleware)
 
     # Assert that the execution was synchronous.
     if isawaitable(result):
         ensure_future(cast(Awaitable[ExecutionResult], result)).cancel()
-        raise RuntimeError("GraphQL execution failed to complete synchronously.")
+        raise RuntimeError(
+            "GraphQL execution failed to complete synchronously.")
 
     return cast(ExecutionResult, result)
 
 
 def graphql_impl(
-    schema,
-    source,
-    root_value,
-    context_value,
-    variable_values,
-    operation_name,
-    field_resolver,
-    middleware,
-) -> MaybeAwaitable[ExecutionResult]:
+        schema,
+        source,
+        root_value,
+        context_value,
+        variable_values,
+        operation_name,
+        field_resolver,
+        middleware) -> MaybeAwaitable[ExecutionResult]:
     """Execute a query, return asynchronously only if necessary."""
     # Validate Schema
     schema_validation_errors = validate_schema(schema)
@@ -142,7 +139,6 @@ def graphql_impl(
 
     # Validate
     from .validation import validate
-
     validation_errors = validate(schema, document)
     if validation_errors:
         return ExecutionResult(data=None, errors=validation_errors)
@@ -156,5 +152,4 @@ def graphql_impl(
         variable_values,
         operation_name,
         field_resolver,
-        middleware,
-    )
+        middleware)
diff --git a/tests/execution/test_middleware.py b/tests/execution/test_middleware.py
diff --git a/tests/type/test_validation.py b/tests/type/test_validation.py
