Gqlgen: Subscriptions don't support directives

[jekaspekas]

Expected Behaviour

Directive should affect the subscriptions. At example below not authenticated user should not receive data about created messages.

Actual Behavior

Query and Mutation support directives. And only authenticated users can read and create messages.
Subscriptions don't support directives. And all users can receive data about created messages.

Minimal graphql.schema and models to reproduce

schema {
    query: Query
    mutation: Mutation
    subscription: Subscription
}
type Query {
    message(id: ID!): Message @isAuthenticated
}
type Mutation {
    messageCreate(subject: String!, text: String!): Message @isAuthenticated
}
type Subscription {
    messageCreated: Message! @isAuthenticated
}
type Message {
    messageID: ID!
    subject: String!
    text: String!
}
directive @isAuthenticated on FIELD_DEFINITION

[ghost]

I noticed the same behaviour.
Curious what the situation is as I wanted to extend the system to support delivering subscription messages to clients after they come back online using NATS or the Database. Which would mean changing some codegen aspects I presume

Hope you don't mind me adding this context to this issue.

[jekaspekas]

Solved!
I changed this standard code of the generated server:

func (ec *executionContext) _Subscription_messageCreated(ctx context.Context, field graphql.CollectedField) func() graphql.Marshaler {
    ctx = graphql.WithResolverContext(ctx, &graphql.ResolverContext{Field: field})
    results, err := ec.resolvers.Subscription().MessageCreated(ctx)
    if err != nil {
        ec.Error(ctx, err)
        return nil
    }
    return func() graphql.Marshaler {
        res, ok := <-results
        if !ok {
            return nil
        }
        var out graphql.OrderedMap
        out.Add(field.Alias, func() graphql.Marshaler { return ec._Message(ctx, field.Selections, &res) }())
        return &out
    }
}

to this code

func (ec *executionContext) _Subscription_messageCreated(ctx context.Context, field graphql.CollectedField) func() graphql.Marshaler {
    ctx = graphql.WithResolverContext(ctx, &graphql.ResolverContext{Field: field})

    resTmp := ec.FieldMiddleware(ctx, func(ctx context.Context) (interface{}, error) {
        return ec.resolvers.Subscription().MessageCreated(ctx)
    })
    if resTmp == nil {
        return nil
    }
    results := resTmp.(<-chan models.Message)
    if results == nil {
        return nil
    }

    return func() graphql.Marshaler {
        res, ok := <-results
        if !ok {
            return nil
        }
        var out graphql.OrderedMap
        out.Add(field.Alias, func() graphql.Marshaler { return ec._Message(ctx, field.Selections, &res) }())
        return &out
    }
}

@vektah can you think about changing the generated server template???

[tgwizard]

Bump on this - is has there been some discussion on how to solve this? We currently use the RequestMiddleware to setup dataloaders in the context. This seems to not work for resolvers used as part of resolving a subscription type, and I found a comment in the code linking to this issue.

It seems to me that a new kind of middleware could work, perhaps a SubscriptionMessageMiddleware, which is run after reading from the channel returned by the subscription resolver, but before marshalling and resolving any further sub objects. Does that make sense? If so, I'd love to submit a PR.

[derberg]

Hello @vektah
Any ideas why this is stale? some help is needed?

[derberg]

Hey @vektah
Sorry for stalking but would be awesome if you could share some details on that issue

[vektah]

It just needs someone to think it through and do the work.

I changed this standard code of the generated server:

I don't think its that easy, thats just passing the channel through the middleware. most middleware (field/object/etc) shouldn't know they are being called on a subscription, they should just get the value, repeatedly as it comes through.

SubscriptionMessageMiddleware

That doesn't really solve the problem. Things like authentication middleware shouldn't need to be implemented twice.

[derberg]

@vektah I guess this issue is covered by the latest release https://github.com/99designs/gqlgen/releases/tag/v0.9.1 right?

[robbert229]

@derberg This was not fixed in 0.9.1. I just tested this out.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.