FirebaseDataConnectImpl.kt: use MutableStateFlow to store state, rather than mutexes

[dconeybe]

Using MutableStateFlow to store the state of the FirebaseDataConnectImpl greatly simplifies the overall logic since the state is quite complicated and the state and state transitions are represented very naturally this way.

[google-oss-bot]

Coverage Report ¹

Affected Products

• firebase-dataconnect

  Overall coverage changed from 14.31% (fda3351) to 13.43% (28b47df) by -0.88%.

  Filename	Base (fda3351)	Merge (28b47df)	Diff
  FirebaseDataConnectImpl.kt	43.05%	33.59%	-9.46%
  MutationRefImpl.kt	12.36%	12.50%	+0.14%
  NullableReference.kt	40.00%	0.00%	-40.00%
  SuspendingLazy.kt	31.58%	0.00%	-31.58%

Test Logs

• Base (fda3351): https://github.com/firebase/firebase-android-sdk/actions/runs/14582066433
• Merge (28b47df): https://github.com/firebase/firebase-android-sdk/actions/runs/14599090890

1. https://storage.googleapis.com/firebase-sdk-metric-reports/K7jhmIUB7P.html

[github-actions]

Test Results

   66 files   -   120     66 suites   - 120   1m 19s ⏱️ - 3m 15s
  552 tests  -   683    551 ✅  -   668  1 💤  - 15  0 ❌ ±0 
1 104 runs   - 1 390  1 102 ✅  - 1 360  2 💤  - 30  0 ❌ ±0 

Results for commit b8f0fc2. ± Comparison against base commit fda3351.

This pull request removes 1235 and adds 552 tests. Note that renamed tests count towards both.

com.google.firebase.firestore.AggregateQuerySnapshotTest ‑ createWithCountShouldReturnInstanceWithTheGivenQueryAndCount
com.google.firebase.firestore.AggregateQueryTest ‑ testSourceMustNotBeNull
com.google.firebase.firestore.BlobTest ‑ testComparison
com.google.firebase.firestore.BlobTest ‑ testEquals
com.google.firebase.firestore.BlobTest ‑ testMutableBytes
com.google.firebase.firestore.CollectionReferenceTest ‑ testEquals
com.google.firebase.firestore.DocumentChangeTest ‑ randomTests
com.google.firebase.firestore.DocumentChangeTest ‑ testAdditions
com.google.firebase.firestore.DocumentChangeTest ‑ testChangesWithSortOrderChange
com.google.firebase.firestore.DocumentChangeTest ‑ testDeletions
…

com.google.firebase.dataconnect.AnyValueSerializerUnitTest ‑ descriptor should have expected values
com.google.firebase.dataconnect.AnyValueSerializerUnitTest ‑ deserialize() should throw UnsupportedOperationException
com.google.firebase.dataconnect.AnyValueSerializerUnitTest ‑ serialize() should throw UnsupportedOperationException
com.google.firebase.dataconnect.AnyValueUnitTest ‑ constructor(Boolean) creates an object with the expected value
com.google.firebase.dataconnect.AnyValueUnitTest ‑ constructor(Double) creates an object with the expected value (edge cases)
com.google.firebase.dataconnect.AnyValueUnitTest ‑ constructor(Double) creates an object with the expected value (normal cases)
com.google.firebase.dataconnect.AnyValueUnitTest ‑ constructor(List) creates an object with the expected value (edge cases)
com.google.firebase.dataconnect.AnyValueUnitTest ‑ constructor(List) creates an object with the expected value (normal cases)
com.google.firebase.dataconnect.AnyValueUnitTest ‑ constructor(Map) creates an object with the expected value (edge cases)
com.google.firebase.dataconnect.AnyValueUnitTest ‑ constructor(Map) creates an object with the expected value (normal cases)
…

This pull request removes 16 skipped tests and adds 1 skipped test. Note that renamed tests count towards both.

com.google.firebase.firestore.FirestoreRegistrarTest ‑ storageRegistrar_getComponents_publishesLibVersionComponent
com.google.firebase.firestore.local.MemoryLocalStoreTest ‑ testDoesNotReplaceResumeTokenWithEmptyByteString
com.google.firebase.firestore.local.MemoryLocalStoreTest ‑ testHandlesSetMutationThenAckThenTransformThenAckThenTransform
com.google.firebase.firestore.local.MemoryLocalStoreTest ‑ testIgnoresTargetMappingAfterExistenceFilterMismatch
com.google.firebase.firestore.local.MemoryLocalStoreTest ‑ testPersistsResumeTokens
com.google.firebase.firestore.local.MemoryLocalStoreTest ‑ testQueriesFilterDocumentsThatNoLongerMatch
com.google.firebase.firestore.local.MemoryLocalStoreTest ‑ testQueriesIncludeDocumentsFromOtherQueries
com.google.firebase.firestore.local.MemoryLocalStoreTest ‑ testQueriesIncludeLocallyModifiedDocuments
com.google.firebase.firestore.local.MemoryLocalStoreTest ‑ testUsesTargetMappingToExecuteQueries
com.google.firebase.firestore.local.SQLiteLocalStoreTest ‑ testCollectsGarbageAfterAcknowledgedMutation
…

com.google.firebase.dataconnect.ProtoStructDecoderUnitTest ‑ decodeFromStruct() can encode and decode a list of non-nullable Unit

♻️ This comment has been updated with latest results.

[google-oss-bot]

Size Report ¹

Affected Products

• firebase-dataconnect

  Type	Base (fda3351)	Merge (28b47df)	Diff
  aar	710 kB	703 kB	-6.76 kB (-1.0%)
  apk (release)	10.0 MB	10.0 MB	-2.69 kB (-0.0%)

Test Logs

• Base (fda3351): https://github.com/firebase/firebase-android-sdk/actions/runs/14582066433
• Merge (28b47df): https://github.com/firebase/firebase-android-sdk/actions/runs/14599090890

1. https://storage.googleapis.com/firebase-sdk-metric-reports/XSDcxfdmZZ.html

[github-actions]

Vertex AI Mock Responses Check ⚠️

A newer major version of the mock responses for Vertex AI unit tests is available. update_responses.sh should be updated to clone the latest version of the responses: v10.0

[Copilot]

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

[github-actions]

📝 PRs merging into main branch

Our 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.

[Copilot]

Pull Request Overview

This PR refactors the FirebaseDataConnect implementation to replace mutex-based lazy initializations with a state managed by MutableStateFlow, thereby simplifying synchronization and state management. Key changes include replacing SuspensingLazy properties with direct state flow properties, updating tests to use the new queryManager and grpcClient properties instead of lazy wrappers, and refactoring the close logic to align with the new state management.

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
QueryRefImplUnitTest.kt	Updated lazyQueryManager usage to queryManager for executing queries.
MutationRefImplUnitTest.kt	Updated lazyGrpcClient usage to grpcClient for executing mutations.
QuerySubscriptionImpl.kt	Updated subscription logic to retrieve queryManager from the new state flow.
QueryRefImpl.kt	Modified execute() method to use queryManager instead of lazyQueryManager.
MutationRefImpl.kt	Modified execute() method to use grpcClient instead of lazyGrpcClient.
FirebaseDataConnectImpl.kt	Replaced mutex-based lazy initialization with a MutableStateFlow–based state model and refactored the close logic accordingly.

Comments suppressed due to low confidence (1)

firebase-dataconnect/src/main/kotlin/com/google/firebase/dataconnect/core/FirebaseDataConnectImpl.kt:181

• [nitpick] Consider improving the error message to provide more context or refactor the state handling logic to prevent reaching an unexpected state, which can aid in debugging.

throw IllegalStateException("newState should be Initialized, but got New (error code sh2rf4wwjx)")