Skip to content

Fix workflows for Firebase Sessions #4841

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Apr 5, 2023
Merged

Fix workflows for Firebase Sessions #4841

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c6d2128
Use googleServices.gradle
mrober Apr 3, 2023
c403a3e
Move package level function to object to fix api surface check
mrober Apr 3, 2023
4a44954
Add android manifest file with versionName set to fix android tests
mrober Apr 4, 2023
6090304
Update pinned deps
mrober Apr 4, 2023
56ae038
Make getApplicationInfo look leaner
mrober Apr 4, 2023
e1d419b
Merge branch 'firebase-sessions' into sessions-action
mrober Apr 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions firebase-sessions/firebase-sessions.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,11 @@ android {
}

dependencies {
implementation("com.google.firebase:firebase-common-ktx:20.3.2")
implementation("com.google.firebase:firebase-components:17.1.0")
implementation("com.google.firebase:firebase-encoders-json:18.0.1")
Comment on lines +45 to +47
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good to me here. But curious why we choose pinned versions over a HEAD dependency! Would be a good learning for all of for future.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was an effort across the whole repo to pin dep versions see go/firebase-android-cross-deps

implementation("com.google.firebase:firebase-encoders:17.0.0")
implementation(project(":encoders:firebase-encoders-json"))
implementation("com.google.firebase:firebase-installations-interop:17.1.0")
implementation("com.google.firebase:firebase-common-ktx:20.3.1")
implementation("com.google.firebase:firebase-components:17.1.0")
implementation(libs.androidx.annotation)

runtimeOnly("com.google.firebase:firebase-installations:17.1.3")
Expand Down
26 changes: 26 additions & 0 deletions firebase-sessions/src/androidTest/AndroidManifest.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<!--
~ 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.
-->

<manifest xmlns:android="http://schemas.android.com/apk/res/android"
package="com.google.firebase.sessions.test"
android:versionCode="1"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we do a dynamic version code here? For eg: number of days since January 2023? I'm trying to see if we can get some dynamicity in the data that is generated especially for the ApplicationInfo proto message.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see an easy way to do that inside this manifest file. Maybe we can do it programmatically for that one test later.

android:versionName="1.0.0">

<application>

</application>

</manifest>
15 changes: 0 additions & 15 deletions firebase-sessions/src/main/kotlin/com/google/firebase/sessions/ApplicationInfo.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

package com.google.firebase.sessions

import com.google.firebase.FirebaseApp
import com.google.firebase.encoders.json.NumberedEnum

/** Enum denoting different development environments. */
Expand Down Expand Up @@ -58,17 +57,3 @@ internal data class ApplicationInfo(
/** The android application information for the app. */
val androidAppInfo: AndroidApplicationInfo,
)

internal fun getApplicationInfo(firebaseApp: FirebaseApp): ApplicationInfo {
val packageName = firebaseApp.applicationContext.packageName
val packageInfo = firebaseApp.applicationContext.packageManager.getPackageInfo(packageName, 0)

return ApplicationInfo(
appId = firebaseApp.options.applicationId,
deviceModel = "",
sessionSdkVersion = BuildConfig.VERSION_NAME,
logEnvironment = LogEnvironment.LOG_ENVIRONMENT_PROD,
androidAppInfo =
AndroidApplicationInfo(packageName = packageName, versionName = packageInfo.versionName)
)
}
15 changes: 15 additions & 0 deletions firebase-sessions/src/main/kotlin/com/google/firebase/sessions/SessionEvents.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,4 +74,19 @@ internal object SessionEvents {
),
applicationInfo = getApplicationInfo(firebaseApp)
)

fun getApplicationInfo(firebaseApp: FirebaseApp): ApplicationInfo {
val context = firebaseApp.applicationContext
val packageName = context.packageName
val packageInfo = context.packageManager.getPackageInfo(packageName, 0)

return ApplicationInfo(
appId = firebaseApp.options.applicationId,
deviceModel = "",
sessionSdkVersion = BuildConfig.VERSION_NAME,
logEnvironment = LogEnvironment.LOG_ENVIRONMENT_PROD,
androidAppInfo =
AndroidApplicationInfo(packageName = packageName, versionName = packageInfo.versionName)
)
}
}
2 changes: 1 addition & 1 deletion firebase-sessions/src/test/kotlin/com/google/firebase/sessions/ApplicationInfoTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class ApplicationInfoTest {

@Test
fun applicationInfo_populatesInfoCorrectly() {
val applicationInfo = getApplicationInfo(FakeFirebaseApp.fakeFirebaseApp())
val applicationInfo = SessionEvents.getApplicationInfo(FakeFirebaseApp.fakeFirebaseApp())
assertThat(applicationInfo)
.isEqualTo(
ApplicationInfo(
Expand Down
4 changes: 3 additions & 1 deletion firebase-sessions/test-app/test-app.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
plugins {
id("com.android.application")
id("org.jetbrains.kotlin.android")
id("com.google.gms.google-services")
}

android {
Expand Down Expand Up @@ -47,3 +46,6 @@ dependencies {
implementation("androidx.core:core-ktx:1.9.0")
implementation("com.google.android.material:material:1.8.0")
}

extra["packageName"] = "com.google.firebase.testing.sessions"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting. Why specify the package name here and not in the manifest?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is how we pass a package name to the googleServices.gradle file below.

apply(from = "../../gradle/googleServices.gradle")