Make Sessions throw meaningful message when used with incompatible Pe…

…rf (#5477)
firebase · Oct 30, 2023 · e80997f · e80997f
1 parent d9bdc42
commit e80997f
Show file tree

Hide file tree

Showing 30 changed files with 326 additions and 280 deletions.
diff --git a/firebase-sessions/src/main/kotlin/com/google/firebase/sessions/FirebaseSessionsRegistrar.kt b/firebase-sessions/src/main/kotlin/com/google/firebase/sessions/FirebaseSessionsRegistrar.kt
@@ -1,16 +1,18 @@
-// Copyright 2023 Google LLC
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//      http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
+/*
+ * Copyright 2023 Google LLC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
 
 package com.google.firebase.sessions
 
@@ -101,9 +103,7 @@ internal class FirebaseSessionsRegistrar : ComponentRegistrar {
       Component.builder(SessionLifecycleServiceBinder::class.java)
         .name("sessions-service-binder")
         .add(Dependency.required(firebaseApp))
-        .factory { container ->
-          SessionLifecycleServiceBinderImpl(container.get(firebaseApp))
-        }
+        .factory { container -> SessionLifecycleServiceBinderImpl(container.get(firebaseApp)) }
         .build(),
       LibraryVersionComponent.create(LIBRARY_NAME, BuildConfig.VERSION_NAME),
     )

diff --git a/...se-sessions/src/main/kotlin/com/google/firebase/sessions/SessionLifecycleServiceBinder.kt b/...se-sessions/src/main/kotlin/com/google/firebase/sessions/SessionLifecycleServiceBinder.kt
@@ -58,6 +58,6 @@ internal class SessionLifecycleServiceBinderImpl(private val firebaseApp: Fireba
   }
 
   companion object {
-    const val TAG = "SessionLifecycleServiceBinder"
+    const val TAG = "LifecycleServiceBinder"
   }
 }
diff --git a/...sessions/src/main/kotlin/com/google/firebase/sessions/api/FirebaseSessionsDependencies.kt b/...sessions/src/main/kotlin/com/google/firebase/sessions/api/FirebaseSessionsDependencies.kt
@@ -39,6 +39,19 @@ object FirebaseSessionsDependencies {
    * the Sessions SDK will never generate a session.
    */
   fun addDependency(subscriberName: SessionSubscriber.Name) {
+    if (subscriberName == SessionSubscriber.Name.PERFORMANCE) {
+      throw IllegalArgumentException(
+        """
+          Incompatible versions of Firebase Perf and Firebase Sessions.
+          A safe combination would be:
+            firebase-sessions:1.1.0
+            firebase-crashlytics:18.5.0
+            firebase-perf:20.5.0
+          For more information contact Firebase Support.
+        """
+          .trimIndent()
+      )
+    }
     if (dependencies.containsKey(subscriberName)) {
       Log.d(TAG, "Dependency $subscriberName already added.")
       return

diff --git a/firebase-sessions/src/test/kotlin/com/google/firebase/sessions/SessionLifecycleClientTest.kt b/firebase-sessions/src/test/kotlin/com/google/firebase/sessions/SessionLifecycleClientTest.kt
@@ -46,8 +46,7 @@ import org.robolectric.Shadows.shadowOf
 @MediumTest
 @RunWith(RobolectricTestRunner::class)
 internal class SessionLifecycleClientTest {
-
-  lateinit var fakeService: FakeSessionLifecycleServiceBinder
+  private lateinit var fakeService: FakeSessionLifecycleServiceBinder
 
   @Before
   fun setUp() {
@@ -189,8 +188,8 @@ internal class SessionLifecycleClientTest {
   fun doesNotSendLifecycleEventsWithoutEnabledSubscribers() =
     runTest(UnconfinedTestDispatcher()) {
       val client = SessionLifecycleClient(backgroundDispatcher() + coroutineContext)
-      val crashlyticsSubscriber = addSubscriber(false, SessionSubscriber.Name.CRASHLYTICS)
-      val perfSubscriber = addSubscriber(false, SessionSubscriber.Name.PERFORMANCE)
+      addSubscriber(collectionEnabled = false, SessionSubscriber.Name.CRASHLYTICS)
+      addSubscriber(collectionEnabled = false, SessionSubscriber.Name.MATT_SAYS_HI)
       client.bindToService()
 
       fakeService.serviceConnected()
@@ -205,8 +204,8 @@ internal class SessionLifecycleClientTest {
   fun sendsLifecycleEventsWhenAtLeastOneEnabledSubscriber() =
     runTest(UnconfinedTestDispatcher()) {
       val client = SessionLifecycleClient(backgroundDispatcher() + coroutineContext)
-      val crashlyticsSubscriber = addSubscriber(true, SessionSubscriber.Name.CRASHLYTICS)
-      val perfSubscriber = addSubscriber(false, SessionSubscriber.Name.PERFORMANCE)
+      addSubscriber(collectionEnabled = true, SessionSubscriber.Name.CRASHLYTICS)
+      addSubscriber(collectionEnabled = false, SessionSubscriber.Name.MATT_SAYS_HI)
       client.bindToService()
 
       fakeService.serviceConnected()
@@ -234,31 +233,31 @@ internal class SessionLifecycleClientTest {
     runTest(UnconfinedTestDispatcher()) {
       val client = SessionLifecycleClient(backgroundDispatcher() + coroutineContext)
       val crashlyticsSubscriber = addSubscriber(true, SessionSubscriber.Name.CRASHLYTICS)
-      val perfSubscriber = addSubscriber(true, SessionSubscriber.Name.PERFORMANCE)
+      val mattSaysHiSubscriber = addSubscriber(true, SessionSubscriber.Name.MATT_SAYS_HI)
       client.bindToService()
 
       fakeService.serviceConnected()
       fakeService.broadcastSession("123")
 
       waitForMessages()
       assertThat(crashlyticsSubscriber.sessionChangedEvents).containsExactly(SessionDetails("123"))
-      assertThat(perfSubscriber.sessionChangedEvents).containsExactly(SessionDetails("123"))
+      assertThat(mattSaysHiSubscriber.sessionChangedEvents).containsExactly(SessionDetails("123"))
     }
 
   @Test
   fun handleSessionUpdate_sendsToAllSubscribersAsLongAsOneIsEnabled() =
     runTest(UnconfinedTestDispatcher()) {
       val client = SessionLifecycleClient(backgroundDispatcher() + coroutineContext)
       val crashlyticsSubscriber = addSubscriber(true, SessionSubscriber.Name.CRASHLYTICS)
-      val perfSubscriber = addSubscriber(false, SessionSubscriber.Name.PERFORMANCE)
+      val mattSaysHiSubscriber = addSubscriber(false, SessionSubscriber.Name.MATT_SAYS_HI)
       client.bindToService()
 
       fakeService.serviceConnected()
       fakeService.broadcastSession("123")
 
       waitForMessages()
       assertThat(crashlyticsSubscriber.sessionChangedEvents).containsExactly(SessionDetails("123"))
-      assertThat(perfSubscriber.sessionChangedEvents).containsExactly(SessionDetails("123"))
+      assertThat(mattSaysHiSubscriber.sessionChangedEvents).containsExactly(SessionDetails("123"))
     }
 
   private fun addSubscriber(

diff --git a/...ions/src/test/kotlin/com/google/firebase/sessions/api/FirebaseSessionsDependenciesTest.kt b/...ions/src/test/kotlin/com/google/firebase/sessions/api/FirebaseSessionsDependenciesTest.kt
@@ -19,7 +19,7 @@ package com.google.firebase.sessions.api
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import com.google.common.truth.Truth.assertThat
 import com.google.firebase.sessions.api.SessionSubscriber.Name.CRASHLYTICS
-import com.google.firebase.sessions.api.SessionSubscriber.Name.PERFORMANCE
+import com.google.firebase.sessions.api.SessionSubscriber.Name.MATT_SAYS_HI
 import com.google.firebase.sessions.testing.FakeSessionSubscriber
 import kotlin.time.Duration.Companion.seconds
 import kotlinx.coroutines.Dispatchers
@@ -71,14 +71,14 @@ class FirebaseSessionsDependenciesTest {
 
   @Test
   fun getSubscriber_dependencyAdded_notRegistered_throws() {
-    FirebaseSessionsDependencies.addDependency(PERFORMANCE)
+    FirebaseSessionsDependencies.addDependency(MATT_SAYS_HI)
 
     val thrown =
       assertThrows(IllegalStateException::class.java) {
-        FirebaseSessionsDependencies.getSubscriber(PERFORMANCE)
+        FirebaseSessionsDependencies.getSubscriber(MATT_SAYS_HI)
       }
 
-    assertThat(thrown).hasMessageThat().contains("Subscriber PERFORMANCE has not been registered")
+    assertThat(thrown).hasMessageThat().contains("Subscriber MATT_SAYS_HI has not been registered")
   }
 
   @Test

diff --git a/...ons/src/test/kotlin/com/google/firebase/sessions/testing/FirebaseSessionsFakeRegistrar.kt b/...ons/src/test/kotlin/com/google/firebase/sessions/testing/FirebaseSessionsFakeRegistrar.kt
@@ -64,7 +64,7 @@ internal class FirebaseSessionsFakeRegistrar : ComponentRegistrar {
       Component.builder(SessionsSettings::class.java)
         .name("sessions-settings")
         .add(Dependency.required(firebaseApp))
-        .add(Dependency.required(firebaseInstallationsApi))
+        .add(Dependency.required(blockingDispatcher))
         .add(Dependency.required(backgroundDispatcher))
         .factory { container ->
           SessionsSettings(

diff --git a/firebase-sessions/test-app/src/main/AndroidManifest.xml b/firebase-sessions/test-app/src/main/AndroidManifest.xml
@@ -15,61 +15,61 @@
   -->
 
 <manifest xmlns:android="http://schemas.android.com/apk/res/android"
-    xmlns:tools="http://schemas.android.com/tools">
+  xmlns:tools="http://schemas.android.com/tools">
 
-    <uses-permission android:name="android.permission.FOREGROUND_SERVICE"/>
-    <uses-permission android:name="android.permission.POST_NOTIFICATIONS"/>
-    <uses-permission android:name="android.permission.KILL_BACKGROUND_PROCESSES"/>
+  <application
+    android:icon="@mipmap/ic_launcher"
+    android:label="@string/app_name"
+    android:name=".TestApplication"
+    android:supportsRtl="true"
+    android:theme="@style/Theme.Widget_test_app"
+    tools:targetApi="31">
+    <activity
+      android:exported="true"
+      android:label="@string/app_name"
+      android:name=".MainActivity"
+      android:theme="@style/Theme.Widget_test_app.NoActionBar">
+      <intent-filter>
+        <action android:name="android.intent.action.MAIN" />
+        <category android:name="android.intent.category.LAUNCHER" />
+      </intent-filter>
+    </activity>
 
-    <application
-        android:name=".TestApplication"
-        android:icon="@mipmap/ic_launcher"
-        android:label="@string/app_name"
-        android:supportsRtl="true"
-        android:theme="@style/Theme.Widget_test_app"
-        tools:targetApi="31">
-        <meta-data
-          android:name="firebase_performance_logcat_enabled"
-          android:value="true" />
-        <activity
-          android:name=".MainActivity"
-          android:exported="true"
-          android:label="@string/app_name"
-          android:theme="@style/Theme.Widget_test_app.NoActionBar"
-          >
-          <intent-filter>
-            <action android:name="android.intent.action.MAIN" />
-            <category android:name="android.intent.category.LAUNCHER" />
-          </intent-filter>
-        </activity>
+    <activity
+      android:exported="true"
+      android:label="@string/app_name"
+      android:name=".SecondActivity"
+      android:process=":second"
+      android:theme="@style/Theme.Widget_test_app.NoActionBar" />
 
-        <activity
-          android:name=".SecondActivity"
-          android:exported="true"
-          android:label="@string/app_name"
-          android:theme="@style/Theme.Widget_test_app.NoActionBar" 
-          android:process=":second"/>
+    <meta-data
+      android:name="firebase_performance_logcat_enabled"
+      android:value="true" />
 
-      <receiver
-        android:name="CrashWidgetProvider"
-        android:process=":widgetProcess"
-        android:exported="false">
-        <intent-filter>
-          <action
-            android:name="android.appwidget.action.APPWIDGET_UPDATE" />
-        </intent-filter>
-        <meta-data
-          android:name="android.appwidget.provider"
-          android:resource="@xml/homescreen_widget" />
-      </receiver>
+    <receiver
+      android:exported="false"
+      android:name="CrashWidgetProvider"
+      android:process=":widgetProcess">
+      <intent-filter>
+        <action android:name="android.appwidget.action.APPWIDGET_UPDATE" />
+      </intent-filter>
+      <meta-data
+        android:name="android.appwidget.provider"
+        android:resource="@xml/homescreen_widget" />
+    </receiver>
 
-      <receiver android:name=".CrashBroadcastReceiver" />
+    <receiver android:name=".CrashBroadcastReceiver" />
 
-      <service
-        android:name=".ForegroundService"
-        android:process=":foregroundServiceProcess"
-        android:enabled="true"
-        android:exported="false" />
+    <service
+      android:enabled="true"
+      android:exported="false"
+      android:name=".ForegroundService"
+      android:process=":foregroundServiceProcess" />
 
-    </application>
+  </application>
+
+  <uses-permission android:name="android.permission.POST_NOTIFICATIONS" />
+  <uses-permission android:name="android.permission.KILL_BACKGROUND_PROCESSES" />
+
+  <uses-permission android:name="android.permission.FOREGROUND_SERVICE" />
 </manifest>
diff --git a/...se-sessions/test-app/src/main/kotlin/com/google/firebase/testing/sessions/BaseActivity.kt b/...se-sessions/test-app/src/main/kotlin/com/google/firebase/testing/sessions/BaseActivity.kt
@@ -19,12 +19,12 @@ package com.google.firebase.testing.sessions
 import android.app.ActivityManager
 import android.app.ActivityManager.RunningAppProcessInfo
 import android.app.Application
+import android.os.Build
 import android.os.Bundle
 import android.util.Log
 import androidx.appcompat.app.AppCompatActivity
 import com.google.firebase.FirebaseApp
 
-/**  */
 open class BaseActivity : AppCompatActivity() {
 
   override fun onCreate(savedInstanceState: Bundle?) {
@@ -64,7 +64,12 @@ open class BaseActivity : AppCompatActivity() {
     return processInfo.importance
   }
 
-  private fun getProcessName(): String = Application.getProcessName()
+  private fun getProcessName(): String =
+    if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) {
+      Application.getProcessName()
+    } else {
+      ""
+    }
 
   companion object {
     val TAG = "BaseActivity"

diff --git a/...ions/test-app/src/main/kotlin/com/google/firebase/testing/sessions/CrashWidgetProvider.kt b/...ions/test-app/src/main/kotlin/com/google/firebase/testing/sessions/CrashWidgetProvider.kt
@@ -22,6 +22,7 @@ import android.appwidget.AppWidgetProvider
 import android.content.Context
 import android.content.Intent
 import android.icu.text.SimpleDateFormat
+import android.os.Build
 import android.widget.RemoteViews
 import com.google.firebase.FirebaseApp
 import java.util.Date
@@ -43,7 +44,9 @@ class CrashWidgetProvider : AppWidgetProvider() {
       val views: RemoteViews =
         RemoteViews(context.packageName, R.layout.crash_widget).apply {
           setOnClickPendingIntent(R.id.widgetCrashButton, getPendingCrashIntent(context))
-          setTextViewText(R.id.widgetTimeText, DATE_FMT.format(Date()))
+          if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) {
+            setTextViewText(R.id.widgetTimeText, DATE_FMT?.format(Date()) ?: "")
+          }
         }
 
       // Tell the AppWidgetManager to perform an update on the current
@@ -73,6 +76,11 @@ class CrashWidgetProvider : AppWidgetProvider() {
 
   companion object {
     val CRASH_BUTTON_CLICK = "widgetCrashButtonClick"
-    val DATE_FMT = SimpleDateFormat("HH:mm:ss", Locale.getDefault())
+    val DATE_FMT =
+      if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) {
+        SimpleDateFormat("HH:mm:ss", Locale.US)
+      } else {
+        null
+      }
   }
 }
diff --git a/...e-sessions/test-app/src/main/kotlin/com/google/firebase/testing/sessions/FirstFragment.kt b/...e-sessions/test-app/src/main/kotlin/com/google/firebase/testing/sessions/FirstFragment.kt
@@ -20,6 +20,7 @@ import android.content.Intent
 import android.content.Intent.FLAG_ACTIVITY_LAUNCH_ADJACENT
 import android.content.Intent.FLAG_ACTIVITY_NEW_TASK
 import android.icu.text.SimpleDateFormat
+import android.os.Build
 import android.os.Bundle
 import android.view.LayoutInflater
 import android.view.View
@@ -65,10 +66,12 @@ class FirstFragment : Fragment() {
     }
     binding.buttonForegroundProcess.setOnClickListener {
       if (binding.buttonForegroundProcess.getText().startsWith("Start")) {
-        ForegroundService.startService(
-          getContext()!!,
-          "Starting service at ${DATE_FMT.format(Date())}"
-        )
+        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) {
+          ForegroundService.startService(
+            getContext()!!,
+            "Starting service at ${DATE_FMT?.format(Date()) ?: ""}"
+          )
+        }
         binding.buttonForegroundProcess.setText("Stop foreground service")
       } else {
         ForegroundService.stopService(getContext()!!)
@@ -98,6 +101,11 @@ class FirstFragment : Fragment() {
   }
 
   companion object {
-    val DATE_FMT = SimpleDateFormat("HH:mm:ss", Locale.getDefault())
+    val DATE_FMT =
+      if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) {
+        SimpleDateFormat("HH:mm:ss", Locale.getDefault())
+      } else {
+        null
+      }
   }
 }