Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -96,10 +96,20 @@ lazy val lambda = crossProject(JSPlatform, JVMPlatform)
"org.scodec" %%% "scodec-bits" % "1.2.4",
"org.scalameta" %%% "munit-scalacheck" % munitVersion % Test,
"org.typelevel" %%% "munit-cats-effect" % munitCEVersion % Test,
"org.typelevel" %%% "scalacheck-effect" % scalacheckEffectVersion % Test,
"org.typelevel" %%% "scalacheck-effect-munit" % scalacheckEffectVersion % Test,
"io.circe" %%% "circe-literal" % circeVersion % Test
),
mimaBinaryIssueFilters ++= Seq(
ProblemFilters.exclude[IncompatibleResultTypeProblem]("feral.lambda.IOLambda.setupMemo")
ProblemFilters.exclude[ReversedMissingMethodProblem](
"feral.lambda.ClientContext.clientOption"
), // ClientContext is sealed
ProblemFilters.exclude[IncompatibleMethTypeProblem](
"feral.lambda.ClientContext#Impl.*"
), // ClientContext#Impl is private
ProblemFilters.exclude[IncompatibleResultTypeProblem](
"feral.lambda.ClientContext#Impl.*"
) // ClientContext#Impl is private
)
)
.settings(commonSettings)
Expand Down
16 changes: 9 additions & 7 deletions lambda/js/src/main/scala/feral/lambda/ContextPlatform.scala
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,15 @@ private[lambda] trait ContextCompanionPlatform {
},
context.clientContext.toOption.map { clientContext =>
ClientContext(
ClientContextClient(
clientContext.client.installationId,
clientContext.client.appTitle,
clientContext.client.appVersionName,
clientContext.client.appVersionCode,
clientContext.client.appPackageName
),
clientContext.client.toOption.map { x =>
ClientContextClient(
x.installationId,
x.appTitle,
x.appVersionName,
x.appVersionCode,
x.appPackageName
)
},
ClientContextEnv(
clientContext.env.platformVersion,
clientContext.env.platform,
Expand Down
2 changes: 1 addition & 1 deletion lambda/js/src/main/scala/feral/lambda/facade/Context.scala
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ private[lambda] trait CognitoIdentity extends js.Object {
}

private[lambda] trait ClientContext extends js.Object {
def client: ClientContextClient
def client: js.UndefOr[ClientContextClient]
def custom: js.UndefOr[js.Any]
def env: ClientContextEnv
}
Expand Down
224 changes: 224 additions & 0 deletions lambda/js/src/test/scala/feral/lambda/ContextPlatformSuite.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,224 @@
/*
* Copyright 2021 Typelevel
*
* 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 feral.lambda

import cats.effect._
import cats.syntax.all._
import io.circe.JsonObject
import io.circe.scalajs._
import munit.CatsEffectSuite
import munit.Compare
import munit.ScalaCheckEffectSuite
import org.scalacheck._
import org.scalacheck.effect.PropF

import scala.concurrent.duration._
import scala.scalajs._
import scala.scalajs.js.JSConverters._

class ContextPlatformSuite extends CatsEffectSuite with ScalaCheckEffectSuite {

private val genCognitoIdentity: Gen[facade.CognitoIdentity] =
for {
identityId <- Gen.alphaNumStr
identityPoolId <- Gen.alphaNumStr
} yield new facade.CognitoIdentity {
override def cognitoIdentityId: String = identityId
override def cognitoIdentityPoolId: String = identityPoolId

override def toString(): String =
s"CognitoIdentity(identityId=$identityId, identityPoolId=$identityPoolId)"
}

private val genClient: Gen[facade.ClientContextClient] =
for {
generatedInstallationId <- Gen.alphaNumStr
generatedAppTitle <- Gen.alphaNumStr
generatedAppVersionName <- Gen.alphaNumStr
generatedAppVersionCode <- Gen.alphaNumStr
generatedAppPackageName <- Gen.alphaNumStr
} yield new facade.ClientContextClient {
override def installationId: String = generatedInstallationId
override def appTitle: String = generatedAppTitle
override def appVersionName: String = generatedAppVersionName
override def appVersionCode: String = generatedAppVersionCode
override def appPackageName: String = generatedAppPackageName

override def toString(): String =
s"Client(installationId=$installationId, appTitle=$appTitle, appVersionName=$appVersionName, appVersionCode=$appVersionCode, appPackageName=$appPackageName)"
}

private val genMapStringStringEntry: Gen[(String, String)] =
for {
key <- Gen.alphaNumStr
value <- Gen.alphaNumStr
} yield (key, value)

private val genClientContext: Gen[facade.ClientContext] =
for {
generatedClient <- Gen.option(genClient).map(_.orUndefined)
generatedCustom <- Gen
.option(Gen.mapOf[String, String](genMapStringStringEntry).map { m =>
val dict = js.Dynamic.literal()
m.foreach { case (k, v) => dict.updateDynamic(k)(v) }
dict
})
.map(_.orUndefined)
generatedEnv <- { // TODO this should allow for undefined values but the types don't line up
for {
generatedPlatformVersion <- Gen.asciiPrintableStr
generatedPlatform <- Gen.asciiPrintableStr
generatedMake <- Gen.asciiPrintableStr
generatedModel <- Gen.asciiPrintableStr
generatedLocale <- Gen.asciiPrintableStr
} yield new facade.ClientContextEnv {
override def platformVersion: String = generatedPlatformVersion
override def platform: String = generatedPlatform
override def make: String = generatedMake
override def model: String = generatedModel
override def locale: String = generatedLocale
}
}
} yield new facade.ClientContext {
override def client: js.UndefOr[facade.ClientContextClient] = generatedClient
override def custom: js.UndefOr[js.Any] = generatedCustom
override def env: facade.ClientContextEnv = generatedEnv

override def toString(): String =
s"ClientContext(client=$client, env=$env, custom=$custom)"
}

private implicit val arbContext: Arbitrary[facade.Context] = Arbitrary {
for {
generatedFunctionName <- Gen.asciiPrintableStr
generatedFunctionVersion <- Gen.alphaNumStr
generatedInvokedFunctionArn <-
Gen.asciiPrintableStr // TODO this could be formatted as an ARN
generatedMemoryLimitInMB <- Gen.posNum[Int].map(_.toString)
generatedAwsRequestId <- Gen.alphaNumStr
generatedLogGroupName <- Gen.asciiPrintableStr
generatedLogStreamName <- Gen.asciiPrintableStr
generatedIdentity <- Gen.option(genCognitoIdentity).map(_.orUndefined)
generatedClientContext <- Gen.option(genClientContext).map(_.orUndefined)
generatedRemainingTimeInMillis <- Gen.posNum[Double]
} yield new facade.Context {
override def functionName: String = generatedFunctionName
override def functionVersion: String = generatedFunctionVersion
override def invokedFunctionArn: String = generatedInvokedFunctionArn
override def memoryLimitInMB: String = generatedMemoryLimitInMB
override def awsRequestId: String = generatedAwsRequestId
override def logGroupName: String = generatedLogGroupName
override def logStreamName: String = generatedLogStreamName
override def identity: js.UndefOr[facade.CognitoIdentity] = generatedIdentity
override def clientContext: js.UndefOr[facade.ClientContext] = generatedClientContext
override def getRemainingTimeInMillis(): Double = generatedRemainingTimeInMillis

override def toString(): String =
s"Context(functionName=$functionName, functionVersion=$functionVersion, invokedFunctionArn=$invokedFunctionArn, memoryLimitInMB=$memoryLimitInMB, awsRequestId=$awsRequestId, logGroupName=$logGroupName, logStreamName=$logStreamName, identity=$identity, clientContext=$clientContext, remainingTimeInMillis=${getRemainingTimeInMillis()}"
}
}

private implicit def optionCompare[A, B](
Copy link
Member

Choose a reason for hiding this comment

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

Seems like something that should be upstreamed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, that's probably right. I'll do that in a separate PR.

implicit C: Compare[A, B]): Compare[Option[A], Option[B]] =
new Compare[Option[A], Option[B]] {
override def isEqual(obtained: Option[A], expected: Option[B]): Boolean =
(obtained, expected).mapN(C.isEqual).getOrElse(obtained.isEmpty && expected.isEmpty)
}

private implicit val compareCognitoIdentity
: Compare[CognitoIdentity, facade.CognitoIdentity] =
new Compare[CognitoIdentity, facade.CognitoIdentity] {
override def isEqual(
obtained: CognitoIdentity,
expected: facade.CognitoIdentity): Boolean =
obtained.identityId == expected.cognitoIdentityId && obtained.identityPoolId == expected.cognitoIdentityPoolId
}

private implicit val compareClientContextClient
: Compare[ClientContextClient, facade.ClientContextClient] =
new Compare[ClientContextClient, facade.ClientContextClient] {
override def isEqual(
obtained: ClientContextClient,
expected: facade.ClientContextClient): Boolean =
obtained.installationId == expected.installationId &&
obtained.appTitle == expected.appTitle &&
obtained.appVersionName == expected.appVersionName &&
obtained.appVersionCode == expected.appVersionCode &&
obtained.appPackageName == expected.appPackageName
}
Copy link
Member

Choose a reason for hiding this comment

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

This feels like a lot of boilerplate (which in theory could also have implementation mistakes), is it really worth it? 😅 versus converting one or two examples and just relying on case class equality.

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 feels like a lot of boilerplate

Yeah, agreed. 😅 But…

Unless I'm misunderstanding you, we don't have case class equality available here, since the case classes are all private for bincompat reasons.

I tried adding extends Product to our facade traits and then doing something generic, but the names don't match between the facade and our traits, and anyway the changes started to spiral quite a bit.

I tried to encode all the nullable values into the ScalaCheck generators so that we'd see failures if we incorrectly did things like clientContext.client.installationId. I'm open to suggestions, but obviously the previous amount of coverage didn't catch this kind of issue.

Copy link
Member

@armanbilge armanbilge Oct 16, 2025

Choose a reason for hiding this comment

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

Unless I'm misunderstanding you, we don't have case class equality available here, since the case classes are all private for bincompat reasons.

Well, we always have Object#equals available, and because the implementations are (private) case classes we know that there is a reasonable and correct implementation of equals. So I meant we can just rely on that.

but obviously the previous amount of coverage didn't catch this kind of issue.

No it didn't, but this new amount of coverage also wouldn't catch this kind of issue. Where "this kind of issue" is that the documentation we relied on when originally defining/testing these datatypes did not mention the possibility of nullability/undefinedness.

I tried to encode all the nullable values into the ScalaCheck generators so that we'd see failures

The use of generators lets us capture the combinatorial explosion of values that could be simultaneously nullable (to the best of our knowledge), but I don't feel convinced that the value of this kind of coverage carries its weight in generator boilerplate.


Meanwhile, IMO a more effective kind of coverage is to simply incorporate the linked example from #502 into our test suite.


private implicit val compareClientContextEnv
: Compare[ClientContextEnv, facade.ClientContextEnv] =
new Compare[ClientContextEnv, facade.ClientContextEnv] {
override def isEqual(
obtained: ClientContextEnv,
expected: facade.ClientContextEnv): Boolean =
obtained.platformVersion == expected.platformVersion &&
obtained.platform == expected.platform &&
obtained.make == expected.make &&
obtained.model == expected.model &&
obtained.locale == expected.locale
}

private implicit val compareJsonObjectWithUndefOrAny
: Compare[JsonObject, js.UndefOr[js.Any]] = new Compare[JsonObject, js.UndefOr[js.Any]] {
override def isEqual(obtained: JsonObject, expected: js.UndefOr[js.Any]): Boolean =
expected
.toOption
.flatMap(convertJsToJson(_).toOption)
.flatMap(_.asObject)
.map(_.equals(obtained))
.getOrElse(obtained.isEmpty && expected.isEmpty)
}

private implicit val compareClientContext: Compare[ClientContext, facade.ClientContext] =
new Compare[ClientContext, facade.ClientContext] {
override def isEqual(obtained: ClientContext, expected: facade.ClientContext): Boolean =
(obtained.clientOption, expected.client.toOption)
.mapN(implicitly[Compare[ClientContextClient, facade.ClientContextClient]].isEqual)
.getOrElse(obtained.clientOption.isEmpty && expected.client.isEmpty) &&
implicitly[Compare[ClientContextEnv, facade.ClientContextEnv]]
.isEqual(obtained.env, expected.env) &&
implicitly[Compare[JsonObject, js.UndefOr[js.Any]]]
.isEqual(obtained.custom, expected.custom)
}

test("JS Context can be decoded") {
Prop.forAll { (context: facade.Context) =>
val output: Context[IO] = Context.fromJS[IO](context)

assertEquals(output.functionName, context.functionName)
assertEquals(output.functionVersion, context.functionVersion)
assertEquals(output.invokedFunctionArn, context.invokedFunctionArn)
assertEquals(output.memoryLimitInMB.toString, context.memoryLimitInMB)
assertEquals(output.awsRequestId, context.awsRequestId)
assertEquals(output.logGroupName, context.logGroupName)
assertEquals(output.logStreamName, context.logStreamName)
assertEquals(output.identity, context.identity.toOption)
assertEquals(output.clientContext, context.clientContext.toOption)
}
}

test("remainingTime") {
PropF.forAllF { (context: facade.Context) =>
val output: Context[IO] = Context.fromJS[IO](context)

assertIO(output.remainingTime, context.getRemainingTimeInMillis().millis)
}
}

}
63 changes: 42 additions & 21 deletions lambda/jvm/src/main/scala/feral/lambda/ContextPlatform.scala
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,14 @@ package feral.lambda

import cats.effect.Sync
import com.amazonaws.services.lambda.runtime
import io.circe.Json
import io.circe.JsonObject
import io.circe.jawn.parse
import io.circe.syntax._

import java.util.Collections
import scala.concurrent.duration._
import scala.jdk.CollectionConverters._
import scala.util.chaining._

private[lambda] trait ContextCompanionPlatform {

Expand All @@ -39,26 +42,44 @@ private[lambda] trait ContextCompanionPlatform {
CognitoIdentity(identity.getIdentityId(), identity.getIdentityPoolId())
},
Option(context.getClientContext()).map { clientContext =>
ClientContext(
ClientContextClient(
clientContext.getClient().getInstallationId(),
clientContext.getClient().getAppTitle(),
clientContext.getClient().getAppVersionName(),
clientContext.getClient().getAppVersionCode(),
clientContext.getClient().getAppPackageName()
),
ClientContextEnv(
clientContext.getEnvironment().get("platformVersion"),
clientContext.getEnvironment().get("platform"),
clientContext.getEnvironment().get("make"),
clientContext.getEnvironment().get("model"),
clientContext.getEnvironment().get("locale")
),
JsonObject.fromIterable(clientContext.getCustom().asScala.view.flatMap {
case (k, v) =>
parse(v).toOption.map(k -> _)
})
)
val env =
Option(clientContext.getEnvironment())
.map(_.asScala)
.getOrElse(Map.empty[String, String])
.pipe { env =>
ClientContextEnv(
env.get("platformVersion").orNull,
env.get("platform").orNull,
env.get("make").orNull,
env.get("model").orNull,
env.get("locale").orNull
)
}

val maybeClient =
for {
client <- Option(clientContext.getClient())
} yield ClientContextClient(
client.getInstallationId(),
client.getAppTitle(),
client.getAppVersionName(),
client.getAppVersionCode(),
client.getAppPackageName()
)

val custom =
JsonObject.fromIterable {
Option(clientContext.getCustom())
.getOrElse(Collections.emptyMap[String, String]())
.asScala
.view
.mapValues {
case null => Json.Null
case other => other.asJson
}
}

ClientContext(maybeClient, env, custom)
},
Sync[F].delay(context.getRemainingTimeInMillis().millis)
)
Expand Down
Loading