★ wanayoo — archive 1999 https://github.com/graphql-java/graphql-java/pull/1313Nouvelle recherche | Portail wanayoo
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Have a default context object included in graphql-java #1313

Merged
merged 4 commits into from Nov 23, 2018

Conversation

@bbakerman
Copy link
Member

@bbakerman bbakerman commented Nov 13, 2018

This will help encourage people to get started on a simple map like context object

Inspired by https://projectreactor.io/docs/core/milestone/api/reactor/util/context/Context.html

Out of the box there is a "context" object available. It can be replaced but you can rely on it IF you do nothing.

I have made it mutable (but threadsafe) so people can use it to pass information between fetchers. Because lots of people end up doing that

bbakerman added 2 commits Nov 13, 2018
public Builder context(Object context) {
this.context = context;
return this;
}

public Builder context(GraphQLContext.Builder contextBuilder) {

This comment has been minimized.

@tsroka

tsroka Nov 13, 2018
Member

This function does not look very useful. Is that a convention in gql-java to accept builders?

This comment has been minimized.

@bbakerman

bbakerman Nov 15, 2018
Author Member

yes it is - we could get rid of it I guess but we do this in other places

*/
@PublicApi
@SuppressWarnings("unchecked")
public class GraphQLContext {

This comment has been minimized.

@tsroka

tsroka Nov 13, 2018
Member

maybe annotate with ThreadSafe? Or add comment that it is thread safe?

This comment has been minimized.

@andimarek

andimarek Nov 23, 2018
Member

I like the idea of adding a ThreadSafe annotation.

return this;
}

public Builder context(UnaryOperator<GraphQLContext.Builder> contextBuilderFunction) {

This comment has been minimized.

@tsroka

tsroka Nov 13, 2018
Member

just wondering if it would be easier to understand the api if the method name would be something like .defaultContext in this case, as for me it would be hard to understand which one should be used: context with object or context with builder (without looking at the docs)

*/
@PublicApi
@SuppressWarnings("unchecked")
public class GraphQLContext {

This comment has been minimized.

@andimarek

andimarek Nov 23, 2018
Member

I like the idea of adding a ThreadSafe annotation.

@andimarek
Copy link
Member

@andimarek andimarek commented Nov 23, 2018

this is a small breaking change, because the context was null before but now is not anymore by default.

@bbakerman bbakerman merged commit 4ec08db into graphql-java:master Nov 23, 2018
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@bbakerman
Copy link
Member Author

@bbakerman bbakerman commented Nov 23, 2018

added new annotation as suggested

@andimarek andimarek added this to the 12.0 milestone Nov 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.