Use all middleware defined in decorator

[seb-b]

One thing what isn't super clear to me is how the next is handled? I'm not sure if this calls it for every iteration of the middleware loop or not until the end.

One thing that this also fixed was function style middleware never worked, it was being yielded and then the exception below it was also triggered

[seb-b]

I had a look at the graphene source, looks like they use partials to chain middleware so I've done the same, seems to work ok

[dopry]

@seb-b middleware isn't called in a loop. The intention is not to guarantee that all middlewares run. If my reading of the base code is correct this middleware implementation is a functional implementation of the Chain of Responsibility pattern. see: https://dzone.com/articles/understanding-middleware-pattern-in-expressjs for a discussion of the functional implementation of this pattern

What motivated you to create this PR? It seems like you're not getting a response you expect from the graphql interface because a middleware is breaking the chain and not calling next. Can you go into detail about the issue your encountered that inspired this solution?

[seb-b]

The motivation to create this PR was to fix issue #227, it's referenced in a commit message.

if you check the current implementation here, you can see that the middleware is called in a loop, but it returns on the first iteration. What this PR does is build up a chain of middleware and then returns the whole chain, that chain is then what is called, and it'll exit on any of the middleware if it needs to

[dopry]

The current implementation is broken. But your fix isn't quite correct either. This middleware isn't meant to be called in loop. Each middleware is responsible for calling next, a parent loop shouldn't be doing it. It should be a function chain.

I'm wondering if we should be doing something more along the lines of what is done in graphql-core

def resolve(self, next, root, info: ResolveInfo, **kwargs):
        field_name = info.field_name
        parent_name = info.parent_type.name
        if field_name in self.field_middlewares and parent_name in ROOT_TYPES:
            middlewares = self.field_middlewares[field_name].copy()
            # see: https://github.com/graphql-python/graphql-core/blob/main/src/graphql/execution/middleware.py
            middleware_manager = MiddlewareManager(middlewares)
            # see: https://github.com/graphql-python/graphql-core/blob/b43c820f9e8674eaf24f16e71beef5f613109e33/src/graphql/execution/execute.py#L514
            resolve_func = middleware_manager.get_field_resolver(next)
            # see: https://github.com/graphql-python/graphql-core/blob/b43c820f9e8674eaf24f16e71beef5f613109e33/src/graphql/execution/execute.py#L527
            return resolve_func(root, info, **kwargs)
       else:
            return next(root, info, **kwargs)

[seb-b]

The chain is built up using partials, it returns a function reference, nothing is called until the loop is exited.

I popped your implementation in my dev instance and it caused 5 tests to fail, all to do with middleware not resolving properly (authentication ones mainly). If I could get some clear picture of what is wrong with my implementation that would help me greatly in getting this over the line.

[dopry]

I don't doubt that my example may raise other issues... It is just pseudo code copied from the other code base. It is completely untested. I'll look at it more closely when I get a chance later today. Maybe I was just misreading it.

[dopry]

@seb-b I got a chance to dig into this more, and I get it... I didn't understand how the partials worked... That's my bad. Any chance you can rebase this?

[seb-b]

@dopry no worries! Just rebased