spring-cloud / spring-cloud-openfeign

Support for using OpenFeign in Spring Cloud apps
Apache License 2.0
1.21k stars 784 forks source link

SpringEncoder support easier customization for binary encodings #734

Closed liubao68 closed 1 year ago

liubao68 commented 2 years ago

Is your feature request related to a problem? Please describe. My application needs support a x-application/hessian2 media type, and we created a HttpMessageConverter like this:

public class HessianHttpMessageConverter extends AbstractGenericHttpMessageConverter<Object> {
  public static final String HESSIAN_MEDIA_TYPE_VALUE = "x-application/hessian2";

  public static final MediaType HESSIAN_MEDIA_TYPE = MediaType.valueOf(HESSIAN_MEDIA_TYPE_VALUE);

  public HessianHttpMessageConverter() {
    super(HESSIAN_MEDIA_TYPE);
  }

  @Override
  protected boolean supports(Class<?> clazz) {
    return true;
  }

  @Override
  protected boolean canWrite(MediaType mediaType) {
    return HESSIAN_MEDIA_TYPE.equalsTypeAndSubtype(mediaType);
  }

  @Override
  protected boolean canRead(MediaType mediaType) {
    return HESSIAN_MEDIA_TYPE.equalsTypeAndSubtype(mediaType);
  }

.... other codes

When I using feign with ApacheHttpClient, feign will give a charset of UTF-8, and HttpClient will encode the final stream to UTF-8 string and data get losed.

SpringEncoder charset code snippet:

Charset charset;

MediaType contentType = outputMessage.getHeaders().getContentType();
Charset charsetFromContentType = contentType != null ? contentType.getCharset() : null;

if (encoderProperties != null && encoderProperties.isCharsetFromContentType()
        && charsetFromContentType != null) {
    charset = charsetFromContentType;
}
else if (shouldHaveNullCharset(messageConverter, outputMessage)) {
    charset = null;
}
else {
    charset = StandardCharsets.UTF_8;
}
request.body(outputMessage.getOutputStream().toByteArray(), charset);
return;
}

Describe the solution you'd like

add a easier way for extending SpringEncoder to support other binary type. e.g.

    private boolean binaryContentType(FeignOutputMessage outputMessage) {
        MediaType contentType = outputMessage.getHeaders().getContentType();
        return contentType == null || Stream
                .of(MediaType.APPLICATION_CBOR, MediaType.APPLICATION_OCTET_STREAM, MediaType.APPLICATION_PDF,
                        MediaType.IMAGE_GIF, MediaType.IMAGE_JPEG, MediaType.IMAGE_PNG)
                .anyMatch(mediaType -> mediaType.includes(contentType));
    }

change to

    protected boolean binaryContentType(FeignOutputMessage outputMessage) {
        MediaType contentType = outputMessage.getHeaders().getContentType();
        return contentType == null || Stream
                .of(MediaType.APPLICATION_CBOR, MediaType.APPLICATION_OCTET_STREAM, MediaType.APPLICATION_PDF,
                        MediaType.IMAGE_GIF, MediaType.IMAGE_JPEG, MediaType.IMAGE_PNG)
                .anyMatch(mediaType -> mediaType.includes(contentType));
    }

Describe alternatives you've considered Current I copied the code from SpringEncoder and changed the binaryContentType method to

  private boolean binaryContentType(FeignOutputMessage outputMessage) {
    MediaType contentType = outputMessage.getHeaders().getContentType();
    return contentType == null || HESSIAN_MEDIA_TYPE.includes(contentType) || Stream
        .of(MediaType.APPLICATION_CBOR, MediaType.APPLICATION_OCTET_STREAM, MediaType.APPLICATION_PDF,
            MediaType.IMAGE_GIF, MediaType.IMAGE_JPEG, MediaType.IMAGE_PNG)
        .anyMatch(mediaType -> mediaType.includes(contentType));
  }

Additional context

When I use feign with java URLHttpConnection do not have this problem, because URLHttpConnection will not encode body based on the charset.

OlgaMaciaszek commented 2 years ago

Hello @liubao68, thanks for reporting this. Makes sense.

liubao68 commented 1 year ago

@OlgaMaciaszek I am integrating new version and make a test, and find that this fix is not complete because FeignOutputMessage has private access. I can not create a class to inherit SpringEncoder

public class ExtendedSpringEncoder extends SpringEncoder {
  // ...

  @Override
  protected boolean binaryContentType(FeignOutputMessage outputMessage) {
     // ...
  }
}

will have compile error.

OlgaMaciaszek commented 1 year ago

Thanks, @liubao68. Will fix it.