Fireperf aqs sessionmanager phase2#7997
Open
jrodiz wants to merge 7 commits intofirebase:fireperf-aqsfrom
Open
Fireperf aqs sessionmanager phase2#7997jrodiz wants to merge 7 commits intofirebase:fireperf-aqsfrom
jrodiz wants to merge 7 commits intofirebase:fireperf-aqsfrom
Conversation
Delete the private no-arg constructor and static getInstance() method. The Dagger component factory in FirebasePerfRegistrar now constructs a fresh SessionManager(GaugeManager, PerfSession) instead of delegating to the singleton accessor. Add null-guards for gaugeManager in setApplicationContext, stopGaugeCollectionIfSessionRunningTooLong, logGaugeMetadataIfCollectionEnabled, and startOrStopCollectingGauges to support test-only SessionManager instances that are constructed with a null GaugeManager. Remove the testInstanceCreation unit test whose invariant is now maintained by the DI container rather than the class itself.
…artTrace pattern Add getInstance(SessionManager) seeded-init overload so FirebasePerfEarly can inject the DI-managed SessionManager at startup. The no-arg getInstance() remains as a safe "get-after-init" fallback (creates its own SessionManager if called before early init, e.g. in tests or AppStateUpdateHandler). Add getSessionManager() accessor and resetInstance() @VisibleForTesting helper to support test isolation. Update FirebasePerfEarly to call getInstance(sessionManager) instead of the no-arg version.
…() to TransportManager Trace's 5-arg convenience constructor now resolves SessionManager via sessionManagerFrom(appStateMonitor), which delegates to AppStateMonitor.getSessionManager() and falls back to a no-gauge instance when called in tests with a mocked AppStateMonitor. Trace's Parcel deserialization constructor now uses TransportManager.getInstance().getSessionManager() instead of SessionManager.getInstance(). This is safe because Parcel deserialization always occurs after FirebasePerfEarly has initialized the SDK. Add getSessionManager() accessor to TransportManager and thread sessionManager through initializeForTest() so test helpers can supply the correct instance.
The private single-arg constructor now obtains SessionManager via AppStateMonitor.getInstance().getSessionManager() instead of calling SessionManager.getInstance(). AppStateMonitor.getInstance() is safe here because FirebasePerfEarly (eager component) always seeds it before any network request can be intercepted by user code.
- FirebasePerfRegistrarTest: update component count (3→4) and dependency assertions to include SessionManager - FirebasePerformanceTest: construct spy SessionManager directly instead of spying on the now-deleted getInstance() return value - AppStartTraceTest: use base-class sessionManager field in @after reset instead of SessionManager.getInstance() - TraceTest: pre-seed AppStateMonitor with the test sessionManager in setUp; use sessionManager.updatePerfSession() in session-addition test - NetworkRequestMetricBuilderTest: same AppStateMonitor pre-seed pattern; use sessionManager.updatePerfSession() in session-addition test - TransportManagerTest: replace all SessionManager.getInstance() calls with the base-class sessionManager field; initializeForTest() now accepts and sets the sessionManager parameter
Contributor
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 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 counter productive. 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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Phase 2 of the SessionManager dependency-injection migration for Firebase Performance.
This continues the work started in #7675 (Phase 1: RemoteConfigManager refactor + SessionManager refactor groundwork) by fully eliminating the SessionManager.getInstance() static singleton in favor of Dagger-managed construction and explicit dependency passing.
Changes
instance.
NetworkRequestMetricBuilderTest, and TransportManagerTest to use the new DI-based patterns instead of the removed singleton.
Motivation
Removing the static singleton makes SessionManager's lifecycle explicit and testable, eliminates hidden global state, and aligns with the broader AQS (Android Quality Sessions) migration where session identity is provided by Firebase Sessions rather than managed internally.