Conversation
WalkthroughThe changes add explicit error handling in RPC compilation, introduce alias tracking in the RPC execution plan, and update response marshaling to use aliases as JSON keys. They include new methods and tests verifying alias handling in queries and mutations, and update RPC mapping configurations with renamed mutation RPCs and additional field mappings. Changes
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
🧹 Nitpick comments (1)
v2/pkg/engine/datasource/grpc_datasource/grpc_datasource_test.go (1)
1294-1329: Consider refactoring duplicated test setup code.The gRPC server setup code here duplicates logic from the existing
setupTestGRPCServerfunction. Consider extracting this into a reusable helper to reduce code duplication and improve maintainability.Apply this refactor to use the existing helper:
- // Set up the bufconn listener - lis := bufconn.Listen(1024 * 1024) - - // Create a new gRPC server - server := grpc.NewServer() - - // Register our mock service implementation - mockService := &grpctest.MockService{} - productv1.RegisterProductServiceServer(server, mockService) - - // Start the server in a goroutine - go func() { - if err := server.Serve(lis); err != nil { - t.Errorf("failed to serve: %v", err) - } - }() - - // Clean up the server when the test completes - defer server.Stop() - - // Create a buffer-based dialer - bufDialer := func(context.Context, string) (net.Conn, error) { - return lis.Dial() - } - - // Connect using bufconn dialer - // see https://github.com/grpc/grpc-go/issues/7091 - // nolint: staticcheck - conn, err := grpc.Dial( - "bufnet", - grpc.WithTransportCredentials(insecure.NewCredentials()), - grpc.WithContextDialer(bufDialer), - grpc.WithLocalDNSResolution(), - ) - require.NoError(t, err) - defer conn.Close() + conn, cleanup := setupTestGRPCServer(t) + t.Cleanup(cleanup)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
v2/pkg/engine/datasource/grpc_datasource/compiler.go(1 hunks)v2/pkg/engine/datasource/grpc_datasource/execution_plan.go(1 hunks)v2/pkg/engine/datasource/grpc_datasource/execution_plan_test.go(1 hunks)v2/pkg/engine/datasource/grpc_datasource/execution_plan_visitor.go(1 hunks)v2/pkg/engine/datasource/grpc_datasource/grpc_datasource.go(4 hunks)v2/pkg/engine/datasource/grpc_datasource/grpc_datasource_test.go(1 hunks)v2/pkg/engine/datasource/grpc_datasource/mapping_test_helper.go(1 hunks)v2/pkg/grpctest/mapping/mapping.go(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
v2/pkg/engine/datasource/grpc_datasource/execution_plan.go (1)
pkg/ast/ast_field_alias.go (1)
Alias(8-12)
v2/pkg/engine/datasource/grpc_datasource/execution_plan_visitor.go (1)
pkg/ast/ast_field_alias.go (1)
Alias(8-12)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build and test (go 1.23 / windows-latest)
- GitHub Check: Build and test (go 1.23 / windows-latest)
🔇 Additional comments (9)
v2/pkg/engine/datasource/grpc_datasource/execution_plan.go (1)
169-176: LGTM! Clean alias fallback implementation.The
AliasOrPath()method provides a clean abstraction for accessing the effective field name, properly prioritizing aliases over default JSON paths. The implementation is straightforward and follows good practices.v2/pkg/engine/datasource/grpc_datasource/execution_plan_visitor.go (1)
336-336: Excellent integration of alias support.The alias assignment using
r.operation.FieldAliasString(ref)is correctly placed within theRPCFieldconstruction and properly integrates with the newAliasOrPath()method for comprehensive alias support.v2/pkg/engine/datasource/grpc_datasource/mapping_test_helper.go (1)
90-92: Good naming convention improvement.The update to prefix mutation RPC names with "Mutation" improves consistency and clarity in the mapping configuration. This aligns well with the corresponding changes in the main mapping file.
v2/pkg/engine/datasource/grpc_datasource/grpc_datasource.go (1)
166-166: Comprehensive alias support in JSON marshaling.The systematic replacement of
field.JSONPathwithfield.AliasOrPath()throughout themarshalResponseJSONmethod ensures that GraphQL field aliases are properly reflected in the response JSON keys. This completes the alias support implementation by connecting the execution plan aliases to the final response output.Also applies to: 172-172, 188-188, 193-193, 217-217, 232-232, 238-238
v2/pkg/grpctest/mapping/mapping.go (2)
92-94: Consistent RPC naming convention update.The mutation RPC configuration update maintains consistency with the test helper mappings, properly standardizing the naming convention with "Mutation" prefix.
472-484: Well-structured field mappings for new types.The added field mappings for
SearchResultandActionResulttypes properly handle GraphQL-to-gRPC field name conversions, following the established snake_case naming convention for gRPC fields.v2/pkg/engine/datasource/grpc_datasource/compiler.go (1)
326-333: Excellent error handling improvement!The explicit validation of input and output messages with descriptive error messages prevents compilation from proceeding with invalid message definitions. This enhances robustness and provides clear feedback when protobuf messages are missing from the document.
v2/pkg/engine/datasource/grpc_datasource/grpc_datasource_test.go (1)
1291-1574: Comprehensive alias test coverage with excellent validation!This test thoroughly covers various GraphQL alias scenarios including root fields, nested objects, interfaces, unions, mutations, and enums. The validation logic properly ensures aliased fields appear correctly and original field names are absent, which is crucial for alias functionality verification.
v2/pkg/engine/datasource/grpc_datasource/execution_plan_test.go (1)
2514-3120: Excellent test coverage for GraphQL alias support!The new
TestProductExecutionPlanWithAliasesfunction provides comprehensive coverage of alias functionality across all major GraphQL features including simple fields, nested objects, interfaces, unions, mutations, and complex input types. The test cases are well-structured and follow the established patterns in the file.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
v2/pkg/engine/datasource/grpc_datasource/grpc_datasource_test.go (1)
1290-1539: Excellent comprehensive test coverage for alias functionality!This test function provides thorough coverage of GraphQL alias scenarios including root fields, nested objects, interfaces, unions, mutations, and enum fields. The validation logic correctly verifies that:
- Aliased field names appear in the JSON response
- Original field names are absent from the response
- Data values are correctly preserved
The test cases align well with the PR objectives for adding alias support and demonstrate end-to-end functionality from query parsing to response marshaling.
Consider splitting into separate test functions for better organization.
While the comprehensive coverage is excellent, this single test function contains 8 different test cases which could make debugging more difficult when specific scenarios fail. Consider splitting this into separate focused test functions like:
Test_DataSource_Load_WithSimpleAliasesTest_DataSource_Load_WithNestedAliasesTest_DataSource_Load_WithInterfaceUnionAliasesTest_DataSource_Load_WithMutationAliasesThis would make it easier to identify which specific alias scenario is failing and improve test maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
v2/pkg/engine/datasource/grpc_datasource/grpc_datasource_test.go(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build and test (go 1.23 / windows-latest)
- GitHub Check: Build and test (go 1.23 / windows-latest)
🤖 I have created a release *beep* *boop* --- ## [2.0.0-rc.199](v2.0.0-rc.198...v2.0.0-rc.199) (2025-07-07) ### Features * add support for aliases ([#1209](#1209)) ([9223351](9223351)) ### Bug Fixes * do not trim whitespaces around non-block strings ([#1211](#1211)) ([6f5046b](6f5046b)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added support for aliases. * **Bug Fixes** * Resolved an issue to prevent trimming whitespaces around non-block strings. * **Documentation** * Updated the changelog with details for version 2.0.0-rc.199. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
2 participants
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores
Checklist