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

Support more HTTP methods, keep more details about HTTP matching failures #493

Merged
merged 15 commits into from
Nov 10, 2022
4 changes: 1 addition & 3 deletions build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -231,8 +231,7 @@ lazy val scalacheck = projectMatrix
Dependencies.collectionsCompat.value,
Dependencies.Scalacheck.scalacheck.value
),
libraryDependencies ++= munitDeps.value,
testFrameworks += new TestFramework("weaver.framework.CatsEffect")
Copy link
Member Author

Choose a reason for hiding this comment

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

This is set by the project plugin

libraryDependencies ++= munitDeps.value
)
.jvmPlatform(allJvmScalaVersions, jvmDimSettings)
.jsPlatform(allJsScalaVersions, jsDimSettings)
Expand Down Expand Up @@ -706,7 +705,6 @@ lazy val complianceTests = projectMatrix
Dependencies.Weaver.cats.value % Test
)
},
testFrameworks += new TestFramework("weaver.framework.CatsEffect"),
Test / smithySpecs := Seq(
(ThisBuild / baseDirectory).value / "sampleSpecs" / "test.smithy"
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import org.typelevel.ci.CIString
import smithy4s.aws.SimpleHttpClient
import smithy4s.http.CaseInsensitive
import smithy4s.http.HttpMethod
import org.http4s.Method

final class AwsHttp4sBackend[F[_]: Concurrent](client: Client[F])
extends SimpleHttpClient[F] {
Expand All @@ -53,12 +54,13 @@ object AwsHttp4sBackend {

for {
endpoint <- Uri.fromString(request.uri).liftTo[F]
method = request.httpMethod match {
case HttpMethod.POST => POST
case HttpMethod.GET => GET
case HttpMethod.PATCH => PATCH
case HttpMethod.PUT => PUT
case HttpMethod.DELETE => DELETE
method <- request.httpMethod match {
case HttpMethod.POST => POST.pure[F]
case HttpMethod.GET => GET.pure[F]
case HttpMethod.PATCH => PATCH.pure[F]
case HttpMethod.PUT => PUT.pure[F]
case HttpMethod.DELETE => DELETE.pure[F]
case HttpMethod.OTHER(value) => Method.fromString(value).liftTo[F]
}
req = request.body
.foldLeft(
Expand Down
3 changes: 0 additions & 3 deletions modules/core/src/smithy4s/Refinement.scala
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,6 @@ trait Refinement[A, B] { self =>
*/
def unsafe(a: A): B

@deprecated("use unsafe instead")
def unchecked(a: A): B = unsafe(a)

final val asFunction: A => Either[ConstraintError, B] =
(a: A) =>
apply(a).left.map(msg =>
Expand Down
22 changes: 17 additions & 5 deletions modules/core/src/smithy4s/http/HttpEndpoint.scala
Original file line number Diff line number Diff line change
Expand Up @@ -38,17 +38,27 @@ object HttpEndpoint {

def unapply[Op[_, _, _, _, _], I, E, O, SI, SO](
endpoint: Endpoint[Op, I, E, O, SI, SO]
): Option[HttpEndpoint[I]] = cast(endpoint)
): Option[HttpEndpoint[I]] = cast(endpoint).toOption

def cast[Op[_, _, _, _, _], I, E, O, SI, SO](
endpoint: Endpoint[Op, I, E, O, SI, SO]
): Option[HttpEndpoint[I]] = {
): Either[HttpEndpointError, HttpEndpoint[I]] = {
for {
http <- endpoint.hints.get(Http)
httpMethod <- HttpMethod.fromString(http.method.value)
httpPath <- internals.pathSegments(http.uri.value)
http <- endpoint.hints
.get(Http)
.toRight(HttpEndpointError("Operation doesn't have a @http trait"))
httpMethod = HttpMethod.fromStringOrDefault(http.method.value)
httpPath <- internals
.pathSegments(http.uri.value)
.toRight(
HttpEndpointError(
s"Unable to parse HTTP path template: ${http.uri.value}"
)
)
encoder <- SchemaVisitorPathEncoder(
endpoint.input.addHints(http)
).toRight(
HttpEndpointError("Unable to encode operation input in HTTP path")
)

} yield {
Expand All @@ -61,4 +71,6 @@ object HttpEndpoint {
}
}

case class HttpEndpointError(message: String) extends Exception(message)

}
40 changes: 26 additions & 14 deletions modules/core/src/smithy4s/http/Method.scala
Original file line number Diff line number Diff line change
Expand Up @@ -16,21 +16,23 @@

package smithy4s.http

sealed trait HttpMethod {
sealed trait HttpMethod extends Product with Serializable {
def showUppercase = this match {
case HttpMethod.PUT => "PUT"
case HttpMethod.POST => "POST"
case HttpMethod.DELETE => "DELETE"
case HttpMethod.GET => "GET"
case HttpMethod.PATCH => "PATCH"
case HttpMethod.PUT => "PUT"
case HttpMethod.POST => "POST"
case HttpMethod.DELETE => "DELETE"
case HttpMethod.GET => "GET"
case HttpMethod.PATCH => "PATCH"
case HttpMethod.OTHER(value) => value.toUpperCase
}

def showCapitalised = this match {
case HttpMethod.PUT => "Put"
case HttpMethod.POST => "Post"
case HttpMethod.DELETE => "Delete"
case HttpMethod.GET => "Get"
case HttpMethod.PATCH => "Patch"
case HttpMethod.PUT => "Put"
case HttpMethod.POST => "Post"
case HttpMethod.DELETE => "Delete"
case HttpMethod.GET => "Get"
case HttpMethod.PATCH => "Patch"
case HttpMethod.OTHER(value) => value.capitalize
}
}

Expand All @@ -40,10 +42,20 @@ object HttpMethod {
case object DELETE extends HttpMethod
case object GET extends HttpMethod
case object PATCH extends HttpMethod
case class OTHER(value: String) extends HttpMethod

val values = List(PUT, POST, DELETE, GET, PATCH)
val values: List[HttpMethod] =
List(PUT, POST, DELETE, GET, PATCH)

def fromString(s: String): Option[HttpMethod] = values.find { m =>
CaseInsensitive(s) == CaseInsensitive(m.showCapitalised)
def fromStringOrDefault(s: String): HttpMethod =
fromStringOption(s).getOrElse(OTHER(s.toUpperCase))

private def fromStringOption(s: String): Option[HttpMethod] = {
val nameCI = CaseInsensitive(s)

values
.find { m =>
nameCI == CaseInsensitive(m.showCapitalised)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ package internals

import smithy4s.schema._

private[smithy4s] class ErrorCodeSchemaVisitor(
private[http] class ErrorCodeSchemaVisitor(
Copy link
Member Author

Choose a reason for hiding this comment

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

More narrow visibility scope here - let me know if that makes sense

val cache: CompilationCache[HttpCode]
) extends SchemaVisitor.Cached[HttpCode]
with SchemaVisitor.Default[HttpCode] { compile =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import StringAndBlobCodecSchemaVisitor._
import smithy.api.HttpPayload
import java.nio.ByteBuffer

private[smithy4s] object StringAndBlobCodecSchemaVisitor {
private[http] object StringAndBlobCodecSchemaVisitor {

trait SimpleCodec[A] { self =>
def mediaType: HttpMediaType
Expand Down Expand Up @@ -89,7 +89,7 @@ private[smithy4s] object StringAndBlobCodecSchemaVisitor {

}

private[smithy4s] class StringAndBlobCodecSchemaVisitor
private[http] class StringAndBlobCodecSchemaVisitor
extends SchemaVisitor.Default[CodecResult] {

override def default[A]: CodecResult[A] = noop
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ package internals

import scala.annotation.tailrec

// note: this should be private[http] but it's used in the aws-kernel module too
private[smithy4s] object URIEncoderDecoder {

val digits: String = "0123456789ABCDEF"
Expand Down
2 changes: 1 addition & 1 deletion modules/core/src/smithy4s/http/internals/package.scala
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ package object internals {
}
}

private[smithy4s] def pathSegments(
private[http] def pathSegments(
str: String
): Option[Vector[PathSegment]] = {
str
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ class PathSpec() extends munit.FunSuite {
.cast(
DummyPath
)
.toTry
.get
.path(
PathParams(
Expand Down
1 change: 1 addition & 0 deletions modules/decline/src/Smithy4sCli.scala
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ class Smithy4sCli[Alg[_[_, _, _, _, _]], Op[_, _, _, _, _], F[_]: MonadThrow](
): List[String] =
HttpEndpoint
.cast(endpoint)
.toOption
Copy link
Contributor

Choose a reason for hiding this comment

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

minor: since we're throwing a few lines below, would it be worth it to throw here also rather than just swallow the error?

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah lemme do that

Copy link
Member Author

Choose a reason for hiding this comment

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

actually, I don't think so...

If HttpEndpoint.cast fails, that means the endpoint is not a HTTP one and this method should just return Nil. We're throwing iff HttpEndpoint.cast succeeds and the Http hint is missing, which would be a completely invalid state.

The purpose of protocolSpecificHelp is to generate a list of additional help strings based on... protocol-specific info 😅
so if you're implementing a CLI for a service that only exposes a CLI (and no HTTP interface), you want Nil here.

Copy link
Member Author

Choose a reason for hiding this comment

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

I do the same in Playground and I've been thinking about it recently: kubukoz/smithy-playground#139 - basically, if protocol-specific interpreters are allowed to prefix URLs, maybe we shouldn't trust the Http hint. Or HttpEndpoint, for that matter. Not sure what to think about this, but it's a separate problem from this PR I think.

.map { httpEndpoint =>
val path = endpoint.hints
.get(Http)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ class OperationSpec() extends munit.FunSuite {
val httpEndpoints = endpoints.map(HttpEndpoint.cast(_))

expect(
httpEndpoints.forall(_.isDefined)
httpEndpoints.forall(_.isRight)
)
}
}
Expand Down
17 changes: 0 additions & 17 deletions modules/http4s/src/smithy4s/http4s/SimpleProtocolBuilder.scala
Original file line number Diff line number Diff line change
Expand Up @@ -60,23 +60,6 @@ abstract class SimpleProtocolBuilder[P](val codecs: CodecAPI)(implicit

def client[F[_]: EffectCompat](client: Client[F]) =
new ClientBuilder[Alg, Op, F](client, service)
@deprecated(
kubukoz marked this conversation as resolved.
Show resolved Hide resolved
"Use the ClientBuilder instead, client(client).uri(baseuri).use"
)
def client[F[_]: EffectCompat](
http4sClient: Client[F],
baseUri: Uri
): Either[UnsupportedProtocolError, FunctorAlgebra[Alg, F]] =
client(http4sClient).uri(baseUri).use

@deprecated(
"Use the ClientBuilder instead , client(client).uri(baseuri).resource"
)
def clientResource[F[_]: EffectCompat](
http4sClient: Client[F],
baseUri: Uri
): Resource[F, FunctorAlgebra[Alg, F]] =
client(http4sClient).uri(baseUri).resource

def routes[F[_]: EffectCompat](
impl: FunctorAlgebra[Alg, F]
Expand Down
23 changes: 14 additions & 9 deletions modules/http4s/src/smithy4s/http4s/SmithyHttp4sReverseRouter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -50,16 +50,21 @@ class SmithyHttp4sReverseRouter[Alg[_[_, _, _, _, _]], Op[_, _, _, _, _], F[_]](
def apply[I, E, O, SI, SO](
endpoint: Endpoint[Op, I, E, O, SI, SO]
): SmithyHttp4sClientEndpoint[F, Op, I, E, O, SI, SO] =
SmithyHttp4sClientEndpoint(
baseUri,
client,
endpoint,
compilerContext
).getOrElse(
sys.error(
s"Operation ${endpoint.name} is not bound to http semantics"
SmithyHttp4sClientEndpoint
.make(
baseUri,
client,
endpoint,
compilerContext
)
)
.left
.map { e =>
throw new Exception(
s"Operation ${endpoint.name} is not bound to http semantics",
e
)
}
.merge
}.unsafeCacheBy(
service.endpoints.map(smithy4s.kinds.Kind5.existential(_)),
identity
Expand Down
4 changes: 2 additions & 2 deletions modules/http4s/src/smithy4s/http4s/SmithyHttp4sRouter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,14 @@ class SmithyHttp4sRouter[Alg[_[_, _, _, _, _]], Op[_, _, _, _, _], F[_]](
private val http4sEndpoints: List[SmithyHttp4sServerEndpoint[F]] =
service.endpoints
.map { ep =>
SmithyHttp4sServerEndpoint(
SmithyHttp4sServerEndpoint.make(
impl,
ep,
compilerContext,
errorTransformation
)
}
.collect { case Some(http4sEndpoint) =>
.collect { case Right(http4sEndpoint) =>
http4sEndpoint
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,35 +34,48 @@ import smithy4s.kinds._
* client into a high-level, domain specific function.
*/
// format: off
private[smithy4s] trait SmithyHttp4sClientEndpoint[F[_], Op[_, _, _, _, _], I, E, O, SI, SO] {
private[http4s] trait SmithyHttp4sClientEndpoint[F[_], Op[_, _, _, _, _], I, E, O, SI, SO] {
def send(input: I): F[O]
}
// format: on

private[smithy4s] object SmithyHttp4sClientEndpoint {
private[http4s] object SmithyHttp4sClientEndpoint {

def apply[F[_]: EffectCompat, Op[_, _, _, _, _], I, E, O, SI, SO](
def make[F[_]: EffectCompat, Op[_, _, _, _, _], I, E, O, SI, SO](
baseUri: Uri,
client: Client[F],
endpoint: Endpoint[Op, I, E, O, SI, SO],
compilerContext: CompilerContext[F]
): Option[SmithyHttp4sClientEndpoint[F, Op, I, E, O, SI, SO]] =
HttpEndpoint.cast(endpoint).map { httpEndpoint =>
new SmithyHttp4sClientEndpointImpl[F, Op, I, E, O, SI, SO](
baseUri,
client,
endpoint,
httpEndpoint,
compilerContext
)
): Either[
HttpEndpoint.HttpEndpointError,
SmithyHttp4sClientEndpoint[F, Op, I, E, O, SI, SO]
] =
HttpEndpoint.cast(endpoint).flatMap { httpEndpoint =>
toHttp4sMethod(httpEndpoint.method)
.leftMap { e =>
HttpEndpoint.HttpEndpointError(
"Couldn't parse HTTP method: " + e
)
}
.map { method =>
new SmithyHttp4sClientEndpointImpl[F, Op, I, E, O, SI, SO](
baseUri,
client,
method,
endpoint,
httpEndpoint,
compilerContext
)
}
}

}

// format: off
private[smithy4s] class SmithyHttp4sClientEndpointImpl[F[_], Op[_, _, _, _, _], I, E, O, SI, SO](
private[http4s] class SmithyHttp4sClientEndpointImpl[F[_], Op[_, _, _, _, _], I, E, O, SI, SO](
baseUri: Uri,
client: Client[F],
method: org.http4s.Method,
endpoint: Endpoint[Op, I, E, O, SI, SO],
httpEndpoint: HttpEndpoint[I],
compilerContext: CompilerContext[F]
Expand All @@ -78,7 +91,6 @@ private[smithy4s] class SmithyHttp4sClientEndpointImpl[F[_], Op[_, _, _, _, _],
}

import compilerContext._
private val method: org.http4s.Method = toHttp4sMethod(httpEndpoint.method)
private val inputSchema: Schema[I] = endpoint.input
private val outputSchema: Schema[O] = endpoint.output

Expand Down
Loading