Skip to content

Commit bb8ae1f

Browse files
committed
Changes report fix(#3161): Prevent duplicate _links in allOf child schemas #3199
1 parent 98b8917 commit bb8ae1f

11 files changed

Lines changed: 919 additions & 0 deletions

File tree

springdoc-openapi-starter-common/src/main/java/org/springdoc/core/converters/PolymorphicModelConverter.java

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,28 @@ else if (resolvedSchema.getProperties().containsKey(javaType.getRawClass().getSi
120120
return resolvedSchema;
121121
}
122122

123+
/**
124+
* Removes _links from allOf child schemas to prevent duplication.
125+
* In allOf composition, child schemas (allOf[1+]) should not redefine
126+
* inherited properties like _links that come from the parent (allOf[0]).
127+
*
128+
* @param composedSchema the composed schema with allOf structure
129+
*/
130+
private void removeLinksFromAllOfChild(ComposedSchema composedSchema) {
131+
List<Schema> allOf = composedSchema.getAllOf();
132+
if (allOf != null && allOf.size() > 1) {
133+
// allOf[0] is the parent schema (first element in allOf)
134+
// allOf[1+] are the child's own properties (second element onwards in allOf)
135+
for (int i = 1; i < allOf.size(); i++) {
136+
Schema childSchema = allOf.get(i);
137+
if (childSchema != null && childSchema.getProperties() != null) {
138+
// Remove _links (inherited from parent)
139+
childSchema.getProperties().remove("_links");
140+
}
141+
}
142+
}
143+
}
144+
123145
@Override
124146
public Schema resolve(AnnotatedType type, ModelConverterContext context, Iterator<ModelConverter> chain) {
125147
JavaType javaType = springDocObjectMapper.jsonMapper().constructType(type.getType());
@@ -147,6 +169,12 @@ public Schema resolve(AnnotatedType type, ModelConverterContext context, Iterato
147169
type.resolveAsRef(true);
148170
Schema<?> resolvedSchema = chain.next().resolve(type, context, chain);
149171
resolvedSchema = getResolvedSchema(javaType, resolvedSchema);
172+
173+
if (resolvedSchema instanceof ComposedSchema composedSchema &&
174+
composedSchema.getAllOf() != null &&
175+
!composedSchema.getAllOf().isEmpty()) {
176+
removeLinksFromAllOfChild(composedSchema);
177+
}
150178
if (resolvedSchema == null || resolvedSchema.get$ref() == null) {
151179
return resolvedSchema;
152180
}
@@ -181,6 +209,14 @@ private Schema composePolymorphicSchema(AnnotatedType type, Schema schema, Colle
181209
Class<?> clazz = javaType.getRawClass();
182210
if (TYPES_TO_SKIP.stream().noneMatch(typeToSkip -> typeToSkip.equals(clazz.getSimpleName())))
183211
composedSchemas.forEach(result::addOneOfItem);
212+
213+
// Remove _links from result (composed schema) to prevent duplication
214+
if (result.getOneOf() != null) {
215+
result.getOneOf().stream()
216+
.filter(s -> s.getProperties() != null)
217+
.forEach(s -> s.getProperties().remove("_links"));
218+
}
219+
184220
return result;
185221
}
186222

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
package test.org.springdoc.api.v30.app11;
2+
import io.swagger.v3.oas.annotations.media.Schema;
3+
4+
5+
/**
6+
* Extended DTO that inherits from TestDto using allOf composition.
7+
* This class verifies that the fix for issue #3161 works correctly,
8+
* ensuring _links is not duplicated in the child schema.
9+
*/
10+
@Schema(
11+
description = "Extended DTO with allOf composition",
12+
allOf = {TestDto.class}
13+
)
14+
public class ExtendedTestDto extends TestDto {
15+
16+
private String otherField;
17+
18+
public String getOtherField() {
19+
return otherField;
20+
}
21+
22+
public void setOtherField(String otherField) {
23+
this.otherField = otherField;
24+
}
25+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
package test.org.springdoc.api.v30.app11;
2+
3+
import io.swagger.v3.oas.annotations.Operation;
4+
import io.swagger.v3.oas.annotations.tags.Tag;
5+
6+
import org.springframework.web.bind.annotation.GetMapping;
7+
import org.springframework.web.bind.annotation.RestController;
8+
9+
@RestController
10+
@Tag(name = "hateoas-controller", description = "Hateoas Controller")
11+
public class HateoasController {
12+
13+
@GetMapping(path = "/test-dto", produces = "application/json")
14+
@Operation(summary = "Get Test DTO", description = "Returns a TestDto with HATEOAS links")
15+
public TestDto getTestDto() {
16+
TestDto dto = new TestDto();
17+
dto.setField("test field value");
18+
return dto;
19+
}
20+
21+
@GetMapping(path = "/extended-test-dto", produces = "application/json")
22+
@Operation(summary = "Get Extended Test DTO", description = "Returns an ExtendedTestDto with HATEOAS links")
23+
public ExtendedTestDto getExtendedTestDto() {
24+
ExtendedTestDto dto = new ExtendedTestDto();
25+
dto.setField("parent field value");
26+
dto.setOtherField("extended field value");
27+
return dto;
28+
}
29+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,214 @@
1+
2+
package test.org.springdoc.api.v30.app11;
3+
4+
import org.junit.jupiter.api.Test;
5+
import org.springdoc.core.utils.Constants;
6+
import test.org.springdoc.api.v30.AbstractSpringDocTest;
7+
8+
import org.springframework.boot.autoconfigure.SpringBootApplication;
9+
import org.springframework.boot.test.context.SpringBootTest;
10+
import org.springframework.context.annotation.ComponentScan;
11+
import org.springframework.test.context.TestPropertySource;
12+
import org.springframework.test.web.servlet.MvcResult;
13+
14+
import static org.hamcrest.Matchers.is;
15+
import static org.skyscreamer.jsonassert.JSONAssert.assertEquals;
16+
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
17+
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
18+
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
19+
20+
/**
21+
* Test for issue #3161: Wrong HAL _links are generated in sub type of a schema
22+
* Verifies that _links field is not duplicated in extended schemas using allOf composition
23+
* Tests OpenAPI 3.0.1 spec compliance
24+
*/
25+
@SpringBootTest
26+
@TestPropertySource(properties = { "springdoc.api-docs.version=openapi_3_0" })
27+
public class SpringDocApp11Test extends AbstractSpringDocTest {
28+
29+
/**
30+
* Integration test: Validates the entire OpenAPI specification JSON against the expected schema.
31+
*
32+
* This is the main integration test that ensures:
33+
* 1. The OpenAPI version is correctly set to 3.0.1
34+
* 2. The generated OpenAPI document matches the expected JSON structure exactly
35+
* 3. All schema definitions, paths, and components are correct
36+
* 4. Issue #3161 is resolved (no duplicate _links in child schemas)
37+
*
38+
* The test compares the actual HTTP response from /v3/api-docs endpoint with
39+
* the expected specification stored in results/3.0.1/app11.json file.
40+
*
41+
* @throws Exception if the test fails or HTTP request encounters an error
42+
*/
43+
@Test
44+
public void testApp() throws Exception {
45+
// Extract test number from class name (11 from SpringDocApp11Test)
46+
String className = getClass().getSimpleName();
47+
String testNumber = className.replaceAll("[^0-9]", "");
48+
49+
// Perform GET request to OpenAPI documentation endpoint
50+
MvcResult mockMvcResult = mockMvc.perform(get(Constants.DEFAULT_API_DOCS_URL))
51+
// Verify HTTP status is 200 OK
52+
.andExpect(status().isOk())
53+
// Verify OpenAPI version is 3.0.1
54+
.andExpect(jsonPath("$.openapi", is("3.0.1")))
55+
.andReturn();
56+
57+
// Get the actual generated JSON response
58+
String result = mockMvcResult.getResponse().getContentAsString();
59+
// Load the expected JSON specification from classpath resource
60+
String expected = getContent("results/3.0.1/app" + testNumber + ".json");
61+
62+
// Compare expected and actual JSON in lenient mode (true parameter)
63+
// Lenient mode allows flexibility in JSON comparison (e.g., field order independence)
64+
try {
65+
assertEquals(expected, result, true);
66+
} catch (AssertionError e) {
67+
// Log detailed comparison results for debugging purposes
68+
System.out.println("Expected: " + expected);
69+
System.out.println("Actual: " + result);
70+
throw e;
71+
}
72+
}
73+
74+
/**
75+
* Unit test: Verifies that the parent TestDto includes the _links property from RepresentationModel.
76+
*
77+
* This test ensures that:
78+
* 1. TestDto correctly extends RepresentationModel
79+
* 2. The _links field is automatically included in the OpenAPI schema
80+
* 3. HATEOAS links support is properly recognized and documented
81+
*
82+
* The _links field is essential for REST API clients to navigate between resources
83+
* using HATEOAS (Hypermedia As The Engine Of Application State) principles.
84+
*
85+
* @throws Exception if the test fails or HTTP request encounters an error
86+
*/
87+
@Test
88+
public void testTestDtoHasHateoasLinks() throws Exception {
89+
// Perform GET request to OpenAPI documentation endpoint
90+
mockMvc.perform(get(Constants.DEFAULT_API_DOCS_URL))
91+
// Verify HTTP status is 200 OK
92+
.andExpect(status().isOk())
93+
// Verify that _links property exists in TestDto schema
94+
// Path: $.components.schemas.TestDto.properties._links
95+
.andExpect(jsonPath("$.components.schemas.TestDto.properties._links").exists())
96+
.andReturn();
97+
}
98+
99+
/**
100+
* Unit test: Verifies that ExtendedTestDto correctly uses allOf composition to inherit from TestDto.
101+
*
102+
* This test validates the OpenAPI schema composition pattern:
103+
* 1. ExtendedTestDto uses allOf keyword for schema composition
104+
* 2. The first allOf item is a $ref pointing to the parent TestDto
105+
* 3. The second allOf item contains ExtendedTestDto's own properties
106+
*
107+
* The allOf pattern ensures proper inheritance in OpenAPI where child schemas
108+
* automatically inherit all properties from parent schemas without duplication.
109+
*
110+
* @throws Exception if the test fails or HTTP request encounters an error
111+
*/
112+
@Test
113+
public void testExtendedTestDtoAllOfInheritance() throws Exception {
114+
// Perform GET request to OpenAPI documentation endpoint
115+
mockMvc.perform(get(Constants.DEFAULT_API_DOCS_URL))
116+
// Verify HTTP status is 200 OK
117+
.andExpect(status().isOk())
118+
// Verify that allOf array exists in ExtendedTestDto schema
119+
.andExpect(jsonPath("$.components.schemas.ExtendedTestDto.allOf").exists())
120+
// Verify that the first allOf item references the parent TestDto
121+
// Path: $.components.schemas.ExtendedTestDto.allOf[0].$ref
122+
.andExpect(jsonPath("$.components.schemas.ExtendedTestDto.allOf[0].$ref")
123+
.value("#/components/schemas/TestDto"))
124+
.andReturn();
125+
}
126+
127+
/**
128+
* Critical test: Verifies that ExtendedTestDto does NOT have duplicate _links in its own properties.
129+
*
130+
* This test is the core validation for issue #3161:
131+
* "Wrong HAL _links are generated in sub type of a schema"
132+
*
133+
* The problem was that child schemas incorrectly duplicated the _links field
134+
* even though it was already inherited from the parent schema via allOf.
135+
*
136+
* This test confirms:
137+
* 1. ExtendedTestDto exists in the schema definitions
138+
* 2. ExtendedTestDto uses allOf composition (does not duplicate parent properties)
139+
* 3. The _links field is inherited from TestDto, not redefined in ExtendedTestDto
140+
*
141+
* @throws Exception if the test fails or HTTP request encounters an error
142+
*/
143+
@Test
144+
public void testExtendedTestDtoNoLinksInOwnProperties() throws Exception {
145+
// Perform GET request to OpenAPI documentation endpoint
146+
MvcResult result = mockMvc.perform(get(Constants.DEFAULT_API_DOCS_URL))
147+
// Verify HTTP status is 200 OK
148+
.andExpect(status().isOk())
149+
.andReturn();
150+
151+
// Get the response body as JSON string for content-based assertions
152+
String content = result.getResponse().getContentAsString();
153+
154+
// Verify that ExtendedTestDto schema is defined in components
155+
assert(content.contains("\"ExtendedTestDto\"")) : "ExtendedTestDto not found in schema";
156+
157+
// Verify that ExtendedTestDto uses allOf composition pattern
158+
assert(content.contains("\"allOf\"")) : "allOf not found in ExtendedTestDto";
159+
160+
// Note: Full validation of _links absence is performed by testApp()
161+
// which compares the complete JSON structure with the expected specification
162+
}
163+
164+
/**
165+
* Unit test: Verifies that ExtendedTestDto contains its own unique properties.
166+
*
167+
* This test ensures that:
168+
* 1. ExtendedTestDto defines its own properties in the second allOf item
169+
* 2. The child-specific property "otherField" is correctly included
170+
* 3. Properties are nested in allOf[1].properties structure
171+
*
172+
* The structure should be:
173+
* ExtendedTestDto {
174+
* allOf: [
175+
* { $ref: "#/components/schemas/TestDto" }, // allOf[0] - parent
176+
* {
177+
* type: "object",
178+
* properties: {
179+
* otherField: { ... } // allOf[1].properties.otherField
180+
* }
181+
* }
182+
* ]
183+
* }
184+
*
185+
* @throws Exception if the test fails or HTTP request encounters an error
186+
*/
187+
@Test
188+
public void testExtendedTestDtoHasOwnProperties() throws Exception {
189+
// Perform GET request to OpenAPI documentation endpoint
190+
mockMvc.perform(get(Constants.DEFAULT_API_DOCS_URL))
191+
// Verify HTTP status is 200 OK
192+
.andExpect(status().isOk())
193+
// Verify that otherField property exists in the second allOf item
194+
// Path: $.components.schemas.ExtendedTestDto.allOf[1].properties.otherField
195+
.andExpect(jsonPath("$.components.schemas.ExtendedTestDto.allOf[1].properties.otherField").exists())
196+
.andReturn();
197+
}
198+
199+
/**
200+
* Spring Boot test configuration class.
201+
*
202+
* This inner static class configures the embedded Spring context for testing:
203+
* 1. @SpringBootApplication enables auto-configuration and component scanning
204+
* 2. @ComponentScan explicitly specifies the base package for component discovery
205+
*
206+
* The ComponentScan ensures that HateoasController and other components
207+
* in the test.org.springdoc.api.v30.app11 package are properly registered
208+
* in the Spring context and available for the integration tests.
209+
*/
210+
@SpringBootApplication
211+
@ComponentScan(basePackages = "test.org.springdoc.api.v30.app11")
212+
static class SpringDocTestApp {
213+
}
214+
}
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
package test.org.springdoc.api.v30.app11;
2+
3+
import io.swagger.v3.oas.annotations.media.Schema;
4+
5+
import org.springframework.hateoas.RepresentationModel;
6+
7+
/**
8+
* Parent DTO that extends RepresentationModel for HATEOAS support.
9+
* This class demonstrates the base schema that includes _links field
10+
* automatically added by Spring HATEOAS.
11+
*/
12+
@Schema(
13+
description = "Parent DTO extending RepresentationModel",
14+
subTypes = {ExtendedTestDto.class}
15+
)
16+
public class TestDto extends RepresentationModel<TestDto> {
17+
18+
private String field;
19+
20+
public String getField() {
21+
return field;
22+
}
23+
24+
public void setField(String field) {
25+
this.field = field;
26+
}
27+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
package test.org.springdoc.api.v31.app13;
2+
3+
import io.swagger.v3.oas.annotations.media.Schema;
4+
5+
/**
6+
* Extended DTO that inherits from TestDto using allOf composition.
7+
* This class verifies that the fix for issue #3161 works correctly,
8+
* ensuring _links is not duplicated in the child schema.
9+
*/
10+
@Schema(
11+
description = "Extended DTO with allOf composition"
12+
)
13+
public class ExtendedTestDto extends TestDto {
14+
15+
private String otherField;
16+
17+
public String getOtherField() {
18+
return otherField;
19+
}
20+
21+
public void setOtherField(String otherField) {
22+
this.otherField = otherField;
23+
}
24+
}

0 commit comments

Comments
 (0)