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

Add support for specifying a delegate FirebaseMessagingService #191

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

vuletuanbt
Copy link

@vuletuanbt vuletuanbt commented Mar 12, 2021

We're using multiple push providers and thus need to use a delegate service for handling our notifications.

This PR adds the ability to specify a delegate service during instantiation as this library will otherwise throw an NPE when attempting to invoke getReactNativeHost() when handling a remote message.

@vuletuanbt vuletuanbt changed the title feat: support for delegating Add support for specifying a delegate FirebaseMessagingService Mar 12, 2021
@vuletuan
Copy link

@fabriziomoscon, please help us to review this PR

@jdegger
Copy link
Collaborator

jdegger commented Mar 15, 2021

Thank you for this PR @vuletuan I will also notify @fabriziomoscon about this one

@fabriziomoscon
Copy link
Collaborator

fabriziomoscon commented Mar 22, 2021

@vuletuan which FIrebase library version is your app using?
This library use 17.6.+

When testing this I received a crash for an incoming call:

java.lang.RuntimeException: Unable to instantiate service com.hoxfon.react.RNTwilioVoice.fcm.VoiceFirebaseMessagingService: java.lang.InstantiationException: java.lang.Class<com.hoxfon.react.RNTwilioVoice.fcm.VoiceFirebaseMessagingService> has no zero argument constructor
        at android.app.ActivityThread.handleCreateService(ActivityThread.java:4057)
        at android.app.ActivityThread.access$1800(ActivityThread.java:231)
        at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1968)
        at android.os.Handler.dispatchMessage(Handler.java:107)
        at android.os.Looper.loop(Looper.java:214)
        at android.app.ActivityThread.main(ActivityThread.java:7682)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:516)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:950)

I guess that in this PR you should also update the build.gradle file and the README.md

https://firebase.google.com/support/release-notes/android#messaging_v21-0-1

Copy link
Collaborator

@fabriziomoscon fabriziomoscon left a comment

Choose a reason for hiding this comment

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

see comments

@vuletuan
Copy link

@vuletuan which FIrebase library version is your app using?
This library use 17.6.+

When testing this I received a crash for an incoming call:

java.lang.RuntimeException: Unable to instantiate service com.hoxfon.react.RNTwilioVoice.fcm.VoiceFirebaseMessagingService: java.lang.InstantiationException: java.lang.Class<com.hoxfon.react.RNTwilioVoice.fcm.VoiceFirebaseMessagingService> has no zero argument constructor
        at android.app.ActivityThread.handleCreateService(ActivityThread.java:4057)
        at android.app.ActivityThread.access$1800(ActivityThread.java:231)
        at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1968)
        at android.os.Handler.dispatchMessage(Handler.java:107)
        at android.os.Looper.loop(Looper.java:214)
        at android.app.ActivityThread.main(ActivityThread.java:7682)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:516)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:950)

I guess that in this PR you should also update the build.gradle file and the README.md

https://firebase.google.com/support/release-notes/android#messaging_v21-0-1

I'm sorry for my missing. I will update build.gradle and README.md

@fabriziomoscon
Copy link
Collaborator

Thanks @vuletuanbt you need to change this line:
https://github.com/hoxfon/react-native-twilio-programmable-voice/blob/master/android/build.gradle#L61

@vuletuan
Copy link

@fabriziomoscon I'm using firebase-bom(26.4.0) to install firebase-messaging. I guess my firebase-messaging version could be 21.0.1

@vuletuan
Copy link

@fabriziomoscon
Copy link
Collaborator

@vuletuan I followed your instruction and update my app build.gradle with:

    implementation platform('com.google.firebase:firebase-bom:26.7.0')
    implementation 'com.google.firebase:firebase-messaging'

And upon receiving a call the app crashes with:

java.lang.RuntimeException: Unable to instantiate service com.hoxfon.react.RNTwilioVoice.fcm.VoiceFirebaseMessagingService: java.lang.InstantiationException: java.lang.Class<com.hoxfon.react.RNTwilioVoice.fcm.VoiceFirebaseMessagingService> has no zero argument constructor

By reading this https://firebase.google.com/docs/android/learn-more I can see that using the BoM is a major change in the way Firebase dependencies are handled. So I would ask you that you would submit a PR to migrate Firebase to the latest version (before this PR), so we can test that properly and see that calls in all scenarios can be received.

@vuletuan
Copy link

@vuletuan I followed your instruction and update my app build.gradle with:

    implementation platform('com.google.firebase:firebase-bom:26.7.0')

    implementation 'com.google.firebase:firebase-messaging'

And upon receiving a call the app crashes with:

java.lang.RuntimeException: Unable to instantiate service com.hoxfon.react.RNTwilioVoice.fcm.VoiceFirebaseMessagingService: java.lang.InstantiationException: java.lang.Class<com.hoxfon.react.RNTwilioVoice.fcm.VoiceFirebaseMessagingService> has no zero argument constructor

By reading this https://firebase.google.com/docs/android/learn-more I can see that using the BoM is a major change in the way Firebase dependencies are handled. So I would ask you that you would submit a PR to migrate Firebase to the latest version (before this PR), so we can test that properly and see that calls in all scenarios can be received.

It' ok. I will to help you migrate firebase to lastest version. But first, could you try firebase-bom
26.4.0. Just make sure the problem is about firebase-bom version

@fabriziomoscon
Copy link
Collaborator

@vuletuan by reading the error message I attempted to add an empty () cosntructor:

    public VoiceFirebaseMessagingService() {}
    public VoiceFirebaseMessagingService(FirebaseMessagingService delegate) {
        super();
        this.mFirebaseServiceDelegate = delegate;
        callNotificationManager = new CallNotificationManager();
    }

and now I can receive calls with both firebase-bom:
com.google.firebase:firebase-bom:26.7.0 and
com.google.firebase:firebase-bom:26.4.0

@vuletuan
Copy link

@fabriziomoscon I did not run into it. I will add it to my PR. What about the others?

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.

None yet

4 participants