Skip to content

Commit a0518ef

Browse files
committed
#54 Required fields in nested definitions are being reported as breaking when they are not
1 parent ae36bde commit a0518ef

File tree

4 files changed

+173
-6
lines changed

4 files changed

+173
-6
lines changed

swagger-brake/src/main/java/io/redskap/swagger/brake/core/model/Schema.java

+32-6
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
import lombok.Getter;
1313
import lombok.RequiredArgsConstructor;
1414
import lombok.ToString;
15-
import org.apache.commons.lang3.BooleanUtils;
1615
import org.apache.commons.lang3.StringUtils;
1716
import org.apache.commons.lang3.tuple.Pair;
1817
import org.springframework.util.CollectionUtils;
@@ -107,11 +106,38 @@ public Set<String> getRequiredAttributeNames() {
107106
if (CollectionUtils.isEmpty(schemaAttrs)) {
108107
schemaAttrs = Optional.ofNullable(schema).map(Schema::getSchemaAttributes).orElse(Collections.emptySet());
109108
}
110-
return internalGetAttributeData(schemaAttrs, "", SchemaAttribute::isRequired)
111-
.stream()
112-
.filter(p -> BooleanUtils.isTrue(p.getRight()))
113-
.map(Pair::getLeft)
114-
.collect(toSet());
109+
Map<String, Boolean> attributeRequiredMap = internalGetAttributeData(schemaAttrs, "", SchemaAttribute::isRequired)
110+
.stream()
111+
.collect(toMap(Pair::getLeft, Pair::getRight));
112+
Set<String> result = new HashSet<>();
113+
for (Map.Entry<String, Boolean> entry : attributeRequiredMap.entrySet()) {
114+
String attributeName = entry.getKey();
115+
Boolean attributeIsRequired = entry.getValue();
116+
if (attributeName.contains(".")) {
117+
boolean hierarchicallyNotRequired = false;
118+
List<String> pieces = Arrays.asList(attributeName.split("\\."));
119+
for (int i = 0; i < pieces.size(); i++) {
120+
StringJoiner stringJoiner = new StringJoiner(".");
121+
pieces.subList(0, i + 1).forEach(stringJoiner::add);
122+
String attrToSearchFor = stringJoiner.toString();
123+
Boolean isRequired = attributeRequiredMap.get(attrToSearchFor);
124+
if (!isRequired) {
125+
hierarchicallyNotRequired = true;
126+
break;
127+
}
128+
}
129+
if (!hierarchicallyNotRequired) {
130+
if (attributeIsRequired) {
131+
result.add(attributeName);
132+
}
133+
}
134+
} else {
135+
if (attributeIsRequired) {
136+
result.add(attributeName);
137+
}
138+
}
139+
}
140+
return result;
115141
}
116142

117143
/**
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
package io.redskap.swagger.brake.integration.v2.request;
2+
3+
import static org.assertj.core.api.Assertions.assertThat;
4+
5+
import java.util.Collection;
6+
7+
import io.redskap.swagger.brake.core.BreakingChange;
8+
import io.redskap.swagger.brake.integration.AbstractSwaggerBrakeIntTest;
9+
import org.junit.Test;
10+
import org.junit.runner.RunWith;
11+
import org.springframework.test.context.junit4.SpringRunner;
12+
13+
14+
15+
@RunWith(SpringRunner.class)
16+
public class OptionalRequestParameterAddedIntTest extends AbstractSwaggerBrakeIntTest {
17+
@Test
18+
public void testOptionalRequestParameterAddedWorksCorrectly() {
19+
// given
20+
String oldApiPath = "swaggers/v2/request/optionalparameteradded/swagger_v1.yaml";
21+
String newApiPath = "swaggers/v2/request/optionalparameteradded/swagger_v2.yaml";
22+
// when
23+
Collection<BreakingChange> result = execute(oldApiPath, newApiPath);
24+
// then
25+
assertThat(result).isEmpty();
26+
}
27+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
swagger: '2.0'
2+
info:
3+
description: ABC
4+
version: 1.0.5
5+
title: Swagger Petstore
6+
termsOfService: 'http://swagger.io/terms/'
7+
host: petstore.swagger.io
8+
basePath: /v2
9+
schemes:
10+
- https
11+
- http
12+
paths:
13+
/pet/uploadImage:
14+
put:
15+
tags:
16+
- pet
17+
summary: Update an existing pet
18+
description: ''
19+
operationId: updatePet
20+
parameters:
21+
- in: body
22+
name: body
23+
description: Pet object that needs to be added to the store
24+
required: true
25+
schema:
26+
$ref: '#/definitions/Pet'
27+
responses:
28+
'400':
29+
description: Invalid ID supplied
30+
definitions:
31+
Pet:
32+
type: object
33+
required:
34+
- id
35+
- breed
36+
properties:
37+
id:
38+
type: integer
39+
format: int64
40+
breed:
41+
$ref: '#/definitions/Breed'
42+
Breed:
43+
type: object
44+
required:
45+
- name
46+
properties:
47+
name:
48+
type: string
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
swagger: '2.0'
2+
info:
3+
description: ABC
4+
version: 1.0.5
5+
title: Swagger Petstore
6+
termsOfService: 'http://swagger.io/terms/'
7+
host: petstore.swagger.io
8+
basePath: /v2
9+
schemes:
10+
- https
11+
- http
12+
paths:
13+
/pet/uploadImage:
14+
put:
15+
tags:
16+
- pet
17+
summary: Update an existing pet
18+
description: ''
19+
operationId: updatePet
20+
parameters:
21+
- in: body
22+
name: body
23+
description: Pet object that needs to be added to the store
24+
required: true
25+
schema:
26+
$ref: '#/definitions/Pet'
27+
responses:
28+
'400':
29+
description: Invalid ID supplied
30+
definitions:
31+
Pet:
32+
type: object
33+
required:
34+
- id
35+
- breed
36+
properties:
37+
id:
38+
type: integer
39+
format: int64
40+
petFood:
41+
$ref: '#/definitions/PetFood'
42+
breed:
43+
$ref: '#/definitions/Breed'
44+
PetFood:
45+
type: object
46+
required:
47+
- name
48+
properties:
49+
name:
50+
type: string
51+
Breed:
52+
type: object
53+
required:
54+
- name
55+
properties:
56+
name:
57+
type: string
58+
details:
59+
$ref: '#/definitions/BreedDetails'
60+
BreedDetails:
61+
type: object
62+
required:
63+
- weight
64+
properties:
65+
weight:
66+
type: integer

0 commit comments

Comments
 (0)