airbnb / DeepLinkDispatch

A simple, annotation-based library for making deep link handling better on Android
http://nerds.airbnb.com/deeplinkdispatch/
4.37k stars 407 forks source link

Unit Test for DeepLink URI with Method Annotations #299

Closed tinder-yisongwu closed 3 years ago

tinder-yisongwu commented 4 years ago

Hey team,

We have a unit test question for the Method Annotations of DeepLinkDispatch library. We would like properly unit test the case that a method is properly annotate with @DeepLink of "https://example.com", such as the example below:

object DeeplinkIntents {
  @JvmStatic 
  @DeepLink("https://example.com")
  fun defaultIntent(context: Context, extras: Bundle): Intent {
    return Intent(context, MyActivity::class.java)
  }
}

In order to do that, we need to use reflection to grab the method annotation, but currently in the DeepLinkDispatch Library, the DeepLink.jave annotation has @Retention(RetentionPolicy.CLASS) which disables us from grabbing the annotation.

Is there a reason to apply @Retention(RetentionPolicy.CLASS) instead of @Retention(RetentionPolicy.RUNTIME) here?

Thanks, Yisong

tinder-yisongwu commented 3 years ago

Hey team, what would be the suggested way to unit test @DeepLink("myScheme://testUri") given it is annotated with @retention(RetentionPolicy.CLASS)?

rossbacher commented 3 years ago

The reason it is annotate the way it is is that after complication the annotation is only needed for byte code analyzers and processing tools (e.g. like Proguard/R8 or Dexguard) as we need to keep those methods as they are never directly called and DLD is using reflection to call them after a match. They are not required at runtime at all, so they are just useless ballast that can be deleted.

However we will change the visibility to RetentionPolicy.RUNTIME anyway as we use Dexguard and Google does not consider this a bug https://issuetracker.google.com/issues/168524920 So that should solve your problem.

If you want to somehow create a string match between method/function name and deeplink this way I think a custom lint check might also be an alternative here btw.

rossbacher commented 3 years ago

This is resolved with release https://github.com/airbnb/DeepLinkDispatch/releases/tag/5.3.0