Permissionsdispatcher: Allow the specification of the request code for each @NeedsPermission method

[jasonwyatt]

It would be nice to be able to manually specify the requestCode values used for each @NeedsPermission method. Something like the following:

@NeedsPermission(value = Manifest.permission.WRITE_EXTERNAL_STORAGE, requestCode = 1337)
void writeSomethingToStorage() {
    // do it
}

This would allow consumers of PermissionsDispatcher to not have to worry about collisions with their own requestCodes they may already have in place for other purposes.

[mannodermaus]

Yeah, we've actually discussed this before. It would require some additional logic on the annotation processor's side, since it currently uses auto-incrementing integers as request codes. If we allow users to provide their own codes, this could cause collisions. I'm open to the idea though, and it should be a fairly straightforward extension that I could deliver over the next couple of days.

What do you think, @hotchemi @shiraji?

[hotchemi]

Well, before implementing that I'd like to clarify the exact use case.

they may already have in place for other purposes

I can't come up with the situation since request codes are generated incrementally and these are only used inside of the generated java class right? They wouldn't collide, I guess.

@jasonwyatt Can you give us more detail?

[jasonwyatt]

A simple example would be where you've got an activity which is already performing a check for permissions using the ActivityCompat apis directly. Clearly, it must have already defined a particular requestCode that it associates with that existing permission request.

If that requestCode was set to 0, then it will collide with the generated requestCode used by PermissionsDispatcher the very first time the consumer attempts to integrate.

[hotchemi]

Noted. But in that sense how about replacing existing permissions code to PermisisonsDispatcher?
I guess it simplifies the code base and you don't have to worry about requestCode stuff anymore.

[jasonwyatt]

I suppose that could work.. What would that look like?

[hotchemi]

Umm? I dunno your code but can't you replace with PermissionsDispatcher's annotations?
I mean you don't have to keep requestCode value so just replace and it would work.

[jasonwyatt]

Ohh I thought you meant something new would go into PermissionsDispatcher to pass the requestCode along to the xxxxxxWithCheck(...) methods.

Yeah that could work, and is what I did on my project, but it might be nice to add the flexibility for other consumers of the library (especially large consumers, to make adopting PermissionsDispatcher in an incremental way more feasible)

[mannodermaus]

I agree that the environment of the runtime permission model, as exposed through PermissionsDispatcher, should remain as hermetic as possible, since the benefits to opening up something like custom request codes are negligible for all use cases that I can come up with. If replacing your existing manual logic with PermissionsDispatcher works for you, @jasonwyatt, then this should be preferred for the time being. I would argue that since consumers can apply the library gradually, and we're not forcing anybody to switch out every piece of code that deals with RP at once, the incremental approach is already feasible.

[hotchemi]

Totally agree with @aurae 's thought.
Say, we maintain team definitely wanna keep the library interface simple as much as possible.
If user can deal with the problem with current API that's perfect, so let us re-consider when we find the problem that user can't handle anymore with current API.
Anyway thx for your proposal!