Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

1.x Fix batch request/response encoding for the new Jackson codec. #674

Merged
merged 2 commits into from
Oct 6, 2015
Merged

1.x Fix batch request/response encoding for the new Jackson codec. #674

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0ae0a43
Fix batch request/response encoding for the new Jackson codec.
Oct 6, 2015
b8a4afd
Fix log output in compatibility test
Oct 6, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 8 additions & 3 deletions ...nt/src/main/java/com/netflix/discovery/converters/jackson/AbstractEurekaJacksonCodec.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import java.util.Map;
import java.util.Set;

import com.fasterxml.jackson.annotation.JsonRootName;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.introspect.Annotated;
import com.fasterxml.jackson.databind.introspect.JacksonAnnotationIntrospector;
Expand All @@ -26,14 +27,14 @@ public abstract class AbstractEurekaJacksonCodec {
Arrays.asList("instance-id", "public-ipv4", "public-hostname", "local-ipv4", "availability-zone")
);

public abstract ObjectMapper getObjectMapper();
public abstract <T> ObjectMapper getObjectMapper(Class<T> type);

public <T> void writeTo(T object, OutputStream entityStream) throws IOException {
getObjectMapper().writeValue(entityStream, object);
getObjectMapper(object.getClass()).writeValue(entityStream, object);
}

protected void addMiniConfig(ObjectMapper mapper) {
mapper.addMixInAnnotations(InstanceInfo.class, MiniInstanceInfoMixIn.class);
mapper.addMixIn(InstanceInfo.class, MiniInstanceInfoMixIn.class);
bindAmazonInfoFilter(mapper);
}

Expand Down Expand Up @@ -62,4 +63,8 @@ protected boolean include(PropertyWriter writer) {
});
mapper.setFilters(filters);
}

static boolean hasJsonRootName(Class<?> type) {
return type.getAnnotation(JsonRootName.class) != null;
}
}
45 changes: 33 additions & 12 deletions ...client/src/main/java/com/netflix/discovery/converters/jackson/EurekaJsonJacksonCodec.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@

package com.netflix.discovery.converters.jackson;

import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;

import com.fasterxml.jackson.annotation.JsonInclude.Include;
import com.fasterxml.jackson.databind.DeserializationFeature;
import com.fasterxml.jackson.databind.ObjectMapper;
Expand All @@ -26,36 +29,54 @@
import com.netflix.discovery.converters.jackson.mixin.InstanceInfoJsonMixIn;

/**
* JSON codec defaults to unwrapped mode, as ReplicationList in the replication channel, must be serialized
* unwrapped. The wrapping mode is configured separately for each type, based on presence of
* {@link com.fasterxml.jackson.annotation.JsonRootName} annotation.
*
* @author Tomasz Bak
*/
public class EurekaJsonJacksonCodec extends AbstractEurekaJacksonCodec {

private final ObjectMapper jsonMapper = new ObjectMapper();
private final ObjectMapper wrappedJsonMapper;
private final ObjectMapper unwrappedJsonMapper;

private final Map<Class<?>, ObjectMapper> mappers = new ConcurrentHashMap<>();

public EurekaJsonJacksonCodec() {
this(KeyFormatter.defaultKeyFormatter(), false);
}

public EurekaJsonJacksonCodec(final KeyFormatter keyFormatter, boolean compact) {
// JSON
this.unwrappedJsonMapper = createObjectMapper(keyFormatter, compact, false);
this.wrappedJsonMapper = createObjectMapper(keyFormatter, compact, true);
}

private ObjectMapper createObjectMapper(KeyFormatter keyFormatter, boolean compact, boolean wrapped) {
ObjectMapper newMapper = new ObjectMapper();
SimpleModule jsonModule = new SimpleModule();
jsonModule.setSerializerModifier(EurekaJacksonJsonModifiers.createJsonSerializerModifier(keyFormatter, compact));
jsonModule.setDeserializerModifier(EurekaJacksonJsonModifiers.createJsonDeserializerModifier(keyFormatter, compact));
jsonMapper.registerModule(jsonModule);
jsonMapper.setSerializationInclusion(Include.NON_NULL);
jsonMapper.configure(SerializationFeature.WRAP_ROOT_VALUE, true);
jsonMapper.configure(SerializationFeature.WRITE_SINGLE_ELEM_ARRAYS_UNWRAPPED, false);
jsonMapper.configure(DeserializationFeature.UNWRAP_ROOT_VALUE, true);
jsonMapper.configure(DeserializationFeature.ACCEPT_SINGLE_VALUE_AS_ARRAY, true);
newMapper.registerModule(jsonModule);
newMapper.setSerializationInclusion(Include.NON_NULL);
newMapper.configure(SerializationFeature.WRAP_ROOT_VALUE, wrapped);
newMapper.configure(SerializationFeature.WRITE_SINGLE_ELEM_ARRAYS_UNWRAPPED, false);
newMapper.configure(DeserializationFeature.UNWRAP_ROOT_VALUE, wrapped);
newMapper.configure(DeserializationFeature.ACCEPT_SINGLE_VALUE_AS_ARRAY, true);
if (compact) {
addMiniConfig(jsonMapper);
addMiniConfig(newMapper);
} else {
jsonMapper.addMixIn(InstanceInfo.class, InstanceInfoJsonMixIn.class);
newMapper.addMixIn(InstanceInfo.class, InstanceInfoJsonMixIn.class);
}
return newMapper;
}

@Override
public ObjectMapper getObjectMapper() {
return jsonMapper;
public <T> ObjectMapper getObjectMapper(Class<T> type) {
ObjectMapper mapper = mappers.get(type);
if (mapper == null) {
mapper = hasJsonRootName(type) ? wrappedJsonMapper : unwrappedJsonMapper;
mappers.put(type, mapper);
}
return mapper;
}
}
2 changes: 1 addition & 1 deletion ...-client/src/main/java/com/netflix/discovery/converters/jackson/EurekaXmlJacksonCodec.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public ObjectMapper registerModule(Module module) {
}

@Override
public ObjectMapper getObjectMapper() {
public <T> ObjectMapper getObjectMapper(Class<T> type) {
return xmlMapper;
}
}
24 changes: 12 additions & 12 deletions eureka-client/src/main/java/com/netflix/discovery/converters/wrappers/CodecWrappers.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ public boolean support(MediaType mediaType) {

@Override
public <T> String encode(T object) throws IOException {
return codec.getObjectMapper().writeValueAsString(object);
return codec.getObjectMapper(object.getClass()).writeValueAsString(object);
}

@Override
Expand All @@ -165,12 +165,12 @@ public <T> void encode(T object, OutputStream outputStream) throws IOException {

@Override
public <T> T decode(String textValue, Class<T> type) throws IOException {
return codec.getObjectMapper().readValue(textValue, type);
return codec.getObjectMapper(type).readValue(textValue, type);
}

@Override
public <T> T decode(InputStream inputStream, Class<T> type) throws IOException {
return codec.getObjectMapper().readValue(inputStream, type);
return codec.getObjectMapper(type).readValue(inputStream, type);
}
}

Expand All @@ -190,7 +190,7 @@ public boolean support(MediaType mediaType) {

@Override
public <T> String encode(T object) throws IOException {
return codec.getObjectMapper().writeValueAsString(object);
return codec.getObjectMapper(object.getClass()).writeValueAsString(object);
}

@Override
Expand All @@ -200,12 +200,12 @@ public <T> void encode(T object, OutputStream outputStream) throws IOException {

@Override
public <T> T decode(String textValue, Class<T> type) throws IOException {
return codec.getObjectMapper().readValue(textValue, type);
return codec.getObjectMapper(type).readValue(textValue, type);
}

@Override
public <T> T decode(InputStream inputStream, Class<T> type) throws IOException {
return codec.getObjectMapper().readValue(inputStream, type);
return codec.getObjectMapper(type).readValue(inputStream, type);
}
}

Expand All @@ -225,7 +225,7 @@ public boolean support(MediaType mediaType) {

@Override
public <T> String encode(T object) throws IOException {
return codec.getObjectMapper().writeValueAsString(object);
return codec.getObjectMapper(object.getClass()).writeValueAsString(object);
}

@Override
Expand All @@ -235,12 +235,12 @@ public <T> void encode(T object, OutputStream outputStream) throws IOException {

@Override
public <T> T decode(String textValue, Class<T> type) throws IOException {
return codec.getObjectMapper().readValue(textValue, type);
return codec.getObjectMapper(type).readValue(textValue, type);
}

@Override
public <T> T decode(InputStream inputStream, Class<T> type) throws IOException {
return codec.getObjectMapper().readValue(inputStream, type);
return codec.getObjectMapper(type).readValue(inputStream, type);
}
}

Expand All @@ -260,7 +260,7 @@ public boolean support(MediaType mediaType) {

@Override
public <T> String encode(T object) throws IOException {
return codec.getObjectMapper().writeValueAsString(object);
return codec.getObjectMapper(object.getClass()).writeValueAsString(object);
}

@Override
Expand All @@ -270,12 +270,12 @@ public <T> void encode(T object, OutputStream outputStream) throws IOException {

@Override
public <T> T decode(String textValue, Class<T> type) throws IOException {
return codec.getObjectMapper().readValue(textValue, type);
return codec.getObjectMapper(type).readValue(textValue, type);
}

@Override
public <T> T decode(InputStream inputStream, Class<T> type) throws IOException {
return codec.getObjectMapper().readValue(inputStream, type);
return codec.getObjectMapper(type).readValue(inputStream, type);
}
}

Expand Down
2 changes: 1 addition & 1 deletion eureka-client/src/test/java/com/netflix/discovery/converters/CodecLoadTester.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ public int call(Object object) {
codec.writeTo(object, captureStream);
byte[] bytes = captureStream.toByteArray();
InputStream source = new ByteArrayInputStream(bytes);
Applications readValue = (Applications) codec.getObjectMapper().readValue(source, object.getClass());
Applications readValue = codec.getObjectMapper(object.getClass()).readValue(source, Applications.class);
secondHolder.value = readValue;

return bytes.length;
Expand Down
65 changes: 65 additions & 0 deletions ...a-client/src/test/java/com/netflix/discovery/converters/EurekaCodecCompatibilityTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,14 @@
import com.netflix.discovery.shared.Applications;
import com.netflix.discovery.util.EurekaEntityComparators;
import com.netflix.discovery.util.InstanceInfoGenerator;
import com.netflix.eureka.cluster.protocol.ReplicationInstance;
import com.netflix.eureka.cluster.protocol.ReplicationInstanceResponse;
import com.netflix.eureka.cluster.protocol.ReplicationList;
import com.netflix.eureka.cluster.protocol.ReplicationListResponse;
import com.netflix.eureka.registry.PeerAwareInstanceRegistryImpl.Action;
import org.junit.Test;

import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.is;
import static org.junit.Assert.assertThat;

Expand Down Expand Up @@ -238,6 +244,65 @@ public void call(EncoderWrapper encodingCodec, DecoderWrapper decodingCodec) thr
verifyAllPairs(codingAction, Applications.class, jsonCodes);
}

@Test
public void testBatchRequestEncoding() throws Exception {
InstanceInfo instance = InstanceInfoGenerator.takeOne();
List<ReplicationInstance> replicationInstances = new ArrayList<>();
replicationInstances.add(new ReplicationInstance(
instance.getAppName(),
instance.getId(),
System.currentTimeMillis(),
null,
instance.getStatus().name(),
instance,
Action.Register
));
final ReplicationList replicationList = new ReplicationList(replicationInstances);

Action2 codingAction = new Action2() {
@Override
public void call(EncoderWrapper encodingCodec, DecoderWrapper decodingCodec) throws IOException {
String encodedString = encodingCodec.encode(replicationList);

ReplicationList decodedValue = decodingCodec.decode(encodedString, ReplicationList.class);
assertThat(decodedValue.getReplicationList().size(), is(equalTo(1)));
}
};

// In replication channel we use JSON only
List<CodecWrapper> jsonCodes = Arrays.asList(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about the reverse (encoded with JacksonJson and decoded with Legacy)?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test covers all combinations:
Encoding ReplicationList using {JacksonJson,JacksonJson}
Encoding ReplicationList using {JacksonJson,LegacyJacksonJson}
Encoding ReplicationList using {LegacyJacksonJson,JacksonJson}
Encoding ReplicationList using {LegacyJacksonJson,LegacyJacksonJson}

Encoding ReplicationListResponse using {JacksonJson,JacksonJson}
Encoding ReplicationListResponse using {JacksonJson,LegacyJacksonJson}
Encoding ReplicationListResponse using {LegacyJacksonJson,JacksonJson}
Encoding ReplicationListResponse using {LegacyJacksonJson,LegacyJacksonJson}

On Tue, Oct 6, 2015 at 2:50 PM, David Liu notifications@github.com wrote:

In
eureka-client/src/test/java/com/netflix/discovery/converters/EurekaCodecCompatibilityTest.java
#674 (comment):

  •            Action.Register

  •    ));

  •    final ReplicationList replicationList = new ReplicationList(replicationInstances);

  •    Action2 codingAction = new Action2() {

  •        @Override

  •        public void call(EncoderWrapper encodingCodec, DecoderWrapper decodingCodec) throws IOException {

  •            String encodedString = encodingCodec.encode(replicationList);

  •            ReplicationList decodedValue = decodingCodec.decode(encodedString, ReplicationList.class);

  •            assertThat(decodedValue.getReplicationList().size(), is(equalTo(1)));

  •        }

  •    };

  •    // In replication channel we use JSON only

  •    List<CodecWrapper> jsonCodes = Arrays.asList(

what about the reverse (encoded with JacksonJson and decoded with Legacy)?

—
Reply to this email directly or view it on GitHub
https://github.com/Netflix/eureka/pull/674/files#r41327448.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh yes, thanks. I misread.

new CodecWrappers.JacksonJson(),
new CodecWrappers.LegacyJacksonJson()
);

verifyAllPairs(codingAction, ReplicationList.class, jsonCodes);
}

@Test
public void testBatchResponseEncoding() throws Exception {
List<ReplicationInstanceResponse> responseList = new ArrayList<>();
responseList.add(new ReplicationInstanceResponse(200, InstanceInfoGenerator.takeOne()));
final ReplicationListResponse replicationListResponse = new ReplicationListResponse(responseList);

Action2 codingAction = new Action2() {
@Override
public void call(EncoderWrapper encodingCodec, DecoderWrapper decodingCodec) throws IOException {
String encodedString = encodingCodec.encode(replicationListResponse);

ReplicationListResponse decodedValue = decodingCodec.decode(encodedString, ReplicationListResponse.class);
assertThat(decodedValue.getResponseList().size(), is(equalTo(1)));
}
};

// In replication channel we use JSON only
List<CodecWrapper> jsonCodes = Arrays.asList(
new CodecWrappers.JacksonJson(),
new CodecWrappers.LegacyJacksonJson()
);

verifyAllPairs(codingAction, ReplicationListResponse.class, jsonCodes);
}

public void verifyAllPairs(Action2 codingAction, Class<?> typeToEncode, List<CodecWrapper> codecHolders) throws Exception {
for (EncoderWrapper encodingCodec : codecHolders) {
for (DecoderWrapper decodingCodec : codecHolders) {
Expand Down