diff --git a/modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/processors/ExternalRefProcessor.java b/modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/processors/ExternalRefProcessor.java index 4320f43208..ae7bef92ff 100644 --- a/modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/processors/ExternalRefProcessor.java +++ b/modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/processors/ExternalRefProcessor.java @@ -57,11 +57,24 @@ public ExternalRefProcessor(ResolverCache cache, OpenAPI openAPI) { private String finalNameRec(Map schemas, String possiblyConflictingDefinitionName, Schema newSchema, int iteration) { + return finalNameRec(schemas, possiblyConflictingDefinitionName, newSchema, iteration, null); + } + + private String finalNameRec(Map schemas, String possiblyConflictingDefinitionName, Schema newSchema, + int iteration, String incomingRef) { String tryName = iteration == 0 ? possiblyConflictingDefinitionName : possiblyConflictingDefinitionName + "_" + iteration; Schema existingModel = schemas.get(tryName); if (existingModel != null) { if (existingModel.get$ref() != null) { + // The name is currently held by a not-yet-resolved external $ref placeholder. It is only safe to + // reuse the name when that placeholder targets the same reference we are resolving; otherwise the + // placeholder is a distinct schema that would be silently overwritten (see issue #2333). + if (incomingRef != null && existingModel.get$ref() != null + && !existingModel.get$ref().equals(incomingRef)) { + LOGGER.debug("A different external $ref already claims the name " + tryName); + return finalNameRec(schemas, possiblyConflictingDefinitionName, newSchema, ++iteration, incomingRef); + } // use the new model existingModel = null; } else if (!newSchema.equals(existingModel)) { @@ -69,7 +82,7 @@ private String finalNameRec(Map schemas, String possiblyConflict return tryName; } LOGGER.debug("A model for " + existingModel + " already exists"); - return finalNameRec(schemas, possiblyConflictingDefinitionName, newSchema, ++iteration); + return finalNameRec(schemas, possiblyConflictingDefinitionName, newSchema, ++iteration, incomingRef); } }else{ // validate the name @@ -111,7 +124,7 @@ public String processRefToExternalSchema(String $ref, RefFormat refFormat) { } final String possiblyConflictingDefinitionName = computeDefinitionName($ref); - newRef = finalNameRec(schemas, possiblyConflictingDefinitionName, schema, 0); + newRef = finalNameRec(schemas, possiblyConflictingDefinitionName, schema, 0, $ref); cache.putRenamedRef($ref, newRef); Schema existingModel = schemas.get(newRef); if(existingModel != null && existingModel.get$ref() != null) { diff --git a/modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/test/Issue2333Test.java b/modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/test/Issue2333Test.java new file mode 100644 index 0000000000..0d5861f80f --- /dev/null +++ b/modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/test/Issue2333Test.java @@ -0,0 +1,57 @@ +package io.swagger.v3.parser.test; + +import io.swagger.v3.oas.models.OpenAPI; +import io.swagger.v3.oas.models.media.Schema; +import io.swagger.v3.parser.OpenAPIV3Parser; +import io.swagger.v3.parser.core.models.ParseOptions; +import io.swagger.v3.parser.core.models.SwaggerParseResult; +import org.testng.annotations.Test; + +import java.util.Map; + +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertNotNull; +import static org.testng.Assert.assertTrue; + +public class Issue2333Test { + + /** + * Two schemas defined in different files but sharing the same name (both {@code Pet}) must be resolved as + * distinct components. Previously the resolver reused the first model for both references, so a schema that + * was still an unresolved external {@code $ref} placeholder was silently overwritten by an unrelated one. + */ + @Test + public void referencedModelsWithSameNameFromDifferentFilesAreNotMerged() { + ParseOptions options = new ParseOptions(); + options.setResolve(true); + options.setResolveResponses(true); + + SwaggerParseResult result = new OpenAPIV3Parser() + .readLocation("./src/test/resources/issue-2333/main.yaml", null, options); + + assertNotNull(result.getOpenAPI()); + Map schemas = result.getOpenAPI().getComponents().getSchemas(); + + // SomeItem points at inventory.yaml's Pet (the "name" property) + Schema someItem = resolve(schemas, schemas.get("SomeItem")); + assertNotNull(someItem.getProperties()); + assertTrue(someItem.getProperties().containsKey("name"), + "SomeItem should resolve to inventory.yaml's Pet (name), but was " + someItem.getProperties().keySet()); + assertEquals(someItem.getProperties().size(), 1); + + // Pet points at pets.yaml's Pet (the "id" property) and must not be overwritten by inventory.yaml's Pet + Schema pet = resolve(schemas, schemas.get("Pet")); + assertNotNull(pet.getProperties()); + assertTrue(pet.getProperties().containsKey("id"), + "Pet should resolve to pets.yaml's Pet (id), but was " + pet.getProperties().keySet()); + assertEquals(pet.getProperties().size(), 1); + } + + private Schema resolve(Map schemas, Schema schema) { + if (schema != null && schema.get$ref() != null) { + String name = schema.get$ref().substring(schema.get$ref().lastIndexOf('/') + 1); + return schemas.get(name); + } + return schema; + } +} diff --git a/modules/swagger-parser-v3/src/test/resources/issue-2333/inventory.yaml b/modules/swagger-parser-v3/src/test/resources/issue-2333/inventory.yaml new file mode 100644 index 0000000000..dbd4865549 --- /dev/null +++ b/modules/swagger-parser-v3/src/test/resources/issue-2333/inventory.yaml @@ -0,0 +1,12 @@ +openapi: 3.0.3 +info: + title: Inventory API + version: 1.0.0 +paths: {} +components: + schemas: + Pet: + type: object + properties: + name: + type: string diff --git a/modules/swagger-parser-v3/src/test/resources/issue-2333/main.yaml b/modules/swagger-parser-v3/src/test/resources/issue-2333/main.yaml new file mode 100644 index 0000000000..19d2122536 --- /dev/null +++ b/modules/swagger-parser-v3/src/test/resources/issue-2333/main.yaml @@ -0,0 +1,11 @@ +openapi: 3.0.3 +info: + title: Pet Store API + version: 1.0.0 +paths: {} +components: + schemas: + SomeItem: + $ref: "inventory.yaml#/components/schemas/Pet" + Pet: + $ref: "pets.yaml#/components/schemas/Pet" diff --git a/modules/swagger-parser-v3/src/test/resources/issue-2333/pets.yaml b/modules/swagger-parser-v3/src/test/resources/issue-2333/pets.yaml new file mode 100644 index 0000000000..6071644e6f --- /dev/null +++ b/modules/swagger-parser-v3/src/test/resources/issue-2333/pets.yaml @@ -0,0 +1,12 @@ +openapi: 3.0.3 +info: + title: Pets API + version: 1.0.0 +paths: {} +components: + schemas: + Pet: + type: object + properties: + id: + type: integer