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

Push device deregistration does not clear/ reset device details #1177

Closed
ben-xD opened this issue Sep 14, 2021 · 6 comments · Fixed by #1259
Closed

Push device deregistration does not clear/ reset device details #1177

ben-xD opened this issue Sep 14, 2021 · 6 comments · Fixed by #1259
Assignees
Labels
bug Something isn't working. It's clear that this does need to be fixed.

Comments

@ben-xD
Copy link
Contributor

ben-xD commented Sep 14, 2021

This is quite low priority, but here it is:

As per RSH3g2a:

On On event Deregistered, Clears all local DeviceDetails.

Unfortunately, ably-cocoa does not clear local device details.

In Java, this can be seen by:
https://github.com/ably/ably-java/blob/2b8e3c7e65ee7109f225cad115df38d61869fc3f/android/src/main/java/io/ably/lib/push/ActivationStateMachine.java#L483

Ably-cocoa does not even have a method to reset the device details, and so is definitely not calling this.

This can be seen functionality in the Ably flutter app, in the following recording:

Screen.Recording.2021-09-14.at.15.03.47.mov
  • On the left is Ably-flutter running on Android. I activate and deactivate, and the device details clearly changes.
  • On the right is Ably-flutter running on iOS, and activation/ deactivation doesn't change the device details.
  • iOS 🐛: When deactivating the device, the LocalDevice should be reset. It should not keep the same local device information. This means the deviceId and APNs device token (registration tokens on Android) could be different. The APNs device token on iOS and FCM registration token on Android remains the same. A different APNs device token might be possible on iOS, potentially by calling unregisterForRemoteNotifications. You could test this.

Note on the flutter app: The app is just calling the device method to show the device information.

┆Issue is synchronized with this Jira Bug by Unito

@ben-xD ben-xD added the bug Something isn't working. It's clear that this does need to be fixed. label Sep 14, 2021
@maratal
Copy link
Collaborator

maratal commented Oct 1, 2021

Isn't [local setAndPersistIdentityTokenDetails:nil]; in ARTPushActivationStateWaitingForDeregistration:transition is for?

@ben-xD
Copy link
Contributor Author

ben-xD commented Oct 4, 2021

I'm not sure what you're saying @maratal, can you clarify? 🙂

@maratal maratal linked a pull request Oct 4, 2021 that will close this issue
@maratal
Copy link
Collaborator

maratal commented Oct 4, 2021

@ben-xD couldn't you play with PR branch? push-interactive-app is so lame, it requires a lot of work to fix it (I'm into it now). Thanks.

@ben-xD
Copy link
Contributor Author

ben-xD commented Oct 11, 2021

I initially didn't understand Marat's comments but he clarified it with me.

Isn't [local setAndPersistIdentityTokenDetails:nil]; in ARTPushActivationStateWaitingForDeregistration:transition is for?

He said this is an outdated comment and not relevant anymore.

@ben-xD couldn't you play with PR branch? push-interactive-app is so lame, it requires a lot of work to fix it (I'm into it now). Thanks.

He wants me to test it in Ably-Flutter because https://github.com/ably/push-example-ios is not well written, and we definitely agree it needs significant improvements.

@maratal
Copy link
Collaborator

maratal commented Jan 21, 2022

Reopening due to #1272

@maratal maratal reopened this Jan 21, 2022
maratal added a commit that referenced this issue Nov 12, 2023
…e details) and only resets clientId, since it affects push device registration with a different clientId (after deregistration with previous clientId). This only affects clients that do not restart their apps after deregistration, since clientId is loaded into deviceId once per app launch. This commit resets clientId after deregistration.
maratal added a commit that referenced this issue Nov 12, 2023
…e details) and only resets clientId, since it affects push device registration with a different clientId (after deregistration with previous clientId). This only affects clients that do not restart their apps after deregistration, since clientId is loaded into deviceId once per app launch. This commit resets clientId after deregistration.
maratal added a commit that referenced this issue Nov 12, 2023
…e details) and only resets clientId, since it affects push device registration with a different clientId (after deregistration with previous clientId). This only affects clients that do not restart their apps after deregistration, since clientId is loaded into deviceId once per app launch. This commit resets clientId after deregistration.
@maratal
Copy link
Collaborator

maratal commented Nov 14, 2023

Partially fixed by #1832

maratal added a commit that referenced this issue Dec 10, 2023
…ar device details) and only resets clientId, since it affects push device registration with a different clientId (after deregistration with previous clientId). This only affects clients that do not restart their apps after deregistration, since clientId is loaded into deviceId once per app launch. This commit resets clientId after deregistration."

This reverts commit bff9205.
@sync-by-unito sync-by-unito bot closed this as completed Jan 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working. It's clear that this does need to be fixed.
3 participants