http4s server v0.18 : replace cats.effect.IO with generic F

[amarrella]

I think it would be nice remove the references to cats.effect.IOin the code and replace it with a generic F[_]: Effect.

This would help in writing finally tagless services without committing to IO too early.

Making the change in the current http4s_0.18 generator might break some code(you might need to either specify the type of service() or alternatively replace the current traits with abstract classes (if we want to specify F[_]: Effect there, which is probably the best place.

Alternatively we could create a http4s_0.18_tagless generator, I don't know, but I think this change / addition will be worth it considering that the interest in the finally tagless pattern is growing.

I'll try to submit a PR this week, but first let me know if I should do a breaking change or create a new generator.

[gheine]

I totally agree. Meant to look into this myself but happy to let you have a go. I think it's ok to do a breaking change, given that the http4s generators are flagged alpha and also given that http4s 0.18 was only just released.

[fiadliel]

Defining the base traits in terms of F[_] (no constraints) would be awesome for testing!

[amarrella]

@fiadliel would you add the effect constraint to the methods then? because both the encoder / decoder and Response[F[_]] put constraints on F.

[fiadliel]

Encoder and decoder are implementation details. They are not referenced in the interfaces.

  package interfaces {

    trait Client[F[_]] {
      def baseUrl: org.http4s.Uri
      def api1: api.v1.Api1[F]
      def api2: api.v1.Api2[F]
    }

    trait Api1[F[_]] {
      def doSomething(): F[DoSomethingResponse]
    }
  }

Then in the implementation:

class Client[F[_]: Effect](...) extends interfaces.Client[F] {
  def api1: Api1[F] = Api1

  object Api1 extends Api1[F] {
  }
}

[amarrella]

Ok, for the server would you also put only the signatures of the methods we provide for free in the trait and add an implementation with those defined as well?

Something like

trait FooRoutes[F[_]] {
  import Matchers._

  implicit def circeJsonDecoder[A](implicit decoder: _root_.io.circe.Decoder[A])
  implicit def circeJsonEncoder[A](implicit encoder: _root_.io.circe.Encoder[A])

  case class PostRequest(id: String, opt: _root_.scala.Option[Int], list: Seq[String], map: Map[String, String])

  implicit def PostRequestDecoder: _root_.io.circe.Decoder[PostRequest]

  sealed trait PostResponse

  object PostResponse {
    case class HTTP200(headers: Seq[org.http4s.Header] = Nil) extends PostResponse
  }

  def post(
            _req: org.http4s.Request[F],
            id: String,
            opt: _root_.scala.Option[Int],
            list: Seq[String],
            map: Map[String, String]
          ): F[PostResponse]

  def apiVersionMatch(req: org.http4s.Message[F]): Boolean

  def service(): org.http4s.HttpService[F]
}

// (with the right implementation instead of ???)
abstract class FooRoutesInterpreter[F[_]: Effect] extends FooRoutes[F] with Http4sDsl[F] {
  implicit def circeJsonDecoder[A](implicit decoder: _root_.io.circe.Decoder[A]) = ???
  implicit def circeJsonEncoder[A](implicit encoder: _root_.io.circe.Encoder[A]) = ???

  implicit val PostRequestDecoder: _root_.io.circe.Decoder[PostRequest] = ???

  def apiVersionMatch(req: org.http4s.Message[F]): Boolean = ???

  def service() = org.http4s.HttpService[F] { ??? }

[amarrella]

(i called it interpreter because it gives an intermediate interpretation, not sure if it's a good name)

[fiadliel]

I wasn't thinking of the server side, but probably there you need to do
trait FooRoutes[F[_]: Sync] due to the implementations for creating decoders/encoders. In general, keep things general, and specify the simplest required typeclass. The client trait is easier to make generic to all F[_].

[amarrella]

I can't do trait with [F[_]: Sync], i need a class I think

[fiadliel]

Ah, true - that means that to keep this both a trait, and generic, the typeclass requirements have to pushed down to individual methods.

[amarrella]

Also, unfortunately org.http4s.circe.jsonOf[F, A] wants Effect so I can't go simpler

[amarrella]

I can push down to the methods, I just don't like all the "boilerplate" it creates also for who is going to extend the method (who will need to specify the type argument for each individual method). Do we really need the routes to be a trait?

[fiadliel]

Hmm what if the trait was this simple:

sealed trait GetResponse

object GetResponse {
  case class HTTP200(value: models.Val, headers: Seq[org.http4s.Header] = Nil) extends GetResponse
}

trait ApiServerRoutes[F[_]: Monad] {
  def get(_req: org.http4s.Request[F]): F[GetResponse]

    def service() = org.http4s.HttpService[F] {
    case _req @ GET -> Root / "something" if apiVersionMatch(_req) =>
      get(_req).flatMap {
        case GetResponse.HTTP200(value, headers) => Ok(value).putHeaders(headers: _*)
      }
  }
}

It's then the implementation that pulls in circe en/decoding, etc.

[amarrella]

Hmm I think your reply is missing a piece?

[fiadliel]

Well, the implementation of apiVersionMatch shouldn't change, if that's the bit you mean.

[fiadliel]

Plus you need

import cats.Monad
import cats.implicits._

[amarrella]

trait ApiServerRoutes[F[_]: Monad] is illegal, you need to make it a class
The compiler complains:
traits cannot have type parameters with context bounds `: ...' nor view bounds `<% ...'

[fiadliel]

oh right, that again. Yes, you add that requirement then on individual methods. Here, it's only needed on service.

trait ApiServerRoutes[F[_]] {
  def service()(implicit F: Monad[F]) = org.http4s.HttpService[F] {
    case _req @ GET -> Root / "something" if apiVersionMatch(_req) =>
      get(_req).flatMap {
        case GetResponse.HTTP200(value, headers) => Ok(value).putHeaders(headers: _*)
      }
  }
}

[fiadliel]

Then you can do:

class MyTestRoutes extends ApiServerRoutes[Id] {
  override def get(_req: org.http4s.Request[Id]): Id[GetResponse] = ???
}

[fiadliel]

Hmm except you need an EntityEncoder still. :/ Well, I'll think about it further. Anyway hardcoding IO is something to avoid if possible.

[amarrella]

We could move F to service, require that it is an effect at that level only and take the encoder / decoder as implicit parameters

[amarrella]

Except that if we want to do anything with the request in the other methods we need encoder / decoder there as well :/

[fiadliel]

Yeah... as long as we need circe JSON decoder, we need Sync, and as long as we need JSON encoder, we need Applicative. Unfortunately the HttpService type requires that we do JSON conversion, so my hope that we could write the service in terms of Id isn't reasonable. Still, it just means that service requires Sync. (we can also then define the EntityEncoder/EntityDecoder inside the definition of service...?

[amarrella]

Creating a Sync[Id] shouldn't be a problem right?

But we pass around the request so entityencoder / decoder either are implicit parameters or they need to be defined at the trait level.

[fiadliel]

Sync[Id] is not lawful, Id doesn't capture any effects.

But aha! There is a Sync[StateT[F, S, ?]] if there is a Sync[F] - which is acceptable. So you can use StateT to do some useful tests.

[amarrella]

Ok, summarizing I can move the requirements to the methods (with Sync or Applicative where appropriate) and go back to being a trait.

I think encoder and decoder are handy to be defined in the trait though, otherwise they need to be passed around to almost every method).

[fiadliel]

Yes! But I think you can push the typeclass requirement to just service, it looks like all the JSON encoding/decoding happens there?

[amarrella]

not if you do anything with _req in the other methods. If for some reason you do an attemptAs there (I can't see any reason to do that, but...) you need the encoder again

[amarrella]

(also, service would need to receive io.circe.Encoder and io.circe.Decoder as implicit parameters in this case right?)

[fiadliel]

You can create encoder and decoder in the definition of service.

And I don't see why you would want to decode body from that when there's an argument doing body decoding already (alternatively, remove that argument and pass the decoder instead).

[amarrella]

I implemented all the changes discussed in #336, I still have doubts on moving the encoder / decoder definitions inside service. What if we want to override service?

[fiadliel]

Okay it may work there like that - but would recommend that you test F (both without/with Sync), and see that things fail with a vaguely understandable error message if F isn't Sync.

[amarrella]

I tested the server, and if F isn't Sync it fails when only when we call .service, with the following error could not find implicit value for parameter sync: cats.effect.Sync[cats.Id] which i think it's clear enough.