diff --git a/modules/openapi-generator-gradle-plugin/src/test/resources/specs/petstore-v3.1.yaml b/modules/openapi-generator-gradle-plugin/src/test/resources/specs/petstore-v3.1.yaml index 0a8d3d15973c..426d253a62d9 100644 --- a/modules/openapi-generator-gradle-plugin/src/test/resources/specs/petstore-v3.1.yaml +++ b/modules/openapi-generator-gradle-plugin/src/test/resources/specs/petstore-v3.1.yaml @@ -10,7 +10,7 @@ paths: /v3/pets: get: summary: List all pets - operationId: listPets + operationId: listPetsV3 tags: - pets parameters: @@ -41,7 +41,7 @@ paths: $ref: "#/components/schemas/Error" post: summary: Create a pet - operationId: createPets + operationId: createPetsV3 tags: - pets responses: @@ -56,7 +56,7 @@ paths: /v3/pets/{petId}: get: summary: Info for a specific pet - operationId: showPetById + operationId: showPetByIdV3 tags: - pets parameters: diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/config/MergedSpecBuilder.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/config/MergedSpecBuilder.java index f56dc392e882..27aa523ee363 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/config/MergedSpecBuilder.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/config/MergedSpecBuilder.java @@ -1,25 +1,27 @@ package org.openapitools.codegen.config; -import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.dataformat.yaml.YAMLFactory; -import com.google.common.collect.ImmutableMap; +import com.fasterxml.jackson.core.JsonProcessingException; import io.swagger.parser.OpenAPIParser; +import io.swagger.v3.core.util.Json; +import io.swagger.v3.core.util.Yaml; +import io.swagger.v3.oas.models.Components; import io.swagger.v3.oas.models.OpenAPI; +import io.swagger.v3.oas.models.PathItem; +import io.swagger.v3.oas.models.Paths; +import io.swagger.v3.oas.models.info.Info; import io.swagger.v3.oas.models.servers.Server; import io.swagger.v3.parser.core.models.ParseOptions; -import org.apache.commons.lang3.ObjectUtils; import org.openapitools.codegen.auth.AuthParser; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.io.File; import java.io.IOException; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; -import java.nio.file.Paths; import java.nio.file.StandardOpenOption; import java.util.*; -import java.util.function.Predicate; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -27,6 +29,8 @@ public class MergedSpecBuilder { private static final Logger LOGGER = LoggerFactory.getLogger(MergedSpecBuilder.class); + private static final Set SPEC_EXTENSIONS = new HashSet<>(Arrays.asList(".yaml", ".yml", ".json")); + private final String inputSpecRootDirectory; private final String mergeFileName; private final String mergedFileInfoName; @@ -56,11 +60,10 @@ public String buildMergedSpec() { } LOGGER.info("In spec root directory {} found specs {}", inputSpecRootDirectory, specRelatedPaths); - String openapiVersion = null; boolean isJson = false; ParseOptions options = new ParseOptions(); options.setResolve(true); - List allPaths = new ArrayList<>(); + List parsedSpecs = new ArrayList<>(); List allServers = new ArrayList<>(); for (String specRelatedPath : specRelatedPaths) { @@ -72,26 +75,37 @@ public String buildMergedSpec() { .readLocation(specPath, AuthParser.parse(auth), options) .getOpenAPI(); - if (openapiVersion == null) { - openapiVersion = result.getOpenapi(); - if (specRelatedPath.toLowerCase(Locale.ROOT).endsWith(".json")) { - isJson = true; - } + if (result == null) { + LOGGER.error("Failed to read file: {}. It would be ignored", specPath); + continue; + } + + if (parsedSpecs.isEmpty() && specRelatedPath.toLowerCase(Locale.ROOT).endsWith(".json")) { + isJson = true; } - allServers.addAll(ObjectUtils.defaultIfNull(result.getServers(), Collections.emptyList())); - allPaths.add(new SpecWithPaths(specRelatedPath, result.getPaths().keySet())); + allServers.addAll(Optional.ofNullable(result.getServers()).orElse(Collections.emptyList())); + parsedSpecs.add(result); } catch (Exception e) { LOGGER.error("Failed to read file: {}. It would be ignored", specPath); } } - Map mergedSpec = generatedMergedSpec(openapiVersion, allPaths, allServers); + if (parsedSpecs.isEmpty()) { + throw new RuntimeException("Spec directory doesn't contain any valid specification"); + } + + OpenAPI merged = mergeSpecs(parsedSpecs, allServers); + String mergedFilename = this.mergeFileName + (isJson ? ".json" : ".yaml"); - Path mergedFilePath = Paths.get(inputSpecRootDirectory, mergedFilename); + Path mergedFilePath = java.nio.file.Paths.get(inputSpecRootDirectory, mergedFilename); try { - ObjectMapper objectMapper = isJson ? new ObjectMapper() : new ObjectMapper(new YAMLFactory()); - Files.write(mergedFilePath, objectMapper.writeValueAsBytes(mergedSpec), StandardOpenOption.CREATE, StandardOpenOption.WRITE); + String content = isJson + ? Json.mapper().writerWithDefaultPrettyPrinter().writeValueAsString(merged) + : Yaml.mapper().writerWithDefaultPrettyPrinter().writeValueAsString(merged); + Files.write(mergedFilePath, content.getBytes(StandardCharsets.UTF_8), StandardOpenOption.CREATE, StandardOpenOption.WRITE); + } catch (JsonProcessingException e) { + throw new RuntimeException("Failed to serialize merged spec", e); } catch (IOException e) { throw new RuntimeException(e); } @@ -99,43 +113,113 @@ public String buildMergedSpec() { return mergedFilePath.toString(); } - private Map generatedMergedSpec(String openapiVersion, List allPaths, List allServers) { - Map spec = generateHeader(openapiVersion, mergedFileInfoName, mergedFileInfoDescription, mergedFileInfoVersion, allServers); - Map paths = new HashMap<>(); - spec.put("paths", paths); - - for (SpecWithPaths specWithPaths : allPaths) { - for (String path : specWithPaths.paths) { - String specRelatedPath = "./" + specWithPaths.specRelatedPath + "#/paths/" + path.replace("/", "~1"); - paths.put(path, ImmutableMap.of( - "$ref", specRelatedPath - )); + /** + * Merges a list of parsed OpenAPI specs into a single spec. + * + *

Path items are merged by HTTP method: if two specs define the same URL path, their + * operations are combined (e.g. GET from one file + POST from another) rather than one + * overwriting the other. A warning is logged when the same path+method appears in multiple specs; + * the first occurrence is kept.

+ * + *

Component maps (schemas, responses, requestBodies, parameters, headers, examples, + * links, callbacks, securitySchemes) are merged by name. Structurally identical duplicates + * are silently deduplicated. A warning is logged if the same component name appears with + * different definitions; the first definition is kept.

+ */ + OpenAPI mergeSpecs(List specs, List allServers) { + OpenAPI merged = new OpenAPI(); + merged.openapi(specs.get(0).getOpenapi() != null ? specs.get(0).getOpenapi() : "3.0.3"); + + Info info = new Info() + .title(mergedFileInfoName) + .description(mergedFileInfoDescription) + .version(mergedFileInfoVersion); + merged.info(info); + + List distinctServerUrls = allServers.stream() + .map(Server::getUrl) + .filter(Objects::nonNull) + .distinct() + .collect(Collectors.toList()); + if (distinctServerUrls.isEmpty()) { + merged.addServersItem(new Server().url("http://localhost:8080")); + } else { + distinctServerUrls.forEach(url -> merged.addServersItem(new Server().url(url))); + } + + merged.setPaths(new Paths()); + merged.setComponents(new Components()); + + for (OpenAPI spec : specs) { + if (spec.getPaths() != null) { + spec.getPaths().forEach((pathKey, incomingPathItem) -> { + PathItem existing = merged.getPaths().get(pathKey); + if (existing == null) { + merged.getPaths().addPathItem(pathKey, incomingPathItem); + } else { + mergePathItem(existing, incomingPathItem, pathKey); + } + }); + } + if (spec.getComponents() != null) { + mergeComponents(merged.getComponents(), spec.getComponents()); } } - return spec; + return merged; } - private static Map generateHeader(String openapiVersion, String title, String description, String version, List allServers) { - Map map = new HashMap<>(); - map.put("openapi", openapiVersion); - map.put("info", ImmutableMap.of( - "title", title, - "description", description, - "version", version - )); - - Set> servers = allServers.stream() - .map(Server::getUrl) - .distinct() - .map(url -> ImmutableMap.of("url", url)) - .collect(Collectors.collectingAndThen(Collectors.toSet(), Optional::of)) - .filter(Predicate.not(Set::isEmpty)) - .orElseGet(() -> Collections.singleton(ImmutableMap.of("url", "http://localhost:8080"))); + /** + * Merges HTTP method operations from {@code incoming} into {@code existing} for the same path URL. + * Path-level metadata (summary, description, servers, parameters, extensions) is kept from + * {@code existing} (i.e. the first spec that defined this path). A warning is logged for any + * path+method that already exists in {@code existing}. + */ + private void mergePathItem(PathItem existing, PathItem incoming, String pathKey) { + if (incoming.readOperationsMap() == null) { + return; + } + incoming.readOperationsMap().forEach((method, operation) -> { + if (existing.readOperationsMap() != null && existing.readOperationsMap().containsKey(method)) { + LOGGER.warn("Path+method collision during spec merge: {} {} is defined in multiple specs. Keeping the first occurrence.", method, pathKey); + } else { + existing.operation(method, operation); + } + }); + } - map.put("servers", servers); + /** + * Merges all component maps from {@code source} into {@code target}. + * Identical definitions are silently deduplicated. Conflicting definitions (same name, different + * structure) generate a warning and keep the first definition. + */ + private void mergeComponents(Components target, Components source) { + mergeComponentMap(target.getSchemas(), source.getSchemas(), "schema", target::addSchemas); + mergeComponentMap(target.getResponses(), source.getResponses(), "response", target::addResponses); + mergeComponentMap(target.getRequestBodies(), source.getRequestBodies(), "requestBody", target::addRequestBodies); + mergeComponentMap(target.getParameters(), source.getParameters(), "parameter", target::addParameters); + mergeComponentMap(target.getHeaders(), source.getHeaders(), "header", target::addHeaders); + mergeComponentMap(target.getExamples(), source.getExamples(), "example", target::addExamples); + mergeComponentMap(target.getLinks(), source.getLinks(), "link", target::addLinks); + mergeComponentMap(target.getCallbacks(), source.getCallbacks(), "callback", target::addCallbacks); + mergeComponentMap(target.getSecuritySchemes(), source.getSecuritySchemes(), "securityScheme", target::addSecuritySchemes); + } - return map; + private void mergeComponentMap(Map existing, Map incoming, + String typeName, java.util.function.BiConsumer adder) { + if (incoming == null) { + return; + } + incoming.forEach((name, value) -> { + if (existing != null && existing.containsKey(name)) { + if (!Objects.equals(existing.get(name), value)) { + LOGGER.warn("Component {} name conflict during spec merge: '{}' is defined in multiple specs with different definitions. Keeping the first definition.", typeName, name); + } + // identical or keeping first — either way, skip + } else { + adder.accept(name, value); + } + }); } private List getAllSpecFilesInDirectory() { @@ -143,7 +227,12 @@ private List getAllSpecFilesInDirectory() { try (Stream pathStream = Files.walk(rootDirectory)) { return pathStream .filter(path -> !Files.isDirectory(path)) + .filter(path -> { + String name = path.getFileName().toString().toLowerCase(Locale.ROOT); + return SPEC_EXTENSIONS.stream().anyMatch(name::endsWith); + }) .map(path -> rootDirectory.relativize(path).toString()) + .sorted() .collect(Collectors.toList()); } catch (IOException e) { throw new RuntimeException("Exception while listing files in spec root directory: " + inputSpecRootDirectory, e); @@ -152,22 +241,12 @@ private List getAllSpecFilesInDirectory() { private void deleteMergedFileFromPreviousRun() { try { - Files.deleteIfExists(Paths.get(inputSpecRootDirectory + File.separator + mergeFileName + ".json")); - } catch (IOException e) { + Files.deleteIfExists(java.nio.file.Paths.get(inputSpecRootDirectory + File.separator + mergeFileName + ".json")); + } catch (IOException ignored) { } try { - Files.deleteIfExists(Paths.get(inputSpecRootDirectory + File.separator + mergeFileName + ".yaml")); - } catch (IOException e) { - } - } - - private static class SpecWithPaths { - private final String specRelatedPath; - private final Set paths; - - private SpecWithPaths(final String specRelatedPath, final Set paths) { - this.specRelatedPath = specRelatedPath; - this.paths = paths; + Files.deleteIfExists(java.nio.file.Paths.get(inputSpecRootDirectory + File.separator + mergeFileName + ".yaml")); + } catch (IOException ignored) { } } } diff --git a/modules/openapi-generator/src/test/java/org/openapitools/codegen/config/MergedSpecBuilderTest.java b/modules/openapi-generator/src/test/java/org/openapitools/codegen/config/MergedSpecBuilderTest.java index 42f9e0a547d1..330d1429c70b 100644 --- a/modules/openapi-generator/src/test/java/org/openapitools/codegen/config/MergedSpecBuilderTest.java +++ b/modules/openapi-generator/src/test/java/org/openapitools/codegen/config/MergedSpecBuilderTest.java @@ -3,22 +3,28 @@ import com.google.common.collect.ImmutableMap; import io.swagger.parser.OpenAPIParser; import io.swagger.v3.oas.models.OpenAPI; +import io.swagger.v3.oas.models.PathItem; import io.swagger.v3.parser.core.models.ParseOptions; import org.openapitools.codegen.ClientOptInput; import org.openapitools.codegen.DefaultGenerator; import org.openapitools.codegen.java.assertions.JavaFileAssert; import org.openapitools.codegen.languages.SpringCodegen; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.read.ListAppender; +import org.slf4j.LoggerFactory; import org.testng.annotations.Test; import java.io.File; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Paths; +import java.util.List; import java.util.Map; import java.util.function.Function; import java.util.stream.Collectors; import static org.openapitools.codegen.languages.SpringCodegen.*; +import static org.testng.Assert.*; public class MergedSpecBuilderTest { @@ -101,4 +107,242 @@ private void assertFilesFromMergedSpec(String mergedSpec) throws IOException { .assertMethod("getSpec2Field").hasReturnType("BigDecimal"); } + // ---- Path-collision tests ---- + + @Test + public void shouldMergeSpecsWithCollidingPaths_yaml() throws IOException { + shouldMergeSpecsWithCollidingPaths("yaml"); + } + + @Test + public void shouldMergeSpecsWithCollidingPaths_json() throws IOException { + shouldMergeSpecsWithCollidingPaths("json"); + } + + private void shouldMergeSpecsWithCollidingPaths(String fileExt) throws IOException { + File dir = Files.createTempDirectory("spec-collision").toFile().getCanonicalFile(); + dir.deleteOnExit(); + + Files.copy(Paths.get("src/test/resources/bugs/mergerTest/spec1." + fileExt), dir.toPath().resolve("spec1." + fileExt)); + Files.copy(Paths.get("src/test/resources/bugs/mergerTest/spec-collision." + fileExt), dir.toPath().resolve("spec-collision." + fileExt)); + + String mergedSpec = new MergedSpecBuilder(dir.getAbsolutePath().replace('\\', '/'), "_merged") + .buildMergedSpec(); + + ParseOptions parseOptions = new ParseOptions(); + parseOptions.setResolve(true); + OpenAPI openAPI = new OpenAPIParser().readLocation(mergedSpec, null, parseOptions).getOpenAPI(); + + assertNotNull(openAPI.getPaths(), "Merged spec must have paths"); + + // /spec1 must have both GET (from spec1) and POST (from spec-collision) + PathItem spec1Path = openAPI.getPaths().get("/spec1"); + assertNotNull(spec1Path, "/spec1 path must exist in merged spec"); + assertNotNull(spec1Path.getGet(), "/spec1 GET must be present (from spec1)"); + assertNotNull(spec1Path.getPost(), "/spec1 POST must be present (from spec-collision)"); + + // /collision path from spec-collision must also be present + assertNotNull(openAPI.getPaths().get("/collision"), "/collision path must exist in merged spec"); + + // schemas from both specs must be present + assertNotNull(openAPI.getComponents().getSchemas().get("Spec1Model"), "Spec1Model schema must exist"); + assertNotNull(openAPI.getComponents().getSchemas().get("CollisionModel"), "CollisionModel schema must exist"); + } + + // ---- Vendor extensions tests ---- + + @Test + public void shouldPreserveVendorExtensions_yaml() throws IOException { + shouldPreserveVendorExtensions("yaml"); + } + + @Test + public void shouldPreserveVendorExtensions_json() throws IOException { + shouldPreserveVendorExtensions("json"); + } + + private void shouldPreserveVendorExtensions(String fileExt) throws IOException { + File dir = Files.createTempDirectory("spec-extensions").toFile().getCanonicalFile(); + dir.deleteOnExit(); + + Files.copy(Paths.get("src/test/resources/bugs/mergerTest/spec-extensions." + fileExt), dir.toPath().resolve("spec-extensions." + fileExt)); + + String mergedSpec = new MergedSpecBuilder(dir.getAbsolutePath().replace('\\', '/'), "_merged") + .buildMergedSpec(); + + ParseOptions parseOptions = new ParseOptions(); + parseOptions.setResolve(true); + OpenAPI openAPI = new OpenAPIParser().readLocation(mergedSpec, null, parseOptions).getOpenAPI(); + + assertNotNull(openAPI.getPaths(), "Merged spec must have paths"); + + PathItem extPath = openAPI.getPaths().get("/ext-path"); + assertNotNull(extPath, "/ext-path must exist"); + assertNotNull(extPath.getExtensions(), "Path-level extensions must be preserved"); + assertEquals(extPath.getExtensions().get("x-custom-path-ext"), "path-level-value", "x-custom-path-ext must be preserved on path"); + + assertNotNull(extPath.getGet(), "GET operation must exist on /ext-path"); + assertNotNull(extPath.getGet().getExtensions(), "Operation-level extensions must be preserved"); + assertEquals(extPath.getGet().getExtensions().get("x-custom-op-ext"), "operation-level-value", "x-custom-op-ext must be preserved on operation"); + + assertNotNull(openAPI.getComponents().getSchemas().get("ExtModel"), "ExtModel must exist"); + assertNotNull(openAPI.getComponents().getSchemas().get("ExtModel").getExtensions(), "Schema-level extensions must be preserved"); + assertEquals(openAPI.getComponents().getSchemas().get("ExtModel").getExtensions().get("x-custom-schema-ext"), "schema-level-value", "x-custom-schema-ext must be preserved on schema"); + } + + // ---- Component merging tests ---- + + @Test + public void shouldMergeComponentsFromBothSpecs_yaml() throws IOException { + shouldMergeComponentsFromBothSpecs("yaml"); + } + + @Test + public void shouldMergeComponentsFromBothSpecs_json() throws IOException { + shouldMergeComponentsFromBothSpecs("json"); + } + + private void shouldMergeComponentsFromBothSpecs(String fileExt) throws IOException { + File dir = Files.createTempDirectory("spec-components").toFile().getCanonicalFile(); + dir.deleteOnExit(); + + Files.copy(Paths.get("src/test/resources/bugs/mergerTest/spec1." + fileExt), dir.toPath().resolve("spec1." + fileExt)); + Files.copy(Paths.get("src/test/resources/bugs/mergerTest/spec2." + fileExt), dir.toPath().resolve("spec2." + fileExt)); + Files.copy(Paths.get("src/test/resources/bugs/mergerTest/spec-collision." + fileExt), dir.toPath().resolve("spec-collision." + fileExt)); + + String mergedSpec = new MergedSpecBuilder(dir.getAbsolutePath().replace('\\', '/'), "_merged") + .buildMergedSpec(); + + ParseOptions parseOptions = new ParseOptions(); + parseOptions.setResolve(true); + OpenAPI openAPI = new OpenAPIParser().readLocation(mergedSpec, null, parseOptions).getOpenAPI(); + + Map schemas = openAPI.getComponents().getSchemas(); + assertNotNull(schemas.get("Spec1Model"), "Spec1Model must be present"); + assertNotNull(schemas.get("Spec2Model"), "Spec2Model must be present"); + assertNotNull(schemas.get("CollisionModel"), "CollisionModel must be present"); + } + + // ---- Identical duplicate schema test ---- + + @Test + public void shouldHandleDuplicateIdenticalSchemas_yaml() throws IOException { + shouldHandleDuplicateIdenticalSchemas("yaml"); + } + + @Test + public void shouldHandleDuplicateIdenticalSchemas_json() throws IOException { + shouldHandleDuplicateIdenticalSchemas("json"); + } + + /** + * spec-collision defines Spec1Model identically to spec1 — same name, same structure. + * The merged result must contain exactly one Spec1Model without errors. + */ + private void shouldHandleDuplicateIdenticalSchemas(String fileExt) throws IOException { + File dir = Files.createTempDirectory("spec-dup-schema").toFile().getCanonicalFile(); + dir.deleteOnExit(); + + Files.copy(Paths.get("src/test/resources/bugs/mergerTest/spec1." + fileExt), dir.toPath().resolve("spec1." + fileExt)); + Files.copy(Paths.get("src/test/resources/bugs/mergerTest/spec-collision." + fileExt), dir.toPath().resolve("spec-collision." + fileExt)); + + // Must not throw + String mergedSpec = new MergedSpecBuilder(dir.getAbsolutePath().replace('\\', '/'), "_merged") + .buildMergedSpec(); + + ParseOptions parseOptions = new ParseOptions(); + parseOptions.setResolve(true); + OpenAPI openAPI = new OpenAPIParser().readLocation(mergedSpec, null, parseOptions).getOpenAPI(); + + // Spec1Model is defined in both spec1 and spec-collision with identical structure + assertNotNull(openAPI.getComponents().getSchemas().get("Spec1Model"), "Spec1Model must be present exactly once"); + } + + // ---- Non-spec file filter test ---- + + @Test + public void shouldIgnoreNonSpecFiles_yaml() throws IOException { + shouldIgnoreNonSpecFiles("yaml"); + } + + @Test + public void shouldIgnoreNonSpecFiles_json() throws IOException { + shouldIgnoreNonSpecFiles("json"); + } + + private void shouldIgnoreNonSpecFiles(String fileExt) throws IOException { + File dir = Files.createTempDirectory("spec-noext").toFile().getCanonicalFile(); + dir.deleteOnExit(); + + Files.copy(Paths.get("src/test/resources/bugs/mergerTest/spec1." + fileExt), dir.toPath().resolve("spec1." + fileExt)); + // Copy the non-spec .txt file into the same directory + Files.copy(Paths.get("src/test/resources/bugs/mergerTest/spec-noext.txt"), dir.toPath().resolve("spec-noext.txt")); + + // Must not throw despite the .txt file being present + String mergedSpec = new MergedSpecBuilder(dir.getAbsolutePath().replace('\\', '/'), "_merged") + .buildMergedSpec(); + + ParseOptions parseOptions = new ParseOptions(); + parseOptions.setResolve(true); + OpenAPI openAPI = new OpenAPIParser().readLocation(mergedSpec, null, parseOptions).getOpenAPI(); + + // Spec from spec1 must be present; no error from the .txt file + assertNotNull(openAPI.getPaths().get("/spec1"), "/spec1 path must be present"); + assertNotNull(openAPI.getComponents().getSchemas().get("Spec1Model"), "Spec1Model must be present"); + } + + // ---- Schema name conflict warning test ---- + + @Test + public void shouldWarnOnSchemaNameConflict_yaml() throws IOException { + shouldWarnOnSchemaNameConflict("yaml"); + } + + @Test + public void shouldWarnOnSchemaNameConflict_json() throws IOException { + shouldWarnOnSchemaNameConflict("json"); + } + + /** + * spec1 and spec-schema-conflict both define Spec1Model but with different properties. + * The merge must keep the first definition and emit a WARN log. + */ + private void shouldWarnOnSchemaNameConflict(String fileExt) throws IOException { + ch.qos.logback.classic.Logger logger = + (ch.qos.logback.classic.Logger) LoggerFactory.getLogger(MergedSpecBuilder.class); + ListAppender listAppender = new ListAppender<>(); + listAppender.start(); + logger.addAppender(listAppender); + + try { + File dir = Files.createTempDirectory("spec-schema-conflict").toFile().getCanonicalFile(); + dir.deleteOnExit(); + + Files.copy(Paths.get("src/test/resources/bugs/mergerTest/spec1." + fileExt), dir.toPath().resolve("spec1." + fileExt)); + Files.copy(Paths.get("src/test/resources/bugs/mergerTest/spec-schema-conflict." + fileExt), dir.toPath().resolve("spec-schema-conflict." + fileExt)); + + String mergedSpec = new MergedSpecBuilder(dir.getAbsolutePath().replace('\\', '/'), "_merged") + .buildMergedSpec(); + + ParseOptions parseOptions = new ParseOptions(); + parseOptions.setResolve(true); + OpenAPI openAPI = new OpenAPIParser().readLocation(mergedSpec, null, parseOptions).getOpenAPI(); + + // First alphabetical file (spec-schema-conflict: differentField) is kept + assertNotNull(openAPI.getComponents().getSchemas().get("Spec1Model"), "Spec1Model must be present"); + assertNotNull(openAPI.getComponents().getSchemas().get("Spec1Model").getProperties().get("differentField"), + "differentField (from first-alphabetical spec) must be kept"); + assertNull(openAPI.getComponents().getSchemas().get("Spec1Model").getProperties().get("spec1Field"), + "spec1Field (from second spec) must NOT be present"); + + // A WARN about the conflict must have been logged + List warnLogs = listAppender.list.stream() + .filter(e -> e.getLevel() == ch.qos.logback.classic.Level.WARN) + .filter(e -> e.getFormattedMessage().contains("Spec1Model")) + .collect(Collectors.toList()); + assertFalse(warnLogs.isEmpty(), "A WARN log about the Spec1Model name conflict must be emitted"); + } finally { + logger.detachAppender(listAppender); + } + } } diff --git a/modules/openapi-generator/src/test/resources/bugs/mergerTest/spec-collision.json b/modules/openapi-generator/src/test/resources/bugs/mergerTest/spec-collision.json new file mode 100644 index 000000000000..a027821d8639 --- /dev/null +++ b/modules/openapi-generator/src/test/resources/bugs/mergerTest/spec-collision.json @@ -0,0 +1,70 @@ +{ + "openapi": "3.0.3", + "info": { + "version": "1.0.0", + "title": "Collision Test Spec" + }, + "servers": [ + { "url": "api.my-domain.com/my-context-root/v1" } + ], + "paths": { + "/spec1": { + "post": { + "tags": ["spec1"], + "summary": "create spec1", + "operationId": "createSpec1", + "requestBody": { + "required": true, + "content": { + "application/json": { + "schema": { "$ref": "#/components/schemas/Spec1Model" } + } + } + }, + "responses": { + "201": { + "description": "Created", + "content": { + "application/json": { + "schema": { "$ref": "#/components/schemas/Spec1Model" } + } + } + } + } + } + }, + "/collision": { + "get": { + "tags": ["collision"], + "summary": "collision endpoint", + "operationId": "collisionOperation", + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": { "$ref": "#/components/schemas/CollisionModel" } + } + } + } + } + } + } + }, + "components": { + "schemas": { + "CollisionModel": { + "type": "object", + "properties": { + "collisionField": { "type": "string" } + } + }, + "Spec1Model": { + "type": "object", + "properties": { + "spec1Field": { "type": "string" } + } + } + } + } +} diff --git a/modules/openapi-generator/src/test/resources/bugs/mergerTest/spec-collision.yaml b/modules/openapi-generator/src/test/resources/bugs/mergerTest/spec-collision.yaml new file mode 100644 index 000000000000..52c4fe2dab8f --- /dev/null +++ b/modules/openapi-generator/src/test/resources/bugs/mergerTest/spec-collision.yaml @@ -0,0 +1,52 @@ +openapi: 3.0.3 +info: + version: 1.0.0 + title: Collision Test Spec +servers: + - url: api.my-domain.com/my-context-root/v1 +paths: + /spec1: + post: + tags: + - spec1 + summary: create spec1 + operationId: createSpec1 + requestBody: + required: true + content: + application/json: + schema: + $ref: '#/components/schemas/Spec1Model' + responses: + '201': + description: Created + content: + application/json: + schema: + $ref: '#/components/schemas/Spec1Model' + /collision: + get: + tags: + - collision + summary: collision endpoint + operationId: collisionOperation + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/CollisionModel' + +components: + schemas: + CollisionModel: + type: object + properties: + collisionField: + type: string + Spec1Model: + type: object + properties: + spec1Field: + type: string diff --git a/modules/openapi-generator/src/test/resources/bugs/mergerTest/spec-extensions.json b/modules/openapi-generator/src/test/resources/bugs/mergerTest/spec-extensions.json new file mode 100644 index 000000000000..38fd32ff96e8 --- /dev/null +++ b/modules/openapi-generator/src/test/resources/bugs/mergerTest/spec-extensions.json @@ -0,0 +1,42 @@ +{ + "openapi": "3.0.3", + "info": { + "version": "1.0.0", + "title": "Extensions Test Spec" + }, + "servers": [ + { "url": "api.my-domain.com/my-context-root/v1" } + ], + "paths": { + "/ext-path": { + "x-custom-path-ext": "path-level-value", + "get": { + "tags": ["ext"], + "summary": "extensions test", + "operationId": "extOperation", + "x-custom-op-ext": "operation-level-value", + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": { "$ref": "#/components/schemas/ExtModel" } + } + } + } + } + } + } + }, + "components": { + "schemas": { + "ExtModel": { + "type": "object", + "x-custom-schema-ext": "schema-level-value", + "properties": { + "extField": { "type": "string" } + } + } + } + } +} diff --git a/modules/openapi-generator/src/test/resources/bugs/mergerTest/spec-extensions.yaml b/modules/openapi-generator/src/test/resources/bugs/mergerTest/spec-extensions.yaml new file mode 100644 index 000000000000..85161c8cf770 --- /dev/null +++ b/modules/openapi-generator/src/test/resources/bugs/mergerTest/spec-extensions.yaml @@ -0,0 +1,31 @@ +openapi: 3.0.3 +info: + version: 1.0.0 + title: Extensions Test Spec +servers: + - url: api.my-domain.com/my-context-root/v1 +paths: + /ext-path: + x-custom-path-ext: path-level-value + get: + tags: + - ext + summary: extensions test + operationId: extOperation + x-custom-op-ext: operation-level-value + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/ExtModel' + +components: + schemas: + ExtModel: + type: object + x-custom-schema-ext: schema-level-value + properties: + extField: + type: string diff --git a/modules/openapi-generator/src/test/resources/bugs/mergerTest/spec-noext.txt b/modules/openapi-generator/src/test/resources/bugs/mergerTest/spec-noext.txt new file mode 100644 index 000000000000..b92555272056 --- /dev/null +++ b/modules/openapi-generator/src/test/resources/bugs/mergerTest/spec-noext.txt @@ -0,0 +1 @@ +This is not a spec file. It should be silently ignored by MergedSpecBuilder. diff --git a/modules/openapi-generator/src/test/resources/bugs/mergerTest/spec-schema-conflict.json b/modules/openapi-generator/src/test/resources/bugs/mergerTest/spec-schema-conflict.json new file mode 100644 index 000000000000..c6472790c110 --- /dev/null +++ b/modules/openapi-generator/src/test/resources/bugs/mergerTest/spec-schema-conflict.json @@ -0,0 +1,39 @@ +{ + "openapi": "3.0.3", + "info": { + "version": "1.0.0", + "title": "Schema Conflict Test Spec" + }, + "servers": [ + { "url": "api.my-domain.com/my-context-root/v1" } + ], + "paths": { + "/schema-conflict": { + "get": { + "tags": ["schemaConflict"], + "summary": "schema conflict test", + "operationId": "schemaConflictOperation", + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": { "$ref": "#/components/schemas/Spec1Model" } + } + } + } + } + } + } + }, + "components": { + "schemas": { + "Spec1Model": { + "type": "object", + "properties": { + "differentField": { "type": "integer" } + } + } + } + } +} diff --git a/modules/openapi-generator/src/test/resources/bugs/mergerTest/spec-schema-conflict.yaml b/modules/openapi-generator/src/test/resources/bugs/mergerTest/spec-schema-conflict.yaml new file mode 100644 index 000000000000..54ed1b860ea8 --- /dev/null +++ b/modules/openapi-generator/src/test/resources/bugs/mergerTest/spec-schema-conflict.yaml @@ -0,0 +1,28 @@ +openapi: 3.0.3 +info: + version: 1.0.0 + title: Schema Conflict Test Spec +servers: + - url: api.my-domain.com/my-context-root/v1 +paths: + /schema-conflict: + get: + tags: + - schemaConflict + summary: schema conflict test + operationId: schemaConflictOperation + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/Spec1Model' + +components: + schemas: + Spec1Model: + type: object + properties: + differentField: + type: integer