Skip to content

Commit bb8a82c

Browse files
esaezgilewaostrowska
esaezgil
and
ewaostrowska
authored
fix(converter): keep x-nullable in shared $ref responses (#2276)
* fix(converter): prevent x-nullable loss for shared $ref responses The convert(vendorExtensions) method mutated the original map when filtering out x-nullable, x-example, and x-examples extensions. When multiple endpoints referenced the same response via $ref, the first conversion removed x-nullable from the shared source object, causing subsequent conversions to miss it and produce nullable=null. Create a new map instead of mutating the original to preserve vendor extensions for all endpoints sharing the same response definition. * refactor convert method and add test --------- Co-authored-by: Ewa Ostrowska <ewa.ostrowska@smartbear.com>
1 parent b6ed0cc commit bb8a82c

5 files changed

Lines changed: 155 additions & 12 deletions

File tree

‎modules/swagger-parser-v2-converter/src/main/java/io/swagger/v3/parser/converter/SwaggerConverter.java‎

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -59,15 +59,13 @@
5959

6060

6161
import java.math.BigDecimal;
62-
import java.util.ArrayList;
63-
import java.util.HashMap;
64-
import java.util.LinkedHashMap;
65-
import java.util.List;
66-
import java.util.Map;
67-
import java.util.Optional;
62+
import java.util.*;
6863
import java.util.stream.Collectors;
6964

7065
public class SwaggerConverter implements SwaggerParserExtension {
66+
67+
private static final Set<String> STRIPPED_EXTENSION_KEYS = Collections.unmodifiableSet(new HashSet<>(Arrays.asList("x-example", "x-examples", "x-nullable")));
68+
7169
private List<String> globalConsumes = new ArrayList<>();
7270
private List<String> globalProduces = new ArrayList<>();
7371
private Components components = new Components();
@@ -654,14 +652,17 @@ public Operation convert(io.swagger.models.Operation v2Operation) {
654652
}
655653

656654
private Map<String, Object> convert(Map<String, Object> vendorExtensions) {
657-
if (vendorExtensions != null && vendorExtensions.size() > 0) {
658-
vendorExtensions.entrySet().removeIf(extension -> (
659-
extension.getKey().equals("x-example")) ||
660-
extension.getKey().equals("x-examples") ||
661-
extension.getKey().equals("x-nullable"));
655+
if (vendorExtensions == null || vendorExtensions.isEmpty()) {
656+
return vendorExtensions;
662657
}
663658

664-
return vendorExtensions;
659+
Map<String, Object> result = new LinkedHashMap<>();
660+
for (Map.Entry<String, Object> entry : vendorExtensions.entrySet()) {
661+
if (!STRIPPED_EXTENSION_KEYS.contains(entry.getKey())) {
662+
result.put(entry.getKey(), entry.getValue());
663+
}
664+
}
665+
return result;
665666
}
666667

667668
private Schema convertFormDataToSchema(io.swagger.models.parameters.Parameter formParam) {

‎modules/swagger-parser-v2-converter/src/test/java/io/swagger/parser/test/V2ConverterTest.java‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ public class V2ConverterTest {
100100

101101
private static final String ISSUE_1767_YAML = "issue-1767.yaml";
102102
private static final String ISSUE_1796_YAML = "issue-1796.yaml";
103+
private static final String ISSUE_2269_YAML = "issue-2269.yaml";
103104

104105
private static final String API_BATCH_PATH = "/api/batch/";
105106
private static final String PETS_PATH = "/pets";
@@ -928,6 +929,42 @@ public void testConvertFormDataAsObjectSchema() throws Exception {
928929

929930
}
930931

932+
@Test
933+
public void testIssue2269SharedResponseNullable() {
934+
SwaggerConverter converter = new SwaggerConverter();
935+
ParseOptions parseOptions = new ParseOptions();
936+
parseOptions.setResolve(true);
937+
parseOptions.setResolveFully(true);
938+
SwaggerParseResult result = converter.readLocation(
939+
"src/test/resources/" + ISSUE_2269_YAML, null, parseOptions);
940+
assertNotNull(result);
941+
OpenAPI oas = result.getOpenAPI();
942+
assertNotNull(oas);
943+
944+
for (String endpoint : Arrays.asList("/endpoint1", "/endpoint2")) {
945+
Map<String, Schema> properties = oas.getPaths().get(endpoint).getGet()
946+
.getResponses().get("200").getContent().values().iterator().next()
947+
.getSchema().getProperties();
948+
assertNotNull(properties, endpoint + " schema properties must not be null");
949+
950+
Schema nullableField = properties.get("nullable_field");
951+
assertNotNull(nullableField, endpoint + " must have nullable_field");
952+
assertEquals(nullableField.getNullable(), Boolean.TRUE,
953+
endpoint + " nullable_field must have nullable: true");
954+
assertNotNull(nullableField.getExtensions(), endpoint + " nullable_field must retain extensions");
955+
assertEquals(nullableField.getExtensions().get("x-internal-note"), "abc",
956+
endpoint + " nullable_field must retain x-internal-note extension");
957+
958+
Schema nonNullableField = properties.get("non_nullable_field");
959+
assertNotNull(nonNullableField, endpoint + " must have non_nullable_field");
960+
assertEquals(nonNullableField.getNullable(), Boolean.FALSE,
961+
endpoint + " non_nullable_field must have nullable: false");
962+
assertNotNull(nonNullableField.getExtensions(), endpoint + " non_nullable_field must retain extensions");
963+
assertEquals(nonNullableField.getExtensions().get("x-internal-note"), "def",
964+
endpoint + " non_nullable_field must retain x-internal-note extension");
965+
}
966+
}
967+
931968
/**
932969
* A clone (almost) of {@link OpenAPIV3Parser#readContents(String, List, ParseOptions)}.
933970
*/

‎modules/swagger-parser-v2-converter/src/test/resources/issue-2269.yaml‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
swagger: '2.0'
2+
info:
3+
version: 1.0.0
4+
title: Issue 2269 - x-nullable preservation for shared $ref responses
5+
6+
responses:
7+
SharedResponse:
8+
description: A shared response with multiple fields using x-nullable
9+
schema:
10+
type: object
11+
properties:
12+
data:
13+
type: string
14+
description: Required field
15+
nullable_field:
16+
type: integer
17+
x-nullable: true
18+
x-internal-note: abc
19+
non_nullable_field:
20+
type: integer
21+
x-nullable: false
22+
x-internal-note: def
23+
required:
24+
- data
25+
26+
paths:
27+
/endpoint1:
28+
get:
29+
operationId: endpoint1
30+
summary: First endpoint using shared response
31+
responses:
32+
'200':
33+
$ref: '#/responses/SharedResponse'
34+
35+
/endpoint2:
36+
get:
37+
operationId: endpoint2
38+
summary: Second endpoint using shared response
39+
responses:
40+
'200':
41+
$ref: '#/responses/SharedResponse'

‎modules/swagger-parser/src/test/java/io/swagger/parser/OpenAPIParserTest.java‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -686,6 +686,32 @@ public void testIssue1552() throws Exception {
686686
assertNotNull(schema.getProperties().get("foo"));
687687
}
688688

689+
@Test(description = "Issue 2269: preserve x-nullable in shared responses for swagger 2.0 specs")
690+
public void testSwagger2SharedResponseNullable() {
691+
ParseOptions options = new ParseOptions();
692+
options.setResolve(true);
693+
options.setResolveFully(true);
694+
695+
SwaggerParseResult result = new OpenAPIParser().readLocation("issue2269.yaml", null, options);
696+
697+
assertNotNull(result);
698+
assertNotNull(result.getOpenAPI());
699+
OpenAPI openAPI = result.getOpenAPI();
700+
701+
Schema endpoint1Field = (Schema) openAPI.getPaths().get("/endpoint1").getGet()
702+
.getResponses().get("200").getContent().values().iterator().next()
703+
.getSchema().getProperties().get("optional_field");
704+
assertNotNull(endpoint1Field, "Endpoint 1 should have optional_field");
705+
assertEquals(endpoint1Field.getNullable(), Boolean.TRUE, "Endpoint 1 optional_field should be nullable");
706+
707+
Schema endpoint2Field = (Schema) openAPI.getPaths().get("/endpoint2").getGet()
708+
.getResponses().get("200").getContent().values().iterator().next()
709+
.getSchema().getProperties().get("optional_field");
710+
assertNotNull(endpoint2Field, "Endpoint 2 should have optional_field");
711+
assertEquals(endpoint2Field.getNullable(), Boolean.TRUE, "Endpoint 2 optional_field should be nullable");
712+
713+
}
714+
689715
@org.testng.annotations.Test(description = "convert response schema")
690716
public void testIssue1552AdditionalProps() throws Exception {
691717
ParseOptions options = new ParseOptions();

‎modules/swagger-parser/src/test/resources/issue2269.yaml‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
swagger: '2.0'
2+
info:
3+
version: 1.0.0
4+
title: Swagger 2.0 Shared Response Test
5+
description: Test case for verifying x-nullable is preserved when multiple endpoints share the same response definition
6+
7+
responses:
8+
SharedResponse:
9+
description: A shared response with nullable field
10+
schema:
11+
type: object
12+
properties:
13+
data:
14+
type: string
15+
description: Required field
16+
optional_field:
17+
type: integer
18+
description: Optional nullable field
19+
x-nullable: true
20+
required:
21+
- data
22+
23+
paths:
24+
/endpoint1:
25+
get:
26+
operationId: endpoint1
27+
summary: First endpoint using shared response
28+
responses:
29+
'200':
30+
$ref: '#/responses/SharedResponse'
31+
32+
/endpoint2:
33+
get:
34+
operationId: endpoint2
35+
summary: Second endpoint using shared response
36+
responses:
37+
'200':
38+
$ref: '#/responses/SharedResponse'

0 commit comments

Comments
 (0)