diff --git a/core/src/main/scala/za/co/absa/db/fadb/DBFunction.scala b/core/src/main/scala/za/co/absa/db/fadb/DBFunction.scala index 68c253f2..cf70b4eb 100644 --- a/core/src/main/scala/za/co/absa/db/fadb/DBFunction.scala +++ b/core/src/main/scala/za/co/absa/db/fadb/DBFunction.scala @@ -18,9 +18,10 @@ package za.co.absa.db.fadb import cats.MonadError import cats.implicits.toFlatMapOps +import za.co.absa.db.fadb.exceptions.StatusException import za.co.absa.db.fadb.status.aggregation.StatusAggregator import za.co.absa.db.fadb.status.handling.StatusHandling -import za.co.absa.db.fadb.status.{FailedOrRows, FailedOrRow, Row} +import za.co.absa.db.fadb.status.{FailedOrRow, FailedOrRows, FunctionStatus} import scala.language.higherKinds @@ -148,7 +149,7 @@ abstract class DBFunctionWithStatus[I, R, E <: DBEngine[F], F[_]](functionNameOv protected def query(values: I)(implicit me: MonadError[F, Throwable]): F[dBEngine.QueryWithStatusType[R]] // To be provided by an implementation of QueryStatusHandling - override def checkStatus[D](statusWithData: Row[D]): FailedOrRow[D] + override def checkStatus(functionStatus: FunctionStatus): Option[StatusException] } object DBFunction { diff --git a/core/src/main/scala/za/co/absa/db/fadb/status/handling/StatusHandling.scala b/core/src/main/scala/za/co/absa/db/fadb/status/handling/StatusHandling.scala index bb85da90..43e773c2 100644 --- a/core/src/main/scala/za/co/absa/db/fadb/status/handling/StatusHandling.scala +++ b/core/src/main/scala/za/co/absa/db/fadb/status/handling/StatusHandling.scala @@ -16,19 +16,20 @@ package za.co.absa.db.fadb.status.handling -import za.co.absa.db.fadb.status.{FailedOrRow, Row} +import za.co.absa.db.fadb.exceptions.StatusException +import za.co.absa.db.fadb.status.FunctionStatus /** * `StatusHandling` is a base trait that defines the interface for handling the status of a function invocation. - * It provides a method to check the status of a function invocation with data. + * It provides a method to check the status of a function invocation. */ trait StatusHandling { /** * Checks the status of a function invocation. - * @param statusWithData - The status of the function invocation with data. - * @return Either a `StatusException` if the status code indicates an error, or the data (along with the status - * information so that it's retrievable) if the status code is successful. + * @param functionStatus - The status of the function invocation. + * @return Some with a `StatusException` if the status code indicates an error, + * or None if the status code is successful. */ - def checkStatus[D](statusWithData: Row[D]): FailedOrRow[D] + def checkStatus(functionStatus: FunctionStatus): Option[StatusException] } diff --git a/core/src/main/scala/za/co/absa/db/fadb/status/handling/implementations/StandardStatusHandling.scala b/core/src/main/scala/za/co/absa/db/fadb/status/handling/implementations/StandardStatusHandling.scala index 68c0dfcd..3a18c4de 100644 --- a/core/src/main/scala/za/co/absa/db/fadb/status/handling/implementations/StandardStatusHandling.scala +++ b/core/src/main/scala/za/co/absa/db/fadb/status/handling/implementations/StandardStatusHandling.scala @@ -17,7 +17,7 @@ package za.co.absa.db.fadb.status.handling.implementations import za.co.absa.db.fadb.exceptions._ -import za.co.absa.db.fadb.status.{FailedOrRow, Row} +import za.co.absa.db.fadb.status.FunctionStatus import za.co.absa.db.fadb.status.handling.StatusHandling /** @@ -29,16 +29,15 @@ trait StandardStatusHandling extends StatusHandling { /** * Checks the status of a function invocation. */ - override def checkStatus[D](statusWithData: Row[D]): FailedOrRow[D] = { - val functionStatus = statusWithData.functionStatus + override def checkStatus(functionStatus: FunctionStatus): Option[StatusException] = { functionStatus.statusCode / 10 match { - case 1 => Right(statusWithData) - case 2 => Left(ServerMisconfigurationException(functionStatus)) - case 3 => Left(DataConflictException(functionStatus)) - case 4 => Left(DataNotFoundException(functionStatus)) - case 5 | 6 | 7 | 8 => Left(ErrorInDataException(functionStatus)) - case 9 => Left(OtherStatusException(functionStatus)) - case _ => Left(StatusOutOfRangeException(functionStatus)) + case 1 => None + case 2 => Some(ServerMisconfigurationException(functionStatus)) + case 3 => Some(DataConflictException(functionStatus)) + case 4 => Some(DataNotFoundException(functionStatus)) + case 5 | 6 | 7 | 8 => Some(ErrorInDataException(functionStatus)) + case 9 => Some(OtherStatusException(functionStatus)) + case _ => Some(StatusOutOfRangeException(functionStatus)) } } } diff --git a/core/src/main/scala/za/co/absa/db/fadb/status/handling/implementations/UserDefinedStatusHandling.scala b/core/src/main/scala/za/co/absa/db/fadb/status/handling/implementations/UserDefinedStatusHandling.scala index c1e81953..0a944ecf 100644 --- a/core/src/main/scala/za/co/absa/db/fadb/status/handling/implementations/UserDefinedStatusHandling.scala +++ b/core/src/main/scala/za/co/absa/db/fadb/status/handling/implementations/UserDefinedStatusHandling.scala @@ -16,9 +16,9 @@ package za.co.absa.db.fadb.status.handling.implementations -import za.co.absa.db.fadb.exceptions.OtherStatusException +import za.co.absa.db.fadb.exceptions.{OtherStatusException, StatusException} +import za.co.absa.db.fadb.status.FunctionStatus import za.co.absa.db.fadb.status.handling.StatusHandling -import za.co.absa.db.fadb.status.{FailedOrRow, Row} /** * Trait represents user defined status handling @@ -26,10 +26,10 @@ import za.co.absa.db.fadb.status.{FailedOrRow, Row} trait UserDefinedStatusHandling extends StatusHandling { def OKStatuses: Set[Integer] - override def checkStatus[D](statusWithData: Row[D]): FailedOrRow[D] = - if (OKStatuses.contains(statusWithData.functionStatus.statusCode)) { - Right(statusWithData) + override def checkStatus(functionStatus: FunctionStatus): Option[StatusException] = + if (OKStatuses.contains(functionStatus.statusCode)) { + None } else { - Left(OtherStatusException(statusWithData.functionStatus)) + Some(OtherStatusException(functionStatus)) } } diff --git a/core/src/test/scala/za/co/absa/db/fadb/status/handling/implementations/StandardStatusHandlingUnitTests.scala b/core/src/test/scala/za/co/absa/db/fadb/status/handling/implementations/StandardStatusHandlingUnitTests.scala index 32a5dda7..37eb69bf 100644 --- a/core/src/test/scala/za/co/absa/db/fadb/status/handling/implementations/StandardStatusHandlingUnitTests.scala +++ b/core/src/test/scala/za/co/absa/db/fadb/status/handling/implementations/StandardStatusHandlingUnitTests.scala @@ -25,73 +25,65 @@ class StandardStatusHandlingUnitTests extends AnyFunSuiteLike { private val standardQueryStatusHandling = new StandardStatusHandling {} - test("checkStatus should return Right when status code is in the range 10-19") { + test("checkStatus should return None when status code is in the range 10-19") { for (statusCode <- 10 to 19) { val functionStatus = FunctionStatus(statusCode, "Success") - val statusWithData = Row(functionStatus, "Data") - val result = standardQueryStatusHandling.checkStatus(statusWithData) - result shouldBe Right(statusWithData) + val result = standardQueryStatusHandling.checkStatus(functionStatus) + result shouldBe None } } - test("checkStatus should return Left with ServerMisconfigurationException when status code is in the range 20-29") { + test("checkStatus should return Some with ServerMisconfigurationException when status code is in the range 20-29") { for (statusCode <- 20 to 29) { val functionStatus = FunctionStatus(statusCode, "Server Misconfiguration") - val statusWithData = Row(functionStatus, "Data") - val result = standardQueryStatusHandling.checkStatus(statusWithData) - result shouldBe Left(ServerMisconfigurationException(functionStatus)) + val result = standardQueryStatusHandling.checkStatus(functionStatus) + result shouldBe Some(ServerMisconfigurationException(functionStatus)) } } - test("checkStatus should return Left with DataConflictException when status code is in the range 30-39") { + test("checkStatus should return Some with DataConflictException when status code is in the range 30-39") { for (statusCode <- 30 to 39) { val functionStatus = FunctionStatus(statusCode, "Data Conflict") - val statusWithData = Row(functionStatus, "Data") - val result = standardQueryStatusHandling.checkStatus(statusWithData) - result shouldBe Left(DataConflictException(functionStatus)) + val result = standardQueryStatusHandling.checkStatus(functionStatus) + result shouldBe Some(DataConflictException(functionStatus)) } } - test("checkStatus should return Left with DataNotFoundException when status code is in the range 40-49") { + test("checkStatus should return Some with DataNotFoundException when status code is in the range 40-49") { for (statusCode <- 40 to 49) { val functionStatus = FunctionStatus(statusCode, "Data Not Found") - val statusWithData = Row(functionStatus, "Data") - val result = standardQueryStatusHandling.checkStatus(statusWithData) - result shouldBe Left(DataNotFoundException(functionStatus)) + val result = standardQueryStatusHandling.checkStatus(functionStatus) + result shouldBe Some(DataNotFoundException(functionStatus)) } } - test("checkStatus should return Left with ErrorInDataException when status code is in the range 50-89") { + test("checkStatus should return Some with ErrorInDataException when status code is in the range 50-89") { for (statusCode <- 50 to 89) { val functionStatus = FunctionStatus(statusCode, "Error in Data") - val statusWithData = Row(functionStatus, "Data") - val result = standardQueryStatusHandling.checkStatus(statusWithData) - result shouldBe Left(ErrorInDataException(functionStatus)) + val result = standardQueryStatusHandling.checkStatus(functionStatus) + result shouldBe Some(ErrorInDataException(functionStatus)) } } - test("checkStatus should return Left with OtherStatusException when status code is in the range 90-99") { + test("checkStatus should return Some with OtherStatusException when status code is in the range 90-99") { for (statusCode <- 90 to 99) { val functionStatus = FunctionStatus(statusCode, "Other Status") - val statusWithData = Row(functionStatus, "Data") - val result = standardQueryStatusHandling.checkStatus(statusWithData) - result shouldBe Left(OtherStatusException(functionStatus)) + val result = standardQueryStatusHandling.checkStatus(functionStatus) + result shouldBe Some(OtherStatusException(functionStatus)) } } - test("checkStatus should return Left with StatusOutOfRangeException when status code is not in any known range") { + test("checkStatus should return Some with StatusOutOfRangeException when status code is not in any known range") { for (statusCode <- 0 to 9) { val functionStatus = FunctionStatus(statusCode, "Out of range") - val statusWithData = Row(functionStatus, "Data") - val result = standardQueryStatusHandling.checkStatus(statusWithData) - result shouldBe Left(StatusOutOfRangeException(functionStatus)) + val result = standardQueryStatusHandling.checkStatus(functionStatus) + result shouldBe Some(StatusOutOfRangeException(functionStatus)) } for (statusCode <- 100 to 110) { val functionStatus = FunctionStatus(statusCode, "Out of range") - val statusWithData = Row(functionStatus, "Data") - val result = standardQueryStatusHandling.checkStatus(statusWithData) - result shouldBe Left(StatusOutOfRangeException(functionStatus)) + val result = standardQueryStatusHandling.checkStatus(functionStatus) + result shouldBe Some(StatusOutOfRangeException(functionStatus)) } } diff --git a/doobie/src/main/scala/za/co/absa/db/fadb/doobie/DoobieEngine.scala b/doobie/src/main/scala/za/co/absa/db/fadb/doobie/DoobieEngine.scala index 1a14bfb5..90fc6d43 100644 --- a/doobie/src/main/scala/za/co/absa/db/fadb/doobie/DoobieEngine.scala +++ b/doobie/src/main/scala/za/co/absa/db/fadb/doobie/DoobieEngine.scala @@ -59,12 +59,12 @@ class DoobieEngine[F[_]: Async](val transactor: Transactor[F]) extends DBEngine[ * and Doobie's `Read` wasn't able to work with it. * * @param query the Doobie query to execute - * @param readStatusWithDataR the `Read[StatusWithData[R]]` instance used to read the query result into `StatusWithData[R]` + * @param readStatusWithData the `Read[StatusWithData[R]]` instance used to read the query result into `StatusWithData[R]` * @return the query result */ private def executeQueryWithStatus[R]( query: QueryWithStatusType[R] - )(implicit readStatusWithDataR: Read[StatusWithData[R]]): F[Seq[FailedOrRow[R]]] = { + )(implicit readStatusWithData: Read[StatusWithData[R]]): F[Seq[FailedOrRow[R]]] = { query.fragment.query[StatusWithData[R]].to[Seq].transact(transactor).map(_.map(query.getResultOrException)) } @@ -84,6 +84,6 @@ class DoobieEngine[F[_]: Async](val transactor: Transactor[F]) extends DBEngine[ * @return the query result */ override def runWithStatus[R](query: QueryWithStatusType[R]): F[Seq[FailedOrRow[R]]] = { - executeQueryWithStatus(query)(query.readStatusWithDataR) + executeQueryWithStatus(query)(query.readStatusWithData) } } diff --git a/doobie/src/main/scala/za/co/absa/db/fadb/doobie/DoobieFunction.scala b/doobie/src/main/scala/za/co/absa/db/fadb/doobie/DoobieFunction.scala index ca7fd5a9..fdf8d3bb 100644 --- a/doobie/src/main/scala/za/co/absa/db/fadb/doobie/DoobieFunction.scala +++ b/doobie/src/main/scala/za/co/absa/db/fadb/doobie/DoobieFunction.scala @@ -22,7 +22,8 @@ import doobie.util.Read import doobie.util.fragment.Fragment import za.co.absa.db.fadb.DBFunction._ import za.co.absa.db.fadb.DBSchema -import za.co.absa.db.fadb.status.{FailedOrRow, Row} +import za.co.absa.db.fadb.exceptions.StatusException +import za.co.absa.db.fadb.status.FunctionStatus import scala.language.higherKinds @@ -122,12 +123,7 @@ trait DoobieFunction[I, R, F[_]] extends DoobieFunctionBase[R] { trait DoobieFunctionWithStatus[I, R, F[_]] extends DoobieFunctionBase[R] { - /** - * The `Read[StatusWithData[R]]` instance used to read the query result with status into `StatusWithData[R]`. - */ - implicit def readStatusWithDataR(implicit readR: Read[R]): Read[StatusWithData[R]] = Read[(Int, String, R)].map { - case (status, status_text, data) => StatusWithData(status, status_text, data) - } + implicit val readStatusWithData: Read[StatusWithData[R]] /** * Function that generates a sequence of `Fragment`s representing the SQL query from input values for the function. @@ -141,12 +137,10 @@ trait DoobieFunctionWithStatus[I, R, F[_]] extends DoobieFunctionBase[R] { * @param selectEntry columns names for the select statement * @param functionName name of the function * @param alias alias for the sql query - * @param read Read instance for R type * @param me MonadError instance for F type * @return the `Fragment` representing the SQL query */ private def meSql(values: I, selectEntry: String, functionName: String, alias: String)(implicit - read: Read[StatusWithData[R]], me: MonadError[F, Throwable] ): F[Fragment] = { me.catchNonFatal { @@ -185,18 +179,17 @@ trait DoobieFunctionWithStatus[I, R, F[_]] extends DoobieFunctionBase[R] { /** * Generates a `Fragment` representing the SQL query for the function. * @param values the input values for the function - * @param read Read instance for `StatusWithData[R]` * @param me MonadError instance for F type * @return the `Fragment` representing the SQL query */ protected final def sql( values: I - )(implicit read: Read[StatusWithData[R]], me: MonadError[F, Throwable]): F[Fragment] = { - meSql(values, selectEntry, functionName, alias)(read, me) + )(me: MonadError[F, Throwable]): F[Fragment] = { + meSql(values, selectEntry, functionName, alias)(me) } // This is to be mixed in by an implementation of StatusHandling - def checkStatus[D](statusWithData: Row[D]): FailedOrRow[D] + def checkStatus(functionStatus: FunctionStatus): Option[StatusException] } /** @@ -233,7 +226,7 @@ object DoobieFunction { * @param schema the database schema * @param dbEngine the `DoobieEngine` instance used to execute SQL queries * @param readR Read instance for `R` - * @param readSelectWithStatus Read instance for `StatusWithData[R]` + * @param readStatusWithData Read instance for `StatusWithData[R]` * @tparam F the effect type, which must have an `Async` and a `Monad` instance */ abstract class DoobieSingleResultFunctionWithStatus[I, R, F[_]]( @@ -243,7 +236,7 @@ object DoobieFunction { override val schema: DBSchema, val dbEngine: DoobieEngine[F], val readR: Read[R], - val readSelectWithStatus: Read[StatusWithData[R]] + val readStatusWithData: Read[StatusWithData[R]] ) extends DBSingleResultFunctionWithStatus[I, R, DoobieEngine[F], F](functionNameOverride) with DoobieFunctionWithStatus[I, R, F] @@ -269,7 +262,8 @@ object DoobieFunction { )(implicit override val schema: DBSchema, val dbEngine: DoobieEngine[F], - val readR: Read[R] + val readR: Read[R], + val readStatusWithData: Read[StatusWithData[R]] ) extends DBMultipleResultFunctionWithStatus[I, R, DoobieEngine[F], F](functionNameOverride) with DoobieFunctionWithStatus[I, R, F] @@ -286,7 +280,8 @@ object DoobieFunction { )(implicit override val schema: DBSchema, val dbEngine: DoobieEngine[F], - val readR: Read[R] + val readR: Read[R], + val readStatusWithData: Read[StatusWithData[R]] ) extends DBMultipleResultFunctionWithAggStatus[I, R, DoobieEngine[F], F](functionNameOverride) with DoobieFunctionWithStatus[I, R, F] @@ -312,7 +307,8 @@ object DoobieFunction { )(implicit override val schema: DBSchema, val dbEngine: DoobieEngine[F], - val readR: Read[R] + val readR: Read[R], + val readStatusWithData: Read[StatusWithData[R]] ) extends DBOptionalResultFunctionWithStatus[I, R, DoobieEngine[F], F](functionNameOverride) with DoobieFunctionWithStatus[I, R, F] } diff --git a/doobie/src/main/scala/za/co/absa/db/fadb/doobie/DoobieQuery.scala b/doobie/src/main/scala/za/co/absa/db/fadb/doobie/DoobieQuery.scala index 66d50eb2..0f11c6c6 100644 --- a/doobie/src/main/scala/za/co/absa/db/fadb/doobie/DoobieQuery.scala +++ b/doobie/src/main/scala/za/co/absa/db/fadb/doobie/DoobieQuery.scala @@ -18,6 +18,7 @@ package za.co.absa.db.fadb.doobie import doobie.util.Read import doobie.util.fragment.Fragment +import za.co.absa.db.fadb.exceptions.StatusException import za.co.absa.db.fadb.status.{FailedOrRow, FunctionStatus, Row} import za.co.absa.db.fadb.{Query, QueryWithStatus} @@ -36,20 +37,20 @@ class DoobieQuery[R](val fragment: Fragment)(implicit val readR: Read[R]) extend * * @param fragment the Doobie fragment representing the SQL query * @param checkStatus the function to check the status of the query - * @param readStatusWithDataR the `Read[StatusWithData[R]]` instance used to read the query result into `StatusWithData[R]` + * @param readStatusWithData the `Read[StatusWithData[R]]` instance used to read the query result into `StatusWithData[R]` */ class DoobieQueryWithStatus[R]( val fragment: Fragment, - checkStatus: Row[R] => FailedOrRow[R] -)(implicit val readStatusWithDataR: Read[StatusWithData[R]]) - extends QueryWithStatus[StatusWithData[R], R, R] { + checkStatus: FunctionStatus => Option[StatusException] +)(implicit val readStatusWithData: Read[StatusWithData[R]]) + extends QueryWithStatus[StatusWithData[R], Option[R], R] { /* * Processes the status of the query and returns the status with data * @param initialResult - the initial result of the query * @return data with status */ - override def processStatus(initialResult: StatusWithData[R]): Row[R] = + override def processStatus(initialResult: StatusWithData[R]): Row[Option[R]] = Row(FunctionStatus(initialResult.status, initialResult.statusText), initialResult.data) /* @@ -57,6 +58,9 @@ class DoobieQueryWithStatus[R]( * @param statusWithData - the status with data * @return either a status exception or the data */ - override def toStatusExceptionOrData(statusWithData: Row[R]): FailedOrRow[R] = - checkStatus(statusWithData) + override def toStatusExceptionOrData(statusWithData: Row[Option[R]]): FailedOrRow[R] = + checkStatus(statusWithData.functionStatus).toLeft( + statusWithData.data.getOrElse(throw new IllegalStateException("Status is OK but data is missing")) + ).map(r => Row(statusWithData.functionStatus, r)) + } diff --git a/doobie/src/main/scala/za/co/absa/db/fadb/doobie/StatusWithData.scala b/doobie/src/main/scala/za/co/absa/db/fadb/doobie/StatusWithData.scala index 5da6b4fa..4d6178e1 100644 --- a/doobie/src/main/scala/za/co/absa/db/fadb/doobie/StatusWithData.scala +++ b/doobie/src/main/scala/za/co/absa/db/fadb/doobie/StatusWithData.scala @@ -16,12 +16,30 @@ package za.co.absa.db.fadb.doobie +import doobie.Read + /** - * Represents a function status with data (basically a row returned from a DB). - * - * Note: data here represent one row that has status-related fields omitted. In some scenarios, it actually might - * be missing - i.e. query returns only status information with no data - pretty common scenario when a DB - * function returns error, e.g.: (41, 'NoDataFound', NULL). In this case, R must be an Option - to be specified - * by a user. + * Represents a function status with data (basically a row returned from a DB). */ -case class StatusWithData[R](status: Int, statusText: String, data: R) +case class StatusWithData[R](status: Int, statusText: String, data: Option[R]) + +object StatusWithData { + + implicit def read[T](implicit r: Read[Option[T]]): Read[StatusWithData[T]] = + Read[(Int, String, Option[T])].map { + case (status, statusText, data) => StatusWithData(status, statusText, data) + } + + // This is for backward compatibility of #133 (which makes the data optional by default when reading from DB), + // as some users were overcoming the bug by wrapping the result type in Option themselves. + // Doobie provides Read for Option[T] for most of the types out of the box, except when T itself is an Option. + implicit def readOptionToReadDoubleOption[T]( + implicit r: Read[StatusWithData[T]] + ): Read[StatusWithData[Option[T]]] = { + r.map { + case statusWithData @ StatusWithData(_, _, Some(data)) => statusWithData.copy(data = Some(Some(data))) + case statusWithData @ StatusWithData(_, _, None) => statusWithData.copy(data = None) + } + } + +} diff --git a/doobie/src/test/scala/za/co/absa/db/fadb/doobie/DoobieMultipleResultFunctionWithAggStatusIntegrationTests.scala b/doobie/src/test/scala/za/co/absa/db/fadb/doobie/DoobieMultipleResultFunctionWithAggStatusIntegrationTests.scala index 809115c2..b7b56d09 100644 --- a/doobie/src/test/scala/za/co/absa/db/fadb/doobie/DoobieMultipleResultFunctionWithAggStatusIntegrationTests.scala +++ b/doobie/src/test/scala/za/co/absa/db/fadb/doobie/DoobieMultipleResultFunctionWithAggStatusIntegrationTests.scala @@ -23,6 +23,7 @@ import doobie.implicits.toSqlInterpolator import org.scalatest.funsuite.AnyFunSuite import za.co.absa.db.fadb.DBSchema import za.co.absa.db.fadb.doobie.DoobieFunction.DoobieMultipleResultFunctionWithAggStatus +import za.co.absa.db.fadb.exceptions.DataNotFoundException import za.co.absa.db.fadb.status.aggregation.implementations.ByMajorityErrorsStatusAggregator import za.co.absa.db.fadb.status.handling.implementations.StandardStatusHandling import za.co.absa.db.fadb.status.{FunctionStatus, Row} @@ -35,19 +36,29 @@ class DoobieMultipleResultFunctionWithAggStatusIntegrationTests extends AnyFunSu } class GetActorsByLastname(implicit schema: DBSchema, dbEngine: DoobieEngine[IO]) - // Option[Actor] because: Actor might not exist, and the function would return only status info without actor data - extends DoobieMultipleResultFunctionWithAggStatus[GetActorsByLastnameQueryParameters, Option[Actor], IO](getActorsByLastnameQueryFragments) + extends DoobieMultipleResultFunctionWithAggStatus[GetActorsByLastnameQueryParameters, Actor, IO](getActorsByLastnameQueryFragments) + with StandardStatusHandling + with ByMajorityErrorsStatusAggregator { + override def fieldsToSelect: Seq[String] = super.fieldsToSelect ++ Seq("actor_id", "first_name", "last_name") + } + + class GetActorsByLastnameOption(implicit schema: DBSchema, dbEngine: DoobieEngine[IO]) + extends DoobieMultipleResultFunctionWithAggStatus[GetActorsByLastnameQueryParameters, Option[Actor], IO]( + getActorsByLastnameQueryFragments, + Some("get_actors_by_lastname") + ) with StandardStatusHandling with ByMajorityErrorsStatusAggregator { override def fieldsToSelect: Seq[String] = super.fieldsToSelect ++ Seq("actor_id", "first_name", "last_name") } private val getActorsByLastname = new GetActorsByLastname()(Integration, new DoobieEngine(transactor)) + private val getActorsByLastnameOption = new GetActorsByLastnameOption()(Integration, new DoobieEngine(transactor)) test("Retrieving multiple actors from database, lastName match") { val expectedResultElem = Set( - Row(FunctionStatus(11, "OK, match on last name only"), Some(Actor(51, "Fred", "Weasley"))), - Row(FunctionStatus(11, "OK, match on last name only"), Some(Actor(52, "George", "Weasley"))), + Row(FunctionStatus(11, "OK, match on last name only"), Actor(51, "Fred", "Weasley")), + Row(FunctionStatus(11, "OK, match on last name only"), Actor(52, "George", "Weasley")), ) val results = getActorsByLastname(GetActorsByLastnameQueryParameters("Weasley")).unsafeRunSync() @@ -61,7 +72,7 @@ class DoobieMultipleResultFunctionWithAggStatusIntegrationTests extends AnyFunSu test("Retrieving single actor from database, full match") { val expectedResultElem = Row( - FunctionStatus(12, "OK, full match"), Some(Actor(50, "Liza", "Simpson")) + FunctionStatus(12, "OK, full match"), Actor(50, "Liza", "Simpson") ) val results = getActorsByLastname(GetActorsByLastnameQueryParameters("Simpson", Some("Liza"))).unsafeRunSync() @@ -76,7 +87,7 @@ class DoobieMultipleResultFunctionWithAggStatusIntegrationTests extends AnyFunSu test("Retrieving single actor from database, lastname match") { val expectedResultElem = Row( - FunctionStatus(11, "OK, match on last name only"), Some(Actor(50, "Liza", "Simpson")) + FunctionStatus(11, "OK, match on last name only"), Actor(50, "Liza", "Simpson") ) val results = getActorsByLastname(GetActorsByLastnameQueryParameters("Simpson")).unsafeRunSync() @@ -98,4 +109,20 @@ class DoobieMultipleResultFunctionWithAggStatusIntegrationTests extends AnyFunSu case Right(_) => fail("should not be right") } } + + test("backwards compatibility with already Optioned result #133)") { + val foundExpectedResult = Set( + Row(FunctionStatus(11, "OK, match on last name only"), Some(Actor(51, "Fred", "Weasley"))), + Row(FunctionStatus(11, "OK, match on last name only"), Some(Actor(52, "George", "Weasley"))) + ) + val foundActualResult = + getActorsByLastnameOption(GetActorsByLastnameQueryParameters("Weasley")).unsafeRunSync().map(_.toSet) + assert(foundActualResult.contains(foundExpectedResult)) + + val notFoundExpectedResult = Left(DataNotFoundException(FunctionStatus(41, "No actor found"))) + val notFoundActualResult = + getActorsByLastname(GetActorsByLastnameQueryParameters("TotallyNonExisting!")).unsafeRunSync() + assert(notFoundActualResult == notFoundExpectedResult) + } + } diff --git a/doobie/src/test/scala/za/co/absa/db/fadb/doobie/DoobieMultipleResultFunctionWithStatusIntegrationTests.scala b/doobie/src/test/scala/za/co/absa/db/fadb/doobie/DoobieMultipleResultFunctionWithStatusIntegrationTests.scala index f02046ae..c12cde87 100644 --- a/doobie/src/test/scala/za/co/absa/db/fadb/doobie/DoobieMultipleResultFunctionWithStatusIntegrationTests.scala +++ b/doobie/src/test/scala/za/co/absa/db/fadb/doobie/DoobieMultipleResultFunctionWithStatusIntegrationTests.scala @@ -35,20 +35,29 @@ class DoobieMultipleResultFunctionWithStatusIntegrationTests extends AnyFunSuite } class GetActorsByLastname(implicit schema: DBSchema, dbEngine: DoobieEngine[IO]) - // Option[Actor] because: Actor might not exist, and the function would return only status info without actor data - extends DoobieMultipleResultFunctionWithStatus[GetActorsByLastnameQueryParameters, Option[Actor], IO]( + extends DoobieMultipleResultFunctionWithStatus[GetActorsByLastnameQueryParameters, Actor, IO]( getActorsByLastnameQueryFragments ) with StandardStatusHandling { override def fieldsToSelect: Seq[String] = super.fieldsToSelect ++ Seq("actor_id", "first_name", "last_name") } + class GetActorsByLastnameOption(implicit schema: DBSchema, dbEngine: DoobieEngine[IO]) + extends DoobieMultipleResultFunctionWithStatus[GetActorsByLastnameQueryParameters, Option[Actor], IO]( + getActorsByLastnameQueryFragments, + Some("get_actors_by_lastname") + ) + with StandardStatusHandling { + override def fieldsToSelect: Seq[String] = super.fieldsToSelect ++ Seq("actor_id", "first_name", "last_name") + } + private val getActorsByLastname = new GetActorsByLastname()(Integration, new DoobieEngine(transactor)) + private val getActorsByLastnameOption = new GetActorsByLastnameOption()(Integration, new DoobieEngine(transactor)) test("Retrieving multiple actors from database, lastName match") { val expectedResultElem = Set( - Right(Row(FunctionStatus(11, "OK, match on last name only"), Some(Actor(51, "Fred", "Weasley")))), - Right(Row(FunctionStatus(11, "OK, match on last name only"), Some(Actor(52, "George", "Weasley")))) + Right(Row(FunctionStatus(11, "OK, match on last name only"), Actor(51, "Fred", "Weasley"))), + Right(Row(FunctionStatus(11, "OK, match on last name only"), Actor(52, "George", "Weasley"))) ) val results = getActorsByLastname(GetActorsByLastnameQueryParameters("Weasley")).unsafeRunSync() @@ -57,7 +66,7 @@ class DoobieMultipleResultFunctionWithStatusIntegrationTests extends AnyFunSuite test("Retrieving single actor from database, full match") { val expectedResultElem = Set( - Right(Row(FunctionStatus(12, "OK, full match"), Some(Actor(50, "Liza", "Simpson")))) + Right(Row(FunctionStatus(12, "OK, full match"), Actor(50, "Liza", "Simpson"))) ) val results = getActorsByLastname(GetActorsByLastnameQueryParameters("Simpson", Some("Liza"))).unsafeRunSync() @@ -66,7 +75,7 @@ class DoobieMultipleResultFunctionWithStatusIntegrationTests extends AnyFunSuite test("Retrieving single actor from database, lastname match") { val expectedResultElem = Set( - Right(Row(FunctionStatus(11, "OK, match on last name only"), Some(Actor(50, "Liza", "Simpson")))) + Right(Row(FunctionStatus(11, "OK, match on last name only"), Actor(50, "Liza", "Simpson"))) ) val results = getActorsByLastname(GetActorsByLastnameQueryParameters("Simpson")).unsafeRunSync() @@ -81,4 +90,19 @@ class DoobieMultipleResultFunctionWithStatusIntegrationTests extends AnyFunSuite assert(results.head.isLeft) assert(results.head == expectedErr) } + + test("backwards compatibility with already Optioned result #133)") { + val foundExpectedResult = Set( + Right(Row(FunctionStatus(11, "OK, match on last name only"), Some(Actor(51, "Fred", "Weasley")))), + Right(Row(FunctionStatus(11, "OK, match on last name only"), Some(Actor(52, "George", "Weasley")))) + ) + val foundActualResult = getActorsByLastnameOption(GetActorsByLastnameQueryParameters("Weasley")).unsafeRunSync().toSet + assert(foundActualResult == foundExpectedResult) + + val notFoundExpectedResult = Seq(Left(DataNotFoundException(FunctionStatus(41, "No actor found")))) + val notFoundActualResult = + getActorsByLastname(GetActorsByLastnameQueryParameters("TotallyNonExisting!")).unsafeRunSync() + assert(notFoundActualResult == notFoundExpectedResult) + } + } diff --git a/doobie/src/test/scala/za/co/absa/db/fadb/doobie/DoobieOtherTypesIntegrationTests.scala b/doobie/src/test/scala/za/co/absa/db/fadb/doobie/DoobieOtherTypesIntegrationTests.scala index 2d8c0bcb..f7cbba08 100644 --- a/doobie/src/test/scala/za/co/absa/db/fadb/doobie/DoobieOtherTypesIntegrationTests.scala +++ b/doobie/src/test/scala/za/co/absa/db/fadb/doobie/DoobieOtherTypesIntegrationTests.scala @@ -52,7 +52,7 @@ class DoobieOtherTypesIntegrationTests extends AnyFunSuite with DoobieTest { extends DoobieSingleResultFunction[Int, OtherTypesData, IO] (values => Seq(fr"$values")) class InsertOtherTypes(implicit schema: DBSchema, dbEngine: DoobieEngine[IO]) - extends DoobieSingleResultFunctionWithStatus[OtherTypesData, Option[Int], IO] ( + extends DoobieSingleResultFunctionWithStatus[OtherTypesData, Int, IO] ( values => Seq( fr"${values.id}", fr"${values.ltreeCol}::ltree", diff --git a/doobie/src/test/scala/za/co/absa/db/fadb/doobie/DoobieSingleResultFunctionWithStatusIntegrationTests.scala b/doobie/src/test/scala/za/co/absa/db/fadb/doobie/DoobieSingleResultFunctionWithStatusIntegrationTests.scala index 41e381f7..31906272 100644 --- a/doobie/src/test/scala/za/co/absa/db/fadb/doobie/DoobieSingleResultFunctionWithStatusIntegrationTests.scala +++ b/doobie/src/test/scala/za/co/absa/db/fadb/doobie/DoobieSingleResultFunctionWithStatusIntegrationTests.scala @@ -46,15 +46,18 @@ class DoobieSingleResultFunctionWithStatusIntegrationTests extends AnyFunSuite w override def fieldsToSelect: Seq[String] = super.fieldsToSelect ++ Seq("input_value") } - class ErrorIfNotOneWithStatus(functionNameOverride: String)(implicit schema: DBSchema, dbEngine: DoobieEngine[IO]) - extends DoobieSingleResultFunctionWithStatus[Int, Option[Int], IO]( - params => Seq(fr"$params"), Some(functionNameOverride) - ) with StandardStatusHandling { + class ErrorIfNotOneOption(implicit schema: DBSchema, dbEngine: DoobieEngine[IO]) + extends DoobieSingleResultFunctionWithStatus[Int, Option[Int], IO] ( + params => Seq(fr"$params"), + Some("error_if_not_one") + ) + with StandardStatusHandling { override def fieldsToSelect: Seq[String] = super.fieldsToSelect ++ Seq("input_value") } private val createActor = new CreateActor()(Integration, new DoobieEngine(transactor)) private val errorIfNotOne = new ErrorIfNotOne()(Integration, new DoobieEngine(transactor)) + private val errorIfNotOneOption = new ErrorIfNotOneOption()(Integration, new DoobieEngine(transactor)) test("Creating actor within a function with status handling") { val requestBody = CreateActorRequestBody("Pavel", "Marek") @@ -62,30 +65,25 @@ class DoobieSingleResultFunctionWithStatusIntegrationTests extends AnyFunSuite w assert(result == Right(125)) } - test("Successful function call with status handling") { - val result = errorIfNotOne(1).unsafeRunSync() - assert(result.isRight) - } - - test("Unsuccessful function call with status handling. Asserting on error when Int not wrapped in Option") { - // SQL `NULL` read at column 3 (JDBC type Integer) but mapping is to a non-Option type; use Option here. Note that JDBC column indexing is 1-based. - assertThrows[NonNullableColumnRead](errorIfNotOne(2).unsafeRunSync()) - } - - test("Unsuccessful function call with status handling. Asserting on error when Int wrapped in Option") { - val errorIfNotOne = new ErrorIfNotOneWithStatus("error_if_not_one")(Integration, new DoobieEngine(transactor)) - + test("Unsuccessful function call with status handling") { val result = errorIfNotOne(2).unsafeRunSync() assert(result.isLeft) } - test("Successful function call with status handling. Asserting on success when Int wrapped in Option") { - val errorIfNotOne = new ErrorIfNotOneWithStatus("error_if_not_one")(Integration, new DoobieEngine(transactor)) - + test("Successful function call with status handling") { val result = errorIfNotOne(1).unsafeRunSync() result match { case Left(_) => fail("should not be left") - case Right(value) => assert(value.data.contains(1)) + case Right(value) => assert(value.data === 1) } } + + test("backwards compatibility with already Optioned result #133)") { + assert(errorIfNotOneOption(2).unsafeRunSync().isLeft) + errorIfNotOneOption(1).unsafeRunSync() match { + case Left(_) => fail("should not be left") + case Right(value) => assert(value.data === Some(1)) + } + } + } diff --git a/slick/src/main/scala/za/co/absa/db/fadb/slick/SlickFunction.scala b/slick/src/main/scala/za/co/absa/db/fadb/slick/SlickFunction.scala index 5093c2fc..8bb35c65 100644 --- a/slick/src/main/scala/za/co/absa/db/fadb/slick/SlickFunction.scala +++ b/slick/src/main/scala/za/co/absa/db/fadb/slick/SlickFunction.scala @@ -20,7 +20,8 @@ import cats.MonadError import slick.jdbc.{GetResult, SQLActionBuilder} import za.co.absa.db.fadb.DBFunction._ import za.co.absa.db.fadb.DBSchema -import za.co.absa.db.fadb.status.{FailedOrRow, Row} +import za.co.absa.db.fadb.exceptions.StatusException +import za.co.absa.db.fadb.status.{FailedOrRow, FunctionStatus, Row} import scala.concurrent.Future import scala.language.higherKinds @@ -89,7 +90,7 @@ private[slick] trait SlickFunctionWithStatus[I, R] extends SlickFunctionBase[I, } // Expected to be mixed in by an implementation of StatusHandling - def checkStatus[D](statusWithData: Row[D]): FailedOrRow[D] + def checkStatus(functionStatus: FunctionStatus): Option[StatusException] } object SlickFunction { diff --git a/slick/src/main/scala/za/co/absa/db/fadb/slick/SlickQuery.scala b/slick/src/main/scala/za/co/absa/db/fadb/slick/SlickQuery.scala index 2bffc5f4..63262e77 100644 --- a/slick/src/main/scala/za/co/absa/db/fadb/slick/SlickQuery.scala +++ b/slick/src/main/scala/za/co/absa/db/fadb/slick/SlickQuery.scala @@ -17,6 +17,7 @@ package za.co.absa.db.fadb.slick import slick.jdbc.{GetResult, PositionedResult, SQLActionBuilder} +import za.co.absa.db.fadb.exceptions.StatusException import za.co.absa.db.fadb.status.{FailedOrRow, FunctionStatus, Row} import za.co.absa.db.fadb.{Query, QueryWithStatus} @@ -39,7 +40,7 @@ class SlickQuery[R](val sql: SQLActionBuilder, val getResult: GetResult[R]) exte class SlickQueryWithStatus[R]( val sql: SQLActionBuilder, val getResult: GetResult[R], - checkStatus: Row[PositionedResult] => FailedOrRow[PositionedResult] + checkStatus: FunctionStatus => Option[StatusException] ) extends QueryWithStatus[PositionedResult, PositionedResult, R] { /** @@ -61,13 +62,9 @@ class SlickQueryWithStatus[R]( override def toStatusExceptionOrData( statusWithData: Row[PositionedResult] ): FailedOrRow[R] = { - checkStatus(statusWithData) match { - case Left(statusException) => Left(statusException) - case Right(value) => - val status = value.functionStatus - val data = getResult(value.data) - Right(Row(status, data)) - } + checkStatus(statusWithData.functionStatus).toLeft( + Row(statusWithData.functionStatus, getResult(statusWithData.data)) + ) } /**