Default response content type using GraphQL spec #35769

ivanp · 2023-09-06T09:02:11Z

So the default content type header was not recognized properly by GraphQL gateway. I tried to submit PR but they said it doesn't conform to the GraphQL spec.

Default response content type header should conform to [GraphQL over HTTP spec](https://github.com/graphql/graphql-over-http/blob/main/spec/GraphQLOverHTTP.md#serialization-format).

phillip-kruger · 2023-09-08T04:42:24Z

@jmartisk can you have look ?

jmartisk

Tests are failing - you need to also update https://github.com/quarkusio/quarkus/blob/3.4.0.CR1/extensions/smallrye-graphql/runtime/src/main/java/io/quarkus/smallrye/graphql/runtime/SmallRyeGraphQLExecutionHandler.java#L299 - please update it to accept both the new (fixed) content-type and the original one, for backward compatibility

gsmet · 2023-10-05T14:56:37Z

@jmartisk this hasn't made any progress and I think it's really something that needs fixing. Could you have a look? Thanks.

jmartisk · 2023-10-06T07:22:10Z

I've added a commit that hopefully completes the fix

gsmet · 2023-10-06T09:00:18Z

...ntime/src/main/java/io/quarkus/smallrye/graphql/runtime/SmallRyeGraphQLExecutionHandler.java

@@ -297,7 +297,8 @@ private static String getCharset(String mimeType) {
    private boolean isValidAcceptRequest(String mimeType) {
        // At this point we only accept two


Change looks good to fix the tests but this comment looks outdated. I wonder if we should explain a bit more and put the two valid ones first and the last one with a comment // deprecated content-type supported for compatibility.

Ah, that's a very good observation, thanks. Fixed

quarkus-bot · 2023-10-06T10:48:21Z

✔️ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

jmartisk · 2023-10-11T06:45:29Z

@gsmet can you please have another look? I would merge it but I can't as long as there's a -1 from you :)

gsmet

Yeah sorry, I was busy on other things, let's merge!

This MR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [flow-bin](https://github.com/flowtype/flow-bin) ([changelog](https://github.com/facebook/flow/blob/master/Changelog.md)) | devDependencies | minor | [`^0.217.0` -> `^0.219.0`](https://renovatebot.com/diffs/npm/flow-bin/0.217.2/0.219.0) | | [io.hypersistence:hypersistence-utils-hibernate-62](https://github.com/vladmihalcea/hypersistence-utils) | compile | minor | `3.5.3` -> `3.6.0` | | [io.quarkus:quarkus-maven-plugin](https://github.com/quarkusio/quarkus) | build | patch | `3.4.2` -> `3.4.3` | | [io.quarkus:quarkus-universe-bom](https://github.com/quarkusio/quarkus-platform) | import | patch | `3.4.2` -> `3.4.3` | --- ### Release Notes <details> <summary>flowtype/flow-bin</summary> ### [`v0.219.0`](flow/flow-bin@1268ec5...c184c5d) [Compare Source](flow/flow-bin@1268ec5...c184c5d) ### [`v0.218.1`](flow/flow-bin@12afce4...1268ec5) [Compare Source](flow/flow-bin@12afce4...1268ec5) ### [`v0.218.0`](flow/flow-bin@dc93913...12afce4) [Compare Source](flow/flow-bin@dc93913...12afce4) </details> <details> <summary>vladmihalcea/hypersistence-utils</summary> ### [`v3.6.0`](https://github.com/vladmihalcea/hypersistence-utils/blob/HEAD/changelog.txt#Version-360---October-12-2023) \================================================================================ Implement QueryStackTraceLogger using StackWalker [#659](vladmihalcea/hypersistence-utils#659) Add JFR based query logger [#658](vladmihalcea/hypersistence-utils#658) Adds support for using MonetaryAmount in [@ElementCollection](https://github.com/ElementCollection) [#652](vladmihalcea/hypersistence-utils#652) </details> <details> <summary>quarkusio/quarkus</summary> ### [`v3.4.3`](https://github.com/quarkusio/quarkus/releases/tag/3.4.3) [Compare Source](quarkusio/quarkus@3.4.2...3.4.3) ##### Complete changelog - [#36408](quarkusio/quarkus#36408) - Ensure that SSE builder works in native - [#36404](quarkusio/quarkus#36404) - Do not exclude properties from recording that are available in sources that should always be included - [#36403](quarkusio/quarkus#36403) - IBM Db2 - Register resource bundle classes for reflection - [#36402](quarkusio/quarkus#36402) - Native Picocli build breaks SSE client - [#36399](quarkusio/quarkus#36399) - quarkus-jdbc-db2: resource bundle missing - [#36377](quarkusio/quarkus#36377) - Allow `@ClientHeaderParam` to override User-Agent - [#36371](quarkusio/quarkus#36371) - Fix issue in Java migration in dev-mode - [#36351](quarkusio/quarkus#36351) - Properly handle invalid response body errors in Reactive REST Client - [#36329](quarkusio/quarkus#36329) - Custom User-Agent header ignored - [#36326](quarkusio/quarkus#36326) - Cannot load fixed or default YAML configuration when running native build - [#36302](quarkusio/quarkus#36302) - Fix headers and preambles in all guides and reintroduce some keywords - [#36299](quarkusio/quarkus#36299) - quarkus-flyway: Java-Migration does not get picked up on live reload - [#36290](quarkusio/quarkus#36290) - Fixed URL for configuring JSON support - [#36257](quarkusio/quarkus#36257) - Rest client call hangs when receiving an invalid chunked response and does not release resources (e.g. Bulkhead semaphore) - [#36147](quarkusio/quarkus#36147) - Bump org.eclipse.parsson:parsson from 1.1.2 to 1.1.4 - [#36096](quarkusio/quarkus#36096) - Build cache - Use notCacheableBecause instead of storeEnabled - [#35929](quarkusio/quarkus#35929) - Do not store build cache for core extensions having config - [#35927](quarkusio/quarkus#35927) - Build cache - Core extensions containing config shouldn't be cached - [#35769](quarkusio/quarkus#35769) - Default response content type using GraphQL spec </details> <details> <summary>quarkusio/quarkus-platform</summary> ### [`v3.4.3`](quarkusio/quarkus-platform@3.4.2...3.4.3) [Compare Source](quarkusio/quarkus-platform@3.4.2...3.4.3) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever MR is behind base branch, or you tick the rebase/retry checkbox. 👻 **Immortal**: This MR will be recreated if closed unmerged. Get [config help](https://github.com/renovatebot/renovate/discussions) if that's undesired. --- - [ ] If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

Default response content type using GraphQL spec

bff1fb7

Default response content type header should conform to [GraphQL over HTTP spec](https://github.com/graphql/graphql-over-http/blob/main/spec/GraphQLOverHTTP.md#serialization-format).

quarkus-bot bot added area/graphql area/smallrye labels Sep 6, 2023

This comment has been minimized.

Sign in to view

jmartisk requested changes Sep 12, 2023

View reviewed changes

jmartisk added triage/backport? triage/backport-3.2 labels Oct 6, 2023

This comment has been minimized.

Sign in to view

gsmet requested changes Oct 6, 2023

View reviewed changes

Accept requests that ask for application/graphql-response+json

5550b70

jmartisk force-pushed the patch-1 branch from 24303b0 to 5550b70 Compare October 6, 2023 09:12

jmartisk approved these changes Oct 6, 2023

View reviewed changes

phillip-kruger approved these changes Oct 7, 2023

View reviewed changes

gsmet added the triage/backport? label Oct 11, 2023

gsmet approved these changes Oct 11, 2023

View reviewed changes

gsmet merged commit 3d81f91 into quarkusio:main Oct 11, 2023
21 checks passed

quarkus-bot bot added this to the 3.5 - main milestone Oct 11, 2023

gsmet modified the milestones: 3.5.0.CR1, 3.6 - main, 3.4.3 Oct 11, 2023

gsmet removed the triage/backport-3.4? label Oct 11, 2023

dariuszkuc mentioned this pull request Oct 13, 2023

feat: Extend support for content types that include JSON apollographql/federation#2813

Closed

gsmet removed the triage/backport? label Oct 17, 2023

aloubyansky removed this from the 3.4.3 milestone Nov 2, 2023

aloubyansky added this to the 3.2.8.Final milestone Nov 2, 2023

aloubyansky removed the triage/backport-3.2 label Nov 2, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Default response content type using GraphQL spec #35769

Default response content type using GraphQL spec #35769

ivanp commented Sep 6, 2023

phillip-kruger commented Sep 8, 2023

This comment has been minimized.

jmartisk left a comment

gsmet commented Oct 5, 2023

jmartisk commented Oct 6, 2023

This comment has been minimized.

gsmet Oct 6, 2023

jmartisk Oct 6, 2023

quarkus-bot bot commented Oct 6, 2023

jmartisk commented Oct 11, 2023

gsmet left a comment

		@@ -297,7 +297,8 @@ private static String getCharset(String mimeType) {
		private boolean isValidAcceptRequest(String mimeType) {
		// At this point we only accept two

Default response content type using GraphQL spec #35769

Default response content type using GraphQL spec #35769

Conversation

ivanp commented Sep 6, 2023

phillip-kruger commented Sep 8, 2023

This comment has been minimized.

jmartisk left a comment

Choose a reason for hiding this comment

gsmet commented Oct 5, 2023

jmartisk commented Oct 6, 2023

This comment has been minimized.

gsmet Oct 6, 2023

Choose a reason for hiding this comment

jmartisk Oct 6, 2023

Choose a reason for hiding this comment

quarkus-bot bot commented Oct 6, 2023

jmartisk commented Oct 11, 2023

gsmet left a comment

Choose a reason for hiding this comment