Protected methods in HttpMessageConverterExtractor and DecoderHttpMessageReader to extract MediaType [SPR-16856]

[spring-projects-issues]

GanMing opened SPR-16856 and commented

Some server-side responses to ContentType are incorrect, but we expect to use the right way to extract data. Now I can specify a correct MediaType to extract.

Usually we may need to access many third party services. It is extremely difficult for us to ask all service providers to return ContentType to us according to the standard requirements. So we need to have a capability to specify the MediaType itself.

---

Affects: 5.0.6

Referenced from: pull request #1834, and commits fd946b8

[spring-projects-issues]

Rossen Stoyanchev commented

Even when a content-type is not present, the Java type alone should be sufficient to make the right choice in most cases. For example the JSON in your tests should work fine.

If you really need to fix the content type, you can do it through a filter:

public static void main(String[] args) {

    WebClient client = WebClient.builder()
            .filter((request, next) -> {
                return next.exchange(request)
                        .map(orig -> setContentType(orig, MediaType.APPLICATION_JSON));
            })
            .build();
}

private static ClientResponse setContentType(ClientResponse orig, MediaType mediaType) {
    return ClientResponse.from(orig)
            .headers(headers -> headers.setContentType(mediaType))
            .build();
}

[spring-projects-issues]

GanMing commented

Thank you for your guidance. In essence, I have been thinking about this issue for a long time. I also tried to handle it in a filter, and the reason why I asked for such a request is because I expect not to modify the value returned by the response, but rather to intervene in the choice of the extractor because I worry that I may not know Whether the specific value returned by the response will affect other functions, such as the contentType may return a custom characterset.

So I want to influence it in a smaller scope.

It's even possible that I modified the response's data, causing his signature to fail validation and causing the response to be discarded.

So in fact we need one of these features.

[spring-projects-issues]

GanMing commented

Filter is global. When an instance needs to handle a variety of third-party requests, it is difficult to ensure mutual compatibility.

In addition, when the header participates in the signature request, when the head value is modified and the body is extracted, it must be changed back before verifying the signature, and the head is currently unmodifiable.

Is there a better way to achieve this?

[spring-projects-issues]

Rossen Stoyanchev commented

Filter is global. When an instance needs to handle a variety of third-party requests, it is difficult to ensure mutual compatibility.

Filters can be less global. I've updated the docs to reflect that. You can pass request attributes to filters (per request option), or you can make client replicas with different sets of filters.

It's even possible that I modified the response's data..

I'm not suggesting to modify the response data. Only to fix the content-type.

---

We're talking about REST services that are misbehaving by not setting the "content-type" header, right? So you need a workaround to make up for that but such a workaround does not belong in the main API whose aim is to make easy and promote the main way of doing things. Just so long as there are ways to adapt and work around quirks that are outside of your control.

You can use a filter to set the content type of the response, along with a request attribute to indicate what content-type the filter should use. Or if you don't like that option, for some reason that I cannot yet understand, write your own BodyExtractor. These options may be slightly less convenient but they don't need to be.

[spring-projects-issues]

GanMing commented

I admire your responsible attitude.
Usually access to their own micro-services, this is no problem, the use experience is very good.

However, it is inconvenient to use the access of some third-party service providers. It is possible that my understanding is incorrect.

I think RestTemplate and WebClient are thread-safe when used. In some services, I see different requests using the same instance.

For example, the following class:
https://github.com/spring-projects/spring-security/blob/master/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/userinfo/DefaultReactiveOAuth2UserService.java

Due to the different request ContentType may not be all correct. Need to selectively correct incorrect requests.

A better approach should be to implement a BodyExtractor.

Unfortunately, both readWithMessageReaders and messageReader in BodyExtractors are private and do not provide an alternative to UnsupportedMediaTypeException.

Because of the inability to reference these private methods, writing a new bodyExtractor duplicates a lot of duplicate code and then makes very few changes, so it's awkward to use.

Hope to improve it.

The "global" I said may be incorrect. I want to express that Filter handles all the requests in the instance, but people often only want to modify individual requests when they use it.

Finally, I would like to ask, in addition to "mutate", how to add a filter after "build"?

Because mutate creates a new instance, it may cause unnecessary consumption when the number of requests is large.

---

Comparing "toFormData" and "toMultipartData" in BodyExtractors is just different from ResolvableType and MediaType. These will ignore ContentType. Is it possible to provide a generic method to make it easier to implement toXML, toJSON and so on?

[spring-projects-issues]

Rossen Stoyanchev commented

RestTemplate and WebClient are thread-safe.

I've done an update to improve readability. You'll see to implement BodyExtractor from scratch is trivial. It literally comes down to find a reader and invoke it.

I've also updated HttpMessageConverterExtractor to make the method extracting the MediaType protected.

[spring-projects-issues]

Rossen Stoyanchev commented

Modified title (was: "Allows the BodyExtractor and HttpMessageConverterExtractor to specify a correct MediaType to extract").

[spring-projects-issues]

GanMing commented

Okay, this is much better, so can you update the readability of getContentType in DecoderHttpMessageReader and keep it consistent with HttpMessageConverterExtractor.

In addition, I want to talk to you about my problems. I can't stand these warnings in my code.I don't like using @SuppressWarnings("unchecked").

BodyExtractor<Mono<Map<String, Object>>, ReactiveHttpInputMessage> bodyExtractor = (msg, context) -> {
     ResolvableType elementType = ResolvableType.forClassWithGenerics(Map.class, String.class, Object.class);
     HttpMessageReader<Map<String, Object>> reader = context.messageReaders()
               .stream().filter(messageReader -> messageReader.canRead(elementType, MediaType.APPLICATION_JSON))
               .findFirst()
               //What actually intimidates me is the following unchecked generic cast-cause warning, because it interferes with the developer's focus.
               .map(r -> (HttpMessageReader<Map<String, Object>>) r)
               .orElseThrow(() -> new IllegalStateException(
                                 "No HttpMessageReader for \"" + MediaType.APPLICATION_JSON_VALUE + "\" and \"" + elementType + "\""));
     return reader.readMono(elementType, msg, context.hints());
};

If the "BodyExtractors.findReader" method can be made public, then this can be improved, at least without warning, so that I won't let my code be warned everywhere, such as the following:

BodyExtractor<Mono<Map<String, Object>>, ReactiveHttpInputMessage> bodyExtractor = (msg, context) -> {
     ResolvableType elementType = ResolvableType.forClassWithGenerics(Map.class, String.class, Object.class);
     HttpMessageReader<Map<String, Object>> reader = BodyExtractors.findReader(elementType, MediaType.APPLICATION_JSON, context);
     return reader.readMono(elementType, msg, context.hints());
};

Of course, this method only hides the warning. It is not the best method.

 

For example, to provide a method that can improve the reliability of generic conversion?

public static <T> BodyExtractor<Mono<T>, ReactiveHttpInputMessage> toMono(Class<? extends T> elementClass, MediaType mediaType) {
     ResolvableType elementType = ResolvableType.forClass(elementClass);
     return (inputMessage, context) -> {
          HttpMessageReader<T> reader = findReader(elementType, mediaType, context);
          return reader.readMono(elementType, inputMessage, context.hints());
     };
}

Or can you provide a better guide?

[spring-projects-issues]

GanMing commented

Implement BodyExtractor generic unfriendly from scratch.

[spring-projects-issues]

Rossen Stoyanchev commented

I've updated DecoderHttpMessageReader.

If the "BodyExtractors.findReader" method can be made public

Sorry, but the answer is no. BodyExtractors is specifically designed for use in places where Ctrl+Space should suggest the available BodyExtractor implementations. A method such as findReader is to help implement a BodyExtractor is a completely different purpose, more of an SPI. We could expose such a method elsewhere but given we're talking about 5 lines of code, and there is no obvious place for it.