Access to full Response object in Converter

[mattinger]

trafficstars

What kind of issue is this?

• [ ] Question. This issue tracker is not the place for questions. If you want to ask how to do something, or to understand why something isn't working the way you expect it to, use Stack Overflow. https://stackoverflow.com/questions/tagged/retrofit

• [ ] Bug report. If you’ve found a bug, spend the time to write a failing test. Bugs with tests get fixed. Here’s an example: https://gist.github.com/swankjesse/6608b4713ad80988cdc9

• [ X] Feature Request. Start by telling us what problem you’re trying to solve. Often a solution already exists! Don’t send pull requests to implement new features without first getting our support. Sometimes we leave features out on purpose to keep the project small.

Our application is consuming hypermedia responses which have some links:

{
    "_links": {
      "foo": {
        "href": "/bar"
      }
    }
}

We want to move to retrofit, and we can use the @Url annotation for dynamic urls.

The issue i'm having is that if the href is absolute, which is a new requirement for us, we will follow it correctly. However the links we get on the resulting page, would still be relative, but would use the baseUrl of the retrofit client, rather than the base of the response where the links were loaded from.

So the first response might have the following link sequence:

https://my.api.com/call link1 -> https://some.other.api.com/call

https://some.other.api.com/call link2 -> /call2

The way we're dealing with this now is to turn all links into absolute links at parse time, since we have the Response object available to us to the get the url that loaded the response.

However, with retrofit, i'm not seeing a way of doing this within a Converter.Factory. It seems to be agnostic to where the response body actually came from. So link2 would still use https://my.api.com as the baseUrl, since that's what's on the retrofit client

I'd like to be able to access the request URL as part of the Converter.Factory instance so that i can use it in my parsing logic.

[JakeWharton]

Yeah I think in the next major version I'll abandon the unified Converter interface and create separate ones so that additional information can be provided. There's a bunch of design here with things like multipart bodies and form URL bodies that will need figured out. Also we're looking to add web socket and SSE support which further complicates.

[JakeWharton]

I can't think of a good workaround for now either.

The only way to sneak data into the converter would be to put it inside the content type. So you could have an interceptor which, when receiving a response, grabbed the final URL and tucked it into the content type. Then your converter would parse out that URL from the content type and use it to resolve the links into absolute ones. Really not great, but it would work...

[mattinger]

I wasn't suggesting changing the Converter interface. It's more the factory class i was thinking. Looking further at the HttpServiceMethod it looks like the body isn't available at the time the responseBodyConverter is being created. So I don't really see a good workaround either. Perhaps i can work around it at a higher level, and post process the object after retrofit returns and examine all the various annotations we use to determine what a url is, and do it then.

[JakeWharton]

Yeah the factory is called once per endpoint and then reused for the life of the instance.

On Tue, Jan 22, 2019, 6:42 PM Matt Inger <[email protected] wrote:

I wasn't suggesting changing the Converter interface. It's more the factory class i was thinking. Looking further at the HttpServiceMethod it looks like the body isn't available at the time the responseBodyConverter is being created. So I don't really see a good workaround either. Perhaps i can work around it at a higher level, and post process the object after retrofit returns and examine all the various annotations we use to determine what a url is, and do it then.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/square/retrofit/issues/2987#issuecomment-456608924, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEEESwmprVQHApw8u1xb8kgzwuBZoW3ks5vF6HqgaJpZM4ZSZ42 .

[JakeWharton]

Widening this to access to the full Response in converting a response. It's been requested for access to headers in the past, although I can't find an issue for that. Perhaps it was requested elsewhere. There is #2480 which covers a converter being able to contribute headers for creating a request.

[PaulKlauser]

Can't find an existing issue for accessing headers in a converter, but here's my +1 for that use case

[zjc17]

Is the issue below solves?

So the first response might have the following link sequence:

https://my.api.com/call link1 -> https://some.other.api.com/call

https://some.other.api.com/call link2 -> /call2

 @Test
  public void getWithJavaUriUrl() {
    class Example {
      @GET
      Call<ResponseBody> method(@Url URI url) {
        return null;
      }
    }

    Request request = buildRequest(Example.class, URI.create("foo/bar/"));
    assertThat(request.method()).isEqualTo("GET");
    assertThat(request.headers().size()).isZero();
    assertThat(request.url().toString()).isEqualTo("http://example.com/foo/bar/");
    assertThat(request.body()).isNull();
  }

  @Test
  public void getWithStringUrlAbsolute() {
    class Example {
      @GET
      Call<ResponseBody> method(@Url String url) {
        return null;
      }
    }

    Request request = buildRequest(Example.class, "https://example2.com/foo/bar/");
    assertThat(request.method()).isEqualTo("GET");
    assertThat(request.headers().size()).isZero();
    assertThat(request.url().toString()).isEqualTo("https://example2.com/foo/bar/");
    assertThat(request.body()).isNull();
  }

[AlfredAndroidTedmob]

My team and I were waiting for a solution on how to read headers from the response into the result JSON DTO with Retrofit. For our code, this would require an update to the GSON converter to add a custom annotation (something like "ResponseHeader") that would set the corresponding field.

Some backend teams keep telling us: "It's the protocol to return stuff like page count, page total, and nonce, in the headers of the response. We don't want to add it in the body just for you."

I currently have to deal with the WooCommerce apis, and several apis return important headers ("X-WP-TotalPages", "X-WC-Store-API-Nonce", ...). The below is my attempt to add the Nonce as part of the DTO needed for the app:

class CartInfoDTO(
    @field:[Expose SerializedName("id")] val id: Long?,
    @field:[Expose SerializedName("created_at")] val createdAt: Long?,
    @field:[Expose SerializedName("items_count")] val itemsCount: Int?,
    @field:[Expose SerializedName("items_qty")] val itemsQty: Int?,
    @field:[Expose SerializedName("customer")] val customer: CustomerDTO?,
    @field:[Expose SerializedName("totals")] val totals: Totals?,
    @field:[Expose SerializedName("coupon_code")] val couponCode: String?,
    @field:[Expose SerializedName("discounted_total_price")] val discountedTotalPrice: Double?,
    @field:[Expose SerializedName("discount_amount")] val discountAmount: Double?,
    var nonce: String?,
)

And having to read the response as is:

@GET
fun getCartInfo(
    @Url url: String = "${ApiContract.SHOP_BASE_URL_ALT}store/cart"
): Observable<Response<CartInfoDTO>>

class GetCartInfoUseCase
@Inject constructor(
        private val api: ShopRestApi,
        @Named("subscribe") subscribeScheduler: Scheduler,
        @Named("observe") observeScheduler: Scheduler
) : UseCase<CartInfoDTO, Unit>(subscribeScheduler, observeScheduler) {
    
    override fun buildUseCaseObservable(params: Unit): Observable<CartInfoDTO> {
        return api.getCartInfo()
            .map {
                if (it.isSuccessful) {
                    it.body()!!
                        .apply { nonce = it.headers()["X-WC-Store-API-Nonce"] }
                } else {
                    throw HttpException(it)
                }
            }
    }
}