Allow parameters with custom annotations in service methods

[HylkeB]

What
Support for parameters with custom annotations.
For example:

annotation class CustomAnnotation

interface SomeApiService {
  @GET("operation")
  fun getData(@CustomAnnotation customValue: String)
}

Why
When implementing a custom Call.Factory implementation, its nice to be able to access custom data provided to the request. We can access the Invocation from the newCall method, which can be used to inspect all annotations (both method and parameter annotations), as well as the arguments provided. Using this we can access what value is provided for the argument annotated with @CustomAnnotation. See example below:

class CustomCallFactory : Call.Factory {
  override fun newCall(request: Request): Call {
    val invocation = request.tag(Invocation::class.java)!!
    val customAnnotationArgumentIndex = invocation.method().parameterAnnotations.indexOfFirst { it.find { it is CustomAnnotation } } ?: -1
    if (customAnnotationArgumentIndex != -1) {
      val customValue = invocation.arguments()[customAnnotationArgumentIndex]
    }
  }
}

In my particular use-case, I want to use this for a @ForceRefresh annotation, since I am trying to implement a secure caching solution (protected by an app specific pincode and AndroidKeystore, backed by an sqlcipher database), and sometimes you dont want to use the cache (hence the ForceRefresh annotation). Custom method annotations is not an issue luckily, but custom argument annotations are not supported yet.

How
Don't throw the No Retrofit annotation found. exception, but instead only throw an exception when there is a parameter without any annotation. Furthermore I made the logic that applies the handlers null-aware for this and I updated the unit test assertion.

[JakeWharton]

I'm not satisfied with the implementation as-is, but I'm not opposed to the idea. We've long wanted custom parameter annotations which support things like arbitrarily mutating the request but it's a complicated thing to implement with a large API surface.

The biggest problem that I see on first glance is that it's far too easy to create an argument that does nothing. At the very least I would want each annotation to be registered and arguments with no built-in nor registered annotations are still a failure. This would ensure, at the very least, that an argument is never simply dropped.

I'm not sure whether it's worth it to add this feature as-is or in a similar manner to as proposed or try to tackle the larger problem of completely arbitrary arguments which can mutate things like how the request is created. We already have this mechanism internally, but this would mean exposing it in a general way.

Also there are no tests so before doing anything we need to ensure coverage.

[HylkeB]

Thanks for your feedback. I have updated the PR to include the required unit test so custom annotations have test coverage.

I agree with your statement that a more integrated way of providing handlers for custom annotations would be better. To support this I think we need to do the following:

• The abstract ParameterHandler class needs to be public
• The RequestFactory.Builder class needs to be public
  • Also the internal methods will need to be public
• Add a means to register custom ParameterHandlers to custom Annotations on a retrofit instance
  • Much like the addConverterFactory and addCallAdapterFactory

This way developers can add custom parameter handlers to retrofit. Using the existing methods on RequestFactory.Builder this ParameterHandler can modify the request good enough I think. Another possibility would be to also expose the private OkHttp Request.Builder, so you can do literally everything, but I am not sure if that is a good idea or not.

I think I will have time to create a separate PR for the better approach, so we can compare them.

@JakeWharton Can you give your opinion on this proposed solution?

p.s.
I have no idea why the github build checks fail, I feel like my changes should not cause the pipelines to break.

[HylkeB]

I have attempted to implement a better way of supporting custom parameter annotations, but my first intuition of using ParameterHandlers and making the retrofit.RequestBuilder class public became quite a mess and also did not solve the problem completely.

Instead I opted for a more naive approach where the developer has full freedom over the request if he so pleases. See this PR

What I found out when trying to implement custom parameter annotations in the way described in my previous comment is the following:

• You only have the very limited means of customizing a request provided in the retrofit.RequestBuilder at your disposal
  • To be able to do everything to the request, the retrofit.RequestBuilder would become basically a copy of okhttp3.Request.Builder
• You only have means to change the request based on parameter annotations, but its not possible change the request based on method annotation