diff --git a/build.gradle.kts b/build.gradle.kts index 75b8a62c6a1..aff328c67b9 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -133,7 +133,7 @@ apiValidation { "exemplar", "exemplarchat", "detektive", - "misk-schema-migrator-gradle-plugin" + "misk-schema-migrator-gradle-plugin", ) ignoredProjects.addAll(subprojects.map { it.name }.filter { it in ignorable }) additionalSourceSets.add("testFixtures") @@ -191,13 +191,13 @@ val hibernateProjects = listOf( "misk-jdbc-testing", "misk-hibernate-testing", "misk-rate-limiting-bucket4j-mysql", - "misk-sqldelight" + "misk-sqldelight", ) val redisProjects = listOf( "misk-redis", "misk-redis-lettuce", - "misk-rate-limiting-bucket4j-redis" + "misk-rate-limiting-bucket4j-redis", ) val detektConfig = file("detekt.yaml") @@ -275,6 +275,7 @@ subprojects { add("api", platform(rootProject.libs.jacksonBom)) add("api", platform(rootProject.libs.jerseyBom)) add("api", platform(rootProject.libs.jettyBom)) + add("api", platform(rootProject.libs.jettyEe9Bom)) add("api", platform(rootProject.libs.kotlinBom)) add("api", platform(rootProject.libs.nettyBom)) add("api", platform(rootProject.libs.prometheusClientBom)) @@ -314,7 +315,7 @@ subprojects { "dd.civisibility.git.upload.enabled" to false, "dd.integration.opentracing.enabled" to true, "dd.instrumentation.telemetry.enabled" to false, - ) + ), ) develocity.testRetry { maxRetries.set(1) @@ -380,7 +381,7 @@ subprojects { if (name in configurationNames) { attributes.attribute( Usage.USAGE_ATTRIBUTE, - this@subprojects.objects.named(Usage::class, Usage.JAVA_RUNTIME) + this@subprojects.objects.named(Usage::class, Usage.JAVA_RUNTIME), ) } @@ -485,7 +486,7 @@ abstract class StartRedisTask @Inject constructor( val portIsOccupied = try { Socket("localhost", redisPort).close() true - } catch (e: IOException) { + } catch (_: IOException) { false } if (portIsOccupied) { @@ -502,7 +503,7 @@ abstract class StartRedisTask @Inject constructor( "-p", "$redisPort:6379", redisImage, "redis-server", - "--loglevel debug" + "--loglevel debug", ) execOperations.exec { workingDir(rootDir.get().asFile) @@ -536,7 +537,7 @@ abstract class StartRedisClusterTask @Inject constructor( val portIsOccupied = try { Socket("localhost", redisSeedPort).close() true - } catch (e: IOException) { + } catch (_: IOException) { false } if (portIsOccupied) { @@ -555,26 +556,28 @@ abstract class StartRedisClusterTask @Inject constructor( "-e", "MASTERS=3", "-e", "SLAVES_PER_MASTER=1", "-p", "7000-7005:7000-7005", - redisImage + redisImage, ) execOperations.exec { workingDir(rootDir.get().asFile) commandLine(*dockerArguments) } - waitForRedisCluster(redisContainerName,redisSeedPort) + waitForRedisCluster(redisContainerName, redisSeedPort) logger.info("Started Redis Cluster docker image $redisImage on port $redisSeedPort") } - private fun waitForRedisCluster(containerName:String, port:Int){ + private fun waitForRedisCluster(containerName: String, port: Int) { println("Waiting for Redis cluster to become available...") val deadline = System.currentTimeMillis() + 60.seconds.inWholeMilliseconds fun clusterReady(): Boolean { try { - val process = ProcessBuilder("docker", "exec", containerName, - "redis-cli", "-c", "-p", port.toString(), "cluster", "info") + val process = ProcessBuilder( + "docker", "exec", containerName, + "redis-cli", "-c", "-p", port.toString(), "cluster", "info", + ) .redirectErrorStream(true) .start() @@ -582,7 +585,7 @@ abstract class StartRedisClusterTask @Inject constructor( process.waitFor(5, TimeUnit.SECONDS) return "cluster_state:ok" in output && "slots_assigned:16384" in output - } catch (e: Exception) { + } catch (_: Exception) { return false } } diff --git a/detektive/src/main/kotlin/cash/detektive/javacompat/AnnotatePublicApisWithJvmOverloads.kt b/detektive/src/main/kotlin/cash/detektive/javacompat/AnnotatePublicApisWithJvmOverloads.kt index 1560266d905..968d87b56dd 100644 --- a/detektive/src/main/kotlin/cash/detektive/javacompat/AnnotatePublicApisWithJvmOverloads.kt +++ b/detektive/src/main/kotlin/cash/detektive/javacompat/AnnotatePublicApisWithJvmOverloads.kt @@ -76,7 +76,10 @@ class AnnotatePublicApisWithJvmOverloads(config: Config) : Rule(config) { ) element.addAfter(KtPsiFactory.contextual(element.parent, markGenerated = true).createWhiteSpace(), null) } else if (elementType == ElementType.FUNCTION) { - annotation.addBefore(KtPsiFactory.contextual(element.parent, markGenerated = true).createNewLine(), null) + annotation.addBefore( + KtPsiFactory.contextual(element.parent, markGenerated = true).createNewLine(), + null, + ) } } } diff --git a/detektive/src/test/kotlin/cash/detektive/javacompat/AnnotatePublicApisWithJvmOverloadsTest.kt b/detektive/src/test/kotlin/cash/detektive/javacompat/AnnotatePublicApisWithJvmOverloadsTest.kt index 0da8ce9e5b8..1ebbe237a9e 100644 --- a/detektive/src/test/kotlin/cash/detektive/javacompat/AnnotatePublicApisWithJvmOverloadsTest.kt +++ b/detektive/src/test/kotlin/cash/detektive/javacompat/AnnotatePublicApisWithJvmOverloadsTest.kt @@ -1,6 +1,7 @@ package cash.detektive.javacompat import cash.detektive.javacompat.AnnotatePublicApisWithJvmOverloads.ElementType +import io.github.detekt.parser.DetektPomModel import io.github.detekt.test.utils.compileForTest import io.gitlab.arturbosch.detekt.api.Config import io.gitlab.arturbosch.detekt.api.Severity @@ -12,12 +13,11 @@ import io.gitlab.arturbosch.detekt.test.getContextForPaths import java.io.File import org.assertj.core.api.Assertions.assertThat import org.jetbrains.kotlin.cli.jvm.compiler.KotlinCoreEnvironment -import org.jetbrains.kotlin.com.intellij.mock.MockProject import org.jetbrains.kotlin.com.intellij.mock.MockApplication +import org.jetbrains.kotlin.com.intellij.mock.MockProject import org.jetbrains.kotlin.com.intellij.openapi.diagnostic.Logger import org.jetbrains.kotlin.com.intellij.openapi.util.Disposer import org.jetbrains.kotlin.com.intellij.pom.PomModel -import io.github.detekt.parser.DetektPomModel import org.jetbrains.kotlin.config.CompilerConfigurationKey import org.jetbrains.kotlin.config.languageVersionSettings import org.jetbrains.kotlin.resolve.calls.smartcasts.DataFlowValueFactoryImpl diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 4fc948e84ff..fb55b2b3803 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -12,6 +12,7 @@ googleHttp = "2.0.0" guava = "33.5.0-jre" hoplite = "2.7.5" jackson = "2.21.2" +jetty = "12.0.23" jooq = "3.19.29" junit = "5.14.2" kotest = "6.0.7" @@ -136,20 +137,21 @@ jerseyBom = { module = "org.glassfish.jersey:jersey-bom", version = "3.1.11" } jetbrainsAnnotations = { module = "org.jetbrains:annotations", version = "26.0.2-1" } jettyAlpnServer = { module = "org.eclipse.jetty:jetty-alpn-server" } jettyAlpnServerJava = { module = "org.eclipse.jetty:jetty-alpn-java-server" } -jettyBom = { module = "org.eclipse.jetty:jetty-bom", version = "11.0.26" } +jettyBom = { module = "org.eclipse.jetty:jetty-bom", version.ref = "jetty" } +jettyEe9Bom = { module = "org.eclipse.jetty.ee9:jetty-ee9-bom", version.ref = "jetty" } +jettyEe9Nested = { module = "org.eclipse.jetty.ee9:jetty-ee9-nested" } jettyHttp = { module = "org.eclipse.jetty:jetty-http" } -jettyHttp2 = { module = "org.eclipse.jetty.http2:http2-server" } -jettyHttp2Common = { module = "org.eclipse.jetty.http2:http2-common" } +jettyHttp2 = { module = "org.eclipse.jetty.http2:jetty-http2-server", version.ref = "jetty" } +jettyHttp2Common = { module = "org.eclipse.jetty.http2:jetty-http2-common", version.ref = "jetty" } jettyIo = { module = "org.eclipse.jetty:jetty-io" } jettyServer = { module = "org.eclipse.jetty:jetty-server" } -jettyServlet = { module = "org.eclipse.jetty:jetty-servlet" } +jettyServlet = { module = "org.eclipse.jetty.ee9:jetty-ee9-servlet" } jettyServletApi = { module = "org.eclipse.jetty.toolchain:jetty-jakarta-servlet-api", version = "5.0.2" } -jettyServlets = { module = "org.eclipse.jetty:jetty-servlets" } +jettyServlets = { module = "org.eclipse.jetty.ee9:jetty-ee9-servlets" } jettyUds = { module = "org.eclipse.jetty:jetty-unixdomain-server" } -jettyUnixSocket = { module = "org.eclipse.jetty:jetty-unixsocket-server" } jettyUtil = { module = "org.eclipse.jetty:jetty-util" } -jettyWebsocketApi = { module = "org.eclipse.jetty.websocket:websocket-jetty-api" } -jettyWebsocketServer = { module = "org.eclipse.jetty.websocket:websocket-jetty-server" } +jettyWebsocketApiEE9 = { module = "org.eclipse.jetty.ee9.websocket:jetty-ee9-websocket-jetty-api" } +jettyWebsocketServerEE9 = { module = "org.eclipse.jetty.ee9.websocket:jetty-ee9-websocket-jetty-server" } jnrUnixsocket = { module = "com.github.jnr:jnr-unixsocket", version = "0.38.24" } jooq = { module = "org.jooq:jooq" } jooqBom = { module = "org.jooq:jooq-bom", version.ref = "jooq" } diff --git a/misk-action-scopes/src/test/kotlin/misk/scope/ActionScopePropagationTest.kt b/misk-action-scopes/src/test/kotlin/misk/scope/ActionScopePropagationTest.kt index d7c44e171e0..21f1f74f181 100644 --- a/misk-action-scopes/src/test/kotlin/misk/scope/ActionScopePropagationTest.kt +++ b/misk-action-scopes/src/test/kotlin/misk/scope/ActionScopePropagationTest.kt @@ -31,9 +31,7 @@ internal class ActionScopePropagationTest { val seedData: Map, Any> = mapOf(keyOf(Names.named("from-seed")) to "my seed data") - val callable = scope.create(seedData).inScope { - scope.propagate(Callable { tester.fooValue() }) - } + val callable = scope.create(seedData).inScope { scope.propagate(Callable { tester.fooValue() }) } scope.create(seedData).inScope { // Submit to same thread after we've already entered the scope @@ -50,9 +48,7 @@ internal class ActionScopePropagationTest { val seedData: Map, Any> = mapOf(keyOf(Names.named("from-seed")) to "my seed data") - val callable = scope.create(seedData).inScope { - scope.propagate(Callable { tester.fooValue() }) - } + val callable = scope.create(seedData).inScope { scope.propagate(Callable { tester.fooValue() }) } // Submit to other thread after we've exited the scope val result = singleThreadExecutor.submit(callable).get() @@ -69,9 +65,7 @@ internal class ActionScopePropagationTest { // Propagate on the the KCallable directly val f: KFunction = tester::fooValue - val callable = scope.create(seedData).inScope { - scope.propagate(f) - } + val callable = scope.create(seedData).inScope { scope.propagate(f) } scope.create(seedData).inScope { // Submit to same thread after we've already entered the scope @@ -90,9 +84,7 @@ internal class ActionScopePropagationTest { // Propagate on the the KCallable directly val f: KFunction = tester::fooValue - val callable = scope.create(seedData).inScope { - scope.propagate(f) - } + val callable = scope.create(seedData).inScope { scope.propagate(f) } // Submit to other thread after we've exited the scope val result = singleThreadExecutor.submit(Callable { callable.call() }).get() @@ -108,9 +100,7 @@ internal class ActionScopePropagationTest { val seedData: Map, Any> = mapOf(keyOf(Names.named("from-seed")) to "my seed data") // Propagate on a lambda directly - val function = scope.create(seedData).inScope { - scope.propagate { tester.fooValue() } - } + val function = scope.create(seedData).inScope { scope.propagate { tester.fooValue() } } scope.create(seedData).inScope { // Submit to same thread after we've already entered the scope @@ -128,9 +118,7 @@ internal class ActionScopePropagationTest { val seedData: Map, Any> = mapOf(keyOf(Names.named("from-seed")) to "my seed data") // Propagate on a lambda directly - val function = scope.create(seedData).inScope { - scope.propagate { tester.fooValue() } - } + val function = scope.create(seedData).inScope { scope.propagate { tester.fooValue() } } // Submit to other thread after we've exited the scope val result = singleThreadExecutor.submit(Callable { function() }).get() diff --git a/misk-action-scopes/src/test/kotlin/misk/scope/ActionScopedTest.kt b/misk-action-scopes/src/test/kotlin/misk/scope/ActionScopedTest.kt index cd21859b68e..85d25048176 100644 --- a/misk-action-scopes/src/test/kotlin/misk/scope/ActionScopedTest.kt +++ b/misk-action-scopes/src/test/kotlin/misk/scope/ActionScopedTest.kt @@ -175,9 +175,7 @@ internal class ActionScopedTest { val seedData: Map, Any> = mapOf(keyOf(Names.named("from-seed")) to "seed-value") scope.create(seedData).inScope { - runBlocking(scope.asContextElement()) { - assertThat(foo.get()).isEqualTo("seed-value and bar and foo!") - } + runBlocking(scope.asContextElement()) { assertThat(foo.get()).isEqualTo("seed-value and bar and foo!") } } } @@ -218,14 +216,12 @@ internal class ActionScopedTest { val instance = scope.snapshotActionScopeInstance() thread { - try { - instance.inScope { - assertThat(foo.get()).isEqualTo("seed-value and bar and foo!") + try { + instance.inScope { assertThat(foo.get()).isEqualTo("seed-value and bar and foo!") } + } catch (t: Throwable) { + thrown = t } - } catch (t: Throwable) { - thrown = t } - } .join() assertThat(thrown).isNull() } @@ -260,14 +256,12 @@ internal class ActionScopedTest { val instance = scope.snapshotActionScopeInstance() thread { - try { - instance.inScope { - assertThat(foo.get()).isEqualTo("seed-value and bar and foo!") + try { + instance.inScope { assertThat(foo.get()).isEqualTo("seed-value and bar and foo!") } + } catch (t: Throwable) { + thrown = t } - } catch (t: Throwable) { - thrown = t } - } .join() assertThat(thrown).isNull() } diff --git a/misk-action-scopes/src/test/kotlin/misk/scope/coroutine/ActionScopedCoroutineTest.kt b/misk-action-scopes/src/test/kotlin/misk/scope/coroutine/ActionScopedCoroutineTest.kt index 201f98aebc6..f2b9c43131b 100644 --- a/misk-action-scopes/src/test/kotlin/misk/scope/coroutine/ActionScopedCoroutineTest.kt +++ b/misk-action-scopes/src/test/kotlin/misk/scope/coroutine/ActionScopedCoroutineTest.kt @@ -30,11 +30,7 @@ internal class ActionScopedCoroutineTest { val seedData: Map, Any> = mapOf(keyOf(Names.named("from-seed")) to "my seed data") - val value = scope.create(seedData).inScope { - scope.runBlocking { - tester.fooValue() - } - } + val value = scope.create(seedData).inScope { scope.runBlocking { tester.fooValue() } } assertThat(value).isEqualTo("my seed data and bar and foo!") } @@ -48,11 +44,7 @@ internal class ActionScopedCoroutineTest { val seedData: Map, Any> = mapOf(keyOf(Names.named("from-seed")) to "my seed data") - val value = scope.create(seedData).inScope { - scope.runBlocking(Dispatchers.IO) { - tester.fooValue() - } - } + val value = scope.create(seedData).inScope { scope.runBlocking(Dispatchers.IO) { tester.fooValue() } } assertThat(value).isEqualTo("my seed data and bar and foo!") } diff --git a/misk-action-scopes/src/test/kotlin/misk/scope/executor/ActionScopedExecutorServiceTest.kt b/misk-action-scopes/src/test/kotlin/misk/scope/executor/ActionScopedExecutorServiceTest.kt index bd63c3b3d98..7d8f20b4855 100644 --- a/misk-action-scopes/src/test/kotlin/misk/scope/executor/ActionScopedExecutorServiceTest.kt +++ b/misk-action-scopes/src/test/kotlin/misk/scope/executor/ActionScopedExecutorServiceTest.kt @@ -36,9 +36,7 @@ internal class ActionScopedExecutorServiceTest { val seedData: Map, Any> = mapOf(keyOf(Names.named("from-seed")) to "my seed data") - val future = scope.create(seedData).inScope { - executor.submit(Callable { tester.fooValue() }) - } + val future = scope.create(seedData).inScope { executor.submit(Callable { tester.fooValue() }) } assertThat(future.get()).isEqualTo("my seed data and bar and foo!") } diff --git a/misk-actions/src/main/kotlin/misk/MiskCaller.kt b/misk-actions/src/main/kotlin/misk/MiskCaller.kt index cc84f5548a0..ab949135315 100644 --- a/misk-actions/src/main/kotlin/misk/MiskCaller.kt +++ b/misk-actions/src/main/kotlin/misk/MiskCaller.kt @@ -16,8 +16,8 @@ constructor( /** * When true, this caller is authorized for all endpoints regardless of required capabilities, services, or users. * - * This is intended for use in controlled environments (e.g., staging playpens) where a trusted caller needs - * blanket access for testing. It should never be set in production. + * This is intended for use in controlled environments (e.g., staging playpens) where a trusted caller needs blanket + * access for testing. It should never be set in production. */ val allowAll: Boolean = false, ) { diff --git a/misk-actions/src/main/kotlin/misk/web/Http.kt b/misk-actions/src/main/kotlin/misk/web/Http.kt index 34f66f0d72b..9e5e15ca7a9 100644 --- a/misk-actions/src/main/kotlin/misk/web/Http.kt +++ b/misk-actions/src/main/kotlin/misk/web/Http.kt @@ -81,9 +81,7 @@ annotation class ResponseContentType(vararg val value: String) * } * ``` */ -@Retention(AnnotationRetention.RUNTIME) -@Target(AnnotationTarget.FUNCTION) -annotation class EnableUnframedRequests +@Retention(AnnotationRetention.RUNTIME) @Target(AnnotationTarget.FUNCTION) annotation class EnableUnframedRequests /** * When the service is overloaded Misk will intervene and reject calls by returning "HTTP 503 Service Unavailable". We diff --git a/misk-admin/src/main/kotlin/misk/web/dashboard/AdminDashboardModule.kt b/misk-admin/src/main/kotlin/misk/web/dashboard/AdminDashboardModule.kt index 9084b69d7f8..167c20fc283 100644 --- a/misk-admin/src/main/kotlin/misk/web/dashboard/AdminDashboardModule.kt +++ b/misk-admin/src/main/kotlin/misk/web/dashboard/AdminDashboardModule.kt @@ -51,9 +51,8 @@ constructor( } // Module that allows testing/development environments to bind up the admin dashboard -class AdminDashboardTestingModule @JvmOverloads constructor( - private val enableTurbo: Boolean = true, -) : KAbstractModule() { +class AdminDashboardTestingModule @JvmOverloads constructor(private val enableTurbo: Boolean = true) : + KAbstractModule() { override fun configure() { // Set dummy values for access, these shouldn't matter, // as test environments should prefer to use the FakeCallerAuthenticator. diff --git a/misk-admin/src/main/kotlin/misk/web/dashboard/BaseDashboardModule.kt b/misk-admin/src/main/kotlin/misk/web/dashboard/BaseDashboardModule.kt index 2f97276fdb9..99832076bfd 100644 --- a/misk-admin/src/main/kotlin/misk/web/dashboard/BaseDashboardModule.kt +++ b/misk-admin/src/main/kotlin/misk/web/dashboard/BaseDashboardModule.kt @@ -17,7 +17,9 @@ import misk.web.v2.DashboardV2RedirectAction * - `admin-dashboard` tab which loads all other tabs and provides navbar, menu links, auth * - `@misk` packages used by Misk-Web tabs from window to provide faster tab loads */ -class BaseDashboardModule @JvmOverloads constructor( +class BaseDashboardModule +@JvmOverloads +constructor( private val isDevelopment: Boolean, private val layoutConfig: DashboardLayoutConfig = DashboardLayoutConfig(), ) : KAbstractModule() { diff --git a/misk-admin/src/main/kotlin/misk/web/v2/DashboardPageLayout.kt b/misk-admin/src/main/kotlin/misk/web/v2/DashboardPageLayout.kt index 997897a9e6e..b903f8e83e3 100644 --- a/misk-admin/src/main/kotlin/misk/web/v2/DashboardPageLayout.kt +++ b/misk-admin/src/main/kotlin/misk/web/v2/DashboardPageLayout.kt @@ -19,9 +19,7 @@ import misk.web.v2.DashboardIndexAction.Companion.titlecase import wisp.deployment.Deployment /** Configuration for dashboard page layout behavior. */ -data class DashboardLayoutConfig @JvmOverloads constructor( - val enableTurbo: Boolean = true, -) +data class DashboardLayoutConfig @JvmOverloads constructor(val enableTurbo: Boolean = true) /** * Builds dashboard UI for index homepage. diff --git a/misk-admin/src/test/kotlin/misk/web/metadata/MetadataTestingModule.kt b/misk-admin/src/test/kotlin/misk/web/metadata/MetadataTestingModule.kt index 843d101e198..cd9bc36c5c7 100644 --- a/misk-admin/src/test/kotlin/misk/web/metadata/MetadataTestingModule.kt +++ b/misk-admin/src/test/kotlin/misk/web/metadata/MetadataTestingModule.kt @@ -21,9 +21,7 @@ import misk.web.metadata.all.AllMetadataAccess import misk.web.metadata.all.AllMetadataModule // Common test module used to be able to test admin dashboard WebActions -class MetadataTestingModule( - private val enableTurbo: Boolean = true, -) : KAbstractModule() { +class MetadataTestingModule(private val enableTurbo: Boolean = true) : KAbstractModule() { override fun configure() { install(TestWebActionModule()) install(AdminDashboardTestingModule(enableTurbo = enableTurbo)) diff --git a/misk-admin/src/test/kotlin/misk/web/v2/DashboardPageLayoutTest.kt b/misk-admin/src/test/kotlin/misk/web/v2/DashboardPageLayoutTest.kt index 44d8158f3d3..4874b72a5be 100644 --- a/misk-admin/src/test/kotlin/misk/web/v2/DashboardPageLayoutTest.kt +++ b/misk-admin/src/test/kotlin/misk/web/v2/DashboardPageLayoutTest.kt @@ -14,7 +14,6 @@ import misk.web.FakeHttpCall import misk.web.HttpCall import misk.web.metadata.MetadataTestingModule import okhttp3.HttpUrl.Companion.toHttpUrl -import org.junit.jupiter.api.Nested import org.junit.jupiter.api.Test @MiskTest diff --git a/misk-aws/src/main/kotlin/misk/jobqueue/sqs/AwsSqsBatchJobHandlerModule.kt b/misk-aws/src/main/kotlin/misk/jobqueue/sqs/AwsSqsBatchJobHandlerModule.kt index fce47744537..a82b4a1bd7f 100644 --- a/misk-aws/src/main/kotlin/misk/jobqueue/sqs/AwsSqsBatchJobHandlerModule.kt +++ b/misk-aws/src/main/kotlin/misk/jobqueue/sqs/AwsSqsBatchJobHandlerModule.kt @@ -16,8 +16,9 @@ import misk.jobqueue.QueueName * queue. */ @Deprecated( - message = "AWS SDK v1 SQS jobqueue is deprecated. Use the AWS SDK v2 SQS jobqueue in " + - "misk-aws2-sqs (misk.aws2.sqs.jobqueue) instead." + message = + "AWS SDK v1 SQS jobqueue is deprecated. Use the AWS SDK v2 SQS jobqueue in " + + "misk-aws2-sqs (misk.aws2.sqs.jobqueue) instead." ) class AwsSqsBatchJobHandlerModule private constructor( @@ -35,11 +36,7 @@ private constructor( } install(DefaultAsyncSwitchModule()) - install( - ServiceModule() - .dependsOn(dependsOn) - .dependsOn() - ) + install(ServiceModule().dependsOn(dependsOn).dependsOn()) } companion object { diff --git a/misk-aws/src/main/kotlin/misk/jobqueue/sqs/AwsSqsJobHandlerModule.kt b/misk-aws/src/main/kotlin/misk/jobqueue/sqs/AwsSqsJobHandlerModule.kt index 89bfa479414..88244827cc1 100644 --- a/misk-aws/src/main/kotlin/misk/jobqueue/sqs/AwsSqsJobHandlerModule.kt +++ b/misk-aws/src/main/kotlin/misk/jobqueue/sqs/AwsSqsJobHandlerModule.kt @@ -16,8 +16,9 @@ import misk.jobqueue.QueueName * queue. */ @Deprecated( - message = "AWS SDK v1 SQS jobqueue is deprecated. Use the AWS SDK v2 SQS jobqueue in " + - "misk-aws2-sqs (misk.aws2.sqs.jobqueue.SqsJobHandlerModule) instead." + message = + "AWS SDK v1 SQS jobqueue is deprecated. Use the AWS SDK v2 SQS jobqueue in " + + "misk-aws2-sqs (misk.aws2.sqs.jobqueue.SqsJobHandlerModule) instead." ) class AwsSqsJobHandlerModule private constructor( @@ -35,11 +36,7 @@ private constructor( } install(DefaultAsyncSwitchModule()) - install( - ServiceModule() - .dependsOn(dependsOn) - .dependsOn() - ) + install(ServiceModule().dependsOn(dependsOn).dependsOn()) } companion object { diff --git a/misk-aws/src/main/kotlin/misk/jobqueue/sqs/AwsSqsJobQueueConfig.kt b/misk-aws/src/main/kotlin/misk/jobqueue/sqs/AwsSqsJobQueueConfig.kt index 9001967ef28..d52010ea347 100644 --- a/misk-aws/src/main/kotlin/misk/jobqueue/sqs/AwsSqsJobQueueConfig.kt +++ b/misk-aws/src/main/kotlin/misk/jobqueue/sqs/AwsSqsJobQueueConfig.kt @@ -8,8 +8,9 @@ import misk.tasks.RepeatedTaskQueueConfig /** [AwsSqsJobQueueConfig] is the configuration for job queueing backed by Amazon's Simple Queuing Service */ @Deprecated( - message = "AWS SDK v1 SQS jobqueue is deprecated. Use misk.aws2.sqs.jobqueue.config.SqsConfig " + - "with misk.aws2.sqs.jobqueue.SqsJobQueueModule instead." + message = + "AWS SDK v1 SQS jobqueue is deprecated. Use misk.aws2.sqs.jobqueue.config.SqsConfig " + + "with misk.aws2.sqs.jobqueue.SqsJobQueueModule instead." ) class AwsSqsJobQueueConfig @JvmOverloads @@ -73,8 +74,9 @@ constructor( * The [AwsSqsJobReceiverPolicy] gives two options for how consumers are created based on the flags. */ @Deprecated( - message = "AWS SDK v1 SQS jobqueue is deprecated. Use the AWS SDK v2 SQS jobqueue in " + - "misk-aws2-sqs (misk.aws2.sqs.jobqueue) instead." + message = + "AWS SDK v1 SQS jobqueue is deprecated. Use the AWS SDK v2 SQS jobqueue in " + + "misk-aws2-sqs (misk.aws2.sqs.jobqueue) instead." ) enum class AwsSqsJobReceiverPolicy { /** diff --git a/misk-aws/src/main/kotlin/misk/jobqueue/sqs/AwsSqsJobQueueModule.kt b/misk-aws/src/main/kotlin/misk/jobqueue/sqs/AwsSqsJobQueueModule.kt index 4278b07262c..3991f2f77d2 100644 --- a/misk-aws/src/main/kotlin/misk/jobqueue/sqs/AwsSqsJobQueueModule.kt +++ b/misk-aws/src/main/kotlin/misk/jobqueue/sqs/AwsSqsJobQueueModule.kt @@ -31,8 +31,9 @@ import wisp.lease.LeaseManager /** [AwsSqsJobQueueModule] installs job queue support provided by SQS. */ @Deprecated( - message = "AWS SDK v1 SQS jobqueue is deprecated. Use the AWS SDK v2 SQS jobqueue in " + - "misk-aws2-sqs (misk.aws2.sqs.jobqueue.SqsJobQueueModule) instead." + message = + "AWS SDK v1 SQS jobqueue is deprecated. Use the AWS SDK v2 SQS jobqueue in " + + "misk-aws2-sqs (misk.aws2.sqs.jobqueue.SqsJobQueueModule) instead." ) open class AwsSqsJobQueueModule(private val config: AwsSqsJobQueueConfig) : KAbstractModule() { override fun configure() { @@ -84,9 +85,7 @@ open class AwsSqsJobQueueModule(private val config: AwsSqsJobQueueConfig) : KAbs .forEach { (queueName, config) -> externalQueueConfigBinder.addBinding(queueName).toInstance(config) } install(DefaultAsyncSwitchModule()) - install( - ServiceModule().dependsOn() - ) + install(ServiceModule().dependsOn()) } open fun , ClientT> configureClient(builder: BuilderT) {} @@ -218,8 +217,9 @@ open class AwsSqsJobQueueModule(private val config: AwsSqsJobQueueConfig) : KAbs /** Modify a [QueueBufferConfig] to disable all receive pre-fetching settings. */ @Deprecated( - message = "AWS SDK v1 SQS jobqueue is deprecated. Use the AWS SDK v2 SQS jobqueue in " + - "misk-aws2-sqs (misk.aws2.sqs.jobqueue) instead." + message = + "AWS SDK v1 SQS jobqueue is deprecated. Use the AWS SDK v2 SQS jobqueue in " + + "misk-aws2-sqs (misk.aws2.sqs.jobqueue) instead." ) fun QueueBufferConfig.withNoPrefetching(): QueueBufferConfig { return withMaxInflightReceiveBatches(0).withAdapativePrefetching(false).withMaxDoneReceiveBatches(0) diff --git a/misk-aws/src/main/kotlin/misk/jobqueue/sqs/AwsSqsQueueConfig.kt b/misk-aws/src/main/kotlin/misk/jobqueue/sqs/AwsSqsQueueConfig.kt index 3b3f15b36a9..8011b5bed87 100644 --- a/misk-aws/src/main/kotlin/misk/jobqueue/sqs/AwsSqsQueueConfig.kt +++ b/misk-aws/src/main/kotlin/misk/jobqueue/sqs/AwsSqsQueueConfig.kt @@ -5,8 +5,9 @@ package misk.jobqueue.sqs * is in another account, it will require an IAM policy enabling cross account access */ @Deprecated( - message = "AWS SDK v1 SQS jobqueue is deprecated. Use misk.aws2.sqs.jobqueue.config.SqsQueueConfig " + - "with misk.aws2.sqs.jobqueue.SqsJobQueueModule instead." + message = + "AWS SDK v1 SQS jobqueue is deprecated. Use misk.aws2.sqs.jobqueue.config.SqsQueueConfig " + + "with misk.aws2.sqs.jobqueue.SqsJobQueueModule instead." ) data class AwsSqsQueueConfig @JvmOverloads diff --git a/misk-aws/src/main/kotlin/misk/jobqueue/sqs/DeadLetterQueueProvider.kt b/misk-aws/src/main/kotlin/misk/jobqueue/sqs/DeadLetterQueueProvider.kt index 10a27457081..21ed0b47836 100644 --- a/misk-aws/src/main/kotlin/misk/jobqueue/sqs/DeadLetterQueueProvider.kt +++ b/misk-aws/src/main/kotlin/misk/jobqueue/sqs/DeadLetterQueueProvider.kt @@ -12,8 +12,9 @@ import misk.jobqueue.QueueName */ @ImplementedBy(DefaultDeadLetterQueueProvider::class) @Deprecated( - message = "AWS SDK v1 SQS jobqueue is deprecated. Use " + - "misk.aws2.sqs.jobqueue.DeadLetterQueueProvider with the AWS SDK v2 SQS jobqueue instead." + message = + "AWS SDK v1 SQS jobqueue is deprecated. Use " + + "misk.aws2.sqs.jobqueue.DeadLetterQueueProvider with the AWS SDK v2 SQS jobqueue instead." ) interface DeadLetterQueueProvider { fun deadLetterQueueFor(queue: QueueName): QueueName @@ -22,8 +23,9 @@ interface DeadLetterQueueProvider { /** Default provider of dead-letter [QueueName]. Returns the name of the main queue suffixed with "_dlq". */ @Singleton @Deprecated( - message = "AWS SDK v1 SQS jobqueue is deprecated. Use " + - "misk.aws2.sqs.jobqueue.DefaultDeadLetterQueueProvider with the AWS SDK v2 SQS jobqueue instead." + message = + "AWS SDK v1 SQS jobqueue is deprecated. Use " + + "misk.aws2.sqs.jobqueue.DefaultDeadLetterQueueProvider with the AWS SDK v2 SQS jobqueue instead." ) class DefaultDeadLetterQueueProvider @Inject constructor() : DeadLetterQueueProvider { override fun deadLetterQueueFor(queue: QueueName): QueueName = queue.deadLetterQueue @@ -35,8 +37,9 @@ class DefaultDeadLetterQueueProvider @Inject constructor() : DeadLetterQueueProv * For apps with queues that share a single dead-letter queue. */ @Deprecated( - message = "AWS SDK v1 SQS jobqueue is deprecated. Use " + - "misk.aws2.sqs.jobqueue.StaticDeadLetterQueueProvider with the AWS SDK v2 SQS jobqueue instead." + message = + "AWS SDK v1 SQS jobqueue is deprecated. Use " + + "misk.aws2.sqs.jobqueue.StaticDeadLetterQueueProvider with the AWS SDK v2 SQS jobqueue instead." ) class StaticDeadLetterQueueProvider(queue: String) : DeadLetterQueueProvider { private val dlq = QueueName(queue) diff --git a/misk-aws/src/main/kotlin/misk/jobqueue/sqs/SqsConsumerAllocator.kt b/misk-aws/src/main/kotlin/misk/jobqueue/sqs/SqsConsumerAllocator.kt index bdd7d6eea07..85af1f6bd46 100644 --- a/misk-aws/src/main/kotlin/misk/jobqueue/sqs/SqsConsumerAllocator.kt +++ b/misk-aws/src/main/kotlin/misk/jobqueue/sqs/SqsConsumerAllocator.kt @@ -13,8 +13,9 @@ import wisp.lease.LeaseManager */ @Singleton @Deprecated( - message = "AWS SDK v1 SQS jobqueue is deprecated. Use the AWS SDK v2 SQS jobqueue in " + - "misk-aws2-sqs (misk.aws2.sqs.jobqueue) instead." + message = + "AWS SDK v1 SQS jobqueue is deprecated. Use the AWS SDK v2 SQS jobqueue in " + + "misk-aws2-sqs (misk.aws2.sqs.jobqueue) instead." ) class SqsConsumerAllocator @Inject diff --git a/misk-aws/src/main/kotlin/misk/jobqueue/sqs/SqsJobConsumer.kt b/misk-aws/src/main/kotlin/misk/jobqueue/sqs/SqsJobConsumer.kt index 48f8145dce1..056cd996d7f 100644 --- a/misk-aws/src/main/kotlin/misk/jobqueue/sqs/SqsJobConsumer.kt +++ b/misk-aws/src/main/kotlin/misk/jobqueue/sqs/SqsJobConsumer.kt @@ -25,9 +25,9 @@ import kotlin.math.ceil import kotlin.math.max import kotlin.math.min import misk.annotation.ExperimentalMiskApi -import misk.inject.AsyncSwitch import misk.feature.Feature import misk.feature.FeatureFlags +import misk.inject.AsyncSwitch import misk.jobqueue.BatchJobHandler import misk.jobqueue.JobConsumer import misk.jobqueue.JobHandler diff --git a/misk-aws/src/main/kotlin/misk/s3/RealS3Module.kt b/misk-aws/src/main/kotlin/misk/s3/RealS3Module.kt index 76b68f65e91..f3c3f223b0e 100644 --- a/misk-aws/src/main/kotlin/misk/s3/RealS3Module.kt +++ b/misk-aws/src/main/kotlin/misk/s3/RealS3Module.kt @@ -9,8 +9,8 @@ import misk.cloud.aws.AwsRegion import misk.inject.KAbstractModule @Deprecated( - message = "AWS SDK v1 S3 is deprecated. Use the AWS SDK v2 S3 module in " + - "misk-aws2-s3 (misk.aws2.s3.S3Module) instead." + message = + "AWS SDK v1 S3 is deprecated. Use the AWS SDK v2 S3 module in " + "misk-aws2-s3 (misk.aws2.s3.S3Module) instead." ) open class RealS3Module : KAbstractModule() { override fun configure() { diff --git a/misk-aws/src/test/kotlin/misk/jobqueue/sqs/SqsAsyncSwitchTest.kt b/misk-aws/src/test/kotlin/misk/jobqueue/sqs/SqsAsyncSwitchTest.kt index 006e4a5fec6..66d28199dd7 100644 --- a/misk-aws/src/test/kotlin/misk/jobqueue/sqs/SqsAsyncSwitchTest.kt +++ b/misk-aws/src/test/kotlin/misk/jobqueue/sqs/SqsAsyncSwitchTest.kt @@ -25,9 +25,7 @@ internal class SqsAsyncSwitchTest { @MiskExternalDependency private val dockerSqs = DockerSqs @MiskTestModule private val module = - Modules.override( - SqsJobQueueTestModule(dockerSqs.credentials, dockerSqs.client), - ).with(FakeSwitchModule()) + Modules.override(SqsJobQueueTestModule(dockerSqs.credentials, dockerSqs.client)).with(FakeSwitchModule()) @Inject private lateinit var sqs: AmazonSQS @Inject private lateinit var queue: JobQueue @Inject private lateinit var consumer: JobConsumer diff --git a/misk-aws2-sqs/src/main/kotlin/misk/aws2/sqs/jobqueue/SqsJobEnqueuer.kt b/misk-aws2-sqs/src/main/kotlin/misk/aws2/sqs/jobqueue/SqsJobEnqueuer.kt index 100122a514f..1faf783659d 100644 --- a/misk-aws2-sqs/src/main/kotlin/misk/aws2/sqs/jobqueue/SqsJobEnqueuer.kt +++ b/misk-aws2-sqs/src/main/kotlin/misk/aws2/sqs/jobqueue/SqsJobEnqueuer.kt @@ -12,7 +12,6 @@ import java.time.Duration import java.util.concurrent.CompletableFuture import misk.aws2.sqs.jobqueue.config.SqsConfig import misk.jobqueue.QueueName -import misk.aws2.sqs.jobqueue.parentQueue import misk.jobqueue.v2.JobEnqueuer import misk.moshi.adapter import misk.tokens.TokenGenerator diff --git a/misk-aws2-sqs/src/main/kotlin/misk/aws2/sqs/jobqueue/SqsJobQueueModule.kt b/misk-aws2-sqs/src/main/kotlin/misk/aws2/sqs/jobqueue/SqsJobQueueModule.kt index 77e66e9f7bf..c4fe5cc4fe0 100644 --- a/misk-aws2-sqs/src/main/kotlin/misk/aws2/sqs/jobqueue/SqsJobQueueModule.kt +++ b/misk-aws2-sqs/src/main/kotlin/misk/aws2/sqs/jobqueue/SqsJobQueueModule.kt @@ -1,7 +1,6 @@ package misk.aws2.sqs.jobqueue import com.google.inject.Provides -import com.google.inject.multibindings.OptionalBinder import jakarta.inject.Singleton import misk.ReadyService import misk.ServiceModule diff --git a/misk-aws2-sqs/src/main/kotlin/misk/aws2/sqs/jobqueue/SubscriptionService.kt b/misk-aws2-sqs/src/main/kotlin/misk/aws2/sqs/jobqueue/SubscriptionService.kt index 0c024f7f888..b4bd3ab6615 100644 --- a/misk-aws2-sqs/src/main/kotlin/misk/aws2/sqs/jobqueue/SubscriptionService.kt +++ b/misk-aws2-sqs/src/main/kotlin/misk/aws2/sqs/jobqueue/SubscriptionService.kt @@ -48,15 +48,16 @@ constructor( logger.info { "Starting AWS SQS SubscriptionService with config=$effectiveConfig" } handlers.forEach { (queueName, handler) -> val queueConfig = effectiveConfig.getQueueConfig(queueName) - logger.info { "Subscribing to queue ${queueName.value} with config: concurrency=${queueConfig.concurrency}, parallelism=${queueConfig.parallelism}" } + logger.info { + "Subscribing to queue ${queueName.value} with config: concurrency=${queueConfig.concurrency}, parallelism=${queueConfig.parallelism}" + } consumer.subscribe(queueName, handler, queueConfig) } } /** - * Resolves the effective configuration. - * If a dynamic config flag is configured and returns a valid config, it completely replaces the YAML config. - * Otherwise, the YAML config is used. + * Resolves the effective configuration. If a dynamic config flag is configured and returns a valid config, it + * completely replaces the YAML config. Otherwise, the YAML config is used. * * In both cases, if region is not specified in the config, it is populated from the AWS environment. */ @@ -89,8 +90,8 @@ constructor( } /** - * Applies the AWS region default to the config if not already specified. - * This ensures dynamic config behaves the same as YAML config with respect to region auto-population. + * Applies the AWS region default to the config if not already specified. This ensures dynamic config behaves the same + * as YAML config with respect to region auto-population. */ private fun applyRegionDefault(sqsConfig: SqsConfig): SqsConfig { return if (sqsConfig.all_queues.region == null) { diff --git a/misk-aws2-sqs/src/main/kotlin/misk/aws2/sqs/jobqueue/config/SqsConfig.kt b/misk-aws2-sqs/src/main/kotlin/misk/aws2/sqs/jobqueue/config/SqsConfig.kt index ea6dba32749..ab2c36f52aa 100644 --- a/misk-aws2-sqs/src/main/kotlin/misk/aws2/sqs/jobqueue/config/SqsConfig.kt +++ b/misk-aws2-sqs/src/main/kotlin/misk/aws2/sqs/jobqueue/config/SqsConfig.kt @@ -10,10 +10,10 @@ import misk.jobqueue.QueueName * overriding configuration for a given queue `buffered_batch_flush_frequency_ms` controls how often buffered messages * are flushed to SQS when using enqueueBuffered * - * `config_feature_flag` allows specifying a dynamic config name that returns a JSON object matching the - * structure of SqsConfig. When set, the dynamic config is evaluated at service startup and **completely replaces** - * the YAML configuration. This allows dynamic configuration changes with a service restart (without requiring a code - * deploy). If not set, or if the dynamic config returns null/empty, the YAML configuration is used. + * `config_feature_flag` allows specifying a dynamic config name that returns a JSON object matching the structure of + * SqsConfig. When set, the dynamic config is evaluated at service startup and **completely replaces** the YAML + * configuration. This allows dynamic configuration changes with a service restart (without requiring a code deploy). If + * not set, or if the dynamic config returns null/empty, the YAML configuration is used. */ data class SqsConfig @JvmOverloads @@ -22,9 +22,9 @@ constructor( val per_queue_overrides: Map = emptyMap(), val buffered_batch_flush_frequency_ms: Long = 50, /** - * Dynamic config name that returns a JSON object matching SqsConfig structure. - * When set and returns a valid config, it completely replaces the YAML config. - * Example value: {"all_queues": {"concurrency": 10}, "per_queue_overrides": {"my_queue": {"concurrency": 20}}} + * Dynamic config name that returns a JSON object matching SqsConfig structure. When set and returns a valid config, + * it completely replaces the YAML config. Example value: {"all_queues": {"concurrency": 10}, "per_queue_overrides": + * {"my_queue": {"concurrency": 20}}} */ val config_feature_flag: String? = null, ) : Config { diff --git a/misk-aws2-sqs/src/main/kotlin/misk/aws2/sqs/jobqueue/coordinated/AwsSqsJobQueueConfig.kt b/misk-aws2-sqs/src/main/kotlin/misk/aws2/sqs/jobqueue/coordinated/AwsSqsJobQueueConfig.kt index 1c2878ba6d7..06189ce6efb 100644 --- a/misk-aws2-sqs/src/main/kotlin/misk/aws2/sqs/jobqueue/coordinated/AwsSqsJobQueueConfig.kt +++ b/misk-aws2-sqs/src/main/kotlin/misk/aws2/sqs/jobqueue/coordinated/AwsSqsJobQueueConfig.kt @@ -1,9 +1,9 @@ package misk.aws2.sqs.jobqueue.coordinated -import misk.config.Config import misk.aws2.sqs.jobqueue.coordinated.SqsJobConsumer.Companion.CONSUMERS_PER_QUEUE import misk.aws2.sqs.jobqueue.coordinated.SqsJobConsumer.Companion.POD_CONSUMERS_PER_QUEUE import misk.aws2.sqs.jobqueue.coordinated.SqsJobConsumer.Companion.POD_MAX_JOBQUEUE_CONSUMERS +import misk.config.Config import misk.tasks.RepeatedTaskQueueConfig /** [AwsSqsJobQueueConfig] is the configuration for job queueing backed by Amazon's Simple Queuing Service */ diff --git a/misk-aws2-sqs/src/main/kotlin/misk/aws2/sqs/jobqueue/coordinated/CoordinatedAwsSqsBatchJobHandlerModule.kt b/misk-aws2-sqs/src/main/kotlin/misk/aws2/sqs/jobqueue/coordinated/CoordinatedAwsSqsBatchJobHandlerModule.kt index 32b41dc0241..76da16e2e1e 100644 --- a/misk-aws2-sqs/src/main/kotlin/misk/aws2/sqs/jobqueue/coordinated/CoordinatedAwsSqsBatchJobHandlerModule.kt +++ b/misk-aws2-sqs/src/main/kotlin/misk/aws2/sqs/jobqueue/coordinated/CoordinatedAwsSqsBatchJobHandlerModule.kt @@ -32,11 +32,7 @@ private constructor( } install(DefaultAsyncSwitchModule()) - install( - ServiceModule() - .dependsOn(dependsOn) - .dependsOn() - ) + install(ServiceModule().dependsOn(dependsOn).dependsOn()) } companion object { diff --git a/misk-aws2-sqs/src/main/kotlin/misk/aws2/sqs/jobqueue/coordinated/CoordinatedAwsSqsJobHandlerModule.kt b/misk-aws2-sqs/src/main/kotlin/misk/aws2/sqs/jobqueue/coordinated/CoordinatedAwsSqsJobHandlerModule.kt index 7a91252c0cf..cad26793072 100644 --- a/misk-aws2-sqs/src/main/kotlin/misk/aws2/sqs/jobqueue/coordinated/CoordinatedAwsSqsJobHandlerModule.kt +++ b/misk-aws2-sqs/src/main/kotlin/misk/aws2/sqs/jobqueue/coordinated/CoordinatedAwsSqsJobHandlerModule.kt @@ -32,11 +32,7 @@ private constructor( } install(DefaultAsyncSwitchModule()) - install( - ServiceModule() - .dependsOn(dependsOn) - .dependsOn() - ) + install(ServiceModule().dependsOn(dependsOn).dependsOn()) } companion object { diff --git a/misk-aws2-sqs/src/main/kotlin/misk/aws2/sqs/jobqueue/coordinated/ForSqsHandling.kt b/misk-aws2-sqs/src/main/kotlin/misk/aws2/sqs/jobqueue/coordinated/ForSqsHandling.kt index 10c024b3692..e513db74bff 100644 --- a/misk-aws2-sqs/src/main/kotlin/misk/aws2/sqs/jobqueue/coordinated/ForSqsHandling.kt +++ b/misk-aws2-sqs/src/main/kotlin/misk/aws2/sqs/jobqueue/coordinated/ForSqsHandling.kt @@ -2,6 +2,4 @@ package misk.aws2.sqs.jobqueue.coordinated import jakarta.inject.Qualifier -@Qualifier -@Retention(AnnotationRetention.RUNTIME) -internal annotation class ForSqsHandling +@Qualifier @Retention(AnnotationRetention.RUNTIME) internal annotation class ForSqsHandling diff --git a/misk-aws2-sqs/src/main/kotlin/misk/aws2/sqs/jobqueue/coordinated/ForSqsReceiving.kt b/misk-aws2-sqs/src/main/kotlin/misk/aws2/sqs/jobqueue/coordinated/ForSqsReceiving.kt index 3860ccf356b..be37723bb9d 100644 --- a/misk-aws2-sqs/src/main/kotlin/misk/aws2/sqs/jobqueue/coordinated/ForSqsReceiving.kt +++ b/misk-aws2-sqs/src/main/kotlin/misk/aws2/sqs/jobqueue/coordinated/ForSqsReceiving.kt @@ -2,6 +2,4 @@ package misk.aws2.sqs.jobqueue.coordinated import jakarta.inject.Qualifier -@Qualifier -@Retention(AnnotationRetention.RUNTIME) -internal annotation class ForSqsReceiving +@Qualifier @Retention(AnnotationRetention.RUNTIME) internal annotation class ForSqsReceiving diff --git a/misk-aws2-sqs/src/main/kotlin/misk/aws2/sqs/jobqueue/coordinated/QueueResolver.kt b/misk-aws2-sqs/src/main/kotlin/misk/aws2/sqs/jobqueue/coordinated/QueueResolver.kt index 6eebbabfe42..8dcd7fe7f70 100644 --- a/misk-aws2-sqs/src/main/kotlin/misk/aws2/sqs/jobqueue/coordinated/QueueResolver.kt +++ b/misk-aws2-sqs/src/main/kotlin/misk/aws2/sqs/jobqueue/coordinated/QueueResolver.kt @@ -60,10 +60,7 @@ internal constructor( val queueUrl = sqs .getQueueUrl( - GetQueueUrlRequest.builder() - .queueName(sqsQueueName.value) - .queueOwnerAWSAccountId(accountId.value) - .build() + GetQueueUrlRequest.builder().queueName(sqsQueueName.value).queueOwnerAWSAccountId(accountId.value).build() ) .queueUrl() ensureUrlWithProperTarget(queueUrl) diff --git a/misk-aws2-sqs/src/main/kotlin/misk/aws2/sqs/jobqueue/coordinated/ResolvedQueue.kt b/misk-aws2-sqs/src/main/kotlin/misk/aws2/sqs/jobqueue/coordinated/ResolvedQueue.kt index b15f6477fa8..2f8eeb056f8 100644 --- a/misk-aws2-sqs/src/main/kotlin/misk/aws2/sqs/jobqueue/coordinated/ResolvedQueue.kt +++ b/misk-aws2-sqs/src/main/kotlin/misk/aws2/sqs/jobqueue/coordinated/ResolvedQueue.kt @@ -1,7 +1,7 @@ package misk.aws2.sqs.jobqueue.coordinated -import java.util.concurrent.CompletionException import java.util.concurrent.CompletableFuture +import java.util.concurrent.CompletionException import misk.cloud.aws.AwsAccountId import misk.cloud.aws.AwsRegion import misk.feature.Feature diff --git a/misk-aws2-sqs/src/main/kotlin/misk/aws2/sqs/jobqueue/coordinated/SqsJob.kt b/misk-aws2-sqs/src/main/kotlin/misk/aws2/sqs/jobqueue/coordinated/SqsJob.kt index 3983b3dc245..5d8f40bc722 100644 --- a/misk-aws2-sqs/src/main/kotlin/misk/aws2/sqs/jobqueue/coordinated/SqsJob.kt +++ b/misk-aws2-sqs/src/main/kotlin/misk/aws2/sqs/jobqueue/coordinated/SqsJob.kt @@ -89,15 +89,8 @@ internal class SqsJob( } private fun deleteMessage(queue: ResolvedQueue, message: Message) { - val request = - DeleteMessageRequest.builder() - .queueUrl(queue.url) - .receiptHandle(message.receiptHandle()) - .build() - val (deleteDuration) = - timed { - queue.call { it.deleteMessage(request) } - } + val request = DeleteMessageRequest.builder().queueUrl(queue.url).receiptHandle(message.receiptHandle()).build() + val (deleteDuration) = timed { queue.call { it.deleteMessage(request) } } metrics.sqsDeleteTime.record(deleteDuration.toMillis().toDouble(), queueName.value, queueName.value) } diff --git a/misk-aws2-sqs/src/main/kotlin/misk/aws2/sqs/jobqueue/coordinated/SqsJobQueue.kt b/misk-aws2-sqs/src/main/kotlin/misk/aws2/sqs/jobqueue/coordinated/SqsJobQueue.kt index f3fcd63f4c2..158831f38d9 100644 --- a/misk-aws2-sqs/src/main/kotlin/misk/aws2/sqs/jobqueue/coordinated/SqsJobQueue.kt +++ b/misk-aws2-sqs/src/main/kotlin/misk/aws2/sqs/jobqueue/coordinated/SqsJobQueue.kt @@ -48,15 +48,14 @@ internal constructor( .messageAttributes( attributes .mapValues { it.value.toMessageAttributeValue() } - .plus(SqsJob.JOBQUEUE_METADATA_ATTR to createMetadataMessageAttributeValue(span, queueName, idempotenceKey)) + .plus( + SqsJob.JOBQUEUE_METADATA_ATTR to createMetadataMessageAttributeValue(span, queueName, idempotenceKey) + ) ) .build() val (sendDuration) = timed { - queue.callSend( - unbufferedLambda = { it.sendMessage(request) }, - bufferedLambda = { it.sendMessage(request) }, - ) + queue.callSend(unbufferedLambda = { it.sendMessage(request) }, bufferedLambda = { it.sendMessage(request) }) } return@executeWithTracingAndErrorHandling sendDuration } @@ -88,7 +87,9 @@ internal constructor( } timed { - client.sendMessageBatch(SendMessageBatchRequest.builder().queueUrl(queue.url).entries(messageEntries).build()) + client.sendMessageBatch( + SendMessageBatchRequest.builder().queueUrl(queue.url).entries(messageEntries).build() + ) } } diff --git a/misk-aws2-sqs/src/test/kotlin/misk/aws2/sqs/jobqueue/SqsAsyncSwitchTest.kt b/misk-aws2-sqs/src/test/kotlin/misk/aws2/sqs/jobqueue/SqsAsyncSwitchTest.kt index ecef0078071..ff39532a200 100644 --- a/misk-aws2-sqs/src/test/kotlin/misk/aws2/sqs/jobqueue/SqsAsyncSwitchTest.kt +++ b/misk-aws2-sqs/src/test/kotlin/misk/aws2/sqs/jobqueue/SqsAsyncSwitchTest.kt @@ -3,6 +3,8 @@ package misk.aws2.sqs.jobqueue import jakarta.inject.Inject import java.time.Duration import java.util.concurrent.TimeUnit +import kotlin.test.assertEquals +import kotlin.test.assertTrue import kotlinx.coroutines.test.runTest import misk.aws2.sqs.jobqueue.config.SqsConfig import misk.aws2.sqs.jobqueue.config.SqsQueueConfig @@ -22,8 +24,6 @@ import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test import software.amazon.awssdk.services.sqs.model.CreateQueueRequest import software.amazon.awssdk.services.sqs.model.QueueAttributeName -import kotlin.test.assertEquals -import kotlin.test.assertTrue @MiskTest(startService = true) class SqsAsyncSwitchTest { diff --git a/misk-aws2-sqs/src/test/kotlin/misk/aws2/sqs/jobqueue/SubscriptionServiceAwsEnvironmentDefaultsTest.kt b/misk-aws2-sqs/src/test/kotlin/misk/aws2/sqs/jobqueue/SubscriptionServiceAwsEnvironmentDefaultsTest.kt index e94d8f25d48..dd58969d760 100644 --- a/misk-aws2-sqs/src/test/kotlin/misk/aws2/sqs/jobqueue/SubscriptionServiceAwsEnvironmentDefaultsTest.kt +++ b/misk-aws2-sqs/src/test/kotlin/misk/aws2/sqs/jobqueue/SubscriptionServiceAwsEnvironmentDefaultsTest.kt @@ -12,22 +12,19 @@ import misk.testing.MiskTestModule import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.Test -/** - * Tests that AWS environment defaults are applied to YAML config when region is not specified. - */ +/** Tests that AWS environment defaults are applied to YAML config when region is not specified. */ @MiskTest(startService = false) class SubscriptionServiceAwsEnvironmentDefaultsTest { @MiskExternalDependency private val dockerSqs = DockerSqs @MiskExternalDependency private val queueCreator = SubscriptionServiceTestQueueCreator(dockerSqs) @MiskTestModule - private val module = SubscriptionServiceTestModule( - dockerSqs = dockerSqs, - // YAML config without region - should be populated from AWS environment - yamlConfig = SqsConfig( - all_queues = SqsQueueConfig(concurrency = 3, parallelism = 2), - ), - ) + private val module = + SubscriptionServiceTestModule( + dockerSqs = dockerSqs, + // YAML config without region - should be populated from AWS environment + yamlConfig = SqsConfig(all_queues = SqsQueueConfig(concurrency = 3, parallelism = 2)), + ) @Inject private lateinit var subscriptionService: SubscriptionService @Inject private lateinit var serviceManager: ServiceManager diff --git a/misk-aws2-sqs/src/test/kotlin/misk/aws2/sqs/jobqueue/SubscriptionServiceConfigValidationTest.kt b/misk-aws2-sqs/src/test/kotlin/misk/aws2/sqs/jobqueue/SubscriptionServiceConfigValidationTest.kt index a9125497d04..6526c34c851 100644 --- a/misk-aws2-sqs/src/test/kotlin/misk/aws2/sqs/jobqueue/SubscriptionServiceConfigValidationTest.kt +++ b/misk-aws2-sqs/src/test/kotlin/misk/aws2/sqs/jobqueue/SubscriptionServiceConfigValidationTest.kt @@ -10,27 +10,20 @@ import misk.testing.MiskTest import misk.testing.MiskTestModule import org.junit.jupiter.api.Test -/** - * Tests that startup fails when config_feature_flag is configured but DynamicConfig is not bound. - */ +/** Tests that startup fails when config_feature_flag is configured but DynamicConfig is not bound. */ @MiskTest(startService = false) class SubscriptionServiceConfigValidationTest { @MiskExternalDependency private val dockerSqs = DockerSqs @MiskExternalDependency private val queueCreator = SubscriptionServiceTestQueueCreator(dockerSqs) @MiskTestModule - private val module = SubscriptionServiceTestModule( - dockerSqs = dockerSqs, - installFakeFeatureFlags = false, - ) + private val module = SubscriptionServiceTestModule(dockerSqs = dockerSqs, installFakeFeatureFlags = false) @Inject private lateinit var serviceManager: ServiceManager @Test fun `startUp fails when dynamic config flag configured but DynamicConfig not bound`() { - val exception = assertFailsWith { - serviceManager.startAsync().awaitHealthy() - } + val exception = assertFailsWith { serviceManager.startAsync().awaitHealthy() } val cause = exception.suppressedExceptions.firstOrNull()?.cause assertTrue(cause is IllegalStateException) diff --git a/misk-aws2-sqs/src/test/kotlin/misk/aws2/sqs/jobqueue/SubscriptionServiceDynamicConfigTest.kt b/misk-aws2-sqs/src/test/kotlin/misk/aws2/sqs/jobqueue/SubscriptionServiceDynamicConfigTest.kt index fc41dd908d0..ece0712413d 100644 --- a/misk-aws2-sqs/src/test/kotlin/misk/aws2/sqs/jobqueue/SubscriptionServiceDynamicConfigTest.kt +++ b/misk-aws2-sqs/src/test/kotlin/misk/aws2/sqs/jobqueue/SubscriptionServiceDynamicConfigTest.kt @@ -17,8 +17,7 @@ import org.junit.jupiter.api.Test /** * Tests for [SubscriptionService] dynamic config override behavior. * - * Uses `startService = false` to allow per-test configuration of dynamic config - * values before the service starts up. + * Uses `startService = false` to allow per-test configuration of dynamic config values before the service starts up. */ @MiskTest(startService = false) class SubscriptionServiceDynamicConfigTest { @@ -26,13 +25,15 @@ class SubscriptionServiceDynamicConfigTest { @MiskExternalDependency private val queueCreator = SubscriptionServiceTestQueueCreator(dockerSqs) @MiskTestModule - private val module = SubscriptionServiceTestModule( - dockerSqs = dockerSqs, - yamlConfig = SqsConfig( - all_queues = SqsQueueConfig(concurrency = 5, parallelism = 2, region = "us-east-1"), - config_feature_flag = "test-sqs-config", - ), - ) + private val module = + SubscriptionServiceTestModule( + dockerSqs = dockerSqs, + yamlConfig = + SqsConfig( + all_queues = SqsQueueConfig(concurrency = 5, parallelism = 2, region = "us-east-1"), + config_feature_flag = "test-sqs-config", + ), + ) @Inject private lateinit var subscriptionService: SubscriptionService @Inject private lateinit var serviceManager: ServiceManager diff --git a/misk-aws2-sqs/src/test/kotlin/misk/aws2/sqs/jobqueue/SubscriptionServiceTestModule.kt b/misk-aws2-sqs/src/test/kotlin/misk/aws2/sqs/jobqueue/SubscriptionServiceTestModule.kt index 22f98a11542..6988bcd83d3 100644 --- a/misk-aws2-sqs/src/test/kotlin/misk/aws2/sqs/jobqueue/SubscriptionServiceTestModule.kt +++ b/misk-aws2-sqs/src/test/kotlin/misk/aws2/sqs/jobqueue/SubscriptionServiceTestModule.kt @@ -24,15 +24,16 @@ import software.amazon.awssdk.services.sqs.model.QueueAttributeName * * @param dockerSqs The Docker SQS instance * @param yamlConfig The YAML-based SqsConfig (always required as fallback) - * @param installFakeFeatureFlags Whether to install FakeFeatureFlagsModule (default true). - * Set to false to test behavior when DynamicConfig is not bound. + * @param installFakeFeatureFlags Whether to install FakeFeatureFlagsModule (default true). Set to false to test + * behavior when DynamicConfig is not bound. */ class SubscriptionServiceTestModule( private val dockerSqs: DockerSqs, - private val yamlConfig: SqsConfig = SqsConfig( - all_queues = SqsQueueConfig(concurrency = 5, parallelism = 2, region = "us-east-1"), - config_feature_flag = "test-sqs-config", - ), + private val yamlConfig: SqsConfig = + SqsConfig( + all_queues = SqsQueueConfig(concurrency = 5, parallelism = 2, region = "us-east-1"), + config_feature_flag = "test-sqs-config", + ), private val installFakeFeatureFlags: Boolean = true, ) : KAbstractModule() { override fun configure() { @@ -61,6 +62,7 @@ class SubscriptionServiceTestQueueCreator(private val dockerSqs: DockerSqs) : Ex private val queues = listOf("test-queue", "test-queue_retryq", "test-queue_dlq") override fun startup() {} + override fun shutdown() {} override fun beforeEach() { diff --git a/misk-aws2-sqs/src/test/kotlin/misk/aws2/sqs/jobqueue/config/SqsConfigTest.kt b/misk-aws2-sqs/src/test/kotlin/misk/aws2/sqs/jobqueue/config/SqsConfigTest.kt index 59b8c72ff80..8774b5fd858 100644 --- a/misk-aws2-sqs/src/test/kotlin/misk/aws2/sqs/jobqueue/config/SqsConfigTest.kt +++ b/misk-aws2-sqs/src/test/kotlin/misk/aws2/sqs/jobqueue/config/SqsConfigTest.kt @@ -44,9 +44,7 @@ class SqsConfigTest { @Test fun `getQueueConfig returns all_queues when no per_queue_override exists`() { - val config = SqsConfig( - all_queues = SqsQueueConfig(concurrency = 10, parallelism = 5), - ) + val config = SqsConfig(all_queues = SqsQueueConfig(concurrency = 10, parallelism = 5)) val queueConfig = config.getQueueConfig(misk.jobqueue.QueueName("test-queue")) @@ -56,12 +54,11 @@ class SqsConfigTest { @Test fun `getQueueConfig returns per_queue_override when it exists`() { - val config = SqsConfig( - all_queues = SqsQueueConfig(concurrency = 1, parallelism = 1), - per_queue_overrides = mapOf( - "test-queue" to SqsQueueConfig(concurrency = 20, parallelism = 10), - ), - ) + val config = + SqsConfig( + all_queues = SqsQueueConfig(concurrency = 1, parallelism = 1), + per_queue_overrides = mapOf("test-queue" to SqsQueueConfig(concurrency = 20, parallelism = 10)), + ) val queueConfig = config.getQueueConfig(misk.jobqueue.QueueName("test-queue")) @@ -71,12 +68,11 @@ class SqsConfigTest { @Test fun `getQueueConfig inherits nullable fields from all_queues`() { - val config = SqsConfig( - all_queues = SqsQueueConfig(region = "us-west-2", wait_timeout = 20), - per_queue_overrides = mapOf( - "test-queue" to SqsQueueConfig(concurrency = 10), - ), - ) + val config = + SqsConfig( + all_queues = SqsQueueConfig(region = "us-west-2", wait_timeout = 20), + per_queue_overrides = mapOf("test-queue" to SqsQueueConfig(concurrency = 10)), + ) val queueConfig = config.getQueueConfig(misk.jobqueue.QueueName("test-queue")) diff --git a/misk-clustering-dynamodb/src/main/kotlin/misk/clustering/dynamo/DynamoClusterModule.kt b/misk-clustering-dynamodb/src/main/kotlin/misk/clustering/dynamo/DynamoClusterModule.kt index b7aab521367..007babd6949 100644 --- a/misk-clustering-dynamodb/src/main/kotlin/misk/clustering/dynamo/DynamoClusterModule.kt +++ b/misk-clustering-dynamodb/src/main/kotlin/misk/clustering/dynamo/DynamoClusterModule.kt @@ -24,11 +24,7 @@ class DynamoClusterModule @JvmOverloads constructor(private val config: DynamoCl bind().toInstance(defaultCluster) install(ServiceModule()) install(DefaultAsyncSwitchModule()) - install( - ServiceModule() - .dependsOn() - .enhancedBy() - ) + install(ServiceModule().dependsOn().enhancedBy()) install(ServiceModule(ForDynamoDbClusterWatching::class)) } diff --git a/misk-cron/src/main/kotlin/misk/cron/CronModule.kt b/misk-cron/src/main/kotlin/misk/cron/CronModule.kt index f7ff1c039b2..40d9fda413a 100644 --- a/misk-cron/src/main/kotlin/misk/cron/CronModule.kt +++ b/misk-cron/src/main/kotlin/misk/cron/CronModule.kt @@ -46,12 +46,8 @@ constructor( ) ) - install( - ServiceModule().dependsOn() - ) - install( - ServiceModule().dependsOn(dependencies).dependsOn() - ) + install(ServiceModule().dependsOn()) + install(ServiceModule().dependsOn(dependencies).dependsOn()) } @Provides diff --git a/misk-grpc-reflect/src/main/kotlin/misk/grpc/reflect/GrpcReflectModule.kt b/misk-grpc-reflect/src/main/kotlin/misk/grpc/reflect/GrpcReflectModule.kt index d17eabe227a..261b63e6506 100644 --- a/misk-grpc-reflect/src/main/kotlin/misk/grpc/reflect/GrpcReflectModule.kt +++ b/misk-grpc-reflect/src/main/kotlin/misk/grpc/reflect/GrpcReflectModule.kt @@ -61,16 +61,11 @@ class GrpcReflectModule : KAbstractModule() { } val schemaLoader = SchemaLoader(fileSystem) - schemaLoader.initRoots( - sourcePath = sourceLocations.toList(), - protoPath = listOf(Location.get(".")), - ) + schemaLoader.initRoots(sourcePath = sourceLocations.toList(), protoPath = listOf(Location.get("."))) schemaLoader.loadExhaustively = true val schema = schemaLoader.loadSchema() - val pruningRules = PruningRules.Builder() - .addRoot(implementedServices) - .build() + val pruningRules = PruningRules.Builder().addRoot(implementedServices).build() return schema.prune(pruningRules) } diff --git a/misk-grpc-reflect/src/test/kotlin/misk/grpc/GrpcReflectTestingModule.kt b/misk-grpc-reflect/src/test/kotlin/misk/grpc/GrpcReflectTestingModule.kt index de579da2465..3ccd494c158 100644 --- a/misk-grpc-reflect/src/test/kotlin/misk/grpc/GrpcReflectTestingModule.kt +++ b/misk-grpc-reflect/src/test/kotlin/misk/grpc/GrpcReflectTestingModule.kt @@ -19,7 +19,7 @@ import misk.web.jetty.JettyService import okhttp3.HttpUrl class GrpcReflectTestingModule : KAbstractModule() { - val webConfig = WebTestingModule.TESTING_WEB_CONFIG.copy(port = 9090) + val webConfig = WebTestingModule.TESTING_WEB_CONFIG.copy(port = 9999) override fun configure() { install(WebTestingModule(webConfig = webConfig)) diff --git a/misk-grpc-reflect/src/test/kotlin/misk/grpc/GrpcReflectTransitiveServiceTest.kt b/misk-grpc-reflect/src/test/kotlin/misk/grpc/GrpcReflectTransitiveServiceTest.kt index b88790ab244..6a19217638a 100644 --- a/misk-grpc-reflect/src/test/kotlin/misk/grpc/GrpcReflectTransitiveServiceTest.kt +++ b/misk-grpc-reflect/src/test/kotlin/misk/grpc/GrpcReflectTransitiveServiceTest.kt @@ -41,17 +41,14 @@ class GrpcReflectTransitiveServiceTest { val response = responses.read() val serviceNames = response!!.list_services_response!!.service.map { it.name } - assertThat(serviceNames).containsExactlyInAnyOrder( - "grpc.reflection.v1alpha.ServerReflection", - "transitive.MainService", - ) + assertThat(serviceNames) + .containsExactlyInAnyOrder("grpc.reflection.v1alpha.ServerReflection", "transitive.MainService") } } } @Singleton - private class FakeMainServiceAction @Inject constructor() : - MainServiceEchoBlockingServer, WebAction { + private class FakeMainServiceAction @Inject constructor() : MainServiceEchoBlockingServer, WebAction { override fun Echo(request: EchoRequest): EchoResponse = error("unsupported") } } diff --git a/misk-hibernate/src/main/kotlin/misk/hibernate/HibernateExceptionClassifier.kt b/misk-hibernate/src/main/kotlin/misk/hibernate/HibernateExceptionClassifier.kt index c594a5c0553..88de7dbea38 100644 --- a/misk-hibernate/src/main/kotlin/misk/hibernate/HibernateExceptionClassifier.kt +++ b/misk-hibernate/src/main/kotlin/misk/hibernate/HibernateExceptionClassifier.kt @@ -1,8 +1,8 @@ package misk.hibernate +import javax.persistence.OptimisticLockException import misk.jdbc.DataSourceType import misk.jdbc.retry.DefaultExceptionClassifier -import javax.persistence.OptimisticLockException import org.hibernate.StaleObjectStateException import org.hibernate.exception.LockAcquisitionException @@ -14,9 +14,8 @@ import org.hibernate.exception.LockAcquisitionException * - [LockAcquisitionException]: Database lock acquisition failure * - [OptimisticLockException]: JPA optimistic locking failure */ -internal class HibernateExceptionClassifier @JvmOverloads constructor( - dataSourceType: DataSourceType? = null -) : DefaultExceptionClassifier(dataSourceType) { +internal class HibernateExceptionClassifier @JvmOverloads constructor(dataSourceType: DataSourceType? = null) : + DefaultExceptionClassifier(dataSourceType) { override fun isRetryable(th: Throwable): Boolean { return when (th) { diff --git a/misk-hibernate/src/main/kotlin/misk/hibernate/RealTransacter.kt b/misk-hibernate/src/main/kotlin/misk/hibernate/RealTransacter.kt index 4c969058219..48d8030299a 100644 --- a/misk-hibernate/src/main/kotlin/misk/hibernate/RealTransacter.kt +++ b/misk-hibernate/src/main/kotlin/misk/hibernate/RealTransacter.kt @@ -33,9 +33,7 @@ import misk.vitess.Shard import misk.vitess.Shard.Companion.SINGLE_SHARD_SET import org.hibernate.FlushMode import org.hibernate.SessionFactory -import org.hibernate.StaleObjectStateException import org.hibernate.exception.ConstraintViolationException -import org.hibernate.exception.LockAcquisitionException import org.hibernate.resource.jdbc.spi.PhysicalConnectionHandlingMode private val logger = getLogger() @@ -250,15 +248,17 @@ private constructor( } private fun transactionWithRetriesInternal(block: () -> T): T { - val backoff = ExponentialBackoff( - baseDelay = Duration.ofMillis(options.minRetryDelayMillis), - maxDelay = Duration.ofMillis(options.maxRetryDelayMillis), - jitter = Duration.ofMillis(options.retryJitterMillis) - ) - val retryConfig = RetryConfig.Builder(options.maxAttempts, backoff) - .shouldRetry { exceptionClassifier.isRetryable(it) } - .onRetry { attempt, e -> logger.info(e) { "$qualifierName transaction failed, retrying (attempt $attempt)" } } - .build() + val backoff = + ExponentialBackoff( + baseDelay = Duration.ofMillis(options.minRetryDelayMillis), + maxDelay = Duration.ofMillis(options.maxRetryDelayMillis), + jitter = Duration.ofMillis(options.retryJitterMillis), + ) + val retryConfig = + RetryConfig.Builder(options.maxAttempts, backoff) + .shouldRetry { exceptionClassifier.isRetryable(it) } + .onRetry { attempt, e -> logger.info(e) { "$qualifierName transaction failed, retrying (attempt $attempt)" } } + .build() return retry(retryConfig) { block() } } diff --git a/misk-hibernate/src/main/kotlin/misk/hibernate/Session.kt b/misk-hibernate/src/main/kotlin/misk/hibernate/Session.kt index bd2af5b7a9d..e7d598faab8 100644 --- a/misk-hibernate/src/main/kotlin/misk/hibernate/Session.kt +++ b/misk-hibernate/src/main/kotlin/misk/hibernate/Session.kt @@ -67,23 +67,24 @@ private val logger = getLogger() * database will throw a [java.sql.SQLException] (`Unknown system variable 'transaction_mode'`). */ fun Session.allowCrossShardTransactions() { - hibernateSession.doWork { connection -> - connection.createStatement().execute("SET transaction_mode = 'multi'") - } + hibernateSession.doWork { connection -> connection.createStatement().execute("SET transaction_mode = 'multi'") } // Reset to UNSPECIFIED after the transaction completes (commit or rollback) so it doesn't leak // to subsequent transactions via connection pool reuse. UNSPECIFIED falls back to whatever the // vtgate-level default is, so this is safe regardless of the vtgate's configured transaction mode. - hibernateSession.transaction.registerSynchronization(object : Synchronization { - override fun beforeCompletion() {} - override fun afterCompletion(status: Int) { - try { - hibernateSession.doWork { connection -> - connection.createStatement().execute("SET transaction_mode = 'unspecified'") + hibernateSession.transaction.registerSynchronization( + object : Synchronization { + override fun beforeCompletion() {} + + override fun afterCompletion(status: Int) { + try { + hibernateSession.doWork { connection -> + connection.createStatement().execute("SET transaction_mode = 'unspecified'") + } + } catch (e: Exception) { + logger.error(e) { "Failed to reset transaction_mode after transaction completion" } } - } catch (e: Exception) { - logger.error(e) { "Failed to reset transaction_mode after transaction completion" } } } - }) + ) } diff --git a/misk-hibernate/src/test/kotlin/misk/hibernate/HibernateExceptionClassifierTest.kt b/misk-hibernate/src/test/kotlin/misk/hibernate/HibernateExceptionClassifierTest.kt index a3fbd332b92..54fc75c7e7a 100644 --- a/misk-hibernate/src/test/kotlin/misk/hibernate/HibernateExceptionClassifierTest.kt +++ b/misk-hibernate/src/test/kotlin/misk/hibernate/HibernateExceptionClassifierTest.kt @@ -68,9 +68,7 @@ class HibernateExceptionClassifierTest { @Test fun `inherits database-specific behavior from base classifier`() { val classifier = HibernateExceptionClassifier(DataSourceType.VITESS_MYSQL) - val exception = SQLException( - "vttablet: rpc error: code = Aborted desc = transaction 123: not found" - ) + val exception = SQLException("vttablet: rpc error: code = Aborted desc = transaction 123: not found") assertThat(classifier.isRetryable(exception)).isTrue() } } diff --git a/misk-hibernate/src/test/kotlin/misk/hibernate/vitess/EmptyInScatterIntegrationTest.kt b/misk-hibernate/src/test/kotlin/misk/hibernate/vitess/EmptyInScatterIntegrationTest.kt index 1626a1fe0b5..38279689205 100644 --- a/misk-hibernate/src/test/kotlin/misk/hibernate/vitess/EmptyInScatterIntegrationTest.kt +++ b/misk-hibernate/src/test/kotlin/misk/hibernate/vitess/EmptyInScatterIntegrationTest.kt @@ -22,20 +22,18 @@ import org.junit.jupiter.api.Test import org.junit.jupiter.api.assertThrows /** - * Demonstrates that an empty `IN` collection on a sharding-key column causes a scatter plan at - * vtgate. After cashapp/misk#3795 disabled server-side prepared statements for VITESS_MYSQL, - * Hibernate renders an empty `IN` predicate by dropping it from the SQL entirely — so a query - * like `WHERE id IN ()` is sent to vtgate as a fully unconstrained `SELECT`, which the planner - * resolves to `engine.Scatter` because no vindex predicate remains. The `--no-scatter` vtgate - * flag then rejects the plan with `ScatterQueryException`. + * Demonstrates that an empty `IN` collection on a sharding-key column causes a scatter plan at vtgate. After + * cashapp/misk#3795 disabled server-side prepared statements for VITESS_MYSQL, Hibernate renders an empty `IN` + * predicate by dropping it from the SQL entirely — so a query like `WHERE id IN ()` is sent to vtgate as a fully + * unconstrained `SELECT`, which the planner resolves to `engine.Scatter` because no vindex predicate remains. The + * `--no-scatter` vtgate flag then rejects the plan with `ScatterQueryException`. * * Three mitigations are validated: - * - * 1. `.allowScatter()` query hint — opts the query into a real scatter at vtgate. - * 2. Caller-side empty-collection guard — short-circuit before issuing the query. Recommended - * for any code path where an `IN` collection on the sharding key can legitimately be empty, - * since an empty `IN` can never match anything and the query would always return zero rows. - * 3. (Future, not tested here) Auto short-circuit inside misk-hibernate's query renderer. + * 1. `.allowScatter()` query hint — opts the query into a real scatter at vtgate. + * 2. Caller-side empty-collection guard — short-circuit before issuing the query. Recommended for any code path where + * an `IN` collection on the sharding key can legitimately be empty, since an empty `IN` can never match anything and + * the query would always return zero rows. + * 3. (Future, not tested here) Auto short-circuit inside misk-hibernate's query renderer. */ @MiskTest(startService = true) class EmptyInScatterIntegrationTest { @@ -76,9 +74,7 @@ class EmptyInScatterIntegrationTest { // entirely different routes because the SQL it receives is different. val exception = assertThrows { - transacter.transaction { session -> - queryFactory.newQuery().idIn(emptyList()).list(session) - } + transacter.transaction { session -> queryFactory.newQuery().idIn(emptyList()).list(session) } } assertThat(exception.cause).isInstanceOf(ScatterQueryException::class.java) diff --git a/misk-hibernate/src/test/kotlin/misk/hibernate/vitess/VitessTransactionModeIntegrationTest.kt b/misk-hibernate/src/test/kotlin/misk/hibernate/vitess/VitessTransactionModeIntegrationTest.kt index f96a4083005..0ac9e76806b 100644 --- a/misk-hibernate/src/test/kotlin/misk/hibernate/vitess/VitessTransactionModeIntegrationTest.kt +++ b/misk-hibernate/src/test/kotlin/misk/hibernate/vitess/VitessTransactionModeIntegrationTest.kt @@ -27,8 +27,8 @@ import org.junit.jupiter.api.Test import org.junit.jupiter.api.assertThrows /** - * Test suite that verifies that cross-shard transactions (reads and writes) are rejected in SINGLE - * transaction mode and can be opted in via [allowCrossShardTransactions]. + * Test suite that verifies that cross-shard transactions (reads and writes) are rejected in SINGLE transaction mode and + * can be opted in via [allowCrossShardTransactions]. */ @MiskTest(startService = true) class VitessTransactionModeIntegrationTest { @@ -40,8 +40,7 @@ class VitessTransactionModeIntegrationTest { port = 29303, ) - @MiskTestModule - val module = MoviesTestModule(type = DataSourceType.VITESS_MYSQL, singleTransactionMode = true) + @MiskTestModule val module = MoviesTestModule(type = DataSourceType.VITESS_MYSQL, singleTransactionMode = true) @Inject @Movies lateinit var transacter: Transacter @Inject lateinit var queryFactory: Query.Factory @@ -56,14 +55,11 @@ class VitessTransactionModeIntegrationTest { val shard2 = Shard(keyspace, "80-") // Insert movies one at a time in separate transactions (each is single-shard, so succeeds). - val movieIds = (1..10).map { i -> - transacter.transaction { session -> session.save(DbMovie("Movie $i")) } - } + val movieIds = (1..10).map { i -> transacter.transaction { session -> session.save(DbMovie("Movie $i")) } } // Partition IDs by shard and pick one from each. val byShard = movieIds.groupBy { id -> if (shard1.contains(Shard.Key.hash(id.id))) shard1 else shard2 } - assertThat(byShard.keys).hasSize(2) - .withFailMessage("All movie IDs landed on the same shard: $movieIds") + assertThat(byShard.keys).hasSize(2).withFailMessage("All movie IDs landed on the same shard: $movieIds") crossShardIdA = byShard[shard1]!!.first() crossShardIdB = byShard[shard2]!!.first() } @@ -77,9 +73,7 @@ class VitessTransactionModeIntegrationTest { @Test fun `cross-shard read succeeds in SINGLE transaction mode`() { // Vitess v23+ allows read-only multi-shard transactions in SINGLE mode (vitessio/vitess#18173). - val movies = transacter.transaction { session -> - queryFactory.newQuery().allowScatter().list(session) - } + val movies = transacter.transaction { session -> queryFactory.newQuery().allowScatter().list(session) } assertThat(movies).isNotEmpty() } @@ -102,14 +96,15 @@ class VitessTransactionModeIntegrationTest { @Test fun `multi-shard write fails with SINGLE transaction mode`() { // Writing to multiple shards in the same transaction is rejected by SINGLE mode. - val exception = assertThrows { - transacter.transaction { session -> - val movieA = session.load(crossShardIdA) - val movieB = session.load(crossShardIdB) - movieA.name = "Updated A" - movieB.name = "Updated B" + val exception = + assertThrows { + transacter.transaction { session -> + val movieA = session.load(crossShardIdA) + val movieB = session.load(crossShardIdB) + movieA.name = "Updated A" + movieB.name = "Updated B" + } } - } assertThat(generateSequence(exception as Throwable) { it.cause }.any { it is CrossShardTransactionException }) .isTrue() @@ -142,14 +137,15 @@ class VitessTransactionModeIntegrationTest { // Second: without opt-in, a cross-shard write should still fail. // If this passes (doesn't throw), the session variable leaked through the connection pool. - val exception = assertThrows { - transacter.transaction { session -> - val movieA = session.load(crossShardIdA) - val movieB = session.load(crossShardIdB) - movieA.name = "Should Fail A" - movieB.name = "Should Fail B" + val exception = + assertThrows { + transacter.transaction { session -> + val movieA = session.load(crossShardIdA) + val movieB = session.load(crossShardIdB) + movieA.name = "Should Fail A" + movieB.name = "Should Fail B" + } } - } assertThat(generateSequence(exception as Throwable) { it.cause }.any { it is CrossShardTransactionException }) .isTrue() diff --git a/misk-jdbc/src/main/kotlin/misk/jdbc/DeclarativeSchemaMigrator.kt b/misk-jdbc/src/main/kotlin/misk/jdbc/DeclarativeSchemaMigrator.kt index 8cf7fa0d05c..2c0b12beadc 100644 --- a/misk-jdbc/src/main/kotlin/misk/jdbc/DeclarativeSchemaMigrator.kt +++ b/misk-jdbc/src/main/kotlin/misk/jdbc/DeclarativeSchemaMigrator.kt @@ -4,8 +4,8 @@ import java.io.File import java.sql.ResultSet import java.util.regex.Pattern import misk.logging.getLogger -import misk.spirit.Spirit import misk.resources.ResourceLoader +import misk.spirit.Spirit import net.sf.jsqlparser.parser.CCJSqlParserUtil import net.sf.jsqlparser.statement.create.table.CreateTable @@ -36,10 +36,7 @@ internal class DeclarativeSchemaMigrator( dataSourceService.dataSource.connection.use { conn -> conn.createStatement().use { stmt -> // Spirit outputs multiple DDL statements (one per line), execute each individually. - ddl.lines() - .map { it.trim() } - .filter { it.isNotBlank() } - .forEach { stmt.execute(it) } + ddl.lines().map { it.trim() }.filter { it.isNotBlank() }.forEach { stmt.execute(it) } } } } diff --git a/misk-jdbc/src/main/kotlin/misk/jdbc/SchemaMigratorFactory.kt b/misk-jdbc/src/main/kotlin/misk/jdbc/SchemaMigratorFactory.kt index d2ec44f04e9..911223a8630 100644 --- a/misk-jdbc/src/main/kotlin/misk/jdbc/SchemaMigratorFactory.kt +++ b/misk-jdbc/src/main/kotlin/misk/jdbc/SchemaMigratorFactory.kt @@ -4,10 +4,7 @@ import kotlin.reflect.KClass import misk.resources.ResourceLoader import misk.spirit.Spirit -/** - * Creates a [SchemaMigrator] without Guice. Useful for standalone tools like the - * schema-migrator Gradle plugin. - */ +/** Creates a [SchemaMigrator] without Guice. Useful for standalone tools like the schema-migrator Gradle plugin. */ fun createSchemaMigrator( qualifier: KClass, config: DataSourceConfig, diff --git a/misk-jdbc/src/main/kotlin/misk/jdbc/SchemaMigratorRunner.kt b/misk-jdbc/src/main/kotlin/misk/jdbc/SchemaMigratorRunner.kt index d7d6973cb41..3e4c51dcc68 100644 --- a/misk-jdbc/src/main/kotlin/misk/jdbc/SchemaMigratorRunner.kt +++ b/misk-jdbc/src/main/kotlin/misk/jdbc/SchemaMigratorRunner.kt @@ -6,11 +6,11 @@ import misk.resources.ResourceLoader import wisp.deployment.TESTING /** - * Standalone entry point for running schema migrations without Guice. - * Used by the schema-migrator Gradle plugin via JavaExec. + * Standalone entry point for running schema migrations without Guice. Used by the schema-migrator Gradle plugin via + * JavaExec. * - * Reads configuration from stdin as a Properties file to avoid leaking - * sensitive values (like passwords) in process argument lists. + * Reads configuration from stdin as a Properties file to avoid leaking sensitive values (like passwords) in process + * argument lists. */ object SchemaMigratorRunner { @Qualifier @@ -22,35 +22,38 @@ object SchemaMigratorRunner { val props = Properties() props.load(System.`in`) - val config = DataSourceConfig( - host = props.getProperty("host")?.ifBlank { null }, - port = props.getProperty("port")?.ifBlank { null }?.toIntOrNull(), - type = DataSourceType.valueOf(props.getProperty("databaseType")), - migrations_resource = props.getProperty("migrationsResource"), - database = props.getProperty("database"), - username = props.getProperty("username"), - password = props.getProperty("password"), - migrations_format = MigrationsFormat.valueOf(props.getProperty("migrationsFormat")), - ) + val config = + DataSourceConfig( + host = props.getProperty("host")?.ifBlank { null }, + port = props.getProperty("port")?.ifBlank { null }?.toIntOrNull(), + type = DataSourceType.valueOf(props.getProperty("databaseType")), + migrations_resource = props.getProperty("migrationsResource"), + database = props.getProperty("database"), + username = props.getProperty("username"), + password = props.getProperty("password"), + migrations_format = MigrationsFormat.valueOf(props.getProperty("migrationsFormat")), + ) val configWithDefaults = config.withDefaults() - val dataSourceService = DataSourceService( - qualifier = SchemaMigratorDatabase::class, - baseConfig = configWithDefaults, - deployment = TESTING, - dataSourceDecorators = emptySet(), - databasePool = RealDatabasePool, - ) + val dataSourceService = + DataSourceService( + qualifier = SchemaMigratorDatabase::class, + baseConfig = configWithDefaults, + deployment = TESTING, + dataSourceDecorators = emptySet(), + databasePool = RealDatabasePool, + ) dataSourceService.startAsync().awaitRunning() try { - val migrator = createSchemaMigrator( - qualifier = SchemaMigratorDatabase::class, - config = configWithDefaults, - dataSourceService = dataSourceService, - resourceLoader = ResourceLoader.SYSTEM, - ) + val migrator = + createSchemaMigrator( + qualifier = SchemaMigratorDatabase::class, + config = configWithDefaults, + dataSourceService = dataSourceService, + resourceLoader = ResourceLoader.SYSTEM, + ) migrator.applyAll("SchemaMigratorPlugin") } finally { dataSourceService.stopAsync().awaitTerminated() diff --git a/misk-jdbc/src/main/kotlin/misk/jdbc/retry/DefaultExceptionClassifier.kt b/misk-jdbc/src/main/kotlin/misk/jdbc/retry/DefaultExceptionClassifier.kt index 2248b92d458..6f53b6974bb 100644 --- a/misk-jdbc/src/main/kotlin/misk/jdbc/retry/DefaultExceptionClassifier.kt +++ b/misk-jdbc/src/main/kotlin/misk/jdbc/retry/DefaultExceptionClassifier.kt @@ -1,9 +1,9 @@ package misk.jdbc.retry -import misk.jdbc.DataSourceType import java.sql.SQLException import java.sql.SQLRecoverableException import java.sql.SQLTransientException +import misk.jdbc.DataSourceType /** * Default exception classifier that handles common SQL-level retryable exceptions. @@ -11,16 +11,15 @@ import java.sql.SQLTransientException * This classifier handles: * - SQLRecoverableException, SQLTransientException: Standard JDBC recoverable exceptions * - RetryTransactionException: Explicit retry request - * - SQLException with specific messages: Connection closed, Vitess transaction not found, - * CockroachDB restart transaction, TiDB write conflict + * - SQLException with specific messages: Connection closed, Vitess transaction not found, CockroachDB restart + * transaction, TiDB write conflict * * Subclasses can extend this to add ORM-specific exception handling. * * @param dataSourceType The type of database, used to enable database-specific retry logic */ -open class DefaultExceptionClassifier @JvmOverloads constructor( - private val dataSourceType: DataSourceType? = null -) : ExceptionClassifier { +open class DefaultExceptionClassifier @JvmOverloads constructor(private val dataSourceType: DataSourceType? = null) : + ExceptionClassifier { override fun isRetryable(th: Throwable): Boolean { return when (th) { @@ -32,12 +31,11 @@ open class DefaultExceptionClassifier @JvmOverloads constructor( } } - private fun isMessageRetryable(th: SQLException) = - isConnectionClosed(th) || isDatabaseSpecificRetryable(th) + private fun isMessageRetryable(th: SQLException) = isConnectionClosed(th) || isDatabaseSpecificRetryable(th) /** - * This is thrown as a raw SQLException from Hikari even though it is most certainly a - * recoverable exception. See com/zaxxer/hikari/pool/ProxyConnection.java:493 + * This is thrown as a raw SQLException from Hikari even though it is most certainly a recoverable exception. See + * com/zaxxer/hikari/pool/ProxyConnection.java:493 */ private fun isConnectionClosed(th: SQLException) = th.message == "Connection is closed" @@ -60,16 +58,14 @@ open class DefaultExceptionClassifier @JvmOverloads constructor( } } - /** - * Vitess-specific retryable exceptions. - */ + /** Vitess-specific retryable exceptions. */ private fun isVitessRetryable(th: SQLException): Boolean { return isVitessTransactionNotFound(th) } /** - * We get this error as a MySQLQueryInterruptedException when a tablet gracefully terminates, - * we just need to retry the transaction and the new primary should handle it. + * We get this error as a MySQLQueryInterruptedException when a tablet gracefully terminates, we just need to retry + * the transaction and the new primary should handle it. * * ``` * vttablet: rpc error: code = Aborted desc = transaction 1572922696317821557: @@ -83,10 +79,9 @@ open class DefaultExceptionClassifier @JvmOverloads constructor( /** * CockroachDB-specific retryable exceptions. * - * "Messages with the error code 40001 and the string restart transaction indicate that a - * transaction failed because it conflicted with another concurrent or recent transaction - * accessing the same data. The transaction needs to be retried by the client." - * https://www.cockroachlabs.com/docs/stable/common-errors.html#restart-transaction + * "Messages with the error code 40001 and the string restart transaction indicate that a transaction failed because + * it conflicted with another concurrent or recent transaction accessing the same data. The transaction needs to be + * retried by the client." https://www.cockroachlabs.com/docs/stable/common-errors.html#restart-transaction */ private fun isCockroachRetryable(th: SQLException): Boolean { return th.errorCode == 40001 && messageContainsAll(th, "restart transaction") @@ -95,9 +90,8 @@ open class DefaultExceptionClassifier @JvmOverloads constructor( /** * TiDB-specific retryable exceptions. * - * "Transactions in TiKV encounter write conflicts". This can happen when optimistic transaction - * mode is on. Conflicts are detected during transaction commit - * https://docs.pingcap.com/tidb/dev/tidb-faq#error-9007-hy000-write-conflict + * "Transactions in TiKV encounter write conflicts". This can happen when optimistic transaction mode is on. Conflicts + * are detected during transaction commit https://docs.pingcap.com/tidb/dev/tidb-faq#error-9007-hy000-write-conflict */ private fun isTidbRetryable(th: SQLException): Boolean { return th.errorCode == 9007 diff --git a/misk-jdbc/src/main/kotlin/misk/jdbc/retry/ExceptionClassifier.kt b/misk-jdbc/src/main/kotlin/misk/jdbc/retry/ExceptionClassifier.kt index b8054817f5a..21b82ac40b1 100644 --- a/misk-jdbc/src/main/kotlin/misk/jdbc/retry/ExceptionClassifier.kt +++ b/misk-jdbc/src/main/kotlin/misk/jdbc/retry/ExceptionClassifier.kt @@ -1,11 +1,7 @@ package misk.jdbc.retry -/** - * Interface for classifying exceptions to determine if a transaction should be retried. - */ +/** Interface for classifying exceptions to determine if a transaction should be retried. */ interface ExceptionClassifier { - /** - * Determines if the given throwable should trigger a transaction retry. - */ + /** Determines if the given throwable should trigger a transaction retry. */ fun isRetryable(th: Throwable): Boolean } diff --git a/misk-jdbc/src/main/kotlin/misk/jdbc/retry/RetryDefaults.kt b/misk-jdbc/src/main/kotlin/misk/jdbc/retry/RetryDefaults.kt index bb9b8339fe3..6974310c036 100644 --- a/misk-jdbc/src/main/kotlin/misk/jdbc/retry/RetryDefaults.kt +++ b/misk-jdbc/src/main/kotlin/misk/jdbc/retry/RetryDefaults.kt @@ -1,8 +1,6 @@ package misk.jdbc.retry -/** - * Default retry configuration values shared across transacter implementations. - */ +/** Default retry configuration values shared across transacter implementations. */ object RetryDefaults { const val MAX_ATTEMPTS: Int = 3 const val MIN_RETRY_DELAY_MILLIS: Long = 100 diff --git a/misk-jdbc/src/main/kotlin/misk/jdbc/retry/RetryTransactionException.kt b/misk-jdbc/src/main/kotlin/misk/jdbc/retry/RetryTransactionException.kt index e604fa30cbc..6b09ed1067c 100644 --- a/misk-jdbc/src/main/kotlin/misk/jdbc/retry/RetryTransactionException.kt +++ b/misk-jdbc/src/main/kotlin/misk/jdbc/retry/RetryTransactionException.kt @@ -1,10 +1,8 @@ package misk.jdbc.retry /** - * An exception that signals that the current transaction should be retried. - * This can be thrown by application code to trigger a retry without indicating a failure. + * An exception that signals that the current transaction should be retried. This can be thrown by application code to + * trigger a retry without indicating a failure. */ -open class RetryTransactionException @JvmOverloads constructor( - message: String? = null, - cause: Throwable? = null -) : Exception(message, cause) +open class RetryTransactionException @JvmOverloads constructor(message: String? = null, cause: Throwable? = null) : + Exception(message, cause) diff --git a/misk-jdbc/src/test/kotlin/misk/jdbc/RealTransacterTest.kt b/misk-jdbc/src/test/kotlin/misk/jdbc/RealTransacterTest.kt index 591070dc537..781ba460d42 100644 --- a/misk-jdbc/src/test/kotlin/misk/jdbc/RealTransacterTest.kt +++ b/misk-jdbc/src/test/kotlin/misk/jdbc/RealTransacterTest.kt @@ -14,9 +14,9 @@ import misk.backoff.RetryConfig import misk.backoff.retry import misk.config.Config import misk.config.MiskConfig -import misk.jdbc.retry.RetryTransactionException import misk.environment.DeploymentModule import misk.inject.KAbstractModule +import misk.jdbc.retry.RetryTransactionException import misk.testing.MiskTest import misk.testing.MiskTestModule import misk.testing.MockTracingBackendModule diff --git a/misk-jdbc/src/test/kotlin/misk/jdbc/retry/DefaultExceptionClassifierTest.kt b/misk-jdbc/src/test/kotlin/misk/jdbc/retry/DefaultExceptionClassifierTest.kt index 03fcc35e0c5..51a32840210 100644 --- a/misk-jdbc/src/test/kotlin/misk/jdbc/retry/DefaultExceptionClassifierTest.kt +++ b/misk-jdbc/src/test/kotlin/misk/jdbc/retry/DefaultExceptionClassifierTest.kt @@ -67,63 +67,61 @@ class DefaultExceptionClassifierTest { @Test fun `Vitess transaction not found is retryable when dataSourceType is VITESS_MYSQL`() { val classifier = DefaultExceptionClassifier(DataSourceType.VITESS_MYSQL) - val exception = SQLException( - "vttablet: rpc error: code = Aborted desc = transaction 123: not found" - ) + val exception = SQLException("vttablet: rpc error: code = Aborted desc = transaction 123: not found") assertThat(classifier.isRetryable(exception)).isTrue() } @Test fun `Vitess transaction not found is not retryable when dataSourceType is MYSQL`() { val classifier = DefaultExceptionClassifier(DataSourceType.MYSQL) - val exception = SQLException( - "vttablet: rpc error: code = Aborted desc = transaction 123: not found" - ) + val exception = SQLException("vttablet: rpc error: code = Aborted desc = transaction 123: not found") assertThat(classifier.isRetryable(exception)).isFalse() } @Test fun `Vitess transaction not found is not retryable when dataSourceType is null`() { val classifier = DefaultExceptionClassifier() - val exception = SQLException( - "vttablet: rpc error: code = Aborted desc = transaction 123: not found" - ) + val exception = SQLException("vttablet: rpc error: code = Aborted desc = transaction 123: not found") assertThat(classifier.isRetryable(exception)).isFalse() } @Test fun `CockroachDB restart transaction is retryable when dataSourceType is COCKROACHDB`() { val classifier = DefaultExceptionClassifier(DataSourceType.COCKROACHDB) - val exception = object : SQLException("restart transaction") { - override fun getErrorCode() = 40001 - } + val exception = + object : SQLException("restart transaction") { + override fun getErrorCode() = 40001 + } assertThat(classifier.isRetryable(exception)).isTrue() } @Test fun `CockroachDB restart transaction is not retryable when dataSourceType is MYSQL`() { val classifier = DefaultExceptionClassifier(DataSourceType.MYSQL) - val exception = object : SQLException("restart transaction") { - override fun getErrorCode() = 40001 - } + val exception = + object : SQLException("restart transaction") { + override fun getErrorCode() = 40001 + } assertThat(classifier.isRetryable(exception)).isFalse() } @Test fun `TiDB write conflict is retryable when dataSourceType is TIDB`() { val classifier = DefaultExceptionClassifier(DataSourceType.TIDB) - val exception = object : SQLException("write conflict") { - override fun getErrorCode() = 9007 - } + val exception = + object : SQLException("write conflict") { + override fun getErrorCode() = 9007 + } assertThat(classifier.isRetryable(exception)).isTrue() } @Test fun `TiDB write conflict is not retryable when dataSourceType is MYSQL`() { val classifier = DefaultExceptionClassifier(DataSourceType.MYSQL) - val exception = object : SQLException("write conflict") { - override fun getErrorCode() = 9007 - } + val exception = + object : SQLException("write conflict") { + override fun getErrorCode() = 9007 + } assertThat(classifier.isRetryable(exception)).isFalse() } } diff --git a/misk-jdbc/src/testFixtures/kotlin/misk/jdbc/TestDatabasePool.kt b/misk-jdbc/src/testFixtures/kotlin/misk/jdbc/TestDatabasePool.kt index 1a213b8eace..f5c5ed3b2e6 100644 --- a/misk-jdbc/src/testFixtures/kotlin/misk/jdbc/TestDatabasePool.kt +++ b/misk-jdbc/src/testFixtures/kotlin/misk/jdbc/TestDatabasePool.kt @@ -49,9 +49,7 @@ class SharedLeaseDatabasePool(private val delegate: DatabasePool) : DatabasePool override fun releaseDatabase(config: DataSourceConfig): Unit = synchronized(this) { val key = - leasesByKey.entries - .firstOrNull { (_, lease) -> lease.config.database == config.database } - ?.key ?: return + leasesByKey.entries.firstOrNull { (_, lease) -> lease.config.database == config.database }?.key ?: return val lease = leasesByKey.getValue(key) lease.referenceCount -= 1 @@ -63,17 +61,9 @@ class SharedLeaseDatabasePool(private val delegate: DatabasePool) : DatabasePool private fun DataSourceConfig.leaseKey() = LeaseKey(type = type, host = host, port = port, database = database) - private data class Lease( - val config: DataSourceConfig, - var referenceCount: Int, - ) + private data class Lease(val config: DataSourceConfig, var referenceCount: Int) - private data class LeaseKey( - val type: DataSourceType, - val host: String?, - val port: Int?, - val database: String?, - ) + private data class LeaseKey(val type: DataSourceType, val host: String?, val port: Int?, val database: String?) } /** diff --git a/misk-jooq/src/main/kotlin/misk/jooq/JooqExceptionClassifier.kt b/misk-jooq/src/main/kotlin/misk/jooq/JooqExceptionClassifier.kt index d1e79f4df12..9c950cec297 100644 --- a/misk-jooq/src/main/kotlin/misk/jooq/JooqExceptionClassifier.kt +++ b/misk-jooq/src/main/kotlin/misk/jooq/JooqExceptionClassifier.kt @@ -8,21 +8,19 @@ import org.jooq.exception.DataChangedException * Exception classifier for jOOQ transactions. * * Extends [DefaultExceptionClassifier] to add jOOQ-specific retryable exceptions: - * - [DataChangedException]: jOOQ's optimistic-locking conflict raised by - * `UpdatableRecord.store()`/`refresh()` when the underlying row has changed. Retrying re-reads - * the latest state and re-applies the change. + * - [DataChangedException]: jOOQ's optimistic-locking conflict raised by `UpdatableRecord.store()`/`refresh()` when the + * underlying row has changed. Retrying re-reads the latest state and re-applies the change. * - * Other [org.jooq.exception.DataAccessException] subclasses are intentionally **not** retried as a - * class — they are deterministic failures (constraint violations, type/mapping errors, no-data / - * too-many-rows, dialect mismatches, etc.) and will not change on retry. + * Other [org.jooq.exception.DataAccessException] subclasses are intentionally **not** retried as a class — they are + * deterministic failures (constraint violations, type/mapping errors, no-data / too-many-rows, dialect mismatches, + * etc.) and will not change on retry. * - * A wrapping [org.jooq.exception.DataAccessException] is still retryable when its underlying cause - * is retryable (e.g. a [java.sql.SQLRecoverableException] or a Vitess "transaction not found" - * SQLException). The base classifier's cause-walking logic handles that path. + * A wrapping [org.jooq.exception.DataAccessException] is still retryable when its underlying cause is retryable (e.g. a + * [java.sql.SQLRecoverableException] or a Vitess "transaction not found" SQLException). The base classifier's + * cause-walking logic handles that path. */ -internal class JooqExceptionClassifier( - dataSourceType: DataSourceType? = null -) : DefaultExceptionClassifier(dataSourceType) { +internal class JooqExceptionClassifier(dataSourceType: DataSourceType? = null) : + DefaultExceptionClassifier(dataSourceType) { override fun isRetryable(th: Throwable): Boolean { return when (th) { diff --git a/misk-jooq/src/test/kotlin/misk/jooq/JooqExceptionClassifierTest.kt b/misk-jooq/src/test/kotlin/misk/jooq/JooqExceptionClassifierTest.kt index 14a5f597155..18d1769f292 100644 --- a/misk-jooq/src/test/kotlin/misk/jooq/JooqExceptionClassifierTest.kt +++ b/misk-jooq/src/test/kotlin/misk/jooq/JooqExceptionClassifierTest.kt @@ -100,27 +100,21 @@ class JooqExceptionClassifierTest { @Test fun `inherits database-specific behavior from base classifier`() { val classifier = JooqExceptionClassifier(DataSourceType.VITESS_MYSQL) - val exception = SQLException( - "vttablet: rpc error: code = Aborted desc = transaction 123: not found" - ) + val exception = SQLException("vttablet: rpc error: code = Aborted desc = transaction 123: not found") assertThat(classifier.isRetryable(exception)).isTrue() } @Test fun `Vitess exception not retryable without correct DataSourceType`() { val classifier = JooqExceptionClassifier(DataSourceType.MYSQL) - val exception = SQLException( - "vttablet: rpc error: code = Aborted desc = transaction 123: not found" - ) + val exception = SQLException("vttablet: rpc error: code = Aborted desc = transaction 123: not found") assertThat(classifier.isRetryable(exception)).isFalse() } @Test fun `DataAccessException wrapping a Vitess SQL cause is retryable for VITESS_MYSQL`() { val classifier = JooqExceptionClassifier(DataSourceType.VITESS_MYSQL) - val cause = SQLException( - "vttablet: rpc error: code = Aborted desc = transaction 123: not found" - ) + val cause = SQLException("vttablet: rpc error: code = Aborted desc = transaction 123: not found") assertThat(classifier.isRetryable(DataAccessException("error", cause))).isTrue() } } diff --git a/misk-mcp/src/main/kotlin/misk/mcp/McpResourceTemplate.kt b/misk-mcp/src/main/kotlin/misk/mcp/McpResourceTemplate.kt index 786f54894bd..535a42ca7cb 100644 --- a/misk-mcp/src/main/kotlin/misk/mcp/McpResourceTemplate.kt +++ b/misk-mcp/src/main/kotlin/misk/mcp/McpResourceTemplate.kt @@ -7,10 +7,9 @@ import misk.annotation.ExperimentalMiskApi /** * Abstraction for a resource template in the Model Context Protocol (MCP) specification. * - * Resource templates are parameterized resources that use RFC 6570 URI templates - * (e.g., `schema://database/{tableName}`) to match multiple URIs dynamically. - * When a client reads a URI matching the template, the server extracts the variable - * values and passes them to the handler. + * Resource templates are parameterized resources that use RFC 6570 URI templates (e.g., + * `schema://database/{tableName}`) to match multiple URIs dynamically. When a client reads a URI matching the template, + * the server extracts the variable values and passes them to the handler. * * ## Implementation Requirements * @@ -105,8 +104,8 @@ interface McpResourceTemplate { * * @param request The incoming resource read request containing the resolved URI * @param variables Map of extracted URI template variable names to their matched values. For example, template - * `schema://database/{tableName}` matched against `schema://database/users` produces - * `mapOf("tableName" to "users")`. + * `schema://database/{tableName}` matched against `schema://database/users` produces `mapOf("tableName" to + * "users")`. * @return The resource content with appropriate MIME type and encoding * @throws Exception if the resource access fails catastrophically */ diff --git a/misk-mcp/src/main/kotlin/misk/mcp/McpResourceTemplateModule.kt b/misk-mcp/src/main/kotlin/misk/mcp/McpResourceTemplateModule.kt index faafd0ec85d..54a49ef0b75 100644 --- a/misk-mcp/src/main/kotlin/misk/mcp/McpResourceTemplateModule.kt +++ b/misk-mcp/src/main/kotlin/misk/mcp/McpResourceTemplateModule.kt @@ -26,8 +26,8 @@ import misk.inject.qualifier * * ## Resource Template Grouping with BindingQualifiers * - * Resource templates can be organized into groups using [BindingQualifier] annotations. This allows multiple MCP servers - * to expose different sets of resource templates: + * Resource templates can be organized into groups using [BindingQualifier] annotations. This allows multiple MCP + * servers to expose different sets of resource templates: * ```kotlin * install(McpResourceTemplateModule.create()) * install(McpResourceTemplateModule.create()) @@ -43,10 +43,8 @@ import misk.inject.qualifier */ @ExperimentalMiskApi class McpResourceTemplateModule -private constructor( - private val resourceTemplateClass: KClass, - private val qualifier: BindingQualifier?, -) : KAbstractModule() { +private constructor(private val resourceTemplateClass: KClass, private val qualifier: BindingQualifier?) : + KAbstractModule() { override fun configure() { multibind(qualifier).to(resourceTemplateClass.java) @@ -61,13 +59,18 @@ private constructor( * * @param RT The type of [McpResourceTemplate] implementation to register * @param resourceTemplateClass The [KClass] of the resource template implementation - * @param groupAnnotationClass Optional annotation class for grouping this resource template with a specific MCP server + * @param groupAnnotationClass Optional annotation class for grouping this resource template with a specific MCP + * server * @return A configured McpResourceTemplateModule instance */ fun create( resourceTemplateClass: KClass, groupAnnotationClass: KClass?, - ) = McpResourceTemplateModule(resourceTemplateClass = resourceTemplateClass, qualifier = groupAnnotationClass?.qualifier) + ) = + McpResourceTemplateModule( + resourceTemplateClass = resourceTemplateClass, + qualifier = groupAnnotationClass?.qualifier, + ) /** * Creates an [McpResourceTemplateModule] with reified type parameters for both group annotation and resource @@ -112,13 +115,12 @@ private constructor( * * @param RT The type of [McpResourceTemplate] implementation to register * @param resourceTemplateClass The [KClass] of the resource template implementation - * @param groupAnnotation Optional annotation instance for grouping this resource template with a specific MCP server + * @param groupAnnotation Optional annotation instance for grouping this resource template with a specific MCP + * server * @return A configured McpResourceTemplateModule instance */ - fun create( - resourceTemplateClass: KClass, - groupAnnotation: Annotation?, - ) = McpResourceTemplateModule(resourceTemplateClass = resourceTemplateClass, qualifier = groupAnnotation?.qualifier) + fun create(resourceTemplateClass: KClass, groupAnnotation: Annotation?) = + McpResourceTemplateModule(resourceTemplateClass = resourceTemplateClass, qualifier = groupAnnotation?.qualifier) /** * Creates an [McpResourceTemplateModule] with a reified resource template type and annotation instance. @@ -126,7 +128,8 @@ private constructor( * Convenience method that combines reified resource template type with runtime annotation instance. * * @param RT The type of [McpResourceTemplate] implementation to register - * @param groupAnnotation Optional annotation instance for grouping this resource template with a specific MCP server + * @param groupAnnotation Optional annotation instance for grouping this resource template with a specific MCP + * server * @return A configured McpResourceTemplateModule instance */ inline fun create(groupAnnotation: Annotation?) = diff --git a/misk-mcp/src/main/kotlin/misk/mcp/McpTool.kt b/misk-mcp/src/main/kotlin/misk/mcp/McpTool.kt index 7e96bbed23a..b85405a7c7b 100644 --- a/misk-mcp/src/main/kotlin/misk/mcp/McpTool.kt +++ b/misk-mcp/src/main/kotlin/misk/mcp/McpTool.kt @@ -289,11 +289,10 @@ abstract class McpTool { /** * Handles a tool invocation with the typed [input]. * - * Subclasses must override this method to implement tool behavior. Tools that need access to the - * request's [RequestMeta] (e.g. progress tokens, related task metadata) should extend the - * [MetaAwareMcpTool] (or [MetaAwareStructuredMcpTool] for structured outputs) abstract bridge - * class instead — the bridge implements [MetaAwareTool] and the framework dispatches through - * the meta-aware overload when present. + * Subclasses must override this method to implement tool behavior. Tools that need access to the request's + * [RequestMeta] (e.g. progress tokens, related task metadata) should extend the [MetaAwareMcpTool] (or + * [MetaAwareStructuredMcpTool] for structured outputs) abstract bridge class instead — the bridge implements + * [MetaAwareTool] and the framework dispatches through the meta-aware overload when present. */ abstract suspend fun handle(input: I): ToolResult @@ -555,8 +554,8 @@ abstract class StructuredMcpTool : McpTool() { /** * Convenience base class for MCP tools that require no input parameters. * - * This abstract class extends [McpTool] with no input type, providing a simplified API for tools that - * don't accept any input arguments. Subclasses implement a simpler `handle()` method with no parameters. + * This abstract class extends [McpTool] with no input type, providing a simplified API for tools that don't accept any + * input arguments. Subclasses implement a simpler `handle()` method with no parameters. * * ## When to Use McpToolEmptyInput * @@ -612,13 +611,12 @@ abstract class McpToolEmptyInput : McpTool() { abstract suspend fun handle(): ToolResult } - /** * Convenience base class for structured MCP tools that require no input parameters. * - * This abstract class extends [StructuredMcpTool] with no input type, providing a simplified API for - * tools that return structured output but don't accept any input arguments. Subclasses implement a simpler `handle()` - * method with no parameters. + * This abstract class extends [StructuredMcpTool] with no input type, providing a simplified API for tools that return + * structured output but don't accept any input arguments. Subclasses implement a simpler `handle()` method with no + * parameters. * * ## When to Use StructuredMcpToolEmptyInput * @@ -692,8 +690,7 @@ abstract class StructuredMcpToolEmptyInput : StructuredMcpTool` at invocation time, so - * mixing this base in is the only opt-in step needed — no explicit annotations or registrations. + * The framework's dispatcher branches on `tool is MetaAwareTool<*, *>` at invocation time, so mixing this base in is + * the only opt-in step needed — no explicit annotations or registrations. * * ## Example * diff --git a/misk-mcp/src/main/kotlin/misk/mcp/MetaAwareStructuredMcpTool.kt b/misk-mcp/src/main/kotlin/misk/mcp/MetaAwareStructuredMcpTool.kt index 803e15ffd45..0f893e88028 100644 --- a/misk-mcp/src/main/kotlin/misk/mcp/MetaAwareStructuredMcpTool.kt +++ b/misk-mcp/src/main/kotlin/misk/mcp/MetaAwareStructuredMcpTool.kt @@ -6,12 +6,11 @@ import misk.annotation.ExperimentalMiskApi /** * Bridge base class for [StructuredMcpTool] authors that need access to the request's `_meta`. * - * Subclasses implement only the meta-aware [handle] overload — the input-only `handle(input)` is - * sealed by this class and delegates to the meta-aware overload with `meta = null`. Returns - * [McpTool.ToolResult] (rather than `StructuredToolResult` directly) because - * `StructuredMcpTool.handle` does not narrow the return type — implementations build their typed - * result via the inherited `ToolResult(result: O, ...)` factories, which already produce a - * `StructuredToolResult` typed as `ToolResult`. + * Subclasses implement only the meta-aware [handle] overload — the input-only `handle(input)` is sealed by this class + * and delegates to the meta-aware overload with `meta = null`. Returns [McpTool.ToolResult] (rather than + * `StructuredToolResult` directly) because `StructuredMcpTool.handle` does not narrow the return type — + * implementations build their typed result via the inherited `ToolResult(result: O, ...)` factories, which already + * produce a `StructuredToolResult` typed as `ToolResult`. * * @param I The input type for this tool. * @param O The output type for this tool. diff --git a/misk-mcp/src/main/kotlin/misk/mcp/MetaAwareTool.kt b/misk-mcp/src/main/kotlin/misk/mcp/MetaAwareTool.kt index 133d1d15c94..d5df3bebcb0 100644 --- a/misk-mcp/src/main/kotlin/misk/mcp/MetaAwareTool.kt +++ b/misk-mcp/src/main/kotlin/misk/mcp/MetaAwareTool.kt @@ -6,26 +6,24 @@ import misk.annotation.ExperimentalMiskApi /** * Capability interface declaring the meta-aware overload of [handle]. * - * This interface is the framework's hook for tools that need access to the inbound MCP request's - * `_meta` field ([RequestMeta]). The dispatcher branches on `tool is MetaAwareTool<*, *>` at - * invocation time, so non-meta-aware tools pay zero cost. - * - * Tool authors should not implement this interface directly — instead, extend the appropriate - * abstract bridge class for their base hierarchy: + * This interface is the framework's hook for tools that need access to the inbound MCP request's `_meta` field + * ([RequestMeta]). The dispatcher branches on `tool is MetaAwareTool<*, *>` at invocation time, so non-meta-aware tools + * pay zero cost. * + * Tool authors should not implement this interface directly — instead, extend the appropriate abstract bridge class for + * their base hierarchy: * - [MetaAwareMcpTool] — for plain `McpTool` authors * - [MetaAwareStructuredMcpTool] — for `StructuredMcpTool` authors * - * The bridges hold a `final override` of the input-only `handle(input)` method that delegates to - * the meta-aware overload with `meta = null`, so meta-aware authors only override the - * meta-aware overload — single inheritance, single method. - * - * External base hierarchies (e.g. cash-server's `MoneybotTool`) can add their own bridge by - * implementing this interface and providing the same `final override` of the base's input-only + * The bridges hold a `final override` of the input-only `handle(input)` method that delegates to the meta-aware + * overload with `meta = null`, so meta-aware authors only override the meta-aware overload — single inheritance, single * method. * - * Parameterized in [R] so it composes with bases that return covariant subtypes of `ToolResult` - * or external result types entirely. + * External base hierarchies (e.g. cash-server's `MoneybotTool`) can add their own bridge by implementing this interface + * and providing the same `final override` of the base's input-only method. + * + * Parameterized in [R] so it composes with bases that return covariant subtypes of `ToolResult` or external result + * types entirely. * * @param I The tool's input type. * @param R The tool's return type. diff --git a/misk-mcp/src/main/kotlin/misk/mcp/MiskMcpServer.kt b/misk-mcp/src/main/kotlin/misk/mcp/MiskMcpServer.kt index c494cec1238..a96714c0550 100644 --- a/misk-mcp/src/main/kotlin/misk/mcp/MiskMcpServer.kt +++ b/misk-mcp/src/main/kotlin/misk/mcp/MiskMcpServer.kt @@ -79,7 +79,8 @@ import misk.mcp.internal.build * * The server automatically determines capabilities from the [McpServerConfig] and registered components: * - The server enables tools capability if any [McpTool] implementations are registered - * - The server enables resources capability if any [McpResource] or [McpResourceTemplate] implementations are registered + * - The server enables resources capability if any [McpResource] or [McpResourceTemplate] implementations are + * registered * - The server enables prompts capability if any [McpPrompt] implementations are registered * * @param name The unique name identifier for this MCP server instance @@ -117,7 +118,8 @@ internal constructor( completions = null, logging = null, prompts = if (prompts.isNotEmpty()) config.prompts.asPrompts() else null, - resources = if (resources.isNotEmpty() || resourceTemplates.isNotEmpty()) config.resources.asResources() else null, + resources = + if (resources.isNotEmpty() || resourceTemplates.isNotEmpty()) config.resources.asResources() else null, tools = if (tools.isNotEmpty()) config.tools.asTools() else null, ), enforceStrictCapabilities = config.enforce_strict_capabilities, @@ -131,9 +133,7 @@ internal constructor( name = prompt.name, description = prompt.description, arguments = prompt.arguments, - promptProvider = { request -> - withContext(McpClientConnection(this)) { prompt.handler(request) } - }, + promptProvider = { request -> withContext(McpClientConnection(this)) { prompt.handler(request) } }, ) } @@ -143,9 +143,7 @@ internal constructor( name = resource.name, description = resource.description, mimeType = resource.mimeType, - readHandler = { request -> - withContext(McpClientConnection(this)) { resource.handler(request) } - }, + readHandler = { request -> withContext(McpClientConnection(this)) { resource.handler(request) } }, ) } @@ -191,9 +189,11 @@ internal constructor( var outcome = McpMetrics.ToolCallOutcome.Success try { - withContext(McpClientConnection(this)) { handler(request) }.also { result -> - outcome = if (result.isError == true) McpMetrics.ToolCallOutcome.Error else McpMetrics.ToolCallOutcome.Success - } + withContext(McpClientConnection(this)) { handler(request) } + .also { result -> + outcome = + if (result.isError == true) McpMetrics.ToolCallOutcome.Error else McpMetrics.ToolCallOutcome.Success + } } catch (ex: Exception) { outcome = McpMetrics.ToolCallOutcome.Exception throw ex diff --git a/misk-mcp/src/main/kotlin/misk/mcp/internal/JsonSchemaExtensions.kt b/misk-mcp/src/main/kotlin/misk/mcp/internal/JsonSchemaExtensions.kt index 75301b2d8f1..6a52b91084f 100644 --- a/misk-mcp/src/main/kotlin/misk/mcp/internal/JsonSchemaExtensions.kt +++ b/misk-mcp/src/main/kotlin/misk/mcp/internal/JsonSchemaExtensions.kt @@ -18,7 +18,6 @@ import kotlinx.serialization.json.buildJsonObject import misk.mcp.Description import misk.mcp.serializer - /** * Generates a JSON schema for a serializable Kotlin type, including properties, types, and required fields. * Processes @Description annotations and handles nested objects with configurable recursion depth. @@ -113,17 +112,20 @@ private fun SerialDescriptor.generateJsonSchemaInternal( SerialKind.ENUM -> { label?.let { put("properties", buildLabelProperty(it)) } - put("oneOf", + put( + "oneOf", buildJsonArray { (0 until elementsCount).forEach { index -> val enumValue = getElementName(index) val description = getElementAnnotations(index).description() - add(buildJsonObject { - put("const", JsonPrimitive(enumValue)) - description?.let { put("description", JsonPrimitive(description)) } - }) + add( + buildJsonObject { + put("const", JsonPrimitive(enumValue)) + description?.let { put("description", JsonPrimitive(description)) } + } + ) } - } + }, ) } diff --git a/misk-mcp/src/test/kotlin/misk/mcp/McpServerActionTest.kt b/misk-mcp/src/test/kotlin/misk/mcp/McpServerActionTest.kt index 15283dbfb8f..96d8cf9293e 100644 --- a/misk-mcp/src/test/kotlin/misk/mcp/McpServerActionTest.kt +++ b/misk-mcp/src/test/kotlin/misk/mcp/McpServerActionTest.kt @@ -43,14 +43,14 @@ import misk.mcp.testing.resources.WebSearchResource import misk.mcp.testing.tools.CalculatorTool import misk.mcp.testing.tools.CalculatorToolInput.Operation import misk.mcp.testing.tools.CalculatorToolOutput +import misk.mcp.testing.tools.ClientConnectionTool +import misk.mcp.testing.tools.ClientConnectionToolOutput import misk.mcp.testing.tools.GetNicknameRequest import misk.mcp.testing.tools.HelloWorldTool import misk.mcp.testing.tools.HelloWorldToolOutput import misk.mcp.testing.tools.HierarchicalTool import misk.mcp.testing.tools.HierarchicalToolOutput import misk.mcp.testing.tools.KotlinSdkTool -import misk.mcp.testing.tools.ClientConnectionTool -import misk.mcp.testing.tools.ClientConnectionToolOutput import misk.mcp.testing.tools.NicknameElicitationTool import misk.mcp.testing.tools.ThrowingTool import misk.mcp.testing.tools.VersionMetadata diff --git a/misk-mcp/src/test/kotlin/misk/mcp/McpStatefulServerActionTest.kt b/misk-mcp/src/test/kotlin/misk/mcp/McpStatefulServerActionTest.kt index e5aa5eda1e6..917ea15e552 100644 --- a/misk-mcp/src/test/kotlin/misk/mcp/McpStatefulServerActionTest.kt +++ b/misk-mcp/src/test/kotlin/misk/mcp/McpStatefulServerActionTest.kt @@ -12,8 +12,8 @@ import java.net.HttpURLConnection.HTTP_BAD_REQUEST import java.net.HttpURLConnection.HTTP_NOT_FOUND import java.net.HttpURLConnection.HTTP_NO_CONTENT import kotlin.test.Test -import kotlin.test.assertIs import kotlin.test.assertEquals +import kotlin.test.assertIs import kotlin.test.assertNotNull import kotlin.test.assertTrue import kotlinx.coroutines.channels.SendChannel diff --git a/misk-mcp/src/test/kotlin/misk/mcp/McpToolHandleMetaTest.kt b/misk-mcp/src/test/kotlin/misk/mcp/McpToolHandleMetaTest.kt index 2f9fc7b97a2..0badf4226f3 100644 --- a/misk-mcp/src/test/kotlin/misk/mcp/McpToolHandleMetaTest.kt +++ b/misk-mcp/src/test/kotlin/misk/mcp/McpToolHandleMetaTest.kt @@ -24,9 +24,9 @@ class McpToolHandleMetaTest { @Serializable data class CapturingInput(val name: String) /** - * Tool that extends the [MetaAwareMcpTool] bridge and records what its meta-aware [handle] - * overload receives. The bridge's `final override` of `handle(input)` fills the framework's - * abstract slot, so this class only declares the meta-aware overload. + * Tool that extends the [MetaAwareMcpTool] bridge and records what its meta-aware [handle] overload receives. The + * bridge's `final override` of `handle(input)` fills the framework's abstract slot, so this class only declares the + * meta-aware overload. */ private class CapturingTool : MetaAwareMcpTool() { override val name = "capturing" diff --git a/misk-mcp/src/test/kotlin/misk/mcp/internal/JsonSchemaExtensionsIntegrationTest.kt b/misk-mcp/src/test/kotlin/misk/mcp/internal/JsonSchemaExtensionsIntegrationTest.kt index 0b050a6c002..c4bd7e69a66 100644 --- a/misk-mcp/src/test/kotlin/misk/mcp/internal/JsonSchemaExtensionsIntegrationTest.kt +++ b/misk-mcp/src/test/kotlin/misk/mcp/internal/JsonSchemaExtensionsIntegrationTest.kt @@ -37,12 +37,9 @@ class JsonSchemaExtensionsIntegrationTest { private val sealedFruit: List, ) { enum class SampleEnum { - @Description("First value") - FIRST, - @Description("Second value") - SECOND, - @Description("Third value") - THIRD, + @Description("First value") FIRST, + @Description("Second value") SECOND, + @Description("Third value") THIRD, NO_DESCRIPTION, } diff --git a/misk-mcp/src/test/kotlin/misk/mcp/internal/JsonSchemaExtensionsTest.kt b/misk-mcp/src/test/kotlin/misk/mcp/internal/JsonSchemaExtensionsTest.kt index 956bd210d21..735f4662473 100644 --- a/misk-mcp/src/test/kotlin/misk/mcp/internal/JsonSchemaExtensionsTest.kt +++ b/misk-mcp/src/test/kotlin/misk/mcp/internal/JsonSchemaExtensionsTest.kt @@ -122,8 +122,7 @@ internal class JsonSchemaExtensionsTest { // Test enums @Serializable enum class Status { - @Description("Active status") - ACTIVE, + @Description("Active status") ACTIVE, INACTIVE, PENDING, ARCHIVED, @@ -634,7 +633,8 @@ internal class JsonSchemaExtensionsTest { assertEquals(setOf("ACTIVE", "INACTIVE", "PENDING", "ARCHIVED"), statusValues) // Verify ACTIVE has description - val activeOption = statusOneOf.first { ((it as JsonObject)["const"] as JsonPrimitive).content == "ACTIVE" } as JsonObject + val activeOption = + statusOneOf.first { ((it as JsonObject)["const"] as JsonPrimitive).content == "ACTIVE" } as JsonObject assertEquals(JsonPrimitive("Active status"), activeOption["description"]) // Verify priority enum field diff --git a/misk-mcp/src/testFixtures/kotlin/misk/mcp/testing/resources/UserProfileResource.kt b/misk-mcp/src/testFixtures/kotlin/misk/mcp/testing/resources/UserProfileResource.kt index d23251c541c..b097da49f4e 100644 --- a/misk-mcp/src/testFixtures/kotlin/misk/mcp/testing/resources/UserProfileResource.kt +++ b/misk-mcp/src/testFixtures/kotlin/misk/mcp/testing/resources/UserProfileResource.kt @@ -14,19 +14,17 @@ class UserProfileResource @Inject constructor() : McpResourceTemplate { override val description = "Profile information for a specific user" override val mimeType = "application/json" - override suspend fun handler( - request: ReadResourceRequest, - variables: Map, - ): ReadResourceResult { + override suspend fun handler(request: ReadResourceRequest, variables: Map): ReadResourceResult { val userId = variables["userId"] ?: "unknown" return ReadResourceResult( - contents = listOf( - TextResourceContents( - text = """{"userId": "$userId", "name": "User $userId"}""", - uri = request.uri, - mimeType = mimeType, + contents = + listOf( + TextResourceContents( + text = """{"userId": "$userId", "name": "User $userId"}""", + uri = request.uri, + mimeType = mimeType, + ) ) - ) ) } } diff --git a/misk-mcp/src/testFixtures/kotlin/misk/mcp/testing/tools/ClientConnectionTool.kt b/misk-mcp/src/testFixtures/kotlin/misk/mcp/testing/tools/ClientConnectionTool.kt index 919209a4ee7..8cadddeeae8 100644 --- a/misk-mcp/src/testFixtures/kotlin/misk/mcp/testing/tools/ClientConnectionTool.kt +++ b/misk-mcp/src/testFixtures/kotlin/misk/mcp/testing/tools/ClientConnectionTool.kt @@ -9,16 +9,13 @@ import misk.annotation.ExperimentalMiskApi import misk.mcp.StructuredMcpToolEmptyInput import misk.mcp.action.currentClientConnection -@Serializable -data class ClientConnectionToolOutput( - val sessionId: String, -) +@Serializable data class ClientConnectionToolOutput(val sessionId: String) /** * Test tool that verifies [currentClientConnection] is accessible from within a tool handler. * - * Calls [currentClientConnection] to obtain the [ClientConnection] and returns its session ID - * to prove the connection is available in the handler context. + * Calls [currentClientConnection] to obtain the [ClientConnection] and returns its session ID to prove the connection + * is available in the handler context. */ class ClientConnectionTool @Inject constructor() : StructuredMcpToolEmptyInput() { override val name = "client_connection" @@ -26,16 +23,8 @@ class ClientConnectionTool @Inject constructor() : StructuredMcpToolEmptyInput() { +class HelloWorldTool @Inject constructor() : StructuredMcpToolEmptyInput() { override suspend fun handle(): ToolResult { return ToolResult(result = HelloWorldToolOutput("Hello, world!")) } @@ -23,8 +19,4 @@ class HelloWorldTool @Inject constructor(): StructuredMcpToolEmptyInput { private val delegate = Mapper.STRING @@ -14,6 +14,5 @@ internal object ParallelTestsKeyMapper : Mapper { override fun toString(value: String): String = delegate.toString(mapValue(value)) - private fun mapValue(value: String): String = - value.updateForParallelTests { v, index -> v + "_$index" } + private fun mapValue(value: String): String = value.updateForParallelTests { v, index -> v + "_$index" } } diff --git a/misk-rate-limiting-bucket4j-redis/src/testFixtures/kotlin/misk/ratelimiting/bucket4j/redis/RedisBucket4jRateLimiterTestModule.kt b/misk-rate-limiting-bucket4j-redis/src/testFixtures/kotlin/misk/ratelimiting/bucket4j/redis/RedisBucket4jRateLimiterTestModule.kt index aadd7d3db33..d902e89af27 100644 --- a/misk-rate-limiting-bucket4j-redis/src/testFixtures/kotlin/misk/ratelimiting/bucket4j/redis/RedisBucket4jRateLimiterTestModule.kt +++ b/misk-rate-limiting-bucket4j-redis/src/testFixtures/kotlin/misk/ratelimiting/bucket4j/redis/RedisBucket4jRateLimiterTestModule.kt @@ -3,18 +3,11 @@ package misk.ratelimiting.bucket4j.redis import misk.inject.KAbstractModule /** - * Module for a Redis-backed Bucket4j rate limiter for use in tests, internally using - * [ParallelTestsKeyMapper] to avoid key collisions when tests are run in parallel. + * Module for a Redis-backed Bucket4j rate limiter for use in tests, internally using [ParallelTestsKeyMapper] to avoid + * key collisions when tests are run in parallel. */ -class RedisBucket4jRateLimiterTestModule( - private val qualifier: Annotation? = null, -) : KAbstractModule() { +class RedisBucket4jRateLimiterTestModule(private val qualifier: Annotation? = null) : KAbstractModule() { override fun configure() { - install( - RedisBucket4jRateLimiterModule( - qualifier = qualifier, - keyMapper = ParallelTestsKeyMapper - ) - ) + install(RedisBucket4jRateLimiterModule(qualifier = qualifier, keyMapper = ParallelTestsKeyMapper)) } -} \ No newline at end of file +} diff --git a/misk-redis/src/main/kotlin/misk/redis/RedisClusterConfig.kt b/misk-redis/src/main/kotlin/misk/redis/RedisClusterConfig.kt index 15301e6fb43..e2dccbaf4e1 100644 --- a/misk-redis/src/main/kotlin/misk/redis/RedisClusterConfig.kt +++ b/misk-redis/src/main/kotlin/misk/redis/RedisClusterConfig.kt @@ -21,8 +21,8 @@ class RedisClusterConfig : LinkedHashMap { override fun apply(project: Project) { val extension = create(project) - val schemaMigratorClasspath = project.configurations.create("schemaMigratorClasspath") { - it.isCanBeConsumed = false - it.isCanBeResolved = true - it.defaultDependencies { deps -> - val version = loadDefaultVersion() - deps.add(project.dependencies.create("com.squareup.misk:misk-jdbc:$version")) + val schemaMigratorClasspath = + project.configurations.create("schemaMigratorClasspath") { + it.isCanBeConsumed = false + it.isCanBeResolved = true + it.defaultDependencies { deps -> + val version = loadDefaultVersion() + deps.add(project.dependencies.create("com.squareup.misk:misk-jdbc:$version")) + } } - } // Allow overriding the worker classpath via a Gradle property, e.g. for testing: // -PschemaMigratorClasspath=/path/to/jar1:/path/to/jar2 @@ -63,20 +64,17 @@ class SchemaMigratorPlugin : Plugin { fun loadDefaultVersion(): String { val props = Properties() - val stream = SchemaMigratorPlugin::class.java.classLoader - .getResourceAsStream("misk-schema-migrator.properties") - ?: error("misk-schema-migrator.properties not found in plugin classpath") + val stream = + SchemaMigratorPlugin::class.java.classLoader.getResourceAsStream("misk-schema-migrator.properties") + ?: error("misk-schema-migrator.properties not found in plugin classpath") props.load(stream) - return props.getProperty("version") - ?: error("version property not found in misk-schema-migrator.properties") + return props.getProperty("version") ?: error("version property not found in misk-schema-migrator.properties") } } } -abstract class SchemaMigratorTask @Inject constructor( - @get:Internal - val execOperations: ExecOperations -) : DefaultTask() { +abstract class SchemaMigratorTask @Inject constructor(@get:Internal val execOperations: ExecOperations) : + DefaultTask() { companion object { const val NAME = "migrateSchema" @@ -113,10 +111,12 @@ abstract class SchemaMigratorTask @Inject constructor( appendLine("migrationsFormat=${migrationsFormat.get()}") } - execOperations.javaexec { - it.classpath = workerClasspath - it.mainClass.set("misk.jdbc.SchemaMigratorRunner") - it.standardInput = ByteArrayInputStream(props.toByteArray()) - }.assertNormalExitValue() + execOperations + .javaexec { + it.classpath = workerClasspath + it.mainClass.set("misk.jdbc.SchemaMigratorRunner") + it.standardInput = ByteArrayInputStream(props.toByteArray()) + } + .assertNormalExitValue() } } diff --git a/misk-service/src/main/kotlin/misk/ServiceModule.kt b/misk-service/src/main/kotlin/misk/ServiceModule.kt index 0669229ca2c..f108ae101c2 100644 --- a/misk-service/src/main/kotlin/misk/ServiceModule.kt +++ b/misk-service/src/main/kotlin/misk/ServiceModule.kt @@ -246,10 +246,10 @@ internal data class OptionalDependencyEdge( val switchType: KClass, val edge: DependencyEdge?, /** - * [dependencyKey] is added here to disambiguate when the switch is off. - * without it, guice tries to multibind the same instance for each dependency, when the off impl [edge] is null. + * [dependencyKey] is added here to disambiguate when the switch is off. without it, guice tries to multibind the same + * instance for each dependency, when the off impl [edge] is null. */ - val dependencyKey: Key + val dependencyKey: Key, ) internal data class OptionalEnhancementEdge( @@ -258,8 +258,8 @@ internal data class OptionalEnhancementEdge( val switchType: KClass, val edge: EnhancementEdge?, /** - * [dependencyKey] is added here to disambiguate when the switch is off. - * without it, guice tries to multibind the same instance for each dependency, when the off impl [edge] is null. + * [dependencyKey] is added here to disambiguate when the switch is off. without it, guice tries to multibind the same + * instance for each dependency, when the off impl [edge] is null. */ - val dependencyKey: Key + val dependencyKey: Key, ) diff --git a/misk-service/src/test/kotlin/misk/ServiceModuleTest.kt b/misk-service/src/test/kotlin/misk/ServiceModuleTest.kt index 0ad36173c85..146bd8c5555 100644 --- a/misk-service/src/test/kotlin/misk/ServiceModuleTest.kt +++ b/misk-service/src/test/kotlin/misk/ServiceModuleTest.kt @@ -3,7 +3,6 @@ package misk import com.google.common.util.concurrent.AbstractIdleService import com.google.common.util.concurrent.ServiceManager import com.google.inject.Guice -import com.google.inject.Key import jakarta.inject.Inject import jakarta.inject.Singleton import kotlin.test.assertTrue @@ -202,7 +201,6 @@ class ServiceModuleTest { val disabledSwitch = TestSwitch(enabled = false) val log = StringBuilder() - val injector = Guice.createInjector( MiskTestingServiceModule(), @@ -211,9 +209,10 @@ class ServiceModuleTest { bind().toInstance(log) bind().toInstance(disabledSwitch) install( - ServiceModule().dependsOn() + ServiceModule() + .dependsOn() .dependsOn() - .conditionalOn("test"), + .conditionalOn("test") ) } }, @@ -284,7 +283,6 @@ class ServiceModuleTest { assertThat(log.toString()).contains("EnhancementService.startUp") } - @Test fun conditionalOn_withMultipleEnhancements_whenDisabled_bindsNoOpServiceWithNoEnhancements() { val disabledSwitch = TestSwitch(enabled = false) @@ -296,10 +294,12 @@ class ServiceModuleTest { override fun configure() { bind().toInstance(log) bind().toInstance(disabledSwitch) - install(ServiceModule() - .enhancedBy() - .enhancedBy() - .conditionalOn("test")) + install( + ServiceModule() + .enhancedBy() + .enhancedBy() + .conditionalOn("test") + ) } }, ) diff --git a/misk-spirit/src/main/kotlin/misk/spirit/Spirit.kt b/misk-spirit/src/main/kotlin/misk/spirit/Spirit.kt index a63de867ffd..d775731db3c 100644 --- a/misk-spirit/src/main/kotlin/misk/spirit/Spirit.kt +++ b/misk-spirit/src/main/kotlin/misk/spirit/Spirit.kt @@ -4,8 +4,8 @@ import java.io.File import java.nio.file.Files /** - * Wrapper around the Spirit binary (https://github.com/block/spirit) for generating schema diffs. - * Spirit compares a live MySQL database against SQL files and generates DDL statements. + * Wrapper around the Spirit binary (https://github.com/block/spirit) for generating schema diffs. Spirit compares a + * live MySQL database against SQL files and generates DDL statements. */ class Spirit { companion object { @@ -23,9 +23,7 @@ class Spirit { fun diff(dsn: String, sqlFiles: Map): SchemaDiff { val tempDir = Files.createTempDirectory("spirit-").toFile() try { - sqlFiles.forEach { (filename, content) -> - File(tempDir, filename).writeText(content) - } + sqlFiles.forEach { (filename, content) -> File(tempDir, filename).writeText(content) } return diff(dsn, tempDir) } finally { tempDir.deleteRecursively() @@ -33,9 +31,8 @@ class Spirit { } private fun diff(dsn: String, targetDir: File): SchemaDiff { - val processBuilder = ProcessBuilder( - listOf(spiritBinaryPath, "diff", "--source-dsn", dsn, "--target-dir", targetDir.absolutePath) - ) + val processBuilder = + ProcessBuilder(listOf(spiritBinaryPath, "diff", "--source-dsn", dsn, "--target-dir", targetDir.absolutePath)) processBuilder.redirectErrorStream(true) val process = processBuilder.start() @@ -53,9 +50,7 @@ class Spirit { } // Strip comment lines (lint info), keep DDL statements - val ddl = output.lines() - .filter { it.isNotBlank() && !it.startsWith("-- ") } - .joinToString("\n") + val ddl = output.lines().filter { it.isNotBlank() && !it.startsWith("-- ") }.joinToString("\n") return if (ddl.isBlank()) { SchemaDiff(diff = null, hasDiff = false) @@ -69,8 +64,7 @@ class Spirit { private fun findSpiritBinary(): String { // First, try to let OS resolve it directly (works when PATH is set) try { - val process = ProcessBuilder(listOf(SPIRIT_BINARY, "--version")) - .redirectErrorStream(true).start() + val process = ProcessBuilder(listOf(SPIRIT_BINARY, "--version")).redirectErrorStream(true).start() if (process.waitFor() == 0) return SPIRIT_BINARY } catch (_: Exception) {} @@ -79,8 +73,7 @@ class Spirit { for (path in absolutePaths) { if (File(path).exists()) { try { - val process = ProcessBuilder(listOf(path, "--version")) - .redirectErrorStream(true).start() + val process = ProcessBuilder(listOf(path, "--version")).redirectErrorStream(true).start() if (process.waitFor() == 0) return path } catch (_: Exception) {} } diff --git a/misk-sqldelight/src/main/kotlin/misk/sqldelight/SqlDelightExceptionClassifier.kt b/misk-sqldelight/src/main/kotlin/misk/sqldelight/SqlDelightExceptionClassifier.kt index 01a0ef77fb4..586f981e28a 100644 --- a/misk-sqldelight/src/main/kotlin/misk/sqldelight/SqlDelightExceptionClassifier.kt +++ b/misk-sqldelight/src/main/kotlin/misk/sqldelight/SqlDelightExceptionClassifier.kt @@ -10,11 +10,8 @@ import misk.jdbc.retry.DefaultExceptionClassifier * Extends [DefaultExceptionClassifier] to add SQLDelight-specific retryable exceptions: * - [OptimisticLockException]: SQLDelight's optimistic locking exception */ -class SqlDelightExceptionClassifier -@JvmOverloads -constructor( - dataSourceType: DataSourceType? = null, -) : DefaultExceptionClassifier(dataSourceType) { +class SqlDelightExceptionClassifier @JvmOverloads constructor(dataSourceType: DataSourceType? = null) : + DefaultExceptionClassifier(dataSourceType) { override fun isRetryable(th: Throwable): Boolean { return when (th) { diff --git a/misk-sqldelight/src/test/kotlin/misk/sqldelight/RetryingTransacterTest.kt b/misk-sqldelight/src/test/kotlin/misk/sqldelight/RetryingTransacterTest.kt index fc09a8d3855..1f751a3b1ca 100644 --- a/misk-sqldelight/src/test/kotlin/misk/sqldelight/RetryingTransacterTest.kt +++ b/misk-sqldelight/src/test/kotlin/misk/sqldelight/RetryingTransacterTest.kt @@ -2,8 +2,8 @@ package misk.sqldelight import app.cash.sqldelight.db.OptimisticLockException import app.cash.sqldelight.driver.jdbc.JdbcDriver -import java.sql.SQLRecoverableException import jakarta.inject.Inject +import java.sql.SQLRecoverableException import misk.sqldelight.testing.Movies import misk.sqldelight.testing.MoviesDatabase import misk.sqldelight.testing.MoviesQueries @@ -151,9 +151,11 @@ class RetryingTransacterTest { // Create a fresh database without retry wrapper to avoid nested retries val rawDatabase = MoviesDatabase.invoke(jdbcDriver) val customOptions = TransacterOptions(maxAttempts = 5) - val transacterWithCustomRetries = object : RetryingTransacter(rawDatabase, customOptions), MoviesDatabase { - override val moviesQueries: MoviesQueries get() = rawDatabase.moviesQueries - } + val transacterWithCustomRetries = + object : RetryingTransacter(rawDatabase, customOptions), MoviesDatabase { + override val moviesQueries: MoviesQueries + get() = rawDatabase.moviesQueries + } var attempts = 0 assertThrows { diff --git a/misk-sqldelight/src/test/kotlin/misk/sqldelight/SqlDelightExceptionClassifierTest.kt b/misk-sqldelight/src/test/kotlin/misk/sqldelight/SqlDelightExceptionClassifierTest.kt index b5d2a3962b4..bf761c76c29 100644 --- a/misk-sqldelight/src/test/kotlin/misk/sqldelight/SqlDelightExceptionClassifierTest.kt +++ b/misk-sqldelight/src/test/kotlin/misk/sqldelight/SqlDelightExceptionClassifierTest.kt @@ -45,18 +45,14 @@ class SqlDelightExceptionClassifierTest { @Test fun `inherits database-specific behavior from base classifier`() { val classifier = SqlDelightExceptionClassifier(DataSourceType.VITESS_MYSQL) - val exception = SQLException( - "vttablet: rpc error: code = Aborted desc = transaction 123: not found" - ) + val exception = SQLException("vttablet: rpc error: code = Aborted desc = transaction 123: not found") assertThat(classifier.isRetryable(exception)).isTrue() } @Test fun `Vitess exception not retryable without correct DataSourceType`() { val classifier = SqlDelightExceptionClassifier(DataSourceType.MYSQL) - val exception = SQLException( - "vttablet: rpc error: code = Aborted desc = transaction 123: not found" - ) + val exception = SQLException("vttablet: rpc error: code = Aborted desc = transaction 123: not found") assertThat(classifier.isRetryable(exception)).isFalse() } diff --git a/misk-tailwind/src/test/kotlin/misk/tailwind/TailwindHtmlLayoutTest.kt b/misk-tailwind/src/test/kotlin/misk/tailwind/TailwindHtmlLayoutTest.kt index 49451675bf4..c11e273f31e 100644 --- a/misk-tailwind/src/test/kotlin/misk/tailwind/TailwindHtmlLayoutTest.kt +++ b/misk-tailwind/src/test/kotlin/misk/tailwind/TailwindHtmlLayoutTest.kt @@ -1,20 +1,16 @@ package misk.tailwind -import kotlinx.html.stream.appendHTML -import org.junit.jupiter.api.Test import kotlin.test.assertContains import kotlin.test.assertFalse +import kotlinx.html.stream.appendHTML +import org.junit.jupiter.api.Test class TailwindHtmlLayoutTest { private fun renderLayout(enableTurbo: Boolean): String = - StringBuilder().apply { - appendHTML().TailwindHtmlLayout( - appRoot = "/app", - title = "Test", - enableTurbo = enableTurbo, - ) {} - }.toString() + StringBuilder() + .apply { appendHTML().TailwindHtmlLayout(appRoot = "/app", title = "Test", enableTurbo = enableTurbo) {} } + .toString() @Test fun `turbo scripts included by default`() { diff --git a/misk-testing/api/misk-testing.api b/misk-testing/api/misk-testing.api index c3dac6fb1b4..1f500e1646b 100644 --- a/misk-testing/api/misk-testing.api +++ b/misk-testing/api/misk-testing.api @@ -379,40 +379,43 @@ public final class misk/web/FakeHttpCall : misk/web/HttpCall { public fun (Lokhttp3/HttpUrl;Lmisk/web/SocketAddress;Lmisk/web/DispatchMechanism;Lokhttp3/Headers;)V public fun (Lokhttp3/HttpUrl;Lmisk/web/SocketAddress;Lmisk/web/DispatchMechanism;Lokhttp3/Headers;I)V public fun (Lokhttp3/HttpUrl;Lmisk/web/SocketAddress;Lmisk/web/DispatchMechanism;Lokhttp3/Headers;II)V - public fun (Lokhttp3/HttpUrl;Lmisk/web/SocketAddress;Lmisk/web/DispatchMechanism;Lokhttp3/Headers;IILokhttp3/Headers$Builder;)V - public fun (Lokhttp3/HttpUrl;Lmisk/web/SocketAddress;Lmisk/web/DispatchMechanism;Lokhttp3/Headers;IILokhttp3/Headers$Builder;Z)V - public fun (Lokhttp3/HttpUrl;Lmisk/web/SocketAddress;Lmisk/web/DispatchMechanism;Lokhttp3/Headers;IILokhttp3/Headers$Builder;ZLokhttp3/Headers$Builder;)V - public fun (Lokhttp3/HttpUrl;Lmisk/web/SocketAddress;Lmisk/web/DispatchMechanism;Lokhttp3/Headers;IILokhttp3/Headers$Builder;ZLokhttp3/Headers$Builder;Lokio/BufferedSource;)V - public fun (Lokhttp3/HttpUrl;Lmisk/web/SocketAddress;Lmisk/web/DispatchMechanism;Lokhttp3/Headers;IILokhttp3/Headers$Builder;ZLokhttp3/Headers$Builder;Lokio/BufferedSource;Lokio/BufferedSink;)V - public fun (Lokhttp3/HttpUrl;Lmisk/web/SocketAddress;Lmisk/web/DispatchMechanism;Lokhttp3/Headers;IILokhttp3/Headers$Builder;ZLokhttp3/Headers$Builder;Lokio/BufferedSource;Lokio/BufferedSink;Lmisk/web/actions/WebSocket;)V - public fun (Lokhttp3/HttpUrl;Lmisk/web/SocketAddress;Lmisk/web/DispatchMechanism;Lokhttp3/Headers;IILokhttp3/Headers$Builder;ZLokhttp3/Headers$Builder;Lokio/BufferedSource;Lokio/BufferedSink;Lmisk/web/actions/WebSocket;Lmisk/web/actions/WebSocketListener;)V - public fun (Lokhttp3/HttpUrl;Lmisk/web/SocketAddress;Lmisk/web/DispatchMechanism;Lokhttp3/Headers;IILokhttp3/Headers$Builder;ZLokhttp3/Headers$Builder;Lokio/BufferedSource;Lokio/BufferedSink;Lmisk/web/actions/WebSocket;Lmisk/web/actions/WebSocketListener;Ljava/util/List;)V - public synthetic fun (Lokhttp3/HttpUrl;Lmisk/web/SocketAddress;Lmisk/web/DispatchMechanism;Lokhttp3/Headers;IILokhttp3/Headers$Builder;ZLokhttp3/Headers$Builder;Lokio/BufferedSource;Lokio/BufferedSink;Lmisk/web/actions/WebSocket;Lmisk/web/actions/WebSocketListener;Ljava/util/List;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun (Lokhttp3/HttpUrl;Lmisk/web/SocketAddress;Lmisk/web/DispatchMechanism;Lokhttp3/Headers;IILmisk/web/http/HttpVersion;)V + public fun (Lokhttp3/HttpUrl;Lmisk/web/SocketAddress;Lmisk/web/DispatchMechanism;Lokhttp3/Headers;IILmisk/web/http/HttpVersion;Lokhttp3/Headers$Builder;)V + public fun (Lokhttp3/HttpUrl;Lmisk/web/SocketAddress;Lmisk/web/DispatchMechanism;Lokhttp3/Headers;IILmisk/web/http/HttpVersion;Lokhttp3/Headers$Builder;Z)V + public fun (Lokhttp3/HttpUrl;Lmisk/web/SocketAddress;Lmisk/web/DispatchMechanism;Lokhttp3/Headers;IILmisk/web/http/HttpVersion;Lokhttp3/Headers$Builder;ZLokhttp3/Headers$Builder;)V + public fun (Lokhttp3/HttpUrl;Lmisk/web/SocketAddress;Lmisk/web/DispatchMechanism;Lokhttp3/Headers;IILmisk/web/http/HttpVersion;Lokhttp3/Headers$Builder;ZLokhttp3/Headers$Builder;Lokio/BufferedSource;)V + public fun (Lokhttp3/HttpUrl;Lmisk/web/SocketAddress;Lmisk/web/DispatchMechanism;Lokhttp3/Headers;IILmisk/web/http/HttpVersion;Lokhttp3/Headers$Builder;ZLokhttp3/Headers$Builder;Lokio/BufferedSource;Lokio/BufferedSink;)V + public fun (Lokhttp3/HttpUrl;Lmisk/web/SocketAddress;Lmisk/web/DispatchMechanism;Lokhttp3/Headers;IILmisk/web/http/HttpVersion;Lokhttp3/Headers$Builder;ZLokhttp3/Headers$Builder;Lokio/BufferedSource;Lokio/BufferedSink;Lmisk/web/actions/WebSocket;)V + public fun (Lokhttp3/HttpUrl;Lmisk/web/SocketAddress;Lmisk/web/DispatchMechanism;Lokhttp3/Headers;IILmisk/web/http/HttpVersion;Lokhttp3/Headers$Builder;ZLokhttp3/Headers$Builder;Lokio/BufferedSource;Lokio/BufferedSink;Lmisk/web/actions/WebSocket;Lmisk/web/actions/WebSocketListener;)V + public fun (Lokhttp3/HttpUrl;Lmisk/web/SocketAddress;Lmisk/web/DispatchMechanism;Lokhttp3/Headers;IILmisk/web/http/HttpVersion;Lokhttp3/Headers$Builder;ZLokhttp3/Headers$Builder;Lokio/BufferedSource;Lokio/BufferedSink;Lmisk/web/actions/WebSocket;Lmisk/web/actions/WebSocketListener;Ljava/util/List;)V + public synthetic fun (Lokhttp3/HttpUrl;Lmisk/web/SocketAddress;Lmisk/web/DispatchMechanism;Lokhttp3/Headers;IILmisk/web/http/HttpVersion;Lokhttp3/Headers$Builder;ZLokhttp3/Headers$Builder;Lokio/BufferedSource;Lokio/BufferedSink;Lmisk/web/actions/WebSocket;Lmisk/web/actions/WebSocketListener;Ljava/util/List;ILkotlin/jvm/internal/DefaultConstructorMarker;)V public fun accepts ()Ljava/util/List; public fun addResponseHeaders (Lokhttp3/Headers;)V public fun asOkHttpRequest ()Lokhttp3/Request; public final fun component1 ()Lokhttp3/HttpUrl; - public final fun component10 ()Lokio/BufferedSource; - public final fun component11 ()Lokio/BufferedSink; - public final fun component12 ()Lmisk/web/actions/WebSocket; - public final fun component13 ()Lmisk/web/actions/WebSocketListener; - public final fun component14 ()Ljava/util/List; + public final fun component10 ()Lokhttp3/Headers$Builder; + public final fun component11 ()Lokio/BufferedSource; + public final fun component12 ()Lokio/BufferedSink; + public final fun component13 ()Lmisk/web/actions/WebSocket; + public final fun component14 ()Lmisk/web/actions/WebSocketListener; + public final fun component15 ()Ljava/util/List; public final fun component2 ()Lmisk/web/SocketAddress; public final fun component3 ()Lmisk/web/DispatchMechanism; public final fun component4 ()Lokhttp3/Headers; public final fun component5 ()I public final fun component6 ()I - public final fun component7 ()Lokhttp3/Headers$Builder; - public final fun component8 ()Z - public final fun component9 ()Lokhttp3/Headers$Builder; + public final fun component7 ()Lmisk/web/http/HttpVersion; + public final fun component8 ()Lokhttp3/Headers$Builder; + public final fun component9 ()Z public fun computeRequestHeader (Ljava/lang/String;Lkotlin/jvm/functions/Function1;)V public fun contentType ()Lokhttp3/MediaType; - public final fun copy (Lokhttp3/HttpUrl;Lmisk/web/SocketAddress;Lmisk/web/DispatchMechanism;Lokhttp3/Headers;IILokhttp3/Headers$Builder;ZLokhttp3/Headers$Builder;Lokio/BufferedSource;Lokio/BufferedSink;Lmisk/web/actions/WebSocket;Lmisk/web/actions/WebSocketListener;Ljava/util/List;)Lmisk/web/FakeHttpCall; - public static synthetic fun copy$default (Lmisk/web/FakeHttpCall;Lokhttp3/HttpUrl;Lmisk/web/SocketAddress;Lmisk/web/DispatchMechanism;Lokhttp3/Headers;IILokhttp3/Headers$Builder;ZLokhttp3/Headers$Builder;Lokio/BufferedSource;Lokio/BufferedSink;Lmisk/web/actions/WebSocket;Lmisk/web/actions/WebSocketListener;Ljava/util/List;ILjava/lang/Object;)Lmisk/web/FakeHttpCall; + public final fun copy (Lokhttp3/HttpUrl;Lmisk/web/SocketAddress;Lmisk/web/DispatchMechanism;Lokhttp3/Headers;IILmisk/web/http/HttpVersion;Lokhttp3/Headers$Builder;ZLokhttp3/Headers$Builder;Lokio/BufferedSource;Lokio/BufferedSink;Lmisk/web/actions/WebSocket;Lmisk/web/actions/WebSocketListener;Ljava/util/List;)Lmisk/web/FakeHttpCall; + public static synthetic fun copy$default (Lmisk/web/FakeHttpCall;Lokhttp3/HttpUrl;Lmisk/web/SocketAddress;Lmisk/web/DispatchMechanism;Lokhttp3/Headers;IILmisk/web/http/HttpVersion;Lokhttp3/Headers$Builder;ZLokhttp3/Headers$Builder;Lokio/BufferedSource;Lokio/BufferedSink;Lmisk/web/actions/WebSocket;Lmisk/web/actions/WebSocketListener;Ljava/util/List;ILjava/lang/Object;)Lmisk/web/FakeHttpCall; public fun equals (Ljava/lang/Object;)Z public fun getCookies ()Ljava/util/List; public fun getDispatchMechanism ()Lmisk/web/DispatchMechanism; public final fun getHeadersBuilder ()Lokhttp3/Headers$Builder; + public fun getHttpVersion ()Lmisk/web/http/HttpVersion; public fun getLinkLayerLocalAddress ()Lmisk/web/SocketAddress; public fun getNetworkStatusCode ()I public final fun getRequestBody ()Lokio/BufferedSource; diff --git a/misk-testing/build.gradle.kts b/misk-testing/build.gradle.kts index a31d5e6a521..8f53f042408 100644 --- a/misk-testing/build.gradle.kts +++ b/misk-testing/build.gradle.kts @@ -19,7 +19,6 @@ dependencies { api(libs.okHttp) api(libs.okHttpMockWebServer3) api(libs.openTracingMock) - api(libs.servletApi) api(libs.logbackClassic) api(project(":misk")) api(project(":misk-actions")) diff --git a/misk-testing/src/main/kotlin/misk/testing/MiskTestExtension.kt b/misk-testing/src/main/kotlin/misk/testing/MiskTestExtension.kt index 14d8f1da09d..3ded45be15e 100644 --- a/misk-testing/src/main/kotlin/misk/testing/MiskTestExtension.kt +++ b/misk-testing/src/main/kotlin/misk/testing/MiskTestExtension.kt @@ -31,23 +31,20 @@ internal class MiskTestExtension : BeforeEachCallback, AfterEachCallback { private val runningServices = ConcurrentHashMap.newKeySet>() private val log = getLogger() - private val maxLruSize: Int? = - System.getenv("MISK_TEST_REUSE_LRU_SIZE")?.toIntOrNull()?.takeIf { it > 0 } + private val maxLruSize: Int? = System.getenv("MISK_TEST_REUSE_LRU_SIZE")?.toIntOrNull()?.takeIf { it > 0 } // When MISK_TEST_REUSE_LRU_SIZE is set, the cache evicts least-recently-used entries once // the size is exceeded; the removal listener stops services for evicted injectors. Guava's // cache uses ConcurrentMap internally, so callers don't need extra synchronization. private val injectedModules: Cache, Injector> = run { - val builder = CacheBuilder.newBuilder() - .removalListener, Injector> { notification -> + val builder = + CacheBuilder.newBuilder().removalListener, Injector> { notification -> if (notification.cause == RemovalCause.EXPLICIT) return@removalListener val key = notification.key ?: return@removalListener val injector = notification.value ?: return@removalListener runningServices.remove(key) try { - val serviceManager = injector - .getExistingBinding(Key.get(ServiceManager::class.java)) - ?.provider?.get() + val serviceManager = injector.getExistingBinding(Key.get(ServiceManager::class.java))?.provider?.get() serviceManager?.stopAsync()?.awaitStopped(45, TimeUnit.SECONDS) } catch (e: Exception) { log.warn(e) { "Failed to stop services for evicted injector cache entry" } @@ -95,9 +92,7 @@ internal class MiskTestExtension : BeforeEachCallback, AfterEachCallback { val injector = if (context.reuseInjector()) { try { - injectedModules.get(context.getSortedActionTestModules()) { - Guice.createInjector(module) - } + injectedModules.get(context.getSortedActionTestModules()) { Guice.createInjector(module) } } catch (e: UncheckedExecutionException) { throw e.cause ?: e } diff --git a/misk-testing/src/main/kotlin/misk/web/AbstractWebActionRegistrationTest.kt b/misk-testing/src/main/kotlin/misk/web/AbstractWebActionRegistrationTest.kt index 24edec43527..4dcb008fde8 100644 --- a/misk-testing/src/main/kotlin/misk/web/AbstractWebActionRegistrationTest.kt +++ b/misk-testing/src/main/kotlin/misk/web/AbstractWebActionRegistrationTest.kt @@ -1,20 +1,17 @@ package misk.web import com.google.inject.Injector -import com.google.inject.Module +import jakarta.inject.Inject +import kotlin.reflect.KClass import misk.testing.MiskTest -import misk.testing.MiskTestModule import misk.web.actions.WebAction import org.junit.jupiter.api.Test -import jakarta.inject.Inject -import kotlin.reflect.KClass /** * Abstract base class for testing that all WebAction implementations are properly registered. * - * Extend this class and implement the abstract methods to get automatic verification that - * all WebAction implementations in your service's packages are registered via - * `WebActionModule.create()`. + * Extend this class and implement the abstract methods to get automatic verification that all WebAction implementations + * in your service's packages are registered via `WebActionModule.create()`. * * ## Usage * @@ -37,9 +34,8 @@ import kotlin.reflect.KClass * * ## What This Test Catches * - * This test catches a common mistake: implementing a WebAction but forgetting to register it. - * Without registration via `WebActionModule.create()`, the action won't be exposed - * as an HTTP endpoint. + * This test catches a common mistake: implementing a WebAction but forgetting to register it. Without registration via + * `WebActionModule.create()`, the action won't be exposed as an HTTP endpoint. * * ## Built-in Exclusions * @@ -52,22 +48,21 @@ import kotlin.reflect.KClass @MiskTest abstract class AbstractWebActionRegistrationTest { - @Inject - private lateinit var injector: Injector + @Inject private lateinit var injector: Injector /** * Returns the packages to scan for WebAction implementations. * - * This should be your service's root package(s), e.g. `listOf("com.example.myservice")`. - * Avoid overly broad packages like `listOf("com.example")` which might scan unrelated code. + * This should be your service's root package(s), e.g. `listOf("com.example.myservice")`. Avoid overly broad packages + * like `listOf("com.example")` which might scan unrelated code. */ protected abstract fun webActionPackages(): List /** * Returns true if the given action class should be excluded from registration verification. * - * Override this to exclude specific actions that are intentionally not registered, - * such as test-only actions or actions registered via different mechanisms. + * Override this to exclude specific actions that are intentionally not registered, such as test-only actions or + * actions registered via different mechanisms. * * Example: * ```kotlin @@ -81,8 +76,7 @@ abstract class AbstractWebActionRegistrationTest { /** * Returns a hint for error messages about where to register missing actions. * - * Override this to provide a clearer error message, e.g. `"WebModule"` or - * `"config/modules/WebActionsModule"`. + * Override this to provide a clearer error message, e.g. `"WebModule"` or `"config/modules/WebActionsModule"`. */ protected open fun registrationModuleHint(): String? = null diff --git a/misk-testing/src/main/kotlin/misk/web/FakeHttpCall.kt b/misk-testing/src/main/kotlin/misk/web/FakeHttpCall.kt index 76488ac9106..a0c6804fb57 100644 --- a/misk-testing/src/main/kotlin/misk/web/FakeHttpCall.kt +++ b/misk-testing/src/main/kotlin/misk/web/FakeHttpCall.kt @@ -3,6 +3,7 @@ package misk.web import jakarta.servlet.http.Cookie import misk.web.actions.WebSocket import misk.web.actions.WebSocketListener +import misk.web.http.HttpVersion import okhttp3.Headers import okhttp3.Headers.Companion.headersOf import okhttp3.HttpUrl @@ -20,6 +21,7 @@ constructor( override var requestHeaders: Headers = headersOf(), override var statusCode: Int = 200, override var networkStatusCode: Int = 200, + override val httpVersion: HttpVersion = HttpVersion.HTTP_1_1, val headersBuilder: Headers.Builder = Headers.Builder(), var sendTrailers: Boolean = false, val trailersBuilder: Headers.Builder = Headers.Builder(), diff --git a/misk-testing/src/main/kotlin/misk/web/WebActionRegistrationTesting.kt b/misk-testing/src/main/kotlin/misk/web/WebActionRegistrationTesting.kt index c834634c2f6..ffd0f26a676 100644 --- a/misk-testing/src/main/kotlin/misk/web/WebActionRegistrationTesting.kt +++ b/misk-testing/src/main/kotlin/misk/web/WebActionRegistrationTesting.kt @@ -4,18 +4,17 @@ import com.google.inject.Injector import com.google.inject.Key import com.google.inject.TypeLiteral import io.github.classgraph.ClassGraph +import java.lang.reflect.Modifier +import kotlin.reflect.KClass import misk.web.actions.WebAction import misk.web.actions.WebActionEntry import org.assertj.core.api.Assertions.assertThat -import java.lang.reflect.Modifier -import kotlin.reflect.KClass /** * Test utilities for verifying that all WebAction implementations are properly registered. * - * This utility helps catch a common mistake: implementing a WebAction but forgetting to register - * it via `WebActionModule.create()`. Without registration, the action won't be exposed - * as an HTTP endpoint. + * This utility helps catch a common mistake: implementing a WebAction but forgetting to register it via + * `WebActionModule.create()`. Without registration, the action won't be exposed as an HTTP endpoint. * * ## Usage * @@ -39,22 +38,22 @@ object WebActionRegistrationTester { /** * Configuration options for web action registration testing. * - * @param basePackages Packages to scan for WebAction implementations. Should be the service's - * root package(s), not broad packages like "com.squareup" which would scan too much. - * @param excludePredicate Additional filter to exclude specific action classes beyond the - * built-in exclusions. Return true to exclude the action from verification. - * @param registrationModuleHint A hint shown in error messages about where to register actions. - * Example: "WebModule" or "config/modules/WebActionsModule". + * @param basePackages Packages to scan for WebAction implementations. Should be the service's root package(s), not + * broad packages like "com.squareup" which would scan too much. + * @param excludePredicate Additional filter to exclude specific action classes beyond the built-in exclusions. Return + * true to exclude the action from verification. + * @param registrationModuleHint A hint shown in error messages about where to register actions. Example: "WebModule" + * or "config/modules/WebActionsModule". */ - data class Options @JvmOverloads constructor( + data class Options + @JvmOverloads + constructor( val basePackages: List, val excludePredicate: (KClass) -> Boolean = { false }, val registrationModuleHint: String? = null, ) - /** - * Built-in exclusions for classes that shouldn't be checked for registration. - */ + /** Built-in exclusions for classes that shouldn't be checked for registration. */ private fun shouldExcludeByDefault(clazz: Class<*>): Boolean { return clazz.isInterface || Modifier.isAbstract(clazz.modifiers) || @@ -63,65 +62,50 @@ object WebActionRegistrationTester { } /** - * Asserts that all concrete WebAction implementations found in [options.basePackages] have a - * corresponding [WebActionEntry] registered in the given [injector]. + * Asserts that all concrete WebAction implementations found in [options.basePackages] have a corresponding + * [WebActionEntry] registered in the given [injector]. * * This catches a common mistake: implementing a WebAction but forgetting to register it via * `WebActionModule.create()`. * * @param injector The Guice injector to check for registered actions. * @param options Configuration for scanning and exclusions. - * @throws AssertionError if any WebAction implementations are found that aren't registered, - * with a helpful message including copy-paste registration code. + * @throws AssertionError if any WebAction implementations are found that aren't registered, with a helpful message + * including copy-paste registration code. */ - fun assertAllWebActionsRegistered( - injector: Injector, - options: Options, - ) { + fun assertAllWebActionsRegistered(injector: Injector, options: Options) { val discoveredActions = discoverWebActions(options) val registeredActions = discoverRegisteredWebActions(injector) - val missing = discoveredActions - .filterNot { it in registeredActions } - .sortedBy { it.qualifiedName } + val missing = discoveredActions.filterNot { it in registeredActions }.sortedBy { it.qualifiedName } assertThat(missing) .overridingErrorMessage { buildMissingErrorMessage(missing, options.registrationModuleHint) } .isEmpty() } - /** - * Scans the classpath for concrete WebAction implementations in the specified packages. - */ + /** Scans the classpath for concrete WebAction implementations in the specified packages. */ private fun discoverWebActions(options: Options): Set> { val packages = options.basePackages.toTypedArray() - return ClassGraph() - .enableClassInfo() - .acceptPackages(*packages) - .scan() - .use { result -> - result - .getClassesImplementing(WebAction::class.java.name) - .filter { classInfo -> - val clazz = classInfo.loadClass() - !shouldExcludeByDefault(clazz) - } - .map { it.loadClass().kotlin.asWebActionClass() } - .filterNot { options.excludePredicate(it) } - .toSet() - } + return ClassGraph().enableClassInfo().acceptPackages(*packages).scan().use { result -> + result + .getClassesImplementing(WebAction::class.java.name) + .filter { classInfo -> + val clazz = classInfo.loadClass() + !shouldExcludeByDefault(clazz) + } + .map { it.loadClass().kotlin.asWebActionClass() } + .filterNot { options.excludePredicate(it) } + .toSet() + } } - /** - * Discovers all WebAction classes that have been registered via WebActionEntry bindings. - */ + /** Discovers all WebAction classes that have been registered via WebActionEntry bindings. */ private fun discoverRegisteredWebActions(injector: Injector): Set> { // Try to get the multibound List first return try { - val entries = injector.getInstance( - Key.get(object : TypeLiteral>() {}) - ) + val entries = injector.getInstance(Key.get(object : TypeLiteral>() {})) entries.map { it.actionClass }.toSet() } catch (e: Exception) { // Fall back to scanning all bindings if the list isn't bound @@ -130,8 +114,8 @@ object WebActionRegistrationTester { } /** - * Fallback method to discover WebActionEntry bindings by scanning all injector bindings. - * This handles cases where the multibound list isn't available. + * Fallback method to discover WebActionEntry bindings by scanning all injector bindings. This handles cases where the + * multibound list isn't available. */ private fun discoverWebActionEntriesFromBindings(injector: Injector): Set> { val actionClasses = mutableSetOf>() @@ -150,22 +134,15 @@ object WebActionRegistrationTester { return actionClasses } - /** - * Builds a helpful error message for missing action registrations. - */ - private fun buildMissingErrorMessage( - missing: List>, - registrationModuleHint: String?, - ): String { + /** Builds a helpful error message for missing action registrations. */ + private fun buildMissingErrorMessage(missing: List>, registrationModuleHint: String?): String { if (missing.isEmpty()) return "" val moduleHint = registrationModuleHint?.let { " in $it" } ?: "" return buildString { appendLine("The following WebActions are not registered:") - missing.forEach { actionClass -> - appendLine(" - ${actionClass.qualifiedName}") - } + missing.forEach { actionClass -> appendLine(" - ${actionClass.qualifiedName}") } appendLine() appendLine("Copy and paste the following lines into your WebAction registration module$moduleHint:") appendLine("-----") diff --git a/misk-testing/src/test/kotlin/misk/web/WebActionRegistrationTesterTest.kt b/misk-testing/src/test/kotlin/misk/web/WebActionRegistrationTesterTest.kt index 39d47629cc8..05477ee173f 100644 --- a/misk-testing/src/test/kotlin/misk/web/WebActionRegistrationTesterTest.kt +++ b/misk-testing/src/test/kotlin/misk/web/WebActionRegistrationTesterTest.kt @@ -1,24 +1,24 @@ package misk.web import com.google.inject.Guice +import jakarta.inject.Inject import misk.inject.KAbstractModule import misk.web.actions.WebAction -import org.assertj.core.api.Assertions.assertThat import org.assertj.core.api.Assertions.assertThatThrownBy import org.junit.jupiter.api.Test -import jakarta.inject.Inject class WebActionRegistrationTesterTest { @Test fun `passes when all actions are registered`() { - val injector = Guice.createInjector( - object : KAbstractModule() { - override fun configure() { - install(WebActionModule.create()) + val injector = + Guice.createInjector( + object : KAbstractModule() { + override fun configure() { + install(WebActionModule.create()) + } } - } - ) + ) WebActionRegistrationTester.assertAllWebActionsRegistered( injector, @@ -28,32 +28,33 @@ class WebActionRegistrationTesterTest { // Exclude test actions from other test classes in this package actionClass != RegisteredAction::class }, - ) + ), ) } @Test fun `fails when actions are not registered`() { - val injector = Guice.createInjector( - object : KAbstractModule() { - override fun configure() { - // Intentionally not registering UnregisteredAction + val injector = + Guice.createInjector( + object : KAbstractModule() { + override fun configure() { + // Intentionally not registering UnregisteredAction + } } - } - ) + ) assertThatThrownBy { - WebActionRegistrationTester.assertAllWebActionsRegistered( - injector, - WebActionRegistrationTester.Options( - basePackages = listOf("misk.web"), - excludePredicate = { actionClass -> - // Only check UnregisteredAction for this test - actionClass != UnregisteredAction::class - }, + WebActionRegistrationTester.assertAllWebActionsRegistered( + injector, + WebActionRegistrationTester.Options( + basePackages = listOf("misk.web"), + excludePredicate = { actionClass -> + // Only check UnregisteredAction for this test + actionClass != UnregisteredAction::class + }, + ), ) - ) - } + } .isInstanceOf(AssertionError::class.java) .hasMessageContaining("UnregisteredAction") .hasMessageContaining("WebActionModule.create") @@ -61,13 +62,14 @@ class WebActionRegistrationTesterTest { @Test fun `excludes abstract classes`() { - val injector = Guice.createInjector( - object : KAbstractModule() { - override fun configure() { - // No actions registered + val injector = + Guice.createInjector( + object : KAbstractModule() { + override fun configure() { + // No actions registered + } } - } - ) + ) // Should not fail because AbstractAction is abstract WebActionRegistrationTester.assertAllWebActionsRegistered( @@ -78,19 +80,20 @@ class WebActionRegistrationTesterTest { // Exclude all non-abstract test actions actionClass != AbstractAction::class }, - ) + ), ) } @Test fun `respects custom exclude predicate`() { - val injector = Guice.createInjector( - object : KAbstractModule() { - override fun configure() { - // Intentionally not registering ExcludedAction + val injector = + Guice.createInjector( + object : KAbstractModule() { + override fun configure() { + // Intentionally not registering ExcludedAction + } } - } - ) + ) // Should pass because ExcludedAction is excluded via predicate WebActionRegistrationTester.assertAllWebActionsRegistered( @@ -101,32 +104,31 @@ class WebActionRegistrationTesterTest { // Exclude all test actions true }, - ) + ), ) } @Test fun `error message includes registration module hint`() { - val injector = Guice.createInjector( - object : KAbstractModule() { - override fun configure() { - // Intentionally not registering + val injector = + Guice.createInjector( + object : KAbstractModule() { + override fun configure() { + // Intentionally not registering + } } - } - ) + ) assertThatThrownBy { - WebActionRegistrationTester.assertAllWebActionsRegistered( - injector, - WebActionRegistrationTester.Options( - basePackages = listOf("misk.web"), - excludePredicate = { actionClass -> - actionClass != UnregisteredAction::class - }, - registrationModuleHint = "MyWebModule", + WebActionRegistrationTester.assertAllWebActionsRegistered( + injector, + WebActionRegistrationTester.Options( + basePackages = listOf("misk.web"), + excludePredicate = { actionClass -> actionClass != UnregisteredAction::class }, + registrationModuleHint = "MyWebModule", + ), ) - ) - } + } .isInstanceOf(AssertionError::class.java) .hasMessageContaining("MyWebModule") } @@ -134,19 +136,16 @@ class WebActionRegistrationTesterTest { // Test actions used for verification class RegisteredAction @Inject constructor() : WebAction { - @Get("/registered") - fun get(): String = "registered" + @Get("/registered") fun get(): String = "registered" } class UnregisteredAction @Inject constructor() : WebAction { - @Get("/unregistered") - fun get(): String = "unregistered" + @Get("/unregistered") fun get(): String = "unregistered" } abstract class AbstractAction : WebAction class ExcludedAction @Inject constructor() : WebAction { - @Get("/excluded") - fun get(): String = "excluded" + @Get("/excluded") fun get(): String = "excluded" } } diff --git a/misk-vitess-database-gradle-plugin/src/main/kotlin/misk/vitess/gradle/VitessDatabasePlugin.kt b/misk-vitess-database-gradle-plugin/src/main/kotlin/misk/vitess/gradle/VitessDatabasePlugin.kt index 2cd3de31828..d9f5418e522 100644 --- a/misk-vitess-database-gradle-plugin/src/main/kotlin/misk/vitess/gradle/VitessDatabasePlugin.kt +++ b/misk-vitess-database-gradle-plugin/src/main/kotlin/misk/vitess/gradle/VitessDatabasePlugin.kt @@ -9,46 +9,43 @@ import org.gradle.api.tasks.testing.Test class VitessDatabasePlugin : Plugin { override fun apply(project: Project) { - val startVitessDatabase = project.tasks.register("startVitessDatabase", StartVitessDatabaseTask::class.java) { - it.autoApplySchemaChanges.convention(DefaultSettings.AUTO_APPLY_SCHEMA_CHANGES) - it.containerName.convention(DefaultSettings.CONTAINER_NAME) - it.debugStartup.convention(DefaultSettings.DEBUG_STARTUP) - it.dockerNetworkName.convention(DefaultSettings.VITESS_DOCKER_NETWORK_NAME) - it.enableDeclarativeSchemaChanges.convention(DefaultSettings.ENABLE_DECLARATIVE_SCHEMA_CHANGES) - it.enableInMemoryStorage.convention(DefaultSettings.ENABLE_IN_MEMORY_STORAGE) - it.enableScatters.convention(DefaultSettings.ENABLE_SCATTERS) - it.inMemoryStorageSize.convention(DefaultSettings.IN_MEMORY_STORAGE_SIZE) - it.keepAlive.convention(DefaultSettings.KEEP_ALIVE) - it.lintSchema.convention(DefaultSettings.LINT_SCHEMA) - it.mysqlVersion.convention(DefaultSettings.MYSQL_VERSION) - it.port.convention(DefaultSettings.PORT) - it.schemaDir.convention( - "filesystem:${project.layout.projectDirectory.dir("src/main/resources/vitess/schema").asFile.absolutePath}" - ) - it.sqlMode.convention(DefaultSettings.SQL_MODE) - it.transactionIsolationLevel.convention(DefaultSettings.TRANSACTION_ISOLATION_LEVEL) - it.transactionMode.convention(DefaultSettings.TRANSACTION_MODE) - it.transactionTimeoutSeconds.convention(DefaultSettings.TRANSACTION_TIMEOUT_SECONDS) - it.vitessImage.convention(DefaultSettings.VITESS_IMAGE) - it.vitessVersion.convention(DefaultSettings.VITESS_VERSION) - } + val startVitessDatabase = + project.tasks.register("startVitessDatabase", StartVitessDatabaseTask::class.java) { + it.autoApplySchemaChanges.convention(DefaultSettings.AUTO_APPLY_SCHEMA_CHANGES) + it.containerName.convention(DefaultSettings.CONTAINER_NAME) + it.debugStartup.convention(DefaultSettings.DEBUG_STARTUP) + it.dockerNetworkName.convention(DefaultSettings.VITESS_DOCKER_NETWORK_NAME) + it.enableDeclarativeSchemaChanges.convention(DefaultSettings.ENABLE_DECLARATIVE_SCHEMA_CHANGES) + it.enableInMemoryStorage.convention(DefaultSettings.ENABLE_IN_MEMORY_STORAGE) + it.enableScatters.convention(DefaultSettings.ENABLE_SCATTERS) + it.inMemoryStorageSize.convention(DefaultSettings.IN_MEMORY_STORAGE_SIZE) + it.keepAlive.convention(DefaultSettings.KEEP_ALIVE) + it.lintSchema.convention(DefaultSettings.LINT_SCHEMA) + it.mysqlVersion.convention(DefaultSettings.MYSQL_VERSION) + it.port.convention(DefaultSettings.PORT) + it.schemaDir.convention( + "filesystem:${project.layout.projectDirectory.dir("src/main/resources/vitess/schema").asFile.absolutePath}" + ) + it.sqlMode.convention(DefaultSettings.SQL_MODE) + it.transactionIsolationLevel.convention(DefaultSettings.TRANSACTION_ISOLATION_LEVEL) + it.transactionMode.convention(DefaultSettings.TRANSACTION_MODE) + it.transactionTimeoutSeconds.convention(DefaultSettings.TRANSACTION_TIMEOUT_SECONDS) + it.vitessImage.convention(DefaultSettings.VITESS_IMAGE) + it.vitessVersion.convention(DefaultSettings.VITESS_VERSION) + } // Forward StartVitessDatabaseTask @Input properties to Test tasks so that changes // to Vitess configuration (e.g. enableScatters) invalidate the test cache. // Without this, StartVitessDatabaseTask's @UntrackedTask annotation means its // properties don't participate in any cache fingerprint, so downstream tests // can be served from cache even when the Vitess config has changed. - val inputGetters = StartVitessDatabaseTask::class.java.methods - .filter { it.isAnnotationPresent(Input::class.java) } + val inputGetters = StartVitessDatabaseTask::class.java.methods.filter { it.isAnnotationPresent(Input::class.java) } project.tasks.withType(Test::class.java).configureEach { test -> for (getter in inputGetters) { val name = getter.name.removePrefix("get").replaceFirstChar { it.lowercase() } @Suppress("UNCHECKED_CAST") - test.inputs.property( - "vitess.$name", - startVitessDatabase.flatMap { getter.invoke(it) as Provider } - ) + test.inputs.property("vitess.$name", startVitessDatabase.flatMap { getter.invoke(it) as Provider }) } } } diff --git a/misk-vitess/src/main/kotlin/misk/vitess/CrossShardTransactionException.kt b/misk-vitess/src/main/kotlin/misk/vitess/CrossShardTransactionException.kt index 6070a2fd554..a806bfc87e4 100644 --- a/misk-vitess/src/main/kotlin/misk/vitess/CrossShardTransactionException.kt +++ b/misk-vitess/src/main/kotlin/misk/vitess/CrossShardTransactionException.kt @@ -8,12 +8,12 @@ import misk.jdbc.CheckException * mix of both within the same transaction. * * This exception is thrown when the vtgate is configured with `--transaction_mode=SINGLE`, which rejects any - * transaction that touches more than one shard. Sessions can opt in to cross-shard transactions via - * `SET transaction_mode = 'multi'`. + * transaction that touches more than one shard. Sessions can opt in to cross-shard transactions via `SET + * transaction_mode = 'multi'`. * * Without two-phase commit (TWOPC), cross-shard transactions use best-effort commit semantics — there is no guarantee - * of atomicity across shards. Note that even TWOPC only provides atomic commits for writes; it does not provide - * full ACID cross-shard read isolation. + * of atomicity across shards. Note that even TWOPC only provides atomic commits for writes; it does not provide full + * ACID cross-shard read isolation. * * See https://vitess.io/docs/reference/features/distributed-transaction/ */ diff --git a/misk-vitess/src/test/kotlin/misk/vitess/testing/internal/VitessImageUtilsTest.kt b/misk-vitess/src/test/kotlin/misk/vitess/testing/internal/VitessImageUtilsTest.kt index cac35272a75..71df5499846 100644 --- a/misk-vitess/src/test/kotlin/misk/vitess/testing/internal/VitessImageUtilsTest.kt +++ b/misk-vitess/src/test/kotlin/misk/vitess/testing/internal/VitessImageUtilsTest.kt @@ -15,10 +15,7 @@ class VitessImageUtilsTest { @Test fun `derives vtctldclient from upstream vitess image`() { - assertEquals( - "vitess/vtctldclient:v21.0.4", - deriveVtctldClientImage("vitess/vttestserver:v21.0.4-mysql80"), - ) + assertEquals("vitess/vtctldclient:v21.0.4", deriveVtctldClientImage("vitess/vttestserver:v21.0.4-mysql80")) } @Test @@ -31,18 +28,12 @@ class VitessImageUtilsTest { @Test fun `derives vtctldclient with mysql80 suffix`() { - assertEquals( - "vitess/vtctldclient:v22.0.2", - deriveVtctldClientImage("vitess/vttestserver:v22.0.2-mysql80"), - ) + assertEquals("vitess/vtctldclient:v22.0.2", deriveVtctldClientImage("vitess/vttestserver:v22.0.2-mysql80")) } @Test fun `derives vtctldclient without mysql suffix is unchanged`() { - assertEquals( - "vitess/vtctldclient:v23.0.3", - deriveVtctldClientImage("vitess/vttestserver:v23.0.3"), - ) + assertEquals("vitess/vtctldclient:v23.0.3", deriveVtctldClientImage("vitess/vttestserver:v23.0.3")) } @Test diff --git a/misk-vitess/src/testFixtures/kotlin/misk/vitess/testing/internal/VitessImageUtils.kt b/misk-vitess/src/testFixtures/kotlin/misk/vitess/testing/internal/VitessImageUtils.kt index 53a1a315015..cb29bfc76fc 100644 --- a/misk-vitess/src/testFixtures/kotlin/misk/vitess/testing/internal/VitessImageUtils.kt +++ b/misk-vitess/src/testFixtures/kotlin/misk/vitess/testing/internal/VitessImageUtils.kt @@ -1,14 +1,12 @@ package misk.vitess.testing.internal /** - * Derives the vtctldclient image URL from the vttestserver image URL. - * Swaps `vttestserver` -> `vtctldclient` and strips the `-mysql{version}` suffix from the tag. + * Derives the vtctldclient image URL from the vttestserver image URL. Swaps `vttestserver` -> `vtctldclient` and strips + * the `-mysql{version}` suffix from the tag. * - * Example: `ghcr.io/block/vitess/vttestserver:23.0.3-block.1-mysql84` - * -> `ghcr.io/block/vitess/vtctldclient:23.0.3-block.1` + * Example: `ghcr.io/block/vitess/vttestserver:23.0.3-block.1-mysql84` -> + * `ghcr.io/block/vitess/vtctldclient:23.0.3-block.1` */ internal fun deriveVtctldClientImage(vttestserverImage: String): String { - return vttestserverImage - .replace("vttestserver", "vtctldclient") - .replace(Regex("-mysql\\d+$"), "") + return vttestserverImage.replace("vttestserver", "vtctldclient").replace(Regex("-mysql\\d+$"), "") } diff --git a/misk-vitess/src/testFixtures/kotlin/misk/vitess/testing/utilities/DockerVitess.kt b/misk-vitess/src/testFixtures/kotlin/misk/vitess/testing/utilities/DockerVitess.kt index 7f779e7535f..381070845d2 100644 --- a/misk-vitess/src/testFixtures/kotlin/misk/vitess/testing/utilities/DockerVitess.kt +++ b/misk-vitess/src/testFixtures/kotlin/misk/vitess/testing/utilities/DockerVitess.kt @@ -31,13 +31,14 @@ class DockerVitess( dockerNetworkName: String = DefaultSettings.VITESS_DOCKER_NETWORK_NAME, ) : ExternalDependency { - private val vitessTestDb = VitessTestDb( - containerName = containerName, - dockerNetworkName = dockerNetworkName, - enableScatters = enableScatters, - transactionMode = transactionMode, - port = port, - ) + private val vitessTestDb = + VitessTestDb( + containerName = containerName, + dockerNetworkName = dockerNetworkName, + enableScatters = enableScatters, + transactionMode = transactionMode, + port = port, + ) override fun startup() { vitessTestDb.run() diff --git a/misk/api/misk.api b/misk/api/misk.api index 444d456c0d8..05ece03220f 100644 --- a/misk/api/misk.api +++ b/misk/api/misk.api @@ -1180,6 +1180,7 @@ public abstract interface class misk/web/HttpCall : misk/api/HttpRequest { public fun computeRequestHeader (Ljava/lang/String;Lkotlin/jvm/functions/Function1;)V public fun contentType ()Lokhttp3/MediaType; public abstract fun getCookies ()Ljava/util/List; + public abstract fun getHttpVersion ()Lmisk/web/http/HttpVersion; public abstract fun getLinkLayerLocalAddress ()Lmisk/web/SocketAddress; public abstract fun getNetworkStatusCode ()I public abstract fun getRequestReceivedTimestamp ()J @@ -1362,6 +1363,10 @@ public final class misk/web/ResponseExtensionsKt { } public abstract class misk/web/SocketAddress { + public static final field Companion Lmisk/web/SocketAddress$Companion; +} + +public final class misk/web/SocketAddress$Companion { } public final class misk/web/SocketAddress$Network : misk/web/SocketAddress { @@ -1820,6 +1825,21 @@ public final class misk/web/formatter/ClassNameFormatter$Companion { public final fun format (Lkotlin/reflect/KClass;)Ljava/lang/String; } +public final class misk/web/http/HttpVersion : java/lang/Enum { + public static final field Companion Lmisk/web/http/HttpVersion$Companion; + public static final field HTTP_0_9 Lmisk/web/http/HttpVersion; + public static final field HTTP_1_0 Lmisk/web/http/HttpVersion; + public static final field HTTP_1_1 Lmisk/web/http/HttpVersion; + public static final field HTTP_2_0 Lmisk/web/http/HttpVersion; + public static final field HTTP_3_0 Lmisk/web/http/HttpVersion; + public static fun getEntries ()Lkotlin/enums/EnumEntries; + public static fun valueOf (Ljava/lang/String;)Lmisk/web/http/HttpVersion; + public static fun values ()[Lmisk/web/http/HttpVersion; +} + +public final class misk/web/http/HttpVersion$Companion { +} + public final class misk/web/interceptors/ActionLoggingConfig { public static final field Companion Lmisk/web/interceptors/ActionLoggingConfig$Companion; public fun ()V @@ -2056,14 +2076,14 @@ public final class misk/web/jetty/MeasuredThreadPoolExecutor : misk/web/jetty/Me public fun queueSize ()I } -public final class misk/web/jetty/MeasuredWindowRateControl : org/eclipse/jetty/http2/parser/RateControl { +public final class misk/web/jetty/MeasuredWindowRateControl : org/eclipse/jetty/http2/RateControl { public synthetic fun (ILmisk/metrics/v2/PeakGauge;Lio/prometheus/client/Counter;Lkotlin/jvm/internal/DefaultConstructorMarker;)V public fun onEvent (Ljava/lang/Object;)Z } -public final class misk/web/jetty/MeasuredWindowRateControl$Factory : org/eclipse/jetty/http2/parser/RateControl$Factory { +public final class misk/web/jetty/MeasuredWindowRateControl$Factory : org/eclipse/jetty/http2/RateControl$Factory { public fun (Lmisk/metrics/v2/Metrics;Lmisk/web/WebConfig;)V - public fun newRateControl (Lorg/eclipse/jetty/io/EndPoint;)Lorg/eclipse/jetty/http2/parser/RateControl; + public fun newRateControl (Lorg/eclipse/jetty/io/EndPoint;)Lorg/eclipse/jetty/http2/RateControl; } public final class misk/web/jetty/ThreadPoolQueueMetrics { diff --git a/misk/build.gradle.kts b/misk/build.gradle.kts index 37f57ef51e8..08455b2923d 100644 --- a/misk/build.gradle.kts +++ b/misk/build.gradle.kts @@ -47,14 +47,14 @@ dependencies { api(project(":misk-feature")) implementation(libs.jCommander) implementation(libs.jettyAlpnServer) + implementation(libs.jettyEe9Nested) implementation(libs.jettyHttp) implementation(libs.jettyHttp2) implementation(libs.jettyServlet) implementation(libs.jettyServlets) implementation(libs.jettyUds) - implementation(libs.jettyUnixSocket) - implementation(libs.jettyWebsocketApi) - implementation(libs.jettyWebsocketServer) + implementation(libs.jettyWebsocketApiEE9) + implementation(libs.jettyWebsocketServerEE9) implementation(libs.jnrUnixsocket) implementation(libs.kotlinReflect) implementation(libs.kotlinStdLibJdk8) @@ -119,7 +119,7 @@ wire { rpcCallStyle = "blocking" exclusive = false includes = listOf( - "helloworld.Greeter" + "helloworld.Greeter", ) } @@ -130,7 +130,7 @@ wire { exclusive = false singleMethodServices = true includes = listOf( - "helloworld.Greeter" + "helloworld.Greeter", ) } } @@ -149,7 +149,7 @@ afterEvaluate { } kotlinSourceSets?.getByName("main")?.kotlin?.setSrcDirs( - kotlinSourceSets.getByName("main").kotlin.srcDirs.filter { !it.path.contains(generatedSourceDir) } + kotlinSourceSets.getByName("main").kotlin.srcDirs.filter { !it.path.contains(generatedSourceDir) }, ) kotlinSourceSets?.getByName("test")?.kotlin?.srcDir(generatedSourceDir) diff --git a/misk/src/main/kotlin/misk/MiskApplication.kt b/misk/src/main/kotlin/misk/MiskApplication.kt index ac25bb13a17..d0c90035981 100644 --- a/misk/src/main/kotlin/misk/MiskApplication.kt +++ b/misk/src/main/kotlin/misk/MiskApplication.kt @@ -8,7 +8,6 @@ import com.google.inject.Guice import com.google.inject.Injector import com.google.inject.Key import com.google.inject.Module -import misk.web.WebConfig import java.util.concurrent.TimeUnit import java.util.concurrent.TimeoutException import kotlin.concurrent.thread @@ -17,6 +16,7 @@ import kotlin.time.Duration.Companion.milliseconds import misk.inject.KAbstractModule import misk.inject.getInstance import misk.logging.getLogger +import misk.web.WebConfig import misk.web.jetty.JettyHealthService import misk.web.jetty.JettyService @@ -112,9 +112,10 @@ private constructor(private val injectorGenerator: () -> Injector, commands: Lis // We manage JettyHealthService outside ServiceManager because it must start and // shutdown last to keep the container alive via liveness checks. // Skip instantiation entirely when Jetty is disabled to avoid registering Jetty metrics. - val jettyEnabled = injector.getExistingBinding(Key.get(WebConfig::class.java)) - ?.let { !injector.getInstance().disable_jetty } - ?: true + val jettyEnabled = + injector.getExistingBinding(Key.get(WebConfig::class.java))?.let { + !injector.getInstance().disable_jetty + } ?: true val jettyHealthService: JettyHealthService? measureTimeMillis { log.info { "starting services" } @@ -132,14 +133,15 @@ private constructor(private val injectorGenerator: () -> Injector, commands: Lis } // Start Health Service Last to ensure any dependencies are started. - jettyHealthService = if (jettyEnabled) { - injector.getInstance().also { - it.startAsync() - it.awaitRunning() + jettyHealthService = + if (jettyEnabled) { + injector.getInstance().also { + it.startAsync() + it.awaitRunning() + } + } else { + null } - } else { - null - } } .also { log.info { "all services started successfully in ${it.milliseconds}" } } diff --git a/misk/src/main/kotlin/misk/web/HttpCall.kt b/misk/src/main/kotlin/misk/web/HttpCall.kt index 190513bcb06..2a36d750486 100644 --- a/misk/src/main/kotlin/misk/web/HttpCall.kt +++ b/misk/src/main/kotlin/misk/web/HttpCall.kt @@ -1,9 +1,12 @@ package misk.web +import java.net.InetSocketAddress +import java.net.UnixDomainSocketAddress import jakarta.servlet.http.Cookie import misk.api.HttpRequest import misk.web.actions.WebSocket import misk.web.actions.WebSocketListener +import misk.web.http.HttpVersion import misk.web.mediatype.MediaRange import okhttp3.Headers import okhttp3.MediaType @@ -18,6 +21,16 @@ sealed class SocketAddress { class Network(val ipAddress: String, val port: Int) : SocketAddress() class Unix(val path: String) : SocketAddress() + + companion object { + internal fun from(javaSocketAddress: java.net.SocketAddress): SocketAddress { + return when (javaSocketAddress) { + is InetSocketAddress -> Network(javaSocketAddress.address.hostAddress, javaSocketAddress.port) + is UnixDomainSocketAddress -> Unix(javaSocketAddress.path.toString()) + else -> throw IllegalArgumentException("Unknown SocketAddress type ${javaSocketAddress.javaClass.simpleName}") + } + } + } } /** A live HTTP call from a client for use by a chain of network interceptors. */ @@ -44,6 +57,8 @@ interface HttpCall : HttpRequest { /** Timestamp when the request was received (milliseconds since epoch) */ val requestReceivedTimestamp: Long + val httpVersion: HttpVersion + /** Set both the raw network status code and the meaningful status code that's recorded in metrics */ fun setStatusCodes(statusCode: Int, networkStatusCode: Int) diff --git a/misk/src/main/kotlin/misk/web/ServletHttpCall.kt b/misk/src/main/kotlin/misk/web/ServletHttpCall.kt index 3e03fb3e52a..eaf2e5d8e30 100644 --- a/misk/src/main/kotlin/misk/web/ServletHttpCall.kt +++ b/misk/src/main/kotlin/misk/web/ServletHttpCall.kt @@ -4,13 +4,14 @@ import jakarta.servlet.http.Cookie import jakarta.servlet.http.HttpServletRequest import misk.web.actions.WebSocket import misk.web.actions.WebSocketListener +import misk.web.http.HttpVersion import misk.web.jetty.headers import misk.web.jetty.httpUrl import okhttp3.Headers import okhttp3.HttpUrl import okio.BufferedSink import okio.BufferedSource -import org.eclipse.jetty.server.Request +import org.eclipse.jetty.ee9.nested.Request internal data class ServletHttpCall( override val url: HttpUrl, @@ -43,6 +44,9 @@ internal data class ServletHttpCall( override val responseHeaders: Headers get() = upstreamResponse.headers + override val httpVersion: HttpVersion + get() = upstreamResponse.httpVersion + override fun setStatusCodes(statusCode: Int, networkStatusCode: Int) { _actualStatusCode = statusCode upstreamResponse.statusCode = networkStatusCode @@ -105,6 +109,7 @@ internal data class ServletHttpCall( interface UpstreamResponse { var statusCode: Int val headers: Headers + val httpVersion: HttpVersion fun setHeader(name: String, value: String) diff --git a/misk/src/main/kotlin/misk/web/actions/WebActionFactory.kt b/misk/src/main/kotlin/misk/web/actions/WebActionFactory.kt index f6c9e8797ce..17669b165e0 100644 --- a/misk/src/main/kotlin/misk/web/actions/WebActionFactory.kt +++ b/misk/src/main/kotlin/misk/web/actions/WebActionFactory.kt @@ -231,10 +231,9 @@ constructor( } /** - * Returns a copy of [action] that converts a gRPC action into a protobuf POST action, or null - * if the action is not a gRPC action or does not have @EnableUnframedRequests. This enables a - * single gRPC action ([WireRpc] or [Grpc]) to accept plain HTTP POST with - * `application/x-protobuf` content type (unframed protobuf, without gRPC framing). + * Returns a copy of [action] that converts a gRPC action into a protobuf POST action, or null if the action is not a + * gRPC action or does not have @EnableUnframedRequests. This enables a single gRPC action ([WireRpc] or [Grpc]) to + * accept plain HTTP POST with `application/x-protobuf` content type (unframed protobuf, without gRPC framing). */ private fun transformActionIntoProtobufPost(action: Action): Action? { if (action.dispatchMechanism != DispatchMechanism.GRPC) return null diff --git a/misk/src/main/kotlin/misk/web/exceptions/JsonDataExceptionMapper.kt b/misk/src/main/kotlin/misk/web/exceptions/JsonDataExceptionMapper.kt index 3b7b81755c2..96aafb3c50a 100644 --- a/misk/src/main/kotlin/misk/web/exceptions/JsonDataExceptionMapper.kt +++ b/misk/src/main/kotlin/misk/web/exceptions/JsonDataExceptionMapper.kt @@ -2,18 +2,18 @@ package misk.web.exceptions import com.squareup.moshi.JsonDataException import jakarta.inject.Inject +import java.net.HttpURLConnection import misk.web.Response import misk.web.mediatype.MediaTypes import misk.web.toResponseBody import okhttp3.Headers.Companion.headersOf import org.slf4j.event.Level -import java.net.HttpURLConnection /** * Maps [JsonDataException] to HTTP 400 Bad Request. * - * Moshi throws JsonDataException when the JSON structure is valid but the data doesn't match - * the expected schema (e.g., wrong types, missing required fields). + * Moshi throws JsonDataException when the JSON structure is valid but the data doesn't match the expected schema (e.g., + * wrong types, missing required fields). */ internal class JsonDataExceptionMapper @Inject internal constructor() : ExceptionMapper { override fun toResponse(th: JsonDataException) = BAD_REQUEST_RESPONSE @@ -21,10 +21,11 @@ internal class JsonDataExceptionMapper @Inject internal constructor() : Exceptio override fun loggingLevel(th: JsonDataException): Level = Level.INFO companion object { - val BAD_REQUEST_RESPONSE = Response( - "bad request".toResponseBody(), - headersOf("Content-Type", MediaTypes.TEXT_PLAIN_UTF8), - HttpURLConnection.HTTP_BAD_REQUEST - ) + val BAD_REQUEST_RESPONSE = + Response( + "bad request".toResponseBody(), + headersOf("Content-Type", MediaTypes.TEXT_PLAIN_UTF8), + HttpURLConnection.HTTP_BAD_REQUEST, + ) } } diff --git a/misk/src/main/kotlin/misk/web/extractors/ResponseBodyFeatureBinding.kt b/misk/src/main/kotlin/misk/web/extractors/ResponseBodyFeatureBinding.kt index d760f466ea2..3f5f7e57a2b 100644 --- a/misk/src/main/kotlin/misk/web/extractors/ResponseBodyFeatureBinding.kt +++ b/misk/src/main/kotlin/misk/web/extractors/ResponseBodyFeatureBinding.kt @@ -19,6 +19,7 @@ import misk.web.PathPattern import misk.web.ResponseSink import misk.web.ResponseSinkChannel import misk.web.actions.WebSocketListener +import misk.web.http.HttpVersion import misk.web.interceptors.ResponseBodyMarshallerFactory import misk.web.marshal.Marshaller import misk.web.mediatype.MediaTypes @@ -49,7 +50,10 @@ internal class ResponseBodyFeatureBinding( with(subject.httpCall) { setResponseHeader("Content-Type", MediaTypes.SERVER_EVENT_STREAM) setResponseHeader("Cache-Control", "no-cache") - setResponseHeader("Connection", "keep-alive") + // Keep-Alive is an HTTP/1.0 mechanism only + if (subject.httpCall.httpVersion == HttpVersion.HTTP_1_0) { + setResponseHeader("Connection", "keep-alive") + } setResponseHeader("X-Accel-Buffering", "no") } } diff --git a/misk/src/main/kotlin/misk/web/http/HttpVersion.kt b/misk/src/main/kotlin/misk/web/http/HttpVersion.kt new file mode 100644 index 00000000000..5259a0471cd --- /dev/null +++ b/misk/src/main/kotlin/misk/web/http/HttpVersion.kt @@ -0,0 +1,30 @@ +package misk.web.http + +enum class HttpVersion { + HTTP_0_9, + HTTP_1_0, + HTTP_1_1, + HTTP_2_0, + HTTP_3_0; + + companion object { + internal fun fromServletRequestProtocol(protocol: String) = + when (protocol) { + "HTTP/0.9" -> HTTP_0_9 + "HTTP/1.0" -> HTTP_1_0 + "HTTP/1.1" -> HTTP_1_1 + "HTTP/2.0" -> HTTP_2_0 + "HTTP/3.0" -> HTTP_3_0 + else -> error("Unrecognized protocol: $protocol") + } + + internal fun fromJetty(version: org.eclipse.jetty.http.HttpVersion): HttpVersion = + when (version) { + org.eclipse.jetty.http.HttpVersion.HTTP_0_9 -> HTTP_0_9 + org.eclipse.jetty.http.HttpVersion.HTTP_1_0 -> HTTP_1_0 + org.eclipse.jetty.http.HttpVersion.HTTP_1_1 -> HTTP_1_1 + org.eclipse.jetty.http.HttpVersion.HTTP_2 -> HTTP_2_0 + org.eclipse.jetty.http.HttpVersion.HTTP_3 -> HTTP_3_0 + } + } +} diff --git a/misk/src/main/kotlin/misk/web/interceptors/TracingInterceptor.kt b/misk/src/main/kotlin/misk/web/interceptors/TracingInterceptor.kt index 5458274d7ea..6e2c0859f32 100644 --- a/misk/src/main/kotlin/misk/web/interceptors/TracingInterceptor.kt +++ b/misk/src/main/kotlin/misk/web/interceptors/TracingInterceptor.kt @@ -18,10 +18,8 @@ import misk.web.WebConfig private val logger = getLogger() /** Enables distributed tracing on all web actions, if a client has installed a tracer. */ -internal class TracingInterceptor internal constructor( - private val tracer: Tracer, - private val setSpanKindTag: Boolean, -) : NetworkInterceptor { +internal class TracingInterceptor +internal constructor(private val tracer: Tracer, private val setSpanKindTag: Boolean) : NetworkInterceptor { @Singleton class Factory @Inject constructor() : NetworkInterceptor.Factory { @Inject(optional = true) var tracer: Tracer? = null @@ -30,12 +28,7 @@ internal class TracingInterceptor internal constructor( // NOTE(nb): returning null ensures interceptor is filtered out when generating interceptors to // apply for a specific action. See WebActionModule for implementation details override fun create(action: Action) = - tracer?.let { - TracingInterceptor( - tracer = it, - setSpanKindTag = webConfig?.tracing_set_span_kind ?: true, - ) - } + tracer?.let { TracingInterceptor(tracer = it, setSpanKindTag = webConfig?.tracing_set_span_kind ?: true) } } override fun intercept(chain: NetworkChain) { diff --git a/misk/src/main/kotlin/misk/web/jetty/GenericServletUpstreamResponse.kt b/misk/src/main/kotlin/misk/web/jetty/GenericServletUpstreamResponse.kt index 74abaa527c6..9829f7ed7c5 100644 --- a/misk/src/main/kotlin/misk/web/jetty/GenericServletUpstreamResponse.kt +++ b/misk/src/main/kotlin/misk/web/jetty/GenericServletUpstreamResponse.kt @@ -3,6 +3,7 @@ package misk.web.jetty import jakarta.servlet.http.HttpServletResponse import misk.web.ServletHttpCall import misk.web.actions.WebSocketListener +import misk.web.http.HttpVersion import okhttp3.Headers import okhttp3.Headers.Companion.headersOf @@ -10,7 +11,7 @@ import okhttp3.Headers.Companion.headersOf * A generic implementation of ServletHttpCall.UpstreamResponse that works with standard HttpServletResponse instead of * requiring Jetty's specific Response class. */ -internal class GenericServletUpstreamResponse(private val response: HttpServletResponse) : +internal class GenericServletUpstreamResponse(private val protocol: String, private val response: HttpServletResponse) : ServletHttpCall.UpstreamResponse { private var sendTrailers = false private var trailers = headersOf() @@ -24,6 +25,9 @@ internal class GenericServletUpstreamResponse(private val response: HttpServletR override val headers: Headers get() = response.headers() + override val httpVersion: HttpVersion + get() = HttpVersion.fromServletRequestProtocol(protocol) + override fun setHeader(name: String, value: String) { response.setHeader(name, value) } diff --git a/misk/src/main/kotlin/misk/web/jetty/JettyHealthService.kt b/misk/src/main/kotlin/misk/web/jetty/JettyHealthService.kt index c09f8f73dc4..8e2f18549ad 100644 --- a/misk/src/main/kotlin/misk/web/jetty/JettyHealthService.kt +++ b/misk/src/main/kotlin/misk/web/jetty/JettyHealthService.kt @@ -14,18 +14,19 @@ import misk.logging.getLogger import misk.web.WebConfig import mu.KLogger import okhttp3.HttpUrl +import org.eclipse.jetty.ee9.servlet.ServletContextHandler +import org.eclipse.jetty.ee9.servlet.ServletHolder +import org.eclipse.jetty.ee9.websocket.server.config.JettyWebSocketServletContainerInitializer import org.eclipse.jetty.http.UriCompliance import org.eclipse.jetty.io.ConnectionStatistics +import org.eclipse.jetty.server.Handler import org.eclipse.jetty.server.HttpConfiguration import org.eclipse.jetty.server.HttpConnectionFactory import org.eclipse.jetty.server.NetworkConnector import org.eclipse.jetty.server.Server import org.eclipse.jetty.server.ServerConnector import org.eclipse.jetty.server.handler.StatisticsHandler -import org.eclipse.jetty.servlet.ServletContextHandler -import org.eclipse.jetty.servlet.ServletHolder import org.eclipse.jetty.util.thread.ExecutorThreadPool -import org.eclipse.jetty.websocket.server.config.JettyWebSocketServletContainerInitializer /** * The JettyHealthService is a standalone Jetty Instance for managing health checks in Misk. It is unique in that it @@ -93,7 +94,7 @@ internal constructor( val httpConnectionFactory = HttpConnectionFactory( HttpConfiguration().apply { - uriCompliance = UriCompliance.RFC3986 + uriCompliance = UriCompliance.LEGACY sendServerVersion = false setFormEncodedMethods() } @@ -131,7 +132,8 @@ internal constructor( JettyWebSocketServletContainerInitializer.configure(servletContextHandler, null) server.addManaged(servletContextHandler) - statisticsHandler.handler = servletContextHandler + val handlers = Handler.Sequence().apply { addHandler(servletContextHandler) } + statisticsHandler.handler = handlers } private fun setupServer() { diff --git a/misk/src/main/kotlin/misk/web/jetty/JettyService.kt b/misk/src/main/kotlin/misk/web/jetty/JettyService.kt index 9baab41b605..68ab477e42a 100644 --- a/misk/src/main/kotlin/misk/web/jetty/JettyService.kt +++ b/misk/src/main/kotlin/misk/web/jetty/JettyService.kt @@ -31,6 +31,11 @@ import misk.web.jetty.JettyHealthService.Companion.jettyHealthServiceEnabled import misk.web.mediatype.MediaTypes import okhttp3.HttpUrl import org.eclipse.jetty.alpn.server.ALPNServerConnectionFactory +import org.eclipse.jetty.ee9.servlet.FilterHolder +import org.eclipse.jetty.ee9.servlet.ServletContextHandler +import org.eclipse.jetty.ee9.servlet.ServletHolder +import org.eclipse.jetty.ee9.servlets.CrossOriginFilter +import org.eclipse.jetty.ee9.websocket.server.config.JettyWebSocketServletContainerInitializer import org.eclipse.jetty.http.UriCompliance import org.eclipse.jetty.http2.server.AbstractHTTP2ServerConnectionFactory import org.eclipse.jetty.http2.server.HTTP2CServerConnectionFactory @@ -38,6 +43,7 @@ import org.eclipse.jetty.http2.server.HTTP2ServerConnectionFactory import org.eclipse.jetty.io.ConnectionStatistics import org.eclipse.jetty.server.ConnectionFactory import org.eclipse.jetty.server.Connector +import org.eclipse.jetty.server.Handler import org.eclipse.jetty.server.HttpConfiguration import org.eclipse.jetty.server.HttpConnectionFactory import org.eclipse.jetty.server.NetworkConnector @@ -48,17 +54,11 @@ import org.eclipse.jetty.server.SslConnectionFactory import org.eclipse.jetty.server.handler.ContextHandler import org.eclipse.jetty.server.handler.StatisticsHandler import org.eclipse.jetty.server.handler.gzip.GzipHandler -import org.eclipse.jetty.servlet.FilterHolder -import org.eclipse.jetty.servlet.ServletContextHandler -import org.eclipse.jetty.servlet.ServletHolder -import org.eclipse.jetty.servlets.CrossOriginFilter import org.eclipse.jetty.unixdomain.server.UnixDomainServerConnector -import org.eclipse.jetty.unixsocket.server.UnixSocketConnector +import org.eclipse.jetty.util.HostPort import org.eclipse.jetty.util.JavaVersion -import org.eclipse.jetty.util.MultiException import org.eclipse.jetty.util.ssl.SslContextFactory import org.eclipse.jetty.util.thread.ThreadPool -import org.eclipse.jetty.websocket.server.config.JettyWebSocketServletContainerInitializer @Singleton class JettyService @@ -107,7 +107,7 @@ internal constructor( server, healthExecutor, null, /* scheduler */ - null /* buffer pool */, + null, /* buffer pool */ 1, 1, HttpConnectionFactory(), @@ -120,7 +120,7 @@ internal constructor( val httpConnectionFactories = mutableListOf() val httpConfig = HttpConfiguration() httpConfig.customizeForGrpc() - httpConfig.uriCompliance = UriCompliance.RFC3986 + httpConfig.uriCompliance = UriCompliance.LEGACY httpConfig.sendServerVersion = false if (webConfig.ssl != null) { httpConfig.securePort = webConfig.ssl.port @@ -151,9 +151,9 @@ internal constructor( val httpConnector = ServerConnector( server, - null /* executor */, - null /* scheduler */, - null /* buffer pool */, + null, /* executor */ + null, /* scheduler */ + null, /* buffer pool */ webConfig.acceptors ?: -1, webConfig.selectors ?: -1, *httpConnectionFactories.toTypedArray(), @@ -232,9 +232,9 @@ internal constructor( val httpsConnector = ServerConnector( server, - null /* executor */, - null /* scheduler */, - null /* buffer pool */, + null, /* executor */ + null, /* scheduler */ + null, /* buffer pool */ webConfig.acceptors ?: -1, webConfig.selectors ?: -1, *httpsConnectionFactories.toTypedArray(), @@ -259,59 +259,52 @@ internal constructor( socketConfigs.addAll(webConfig.unix_domain_sockets) } socketConfigs.stream().forEach() { socketConfig -> + // Provide a fallback server authority for the Unix-domain connector. Jetty 12 derives the + // server authority from the connection's local address when a request carries no Host/ + // :authority (e.g. the bare "PRI * HTTP/2.0" HTTP/2 prior-knowledge preface). For a Unix + // domain socket that local address is the socket file path, which HostPort rejects as an + // invalid authority ("Bad Authority"). Requests that do carry a Host/:authority still use + // their own value; this is only a fallback. + val udsHttpConfig = HttpConfiguration(httpConfig) + udsHttpConfig.serverAuthority = HostPort("localhost") + val udsConnFactories = mutableListOf() - udsConnFactories.add(HttpConnectionFactory(httpConfig)) + udsConnFactories.add(HttpConnectionFactory(udsHttpConfig)) if (socketConfig.h2c == true) { - val http2 = HTTP2CServerConnectionFactory(httpConfig) + val http2 = HTTP2CServerConnectionFactory(udsHttpConfig) http2.rateControlFactory = http2RateControlFactory udsConnFactories.add(http2) } - if (isJEP380Supported(socketConfig.path)) { - logger.info("Using UnixDomainServerConnector for ${socketConfig.path}") - val udsConnector = - UnixDomainServerConnector( - server, - null /* executor */, - null /* scheduler */, - null /* buffer pool */, - webConfig.acceptors ?: -1, - webConfig.selectors ?: -1, - *udsConnFactories.toTypedArray(), - ) - val socketFile = File(socketConfig.path) - udsConnector.unixDomainPath = socketFile.toPath() - udsConnector.addBean(connectionMetricsCollector.newConnectionListener("http", 0)) - udsConnector.name = "uds" - - // try to clean up any leftover socket files before connecting - if (socketFile.exists() && !socketFile.delete()) { - logger.warn("Could not delete file $socketFile") - } + logger.info("Using UnixDomainServerConnector for ${socketConfig.path}") + val udsConnector = + UnixDomainServerConnector( + server, + null, /* executor */ + null, /* scheduler */ + null, /* buffer pool */ + webConfig.acceptors ?: -1, + webConfig.selectors ?: -1, + *udsConnFactories.toTypedArray(), + ) + val socketFile = File(socketConfig.path) + udsConnector.unixDomainPath = socketFile.toPath() + udsConnector.addBean(connectionMetricsCollector.newConnectionListener("http", 0)) + udsConnector.name = "uds" + + // try to clean up any leftover socket files before connecting + if (socketFile.exists() && !socketFile.delete()) { + logger.warn("Could not delete file $socketFile") + } - // set file permissions after socket creation so sidecars (e.g. envoy, istio) have access - try { - udsConnector.start() - setFilePermissions(socketFile) - } catch (e: Exception) { - cleanAndThrow(udsConnector, e) - } - server.addConnector(udsConnector) - } else { - val udsConnector = - UnixSocketConnector( - server, - null /* executor */, - null /* scheduler */, - null /* buffer pool */, - webConfig.selectors ?: -1, - *udsConnFactories.toTypedArray(), - ) - udsConnector.setUnixSocket(socketConfig.path) - udsConnector.addBean(connectionMetricsCollector.newConnectionListener("http", 0)) - udsConnector.name = "uds" - server.addConnector(udsConnector) + // set file permissions after socket creation so sidecars (e.g. envoy, istio) have access + try { + udsConnector.start() + setFilePermissions(socketFile) + } catch (e: Exception) { + cleanAndThrow(udsConnector, e) } + server.addConnector(udsConnector) } // TODO(mmihic): Force security handler? @@ -322,7 +315,8 @@ internal constructor( JettyWebSocketServletContainerInitializer.configure(servletContextHandler, null) server.addManaged(servletContextHandler) - statisticsHandler.handler = servletContextHandler + val handlers = Handler.Sequence().apply { addHandler(servletContextHandler) } + statisticsHandler.handler = handlers statisticsHandler.server = server // Kubernetes sends a SIG_TERM and gives us 30 seconds to stop gracefully. @@ -384,10 +378,6 @@ internal constructor( // distinguished from a regular unix socket by the fact that the first byte of // the address is a null byte ('\0'). The address has no connection with filesystem // path names. - } catch (e: MultiException) { - // Jetty wraps multiple InvalidPathExceptions into a MultiException when stopping - // multiple abstract unix domain sockets (addresses starting with '\0'). - if (!isOnlyInvalidPathExceptions(e)) throw e } logger.info { "Stopped Jetty in $stopwatch" } @@ -429,25 +419,25 @@ internal constructor( private val Server.healthUrl: HttpUrl? get() { - return connectors.mapNotNull { it as? NetworkConnector }.firstOrNull { it.name == "health" }?.toHttpUrl() + return connectors.filterIsInstance().firstOrNull { it.name == "health" }?.toHttpUrl() } private val Server.httpUrl: HttpUrl? get() { - return connectors.mapNotNull { it as? NetworkConnector }.firstOrNull { it.name == "http" }?.toHttpUrl() + return connectors.filterIsInstance().firstOrNull { it.name == "http" }?.toHttpUrl() } private val Server.httpsUrl: HttpUrl? get() { - return connectors.mapNotNull { it as? NetworkConnector }.firstOrNull { it.name == "https" }?.toHttpUrl() + return connectors.filterIsInstance().firstOrNull { it.name == "https" }?.toHttpUrl() } internal fun NetworkConnector.toHttpUrl(): HttpUrl { - val context = server.getChildHandlerByClass(ContextHandler::class.java) + val context = server.getDescendant(ContextHandler::class.java) val protocol = defaultConnectionFactory.protocol val scheme = if (protocol.startsWith("SSL-") || protocol == "SSL") "https" else "http" - val virtualHosts = context?.virtualHosts ?: arrayOf() + val virtualHosts = context?.virtualHosts ?: emptyList() val explicitHost = if (virtualHosts.isEmpty()) host else virtualHosts[0] return HttpUrl.Builder() @@ -477,10 +467,6 @@ private fun AbstractHTTP2ServerConnectionFactory.customize(webConfig: WebConfig) } } -private fun isOnlyInvalidPathExceptions(e: MultiException): Boolean { - return e.throwables.isNotEmpty() && e.throwables.all { it is InvalidPathException } -} - /** * JEP-380 is supported when running Java 16+ and the provided socket path is non-abstract. Abstract socket paths are * identified by paths prefixed with an `@` symbol or a null byte. @@ -499,7 +485,7 @@ private fun setFilePermissions(file: File) { private fun cleanAndThrow(connector: Connector, exception: Exception) { val runtimeException = RuntimeException(exception) - if (connector.isStarted()) { + if (connector.isStarted) { try { connector.stop() } catch (e: Exception) { diff --git a/misk/src/main/kotlin/misk/web/jetty/JettyServletUpstreamResponse.kt b/misk/src/main/kotlin/misk/web/jetty/JettyServletUpstreamResponse.kt index 69d8b654b15..175753b9b04 100644 --- a/misk/src/main/kotlin/misk/web/jetty/JettyServletUpstreamResponse.kt +++ b/misk/src/main/kotlin/misk/web/jetty/JettyServletUpstreamResponse.kt @@ -3,10 +3,11 @@ package misk.web.jetty import java.util.function.Supplier import misk.web.ServletHttpCall import misk.web.actions.WebSocketListener +import misk.web.http.HttpVersion import okhttp3.Headers import okhttp3.Headers.Companion.headersOf +import org.eclipse.jetty.ee9.nested.Response import org.eclipse.jetty.http.HttpFields -import org.eclipse.jetty.server.Response internal class JettyServletUpstreamResponse(val response: Response) : ServletHttpCall.UpstreamResponse { var sendTrailers = false @@ -21,6 +22,9 @@ internal class JettyServletUpstreamResponse(val response: Response) : ServletHtt override val headers: Headers get() = response.headers() + override val httpVersion: HttpVersion + get() = HttpVersion.fromJetty(response.httpChannel.request.httpVersion) + override fun setHeader(name: String, value: String) { response.setHeader(name, value) } @@ -35,14 +39,13 @@ internal class JettyServletUpstreamResponse(val response: Response) : ServletHtt sendTrailers = true // Set the callback that'll return trailers at the end of the response body. - response.trailers = - Supplier { - val httpFields = HttpFields.build() - for (i in 0 until trailers.size) { - httpFields.add(trailers.name(i), trailers.value(i)) - } - httpFields + response.trailers = Supplier { + val httpFields = HttpFields.build() + for (i in 0 until trailers.size) { + httpFields.add(trailers.name(i), trailers.value(i)) } + httpFields + } } override fun setTrailer(name: String, value: String) { diff --git a/misk/src/main/kotlin/misk/web/jetty/JettyWebSocket.kt b/misk/src/main/kotlin/misk/web/jetty/JettyWebSocket.kt index 2f571fb3f75..29e889e1496 100644 --- a/misk/src/main/kotlin/misk/web/jetty/JettyWebSocket.kt +++ b/misk/src/main/kotlin/misk/web/jetty/JettyWebSocket.kt @@ -7,16 +7,17 @@ import misk.web.ServletHttpCall import misk.web.actions.WebAction import misk.web.actions.WebSocket import misk.web.actions.WebSocketListener +import misk.web.http.HttpVersion import okhttp3.Headers import okio.ByteString import okio.ByteString.Companion.toByteString import okio.utf8Size -import org.eclipse.jetty.websocket.api.Session -import org.eclipse.jetty.websocket.api.WebSocketAdapter -import org.eclipse.jetty.websocket.api.WriteCallback -import org.eclipse.jetty.websocket.server.JettyServerUpgradeRequest -import org.eclipse.jetty.websocket.server.JettyServerUpgradeResponse -import org.eclipse.jetty.websocket.server.JettyWebSocketCreator +import org.eclipse.jetty.ee9.websocket.api.Session +import org.eclipse.jetty.ee9.websocket.api.WebSocketAdapter +import org.eclipse.jetty.ee9.websocket.api.WriteCallback +import org.eclipse.jetty.ee9.websocket.server.JettyServerUpgradeRequest +import org.eclipse.jetty.ee9.websocket.server.JettyServerUpgradeResponse +import org.eclipse.jetty.ee9.websocket.server.JettyWebSocketCreator private const val MAX_QUEUE_SIZE = 16 * 1024 * 1024 @@ -34,7 +35,7 @@ internal class JettyWebSocket(val request: JettyServerUpgradeRequest, val respon private val adapter = object : WebSocketAdapter() { - override fun onWebSocketConnect(sess: Session?) { + override fun onWebSocketConnect(sess: Session) { super.onWebSocketConnect(sess) sendQueue() } @@ -65,6 +66,9 @@ internal class JettyWebSocket(val request: JettyServerUpgradeRequest, val respon response.statusCode = value } + override val httpVersion: HttpVersion + get() = error("No http version for websocket responses") + override val headers: Headers get() = response.headers() diff --git a/misk/src/main/kotlin/misk/web/jetty/MeasuredWindowRateControl.kt b/misk/src/main/kotlin/misk/web/jetty/MeasuredWindowRateControl.kt index 9eddcc87122..91c6b9929e8 100644 --- a/misk/src/main/kotlin/misk/web/jetty/MeasuredWindowRateControl.kt +++ b/misk/src/main/kotlin/misk/web/jetty/MeasuredWindowRateControl.kt @@ -21,13 +21,13 @@ import java.util.concurrent.atomic.AtomicInteger import misk.metrics.v2.Metrics import misk.metrics.v2.PeakGauge import misk.web.WebConfig -import org.eclipse.jetty.http2.parser.RateControl +import org.eclipse.jetty.http2.RateControl import org.eclipse.jetty.io.EndPoint import org.eclipse.jetty.util.NanoTime /** * Misk's RateControl implementation with observability for monitoring HTTP/2 frame rate limiting. Almost the same - * implementation as [org.eclipse.jetty.http2.parser.WindowRateControl]. + * implementation as [org.eclipse.jetty.http2.WindowRateControl]. */ class MeasuredWindowRateControl private constructor( diff --git a/misk/src/main/kotlin/misk/web/jetty/WebActionsServlet.kt b/misk/src/main/kotlin/misk/web/jetty/WebActionsServlet.kt index 19dd97a4f02..9ac878899e1 100644 --- a/misk/src/main/kotlin/misk/web/jetty/WebActionsServlet.kt +++ b/misk/src/main/kotlin/misk/web/jetty/WebActionsServlet.kt @@ -25,16 +25,15 @@ import okio.BufferedSink import okio.buffer import okio.sink import okio.source +import org.eclipse.jetty.ee9.nested.Request +import org.eclipse.jetty.ee9.nested.Response +import org.eclipse.jetty.ee9.websocket.server.JettyServerUpgradeResponse +import org.eclipse.jetty.ee9.websocket.server.JettyWebSocketServlet +import org.eclipse.jetty.ee9.websocket.server.JettyWebSocketServletFactory import org.eclipse.jetty.http.BadMessageException import org.eclipse.jetty.http.HttpMethod -import org.eclipse.jetty.server.Request -import org.eclipse.jetty.server.Response import org.eclipse.jetty.server.ServerConnector import org.eclipse.jetty.unixdomain.server.UnixDomainServerConnector -import org.eclipse.jetty.unixsocket.server.UnixSocketConnector -import org.eclipse.jetty.websocket.server.JettyServerUpgradeResponse -import org.eclipse.jetty.websocket.server.JettyWebSocketServlet -import org.eclipse.jetty.websocket.server.JettyWebSocketServletFactory @Singleton internal class WebActionsServlet @@ -141,7 +140,7 @@ constructor( if (response is Response) { JettyServletUpstreamResponse(response) } else { - GenericServletUpstreamResponse(response) + GenericServletUpstreamResponse(request.protocol, response) }, requestBody = request.inputStream.source().buffer(), responseBody = responseBody, @@ -261,6 +260,16 @@ internal fun HttpServletResponse.headers(): Headers { return result.build() } +internal fun Response.headers(): Headers { + val result = Headers.Builder() + for (name in headerNames) { + for (value in getHeaders(name)) { + result.addUnsafeNonAscii(name, value) + } + } + return result.build() +} + internal fun HttpServletRequest.httpUrl(): HttpUrl { val rUrl = requestURL.replaceFirst(Regex("^ws://"), "http://") return if (queryString == null) { @@ -298,10 +307,7 @@ private fun extractLinkLayerLocalAddress(request: HttpServletRequest): SocketAdd return when (connector) { is UnixDomainServerConnector -> SocketAddress.Unix(connector.unixDomainPath.toString()) - is UnixSocketConnector -> SocketAddress.Unix(connector.unixSocket) - - is ServerConnector -> - SocketAddress.Network(httpChannel.endPoint.remoteAddress.address.hostAddress, connector.localPort) + is ServerConnector -> SocketAddress.Network(httpChannel.remoteAddress.address.hostAddress, connector.localPort) else -> throw IllegalStateException("Unknown socket connector.") } diff --git a/misk/src/main/kotlin/misk/web/marshal/Marshaller.kt b/misk/src/main/kotlin/misk/web/marshal/Marshaller.kt index ee3b89547ee..54ce15416b3 100644 --- a/misk/src/main/kotlin/misk/web/marshal/Marshaller.kt +++ b/misk/src/main/kotlin/misk/web/marshal/Marshaller.kt @@ -39,12 +39,13 @@ interface Marshaller { companion object { fun actualResponseType(type: KType): Type { val typeLiteral = type.typeLiteral() - val javaType = when { - typeLiteral.rawType == Response::class.java -> { - (typeLiteral.type as ParameterizedType).actualTypeArguments[0] + val javaType = + when { + typeLiteral.rawType == Response::class.java -> { + (typeLiteral.type as ParameterizedType).actualTypeArguments[0] + } + else -> typeLiteral.type } - else -> typeLiteral.type - } // Unwrap wildcard types produced by Kotlin's declaration-site variance. // Response can produce "? extends T" instead of "T" for suspend function return types, // because KType.javaType reconstructs the type from Continuation metadata rather than from diff --git a/misk/src/test/kotlin/misk/client/PropagatingScopeActionInInterceptorsTest.kt b/misk/src/test/kotlin/misk/client/PropagatingScopeActionInInterceptorsTest.kt index 6c29da3f9ed..954c204b373 100644 --- a/misk/src/test/kotlin/misk/client/PropagatingScopeActionInInterceptorsTest.kt +++ b/misk/src/test/kotlin/misk/client/PropagatingScopeActionInInterceptorsTest.kt @@ -64,37 +64,39 @@ class PropagatingScopeActionInInterceptorsTest { @Test fun `propagate action scoped using typed http client`() { - val response = scope.create(seedData).inScope { - client.getDinosaur(Dinosaur.Builder().name("trex").build()).execute() - } + val response = + scope.create(seedData).inScope { client.getDinosaur(Dinosaur.Builder().name("trex").build()).execute() } assertThat(response.body()!!.name).isEqualTo("es-US") } @Test fun `propagate action scoped using typed http client with suspended calls`() { - val response = scope.create(seedData).inScope { - runBlocking(Dispatchers.IO + scope.asContextElement()) { - client.getDinosaur(Dinosaur.Builder().name("trex").build()).execute() + val response = + scope.create(seedData).inScope { + runBlocking(Dispatchers.IO + scope.asContextElement()) { + client.getDinosaur(Dinosaur.Builder().name("trex").build()).execute() + } } - } assertThat(response.body()!!.name).isEqualTo("es-US") } @Test fun `propagate action scoped using gRPC client`() { - val response = scope.create(seedData).inScope { - dinoService.GetDinosour().executeBlocking(Dinosaur.Builder().name("trex").build()) - } + val response = + scope.create(seedData).inScope { + dinoService.GetDinosour().executeBlocking(Dinosaur.Builder().name("trex").build()) + } assertThat(response.name).isEqualTo("es-US") } @Test fun `propagate action scoped using gRPC client with suspended calls`() { - val response = scope.create(seedData).inScope { - runBlocking(Dispatchers.IO + scope.asContextElement()) { - dinoService.GetDinosour().execute(Dinosaur.Builder().name("trex").build()) + val response = + scope.create(seedData).inScope { + runBlocking(Dispatchers.IO + scope.asContextElement()) { + dinoService.GetDinosour().execute(Dinosaur.Builder().name("trex").build()) + } } - } assertThat(response.name).isEqualTo("es-US") } diff --git a/misk/src/test/kotlin/misk/client/TypedPeerHttpClientTest.kt b/misk/src/test/kotlin/misk/client/TypedPeerHttpClientTest.kt index 6312560220f..5994510b47c 100644 --- a/misk/src/test/kotlin/misk/client/TypedPeerHttpClientTest.kt +++ b/misk/src/test/kotlin/misk/client/TypedPeerHttpClientTest.kt @@ -52,9 +52,10 @@ internal class TypedPeerHttpClientTest { val clientFactory = clientInjector.getInstance(Key.get(object : TypeLiteral>() {})) - val clusterMember = object : PeerIdentifier { - override val ipAddress = "127.0.0.1" - } + val clusterMember = + object : PeerIdentifier { + override val ipAddress = "127.0.0.1" + } val client: ReturnADinosaur = clientFactory.client(clusterMember) val response = client.getDinosaur(Dinosaur.Builder().name("trex").build()).execute() diff --git a/misk/src/test/kotlin/misk/web/EnableUnframedRequestsTest.kt b/misk/src/test/kotlin/misk/web/EnableUnframedRequestsTest.kt index cd3b2bb3d98..4555953f6cc 100644 --- a/misk/src/test/kotlin/misk/web/EnableUnframedRequestsTest.kt +++ b/misk/src/test/kotlin/misk/web/EnableUnframedRequestsTest.kt @@ -32,19 +32,15 @@ import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test /** - * Test that @[EnableUnframedRequests] enables protobuf POST on gRPC endpoints, - * for both @[WireRpc] and @[Grpc] actions. + * Test that @[EnableUnframedRequests] enables protobuf POST on gRPC endpoints, for both @[WireRpc] and @[Grpc] actions. */ @MiskTest(startService = true) internal class EnableUnframedRequestsTest { - @MiskTestModule - val module = TestModule() + @MiskTestModule val module = TestModule() - @Inject - lateinit var moshi: Moshi + @Inject lateinit var moshi: Moshi - @Inject - lateinit var jettyService: JettyService + @Inject lateinit var jettyService: JettyService private lateinit var httpClient: OkHttpClient @@ -56,72 +52,52 @@ internal class EnableUnframedRequestsTest { @Test fun `protobuf POST to unframed grpc endpoint`() { - val requestBody = Shipment.Builder() - .shipment_token("abc") - .build() - val expectedResponseBody = Warehouse.Builder() - .warehouse_token("abc") - .build() - - val request = Request.Builder() - .post( - requestBody.encode().toRequestBody( - MediaTypes.APPLICATION_PROTOBUF_MEDIA_TYPE - ) - ) - .url(serverUrlBuilder().encodedPath("/test/UnframedGetDestinationWarehouse").build()) - .build() + val requestBody = Shipment.Builder().shipment_token("abc").build() + val expectedResponseBody = Warehouse.Builder().warehouse_token("abc").build() + + val request = + Request.Builder() + .post(requestBody.encode().toRequestBody(MediaTypes.APPLICATION_PROTOBUF_MEDIA_TYPE)) + .url(serverUrlBuilder().encodedPath("/test/UnframedGetDestinationWarehouse").build()) + .build() val response = httpClient.newCall(request).execute() response.use { assertThat(response.code).isEqualTo(200) val responseBody = Warehouse.ADAPTER.decode(response.body!!.source()) assertThat(responseBody).isEqualTo(expectedResponseBody) - assertThat(response.body!!.contentType()) - .isEqualTo(MediaTypes.APPLICATION_PROTOBUF_MEDIA_TYPE) + assertThat(response.body!!.contentType()).isEqualTo(MediaTypes.APPLICATION_PROTOBUF_MEDIA_TYPE) } } @Test fun `json POST to unframed grpc endpoint`() { - val requestBody = Shipment.Builder() - .shipment_token("abc") - .build() - val expectedResponseBody = Warehouse.Builder() - .warehouse_token("abc") - .build() - - val request = Request.Builder() - .post( - moshi.adapter(Shipment::class.java).toJson(requestBody) - .toRequestBody(MediaTypes.APPLICATION_JSON_MEDIA_TYPE) - ) - .url(serverUrlBuilder().encodedPath("/test/UnframedGetDestinationWarehouse").build()) - .build() + val requestBody = Shipment.Builder().shipment_token("abc").build() + val expectedResponseBody = Warehouse.Builder().warehouse_token("abc").build() + + val request = + Request.Builder() + .post( + moshi.adapter(Shipment::class.java).toJson(requestBody).toRequestBody(MediaTypes.APPLICATION_JSON_MEDIA_TYPE) + ) + .url(serverUrlBuilder().encodedPath("/test/UnframedGetDestinationWarehouse").build()) + .build() val response = httpClient.newCall(request).execute() response.use { assertThat(response.code).isEqualTo(200) val responseBody = moshi.adapter(Warehouse::class.java).fromJson(response.body!!.source()) assertThat(responseBody).isEqualTo(expectedResponseBody) - assertThat(response.body!!.contentType().toString()) - .isEqualTo("application/json;charset=utf-8") + assertThat(response.body!!.contentType().toString()).isEqualTo("application/json;charset=utf-8") } } @Test fun `grpc to unframed grpc endpoint`() { - val requestBody = Shipment.Builder() - .shipment_token("abc") - .build() - val expectedResponseBody = Warehouse.Builder() - .warehouse_token("abc") - .build() - - val grpcClient = GrpcClient.Builder() - .baseUrl(jettyService.httpsServerUrl!!) - .client(httpClient) - .build() + val requestBody = Shipment.Builder().shipment_token("abc").build() + val expectedResponseBody = Warehouse.Builder().warehouse_token("abc").build() + + val grpcClient = GrpcClient.Builder().baseUrl(jettyService.httpsServerUrl!!).client(httpClient).build() val shippingClient = UnframedShippingClient(grpcClient) val responseBody = shippingClient.GetDestinationWarehouse().executeBlocking(requestBody) @@ -132,12 +108,11 @@ internal class EnableUnframedRequestsTest { fun `protobuf POST to unframed @Grpc endpoint`() { val payload = "hello".toByteArray().toByteString() - val request = Request.Builder() - .post( - payload.toRequestBody(MediaTypes.APPLICATION_PROTOBUF_MEDIA_TYPE) - ) - .url(serverUrlBuilder().encodedPath("/test/GrpcAnnotationEcho").build()) - .build() + val request = + Request.Builder() + .post(payload.toRequestBody(MediaTypes.APPLICATION_PROTOBUF_MEDIA_TYPE)) + .url(serverUrlBuilder().encodedPath("/test/GrpcAnnotationEcho").build()) + .build() val response = httpClient.newCall(request).execute() response.use { @@ -149,26 +124,19 @@ internal class EnableUnframedRequestsTest { @Test fun `json POST to unframed @Grpc endpoint`() { - val request = Request.Builder() - .post( - "\"aGVsbG8=\"".toRequestBody(MediaTypes.APPLICATION_JSON_MEDIA_TYPE) - ) - .url(serverUrlBuilder().encodedPath("/test/GrpcAnnotationEcho").build()) - .build() + val request = + Request.Builder() + .post("\"aGVsbG8=\"".toRequestBody(MediaTypes.APPLICATION_JSON_MEDIA_TYPE)) + .url(serverUrlBuilder().encodedPath("/test/GrpcAnnotationEcho").build()) + .build() val response = httpClient.newCall(request).execute() - response.use { - assertThat(response.code).isEqualTo(200) - } + response.use { assertThat(response.code).isEqualTo(200) } } class TestModule : KAbstractModule() { override fun configure() { - install( - WebServerTestingModule( - webConfig = WebServerTestingModule.TESTING_WEB_CONFIG - ) - ) + install(WebServerTestingModule(webConfig = WebServerTestingModule.TESTING_WEB_CONFIG)) install(MiskTestingServiceModule()) install(WebActionModule.create()) install(WebActionModule.create()) @@ -193,13 +161,10 @@ internal class EnableUnframedRequestsTest { } @Suppress("TestFunctionName") - class UnframedGrpcAction @Inject constructor() : - UnframedShippingServer, WebAction { + class UnframedGrpcAction @Inject constructor() : UnframedShippingServer, WebAction { @Unauthenticated override fun GetDestinationWarehouse(shipment: Shipment): Warehouse { - return Warehouse.Builder() - .warehouse_token(shipment.shipment_token) - .build() + return Warehouse.Builder().warehouse_token(shipment.shipment_token).build() } } @@ -208,7 +173,7 @@ internal class EnableUnframedRequestsTest { @WireRpc( path = "/test/UnframedGetDestinationWarehouse", requestAdapter = "com.squareup.protos.test.parsing.Shipment#ADAPTER", - responseAdapter = "com.squareup.protos.test.parsing.Warehouse#ADAPTER" + responseAdapter = "com.squareup.protos.test.parsing.Warehouse#ADAPTER", ) @EnableUnframedRequests fun GetDestinationWarehouse(shipment: Shipment): Warehouse @@ -219,15 +184,16 @@ internal class EnableUnframedRequestsTest { @WireRpc( path = "/test/UnframedGetDestinationWarehouse", requestAdapter = "com.squareup.protos.test.parsing.Shipment#ADAPTER", - responseAdapter = "com.squareup.protos.test.parsing.Warehouse#ADAPTER" + responseAdapter = "com.squareup.protos.test.parsing.Warehouse#ADAPTER", ) - fun GetDestinationWarehouse(): GrpcCall = client.newCall( - GrpcMethod( - path = "/test/UnframedGetDestinationWarehouse", - requestAdapter = Shipment.ADAPTER, - responseAdapter = Warehouse.ADAPTER + fun GetDestinationWarehouse(): GrpcCall = + client.newCall( + GrpcMethod( + path = "/test/UnframedGetDestinationWarehouse", + requestAdapter = Shipment.ADAPTER, + responseAdapter = Warehouse.ADAPTER, + ) ) - ) } private fun serverUrlBuilder(): HttpUrl.Builder { diff --git a/misk/src/test/kotlin/misk/web/InvalidActionsTest.kt b/misk/src/test/kotlin/misk/web/InvalidActionsTest.kt index fb7d1708b89..7c332216530 100644 --- a/misk/src/test/kotlin/misk/web/InvalidActionsTest.kt +++ b/misk/src/test/kotlin/misk/web/InvalidActionsTest.kt @@ -94,12 +94,19 @@ class InvalidActionsTest { fun sayHello(request: HelloRequest): HelloReply } - @Test fun failEnableUnframedRequestsGrpcWithPostProtobufAction() { - val exception = assertThrows("Should throw an exception") { - Guice.createInjector(UnframedGrpcModule()).getInstance(ServiceManager::class.java) - .startAsync().awaitHealthy(Duration.ofSeconds(5)) - } - assertThat(exception.message).contains("Actions [InvalidActionsTest.UnframedHelloRpcAction, InvalidActionsTest.UnframedHelloRpcAction] have identical routing annotations.") + @Test + fun failEnableUnframedRequestsGrpcWithPostProtobufAction() { + val exception = + assertThrows("Should throw an exception") { + Guice.createInjector(UnframedGrpcModule()) + .getInstance(ServiceManager::class.java) + .startAsync() + .awaitHealthy(Duration.ofSeconds(5)) + } + assertThat(exception.message) + .contains( + "Actions [InvalidActionsTest.UnframedHelloRpcAction, InvalidActionsTest.UnframedHelloRpcAction] have identical routing annotations." + ) } class UnframedGrpcModule : KAbstractModule() { @@ -115,14 +122,12 @@ class InvalidActionsTest { class UnframedHelloRpcAction @Inject constructor() : GreeterSayHelloBlockingServer, WebAction { @EnableUnframedRequests - override fun SayHello(request: HelloRequest): HelloReply = HelloReply.Builder() - .message("howdy, ${request.name}") - .build() + override fun SayHello(request: HelloRequest): HelloReply = + HelloReply.Builder().message("howdy, ${request.name}").build() @Post("/helloworld.Greeter/SayHello") @RequestContentType(MediaTypes.APPLICATION_PROTOBUF) @ResponseContentType(MediaTypes.APPLICATION_PROTOBUF) fun sayHelloProtobufOverHttp(@RequestBody request: HelloRequest) = SayHello(request) } - } diff --git a/misk/src/test/kotlin/misk/web/actions/SseActionTest.kt b/misk/src/test/kotlin/misk/web/actions/SseActionTest.kt index b4a659e4666..0fc11aa4dda 100644 --- a/misk/src/test/kotlin/misk/web/actions/SseActionTest.kt +++ b/misk/src/test/kotlin/misk/web/actions/SseActionTest.kt @@ -227,7 +227,6 @@ class SseActionTest { assertThat(response.code).isEqualTo(200) assertThat(response.header("Content-Type")).isEqualTo("text/event-stream") assertThat(response.header("Cache-Control")).isEqualTo("no-cache") - assertThat(response.header("Connection")).isEqualTo("keep-alive") assertThat(response.header("X-Accel-Buffering")).isEqualTo("no") } diff --git a/misk/src/test/kotlin/misk/web/extractors/JsonRequestBodyExceptionTest.kt b/misk/src/test/kotlin/misk/web/extractors/JsonRequestBodyExceptionTest.kt index 2873f62e5ec..81ea1751ea5 100644 --- a/misk/src/test/kotlin/misk/web/extractors/JsonRequestBodyExceptionTest.kt +++ b/misk/src/test/kotlin/misk/web/extractors/JsonRequestBodyExceptionTest.kt @@ -1,6 +1,7 @@ package misk.web.extractors import jakarta.inject.Inject +import java.net.HttpURLConnection.HTTP_BAD_REQUEST import misk.MiskTestingServiceModule import misk.inject.KAbstractModule import misk.testing.MiskTest @@ -21,22 +22,19 @@ import okhttp3.Request import okhttp3.RequestBody.Companion.toRequestBody import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.Test -import java.net.HttpURLConnection.HTTP_BAD_REQUEST /** - * Tests that JSON parsing exceptions (JsonDataException) are properly mapped to HTTP 400 Bad - * Request responses via exception mappers. + * Tests that JSON parsing exceptions (JsonDataException) are properly mapped to HTTP 400 Bad Request responses via + * exception mappers. * * Note: JsonEncodingException extends IOException and is already handled by RequestBodyFeatureBinding. */ @MiskTest(startService = true) internal class JsonRequestBodyExceptionTest { - @MiskTestModule - val module = TestModule() + @MiskTestModule val module = TestModule() - @Inject - lateinit var jettyService: JettyService + @Inject lateinit var jettyService: JettyService private fun serverUrlBuilder(): HttpUrl.Builder { return jettyService.httpServerUrl.newBuilder() @@ -59,10 +57,11 @@ internal class JsonRequestBodyExceptionTest { private fun postJson(path: String, body: String): okhttp3.Response { val httpClient = OkHttpClient() - val request = Request.Builder() - .post(body.toRequestBody("application/json".toMediaType())) - .url(serverUrlBuilder().encodedPath(path).build()) - .build() + val request = + Request.Builder() + .post(body.toRequestBody("application/json".toMediaType())) + .url(serverUrlBuilder().encodedPath(path).build()) + .build() return httpClient.newCall(request).execute() } diff --git a/misk/src/test/kotlin/misk/web/interceptors/RequestDeadlineInterceptorTest.kt b/misk/src/test/kotlin/misk/web/interceptors/RequestDeadlineInterceptorTest.kt index 49b9ff1ffc8..2e45d345d96 100644 --- a/misk/src/test/kotlin/misk/web/interceptors/RequestDeadlineInterceptorTest.kt +++ b/misk/src/test/kotlin/misk/web/interceptors/RequestDeadlineInterceptorTest.kt @@ -22,6 +22,7 @@ import misk.web.ServletHttpCall import misk.web.WebConfig import misk.web.actions.WebAction import misk.web.actions.WebSocketListener +import misk.web.http.HttpVersion import misk.web.interceptors.RequestDeadlineInterceptor.Companion.MISK_REQUEST_DEADLINE_HEADER import misk.web.requestdeadlines.RequestDeadlineMetrics import okhttp3.Headers @@ -474,8 +475,11 @@ class RequestDeadlineInterceptorTest { } // Simple fake UpstreamResponse for testing - internal class FakeUpstreamResponse(override var statusCode: Int = 200, override val headers: Headers = headersOf()) : - ServletHttpCall.UpstreamResponse { + internal class FakeUpstreamResponse( + override var statusCode: Int = 200, + override val headers: Headers = headersOf(), + override val httpVersion: HttpVersion = HttpVersion.HTTP_1_1, + ) : ServletHttpCall.UpstreamResponse { private val headersBuilder = Headers.Builder() private val trailersMap = mutableMapOf() diff --git a/misk/src/test/kotlin/misk/web/jetty/JettyServiceTest.kt b/misk/src/test/kotlin/misk/web/jetty/JettyServiceTest.kt index 8626b2127f1..e4ad7370075 100644 --- a/misk/src/test/kotlin/misk/web/jetty/JettyServiceTest.kt +++ b/misk/src/test/kotlin/misk/web/jetty/JettyServiceTest.kt @@ -9,7 +9,6 @@ import org.assertj.core.api.Assertions.assertThatThrownBy import org.eclipse.jetty.server.Server import org.eclipse.jetty.server.handler.StatisticsHandler import org.eclipse.jetty.server.handler.gzip.GzipHandler -import org.eclipse.jetty.util.MultiException import org.eclipse.jetty.util.thread.ThreadPool import org.junit.jupiter.api.Test import org.mockito.Mockito.doThrow @@ -29,14 +28,13 @@ class JettyServiceTest { } @Test - fun `stop suppresses MultiException when all nested exceptions are InvalidPathException`() { + fun `stop suppresses exception when all nested exceptions are InvalidPathException`() { val server = mock(Server::class.java) - val multi = MultiException() - multi.add(InvalidPathException("http-ingress.sock", "Nul character not allowed")) - multi.add(InvalidPathException("istio-proxy.sock", "Nul character not allowed")) - multi.add(InvalidPathException("grpc-ingress.sock", "Nul character not allowed")) + val exception = InvalidPathException("http-ingress.sock", "Nul character not allowed") + exception.addSuppressed(InvalidPathException("istio-proxy.sock", "Nul character not allowed")) + exception.addSuppressed(InvalidPathException("grpc-ingress.sock", "Nul character not allowed")) `when`(server.isRunning).thenReturn(true) - doThrow(multi).`when`(server).stop() + doThrow(exception).`when`(server).stop() val jettyService = jettyService(server) @@ -44,11 +42,11 @@ class JettyServiceTest { } @Test - fun `stop rethrows MultiException when shutdown failures include non InvalidPathException`() { + fun `stop rethrows exception when shutdown failures include non InvalidPathException`() { val server = mock(Server::class.java) - val multi = MultiException() - multi.add(InvalidPathException("http-ingress.sock", "Nul character not allowed")) - multi.add(RuntimeException("unexpected shutdown failure")) + val multi = RuntimeException() + multi.addSuppressed(InvalidPathException("http-ingress.sock", "Nul character not allowed")) + multi.addSuppressed(RuntimeException("unexpected shutdown failure")) `when`(server.isRunning).thenReturn(true) doThrow(multi).`when`(server).stop() diff --git a/misk/src/test/kotlin/misk/web/jetty/WebActionsServletTest.kt b/misk/src/test/kotlin/misk/web/jetty/WebActionsServletTest.kt index fee19fae1de..0eb0176a1d9 100644 --- a/misk/src/test/kotlin/misk/web/jetty/WebActionsServletTest.kt +++ b/misk/src/test/kotlin/misk/web/jetty/WebActionsServletTest.kt @@ -54,11 +54,19 @@ class WebActionsServletTest { } @Test - fun malformedUriQueryParamsResponseDoesNotContainStacktrace() { - val response = get(path = "/potato", viaUDS = false, viaFileUDS = false, encodedQuery = "test" to "%3C%a%3C") + fun incompleteUtf8EncodingDoesNotFail() { + val response = get(path = "/potato", viaUDS = false, viaFileUDS = false, encodedQuery = "test" to "%C1%BF") + + assertThat(response.body.string()).isEqualTo("TestActionResponse(text=foo)") + assertThat(response.code).isEqualTo(200) + } + + @Test + fun invalidPathCharactersReturns400() { + val response = get(path = "/potato%00bb", viaUDS = false, viaFileUDS = false) - assertThat(response.body.string()).isEqualTo("400: Unable to parse URI query") assertThat(response.code).isEqualTo(400) + assertThat(response.body.string()).contains("HTTP ERROR 400 Bad Request") } @Test @@ -83,7 +91,7 @@ class WebActionsServletTest { .url(jettyService.httpServerUrl.newBuilder().encodedPath("/fooasdf/").build()) .patch("bar".toRequestBody()) ) - assertThat(response.body?.string()).contains("Nothing found at PATCH", "fooasdf") + assertThat(response.body.string()).contains("Nothing found at PATCH", "fooasdf") } internal class WebActionsServletNetworkInterceptor : NetworkInterceptor { @@ -102,10 +110,11 @@ class WebActionsServletTest { ) .build() ) + chain.proceed(chain.httpCall) } class Factory : NetworkInterceptor.Factory { - override fun create(action: Action): NetworkInterceptor? = WebActionsServletNetworkInterceptor() + override fun create(action: Action): NetworkInterceptor = WebActionsServletNetworkInterceptor() } } diff --git a/misk/src/test/kotlin/misk/web/marshal/ActualResponseTypeTest.kt b/misk/src/test/kotlin/misk/web/marshal/ActualResponseTypeTest.kt index f92cdf25b42..fe4b8dbca02 100644 --- a/misk/src/test/kotlin/misk/web/marshal/ActualResponseTypeTest.kt +++ b/misk/src/test/kotlin/misk/web/marshal/ActualResponseTypeTest.kt @@ -71,7 +71,9 @@ internal class ActualResponseTypeTest { @Suppress("unused") class SuspendActions { suspend fun responseBody(): Response = TODO() + suspend fun responseString(): Response = TODO() + suspend fun responseByteString(): Response = TODO() } diff --git a/misk/src/test/kotlin/misk/web/marshal/SuspendJsonResponseTest.kt b/misk/src/test/kotlin/misk/web/marshal/SuspendJsonResponseTest.kt index a5b324791b6..7307a146a6a 100644 --- a/misk/src/test/kotlin/misk/web/marshal/SuspendJsonResponseTest.kt +++ b/misk/src/test/kotlin/misk/web/marshal/SuspendJsonResponseTest.kt @@ -21,12 +21,12 @@ import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.Test /** - * Mirrors [JsonResponseTest] but all actions use `suspend fun`. - * Verifies suspend WebActions work identically to non-suspend for all return type variants. + * Mirrors [JsonResponseTest] but all actions use `suspend fun`. Verifies suspend WebActions work identically to + * non-suspend for all return type variants. * - * Also adds [ReturnAsWrappedResponseBody] and [ReturnAsWrappedResponseBodyNoContentType] - * which test `suspend fun(): Response` — the specific case that was previously - * broken due to WildcardType leaking through KType.javaType for suspend function return types. + * Also adds [ReturnAsWrappedResponseBody] and [ReturnAsWrappedResponseBodyNoContentType] which test `suspend fun(): + * Response` — the specific case that was previously broken due to WildcardType leaking through + * KType.javaType for suspend function return types. */ @MiskTest(startService = true) internal class SuspendJsonResponseTest { @@ -158,8 +158,7 @@ internal class SuspendJsonResponseTest { class ReturnAsWrappedResponseBody @Inject constructor() : WebAction { @Get("/suspend-response/as-wrapped-response-body") @ResponseContentType(MediaTypes.APPLICATION_JSON) - suspend fun call(): Response = - Response("{\"message\":\"as-wrapped-response-body\"}".toResponseBody()) + suspend fun call(): Response = Response("{\"message\":\"as-wrapped-response-body\"}".toResponseBody()) } class ReturnAsWrappedResponseBodyWith201 @Inject constructor() : WebAction { @@ -172,8 +171,7 @@ internal class SuspendJsonResponseTest { // Response without @ResponseContentType — the original error case class ReturnAsWrappedResponseBodyNoContentType @Inject constructor() : WebAction { @Get("/suspend-response/as-wrapped-response-body-no-ct") - suspend fun call(): Response = - Response("{\"message\":\"no-content-type\"}".toResponseBody()) + suspend fun call(): Response = Response("{\"message\":\"no-content-type\"}".toResponseBody()) } class TestModule : KAbstractModule() { diff --git a/misk/src/test/kotlin/misk/web/proxy/WebProxyActionTest.kt b/misk/src/test/kotlin/misk/web/proxy/WebProxyActionTest.kt index 827b4c49fe8..f700c5f53fb 100644 --- a/misk/src/test/kotlin/misk/web/proxy/WebProxyActionTest.kt +++ b/misk/src/test/kotlin/misk/web/proxy/WebProxyActionTest.kt @@ -338,6 +338,8 @@ class WebProxyActionTest { } } + // TODO Permitting this necessitates usage of UriCompliance.LEGACY in misk's HttpConfig. + // We should either make this configurable or switch to a strict compliance mode @Test internal fun getForwardedSlashesOnSlashes() { upstreamServer.enqueue( diff --git a/misk/src/test/kotlin/misk/web/ssl/Http2ConnectivityTest.kt b/misk/src/test/kotlin/misk/web/ssl/Http2ConnectivityTest.kt index cf773e84a59..0d6ac0a17f7 100644 --- a/misk/src/test/kotlin/misk/web/ssl/Http2ConnectivityTest.kt +++ b/misk/src/test/kotlin/misk/web/ssl/Http2ConnectivityTest.kt @@ -226,7 +226,8 @@ class Http2ConnectivityTest { @ResponseContentType(MediaTypes.TEXT_PLAIN_UTF8) fun disconnect(): Response { val request = actionScopedServletRequest.get() as org.eclipse.jetty.server.Request - request.httpChannel.abort(Exception("boom")) // Synthesize a connectivity failure. + request.connectionMetaData.connection.onClose(Exception("boom")) // Synthesize a connectivity failure. + request.connectionMetaData.connection.close() return Response(body = "") } @@ -240,7 +241,8 @@ class Http2ConnectivityTest { @ResponseContentType(MediaTypes.TEXT_PLAIN_UTF8) fun disconnect(): ResponseBody { val request = actionScopedServletRequest.get() as org.eclipse.jetty.server.Request - request.httpChannel.abort(Exception("boom")) // Synthesize a connectivity failure. + request.connectionMetaData.connection.onClose(Exception("boom")) // Synthesize a connectivity failure. + request.connectionMetaData.connection.close() return object : ResponseBody { override fun writeTo(sink: BufferedSink) { diff --git a/misk/src/test/kotlin/misk/web/uds/UDSServerAuthorityTest.kt b/misk/src/test/kotlin/misk/web/uds/UDSServerAuthorityTest.kt new file mode 100644 index 00000000000..046c46587aa --- /dev/null +++ b/misk/src/test/kotlin/misk/web/uds/UDSServerAuthorityTest.kt @@ -0,0 +1,84 @@ +package misk.web.uds + +import jakarta.inject.Inject +import java.net.StandardProtocolFamily +import java.net.UnixDomainSocketAddress +import java.nio.ByteBuffer +import java.nio.channels.SocketChannel +import java.nio.file.Files +import misk.MiskTestingServiceModule +import misk.inject.KAbstractModule +import misk.testing.MiskTest +import misk.testing.MiskTestModule +import misk.web.Get +import misk.web.ResponseContentType +import misk.web.WebActionModule +import misk.web.WebServerTestingModule +import misk.web.WebUnixDomainSocketConfig +import misk.web.actions.WebAction +import misk.web.jetty.JettyService +import misk.web.mediatype.MediaTypes +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Test + +/** + * Jetty 12 derives a request's server authority from the connection's local address when the request carries no + * Host/:authority. On a Unix domain socket that local address is the socket file path, which + * [org.eclipse.jetty.util.HostPort] rejects as an invalid authority -- it logs "Bad Authority" and throws, failing the + * request with a 400 before it ever reaches a web action. + * + * [JettyService] avoids that by setting a fallback server authority on the Unix-domain connector's HttpConfiguration. + */ +@MiskTest(startService = true) +class UDSServerAuthorityTest { + @MiskTestModule val module = TestModule() + + /** An HTTP/1.0 request is valid without a Host header, so Jetty must fall back to the authority. */ + @Test + fun `request without a Host header is served`() { + val response = sendRaw("GET /hello HTTP/1.0\r\n\r\n") + + assertThat(response).startsWith("HTTP/1.1 200") + assertThat(response).endsWith("hello") + } + + private fun sendRaw(request: String): String { + SocketChannel.open(StandardProtocolFamily.UNIX).use { channel -> + channel.connect(UnixDomainSocketAddress.of(socketPath)) + channel.write(ByteBuffer.wrap(request.toByteArray(Charsets.US_ASCII))) + + val response = StringBuilder() + val buffer = ByteBuffer.allocate(1024) + while (channel.read(buffer) != -1) { + buffer.flip() + response.append(Charsets.US_ASCII.decode(buffer)) + buffer.clear() + } + return response.toString() + } + } + + class HelloAction @Inject constructor() : WebAction { + @Get("/hello") @ResponseContentType(MediaTypes.TEXT_PLAIN_UTF8) fun sayHello() = "hello" + } + + inner class TestModule : KAbstractModule() { + override fun configure() { + install( + WebServerTestingModule( + webConfig = + WebServerTestingModule.TESTING_WEB_CONFIG.copy( + unix_domain_sockets = listOf(WebUnixDomainSocketConfig(path = socketPath)) + ) + ) + ) + install(MiskTestingServiceModule()) + install(WebActionModule.create()) + } + } + + companion object { + // Keep this short: Unix domain socket paths are capped near 104 bytes on macOS. + private val socketPath = Files.createTempDirectory("uds").resolve("authority.sock").toString() + } +} diff --git a/wisp/wisp-feature-testing/src/main/kotlin/wisp/feature/testing/FakeLegacyFeatureFlags.kt b/wisp/wisp-feature-testing/src/main/kotlin/wisp/feature/testing/FakeLegacyFeatureFlags.kt index 6022a2b5982..ebd3036a34f 100644 --- a/wisp/wisp-feature-testing/src/main/kotlin/wisp/feature/testing/FakeLegacyFeatureFlags.kt +++ b/wisp/wisp-feature-testing/src/main/kotlin/wisp/feature/testing/FakeLegacyFeatureFlags.kt @@ -65,12 +65,15 @@ class FakeLegacyFeatureFlags @Deprecated("Needed for Misk Provider usage...") co override fun getJsonString(feature: Feature): String = getJsonString(feature, KEY) private fun get(feature: Feature, key: String, attributes: Attributes, clazz: Class): T { - val result = get(feature, key, attributes) - ?: throw IllegalArgumentException("Flag $feature must be overridden with override() before use") + val result = + get(feature, key, attributes) + ?: throw IllegalArgumentException("Flag $feature must be overridden with override() before use") try { return clazz.cast(result) } catch (_: ClassCastException) { - throw IllegalArgumentException("Flag $feature: expecting $clazz but override() was called with ${result.javaClass}") + throw IllegalArgumentException( + "Flag $feature: expecting $clazz but override() was called with ${result.javaClass}" + ) } }