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

Persisted query support in graphql-java #2013

Merged
merged 1 commit into from
Oct 9, 2020
Merged

Conversation

bbakerman
Copy link
Member

This adds the basis for persisted query support in graphql-java.

In short consumers need to have their own cache implementation to be truly useful. The InMemoryPersistedQueryCache one is too bare bones for production use at scale, as its not memory constrained.

ApolloPersistedQuerySupport has been provided which can read persistent query ids from the input extensions.

See #1972

* @return an instance of {@link PreparsedDocumentEntry}
*/
PreparsedDocumentEntry getDocument(ExecutionInput executionInput, Function<ExecutionInput, PreparsedDocumentEntry> computeFunction);
PreparsedDocumentEntry getDocument(ExecutionInput executionInput, Function<ExecutionInput, PreparsedDocumentEntry> parseAndValidateFunction);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a much better name for the callback parameter because that's what it does

}
return Optional.empty();
}
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Later if there is a V2, we can update this

@bbakerman bbakerman added this to the 16.0 milestone Aug 28, 2020
@bbakerman bbakerman linked an issue Aug 28, 2020 that may be closed by this pull request
Copy link

@vojtapol vojtapol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change to ExecutionInput.java is all that is needed. I would be hesitant to add any Apollo-specific classes into this code. I think it should be just a part of the documentation or a small library separate from graphql-java.

For similar reasons, this library does not support Apollo-specific scalars such as Upload.

@bbakerman bbakerman merged commit 4a893b0 into master Oct 9, 2020
@vojtapol
Copy link

vojtapol commented Oct 14, 2020

Too bad it was decided to become so Apollo-dependent. It will cause issues if another client library pops up and uses a different persisted query mechanism.

@bbakerman
Copy link
Member Author

Too bad it was decided to become so Apollo-dependent. It will cause issues if another client library pops up and uses a different persisted query mechanism.

It's not Apollo dependent at all. People have always been able to build out a PreparsedDocumentProvider and so could have done this before hand.

This adds an abstract class PersistedQuerySupport that can be the basis for implementations. I happened to put in an Apollo one to be useful to the VERY popular Apollo JS client.

We have some Apollo specific code already like graphql.execution.instrumentation.tracing.TracingSupport based on it being a de facto standard at the time. This the same thing.

Some one can create new implementations, either from first principles or from extending the PersistedQuerySupport starting point.

Honestly I am not sure on the tone of your reply. At best it seems unappreciative and snarky.

@vojtapol
Copy link

vojtapol commented Oct 19, 2020

@bbakerman Thanks for the explanation. I still think that Apollo-specific code could be in a separate library as not everyone uses Apollo. But since as you say there already was some Apollo-specific code even before this PR it's too late to change that.

@mohzah
Copy link

mohzah commented Feb 18, 2021

@bbakerman Thanks for the work. I have a question, now that we have extensions and client may just pass extension (hash) w/o a query, shouldn't it allow query (in ExecutionInput) to be null?

@bbakerman
Copy link
Member Author

bbakerman commented Feb 26, 2021 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support Apollo's automatic persisted queries
3 participants