Skip to content
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.

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

Firebase messaging frame #509

Merged
merged 4 commits into from
Jun 14, 2024
Merged

Conversation

mr-kew
Copy link
Contributor

@mr-kew mr-kew commented May 16, 2024

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

  • It allows access to the native libraries
  • Firebase messaging APIs will be usable with this library without need for them to be imported & managed separately
  • Tests are not implemented because android version of library doesn't provide getInstance(app: FirebaseApp) which would probably be needed to inject the testing FirebaseApp, also firebase-functions does not implement tests as well
  • JVM version does not seem to exist, so I marked it with TODO, taking inspiration from firebase-crashlytics

@nbransby
Copy link
Member

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
Copy link
Contributor Author

mr-kew commented May 29, 2024

@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
Copy link
Member

nbransby commented Jun 3, 2024

@mr-kew
Copy link
Contributor Author

mr-kew commented Jun 3, 2024

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
Copy link
Member

nbransby commented Jun 3, 2024

@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
Copy link
Contributor Author

mr-kew commented Jun 4, 2024

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 nbransby merged commit 54740d7 into GitLiveApp:master Jun 14, 2024
3 checks passed
@nbransby
Copy link
Member

Good stuff

@mr-kew mr-kew deleted the messaging-api-frame branch September 3, 2024 05:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants