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

Merge plugins #43

Merged
merged 14 commits into from
May 28, 2020
6 changes: 6 additions & 0 deletions core/src/main/scala/higherkindness/mu/rpc/srcgen/Model.scala
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,12 @@ object Model {
}
}

sealed trait ExecutionMode extends Product with Serializable
object ExecutionMode {
case object Compendium extends ExecutionMode
case object Local extends ExecutionMode
}

sealed trait IdlType extends Product with Serializable

object IdlType {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package higherkindness.mu.rpc.srcgen.compendium

import cats.effect.Sync
import org.http4s._
import org.http4s.circe._
import cats.implicits._
import org.http4s.client.Client

trait CompendiumClient[F[_]] {

/** Retrieve a Protocol by its id
*
* @param identifier the protocol identifier
* @param version optional protocol version number
* @return a protocol
*/
def retrieveProtocol(identifier: String, version: Option[Int]): F[Option[RawProtocol]]
Copy link
Member

Choose a reason for hiding this comment

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

Should we make the version a String or something, in anticipation of higherkindness/compendium#284 ?


}

object CompendiumClient {

def apply[F[_]: Sync](
clientF: Client[F],
clientConfig: HttpConfig
): CompendiumClient[F] =
new CompendiumClient[F] {

val baseUrl: String = s"http://${clientConfig.host}:${clientConfig.port}"
Copy link
Member

Choose a reason for hiding this comment

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

suggestion: I'd parametrize the http part of the Url (it could be https).

Copy link
Contributor

Choose a reason for hiding this comment

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

Or merge both in a single property called "connectionUrl" or something like that

Copy link
Member

Choose a reason for hiding this comment

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

Yeah I would put the whole URL, including the port number, into a single property (connectionUrl? serverUrl?). Fewer things for the user to configure, and no assumptions about how they want to host the server.

Copy link
Author

Choose a reason for hiding this comment

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

for now, it could be merged. It was split because we had, at least, two endpoints to call.

So, does this work for both of you?

val versionParam = version.fold("")(v => s"?version=${v.show}")
        val connectionUrl = s"https://${clientConfig.host}:${clientConfig.port}/v0/protocol/$identifier$versionParam"

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I wasn't very clear. My suggestion was to have the server's base URL (e.g. https://devtools.mycorp.com:1234/compendium) passed in as a single property inside HttpConfig. Inside this method we would construct the full URL:

val connectionUrl = s"${clientConfig.serverUrl}/v0/protocol/$identifier$versionParam"

val versionParamName: String = "version"

override def retrieveProtocol(
identifier: String,
version: Option[Int]
): F[Option[RawProtocol]] = {
val versionParam = version.fold("")(v => s"?$versionParamName=${v.show}")
val uri = s"$baseUrl/v0/protocol/$identifier$versionParam"

implicit val rawEntityDecoder = jsonOf[F, RawProtocol]

clientF.get(uri)(res =>
res.status match {
case Status.Ok => res.as[RawProtocol].map(Option(_))
case Status.NotFound => Sync[F].pure(None)
Copy link
Member

Choose a reason for hiding this comment

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

question: If something is not found, it worth to notify the user?

Copy link
Author

Choose a reason for hiding this comment

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

With the current development, it raises an error:

case None =>
              Async[F].raiseError[File](
                ProtocolNotFound(s"Protocol ${protocolAndVersion.name} not found in Compendium. ")
              )

on run function

Copy link
Member

Choose a reason for hiding this comment

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

I think it makes sense to just return None at this point, because we are just an HTTP client and we don't know what we are being used for. It's pretty reasonable to try to get a resource which might not exist, so it's not an exceptional circumstance.

Whoever is using the client can decide whether a missing resource should be an error or not.

case Status.InternalServerError =>
Sync[F].raiseError(UnknownError(s"Error in compendium server"))
case _ =>
Sync[F].raiseError(UnknownError(s"Unknown error with status code ${res.status.code}"))
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
case Status.InternalServerError =>
Sync[F].raiseError(UnknownError(s"Error in compendium server"))
case _ =>
Sync[F].raiseError(UnknownError(s"Unknown error with status code ${res.status.code}"))
case s => Sync[F].raiseError(UnexpectedStatus(s))

Copy link
Contributor

Choose a reason for hiding this comment

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


}
)
}
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/*
* Copyright 2019-2020 47 Degrees, LLC. <http://www.47deg.com>
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package higherkindness.mu.rpc.srcgen.compendium

abstract class CompendiumError(error: String) extends Exception(error)

final case class ProtocolNotFound(msg: String) extends CompendiumError(msg)
final case class SchemaError(msg: String) extends CompendiumError(msg)
final case class UnknownError(msg: String) extends CompendiumError(msg)
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
package higherkindness.mu.rpc.srcgen.compendium

import java.io.{File, PrintWriter}

import cats.effect.{Async, ConcurrentEffect, Resource, Sync}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
import cats.effect.{Async, ConcurrentEffect, Resource, Sync}
import cats.effect.{Resource, ConcurrentEffect}

I think you have a mix of effects here, but I believe that you are just requesting ConcurrentEffect for the client resource usage.


import scala.util.Try
import cats.implicits._
import org.http4s.client.blaze._

import scala.concurrent.ExecutionContext.global

final case class ProtocolAndVersion(name: String, version: Option[String])
final case class FilePrintWriter(file: File, pw: PrintWriter)

case class CompendiumMode[F[_]: Async: ConcurrentEffect](
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
case class CompendiumMode[F[_]: Async: ConcurrentEffect](
final case class CompendiumMode[F[_]: ConcurrentEffect](

protocols: List[ProtocolAndVersion],
fileType: String,
httpConfig: HttpConfig,
path: String
) {

val httpClient = BlazeClientBuilder[F](global).resource

def run(): F[List[File]] =
protocols.traverse(protocolAndVersion =>
httpClient.use(client => {
for {

protocol <- CompendiumClient(client, httpConfig)
.retrieveProtocol(
protocolAndVersion.name,
safeInt(protocolAndVersion.version)
)
file <- protocol match {
case Some(raw) =>
writeTempFile(
raw.raw,
extension = fileType,
identifier = protocolAndVersion.name,
path = path
)
case None =>
Async[F].raiseError[File](
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Async[F].raiseError[File](
ConcurrentEffect[F].raiseError[File](

ProtocolNotFound(s"Protocol ${protocolAndVersion.name} not found in Compendium. ")
)
}
} yield file
})
Copy link
Member

Choose a reason for hiding this comment

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

You will need to replace the Sync[F] below by ConcurrentEffect[F] (writeTempFile function).

)

private def safeInt(s: Option[String]): Option[Int] = s.flatMap(str => Try(str.toInt).toOption)

private def writeTempFile(
msg: String,
extension: String,
identifier: String,
path: String
): F[File] =
Resource
.make(Sync[F].delay {
if (!new File(path).exists()) new File(path).mkdirs()
val file = new File(path + s"/$identifier.$extension")
file.deleteOnExit()
FilePrintWriter(file, new PrintWriter(file))
}) { fpw: FilePrintWriter => Sync[F].delay(fpw.pw.close()) }
.use((fpw: FilePrintWriter) => Sync[F].delay(fpw.pw.write(msg)).as(fpw))
.map(_.file)

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package higherkindness.mu.rpc.srcgen.compendium

import io.circe.{Decoder, Encoder}
import io.circe.generic.semiauto.{deriveDecoder, deriveEncoder}

final case class ErrorResponse(message: String)

object ErrorResponse {
implicit val decoder: Decoder[ErrorResponse] = deriveDecoder[ErrorResponse]
implicit val encoder: Encoder[ErrorResponse] = deriveEncoder[ErrorResponse]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/*
* Copyright 2019-2020 47 Degrees, LLC. <http://www.47deg.com>
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package higherkindness.mu.rpc.srcgen.compendium

final case class HttpConfig(host: String, port: Int)
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*
* Copyright 2019-2020 47 Degrees, LLC. <http://www.47deg.com>
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package higherkindness.mu.rpc.srcgen.compendium

import io.circe.generic.semiauto.{deriveDecoder, deriveEncoder}
import io.circe.{Decoder, Encoder}

final case class RawProtocol(raw: String)

object RawProtocol {
implicit val decoder: Decoder[RawProtocol] = deriveDecoder[RawProtocol]
implicit val encoder: Encoder[RawProtocol] = deriveEncoder[RawProtocol]

}
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,17 @@ package higherkindness.mu.rpc.srcgen

import java.io.File

import cats.effect.{ContextShift, IO => IOCats}
import higherkindness.mu.rpc.srcgen.Model.ExecutionMode._
import sbt.Keys._
import sbt._
import sbt.{Def, settingKey, _}
import sbt.io.{Path, PathFinder}

import higherkindness.mu.rpc.srcgen.Model._
import higherkindness.mu.rpc.srcgen.compendium.{CompendiumMode, HttpConfig, ProtocolAndVersion}
import higherkindness.mu.rpc.srcgen.openapi.OpenApiSrcGenerator.HttpImpl

import scala.concurrent.ExecutionContext.global

object SrcGenPlugin extends AutoPlugin {

override def trigger: PluginTrigger = allRequirements
Expand Down Expand Up @@ -105,6 +109,21 @@ object SrcGenPlugin extends AutoPlugin {
"By default, the streaming implementation is FS2 Stream."
)

lazy val muSrcGenExecutionMode = settingKey[ExecutionMode](
"Execution mode of the plugin. If Compendium, it's required a compendium instance where IDL files are saved. `Local` by default."
)

lazy val muSrcGenCompendiumProtocolIdentifiers: SettingKey[Seq[ProtocolAndVersion]] =
settingKey[Seq[ProtocolAndVersion]](
"Protocol identifiers (and version) to be retrieved from compendium server. By default is an empty list."
)

lazy val muSrcGenCompendiumServerHost: SettingKey[String] =
settingKey[String]("Host of the compendium server. By default, `localhost`.")

lazy val muSrcGenCompendiumServerPort: SettingKey[Int] =
settingKey[Int]("Port of the compendium server. `47047` by default.")

Copy link
Member

Choose a reason for hiding this comment

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

suggestion: I think it worth specifying what are the default values for every new setting. Thoughts?

Copy link
Author

Choose a reason for hiding this comment

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

I have to update documentation too, which has (or should have) the default values. But it's not a bad idea

}

import autoImport._
Expand Down Expand Up @@ -141,7 +160,11 @@ object SrcGenPlugin extends AutoPlugin {
muSrcGenCompressionType := NoCompressionGen,
muSrcGenIdiomaticEndpoints := false,
muSrcGenOpenApiHttpImpl := HttpImpl.Http4sV20,
muSrcGenStreamingImplementation := Fs2Stream
muSrcGenStreamingImplementation := Fs2Stream,
muSrcGenExecutionMode := Local,
muSrcGenCompendiumProtocolIdentifiers := Nil,
muSrcGenCompendiumServerHost := "localhost",
muSrcGenCompendiumServerPort := 47047
)

lazy val taskSettings: Seq[Def.Setting[_]] = {
Expand All @@ -159,16 +182,31 @@ object SrcGenPlugin extends AutoPlugin {
)
},
Def.task {
muSrcGenSourceDirs.value.toSet.foreach { f: File =>
IO.copyDirectory(
f,
muSrcGenIdlTargetDir.value,
CopyOptions(
overwrite = true,
preserveLastModified = true,
preserveExecutable = true
)
)
muSrcGenExecutionMode.value match {
case Compendium =>
implicit val cs: ContextShift[IOCats] = IOCats.contextShift(global)
CompendiumMode[IOCats](
muSrcGenCompendiumProtocolIdentifiers.value.toList,
muSrcGenIdlExtension.value,
HttpConfig(
muSrcGenCompendiumServerHost.value,
muSrcGenCompendiumServerPort.value
),
muSrcGenIdlTargetDir.value.getAbsolutePath
).run()
.unsafeRunSync()
case Local =>
muSrcGenSourceDirs.value.toSet.foreach { f: File =>
IO.copyDirectory(
f,
muSrcGenIdlTargetDir.value,
CopyOptions(
overwrite = true,
preserveLastModified = true,
preserveExecutable = true
)
)
}
}
},
Def.task {
Expand Down
5 changes: 5 additions & 0 deletions plugin/src/sbt-test/sbt-mu-srcgen/compendium/build.sbt
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
version := sys.props("version")

libraryDependencies ++= Seq(
"io.higherkindness" %% "mu-rpc-server" % sys.props("mu")
)
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
addSbtPlugin("io.higherkindness" %% "sbt-mu-srcgen" % sys.props("version"))
7 changes: 7 additions & 0 deletions plugin/src/sbt-test/sbt-mu-srcgen/compendium/test
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
> 'set muSrcGenExecutionMode := higherkindness.mu.rpc.srcgen.Model.ExecutionMode.Compendium'
> 'set muSrcGenIdlType := higherkindness.mu.rpc.srcgen.Model.IdlType.Proto'
> 'set muSrcGenCompendiumProtocolIdentifiers := Seq(higherkindness.mu.rpc.srcgen.compendium.ProtocolAndVersion.apply("shop",None))'
> 'set muSrcGenCompendiumServerPort := 8080'
# > compile
# $ exists target/scala-2.12/resource_managed/main/proto/shop.proto

4 changes: 4 additions & 0 deletions project/ProjectPlugin.scala
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,15 @@ object ProjectPlugin extends AutoPlugin {
lazy val V = new {
val avrohugger: String = "1.0.0-RC22"
val circe: String = "0.13.0"
val hammock = "0.10.0"
Copy link
Member

@juanpedromoreno juanpedromoreno May 22, 2020

Choose a reason for hiding this comment

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

Hammock 0.10.0 is bringing circe 0.12.1, as you can see here. However, that will provoke a collision with the circe version we are using in this build: 0.13.0.

My suggestion would be using an http4s client, as it is evolving at the same time as typelevel ecosystem does. The other option would be to work on Hammock and try to make it updated and released with the latest dependency versions.

Copy link
Author

Choose a reason for hiding this comment

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

ready, please take a look @juanpedromoreno

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
val hammock = "0.10.0"

val monocle: String = "2.0.4"
val mu = "0.22.1"
val scalacheck: String = "1.14.3"
val scalatest: String = "3.1.2"
val scalatestplusScheck: String = "3.1.0.0-RC2"
val skeuomorph: String = "0.0.23"
val slf4j: String = "1.7.30"
val http4s: String = "0.21.1"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
val http4s: String = "0.21.1"
val http4s: String = "0.21.4"

Copy link
Author

Choose a reason for hiding this comment

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

I'll put all the changes in the same commit

}

lazy val srcGenSettings: Seq[Def.Setting[_]] = Seq(
Expand All @@ -33,6 +35,8 @@ object ProjectPlugin extends AutoPlugin {
"io.higherkindness" %% "skeuomorph" % V.skeuomorph,
"com.julianpeeters" %% "avrohugger-core" % V.avrohugger,
"io.circe" %% "circe-generic" % V.circe,
"org.http4s" %% "http4s-blaze-client" % V.http4s,
"org.http4s" %% "http4s-circe" % V.http4s,
"org.scalatest" %% "scalatest" % V.scalatest % Test,
"org.scalacheck" %% "scalacheck" % V.scalacheck % Test,
"org.scalatestplus" %% "scalatestplus-scalacheck" % V.scalatestplusScheck % Test,
Expand Down