Repository: swagger-api/swagger-parser (java)

Issue:
Component schemas with same name are ignored after the second one
When several referenced schemas have the same name but different definitions, only the first two are created in components/schemas, the rest are ignored.
### base.yaml
```
openapi: 3.0.0
paths:
  /endpoint:
    post:
      summary: test summary
      description: test desc
      requestBody:
        required: true
        content:
          application/json:
            schema:
              oneOf:
                - $ref: 'config.yaml#/POST.type1/body'
                - $ref: 'config.yaml#/POST.type2/body'
                - $ref: 'config.yaml#/POST.type3/body'
                - $ref: 'config.yaml#/POST.type4/body'
      responses:
        '200':
          description: OK
```
### config.yaml
```
POST.type1:
  body:
    type: object
    additionalProperties: false
    properties:
      prop1:
        description: prop1
        type: boolean
POST.type2:
  body:
    type: object
    additionalProperties: false
    properties:
      prop2:
        description: prop2
        type: boolean
POST.type3:
  body:
    type: object
    additionalProperties: false
    properties:
      prop3:
        description: prop3
        type: boolean
POST.type4:
  body:
    type: object
    additionalProperties: false
    properties:
      prop3:
        description: prop3
        type: boolean
```
### main.java
```
import io.swagger.v3.oas.models.OpenAPI;
import io.swagger.v3.parser.core.models.ParseOptions;

public class Main {

    public static void main(String[] args) {
        ParseOptions options = new ParseOptions();
        options.setResolve(true);
        OpenAPI openAPI = new OpenAPIV3Parser().read("base.yaml", null, options);
        System.out.println(openAPI.getComponents().getSchemas().keySet());
    }
}
```
### Result
```
[body, body_2]
```
### Expected result
```
[body, body_1, body_2]
```
Note: In this case 3 schemas are expected since `POST.type3.body` and `POST.type4.body` have the same definition.

Pull request:
Fix issue #1501: External schema name is correctly constructed
Calculation of schema name in external ref processor takes into account that more than 2 schemas may have the same name.
The counting of schemas starts from `1` instead of `2`.

Note: the warn level of the logger in line 62 seemed too high for a normal operation so I lowered it. Let me know if you think it was `warn` for an abnormal operation.

Closes #1501 

Patch:
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 520abaa5..f0e426cf 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
@@ -49,6 +49,23 @@ public final class ExternalRefProcessor {
         this.openAPI = openAPI;
     }
 
+    private String finalNameRec(Map<String, Schema> schemas, String possiblyConflictingDefinitionName, Schema newScema,
+        int iteration) {
+        String tryName =
+            iteration == 0 ? possiblyConflictingDefinitionName : possiblyConflictingDefinitionName + "_" + iteration;
+        Schema existingModel = schemas.get(tryName);
+        if (existingModel != null) {
+            if (existingModel.get$ref() != null) {
+                // use the new model
+                existingModel = null;
+            } else if (!newScema.equals(existingModel)) {
+                LOGGER.debug("A model for " + existingModel + " already exists");
+                return finalNameRec(schemas, possiblyConflictingDefinitionName, newScema, ++iteration);
+            }
+        }
+        return tryName;
+    }
+
     public String processRefToExternalSchema(String $ref, RefFormat refFormat) {
         String renamedRef = cache.getRenamedRef($ref);
         if(renamedRef != null) {
@@ -75,34 +92,13 @@ public final class ExternalRefProcessor {
         }
 
         final String possiblyConflictingDefinitionName = computeDefinitionName($ref);
-        String tryName = null;
-        Schema existingModel = schemas.get(possiblyConflictingDefinitionName);
-
-        if (existingModel != null) {
-            LOGGER.warn("A model for " + existingModel + " already exists");
-            if(existingModel.get$ref() != null) {
-                // use the new model
-                existingModel = null;
-            }else{
-                if (!schema.equals(existingModel)){
-                    //We add a number at the end of the definition name
-                    int i = 2;
-                    for (String name : schemas.keySet()) {
-                        if (name.equals(possiblyConflictingDefinitionName)) {
-                            tryName = possiblyConflictingDefinitionName + "_" + i;
-                            existingModel = schemas.get(tryName);
-                            i++;
-                        }
-                    }
-                }
-            }
-        }
-        if (StringUtils.isNotBlank(tryName)){
-            newRef = tryName;
-        }else{
-            newRef = possiblyConflictingDefinitionName;
-        }
+        newRef = finalNameRec(schemas, possiblyConflictingDefinitionName, schema, 0);
         cache.putRenamedRef($ref, newRef);
+        Schema existingModel = schemas.get(newRef);
+        if(existingModel != null && existingModel.get$ref() != null) {
+            // use the new model
+            existingModel = null;
+        }
 
         if(existingModel == null) {
             // don't overwrite existing model reference


Test patch:
diff --git a/modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/test/OpenAPIResolverTest.java b/modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/test/OpenAPIResolverTest.java
index 5987fcdb..59fc4da0 100644
--- a/modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/test/OpenAPIResolverTest.java
+++ b/modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/test/OpenAPIResolverTest.java
@@ -777,7 +777,8 @@ public class OpenAPIResolverTest {
         assertEquals("referred", ((Schema)openAPI.getPaths().get("/oldPerson").getPost().getResponses().get("200").getContent().get("*/*").getSchema().getProperties().get("location")).getExample());
         assertEquals("referred", ((Schema)openAPI.getPaths().get("/yetAnotherPerson").getPost().getResponses().get("200").getContent().get("*/*").getSchema().getProperties().get("location")).getExample());
         assertEquals("local", ((Schema) openAPI.getComponents().getSchemas().get("PersonObj").getProperties().get("location")).getExample());
-        assertEquals("referred", ((Schema) openAPI.getComponents().getSchemas().get("PersonObj_2").getProperties().get("location")).getExample());
+        assertEquals("referred", ((Schema) openAPI.getComponents().getSchemas().get("PersonObj_1").getProperties().get("location")).getExample());
+        assertEquals("referred-again", ((Schema) openAPI.getComponents().getSchemas().get("PersonObj_2").getProperties().get("location")).getExample());
     }
 
 
diff --git a/modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/test/OpenAPIV3ParserTest.java b/modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/test/OpenAPIV3ParserTest.java
index a9ee2ad1..4568b04b 100644
--- a/modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/test/OpenAPIV3ParserTest.java
+++ b/modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/test/OpenAPIV3ParserTest.java
@@ -1082,7 +1082,7 @@ public class OpenAPIV3ParserTest {
         Schema pet = openAPI.getComponents().getSchemas().get("Pet");
         Assert.assertNotNull(pet);
         Assert.assertTrue(pet.getDiscriminator().getMapping().containsKey("Cat"));
-        Assert.assertTrue(pet.getDiscriminator().getMapping().get("Cat").equals("#/components/schemas/Cat_2"));
+        Assert.assertTrue(pet.getDiscriminator().getMapping().get("Cat").equals("#/components/schemas/Cat_1"));
     }
 
     @Test
@@ -1687,7 +1687,7 @@ public class OpenAPIV3ParserTest {
 
         assertEquals(composedCat.getAllOf().size(), 3);
         assertEquals(composedCat.getAllOf().get(0).get$ref(), "#/components/schemas/pet");
-        assertEquals(composedCat.getAllOf().get(1).get$ref(), "#/components/schemas/foo_2");
+        assertEquals(composedCat.getAllOf().get(1).get$ref(), "#/components/schemas/foo_1");
 
         return openAPI;
     }
diff --git a/modules/swagger-parser-v3/src/test/resources/refs-name-conflict/a.yaml b/modules/swagger-parser-v3/src/test/resources/refs-name-conflict/a.yaml
index dccf1c19..20ebd5f1 100644
--- a/modules/swagger-parser-v3/src/test/resources/refs-name-conflict/a.yaml
+++ b/modules/swagger-parser-v3/src/test/resources/refs-name-conflict/a.yaml
@@ -38,6 +38,17 @@ paths:
             '*/*':
               schema:
                 $ref: './refs-name-conflict/b.yaml#/components/schemas/PersonObj'
+  /thisAintAnotherPerson:
+    post:
+      summary: Create no more persons
+      description: Create no more persons
+      responses:
+        '200':
+          description: OK
+          content:
+            '*/*':
+              schema:
+                $ref: './refs-name-conflict/c.yaml#/components/schemas/PersonObj'
 components:
   schemas:
     PersonObj:
diff --git a/modules/swagger-parser-v3/src/test/resources/refs-name-conflict/c.yaml b/modules/swagger-parser-v3/src/test/resources/refs-name-conflict/c.yaml
new file mode 100644
index 00000000..4ce0c221
--- /dev/null
+++ b/modules/swagger-parser-v3/src/test/resources/refs-name-conflict/c.yaml
@@ -0,0 +1,13 @@
+openapi: 3.0.1
+servers: []
+info:
+  version: ''
+  title: ''
+paths: {}
+components:
+  schemas:
+    PersonObj:
+      properties:
+        location:
+          type: string
+          example: referred-again
\ No newline at end of file