search

permissions-dispatcher / PermissionsDispatcher

A declarative API to handle Android runtime permissions.
https://github.com/permissions-dispatcher/PermissionsDispatcher
Apache License 2.0
11.21k stars 1.44k forks source link

fix(ktx): observe ViewModel only when a permission has not been granted #681

Closed hotchemi closed 3 years ago

hotchemi commented 3 years ago

Currently, we allow multiple observers to receive the permission change event but it might cause an unexpected issue.

ref: https://github.com/yt-tkhs/permissionsdispatcher-ktx-issue

yt-tkhs commented 3 years ago

The issue is not fixed on this PR :sob:

But I investigated the issue and found the cause of problem:

After calling PermissionsRequester.launch and permission is granted, PermissionResult.GRANTED is emitted to the permissionRequestResult.

here: https://github.com/permissions-dispatcher/PermissionsDispatcher/blob/master/ktx/src/main/java/permissions/dispatcher/ktx/PermissionRequestViewModel.kt#L6

After that, we call PermissionsRequester.launch again, then requiresPermissions is called 2 points below:

  1. the LiveData permissionRequestResult has initial value which is emitted when permissions is granted at first. so when observe at the next time, initial value (PermissionResult.GRANTED) will be emitted to observer unexpectedly. https://github.com/permissions-dispatcher/PermissionsDispatcher/blob/0f9071451da607319a53b26193898ca08308185b/ktx/src/main/java/permissions/dispatcher/ktx/PermissionRequestViewModel.kt#L17-L19
  2. permission is already granted, so requiresPermissions will be called here. https://github.com/permissions-dispatcher/PermissionsDispatcher/blob/0f9071451da607319a53b26193898ca08308185b/ktx/src/main/java/permissions/dispatcher/ktx/PermissionsRequesterImpl.kt#L26-L29

From above, we can probably fix this issue in one of following ways:

hotchemi commented 3 years ago

cool! let me take a look again🙏

yt-tkhs commented 3 years ago

Thank you so much 🙏

For your reference, this commit can fix the issue: https://github.com/yt-tkhs/PermissionsDispatcher/commit/6b914ba4d6685672f2c8576dbce87a5d90aee01a (Fixed by observing PermissionResult only when permission is not granted. but maybe there are any bad side effects 🤔)

hotchemi commented 3 years ago

I've modified as 506d50524154b41d4e6485b2656751299069982a, let me merge!