thomasgalliker / Plugin.FirebasePushNotifications

Receive and handle firebase push notifications in .NET MAUI apps
MIT License
66 stars 5 forks source link

[Bug] [iOS] Unable to subscribe to topics if not initially registering for push notification #39

Closed sovdchains closed 3 months ago

sovdchains commented 4 months ago

Description

When subscribing to a topic on iOS after first-time registration has already been done previously, the topic subscription will be enqueued indefinitely as FirebasePushNotificationManager.DidReceiveRegistrationToken will never be called resulting in FirebasePushNotificationManager.hasToken never being true and therefore not enabling topic subscription.

Manually setting hasToken to true via reflection (if a Token is given) makes topic subscription possible.

Steps to Reproduce

  1. RegisterForPushNotificationsAsync
  2. Restart
  3. SubscribeTopic

Expected Behavior

Sucessful topic subscription

Actual Behavior

Endless enqueueing

Basic Information

WeirdAnakin commented 4 months ago

What does >restart< mean in that context? Here's my initialization code, and I fail to receive pushes on iOS, but works fine on Android:

                await this.RequestNotificationPermissionsAsync();
                await this.RegisterForPushNotificationsAsync();

                this.firebasePushNotification.SubscribeTopic("com.mycompany.myapp");

                await this.SubscribeEventsAsync();

In AppDelegate, RegisteredForRemoteNotifications is called and I get a valid token, but DidReceiveRemoteNotification is never called.

sovdchains commented 4 months ago

With restart I mean restarting the app itself therefore reinitializing the plugins objects. As far as I understand it should work when there has not been a token requested before. So if you start with a blank installation, request the token and then subscribe the topic it should have worked. I did not test this thoroughly though.

I am currently using this workaround:

internal sealed class PushNotificationService
{
#if IOS
    private static readonly FieldInfo? HasTokenFieldInfo = typeof(FirebasePushNotificationManager).GetField("hasToken", BindingFlags.Instance | BindingFlags.NonPublic);
#endif

    private readonly INotificationPermissions _notificationPermissions;
    private readonly IFirebasePushNotification _firebasePushNotification;

    public PushNotificationService(INotificationPermissions notificationPermissions, IFirebasePushNotification firebasePushNotification)
    {
        _notificationPermissions = notificationPermissions;
        _firebasePushNotification = firebasePushNotification;
    }

    public async Task RegisterForPushNotificationsAsync()
    {
        if (!await _notificationPermissions.RequestPermissionAsync())
            return;

        await _firebasePushNotification.RegisterForPushNotificationsAsync();
        Debug.WriteLine($"Push Token: {_firebasePushNotification.Token}");

#if IOS
        // Workaround for https://github.com/thomasgalliker/Plugin.FirebasePushNotifications/issues/39
        if (!string.IsNullOrEmpty(_firebasePushNotification.Token) && HasTokenFieldInfo is not null)
            HasTokenFieldInfo.SetValue(_firebasePushNotification, true);
#endif

        _firebasePushNotification.SubscribeTopic("all");
    }
}
thomasgalliker commented 3 months ago

Guys, can you try again with the latest version of the plugin? The SubscribeTopic code is still pretty old and needs a refactring. However, I updated a lot of code in the area of token handling on iOS. So, there is a chance that #30 solves this issue here too.

sovdchains commented 3 months ago

@thomasgalliker It seems like hasToken still will only be updated after receiving a new token from FCM. When reusing an already registered one hasToken will still be false, I did not try to register a new topic, but I assume this check still will stop the actual subscription

thomasgalliker commented 3 months ago

Ok ok. I‘ll have a look.

thomasgalliker commented 3 months ago

@sovdchains the hasToken field no longer exists in the latest 2.3 pre-release. Can you remove your workaround and test with the latest pre-release if it works now?

What has changed? I no longer maintain this hasToken - instead, I check if ApnsToken is null.

Question 1 Would it make sense to use a presistent queue for enqueuing topics while no ApnsToken is available? I ask this because today it would not be possible to call SubscribeTopic --> Restart the app --> RegisterForPushNotificationsAsync and then have the topic subscribed automatically. There pendingTopics queue is a simple in-memory queue.

Question 2 Does it make sense to UnsubscribeAllTopics when UnregisterPushNotificationsAsync is called? Obviously, you will no longer receive topic-targeted push notifications when you call UnregisterPushNotificationsAsync - but if you call RegisterForPushNotificationsAsync again, you'd like to have the same list of topics subscribed again, no?

sovdchains commented 3 months ago

@thomasgalliker this seems to work perfectly fine now! On which branch can I find those changes? I would like to take a look to know what has changed in the code.

Question 1 I do not really know if I'll ever have that scenario that I try to subscribe without having received a token yet. In my tests the token always has been fetched when I wanted to subscribe to the topics. But maybe that's just me and my architecture; currently I'll always subscribe after registering on startup to ensure that I'm always up to date with the common topics.

Question 2 I think those concerns should stay separated as I always can unsubscribe them manually. I do not know a lot about the backend of FCM Push Notifications, but if I stay connected to my topics even after unregistering that could be called a feature I guess?

thomasgalliker commented 3 months ago

You can find the changes in PR #51