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

[question] remove delegate permission dispatcher #56

Closed
sibelius opened this issue Dec 9, 2015 · 7 comments
Closed

[question] remove delegate permission dispatcher #56

sibelius opened this issue Dec 9, 2015 · 7 comments

Comments

@sibelius
Copy link
Contributor

sibelius commented Dec 9, 2015

I wonder whether it is possible to remove the delegate pattern used in this lib, and make even easier to handle permissions

@mannodermaus
Copy link
Contributor

What would you suggest instead?

@sibelius
Copy link
Contributor Author

sibelius commented Dec 9, 2015

When we call a method annotated @NeedsPermission, it should call MainActivityPermissionsDispatcher.methodWithCheck(this) instead.

But I don't know whether it is easy or possible to code this

@mannodermaus
Copy link
Contributor

I'm afraid that this isn't really possible, since that would require byte code manipulation of method invocations, which is beyond the scope of both us and the library. However, now that I think about it, we may look into creating custom lint rules for the library, so that your IDE shows a warning when you're calling an annotated method directly, potentially with a quick fix to change the code to the correct method on the delegate object.

@hotchemi
Copy link
Member

I don't like this idea.
We can do that when we manipulate byte code but why we have to do that?
The purpose of annotation processing is to delegate the work to generated class.

@mannodermaus
Copy link
Contributor

I do think that preventing users from accidentally calling permission-protected code directly should be something we need to consider. Using Lint to raise warnings (or even errors) when people bypass the "delegate call chain" should be on our radar.

@mannodermaus
Copy link
Contributor

This discussion will be moved to #59.

@sibelius
Copy link
Contributor Author

@aurae thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants