Introduce MonadError & refactor Doobie classes

[salamonpavel]

In the provided code, MonadError is used to catch exceptions that might be thrown when constructing the SQL query. If an exception is thrown, it is caught and represented as a failure in the F context. MonadError is a type class in Scala that abstracts over data types which define a flatMap operation and can signal errors. This includes types like Option, Either, Try, Future, IO, etc.

Apart from introduction of MonadError into both Slick and Doobie module, the Doobie functions have undergone a further significant refactoring process. Users have to provide function value from input to sequence of Doobie Fragments during the construction phase, thereby ensuring that no code can escape from the computational context F. This not only enhances the safety and predictability of the code but also eliminates a substantial amount of boilerplate. The ability to concatenate Doobie Fragments has facilitated this transformation, enabling flexible and dynamic construction of SQL queries.

However, this refactoring strategy could not be directly applied to the Slick module due to the lack of support for concatenation in SQLActionBuilder. This limitation necessitates the implementation of the sql method within the class body when using Slick.

An example of code throwing exception that's not captured in the computational context. Notice that the Fragment is constructed outside of computational IO context.

  class GetAllDateTimeTypes(implicit schema: DBSchema, dbEngine: DoobieEngine[IO])
      extends DoobieSingleResultFunction[Int, DatesTimes, IO] {

    override def sql(values: Int)(implicit read: Read[DatesTimes]): Fragment =
      throw new Exception("boom!")
      sql"SELECT * FROM ${Fragment.const(functionName)}($values)"
  }

An example where the error is captured within its context. Notice the simplification and reduction of boilerplate code and the fact that function from input type to Seq[Fragment] is needed to be passed to the constructor.

class GetAllDateTimeTypes(implicit schema: DBSchema, dbEngine: DoobieEngine[IO])
      extends DoobieSingleResultFunction[Int, DatesTimes, IO](values => Seq(fr"$values"))

The function passed to the constructor is of this signature.

def toFragmentsSeq: I => Seq[Fragment]

The sql method is already implemented in terms of meSql and made final. toFragmentsSeq is evaluated within the MonadError context. Exceptions can't escape.

protected final def sql(values: I)(implicit read: Read[R], ME: MonadError[F, Throwable]): F[Fragment] = {
    meSql(values, selectEntry, functionName, alias)(read, ME)
  }

private def meSql(values: I, selectEntry: String, functionName: String, alias: String)(implicit
    read: Read[R],
    ME: MonadError[F, Throwable]
  ): F[Fragment] = {
    ME.catchNonFatal {
      val fragments = toFragmentsSeq(values)
      val args = fragments.toList match {
        case head :: tail => tail.foldLeft(head)((acc, frag) => acc ++ fr"," ++ frag)
        case Nil          => fr""
      }
      sql"SELECT ${Fragment.const(selectEntry)} FROM ${Fragment.const(functionName)}($args) ${Fragment.const(alias)};"
    }
  }

In Slick module the similar is accomplished in this way. User still needs to implement sql method in the classes body. meSql then wraps this code in the computational context. Since the sql method is passed by-name, potential exceptions won't escape. From user point of view the Slick api hasn't changed.

protected def sql(values: I): SQLActionBuilder

  protected final def meSql(
    sqlActionBuilder: => SQLActionBuilder
  )(implicit ME: MonadError[Future, Throwable]): Future[SQLActionBuilder] = {
    ME.catchNonFatal(sqlActionBuilder)
  }

Now back to Doobie module....
Given that the arguments to the sql function are passed as a function this opens door for functional composition and reusability.

  implicit def toFragmentsFunctionSemigroup[T]: Semigroup[T => Seq[Fragment]] = {
    (f1: T => Seq[Fragment], f2: T => Seq[Fragment]) => (params: T) => f1(params) ++ f2(params)
  }

  private val firstNameFragment: GetActorsQueryParameters => Seq[Fragment] = params => Seq(fr"${params.firstName}")
  private val lastNameFragment: GetActorsQueryParameters => Seq[Fragment] = params => Seq(fr"${params.lastName}")

  // concatenation of function results after their evaluation
  private val combinedQueryFragments: GetActorsQueryParameters => Seq[Fragment] =
    params => firstNameFragment(params) ++ lastNameFragment(params)

  // using Semigroup's combine method, |+| is syntactical sugar for combine method
  private val combinedUsingSemigroup = firstNameFragment |+| lastNameFragment

  // not combined, defined as one function
  private val getActorsQueryFragments: GetActorsQueryParameters => Seq[Fragment] = {
    values => Seq(fr"${values.firstName}", fr"${values.lastName}")
  }

  // and notice how the boilerplate code was eliminated
  class GetActors(implicit schema: DBSchema, dbEngine: DoobieEngine[IO])
      extends DoobieMultipleResultFunction[GetActorsQueryParameters, Actor, IO](combinedUsingSemigroup)

Fixes #114

[github-actions]

JaCoCo code coverage report - scala 2.12.17

Total Project Coverage	27.05%	🍏

Module	Coverage
fa-db:core Jacoco Report - scala:2.12.17	60.28%	❌
fa-db:slick Jacoco Report - scala:2.12.17	0%	❌
fa-db:doobie Jacoco Report - scala:2.12.17	0%	❌

Files

Module	File	Coverage [1.67%]
fa-db:core Jacoco Report - scala:2.12.17	DBFunction.scala	6.25%	❌
fa-db:slick Jacoco Report - scala:2.12.17	SlickFunction.scala	0%	❌
	SlickQuery.scala	0%	❌
fa-db:doobie Jacoco Report - scala:2.12.17	DoobieFunction.scala	0%	❌

[github-actions]

JaCoCo code coverage report - scala 2.13.12

Total Project Coverage	27.94%	🍏

Module	Coverage
fa-db:core Jacoco Report - scala:2.13.12	61.8%	❌
fa-db:slick Jacoco Report - scala:2.13.12	0%	❌
fa-db:doobie Jacoco Report - scala:2.13.12	0%	❌

Files

Module	File	Coverage [1.7%]
fa-db:core Jacoco Report - scala:2.13.12	DBFunction.scala	6.48%	❌
fa-db:slick Jacoco Report - scala:2.13.12	SlickFunction.scala	0%	❌
	SlickQuery.scala	0%	❌
fa-db:doobie Jacoco Report - scala:2.13.12	DoobieFunction.scala	0%	❌

[jakipatryk]

Just quickly went through the code. Overall the idea LGTM, especially the Doobie part. However, I didn't do a deep review of the internal implementation (I can't now).

[jakipatryk]

Why ME not me? I saw some other place with implicit lowercase, so probably makes sense to be me instead.
(same in other methods)

[salamonpavel]

ok, will change everywhere to lower case

[salamonpavel]

done

[jakipatryk]

What is this for?

[salamonpavel]

Just a left over code, will remove it.

[salamonpavel]

done

[salamonpavel]

@lsulak @jakipatryk
Thanks for the initial review. I have addressed your comments. Additionally I have noticed there was a code duplication in fragments composition - I have extracted the duplicated code into composeFragments method in DoobieFunctionBase. Please let me know if you see any other issues. If not please add your approvals so we can start building on top of this code.

[miroslavpojer]

• pulled
• test run - Failed
• local jacoco run - Failed by test run && 0% code coverage of new module 'doobie' detected
• GH jacoco post - Failed the posts do not provide all available information

Test run
command: sbt test failing with output
[error] /Users/ab024ll/repos/absa/branches/fa-db-113/slick/src/main/scala/za/co/absa/fadb/slick/support/PgUUIDSupport.scala:32:5: Symbol 'type com.typesafe.config.Config' is missing from the classpath. [error] This symbol is required by 'method slick.basic.BasicProfile.loadProfileConfig'. [error] Make sure that type Config is in your classpath and check for conflicting dependencies with-Ylog-classpath. [error] A full rebuild may help if 'BasicProfile.class' was compiled against an incompatible version of com.typesafe.config. [error] driver match { [error] ^ [error] one error found

GH jacoco post
In file build.yml there is missing , in line 96.

[salamonpavel]

• pulled

  • test run - Failed

  • local jacoco run - Failed by test run && 0% code coverage of new module 'doobie' detected

  • GH jacoco post - Failed the posts do not provide all available information

Test run command: sbt test failing with output [error] /Users/ab024ll/repos/absa/branches/fa-db-113/slick/src/main/scala/za/co/absa/fadb/slick/support/PgUUIDSupport.scala:32:5: Symbol 'type com.typesafe.config.Config' is missing from the classpath. [error] This symbol is required by 'method slick.basic.BasicProfile.loadProfileConfig'. [error] Make sure that type Config is in your classpath and check for conflicting dependencies with-Ylog-classpath. [error] A full rebuild may help if 'BasicProfile.class' was compiled against an incompatible version of com.typesafe.config. [error] driver match { [error] ^ [error] one error found

GH jacoco post In file build.yml there is missing , in line 96.

added the missing comma
sbt test command runs successfully, there must be a problem on your side

[miroslavpojer]

• pulled

  • test run - Failed
  • local jacoco run - Failed by test run && 0% code coverage of new module 'doobie' detected
  • GH jacoco post - Failed the posts do not provide all available information

Test run command: sbt test failing with output [error] /Users/ab024ll/repos/absa/branches/fa-db-113/slick/src/main/scala/za/co/absa/fadb/slick/support/PgUUIDSupport.scala:32:5: Symbol 'type com.typesafe.config.Config' is missing from the classpath. [error] This symbol is required by 'method slick.basic.BasicProfile.loadProfileConfig'. [error] Make sure that type Config is in your classpath and check for conflicting dependencies with-Ylog-classpath. [error] A full rebuild may help if 'BasicProfile.class' was compiled against an incompatible version of com.typesafe.config. [error] driver match { [error] ^ [error] one error found
GH jacoco post In file build.yml there is missing , in line 96.

added the missing comma sbt test command runs successfully, there must be a problem on your side

• pulled
• test run - Fixed - Passing
• local jacoco run - Fixed - still 0% coverage of Doobie module
• GH jacoco post - Fixed - now it provide useful output information

[miroslavpojer]

@salamonpavel Create please doobie/README.md file and describe the steps needed to run it tests for doobie module.
I would like to keep this knowledge here for later INT env works.
Thanks

[salamonpavel]

@salamonpavel Create please doobie/README.md file and describe the steps needed to run it tests for doobie module. I would like to keep this knowledge here for later INT env works. Thanks

@miroslavpojer
For execution of integration tests in the doobie module (in fact for any module) you need to manually deploy all sql scripts that these tests need to your local database instance and configure a connection to it.
For the doobie module, the sql scripts can be found in doobie/src/it/database and the connection is hardcoded in DoobieTest trait.

[miroslavpojer]

@salamonpavel Create please doobie/README.md file and describe the steps needed to run it tests for doobie module. I would like to keep this knowledge here for later INT env works. Thanks

@miroslavpojer For execution of integration tests in the doobie module (in fact for any module) you need to manually deploy all sql scripts that these tests need to your local database instance and configure a connection to it. For the doobie module, the sql scripts can be found in doobie/src/it/database and the connection is hardcoded in DoobieTest trait.

Created Issue - #116 to finish found problem with manual Integration test run - short doc & missing test data expected by tests.

[lsulak]

is this actually used?

[salamonpavel]

good catch

[lsulak]

I like the formatting of these.

Btw, some code docs have very long lines, can you improve that?

[salamonpavel]

Ok, shortened the docs.

[lsulak]

I finished with the review, very interesting - the idea behind all these fragments and the way it's used