Fix cloning issue in CodecConfigurer for multipart writers

Closes gh-24194
This commit is contained in:
Rossen Stoyanchev 2019-12-12 21:55:13 +00:00
parent dd9b6287b4
commit b23617637d
5 changed files with 74 additions and 42 deletions

View File

@ -105,25 +105,12 @@ abstract class BaseCodecConfigurer implements CodecConfigurer {
@Override
public List<HttpMessageWriter<?>> getWriters() {
this.defaultCodecs.applyDefaultConfig(this.customCodecs);
return getWritersInternal(false);
}
/**
* Internal method that returns the configured writers.
* @param forMultipart whether to returns writers for general use ("false"),
* or for multipart requests only ("true"). Generally the two sets are the
* same except for the multipart writer itself.
*/
protected List<HttpMessageWriter<?>> getWritersInternal(boolean forMultipart) {
List<HttpMessageWriter<?>> result = new ArrayList<>();
result.addAll(this.customCodecs.getTypedWriters().keySet());
result.addAll(this.defaultCodecs.getTypedWriters(forMultipart));
result.addAll(this.defaultCodecs.getTypedWriters());
result.addAll(this.customCodecs.getObjectWriters().keySet());
result.addAll(this.defaultCodecs.getObjectWriters(forMultipart));
result.addAll(this.defaultCodecs.getObjectWriters());
result.addAll(this.defaultCodecs.getCatchAllWriters());
return result;
}

View File

@ -355,13 +355,23 @@ class BaseDefaultCodecs implements CodecConfigurer.DefaultCodecs, CodecConfigure
}
/**
* Return writers that support specific types.
* @param forMultipart whether to returns writers for general use ("false"),
* or for multipart requests only ("true"). Generally the two sets are the
* same except for the multipart writer itself.
* Return all writers that support specific types.
*/
@SuppressWarnings({ "unchecked", "rawtypes" })
final List<HttpMessageWriter<?>> getTypedWriters(boolean forMultipart) {
@SuppressWarnings({"rawtypes" })
final List<HttpMessageWriter<?>> getTypedWriters() {
if (!this.registerDefaults) {
return Collections.emptyList();
}
List<HttpMessageWriter<?>> writers = getBaseTypedWriters();
extendTypedWriters(writers);
return writers;
}
/**
* Return "base" typed writers only, i.e. common to client and server.
*/
@SuppressWarnings("unchecked")
final List<HttpMessageWriter<?>> getBaseTypedWriters() {
if (!this.registerDefaults) {
return Collections.emptyList();
}
@ -371,10 +381,6 @@ class BaseDefaultCodecs implements CodecConfigurer.DefaultCodecs, CodecConfigure
writers.add(new EncoderHttpMessageWriter<>(new DataBufferEncoder()));
writers.add(new ResourceHttpMessageWriter());
writers.add(new EncoderHttpMessageWriter<>(CharSequenceEncoder.textPlainOnly()));
// No client or server specific multipart writers currently..
if (!forMultipart) {
extendTypedWriters(writers);
}
if (protobufPresent) {
Encoder<?> encoder = this.protobufEncoder != null ? this.protobufEncoder : new ProtobufEncoder();
writers.add(new ProtobufHttpMessageWriter((Encoder) encoder));
@ -390,14 +396,20 @@ class BaseDefaultCodecs implements CodecConfigurer.DefaultCodecs, CodecConfigure
/**
* Return Object writers (JSON, XML, SSE).
* @param forMultipart whether to returns writers for general use ("false"),
* or for multipart requests only ("true"). Generally the two sets are the
* same except for the multipart writer itself.
*/
final List<HttpMessageWriter<?>> getObjectWriters(boolean forMultipart) {
final List<HttpMessageWriter<?>> getObjectWriters() {
if (!this.registerDefaults) {
return Collections.emptyList();
}
List<HttpMessageWriter<?>> writers = getBaseObjectWriters();
extendObjectWriters(writers);
return writers;
}
/**
* Return "base" object writers only, i.e. common to client and server.
*/
final List<HttpMessageWriter<?>> getBaseObjectWriters() {
List<HttpMessageWriter<?>> writers = new ArrayList<>();
if (jackson2Present) {
writers.add(new EncoderHttpMessageWriter<>(getJackson2JsonEncoder()));
@ -409,10 +421,6 @@ class BaseDefaultCodecs implements CodecConfigurer.DefaultCodecs, CodecConfigure
Encoder<?> encoder = this.jaxb2Encoder != null ? this.jaxb2Encoder : new Jaxb2XmlEncoder();
writers.add(new EncoderHttpMessageWriter<>(encoder));
}
// No client or server specific multipart writers currently..
if (!forMultipart) {
extendObjectWriters(writers);
}
return writers;
}

View File

@ -54,9 +54,9 @@ class ClientDefaultCodecsImpl extends BaseDefaultCodecs implements ClientCodecCo
ClientDefaultCodecsImpl(ClientDefaultCodecsImpl other) {
super(other);
this.multipartCodecs = new DefaultMultipartCodecs(other.multipartCodecs);
this.multipartCodecs = (other.multipartCodecs != null ?
new DefaultMultipartCodecs(other.multipartCodecs) : null);
this.sseDecoder = other.sseDecoder;
this.partWritersSupplier = other.partWritersSupplier;
}
@ -132,10 +132,8 @@ class ClientDefaultCodecsImpl extends BaseDefaultCodecs implements ClientCodecCo
DefaultMultipartCodecs() {
}
DefaultMultipartCodecs(@Nullable DefaultMultipartCodecs other) {
if (other != null) {
this.writers.addAll(other.writers);
}
DefaultMultipartCodecs(DefaultMultipartCodecs other) {
this.writers.addAll(other.writers);
}

View File

@ -16,7 +16,11 @@
package org.springframework.http.codec.support;
import java.util.ArrayList;
import java.util.List;
import org.springframework.http.codec.ClientCodecConfigurer;
import org.springframework.http.codec.HttpMessageWriter;
/**
* Default implementation of {@link ClientCodecConfigurer}.
@ -29,11 +33,12 @@ public class DefaultClientCodecConfigurer extends BaseCodecConfigurer implements
public DefaultClientCodecConfigurer() {
super(new ClientDefaultCodecsImpl());
((ClientDefaultCodecsImpl) defaultCodecs()).setPartWritersSupplier(() -> getWritersInternal(true));
((ClientDefaultCodecsImpl) defaultCodecs()).setPartWritersSupplier(this::getPartWriters);
}
private DefaultClientCodecConfigurer(DefaultClientCodecConfigurer other) {
super(other);
((ClientDefaultCodecsImpl) defaultCodecs()).setPartWritersSupplier(this::getPartWriters);
}
@ -52,4 +57,14 @@ public class DefaultClientCodecConfigurer extends BaseCodecConfigurer implements
return new ClientDefaultCodecsImpl((ClientDefaultCodecsImpl) defaultCodecs());
}
private List<HttpMessageWriter<?>> getPartWriters() {
List<HttpMessageWriter<?>> result = new ArrayList<>();
result.addAll(this.customCodecs.getTypedWriters().keySet());
result.addAll(this.defaultCodecs.getBaseTypedWriters());
result.addAll(this.customCodecs.getObjectWriters().keySet());
result.addAll(this.defaultCodecs.getBaseObjectWriters());
result.addAll(this.defaultCodecs.getCatchAllWriters());
return result;
}
}

View File

@ -104,8 +104,8 @@ public class ClientCodecConfigurerTests {
assertThat(getNextEncoder(writers).getClass()).isEqualTo(DataBufferEncoder.class);
assertThat(writers.get(index.getAndIncrement()).getClass()).isEqualTo(ResourceHttpMessageWriter.class);
assertStringEncoder(getNextEncoder(writers), true);
assertThat(writers.get(this.index.getAndIncrement()).getClass()).isEqualTo(MultipartHttpMessageWriter.class);
assertThat(writers.get(index.getAndIncrement()).getClass()).isEqualTo(ProtobufHttpMessageWriter.class);
assertThat(writers.get(this.index.getAndIncrement()).getClass()).isEqualTo(MultipartHttpMessageWriter.class);
assertThat(getNextEncoder(writers).getClass()).isEqualTo(Jackson2JsonEncoder.class);
assertThat(getNextEncoder(writers).getClass()).isEqualTo(Jackson2SmileEncoder.class);
assertThat(getNextEncoder(writers).getClass()).isEqualTo(Jaxb2XmlEncoder.class);
@ -159,7 +159,7 @@ public class ClientCodecConfigurerTests {
}
@Test
public void cloneConfigurer() {
public void clonedConfigurer() {
ClientCodecConfigurer clone = this.configurer.clone();
Jackson2JsonDecoder jackson2Decoder = new Jackson2JsonDecoder();
@ -184,6 +184,30 @@ public class ClientCodecConfigurerTests {
assertThat(writers).hasSize(10);
}
@Test // gh-24194
public void cloneShouldNotDropMultipartCodecs() {
ClientCodecConfigurer clone = this.configurer.clone();
List<HttpMessageWriter<?>> writers =
findCodec(clone.getWriters(), MultipartHttpMessageWriter.class).getPartWriters();
assertThat(writers).hasSize(10);
}
@Test
public void cloneShouldNotBeImpactedByChangesToOriginal() {
ClientCodecConfigurer clone = this.configurer.clone();
this.configurer.registerDefaults(false);
this.configurer.customCodecs().register(new Jackson2JsonEncoder());
List<HttpMessageWriter<?>> writers =
findCodec(clone.getWriters(), MultipartHttpMessageWriter.class).getPartWriters();
assertThat(writers).hasSize(10);
}
private Decoder<?> getNextDecoder(List<HttpMessageReader<?>> readers) {
HttpMessageReader<?> reader = readers.get(this.index.getAndIncrement());
assertThat(reader).isInstanceOf(DecoderHttpMessageReader.class);