test(testing): add test to detect duplicate endpoint names (#3908)

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

doc/docs/openapi.md

@@ -213,6 +213,9 @@ Options can be customised by providing an instance of `OpenAPIDocsOptions` to th
 * `schemaName`: specifies how schema names are created from the full type name. By default, this takes the last
   component of a dot-separated type name. Suffixes might be added at a later stage to disambiguate between different
   schemas with same names.
+* `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).
 
 ## Inlined and referenced schemas
 

doc/testing.md

@@ -354,3 +354,24 @@ Results in:
 ```scala mdoc
 result2.toString
 ```
+
+### Duplicated endpoint names
+
+Duplicate endpoint names will generate duplicate operation ids, when generating OpenAPI or AsyncAPI documentation. As
+the operation ids should be unique, this is reported as an error:
+
+Example 1:
+
+```scala mdoc:silent
+import sttp.tapir.testing.EndpointVerifier
+
+val ep1 = endpoint.name("e1").get.in("a")
+val ep2 = endpoint.name("e1").get.in("b")
+val result3 = EndpointVerifier(List(ep1, ep2))
+```
+
+Results in:
+
+```scala mdoc
+result3.toString
+```

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

@@ -25,9 +25,24 @@ private[openapi] object EndpointToOpenAPIDocs {
 
     val base = apiToOpenApi(api, componentsCreator, docsExtensions)
 
-    es2.map(pathCreator.pathItem).foldLeft(base) { case (current, (path, pathItem)) =>
+    val spec = es2.map(pathCreator.pathItem).foldLeft(base) { case (current, (path, pathItem)) =>
       current.addPathItem(path, pathItem)
     }
+
+    if (options.failOnDuplicateOperationId) {
+
+      val operationIds = spec.paths.pathItems.flatMap { item =>
+        List(item._2.get, item._2.put, item._2.post, item._2.delete, item._2.options, item._2.head, item._2.patch, item._2.trace)
+          .flatMap(_.flatMap(_.operationId))
+      }
+
+      operationIds
+        .groupBy(identity)
+        .filter(_._2.size > 1)
+        .foreach { case (name, _) => throw new IllegalStateException(s"Duplicate endpoints names found: ${name}") }
+    }
+
+    spec
   }
 
   private def apiToOpenApi(info: Info, componentsCreator: EndpointToOpenAPIComponents, docsExtensions: List[DocsExtension[_]]): OpenAPI = {

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

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

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

@@ -4,6 +4,7 @@ import sttp.apispec.openapi.Info
 import sttp.tapir.tests._
 import org.scalatest.funsuite.AnyFunSuite
 import org.scalatest.matchers.should.Matchers
+import sttp.tapir.endpoint
 import sttp.tapir.AnyEndpoint
 import sttp.tapir.tests.Security._
 import sttp.tapir.tests.Basic._
@@ -110,4 +111,27 @@ class EndpointToOpenAPIDocsTest extends AnyFunSuite with Matchers {
       OpenAPIDocsInterpreter().toOpenAPI(e, Info("title", "19.2-beta-RC1"))
     }
   }
+
+  test("should fail when OpenAPIDocsOptions.failOnDuplicateOperationId is true and there are duplicate operationIds") {
+    val e1 = endpoint.get.name("a")
+    val e2 = endpoint.post.name("a")
+
+    val options = OpenAPIDocsOptions.default.copy(failOnDuplicateOperationId = true)
+
+    val es = List(e1, e2)
+
+    assertThrows[IllegalStateException](
+      OpenAPIDocsInterpreter(options).toOpenAPI(es, Info("title", "19.2-beta-RC1"))
+    )
+  }
+  test("should pass when OpenAPIDocsOptions.failOnDuplicateOperationId is false and there are duplicate operationIds") {
+    val e1 = endpoint.get.name("a")
+    val e2 = endpoint.post.name("a")
+
+    val options = OpenAPIDocsOptions.default.copy(failOnDuplicateOperationId = false)
+
+    val es = List(e1, e2)
+
+    OpenAPIDocsInterpreter(options).toOpenAPI(es, Info("title", "19.2-beta-RC1"))
+  }
 }

testing/src/main/scala/sttp/tapir/testing/EndpointVerificationError.scala

@@ -67,3 +67,10 @@ case class UnexpectedBodyError(e: AnyEndpoint, statusCode: StatusCode) extends E
   override def toString: String =
     s"An endpoint ${e.show} may return status code ${statusCode} with body, which is not allowed by specificiation."
 }
+
+/** The given name is used by multiple endpoints. This leads to duplicate operation ids being generated, when interpreting endpoints as
+  * OpenAPI/AsyncAPI documentation. Which, in turn, causes incorrect behavior of code generators, which might silently drop endpoints.
+  */
+case class DuplicatedNameError(name: String) extends EndpointVerificationError {
+  override def toString: String = s"Duplicate endpoints names found: $name"
+}

testing/src/main/scala/sttp/tapir/testing/EndpointVerifier.scala

@@ -12,7 +12,8 @@ object EndpointVerifier {
     findShadowedEndpoints(endpoints, List()).groupBy(_.e).map(_._2.head).toSet ++
       findIncorrectPaths(endpoints).toSet ++
       findDuplicatedMethodDefinitions(endpoints).toSet ++
-      findIncorrectStatusWithBody(endpoints).toSet
+      findIncorrectStatusWithBody(endpoints).toSet ++
+      findDuplicateNames(endpoints).toSet
   }
 
   private def findIncorrectPaths(endpoints: List[AnyEndpoint]): List[IncorrectPathsError] = {
@@ -99,6 +100,15 @@ object EndpointVerifier {
   private def inputDefinedMethods(input: EndpointInput[_]): Vector[Method] = {
     input.traverseInputs { case EndpointInput.FixedMethod(m, _, _) => Vector(m) }
   }
+
+  private def findDuplicateNames(endpoints: List[AnyEndpoint]): List[EndpointVerificationError] = {
+    endpoints
+      .filter(_.info.name.isDefined)
+      .groupBy(_.info.name)
+      .filter(_._2.size > 1)
+      .map { case (name, _) => DuplicatedNameError(name.getOrElse("")) }
+      .toList
+  }
 }
 
 private sealed trait PathComponent

testing/src/test/scala/sttp/tapir/testing/EndpointVerifierTest.scala

@@ -322,6 +322,15 @@ class EndpointVerifierTest extends AnyFlatSpecLike with Matchers {
       UnexpectedBodyError(e5, StatusCode.NoContent)
     )
   }
+
+  it should "detect duplicate names among endpoints, all endpoints names (operationId's) must be unique" in {
+    val e1 = endpoint.in("a").name("Z")
+    val e2 = endpoint.in("b").name("Z")
+
+    val result = EndpointVerifier(List(e1, e2))
+
+    result shouldBe Set(DuplicatedNameError("Z"))
+  }
 }
 
 sealed trait ErrorInfo