dataconnect(chore): refactor variable encoding and data decoding logic into a single class#8142
Conversation
…sDataConnectGrpcStreamingServer.kt and TurbineUtils.kt This enables these classes to be used from both `androidTest` to `test` (thanks to the `@SharedWithAndroidTest` annotation and the associated gradle plugin) instead of just in `androidTest`. A future test will make use of this.
… streaming connection with the backend for realtime queries.
…le(): T` (moved from ConnectRPCIntegrationTest.kt)
…ic tests for DataConnectGrpcRPCs.connect()
…ared errors with itself instead of with the expected list [skip actions]
…sts if ConnectorStreamServiceCoroutineStub.connect() throws an exception
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. |
dconeybe
changed the title
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the DataConnectSerialization class to centralize variable encoding and data decoding logic, offloading these CPU-bound tasks to a background dispatcher. Existing components like MutationRefImpl, LiveQueries, and RegisteredDataDeserializer have been refactored to use this class, and internal models now utilize raw GraphqlError types. Review feedback identifies a missing null-safe call in DataConnectSerialization.kt that would lead to a compilation error and suggests improving consistency in error path string representations.
📝 PRs merging into main branchOur main branch should always be in a releasable state. If you are working on a larger change, or if you don't want this change to see the light of the day just yet, consider using a feature branch first, and only merge into the main branch when the code complete and ready to be released. |
2 participants
This PR refactors the variable encoding and data decoding logic for Firebase Data Connect into a new
DataConnectSerializationclass. This consolidation simplifies the core client logic and improves maintainability by centralizing serialization concerns.Highlights
DataConnectSerializationto handle all encoding and decoding of GraphQL data and variables.DataConnectGrpcClient,MutationRefImpl, andQueryRefImplto delegate serialization tasks to the new class.DataConnectBidiConnectStreamto support null data inExecuteResponse.DataConnectSerializationUnitTest.Changelog
15 files affected
DataConnectGrpcRPCsConnectIntegrationTest.kt: Added null check for data inshouldBeGetStringByKeyQueryResponse.DataConnectBidiConnectStream.kt: Changeddatato be nullable inExecuteResponse.DataConnectGrpcClient.kt: Removed internal serialization logic and updatedOperationResultto useGraphqlError.DataConnectInternal.kt: Addedserializationproperty toFirebaseDataConnectInternal.FirebaseDataConnectInternal.kt: Addedserializationproperty.MutationRefImpl.kt: Updated to useserializationfor encoding variables and decoding data.QueryRefImpl.kt: Updated to useserializationfor encoding variables and decoding data.DataConnectSerialization.kt: New file containing the core serialization logic.ProtoUtil.kt: AddedtoValueProto()andtoErrorInfoImpl()helper functions.DataConnectGrpcClientUnitTest.kt: Removed serialization tests (moved toDataConnectSerializationUnitTest) and updated existing tests.DataConnectSerializationUnitTest.kt: New unit tests forDataConnectSerialization.MutationRefImplUnitTest.kt: Updated mocks to includeserialization.KotestPrinters.kt: Updated import fortoErrorInfoImpl.arbs.kt: Added new arbitraries forGraphqlErrorProtoand updatedoperationResult.