Skip to content

Add normalized document provider to allow caching of normalized documents #4048

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

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

timward60
Copy link
Contributor

@timward60 timward60 commented Jul 11, 2025

This PR enables:

  • The support of the NormalizedDocument implementation 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 timward60 force-pushed the timward-normalized-document-caching branch from da290e2 to bdc9f7c Compare July 11, 2025 04:41
@timward60 timward60 force-pushed the timward-normalized-document-caching branch from 86bb621 to fbb6e8b Compare July 16, 2025 04:39
@timward60 timward60 force-pushed the timward-normalized-document-caching branch from 16b0262 to 8e2e760 Compare July 20, 2025 14:50
@timward60 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 Jul 20, 2025
@timward60 timward60 marked this pull request as ready for review July 20, 2025 15:33
@timward60 timward60 marked this pull request as draft July 20, 2025 15:58
this.errors.set(builder.errors);
this.localContext = builder.localContext;
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());
Copy link
Member

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.

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.

3 participants