Skip to content

Commit 9001803

Browse files
committed
code review
closes gh-2402 Signed-off-by: Emanuel Trandafir <emanueltrandafir1993@gmail.com>
1 parent 558624b commit 9001803

6 files changed

Lines changed: 82 additions & 65 deletions

File tree

spring-cloud-contract-verifier/pom.xml

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
<modelVersion>4.0.0</modelVersion>
66
<properties>
77
<javax.ws.rs-api.version>2.1.1</javax.ws.rs-api.version>
8+
<avro.version>1.12.1</avro.version>
89
</properties>
910
<parent>
1011
<groupId>org.springframework.cloud</groupId>
@@ -182,11 +183,6 @@
182183
<artifactId>cglib</artifactId>
183184
<scope>test</scope>
184185
</dependency>
185-
<dependency>
186-
<groupId>org.spockframework</groupId>
187-
<artifactId>spock-core</artifactId>
188-
<scope>test</scope>
189-
</dependency>
190186
<dependency>
191187
<groupId>org.springframework.boot</groupId>
192188
<artifactId>spring-boot-starter-test</artifactId>
@@ -275,6 +271,11 @@
275271
<artifactId>spock-junit4</artifactId>
276272
<scope>test</scope>
277273
</dependency>
274+
<dependency>
275+
<groupId>org.spockframework</groupId>
276+
<artifactId>spock-spring</artifactId>
277+
<scope>test</scope>
278+
</dependency>
278279
<dependency>
279280
<groupId>org.springframework.boot</groupId>
280281
<artifactId>spring-boot-resttestclient</artifactId>
@@ -283,13 +284,13 @@
283284
<dependency>
284285
<groupId>org.apache.avro</groupId>
285286
<artifactId>avro</artifactId>
286-
<version>1.12.1</version>
287+
<version>${avro.version}</version>
287288
<scope>test</scope>
288289
</dependency>
289290
<dependency>
290291
<groupId>org.apache.avro</groupId>
291292
<artifactId>avro</artifactId>
292-
<version>1.12.1</version>
293+
<version>${avro.version}</version>
293294
<scope>provided</scope>
294295
</dependency>
295296
</dependencies>

spring-cloud-contract-verifier/src/main/java/org/springframework/cloud/contract/verifier/messaging/internal/ContractVerifierObjectMapper.java

Lines changed: 5 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -16,27 +16,24 @@
1616

1717
package org.springframework.cloud.contract.verifier.messaging.internal;
1818

19-
import org.springframework.util.ClassUtils;
20-
21-
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
22-
2319
import tools.jackson.databind.json.JsonMapper;
2420

2521
/**
2622
* Wrapper over {@link JsonMapper} that won't try to parse String but will directly return
2723
* it.
24+
*
2825
* @author Marcin Grzejszczak
2926
*/
3027
public class ContractVerifierObjectMapper {
3128

3229
private final JsonMapper objectMapper;
3330

34-
public ContractVerifierObjectMapper() {
35-
this(new JsonMapper());
31+
public ContractVerifierObjectMapper(JsonMapper objectMapper) {
32+
this.objectMapper = objectMapper;
3633
}
3734

38-
public ContractVerifierObjectMapper(JsonMapper mapper) {
39-
this.objectMapper = usesAvro() ? ignoreAvroFields(mapper) : mapper;
35+
public ContractVerifierObjectMapper() {
36+
this.objectMapper = new JsonMapper();
4037
}
4138

4239
public String writeValueAsString(Object payload) {
@@ -59,23 +56,4 @@ else if (payload instanceof byte[]) {
5956
return this.objectMapper.writeValueAsBytes(payload);
6057
}
6158

62-
private static boolean usesAvro() {
63-
return ClassUtils.isPresent("org.apache.avro.specific.SpecificRecordBase", null);
64-
}
65-
66-
private static JsonMapper ignoreAvroFields(JsonMapper mapper) {
67-
try {
68-
return mapper.rebuild().addMixIn(
69-
ClassUtils.forName("org.apache.avro.specific.SpecificRecordBase",
70-
null), IgnoreAvroMixin.class).build();
71-
}
72-
catch (ClassNotFoundException e) {
73-
throw new RuntimeException(e);
74-
}
75-
}
76-
77-
@JsonIgnoreProperties({ "schema", "specificData", "classSchema", "conversion" })
78-
interface IgnoreAvroMixin {
79-
}
80-
8159
}

spring-cloud-contract-verifier/src/main/java/org/springframework/cloud/contract/verifier/messaging/noop/NoOpContractVerifierAutoConfiguration.java

Lines changed: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,15 @@
1919
import java.util.Map;
2020
import java.util.concurrent.TimeUnit;
2121

22+
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
2223
import org.jspecify.annotations.Nullable;
2324
import tools.jackson.databind.json.JsonMapper;
2425

2526
import org.springframework.beans.factory.ObjectProvider;
2627
import org.springframework.boot.autoconfigure.AutoConfigureOrder;
28+
import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
2729
import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
30+
import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingClass;
2831
import org.springframework.cloud.contract.verifier.converter.YamlContract;
2932
import org.springframework.cloud.contract.verifier.messaging.MessageVerifierReceiver;
3033
import org.springframework.cloud.contract.verifier.messaging.MessageVerifierSender;
@@ -36,7 +39,6 @@
3639

3740
/**
3841
* Verifier auto configuration containing default beans for messaging.
39-
*
4042
* @author Marcin Grzejszczak
4143
*/
4244
@Configuration(proxyBeanMethods = false)
@@ -50,13 +52,14 @@ public MessageVerifierSender<Object> noOpContractVerifierMessageSender() {
5052
return new MessageVerifierSender<>() {
5153

5254
@Override
53-
public void send(Object message, String destination, @Nullable YamlContract contract) {
55+
public void send(Object message, String destination,
56+
@Nullable YamlContract contract) {
5457
noOpStubMessages.send(message, destination, contract);
5558
}
5659

5760
@Override
58-
public <T> void send(T payload, Map<String, Object> headers, String destination,
59-
@Nullable YamlContract contract) {
61+
public <T> void send(T payload, Map<String, Object> headers,
62+
String destination, @Nullable YamlContract contract) {
6063
noOpStubMessages.send(payload, headers, destination, contract);
6164
}
6265
};
@@ -84,17 +87,33 @@ public Object receive(String destination, YamlContract contract) {
8487
@Bean
8588
@ConditionalOnMissingBean(ContractVerifierMessaging.class)
8689
public ContractVerifierMessaging<Object> contractVerifierMessaging() {
87-
return new ContractVerifierMessaging<>(new NoOpStubMessages<>(), new NoOpStubMessages<>());
90+
return new ContractVerifierMessaging<>(new NoOpStubMessages<>(),
91+
new NoOpStubMessages<>());
92+
}
93+
94+
@Bean
95+
@ConditionalOnMissingBean
96+
@ConditionalOnMissingClass("org.apache.avro.specific.SpecificRecordBase")
97+
public ContractVerifierObjectMapper contractVerifierObjectMapper(
98+
ObjectProvider<JsonMapper> jsonMapper) {
99+
JsonMapper mapper = jsonMapper.getIfAvailable(JsonMapper::new);
100+
return new ContractVerifierObjectMapper(mapper);
88101
}
89102

90103
@Bean
91104
@ConditionalOnMissingBean
92-
public ContractVerifierObjectMapper contractVerifierObjectMapper(ObjectProvider<JsonMapper> jsonMapper) {
93-
JsonMapper mapper = jsonMapper.getIfAvailable();
94-
if (mapper != null) {
95-
return new ContractVerifierObjectMapper(mapper);
96-
}
97-
return new ContractVerifierObjectMapper();
105+
@ConditionalOnClass(name = "org.apache.avro.specific.SpecificRecordBase")
106+
public ContractVerifierObjectMapper avroContractVerifierObjectMapper(
107+
ObjectProvider<JsonMapper> jsonMapper) throws ClassNotFoundException {
108+
JsonMapper mapper = jsonMapper.getIfAvailable(JsonMapper::new).rebuild()
109+
.addMixIn(Class.forName("org.apache.avro.specific.SpecificRecordBase"),
110+
IgnoreAvroMixin.class).build();
111+
return new ContractVerifierObjectMapper(mapper);
112+
}
113+
114+
@JsonIgnoreProperties({ "schema", "specificData", "classSchema", "conversion" })
115+
interface IgnoreAvroMixin {
116+
98117
}
99118

100119
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
/*
2+
* Copyright 2013-present the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.cloud.contract.verifier.messaging.internal
18+
19+
import org.springframework.beans.factory.annotation.Autowired
20+
import org.springframework.cloud.contract.verifier.messaging.noop.NoOpContractVerifierAutoConfiguration
21+
import org.springframework.test.context.ContextConfiguration
22+
import spock.lang.Specification
23+
24+
@ContextConfiguration(classes = [NoOpContractVerifierAutoConfiguration])
25+
class ContractVerifierObjectMapperAvroSpec extends Specification {
26+
27+
@Autowired
28+
ContractVerifierObjectMapper mapper
29+
30+
def "should convert an Avro-generated object into a json representation"() {
31+
given:
32+
FooAvro input = FooAvro.newBuilder().setFooAvro("barAvro").build()
33+
when:
34+
String result = mapper.writeValueAsString(input)
35+
then:
36+
result == '{"fooAvro":"barAvro"}'
37+
}
38+
}

spring-cloud-contract-verifier/src/test/groovy/org/springframework/cloud/contract/verifier/messaging/internal/ContractVerifierObjectMapperSpec.groovy

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -32,15 +32,6 @@ class ContractVerifierObjectMapperSpec extends Specification {
3232
result == '''{"foo":"bar"}'''
3333
}
3434

35-
def "should convert an Avro-generated object into a json representation"() {
36-
given:
37-
FooAvro input = FooAvro.newBuilder().setFooAvro("barAvro").build()
38-
when:
39-
String result = mapper.writeValueAsString(input)
40-
then:
41-
result == '''{"fooAvro":"barAvro"}'''
42-
}
43-
4435
def "should convert bytes into a json representation"() {
4536
given:
4637
String input = '''{"foo":"bar"}'''

spring-cloud-contract-verifier/src/test/groovy/org/springframework/cloud/contract/verifier/messaging/internal/FooAvro.java

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -338,14 +338,4 @@ public void customDecode(org.apache.avro.io.ResolvingDecoder in)
338338
}
339339
}
340340
}
341-
}
342-
343-
344-
345-
346-
347-
348-
349-
350-
351-
341+
}

0 commit comments

Comments
 (0)