Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refine Endpoint Module #988

Merged
merged 1 commit into from
Oct 7, 2018
Merged

Refine Endpoint Module #988

merged 1 commit into from
Oct 7, 2018

Conversation

vkostyukov
Copy link
Collaborator

@vkostyukov vkostyukov commented Oct 2, 2018

This PR refines what EndpointModule is and where Endpoint instances are constructed. It doesn't invent anything new but reuses ideas from other libraries solving similar problems.

The following ways of constructing endpoints are now supported.

  1. If you prefer being explicit:
import io.finch._, cats.effect.IO
val foo = Endpoint.path[IO, Int] // Endpoint[IO, Int]
  1. If you prefer being explicit and also would like type-inference work for you:
import io.finch._, cats.effect.IO
val foo = Endpoint[IO].path[Int] // Endpoint[IO, Int]
  1. If you prefer do not repeat yourself and just opt-in for Cats Effect in this source file:
import io.finch._, io.finch.catsEffect._
val foo = path[Int] // Endpoint[IO, Int]
  1. If you prefer to have an extension point for your own effect.
object myIO extends Endpoint.Module[MyIO]

import io.finch._, myIO._
val foo = path[Int] // Endpoint[MyIO, Int]
  1. If you prefer an extension point for your own effect and want to keep the imports minimal.
import io.finch._

object Foo extends Endpoint.Module[MyIO] {
  val foo = path[Int] // Endpoint[MyIO, Int]
}

What's changed

  • All endpoint instances are now defined on Endpoint.* (think Endpoint.param, Endpoint.body) and EndpointModule.* just redirects its calls there similar to how iteratee and Monix do that.
  • Module[F[_]] no longer embeds Outputs and ValidationRules are they are not depending on the effect type (io.finch._ imports them as it does today).
  • There is no longer tried module implementation, IO is used in tests.
  • The syntax package is removed and merged with the rest of the endpoints (note Endpoint.Mappable).
  • We're no longer providing the Endpoint type alias from within EndpointModule.
  • / is renamed to zero and * is renamed to pathAny (the fewer symbolic APIs we have the better).
  • Style has been broken. I want to keep it that way until we replace scalastyle with scalafmt in Code formatter? #196.
  • Implicit conversions from Int and Boolean to path-endpoints were removed. Only String is supported now (the less we implicitly convert the better).

@vkostyukov vkostyukov changed the title Endpoint Module Endpoint Module [WIP] Oct 2, 2018
@codecov-io
Copy link

codecov-io commented Oct 2, 2018

Codecov Report

Merging #988 into cats-effect will increase coverage by 0.41%.
The diff coverage is 78.41%.

Impacted file tree graph

@@               Coverage Diff               @@
##           cats-effect     #988      +/-   ##
===============================================
+ Coverage        84.84%   85.26%   +0.41%     
===============================================
  Files               49       47       -2     
  Lines              884      882       -2     
  Branches            53       50       -3     
===============================================
+ Hits               750      752       +2     
+ Misses             134      130       -4
Impacted Files Coverage Δ
core/src/main/scala/io/finch/endpoint/method.scala 100% <ø> (ø)
core/src/main/scala/io/finch/internal/Mapper.scala 100% <ø> (ø)
core/src/main/scala/io/finch/package.scala 100% <ø> (ø) ⬆️
...amples/src/main/scala/io/finch/iteratee/Main.scala 0% <0%> (ø) ⬆️
core/src/main/scala/io/finch/endpoint/path.scala 100% <100%> (ø)
core/src/main/scala/io/finch/endpoint/cookie.scala 54.54% <54.54%> (ø)
core/src/main/scala/io/finch/Endpoint.scala 76.31% <75.71%> (-2.26%) ⬇️
...e/src/main/scala/io/finch/endpoint/multipart.scala 75.92% <75.92%> (ø)
core/src/main/scala/io/finch/endpoint/body.scala 79.16% <79.16%> (ø)
core/src/main/scala/io/finch/EndpointModule.scala 85.41% <85.41%> (ø)
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ec9e236...229d2da. Read the comment docs.

@vkostyukov vkostyukov changed the title Endpoint Module [WIP] Refine Endpoint Module [WIP] Oct 2, 2018
@vkostyukov vkostyukov force-pushed the vk/endpoint-module branch 4 times, most recently from 6bc8cc5 to 5ecb0d5 Compare October 2, 2018 17:49
@sergeykolbasov
Copy link
Collaborator

sergeykolbasov commented Oct 2, 2018

What is a benifit of having two almost equivalent method definitions like Endpoint.const[F, _] and EndpointModule[F].const comparing to what it was before with Endpoint[F].const?

@vkostyukov
Copy link
Collaborator Author

vkostyukov commented Oct 2, 2018

What is a benifit of having two almost equivalent method definitions like Endpoint.const and EndpointModule.const comparing to what it was before with Endpoint[F].const?

I feel like this structure promote consistency. Endpoint.* is the only place where instances are defined and it uses the very direct API (a call-site has to supply both F[_] and A). And the EndpointModule enables users to drop the F[_] argument assuming it's always the same thing in their apps.

The very same approach is used in several places where I looked-up. Notably, Monix's Iterant and iteratee libraries are structured in the same.

UPDATE: Endpoint[IO].const("foo") still works today (without any code duplication).

@vkostyukov vkostyukov changed the title Refine Endpoint Module [WIP] Refine Endpoint Module Oct 2, 2018
Copy link
Collaborator

@sergeykolbasov sergeykolbasov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Few nitpicks of future leftovers

val i = Input(emptyRequest, Seq.empty)
var c = 0
val e = get(*) { c = c + 1; Ok(c) }
val e = get(pathAny) { c = c + 1; Ok(c) }

e(i).awaitValueUnsafe() shouldBe Some(1)
e(i).awaitValueUnsafe() shouldBe Some(2)
}

it should "not evaluate Futures until matched" in {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

evaluate Effect


e(i).awaitValueUnsafe() shouldBe Some(1)
e(i).awaitValueUnsafe() shouldBe Some(2)
}

it should "not evaluate Futures until matched" in {
import io.finch.catsEffect._

val i = Input(emptyRequest, Seq("a", "10"))
var flag = false

val endpointWithFailedFuture = "a".mapAsync { nil =>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FailedIO here

@vkostyukov vkostyukov merged commit 7824a07 into cats-effect Oct 7, 2018
@vkostyukov vkostyukov deleted the vk/endpoint-module branch October 7, 2018 06:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants