GitLiveApp / firebase-kotlin-sdk

A Kotlin-first SDK for Firebase
https://gitliveapp.github.io/firebase-kotlin-sdk/
Apache License 2.0
1.11k stars 153 forks source link

Firebase messaging frame #509

Closed mr-kew closed 3 months ago

mr-kew commented 4 months ago

Just the general frame for firebase messaging library. See #467.

nbransby commented 4 months ago

There are a few more places to add firebase messaging (for example the readme), if you search everywhere for one of the existing modules to make sure you have added messaging to all of the required places.

For the tests you can just add one test case asserting Firebase.messaging doesn't return null or throw an exception.

mr-kew commented 4 months ago

@nbransby Okay, I did add the messaging to all the places I could find. I hope I did not miss any.

As for the tests, I don't really understand what you mean. I dont have access to Firebase.messaging(app: FirebaseApp) function as the platforms don't have it. So I can just do a simple test like this:

@Test
fun testConstruction() {
    assertNotNull(Firebase.messaging)
}

This works on iOS somehow, but not on Android. Instrumented tests complain about FirebaseApp not being initialized. And as far as I know, I can't get around it because I can't do following setup:

@BeforeTest
fun initializeFirebase() {
    val app = Firebase.apps(context).firstOrNull() ?: Firebase.initialize(
        context,
        FirebaseOptions(
            applicationId = "1:846484016111:ios:dd1f6688bad7af768c841a",
            apiKey = "AIzaSyCK87dcMFhzCz_kJVs2cT2AVlqOTLuyWV0",
            databaseUrl = "https://fir-kotlin-sdk.firebaseio.com",
            storageBucket = "fir-kotlin-sdk.appspot.com",
            projectId = "fir-kotlin-sdk",
            gcmSenderId = "846484016111"
        )
    )

    // This does not exist
    messaging = Firebase.messaging(app)
}
nbransby commented 3 months ago

Hmm,Firebase.messaging should return the initialized app in any case: https://github.com/firebase/firebase-android-sdk/blob/master/firebase-messaging/src/main/java/com/google/firebase/messaging/FirebaseMessaging.java#L121

mr-kew commented 3 months ago

Well the FirebaseApp.getInstance() can throw, so Firebase.messaging fails on that, I think.

I could probably somehow try to leverage the default app instance, but i don't know if it's even worth it.

nbransby commented 3 months ago

@BeforeTest fun initializeFirebase() { val app = Firebase.apps(context).firstOrNull() ?: Firebase.initialize( context, FirebaseOptions( applicationId = "1:846484016111:ios:dd1f6688bad7af768c841a", apiKey = "AIzaSyCK87dcMFhzCz_kJVs2cT2AVlqOTLuyWV0", databaseUrl = "https://fir-kotlin-sdk.firebaseio.com", storageBucket = "fir-kotlin-sdk.appspot.com", projectId = "fir-kotlin-sdk", gcmSenderId = "846484016111" ) ) }

This code should set the default instance as its only when you pass a name into initialize as the third arg it is not considered the default (unless that name is "[DEFAULT]" then its still the default instance

mr-kew commented 3 months ago

Ah you are right. I added the test for iOS and Android (instrumented) then.

I used abstract class instead of expect/actual val context as it felt like an overkill when the context is really available only inside the android instrumented tests. I hope it's fine.

nbransby commented 3 months ago

Good stuff