Allow context to be a function that is called per message

[NeoPhi]

The idea behind this change is to support a distinct context for each processed message. The idea is to mirror the per HTTP request GraphQL context creation flow. Currently the context is fixed on the SubscriptionManager when the initial publish is created and is also used to create the triggerMap. The API change adds createContext: Function to SubscriptionOptions. If specified the new context is passed to filter and onMessage.

[helfer]

@NeoPhi Do you have a specific use-case that this is necessary for?

[NeoPhi]

Our standard HTTP GraphQL flow uses per request context creation to handle authorization and caching. The current onSubscribe semantics creates the context once and it is shared for all messages that are later processed. Given that messages may occur at various times in the future we would want to create the context for each message to avoid any long lived state in the context.

For example before processing a messages we would want to check the user's session to see what their current authorization is. For a standard GraphQL request we do this as part of the per request context creation and would like to use similar semantics when handling subscription message processing.

[helfer]

@NeoPhi I'm not sure how that per-message context would help in your case, because the client only sends a subscribe and an unsubscribe message, and nothing in between. In a sense, a subscription is a single request. For the lifetime of a subscription, I don't think its auth state should ever change. It's better to tear down the old subscription and make a new one.
I'm also skeptical when it comes to creating the context based on the payload of the event. And once you take that out of the equation, the options to the createContext function wouldn't change over time, so you might as well store the value.

TLDR: if it's just for auth, I don't think it makes sense to have a per-event context. Are there other things that you put on the context that actually have to change?

[stubailo]

I would store a connector/dataloader instance for every subscription execution.

[NeoPhi]

The issue is that to store a dataloader on the originally returned context then I need to worry about invalidating that cache between message executions. We also store a normalized time on the context so that all resolvers have the same value of now for calculating data.

[stubailo]

Yep - I'm agreeing with you @NeoPhi, I think generating a new context per execution is a good idea.

[helfer]

Ok, that makes sense.

I'd rather have everything in one function though, can you modify the PR to just do all the work in the onMessage function?

I'd also rather not have too many options, so we we could just overload the context argument, and make it either an object, a function, or a function that returns a promise. That would be similar to how express-graphql does it, so I think it would be fine.

Thoughts?

[stubailo]

Yeah, onMessage sounds like a good place to put it.

[NeoPhi]

Just to confirm, do you mean combining the filter and execute all into onMessage? Since one reason to create the per message context prior to the filter is that no data may need to be sent based on the filter logic.

[helfer]

Yeah, I think if you want the context in the filter, then I'd move that into onMessage. Do you want the context in the filter so you can check permissions before running the query?

[helfer]

Looks good! I'm still not 100% sure we need the context in the filter function, but it probably doesn't hurt, so let's go with it.