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

Automatic Persisted Queries #3118

Merged
merged 6 commits into from
May 5, 2022
Merged

Automatic Persisted Queries #3118

merged 6 commits into from
May 5, 2022

Conversation

SlavaUtesinov
Copy link
Contributor

@SlavaUtesinov SlavaUtesinov commented May 5, 2022

This PR is a preparation for implementation of Automatic Persisted Queries (APQ) in a server project PR - graphql-dotnet/server#132. Briefly, in case of APQ, a query field can be omitted, execution of a request will be implemented by means of additional hash field (sha256Hash) which corresponds to the query which has been saved on server side in advance, The request will look this way:

{
  operationName: 'MyQuery',
  variables: null,
  extensions: {
    persistedQuery: {
      version: 1,
      sha256Hash: hashOfQuery
    }
  }
}

@codecov-commenter
Copy link

Codecov Report

Merging #3118 (3976014) into master (cd42366) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #3118      +/-   ##
==========================================
+ Coverage   84.34%   84.35%   +0.01%     
==========================================
  Files         362      362              
  Lines       15884    15896      +12     
  Branches     2579     2583       +4     
==========================================
+ Hits        13397    13409      +12     
  Misses       1871     1871              
  Partials      616      616              
Impacted Files Coverage Δ
...phQL.NewtonsoftJson/GraphQLRequestJsonConverter.cs 81.35% <100.00%> (+0.99%) ⬆️
...phQL.SystemTextJson/GraphQLRequestJsonConverter.cs 84.31% <100.00%> (+0.98%) ⬆️
src/GraphQL/Transport/GraphQLRequest.cs 100.00% <100.00%> (ø)
src/GraphQL/Resolvers/FuncFieldResolver.cs 90.12% <0.00%> (+0.79%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cd42366...3976014. Read the comment docs.

@sungam3r
Copy link
Member

sungam3r commented May 5, 2022

Protocol here - https://www.apollographql.com/docs/react/api/link/persisted-queries/#protocol (Apollo JS stuff is an implementation).

@sungam3r sungam3r requested a review from Shane32 May 5, 2022 07:52
/// A Document containing GraphQL Operations and Fragments to execute (required).
/// A Document containing GraphQL Operations and Fragments to execute.
/// It can be null in case of automatic persisted queries (https://www.apollographql.com/docs/apollo-server/performance/apq/)
/// when a client sends only SHA-256 hash of the query given that corresponding key-value pair has been persisted on a server beforehand.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// when a client sends only SHA-256 hash of the query given that corresponding key-value pair has been persisted on a server beforehand.
/// when a client sends only SHA-256 hash of the query in <see cref="Extensions"/> given that corresponding key-value pair has been persisted on a server beforehand.

@sungam3r
Copy link
Member

sungam3r commented May 5, 2022

@Shane32 Note that graphql-over-http spec implicitly says that query field is required
изображение
but I think that it is not a big deal to make it optional. We are going to use APQ in production pretty soon. It's a rather simple concept, requires ~20 lines of code (in server project) and it saves a lot of network/memory/CPU reources (being used with document caching). Also note that APQ != document caching implemented in GraphQL.MemoryCache. These are diffferent features that may be used together.

@Shane32
Copy link
Member

Shane32 commented May 5, 2022

FYI, the persisted query implementation can easily be done with a custom IDocumentExecuter which essentially wraps another IDocumentExecuter. Since the implementation returns a proper GraphQL response in the case that a match is not found, I'm pretty sure it does not need to interact with the server project at all. However, the server project needs to be modified to allow requests without a query specified.

@Shane32
Copy link
Member

Shane32 commented May 5, 2022

@sungam3r I use something similar to APQ already - but only for logging purposes. Each query is hashed and logged, allowing me to compute statistics for any query over a large period of time with very little database / storage space required. Adding APQ would be rather trivial to implement, and would enhance performance.

In v6 of GraphQL we should consider allowing IConfigureExecution to 'wrap' a document executer execution, thereby being able to implement APQ, or the MemoryCache, or appending Apollo Trace statistics without having any special code within with document executer itself. Some DI engines already allow such configuration built-in, called decorators, but MSDI does not have decorator support.

@Shane32 Shane32 merged commit b51fbee into graphql-dotnet:master May 5, 2022
@sungam3r sungam3r added the enhancement New feature or request label May 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants