diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/logging/FirebaseSessionsEnforcementCheck.kt b/firebase-perf/src/main/java/com/google/firebase/perf/logging/FirebaseSessionsEnforcementCheck.kt index 089611564e8..1efa37050b7 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/logging/FirebaseSessionsEnforcementCheck.kt +++ b/firebase-perf/src/main/java/com/google/firebase/perf/logging/FirebaseSessionsEnforcementCheck.kt @@ -16,7 +16,6 @@ package com.google.firebase.perf.logging -import com.google.firebase.perf.session.PerfSession import com.google.firebase.perf.session.isLegacy import com.google.firebase.perf.v1.PerfSession as ProtoPerfSession @@ -27,19 +26,16 @@ class FirebaseSessionsEnforcementCheck { private var logger: AndroidLogger = AndroidLogger.getInstance() @JvmStatic - fun checkSession(sessions: List, failureMessage: String) { - sessions.forEach { checkSession(it.sessionId, failureMessage) } - } - - @JvmStatic - fun checkSession(session: PerfSession, failureMessage: String) { - checkSession(session.sessionId(), failureMessage) + fun checkSessionsList(sessions: List, failureMessage: String) { + if (sessions.count { it.sessionId.isLegacy() } == sessions.size) { + sessions.forEach { checkSession(it.sessionId, failureMessage) } + } } @JvmStatic fun checkSession(sessionId: String, failureMessage: String) { if (sessionId.isLegacy()) { - logger.debug("legacy session ${sessionId}: $failureMessage") + logger.verbose("Contains Legacy Session ${sessionId}: $failureMessage") assert(!enforcement) { failureMessage } } } diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/session/FirebasePerformanceSessionSubscriber.kt b/firebase-perf/src/main/java/com/google/firebase/perf/session/FirebasePerformanceSessionSubscriber.kt index 0f93f45586e..f524e52ed34 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/session/FirebasePerformanceSessionSubscriber.kt +++ b/firebase-perf/src/main/java/com/google/firebase/perf/session/FirebasePerformanceSessionSubscriber.kt @@ -17,7 +17,7 @@ package com.google.firebase.perf.session import com.google.firebase.perf.config.ConfigResolver -import com.google.firebase.perf.logging.FirebaseSessionsEnforcementCheck +import com.google.firebase.perf.logging.FirebaseSessionsEnforcementCheck.Companion.checkSession import com.google.firebase.sessions.api.SessionSubscriber class FirebasePerformanceSessionSubscriber(val configResolver: ConfigResolver) : SessionSubscriber { @@ -30,7 +30,7 @@ class FirebasePerformanceSessionSubscriber(val configResolver: ConfigResolver) : override fun onSessionChanged(sessionDetails: SessionSubscriber.SessionDetails) { val currentPerfSession = SessionManager.getInstance().perfSession() // TODO(b/394127311): Add logic to deal with app start gauges if necessary. - FirebaseSessionsEnforcementCheck.checkSession(currentPerfSession, "onSessionChanged") + checkSession(currentPerfSession.sessionId(), "Existing session in onSessionChanged().") val updatedSession = PerfSession.createWithId(sessionDetails.sessionId) SessionManager.getInstance().updatePerfSession(updatedSession) diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/session/SessionManager.java b/firebase-perf/src/main/java/com/google/firebase/perf/session/SessionManager.java index a5f20ec6aa4..873965d7aaa 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/session/SessionManager.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/session/SessionManager.java @@ -18,7 +18,6 @@ import android.content.Context; import androidx.annotation.Keep; import androidx.annotation.VisibleForTesting; -import com.google.firebase.perf.logging.FirebaseSessionsEnforcementCheck; import com.google.firebase.perf.session.gauges.GaugeManager; import com.google.firebase.perf.v1.GaugeMetadata; import com.google.firebase.perf.v1.GaugeMetric; @@ -46,8 +45,6 @@ public static SessionManager getInstance() { /** Returns the currently active PerfSession. */ public final PerfSession perfSession() { - FirebaseSessionsEnforcementCheck.checkSession(perfSession, "PerfSession.perfSession()"); - return perfSession; } @@ -55,7 +52,6 @@ private SessionManager() { // Creates a legacy session by default. This is a safety net to allow initializing // SessionManager - but the current implementation replaces it immediately. this(GaugeManager.getInstance(), PerfSession.createWithId(null)); - FirebaseSessionsEnforcementCheck.checkSession(perfSession, "SessionManager()"); } @VisibleForTesting @@ -78,9 +74,6 @@ public void setApplicationContext(final Context appContext) { * @see PerfSession#isSessionRunningTooLong() */ public void stopGaugeCollectionIfSessionRunningTooLong() { - FirebaseSessionsEnforcementCheck.checkSession( - perfSession, "SessionManager.stopGaugeCollectionIfSessionRunningTooLong"); - if (perfSession.isSessionRunningTooLong()) { gaugeManager.stopCollectingGauges(); } @@ -158,16 +151,12 @@ public void unregisterForSessionUpdates(WeakReference client } private void logGaugeMetadataIfCollectionEnabled() { - FirebaseSessionsEnforcementCheck.checkSession( - perfSession, "logGaugeMetadataIfCollectionEnabled"); if (perfSession.isVerbose()) { gaugeManager.logGaugeMetadata(perfSession.sessionId()); } } private void startOrStopCollectingGauges() { - FirebaseSessionsEnforcementCheck.checkSession(perfSession, "startOrStopCollectingGauges"); - if (perfSession.isVerbose()) { gaugeManager.startCollectingGauges(perfSession); } else { diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/transport/TransportManager.java b/firebase-perf/src/main/java/com/google/firebase/perf/transport/TransportManager.java index d1eb5ae059c..d53443e471c 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/transport/TransportManager.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/transport/TransportManager.java @@ -14,6 +14,8 @@ package com.google.firebase.perf.transport; +import static com.google.firebase.perf.logging.FirebaseSessionsEnforcementCheck.checkSession; +import static com.google.firebase.perf.logging.FirebaseSessionsEnforcementCheck.checkSessionsList; import static com.google.firebase.perf.util.AppProcessesProvider.getProcessName; import static java.util.concurrent.TimeUnit.MILLISECONDS; import static java.util.concurrent.TimeUnit.MINUTES; @@ -39,7 +41,6 @@ import com.google.firebase.perf.config.ConfigResolver; import com.google.firebase.perf.logging.AndroidLogger; import com.google.firebase.perf.logging.ConsoleUrlGenerator; -import com.google.firebase.perf.logging.FirebaseSessionsEnforcementCheck; import com.google.firebase.perf.metrics.validator.PerfMetricValidator; import com.google.firebase.perf.session.SessionManager; import com.google.firebase.perf.util.Constants; @@ -300,8 +301,7 @@ public void log(final TraceMetric traceMetric) { * {@link #isAllowedToDispatch(PerfMetric)}). */ public void log(final TraceMetric traceMetric, final ApplicationProcessState appState) { - FirebaseSessionsEnforcementCheck.checkSession( - traceMetric.getPerfSessionsList(), "log TraceMetric"); + checkSessionsList(traceMetric.getPerfSessionsList(), "log(TraceMetric)"); executorService.execute( () -> syncLog(PerfMetric.newBuilder().setTraceMetric(traceMetric), appState)); } @@ -330,8 +330,7 @@ public void log(final NetworkRequestMetric networkRequestMetric) { */ public void log( final NetworkRequestMetric networkRequestMetric, final ApplicationProcessState appState) { - FirebaseSessionsEnforcementCheck.checkSession( - networkRequestMetric.getPerfSessionsList(), "log NetworkRequestMetric"); + checkSessionsList(networkRequestMetric.getPerfSessionsList(), "log(NetworkRequestMetric)"); executorService.execute( () -> syncLog( @@ -361,7 +360,7 @@ public void log(final GaugeMetric gaugeMetric) { * {@link #isAllowedToDispatch(PerfMetric)}). */ public void log(final GaugeMetric gaugeMetric, final ApplicationProcessState appState) { - FirebaseSessionsEnforcementCheck.checkSession(gaugeMetric.getSessionId(), "log GaugeMetric"); + checkSession(gaugeMetric.getSessionId(), "log(GaugeMetric)"); executorService.execute( () -> syncLog(PerfMetric.newBuilder().setGaugeMetric(gaugeMetric), appState)); } diff --git a/firebase-perf/src/test/java/com/google/firebase/perf/session/FirebaseSessionsTestHelper.kt b/firebase-perf/src/test/java/com/google/firebase/perf/session/FirebaseSessionsTestHelper.kt index a617af94a58..dae8afc3db1 100644 --- a/firebase-perf/src/test/java/com/google/firebase/perf/session/FirebaseSessionsTestHelper.kt +++ b/firebase-perf/src/test/java/com/google/firebase/perf/session/FirebaseSessionsTestHelper.kt @@ -25,3 +25,5 @@ fun createTestSession(suffix: Int): PerfSession { } fun testSessionId(suffix: Int): String = "abc$suffix" + +fun testLegacySessionId(suffix: Int): String = "zabc$suffix" diff --git a/firebase-perf/src/test/java/com/google/firebase/perf/session/PerfSessionTest.java b/firebase-perf/src/test/java/com/google/firebase/perf/session/PerfSessionTest.java index f7c8d400483..47c6d745baf 100644 --- a/firebase-perf/src/test/java/com/google/firebase/perf/session/PerfSessionTest.java +++ b/firebase-perf/src/test/java/com/google/firebase/perf/session/PerfSessionTest.java @@ -15,7 +15,9 @@ package com.google.firebase.perf.session; import static com.google.common.truth.Truth.assertThat; +import static com.google.firebase.perf.session.FirebaseSessionsHelperKt.isLegacy; import static com.google.firebase.perf.session.FirebaseSessionsTestHelperKt.createTestSession; +import static com.google.firebase.perf.session.FirebaseSessionsTestHelperKt.testLegacySessionId; import static com.google.firebase.perf.session.FirebaseSessionsTestHelperKt.testSessionId; import static com.google.firebase.perf.util.Constants.PREFS_NAME; import static org.mockito.Mockito.mock; @@ -224,6 +226,57 @@ public void testBuildAndSortMovesTheVerboseSessionToTop() { assertThat(PerfSession.isVerbose(perfSessions[0])).isTrue(); } + @Test + public void testBuildAndSortMovesTheVerboseSessionToTop_legacySession() { + // Force all the sessions from now onwards to be non-verbose + forceNonVerboseSession(); + + // Next, create 3 non-verbose sessions, including a legacy session. + List sessions = new ArrayList<>(); + sessions.add(PerfSession.createWithId(testLegacySessionId(1))); + sessions.add(PerfSession.createWithId(testSessionId(2))); + sessions.add(PerfSession.createWithId(testSessionId(3))); + + // Force all the sessions from now onwards to be verbose + forceVerboseSession(); + + // Next, create 2 verbose sessions + sessions.add(PerfSession.createWithId(testSessionId(4))); + sessions.add(PerfSession.createWithId(testSessionId(5))); + + // Verify that the first session in the list of sessions was not verbose + assertThat(sessions.get(0).isVerbose()).isFalse(); + + com.google.firebase.perf.v1.PerfSession[] perfSessions = + PerfSession.buildAndSort(ImmutableList.copyOf(sessions)); + + // Verify that after building the proto objects for PerfSessions, the first session in the array + // of proto objects is a verbose session + assertThat(PerfSession.isVerbose(perfSessions[0])).isTrue(); + } + + @Test + public void testBuildAndSortKeepsLegacySessionAtTopWithNoVerboseSessions() { + // Force all the sessions from now onwards to be non-verbose + forceNonVerboseSession(); + + // Next, create 3 non-verbose sessions, including a legacy session. + List sessions = new ArrayList<>(); + sessions.add(PerfSession.createWithId(testLegacySessionId(1))); + sessions.add(PerfSession.createWithId(testSessionId(2))); + sessions.add(PerfSession.createWithId(testSessionId(3))); + + // Verify that the first session in the list of sessions was legacy. + assertThat(isLegacy(sessions.get(0))).isTrue(); + + com.google.firebase.perf.v1.PerfSession[] perfSessions = + PerfSession.buildAndSort(ImmutableList.copyOf(sessions)); + + // Verify that after building the proto objects for PerfSessions, the first session in the array + // of proto objects is a legacy session. + assertThat(isLegacy(perfSessions[0].getSessionId())).isTrue(); + } + @Test public void testIsExpiredReturnsFalseWhenCurrentSessionLengthIsLessThanMaxSessionLength() { Timer mockTimer = mock(Timer.class);