OneSignal / OneSignal-Android-SDK

OneSignal is a free push notification service for mobile apps. This plugin makes it easy to integrate your native Android or Amazon app with OneSignal. https://onesignal.com
Other
606 stars 371 forks source link

[Bug]: AppId change does not work #1704

Closed Danielku15 closed 8 months ago

Danielku15 commented 1 year ago

What happened?

During testing we are changing currently our appId which seemingly is supported by the SDK according to the sources we have seen. We enabled trace logging and could see that the appId change is detected and some registrations are triggered. But then when it comes to the UserStateSynchronizer where a internalSyncUserState is triggered, we see that it sends still the old app ID to the API.

This is wrong because the new app ID should be sent so that the user is moved to the other app.

Steps to reproduce?

Note: I am using The Xamarin library (4.3.0) but the problem seem to be in the android code part. 
1. `com.onesignal.OneSignal.setAppId("ID1")`
2. `com.onesignal.OneSignal.initWithContext(context)`
3. `com.onesignal.OneSignal.setAppId("ID2")`
4. `com.onesignal.OneSignal.initWithContext(context)`

What did you expect to happen?

The device/user should be visible in the the app with ID2 but he is still in ID1 and pushes do not work consequently.

I think the problem resides in the clearing of the current and sync state of the user. here the current state is cleared which leads to the current state cleared here. But here the old toSyncUserState is used and synchronized into the state giving us again the old app ID.

IMO the toSyncUserState also needs to be cleared as part of the appId change workflows.

OneSignal Android SDK version

4.3.0 (Xamarin which seem to wrap 4.8.1 Android SDK

Android version

10

Specific Android models

No response

Relevant log output

[OneSignal] App id has changed:
[OneSignal] From: a677bf34-a2c6-4356-bbb2-c61cc08b5686
[OneSignal]  To: 532a24b3-6b86-4854-9e4f-db2831e79f11
[OneSignal] Clearing the user id, app state, and remoteParams as they are no longer valid
[OneSignal] Last session time set to: -3660
[OneSignal] initWithCachedInAppMessages: null
[OneSignal] Location permissions not added on AndroidManifest file
[OneSignal] LocationController sendAndClearPromptHandlers from non prompt flow
[OneSignal] registerUser:registerForPushFired:true, locationFired: true, remoteParams: null, appId: 532a24b3-6b86-4854-9e4f-db2831e79f11
[OneSignal] registerUser not possible
[OneSignal] Starting request to get Android parameters.
[OneSignal] startPendingTasks with task queue quantity: 0
[OneSignal] OneSignalRestClient: Making request to: https://api.onesignal.com/apps/532a24b3-6b86-4854-9e4f-db2831e79f11/android_params.js
[OneSignal] OneSignalRestClient: Adding header if-none-match: W/"42dc5c955594ca13db1061d845e88390"
[OneSignal] OneSignalRestClient: GET - Using Cached response due to 304: {"awl_list":{},"android_sender_id":"915921981681","chnl_lst":[],"outcomes":{"direct":{"enabled":false},"indirect":{"notification_attribution":{"minutes_since_displayed":60,"limit":10},"enabled":false},"unattributed":{"enabled":false}},"receive_receipts_enable":false}
[OneSignal] OneSignal saveInfluenceParams: InfluenceParams{indirectNotificationAttributionWindow=60, notificationLimit=10, indirectIAMAttributionWindow=1440, iamLimit=10, directEnabled=false, indirectEnabled=false, unattributedEnabled=false}
[OneSignal] Device registered, push token = .....
[OneSignal] registerForPushToken completed with id: ..... status: 1
[OneSignal] registerUser:registerForPushFired:true, locationFired: true, remoteParams: com.onesignal.OneSignalRemoteParams$2@a3334f, appId: null
[OneSignal] registerUser not possible
[OneSignal] UserStateSynchronizer internalSyncUserState from session call: true jsonBody: {"app_id":"a677bf34-a2c6-4356-bbb2-c61cc08b5686","device_os":"10","timezone":3600,"timezone_id":"Europe\/Zurich","language":"en","sdk":"040801","sdk_type":"native","android_package":"com.companyname.pushdemo","device_model":"....","game_version":1,"net_type":1,"carrier":"Salt","rooted":false,"identifier":"......","device_type":1,"tags":{"country":"ch","region":"none","general-news":"true","network-infrastructure":"true","marketing":"false"}}
[OneSignal] OneSignalRestClient: Making request to: https://api.onesignal.com/players
[OneSignal] OneSignalRestClient: POST SEND JSON: {"app_id":"a677bf34-a2c6-4356-bbb2-c61cc08b5686","device_os":"10","timezone":3600,"timezone_id":"Europe\/Zurich","language":"en","sdk":"040801","sdk_type":"native","android_package":"com.companyname.pushdemo","device_model":"....","game_version":1,"net_type":1,"carrier":"Salt","rooted":false,"identifier":".....","device_type":1,"tags":{"country":"ch","region":"none","general-news":"true","network-infrastructure":"true","marketing":"false"}}
[OneSignal] OneSignalRestClient: Successfully finished request to: https://api.onesignal.com/players
[OneSignal] OneSignalRestClient: POST RECEIVED JSON: {"success":true,"id":"cda7d027-8cfd-4779-96a8-b3e5965c0603"}
[OneSignal] doCreateOrNewSession:response: {"success":true,"id":"cda7d027-8cfd-4779-96a8-b3e5965c0603"}
[OneSignal] Device registered, UserId = cda7d027-8cfd-4779-96a8-b3e5965c0603
Thread finished: <Thread Pool> #8
The thread 0x8 has exited with code 0 (0x0).

Code of Conduct

Danielku15 commented 1 year ago

I was able to solve the problem for me. It seems the problem is mainly on the Xamarin wrapper side because it does not expose a plain setAppId but only an overall Initialize which always calls setAppId and `initWithContext. See here

It is crucial that initWithContext is only called once, and setAppId multiple times. The fast second call to initWithContext messes with some asynchronous operations performed when the appId changes.

In detail here is what happens:

  1. The setAppId with a changed ID will clear some internal details like the remoteParams. And when already initialized it will directly start the process to re-initialize.
  2. The immediate call to initWithContext will cause a double re-initialization and it enters this code path where the appId is reset to null for a delayed initialization after the remoteParams are loaded.
  3. When now the init process of the step 1 finishes. appId will be null and the device information will not be properly updated.
  4. Then when the user synchronization happens again. we can see that the old app id is somehow active through the sync state. But actually the overall internal state is messed up.

While the problem is maybe caused by a user error through the lack of exposed APIs on the Xamarin layer, it would be maybe a fix in the Android SDK.

The Android SDK could remember and check whether an initialization is already performed (with async/delayed operations) and do not initiate a second one when called.

brismithers commented 1 year ago

Hi @Danielku15 thank you for the detailed analysis, very helpful! Although the API indicates that the appId can be changed after initialization, it isn't supported. In our next major release our current plan is to clear this up by moving the setting the appId as part of the initialization (i.e. initWithContext(context, "YOUR_APP_ID").

Can you give a little more detail on the use case for changing the appId post-initialization and I can share that with the team. Thanks so much!

Danielku15 commented 1 year ago

This would be a very unfortunate decision. We are currently in the evaluation phase to decide whether OneSignal can serve our usecase. Due to the missing features for a more hierarchical segmentation and permission management in the web portal the dynamic changing of app IDs seems to be currently the only option to achive our needs.

With this functionality planned to be removed, we would have to strike out OneSignal as potential options for our future (push-) notification needs.

Our use case is: The user chooses during the login into our app the country he resides in and then logs in with his credentials. This separation is not directly a location based one, but depends on where our customers have bought regional services where our app will provide auxiliary services and information around those services. Each country manages their communication individually with own "topics" a user can subscribe to and dedicated managers for the countries to send out those notifications to their customer base. User user might logout/login into different countries (always only one).

We already had one session with a sales representative about the possibilities of OneSignal with another session tomorrow. OneSignal until now does not seem to offer such a segmentation capability. Our current approach would have been to initialize the SDK after the login once it is known which country and user the customer is using. The achieve the segmentation and administration needs we would have registered one app per country with the particular configuration and switch the app-ids during the login procedure. This is also where this issue comes into place where we would need to rather synchronously disable the pushes for the old country and activate it for the new country on logout/login.

jennantilla commented 8 months ago

Apologies that we missed responding back to you, @Danielku15. Rest assured your comments were shared internally.

Although changing App IDs is still not supported, use cases like yours continue to inform future product decisions. We highly value your feedback!

We'd still love for you to check our our latest major release and consider upgrading for new features and enhancements.

Thanks!