Add normalized document provider to allow caching of normalized documents by timward60 · Pull Request #4048 · graphql-java/graphql-java
This PR enables:
- The support of the
NormalizedDocumentimplementation behind an experimental API configuration. - A normalized document provider that allows these normalized documents to be cached.
There a few things that could be improved, but would require larger more broader changes, including:
- The calculation of the normalized document is tied to the lazy creation of the selection set. This call path is not using completable futures, which means that normalized document providers that need to make async calls (e.g to distributed cache) will block the calling thread. This is less of an issue if we were using virtual threads but that support is not guranteed by all consumers of the library.
- The abstraction requires the caller to create the caching key, which is acceptable, but it would be more ideal if we could provide some helpers to help calculate a key (for example if we rely on distributed caching, then the serialized output of the normalized document could be version dependent). By providing some incremented normalized document version, then the GraphQL-Java library can make changes with the only consequence being that any distributed cache backed implementations would cause the cache to be explicitely repopulated.
timward60
changed the title
[WIP] Add normalized document provider to allow caching of normalized documents
Add normalized document provider to allow caching of normalized documents
timward60
marked this pull request as ready for review
| this.executionInput = builder.executionInput; | ||
| this.dataLoaderDispatcherStrategy = builder.dataLoaderDispatcherStrategy; | ||
| this.queryTree = FpKit.interThreadMemoize(() -> ExecutableNormalizedOperationFactory.createExecutableNormalizedOperation(graphQLSchema, operationDefinition, fragmentsByName, coercedVariables)); | ||
| this.queryTree = FpKit.interThreadMemoize(() -> this. createGraphQLNormalizedOperation().join()); |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Super nit: there's a space after this.
Hello, this pull request has been inactive for 60 days, so we're marking it as stale. If you would like to continue working on this pull request, please make an update within the next 30 days, or we'll close the pull request.
Hello, as this pull request has been inactive for 90 days, we're closing this pull request. We always welcome contributions, and if you would like to continue, please open a new pull request.