Fail on duplicate schema name (#5068)

I encountered questions by integration partners when we had duplicate
names for models which automatically was supplied suffix numbers. We
don't want internal scala refactors resulting in changes for client and
I would have liked to have caught this earlier so that I could have
assigned the models more appropriate names.

---------

Co-authored-by: felix <[email protected]>

Author: felix-hedenstrom

doc/docs/openapi.md

@@ -216,6 +216,10 @@ Options can be customised by providing an instance of `OpenAPIDocsOptions` to th
 * `failOnDuplicateOperationId`: if set to `true`, the interpreter will throw an exception if it encounters two endpoints
   with the same operation id. An OpenAPI document with duplicate operation ids is not valid. Code generators can 
   silently drop duplicates. This is also verified by the [endpoint verifier](../testing.md).
+* `failOnDuplicateSchemaName`: if set to `true`, the interpreter will throw an exception if it encounters two schemas
+  which (without automatic deduplication by adding a numeric suffix) would be identical. Having automatically resolved 
+  de-duplications might result in different names depending on the order of endpoints. This might result in false 
+  positive changes in the OpenApi document.
 
 ## Inlined and referenced schemas
 

docs/apispec-docs/src/main/scala/sttp/tapir/docs/apispec/schema/SchemasForEndpoints.scala

@@ -11,6 +11,7 @@ class SchemasForEndpoints(
     es: Iterable[AnyEndpoint],
     schemaName: SName => String,
     markOptionsAsNullable: Boolean,
+    failOnDuplicateSchemaName: Boolean,
     additionalOutputs: List[EndpointOutput[_]]
 ) {
 
@@ -24,7 +25,8 @@ class SchemasForEndpoints(
         forInput(e.securityInput) ++ forInput(e.input) ++ forOutput(e.errorOutput) ++ forOutput(e.output)
       ) ++ additionalOutputs.flatMap(forOutput(_))
     )
-    val keysToIds: Map[SchemaKey, SchemaId] = calculateUniqueIds(keyedCombinedSchemas.map(_._1), (key: SchemaKey) => schemaName(key.name))
+    val keysToIds: Map[SchemaKey, SchemaId] =
+      calculateUniqueIds(keyedCombinedSchemas.map(_._1), (key: SchemaKey) => schemaName(key.name), failOnDuplicateSchemaName)
 
     val toSchemaReference = new ToSchemaReference(keysToIds, keyedCombinedSchemas.toMap)
     val tschemaToASchema = new TSchemaToASchema(schemaName, toSchemaReference, markOptionsAsNullable)

docs/apispec-docs/src/main/scala/sttp/tapir/docs/apispec/schema/TapirSchemaToJsonSchema.scala

@@ -14,6 +14,7 @@ object TapirSchemaToJsonSchema {
   def apply(
       schema: TSchema[_],
       markOptionsAsNullable: Boolean,
+      failOnDuplicateSchemaName: Boolean = false,
       metaSchema: MetaSchema = MetaSchemaDraft202012,
       schemaName: TSchema.SName => String = defaultSchemaName
   ): ASchema = {
@@ -32,7 +33,7 @@ object TapirSchemaToJsonSchema {
 
     val keyedSchemas = ToKeyedSchemas.uniqueCombined(asKeyedSchemas)
 
-    val keysToIds = calculateUniqueIds(keyedSchemas.map(_._1), (key: SchemaKey) => schemaName(key.name))
+    val keysToIds = calculateUniqueIds(keyedSchemas.map(_._1), (key: SchemaKey) => schemaName(key.name), failOnDuplicateSchemaName)
     val toSchemaReference = new ToSchemaReference(keysToIds, keyedSchemas.toMap, refRoot = "#/$defs/")
     val tschemaToASchema = new TSchemaToASchema(schemaName, toSchemaReference, markOptionsAsNullable)
     val keysToSchemas = keyedSchemas.map(td => (td._1, tschemaToASchema(td._2, allowReference = false))).toListMap

docs/apispec-docs/src/main/scala/sttp/tapir/docs/apispec/schema/schema.scala

@@ -20,9 +20,9 @@ package object schema {
   private[docs] type KeyedSchema = (SchemaKey, TSchema[_])
   private[docs] type SchemaId = String
 
-  private[docs] def calculateUniqueIds[T](ts: Iterable[T], toIdBase: T => String): Map[T, String] = {
+  private[docs] def calculateUniqueIds[T](ts: Iterable[T], toIdBase: T => String, failOnDuplicateSchemaName: Boolean): Map[T, String] = {
     case class Assigment(idToT: Map[String, T], tToId: Map[T, String])
-    ts
+    val result = ts
       .foldLeft(Assigment(Map.empty, Map.empty)) { case (Assigment(idToT, tToId), t) =>
         val id = uniqueString(toIdBase(t), n => !idToT.contains(n) || idToT.get(n).contains(t))
 
@@ -31,7 +31,21 @@ package object schema {
           tToId + (t -> id)
         )
       }
-      .tToId
+
+    if (failOnDuplicateSchemaName) {
+      val conflicts: Map[T, String] = result.tToId.collect { case (t, id) if toIdBase(t) != id => t -> id }
+
+      if (conflicts.nonEmpty) {
+        // Extract unique base names that had conflicts
+        val baseNames = conflicts.map { case (t, _) => toIdBase(t) }.toSet.toList.sorted
+        throw new IllegalStateException(
+          s"Duplicate schema names found: ${baseNames.mkString(", ")}. " +
+            "Consider using unique class names or customize the schemaName function."
+        )
+      }
+    }
+
+    result.tToId
   }
 
   private[docs] def propagateMetadataForOption[T, E](schema: TSchema[T], opt: TSchemaType.SOption[T, E]): TSchemaType.SOption[T, E] = {

docs/apispec-docs/src/test/scala/sttp/tapir/docs/apispec/schema/TapirSchemaToJsonSchemaTest.scala

@@ -23,7 +23,7 @@ class TapirSchemaToJsonSchemaTest extends AnyFlatSpec with Matchers with OptionV
     val tSchema = implicitly[Schema[Parent]]
 
     // when
-    val result: ASchema = TapirSchemaToJsonSchema(tSchema, markOptionsAsNullable = true)
+    val result: ASchema = TapirSchemaToJsonSchema(tSchema, markOptionsAsNullable = true, failOnDuplicateSchemaName = false)
 
     // then
     result.asJson.deepDropNullValues shouldBe json"""{"$$schema":"https://json-schema.org/draft/2020-12/schema","title":"Parent","required":["innerChildField"],"type":"object","properties":{"innerChildField":{"$$ref":"#/$$defs/Child"}},"$$defs":{"Child":{"title":"Child","required":["childId"],"type":"object","properties":{"childId":{"type":"string"},"childNames":{"type":"array","items":{"type":"string"}}}}}}"""

docs/asyncapi-docs/src/main/scala/sttp/tapir/docs/asyncapi/EndpointToAsyncAPIDocs.scala

@@ -17,7 +17,13 @@ private[asyncapi] object EndpointToAsyncAPIDocs {
     val wsEndpointsWithWrapper = es.map(e => (e, findWebSocket(e))).collect { case (e, Some(ws)) => (e, ws) }
     val wsEndpoints = wsEndpointsWithWrapper.map(_._1).map(nameAllPathCapturesInEndpoint)
     val (keyToSchema, schemas) =
-      new SchemasForEndpoints(wsEndpoints, options.schemaName, markOptionsAsNullable = false, additionalOutputs = Nil)
+      new SchemasForEndpoints(
+        wsEndpoints,
+        options.schemaName,
+        markOptionsAsNullable = false,
+        failOnDuplicateSchemaName = false,
+        additionalOutputs = Nil
+      )
         .apply()
     val (codecToMessageKey, keyToMessage) = new MessagesForEndpoints(schemas, options.schemaName)(
       wsEndpointsWithWrapper.map(_._2)

docs/asyncapi-docs/src/main/scala/sttp/tapir/docs/asyncapi/MessagesForEndpoints.scala

@@ -20,7 +20,7 @@ private[asyncapi] class MessagesForEndpoints(tschemaToASchema: TSchemaToASchema,
     val codecs: Iterable[CodecWithInfo[_]] = wss.flatMap(ws => codecsFor(ws.wrapped))
     val codecToData: ListMap[CodecWithInfo[_], CodecData] = codecs.toList.map(ci => ci -> toData(ci.codec)).toListMap
 
-    val dataToKey = calculateUniqueIds(codecToData.values.toSet, dataToName)
+    val dataToKey = calculateUniqueIds(codecToData.values.toSet, dataToName, failOnDuplicateSchemaName = false)
     val codecToKey = codecToData.map { case (ci, data) => ci.codec -> dataToKey(data) }.toMap[Codec[_, _, _ <: CodecFormat], String]
     val keyToMessage = codecToData.map { case (ci, data) => dataToKey(data) -> message(ci) }
 

docs/openapi-docs/src/main/scala/sttp/tapir/docs/openapi/EndpointToOpenAPIDocs.scala

@@ -18,7 +18,8 @@ private[openapi] object EndpointToOpenAPIDocs {
     val es2 = es.filter(e => findWebSocket(e).isEmpty).map(nameAllPathCapturesInEndpoint)
     val additionalOutputs = es2.flatMap(e => options.defaultDecodeFailureOutput(e.input)).toSet.toList
     val (idToSchema, tschemaToASchema) =
-      new SchemasForEndpoints(es2, options.schemaName, options.markOptionsAsNullable, additionalOutputs).apply()
+      new SchemasForEndpoints(es2, options.schemaName, options.markOptionsAsNullable, options.failOnDuplicateSchemaName, additionalOutputs)
+        .apply()
     val securitySchemes = SecuritySchemesForEndpoints(es2, apiKeyAuthTypeName = "apiKey")
     val pathCreator = new EndpointToOpenAPIPaths(tschemaToASchema, securitySchemes, options)
     val componentsCreator = new EndpointToOpenAPIComponents(idToSchema, securitySchemes)

docs/openapi-docs/src/main/scala/sttp/tapir/docs/openapi/OpenAPIDocsOptions.scala

@@ -11,7 +11,8 @@ case class OpenAPIDocsOptions(
     schemaName: SName => String = defaultSchemaName,
     defaultDecodeFailureOutput: EndpointInput[_] => Option[EndpointOutput[_]] = OpenAPIDocsOptions.defaultDecodeFailureOutput,
     markOptionsAsNullable: Boolean = false,
-    failOnDuplicateOperationId: Boolean = false
+    failOnDuplicateOperationId: Boolean = false,
+    failOnDuplicateSchemaName: Boolean = false
 )
 
 object OpenAPIDocsOptions {

docs/openapi-docs/src/test/scalajvm/sttp/tapir/docs/openapi/EndpointToOpenAPIDocsTest.scala

@@ -12,6 +12,11 @@ import sttp.tapir.tests.Files._
 import sttp.tapir.tests.Mapping._
 import sttp.tapir.tests.Multipart._
 import sttp.tapir.tests.OneOf._
+import io.circe.generic.auto._
+import sttp.tapir.docs.openapi.dtos.a.{Pet => APet}
+import sttp.tapir.docs.openapi.dtos.b.{Pet => BPet}
+import sttp.tapir.generic.auto._
+import sttp.tapir.json.circe._
 
 class EndpointToOpenAPIDocsTest extends AnyFunSuite with Matchers {
 
@@ -134,4 +139,29 @@ class EndpointToOpenAPIDocsTest extends AnyFunSuite with Matchers {
 
     OpenAPIDocsInterpreter(options).toOpenAPI(es, Info("title", "19.2-beta-RC1"))
   }
+
+  test("should fail when OpenAPIDocsOptions.failOnDuplicateSchemaName is true and there are duplicate schema names") {
+    val e: sttp.tapir.Endpoint[Unit, APet, Unit, BPet, Any] = endpoint
+      .in(jsonBody[APet])
+      .out(jsonBody[BPet])
+
+    val options = OpenAPIDocsOptions.default.copy(failOnDuplicateSchemaName = true)
+
+    val thrown = intercept[IllegalStateException] {
+      OpenAPIDocsInterpreter(options).toOpenAPI(e, Info("Entities", "1.0"))
+    }
+
+    thrown.getMessage should include("Duplicate schema names found")
+    thrown.getMessage should include("Pet")
+  }
+
+  test("should pass when OpenAPIDocsOptions.failOnDuplicateSchemaName is false and there are duplicate schema names") {
+    val e: sttp.tapir.Endpoint[Unit, APet, Unit, BPet, Any] = endpoint
+      .in(jsonBody[APet])
+      .out(jsonBody[BPet])
+
+    val options = OpenAPIDocsOptions.default.copy(failOnDuplicateSchemaName = false)
+
+    OpenAPIDocsInterpreter(options).toOpenAPI(e, Info("Entities", "1.0"))
+  }
 }