chore: bump gqlgen to v0.17.76 #1320
Conversation
This version fixes "make generate" and matches the version used in cosmo.
ysmolski
requested review from
a team,
Noroth,
StarpTech,
devsergiy and
jensneuse
as code owners
WalkthroughDependency version bumps and tooling updates; regenerated gqlgen artifacts (headers and added root model types/JSON methods); websocket InitFunc signature change; GraphQL server construction switched to handler.New with added transports/introspection; several v2 stringer/index refactors; minor mock header updates and small test timing sleeps. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (8)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (3)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
examples/federation/go.mod (1)
69-69: Align gqlgen version pinning
examples/federation/go.mod declaresrequire github.com/99designs/gqlgen v0.17.76but replaces it withv0.17.22(generated code targets v0.17.22). Either regenerate the schema and handlers with v0.17.76 and drop the replace directive, or intentionally pin both require and replace to v0.17.22 for consistency.
🧹 Nitpick comments (2)
execution/subscription/legacy_handler_test.go (1)
457-457: Consider usingrequire.Eventuallyfor more robust async testing.The fixed sleep addresses the timing issue pragmatically, but polling would be more reliable across different environments.
For example, you could replace the sleep with:
- time.Sleep(10 * time.Millisecond) - messagesFromServer := client.readFromServer() + require.Eventually(t, func() bool { + messagesFromServer := client.readFromServer() + for _, msg := range messagesFromServer { + if msg.Id == "1" && msg.Type == MessageTypeComplete { + return true + } + } + return false + }, 1*time.Second, 5*time.Millisecond) + messagesFromServer := client.readFromServer()That said, the current approach is acceptable for a dependency update PR.
execution/go.mod (1)
79-83: Consider removing commented-out code.The commented-out replace directive and its explanation can be removed now that the version conflict is resolved by upgrading gqlgen to v0.17.76.
Apply this diff to remove the obsolete comments and commented-out code:
-// cosmo/router dependency uses indirect dependency of gqlgen of version v0.17.39 -// code in this workspace uses v0.17.22 -// this is a workaround to make sure that the correct version is used -// as we cannot pin the specific version in go mod -//replace github.com/99designs/gqlgen => github.com/99designs/gqlgen v0.17.22 -
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (11)
examples/federation/go.sumis excluded by!**/*.sumexecution/federationtesting/accounts/graph/generated/federation.gois excluded by!**/generated/**execution/federationtesting/accounts/graph/generated/generated.gois excluded by!**/generated/**execution/federationtesting/products/graph/generated/federation.gois excluded by!**/generated/**execution/federationtesting/products/graph/generated/generated.gois excluded by!**/generated/**execution/federationtesting/reviews/graph/generated/federation.gois excluded by!**/generated/**execution/federationtesting/reviews/graph/generated/generated.gois excluded by!**/generated/**execution/go.sumis excluded by!**/*.sumgo.workis excluded by!**/*.workgo.work.sumis excluded by!**/*.sumv2/go.sumis excluded by!**/*.sum
📒 Files selected for processing (26)
examples/federation/go.mod(3 hunks)execution/federationtesting/accounts/graph/entity.resolvers.go(1 hunks)execution/federationtesting/accounts/graph/model/models_gen.go(9 hunks)execution/federationtesting/accounts/graph/schema.resolvers.go(1 hunks)execution/federationtesting/products/graph/entity.resolvers.go(1 hunks)execution/federationtesting/products/graph/handler.go(1 hunks)execution/federationtesting/products/graph/model/models_gen.go(2 hunks)execution/federationtesting/products/graph/schema.resolvers.go(1 hunks)execution/federationtesting/reviews/graph/entity.resolvers.go(1 hunks)execution/federationtesting/reviews/graph/model/models_gen.go(2 hunks)execution/federationtesting/reviews/graph/schema.resolvers.go(1 hunks)execution/go.mod(3 hunks)execution/subscription/engine_mock_test.go(1 hunks)execution/subscription/executor_mock_test.go(1 hunks)execution/subscription/handler_mock_test.go(1 hunks)execution/subscription/legacy_handler_test.go(1 hunks)execution/subscription/transport_client_mock_test.go(1 hunks)execution/subscription/websocket/engine_mock_test.go(1 hunks)v2/go.mod(2 hunks)v2/pkg/ast/ast_string.go(6 hunks)v2/pkg/ast/directive_location_string.go(1 hunks)v2/pkg/astvalidation/validation_state_string.go(1 hunks)v2/pkg/introspection/introspection_enum.go(2 hunks)v2/pkg/lexer/identkeyword/identkeyword_string.go(1 hunks)v2/pkg/lexer/keyword/keyword_string.go(1 hunks)v2/pkg/testing/subscriptiontesting/models_gen.go(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-29T09:35:47.969Z
Learnt from: ysmolski
PR: wundergraph/graphql-go-tools#1282
File: v2/pkg/engine/plan/visitor.go:5-5
Timestamp: 2025-08-29T09:35:47.969Z
Learning: The wundergraph/graphql-go-tools project does not support Go versions < 1.23, so compatibility concerns for features available in Go 1.21+ (like cmp.Or) should not be raised.
Applied to files:
examples/federation/go.modv2/go.mod
🧬 Code graph analysis (1)
execution/federationtesting/products/graph/handler.go (1)
pkg/subscription/websocket/init.go (1)
InitFunc(10-10)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build and test (go 1.25 / windows-latest)
- GitHub Check: Build and test (go 1.25 / ubuntu-latest)
- GitHub Check: Build and test (go 1.25 / windows-latest)
🔇 Additional comments (21)
v2/pkg/ast/directive_location_string.go (1)
38-42: LGTM! Standard stringer-generated pattern.The refactored indexing logic using
idx := int(i) - 0is correct and consistent with stringer's template for handling enums with arbitrary starting values. The- 0reflects the first enum value (DirectiveLocationUnknown-0) and is parameterized in the generator template. The bounds check and slice indexing are safe, and behavior is preserved for all valid inputs.execution/federationtesting/reviews/graph/schema.resolvers.go (1)
5-5: Header version bump confirmedGenerated file correctly reflects gqlgen v0.17.76. No further action needed.
execution/federationtesting/products/graph/schema.resolvers.go (1)
5-5: Generated header update matches gqlgen v0.17.76Header metadata aligns with the dependency bump; everything else remains unchanged.
execution/subscription/transport_client_mock_test.go (1)
2-2: Mock Source path refreshedSource annotation now points at
execution/subscription, matching the relocated interfaces.execution/federationtesting/reviews/graph/entity.resolvers.go (1)
5-5: Header metadata updatedGenerated file now records gqlgen v0.17.76; no other adjustments required.
execution/subscription/handler_mock_test.go (1)
2-2: Source annotation aligns with new packageMock header now references
execution/subscription, which syncs with the dependency move.execution/subscription/websocket/engine_mock_test.go (1)
2-2: Updated mock Source headerHeader correctly points to
execution/subscription, matching the shared interface package.execution/federationtesting/products/graph/entity.resolvers.go (1)
5-5: Generated header reflects v0.17.76Version tag matches the upgraded gqlgen release; no further concerns.
execution/subscription/executor_mock_test.go (1)
2-2: Mock Source path updatedHeader now targets
execution/subscription, aligning with the reorganized interfaces.v2/go.mod (3)
6-6: LGTM! Core dependency upgrade aligns with PR objectives.The gqlgen upgrade to v0.17.76 is consistent with the PR's stated goal to fix "make generate" and align with cosmo's version.
80-80: LGTM! Tool directive properly added.The tool directive ensures gqlgen is available as a build tool, which is the correct approach for code generation.
27-40: Dependency versions are free from known advisories. Testify v1.11.1, gqlparser/v2 v2.5.30 (>2.5.14), and protobuf v1.36.9 (>1.33.0) are all past their first-patched releases.execution/federationtesting/products/graph/model/models_gen.go (1)
5-21: LGTM! Standard GraphQL root types added.The addition of empty Mutation, Query, and Subscription types is standard for gqlgen-generated code and aligns with the GraphQL schema structure.
execution/go.mod (2)
6-23: LGTM! Dependencies upgraded consistently.The gqlgen upgrade to v0.17.76 and related dependency updates are consistent with the PR objectives and align with changes in other modules.
85-85: LGTM! Tool directive properly added.The tool directive ensures gqlgen is available as a build tool, consistent with the approach in v2/go.mod.
examples/federation/go.mod (1)
6-14: LGTM! Core dependencies upgraded consistently.The gqlgen and related dependency upgrades align with the PR objectives.
execution/federationtesting/products/graph/handler.go (1)
45-52: LGTM! Correctly adapted to new websocket InitFunc signature.The InitFunc signature has been updated to match the new gqlgen v0.17.76 API, which now requires returning a
*transport.InitPayloadin addition to the context and error. The implementation correctly returnsnilfor the payload, which is appropriate when no custom payload processing is needed.execution/federationtesting/accounts/graph/model/models_gen.go (4)
6-6: LGTM! Bytes import added for new JSON marshaling functionality.The bytes package import is required for the new MarshalJSON method added to the Which enum.
105-194: LGTM! Field tags updated with omitempty directives.The addition of
omitemptytags to optional fields (Name in C and D, Wallet in Purchase, OtherInterfaces in nested types) follows Go best practices and ensures proper JSON serialization of optional fields.
163-164: LGTM! Standard GraphQL Query type added.The empty Query struct is standard for gqlgen-generated code and aligns with the GraphQL schema structure.
338-367: LGTM! Enhanced enum serialization support.The changes improve the Which enum:
- Parameter type modernized from
interface{}toany(Go 1.18+)- Added
UnmarshalJSONandMarshalJSONmethods for direct JSON serializationThese additions enable the enum to be serialized/deserialized to/from JSON in addition to GraphQL, which is useful for scenarios where the enum needs to be persisted or transmitted in JSON format.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
execution/subscription/websocket/client_test.go (1)
250-251: The sleep addresses the timing issue, but consider polling instead.The 4ms sleep allows the connection state to propagate before the assertion. However, hard-coded sleeps can make tests slower than necessary and may still be insufficient on slower systems.
Consider refactoring to poll
IsConnected()instead:assert.Eventually(t, func() bool { actualServerResult := <-serverResponseChan assert.NoError(t, actualServerResult.err) assert.Equal(t, ws.OpClose, actualServerResult.opCode) assert.Equal(t, ws.StatusCode(4400), actualServerResult.statusCode) assert.Equal(t, "unknown reason", actualServerResult.closeReason) - // Connection could be still not closed, give it time to reach the state: - time.Sleep(4 * time.Millisecond) - assert.Equal(t, false, websocketClient.IsConnected()) - return true + return !websocketClient.IsConnected() }, 1*time.Second, 2*time.Millisecond)This approach leverages
assert.Eventually's built-in retry mechanism, making the test both faster (no unnecessary waits) and more reliable (retries until the condition is met or timeout occurs).Also applies to: 277-278, 304-305
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
execution/subscription/websocket/client_test.go(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build and test (go 1.25 / windows-latest)
- GitHub Check: Build and test (go 1.25 / windows-latest)
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
execution/federationtesting/accounts/graph/handler.go(1 hunks)execution/federationtesting/reviews/graph/handler.go(1 hunks)v2/pkg/testing/subscriptiontesting/chat_test.go(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- execution/federationtesting/accounts/graph/handler.go
- execution/federationtesting/reviews/graph/handler.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build and test (go 1.25 / windows-latest)
- GitHub Check: Build and test (go 1.25 / windows-latest)
|
Now it is ready for your review. |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
examples/federation/reviews/graph/model/models_gen.go (1)
24-24: Sameomitemptyconcern as line 7.The
omitemptytag addition onUser.Reviewshas the same API impact as the change on line 7 forProduct.Reviews. Ensure consumers don't rely on this field always being present.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (8)
examples/federation/accounts/graph/generated/federation.gois excluded by!**/generated/**examples/federation/accounts/graph/generated/generated.gois excluded by!**/generated/**examples/federation/go.sumis excluded by!**/*.sumexamples/federation/products/graph/generated/federation.gois excluded by!**/generated/**examples/federation/products/graph/generated/generated.gois excluded by!**/generated/**examples/federation/reviews/graph/generated/federation.gois excluded by!**/generated/**examples/federation/reviews/graph/generated/generated.gois excluded by!**/generated/**go.work.sumis excluded by!**/*.sum
📒 Files selected for processing (15)
examples/federation/accounts/graph/entity.resolvers.go(1 hunks)examples/federation/accounts/graph/model/models_gen.go(1 hunks)examples/federation/accounts/graph/schema.resolvers.go(1 hunks)examples/federation/accounts/server.go(1 hunks)examples/federation/go.mod(1 hunks)examples/federation/products/graph/entity.resolvers.go(1 hunks)examples/federation/products/graph/handler.go(1 hunks)examples/federation/products/graph/model/models_gen.go(1 hunks)examples/federation/products/graph/schema.resolvers.go(1 hunks)examples/federation/products/server.go(1 hunks)examples/federation/reviews/graph/entity.resolvers.go(1 hunks)examples/federation/reviews/graph/model/models_gen.go(2 hunks)examples/federation/reviews/server.go(1 hunks)examples/federation/tools.go(0 hunks)execution/go.mod(2 hunks)
💤 Files with no reviewable changes (1)
- examples/federation/tools.go
✅ Files skipped from review due to trivial changes (3)
- examples/federation/reviews/graph/entity.resolvers.go
- examples/federation/products/graph/schema.resolvers.go
- examples/federation/accounts/graph/schema.resolvers.go
🚧 Files skipped from review as they are similar to previous changes (1)
- examples/federation/go.mod
🧰 Additional context used
🧬 Code graph analysis (2)
examples/federation/reviews/graph/model/models_gen.go (1)
examples/federation/products/graph/model/models_gen.go (2)
Product(5-10)Product(12-12)
examples/federation/products/graph/handler.go (1)
pkg/subscription/websocket/init.go (1)
InitFunc(10-10)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build and test (go 1.25 / windows-latest)
- GitHub Check: Build and test (go 1.25 / windows-latest)
🔇 Additional comments (6)
examples/federation/reviews/server.go (1)
1-1: LGTM! Clean simplification of the codegen directive.Removing
-mod=modaligns with modern Go practices (Go 1.16+) where module mode is the default. This simplifies the codegen workflow without any functional impact.examples/federation/reviews/graph/model/models_gen.go (3)
12-14: LGTM! Query root type added.The empty
Querystruct is a standard GraphQL root type generated by gqlgen. The actual query resolvers would be implemented in the resolver files.
1-27: AI summary inconsistency detected.The AI-generated summary mentions changes not visible in this file:
- Import of
bytespackageWhichtype'sUnmarshalGQLparameter type change- JSON marshaling methods for
WhichtypeThe annotated code only shows:
omitemptytag additions on lines 7 and 24Querystruct addition on lines 12-14These discrepancies suggest the summary may be aggregating information from multiple files or contains inaccuracies.
7-7: Verify client compatibility withomitemptybehaviorThe added
omitemptytag will omitreviewswhen empty or nil instead of serializing it. Confirm that all downstream consumers handle missingreviewsfields correctly.examples/federation/products/server.go (1)
1-1: LGTM on go:generate updateDropping -mod=mod is fine with modern Go; generation will use the version pinned in examples/federation/go.mod (v0.17.76). No further action.
examples/federation/products/graph/handler.go (1)
45-46: Minor summary mismatchThe AI summary mentions renaming the second parameter to ip; code keeps it as _. The implementation is fine; no change needed.
2 participants
This version fixes "make generate" and matches
the version used in cosmo.
Closes #1091
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores