Merge pull request #531 from NDLANO/error-refactoring

Refactor tapir error handling to improve readability

article-api/src/main/scala/no/ndla/articleapi/ComponentRegistry.scala

@@ -11,23 +11,24 @@ package no.ndla.articleapi
 import com.zaxxer.hikari.HikariDataSource
 import no.ndla.articleapi.caching.MemoizeHelpers
 import no.ndla.articleapi.controller.{ArticleControllerV2, InternController, SwaggerDocControllerConfig}
-import no.ndla.articleapi.integration._
+import no.ndla.articleapi.integration.*
 import no.ndla.articleapi.repository.ArticleRepository
-import no.ndla.articleapi.service._
-import no.ndla.articleapi.service.search._
+import no.ndla.articleapi.service.*
+import no.ndla.articleapi.service.search.*
 import no.ndla.articleapi.validation.ContentValidator
 import no.ndla.articleapi.integration.SearchApiClient
-import no.ndla.articleapi.model.api.ErrorHelpers
+import no.ndla.articleapi.model.api.ErrorHandling
 import no.ndla.articleapi.model.domain.DBArticle
 import no.ndla.common.Clock
 import no.ndla.common.configuration.BaseComponentRegistry
 import no.ndla.network.NdlaClient
-import no.ndla.network.tapir.{Routes, Service, SwaggerControllerConfig, TapirErrorHelpers, TapirHealthController}
+import no.ndla.network.tapir.TapirApplication
 import no.ndla.network.clients.{FeideApiClient, RedisClient}
 import no.ndla.search.{BaseIndexService, Elastic4sClient}
 
 class ComponentRegistry(properties: ArticleApiProperties)
     extends BaseComponentRegistry[ArticleApiProperties]
+    with TapirApplication
     with Props
     with DataSource
     with InternController
@@ -50,13 +51,9 @@ class ComponentRegistry(properties: ArticleApiProperties)
     with WriteService
     with ContentValidator
     with Clock
-    with ErrorHelpers
+    with ErrorHandling
     with DBArticle
     with DBMigrator
-    with Routes[Eff]
-    with TapirErrorHelpers
-    with TapirHealthController
-    with SwaggerControllerConfig
     with SwaggerDocControllerConfig
     with FrontpageApiClient {
   override val props: ArticleApiProperties = properties
@@ -65,9 +62,9 @@ class ComponentRegistry(properties: ArticleApiProperties)
   override val dataSource: HikariDataSource = DataSource.getHikariDataSource
   DataSource.connectToDatabase()
 
-  lazy val internController                             = new InternController
-  lazy val articleControllerV2                          = new ArticleControllerV2
-  lazy val healthController: TapirHealthController[Eff] = new TapirHealthController[Eff]
+  lazy val internController                        = new InternController
+  lazy val articleControllerV2                     = new ArticleControllerV2
+  lazy val healthController: TapirHealthController = new TapirHealthController
 
   lazy val articleRepository    = new ArticleRepository
   lazy val articleSearchService = new ArticleSearchService
@@ -98,5 +95,5 @@ class ComponentRegistry(properties: ArticleApiProperties)
     SwaggerDocControllerConfig.swaggerInfo
   )
 
-  override def services: List[Service[Eff]] = swagger.getServices()
+  override def services: List[TapirController] = swagger.getServices()
 }

article-api/src/main/scala/no/ndla/articleapi/MainClass.scala

@@ -11,7 +11,7 @@ package no.ndla.articleapi
 import no.ndla.common.Warmup
 import no.ndla.network.tapir.NdlaTapirMain
 
-class MainClass(override val props: ArticleApiProperties) extends NdlaTapirMain[Eff] {
+class MainClass(override val props: ArticleApiProperties) extends NdlaTapirMain {
   val componentRegistry = new ComponentRegistry(props)
 
   private def warmupRequest = (path: String, options: Map[String, String]) =>

article-api/src/main/scala/no/ndla/articleapi/controller/ArticleControllerV2.scala

@@ -16,14 +16,14 @@ import no.ndla.articleapi.model.domain.Sort
 import no.ndla.articleapi.service.search.{ArticleSearchService, SearchConverterService}
 import no.ndla.articleapi.service.{ConverterService, ReadService, WriteService}
 import no.ndla.articleapi.validation.ContentValidator
-import no.ndla.articleapi.{Eff, Props}
+import no.ndla.articleapi.Props
 import no.ndla.common.ContentURIUtil.parseArticleIdAndRevision
 import no.ndla.common.model.api.CommaSeparatedList.*
 import no.ndla.language.Language.AllLanguages
 import no.ndla.network.tapir.NoNullJsonPrinter.jsonBody
 import no.ndla.network.tapir.Parameters.feideHeader
-import no.ndla.network.tapir.{DynamicHeaders, Service}
-import no.ndla.network.tapir.TapirErrors.errorOutputsFor
+import no.ndla.network.tapir.{DynamicHeaders, TapirController}
+import no.ndla.network.tapir.TapirUtil.errorOutputsFor
 import sttp.model.{Header, MediaType}
 import sttp.tapir.EndpointIO.annotations.{header, jsonbody}
 import sttp.tapir.*
@@ -33,19 +33,13 @@ import sttp.tapir.server.ServerEndpoint
 import scala.util.{Failure, Success, Try}
 
 trait ArticleControllerV2 {
-  this: ReadService
-    with WriteService
-    with ArticleSearchService
-    with SearchConverterService
-    with ConverterService
-    with ContentValidator
-    with Props
-    with ErrorHelpers =>
+  this: ReadService & WriteService & ArticleSearchService & SearchConverterService & ConverterService &
+    ContentValidator & Props & ErrorHandling & TapirController =>
   val articleControllerV2: ArticleControllerV2
 
   import props._
 
-  class ArticleControllerV2() extends Service[Eff] {
+  class ArticleControllerV2() extends TapirController {
     protected val applicationDescription = "Services for accessing articles from NDLA."
 
     override val serviceName: String         = "articles"
@@ -251,7 +245,7 @@ trait ArticleControllerV2 {
                 shouldScroll,
                 feideToken
               )
-            }.handleErrorsOrOk
+            }
         }
     }
 
@@ -287,7 +281,6 @@ trait ArticleControllerV2 {
             pageSize,
             feideToken
           )
-          .handleErrorsOrOk
       }
 
     def postSearch: ServerEndpoint[Any, Eff] = endpoint.post
@@ -328,7 +321,7 @@ trait ArticleControllerV2 {
             shouldScroll,
             feideToken
           )
-        }.handleErrorsOrOk
+        }
       }
 
     def getSingle: ServerEndpoint[Any, Eff] = endpoint.get
@@ -348,7 +341,7 @@ trait ArticleControllerV2 {
           case (Success(articleId), inlineRevision) =>
             val revision = inlineRevision.orElse(revisionQuery)
             readService.withIdV2(articleId, language, fallback, revision, feideToken)
-        }).map(_.Ok()).handleErrorsOrOk
+        }).map(_.Ok())
       }
 
     def getRevisions: ServerEndpoint[Any, Eff] = endpoint.get
@@ -359,7 +352,7 @@ trait ArticleControllerV2 {
       .errorOut(errorOutputsFor(404, 500))
       .out(jsonBody[Seq[Int]])
       .serverLogicPure(articleId => {
-        readService.getRevisions(articleId).handleErrorsOrOk
+        readService.getRevisions(articleId)
       })
 
     def getByExternal: ServerEndpoint[Any, Eff] = endpoint.get
@@ -402,7 +395,6 @@ trait ArticleControllerV2 {
         readService
           .getArticleFrontpageRSS(slug)
           .map(_.Ok(List(Header.contentType(MediaType.ApplicationXml))))
-          .handleErrorsOrOk
       }
 
     override val endpoints: List[ServerEndpoint[Any, Eff]] = List(

article-api/src/main/scala/no/ndla/articleapi/controller/InternController.scala

@@ -8,49 +8,39 @@
 
 package no.ndla.articleapi.controller
 
-import cats.implicits._
+import cats.implicits.*
 import com.typesafe.scalalogging.StrictLogging
-import io.circe.generic.auto._
-import no.ndla.articleapi.{Eff, Props}
-import no.ndla.articleapi.model.api._
+import io.circe.generic.auto.*
+import no.ndla.articleapi.Props
+import no.ndla.articleapi.model.api.*
 import no.ndla.articleapi.model.domain.{ArticleIds, DBArticle}
 import no.ndla.articleapi.repository.ArticleRepository
-import no.ndla.articleapi.service._
+import no.ndla.articleapi.service.*
 import no.ndla.articleapi.service.search.{ArticleIndexService, IndexService}
 import no.ndla.articleapi.validation.ContentValidator
-import no.ndla.common.model.api.CommaSeparatedList._
+import no.ndla.common.model.api.CommaSeparatedList.*
 import no.ndla.common.model.domain.article.Article
 import no.ndla.language.Language
 import no.ndla.network.tapir.NoNullJsonPrinter.jsonBody
-import no.ndla.network.tapir.TapirErrors.errorOutputsFor
+import no.ndla.network.tapir.TapirUtil.errorOutputsFor
 import no.ndla.network.tapir.auth.Permission.ARTICLE_API_WRITE
-import no.ndla.network.tapir.{Service, TapirErrorHelpers}
+import no.ndla.network.tapir.TapirController
 import sttp.model.StatusCode
-import sttp.tapir._
-import sttp.tapir.generic.auto._
+import sttp.tapir.*
+import sttp.tapir.generic.auto.*
 import sttp.tapir.server.ServerEndpoint
 
 import java.util.concurrent.{Executors, TimeUnit}
-import scala.concurrent._
+import scala.concurrent.*
 import scala.concurrent.duration.Duration
 import scala.util.{Failure, Success}
 
 trait InternController {
-  this: ReadService
-    with WriteService
-    with ConverterService
-    with ArticleRepository
-    with IndexService
-    with ArticleIndexService
-    with ContentValidator
-    with TapirErrorHelpers
-    with Props
-    with DBArticle =>
+  this: ReadService & WriteService & ConverterService & ArticleRepository & IndexService & ArticleIndexService &
+    ContentValidator & Props & DBArticle & TapirController =>
   val internController: InternController
 
-  class InternController extends Service[Eff] with StrictLogging {
-    import ErrorHelpers._
-
+  class InternController extends TapirController with StrictLogging {
     override val prefix: EndpointInput[Unit] = "intern"
     override val enableSwagger               = false
     private val stringInternalServerError    = statusCode(StatusCode.InternalServerError).and(stringBody)
@@ -162,7 +152,7 @@ trait InternController {
       .serverLogicPure { case (importValidate, article) =>
         contentValidator
           .validateArticle(article, isImported = importValidate)
-          .handleErrorsOrOk
+
       }
 
     def updateArticle: ServerEndpoint[Any, Eff] = endpoint.post
@@ -183,7 +173,7 @@ trait InternController {
             useImportValidation,
             useSoftValidation
           )
-          .handleErrorsOrOk
+
       }
 
     def deleteArticle: ServerEndpoint[Any, Eff] = endpoint.delete
@@ -194,7 +184,7 @@ trait InternController {
       .requirePermission(ARTICLE_API_WRITE)
       .serverLogicPure { _ => params =>
         val (id, revision) = params
-        writeService.deleteArticle(id, revision).handleErrorsOrOk
+        writeService.deleteArticle(id, revision)
       }
 
     def unpublishArticle: ServerEndpoint[Any, Eff] = endpoint.post
@@ -205,7 +195,7 @@ trait InternController {
       .requirePermission(ARTICLE_API_WRITE)
       .serverLogicPure { _ => params =>
         val (id, revision) = params
-        writeService.unpublishArticle(id, revision).handleErrorsOrOk
+        writeService.unpublishArticle(id, revision)
       }
 
     def partialPublishArticle: ServerEndpoint[Any, Eff] = endpoint.patch
@@ -218,7 +208,7 @@ trait InternController {
       .requirePermission(ARTICLE_API_WRITE)
       .serverLogicPure { _ => params =>
         val (articleId, partialUpdateBody, language, fallback) = params
-        writeService.partialUpdate(articleId, partialUpdateBody, language, fallback).handleErrorsOrOk
+        writeService.partialUpdate(articleId, partialUpdateBody, language, fallback)
       }
 
     override val endpoints: List[ServerEndpoint[Any, Eff]] = List(

article-api/src/main/scala/no/ndla/articleapi/controller/SwaggerDocControllerConfig.scala

@@ -10,10 +10,10 @@ package no.ndla.articleapi.controller
 import no.ndla.articleapi.Props
 import no.ndla.network.tapir.auth.Permission
 import no.ndla.network.tapir.{SwaggerControllerConfig, SwaggerInfo}
-import sttp.tapir._
+import sttp.tapir.*
 
-trait SwaggerDocControllerConfig extends SwaggerControllerConfig {
-  this: Props =>
+trait SwaggerDocControllerConfig {
+  this: Props & SwaggerControllerConfig =>
 
   object SwaggerDocControllerConfig {
     private val scopes = Permission.toSwaggerMap(

article-api/src/main/scala/no/ndla/articleapi/model/api/NDLAErrors.scala

@@ -18,13 +18,13 @@ import no.ndla.network.tapir.{
   AllErrors,
   ErrorBody,
   NotFoundWithSupportedLanguages,
-  TapirErrorHelpers,
+  TapirErrorHandling,
   ValidationErrorBody
 }
 import no.ndla.search.{IndexNotFoundException, NdlaSearchException}
 import org.postgresql.util.PSQLException
 
-trait ErrorHelpers extends TapirErrorHelpers with StrictLogging {
+trait ErrorHandling extends TapirErrorHandling with StrictLogging {
   this: Props with Clock with DataSource =>
 
   import ErrorHelpers._

article-api/src/main/scala/no/ndla/articleapi/service/ReadService.scala

@@ -15,7 +15,7 @@ import no.ndla.articleapi.Props
 import no.ndla.articleapi.caching.MemoizeHelpers
 import no.ndla.articleapi.integration.FrontpageApiClient
 import no.ndla.articleapi.model.api
-import no.ndla.articleapi.model.api.{ArticleSummaryV2, ErrorHelpers, NotFoundException}
+import no.ndla.articleapi.model.api.{ArticleSummaryV2, ErrorHandling, NotFoundException}
 import no.ndla.articleapi.model.domain.*
 import no.ndla.articleapi.model.search.SearchResult
 import no.ndla.articleapi.repository.ArticleRepository
@@ -39,7 +39,7 @@ import scala.util.{Failure, Success, Try}
 
 trait ReadService {
   this: ArticleRepository & FeideApiClient & ConverterService & ArticleSearchService & SearchConverterService &
-    MemoizeHelpers & Props & ErrorHelpers & FrontpageApiClient =>
+    MemoizeHelpers & Props & ErrorHandling & FrontpageApiClient =>
   val readService: ReadService
 
   class ReadService extends StrictLogging {

article-api/src/main/scala/no/ndla/articleapi/service/search/ArticleSearchService.scala

@@ -13,7 +13,7 @@ import com.sksamuel.elastic4s.requests.searches.queries.compound.BoolQuery
 import com.typesafe.scalalogging.StrictLogging
 import no.ndla.articleapi.Props
 import no.ndla.articleapi.model.api
-import no.ndla.articleapi.model.api.{ArticleSummaryV2, ErrorHelpers}
+import no.ndla.articleapi.model.api.{ArticleSummaryV2, ErrorHandling}
 import no.ndla.articleapi.model.domain._
 import no.ndla.articleapi.model.search.SearchResult
 import no.ndla.articleapi.service.ConverterService
@@ -34,7 +34,7 @@ trait ArticleSearchService {
     with ArticleIndexService
     with ConverterService
     with Props
-    with ErrorHelpers =>
+    with ErrorHandling =>
   val articleSearchService: ArticleSearchService
 
   import props._