Support null query when running APQ request#4049
Support null query when running APQ request#4049bbakerman merged 2 commits intographql-java:masterfrom
Conversation
…g APQ request Support null query when running APQ request Full issue described in graphql-java#4008 ---- #### AI description (iteration 1) #### PR Classification Bug fix: Enhance ExecutionInput to properly handle null queries in automatic persisted query (APQ) requests. #### PR Summary This pull request modifies the ExecutionInput logic to return a persisted query marker when a null or empty query is provided along with a persistedQuery extension, ensuring graceful handling of APQ requests. - `src/main/java/graphql/ExecutionInput.java`: Introduces a new static method (`assertQuery`) to check for null/empty queries and returns a persisted query marker if appropriate; also removes the strict non-null assertion in the builder. - `src/test/groovy/graphql/ExecutionInputTest.groovy`: Adds a test case validating that a null query with the persistedQuery extension returns the expected persisted query marker. <!-- GitOpsUserAgent=GitOps.Apps.Server.pullrequestcopilot --> Related work items: #355548 # Conflicts: # src/test/groovy/graphql/ExecutionInputTest.groovy
|
Hello wanted to say, thanks so much for yet another PR! Thanks for all your time and effort |
bbakerman
left a comment
bbakerman
left a comment
There was a problem hiding this comment.
Lets decide on the API shape - specific method or extension entry ??
| def "uses persisted query marker when query is empty and extensions contains persistedQuery"() { | ||
| when: | ||
| def executionInput = ExecutionInput.newExecutionInput() | ||
| .extensions([persistedQuery: "any"]) |
There was a problem hiding this comment.
So the question then is
should we provide a helper builder method (that sets the query for you to the magic value) or use a "map entry" in extentions.
I feel like the former is better but would love to why you chose "extensions" - was there a client reason ?
Lets not do this in this PQ as suggested - I misundertood the problem being solved |
bbakerman
left a comment
bbakerman
left a comment
There was a problem hiding this comment.
I approve but I would love to see the constant and comment suggestion actioned
| def "uses persisted query marker when query is empty and extensions contains persistedQuery"() { | ||
| when: | ||
| def executionInput = ExecutionInput.newExecutionInput() | ||
| .extensions([persistedQuery: "any"]) |
There was a problem hiding this comment.
And hence to this code - leave it as it is
Applied suggestions. |
…matic Persisted Query support
| private static boolean isPersistedQuery(Builder builder) { | ||
| return builder.extensions != null && | ||
| builder.extensions.containsKey(APOLLO_AUTOMATIC_PERSISTED_QUERY_EXTENSION); | ||
| } |
3 participants
Support null query when running APQ request. This is upstream merge of our commit made by @joshjcarrier
#4008
This pull request modifies the ExecutionInput logic to return a persisted query marker when a null or empty query is provided along with a persistedQuery extension, ensuring graceful handling of APQ requests.
src/main/java/graphql/ExecutionInput.java: Introduces a new static method (assertQuery) to check for null/empty queries and returns a persisted query marker if appropriate; also removes the strict non-null assertion in the builder.src/test/groovy/graphql/ExecutionInputTest.groovy: Adds a test case validating that a null query with the persistedQuery extension returns the expected persisted query marker.