Skip to content

Commit 696c9b5

Browse files
committed
Merge branch 'huisoo-fix/3161-hal-links-subtype'
2 parents 8524bdd + 07cae07 commit 696c9b5

11 files changed

Lines changed: 917 additions & 3 deletions

File tree

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

Lines changed: 40 additions & 3 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,7 +169,14 @@ 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);
150-
if (resolvedSchema == null || resolvedSchema.get$ref() == null) {
172+
173+
if (resolvedSchema instanceof ComposedSchema composedSchema &&
174+
composedSchema.getAllOf() != null &&
175+
!composedSchema.getAllOf().isEmpty()) {
176+
removeLinksFromAllOfChild(composedSchema);
177+
}
178+
179+
if (resolvedSchema == null || resolvedSchema.get$ref() == null) {
151180
return resolvedSchema;
152181
}
153182
if (resolvedSchema.get$ref().contains(Components.COMPONENTS_SCHEMAS_REF)) {
@@ -175,13 +204,21 @@ private Schema composePolymorphicSchema(AnnotatedType type, Schema schema, Colle
175204
String ref = schema.get$ref();
176205
List<Schema> composedSchemas = findComposedSchemas(ref, schemas);
177206
if (composedSchemas.isEmpty()) return schema;
178-
ComposedSchema result = new ComposedSchema();
207+
ComposedSchema result = new ComposedSchema();
179208
if (isConcreteClass(type)) result.addOneOfItem(schema);
180209
JavaType javaType = springDocObjectMapper.jsonMapper().constructType(type.getType());
181210
Class<?> clazz = javaType.getRawClass();
182211
if (TYPES_TO_SKIP.stream().noneMatch(typeToSkip -> typeToSkip.equals(clazz.getSimpleName())))
183212
composedSchemas.forEach(result::addOneOfItem);
184-
return result;
213+
214+
// Remove _links from result (composed schema) to prevent duplication
215+
if (result.getOneOf() != null) {
216+
result.getOneOf().stream()
217+
.filter(s -> s.getProperties() != null)
218+
.forEach(s -> s.getProperties().remove("_links"));
219+
}
220+
221+
return result;
185222
}
186223

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