Add ApplicationLoadBalancerRequestEvent#590
Add ApplicationLoadBalancerRequestEvent#590ngwin-ab wants to merge 21 commits intotypelevel:mainfrom
Conversation
|
hi @armanbilge : Could you please review this PR? Thank you! |
lambda/shared/src/test/scala/feral/lambda/events/ApplicationLoadBalancerRequestEventSuite.scala
Outdated
Show resolved
Hide resolved
lambda/shared/src/test/scala/feral/lambda/events/ApplicationLoadBalancerRequestEventSuite.scala
Outdated
Show resolved
Hide resolved
lambda/shared/src/main/scala/feral/lambda/events/ApplicationLoadBalancerRequestEvent.scala
Outdated
Show resolved
Hide resolved
|
@bpholt : hi Brian, thanks for reviewing the PR! I was away from my laptop last week and did not have a chance to work on this until now. I've updated the PR to address your comments, could you please take a look? Thank you! |
|
@armanbilge : hi Arman, could you please review this PR when you have time? Thank you! |
lambda/shared/src/main/scala/feral/lambda/events/ApplicationLoadBalancerRequestEvent.scala
Outdated
Show resolved
Hide resolved
lambda/shared/src/main/scala/feral/lambda/events/ApplicationLoadBalancerRequestEvent.scala
Outdated
Show resolved
Hide resolved
lambda/shared/src/main/scala/feral/lambda/events/ApplicationLoadBalancerRequestEvent.scala
Outdated
Show resolved
Hide resolved
| private implicit val mapStringDecoder: Decoder[Map[CIString, String]] = | ||
| Decoder.decodeMap[CIString, String] | ||
| private implicit val mapListDecoder: Decoder[Map[CIString, List[String]]] = | ||
| Decoder.decodeMap[CIString, List[String]] |
There was a problem hiding this comment.
Hmm, are you sure these are necessary? I feel surprised/confused.
There was a problem hiding this comment.
They are not called directly in the code, they are used implicitly by Circe's Decoder when decoding JSON into an ApplicationLoadBalancerRequestEvent instance
There was a problem hiding this comment.
Can you please double-check? I tried removing them, and it looks like the code still compiles.
lambda/shared/src/test/scala/feral/lambda/events/ApplicationLoadBalancerRequestEventSuite.scala
Outdated
Show resolved
Hide resolved
|
@armanbilge : I've addressed the review comments, could you please take a look? |
| val encodedBody = | ||
| java.util.Base64.getEncoder.encodeToString("hello world".getBytes("UTF-8")) |
There was a problem hiding this comment.
scodec-bits has a nice syntax for this:
| val encodedBody = | |
| java.util.Base64.getEncoder.encodeToString("hello world".getBytes("UTF-8")) | |
| val encodedBody = asciiBytes"hello world".toBase64 |
| import org.typelevel.ci.CIString | ||
| import scodec.bits.ByteVector | ||
|
|
||
| sealed abstract class Elb { |
There was a problem hiding this comment.
Maybe this name is more clear :)
| sealed abstract class Elb { | |
| sealed abstract class ElasticLoadBalancer { |
| private implicit val mapStringDecoder: Decoder[Map[CIString, String]] = | ||
| Decoder.decodeMap[CIString, String] | ||
| private implicit val mapListDecoder: Decoder[Map[CIString, List[String]]] = | ||
| Decoder.decodeMap[CIString, List[String]] |
There was a problem hiding this comment.
Can you please double-check? I tried removing them, and it looks like the code still compiles.
| object Elb { | ||
| def apply(targetGroupArn: String): Elb = Impl(targetGroupArn) | ||
|
|
||
| implicit val decoder: Decoder[Elb] = Decoder.forProduct1("targetGroupArn")(Elb.apply) |
There was a problem hiding this comment.
| implicit val decoder: Decoder[Elb] = Decoder.forProduct1("targetGroupArn")(Elb.apply) | |
| private[events] implicit val decoder: Decoder[Elb] = Decoder.forProduct1("targetGroupArn")(Elb.apply) |
| def apply(elb: Elb): ApplicationLoadBalancerRequestContext = | ||
| Impl(elb) | ||
|
|
||
| implicit val decoder: Decoder[ApplicationLoadBalancerRequestContext] = |
There was a problem hiding this comment.
| implicit val decoder: Decoder[ApplicationLoadBalancerRequestContext] = | |
| private[events] implicit val decoder: Decoder[ApplicationLoadBalancerRequestContext] = |
| queryStringParameters = Some(Map("query" -> "1234ABCD")), | ||
| headers = Some( | ||
| Map( | ||
| org.typelevel.ci.CIString("accept") -> "text/html,application/xhtml+xml", |
There was a problem hiding this comment.
There is a special syntax to shorten this.
| org.typelevel.ci.CIString("accept") -> "text/html,application/xhtml+xml", | |
| ci"accept" -> "text/html,application/xhtml+xml", |
| assert( | ||
| decoded | ||
| .bodyDecoded | ||
| .exists(_ == ByteVector.encodeUtf8("hello world").getOrElse(ByteVector.empty))) |
There was a problem hiding this comment.
| assert( | |
| decoded | |
| .bodyDecoded | |
| .exists(_ == ByteVector.encodeUtf8("hello world").getOrElse(ByteVector.empty))) | |
| assertEquals(decoded.bodyDecoded, Some(asciiBytes"hello world")) |
Added Application Load Balancer Request Event as part of #48, all tests passed.
https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/aws-lambda/trigger/alb.d.ts