dataconnect(chore): realtime refactor in preparation for deduplicating cached and server results#8236
dataconnect(chore): realtime refactor in preparation for deduplicating cached and server results#8236dconeybe wants to merge 14 commits into
Conversation
… entry corresponding to the return being emitted
…nentN() functions
…mbers This change is based on this (excellent) feedback grom gemini: The DataSource.Cache constructor supports a nullable SqliteSequenceNumber? to accommodate legacy cache records that were created before sequence numbers were tracked. The dataSource unit test generator currently only generates non-null sequence numbers, which misses checking this fallback/backward-compatibility flow.
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. |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the internal tracking of data sources by introducing a sealed DataSource interface with Server and Cache variants, allowing the cache variant to carry a SqliteSequenceNumber. Additionally, DataSourcePair has been replaced with a more generic TaggedReference utility to associate metadata with query results across the subscription and query manager flows. A critical issue was identified in QuerySubscriptionImpl.kt where failures in non-realtime query updates are silently ignored using getOrNull() ?: return instead of being propagated to the subscriber, and a code suggestion has been provided to correctly handle and send these failures.
📝 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. |
This PR refactors the internal query source tracking architecture in data connect by introducing a sealed
DataSourcehierarchy (ServerandCache) and aTaggedReferenceutility class. These changes enable the cache and realtime layers to propagateSqliteSequenceNumberdetails, laying the groundwork for optimizing realtime updates and preventing duplicate events.Highlights
DataSourceSealed Hierarchy: Replaced the public-facingDataSourceenum in internal components with a new internal sealedDataSourceinterface (supportingServerandCache(SqliteSequenceNumber?)), carrying sequence number information down from the local database.TaggedReferenceUtility: Renamed and generalized the query-boundDataSourcePairutility into a generic-covariantTaggedReference<out Tag, out T>class with functional.map()operations to transform reference data without losing tag context.RealtimeQueryManagerstream flows to cache incoming realtime frames and tag their corresponding stream responses with the resultingSqliteSequenceNumber.DataConnectGrpcRPCsto use an explicit Kotlinwhenblock for transparent handling of empty cache rows.dataSourcegenerator to produce nullable sequence numbers (modeling backward compatibility scenarios where the database lacks sequence numbers) and added complete property-based tests forTaggedReference.Changelog
See affected files
ExecuteResponse.OperationResultto return the new internalDataSourcehierarchy, propagating gRPC-level cache sequence numbers.ExecuteQueryResult.FromCacheto hold cache sequence numbers and updated return branching using an explicitwhenblock.DataSourcesealed interface representing internal sources (ServerorCache) and a utility to convert to the public enum types.TaggedReferenceand publicDataSourceenum mapping.TaggedReferencestructures and safe error-checking flows.DataSourcePairreferences to useTaggedReference<DataSource, T>.DataSourcePairtoTaggedReference<DataSource, T>.SqliteSequenceNumberalongside responses via the new genericTaggedReferenceutility.TaggedReference<DataSource, T>.Comparableinterface toSqliteSequenceNumbervalue class to support ordering comparison of cache event numbers..map()transformation block.DataSourcestructure with nullable sequence numbers.OperationResultassertions to reflect newDataSourcehierarchy.TaggedReferencewith correct core to publicDataSourcemappings.SqliteSequenceNumberinstances.sqliteSequenceNumberand updateddataSourcetest generators to generate nullable sequence numbers for database backward compatibility checks.equals,hashCode,toStringcontracts, and inlinemapoperations.