diff --git a/.palantir/revapi.yml b/.palantir/revapi.yml index 1e640df0fb6..f9791c2be9a 100644 --- a/.palantir/revapi.yml +++ b/.palantir/revapi.yml @@ -6300,6 +6300,18 @@ acceptedBreaks: - code: "java.method.removed" old: "method java.lang.String misk.web.metadata.Metadata::getId()" justification: "API is unused so change is safe" + "2024.04.22.213320-ac881d1": + com.squareup.misk:misk-action-scopes: + - code: "java.method.removed" + old: "method java.util.Map, java.lang.Object> misk.scope.ActionScope.Instance::asImmediateValues$misk_action_scopes()" + justification: "Removed ActionScope methods that had been deprecated for a month" + - code: "java.method.removed" + old: "method java.util.Map, java.lang.Object> misk.scope.ActionScope::snapshotActionScope()" + justification: "Removed ActionScope methods that had been deprecated for a month" + - code: "java.method.removed" + old: "method misk.scope.ActionScope misk.scope.ActionScope::enter(java.util.Map,\ + \ ? extends java.lang.Object>)" + justification: "Removed ActionScope methods that had been deprecated for a month" misk-0.18.0: com.squareup.misk:misk-gcp: - code: "java.method.numberOfParametersChanged" diff --git a/misk-action-scopes/api/misk-action-scopes.api b/misk-action-scopes/api/misk-action-scopes.api index 1ee39678019..a4bf9387aff 100644 --- a/misk-action-scopes/api/misk-action-scopes.api +++ b/misk-action-scopes/api/misk-action-scopes.api @@ -19,7 +19,6 @@ public final class misk/scope/ActionScope : java/lang/AutoCloseable { public final fun asContextElement ()Lkotlin/coroutines/CoroutineContext$Element; public fun close ()V public final fun create (Ljava/util/Map;)Lmisk/scope/ActionScope$Instance; - public final fun enter (Ljava/util/Map;)Lmisk/scope/ActionScope; public final fun get (Lcom/google/inject/Key;)Ljava/lang/Object; public final fun inScope ()Z public final fun propagate (Ljava/util/concurrent/Callable;)Ljava/util/concurrent/Callable; @@ -27,7 +26,6 @@ public final class misk/scope/ActionScope : java/lang/AutoCloseable { public final fun propagate (Lkotlin/reflect/KFunction;)Lkotlin/reflect/KFunction; public final fun runBlocking (Lkotlin/coroutines/CoroutineContext;Lkotlin/jvm/functions/Function2;)Ljava/lang/Object; public final fun runBlocking (Lkotlin/jvm/functions/Function2;)Ljava/lang/Object; - public final fun snapshotActionScope ()Ljava/util/Map; public final fun snapshotActionScopeInstance ()Lmisk/scope/ActionScope$Instance; } diff --git a/misk-action-scopes/src/main/kotlin/misk/scope/ActionScope.kt b/misk-action-scopes/src/main/kotlin/misk/scope/ActionScope.kt index 7452408b882..44868b9d091 100644 --- a/misk-action-scopes/src/main/kotlin/misk/scope/ActionScope.kt +++ b/misk-action-scopes/src/main/kotlin/misk/scope/ActionScope.kt @@ -58,7 +58,7 @@ class ActionScope @Inject internal constructor( * * Example usage: * ``` - * scope.enter(seedData).use { + * scope.create(seedData).inScope { * runBlocking(scope.asContextElement()) { * async(Dispatchers.IO) { * tester.fooValue() @@ -76,26 +76,11 @@ class ActionScope @Inject internal constructor( return threadLocalInstance.asContextElement(instance) } - @Deprecated( - "Use snapshotActionScopeInstance instead", - ReplaceWith("this.snapshotActionScopeInstance()"), - ) - fun snapshotActionScope(): Map, Any?> { - return snapshotActionScopeInstance().asImmediateValues() - } - fun snapshotActionScopeInstance(): Instance { check(inScope()) { "not running within an ActionScope" } return threadLocalInstance.get() } - @Deprecated("Use create() instead and then call inScope() to enter the scope") - /** Starts the scope on a thread with the provided seed data */ - fun enter(seedData: Map, Any?>): ActionScope { - create(seedData).enter() - return this - } - /** Creates a new scope on the current thread with the provided seed data */ fun create(seedData: Map, Any?>): Instance { check(!inScope()) { @@ -217,12 +202,6 @@ class ActionScope @Inject internal constructor( return lazyValues.getValue(key).value as T } - internal fun asImmediateValues(): Map, Any?> { - return lazyValues - .filterValues { it.isInitialized() } - .mapValues { it.value.value } - } - fun inScope(block: () -> T): T { return scope.enter(this).use { block() 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 03536548556..aec9bb86ed3 100644 --- a/misk-action-scopes/src/test/kotlin/misk/scope/ActionScopePropagationTest.kt +++ b/misk-action-scopes/src/test/kotlin/misk/scope/ActionScopePropagationTest.kt @@ -34,11 +34,11 @@ internal class ActionScopePropagationTest { keyOf(Names.named("from-seed")) to "my seed data" ) - val callable = scope.enter(seedData).use { + val callable = scope.create(seedData).inScope { scope.propagate(Callable { tester.fooValue() }) } - scope.enter(seedData).use { + scope.create(seedData).inScope { // Submit to same thread after we've already entered the scope val result = directExecutor.submit(callable).get() assertThat(result).isEqualTo("my seed data and bar and foo!") @@ -55,7 +55,7 @@ internal class ActionScopePropagationTest { keyOf(Names.named("from-seed")) to "my seed data" ) - val callable = scope.enter(seedData).use { + val callable = scope.create(seedData).inScope { scope.propagate(Callable { tester.fooValue() }) } @@ -76,11 +76,11 @@ internal class ActionScopePropagationTest { // Propagate on the the KCallable directly val f: KFunction = tester::fooValue - val callable = scope.enter(seedData).use { + val callable = scope.create(seedData).inScope { scope.propagate(f) } - scope.enter(seedData).use { + scope.create(seedData).inScope { // Submit to same thread after we've already entered the scope val result = directExecutor.submit( Callable { @@ -103,7 +103,7 @@ internal class ActionScopePropagationTest { // Propagate on the the KCallable directly val f: KFunction = tester::fooValue - val callable = scope.enter(seedData).use { + val callable = scope.create(seedData).inScope { scope.propagate(f) } @@ -127,11 +127,11 @@ internal class ActionScopePropagationTest { ) // Propagate on a lambda directly - val function = scope.enter(seedData).use { + val function = scope.create(seedData).inScope { scope.propagate { tester.fooValue() } } - scope.enter(seedData).use { + scope.create(seedData).inScope { // Submit to same thread after we've already entered the scope val result = directExecutor.submit(Callable { function() }).get() assertThat(result).isEqualTo("my seed data and bar and foo!") @@ -149,7 +149,7 @@ internal class ActionScopePropagationTest { ) // Propagate on a lambda directly - val function = scope.enter(seedData).use { + val function = scope.create(seedData).inScope { scope.propagate { tester.fooValue() } } 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 d422fc03ea3..0f11cf10db8 100644 --- a/misk-action-scopes/src/test/kotlin/misk/scope/ActionScopedTest.kt +++ b/misk-action-scopes/src/test/kotlin/misk/scope/ActionScopedTest.kt @@ -58,7 +58,7 @@ internal class ActionScopedTest { keyOf(Names.named("from-seed")) to "seed-value" ) - scope.enter(seedData).use { assertThat(foo.get()).isEqualTo("seed-value and bar and foo!") } + scope.create(seedData).inScope { assertThat(foo.get()).isEqualTo("seed-value and bar and foo!") } } @Test @@ -67,7 +67,7 @@ internal class ActionScopedTest { val injector = Guice.createInjector(TestActionScopedProviderModule()) injector.injectMembers(this) - scope.enter(mapOf()).use { scope.enter(mapOf()).use { } } + scope.create(mapOf()).inScope { scope.create(mapOf()) } } } @@ -88,7 +88,7 @@ internal class ActionScopedTest { injector.injectMembers(this) // NB(mmihic): Seed data not specified - scope.enter(mapOf()).use { foo.get() } + scope.create(mapOf()).inScope { foo.get() } } } @@ -98,7 +98,7 @@ internal class ActionScopedTest { injector.injectMembers(this) val seedData: Map, Any> = mapOf(keyOf(Names.named("from-seed")) to "null") - val result = scope.enter(seedData).use { nullableFoo.get() } + val result = scope.create(seedData).inScope { nullableFoo.get() } assertThat(result).isNull() } @@ -107,7 +107,7 @@ internal class ActionScopedTest { val injector = Guice.createInjector(TestActionScopedProviderModule()) injector.injectMembers(this) - scope.enter(mapOf()).use { + scope.create(mapOf()).inScope { assertThat(constantString.get()).isEqualTo("constant-value") assertThat(optionalConstantString.get()).isEqualTo(Optional.of("constant-value")) @@ -127,13 +127,13 @@ internal class ActionScopedTest { val emptyOptionalSeedData: Map, Any> = mapOf( optionalStringKey to Optional.empty(), ) - val emptyOptionalResult = scope.enter(emptyOptionalSeedData).use { optional.get() } + val emptyOptionalResult = scope.create(emptyOptionalSeedData).inScope { optional.get() } assertThat(emptyOptionalResult).isEqualTo("empty") val presentOptionalSeedData: Map, Any> = mapOf( optionalStringKey to Optional.of("present"), ) - val presentOptionalResult = scope.enter(presentOptionalSeedData).use { optional.get() } + val presentOptionalResult = scope.create(presentOptionalSeedData).inScope { optional.get() } assertThat(presentOptionalResult).isEqualTo("present") } @@ -143,7 +143,7 @@ internal class ActionScopedTest { injector.injectMembers(this) val seedData: Map, Any> = mapOf(keyOf(Names.named("from-seed")) to "null") - val result = scope.enter(seedData).use { nullableBasedOnFoo.get() } + val result = scope.create(seedData).inScope { nullableBasedOnFoo.get() } assertThat(result).isNull() } @@ -158,7 +158,7 @@ internal class ActionScopedTest { val seedData: Map, Any> = mapOf( keyOf(Names.named("from-seed")) to "illegal-state" ) - scope.enter(seedData).use { zed.get() } + scope.create(seedData).inScope { zed.get() } } } @@ -171,8 +171,8 @@ internal class ActionScopedTest { keyOf(Names.named("from-seed")) to "seed-value" ) - scope.enter(seedData).use { actionScope -> - runBlocking(actionScope.asContextElement()) { + scope.create(seedData).inScope { + runBlocking(scope.asContextElement()) { assertThat(foo.get()).isEqualTo("seed-value and bar and foo!") } } @@ -198,7 +198,7 @@ internal class ActionScopedTest { ) // exhibit A: trying to access scoped things in a new thread results in exceptions - scope.enter(seedData).use { _ -> + scope.create(seedData).inScope { var thrown: Throwable? = null thread { @@ -213,13 +213,13 @@ internal class ActionScopedTest { // exhibit B: trying to access scoped things in a new thread can work, if you take // a snapshot of the scope and use it to instantiate a scope in the new thread. - scope.enter(seedData).use { actionScope -> + scope.create(seedData).inScope { var thrown: Throwable? = null - val snapshot = actionScope.snapshotActionScope() + val instance = scope.snapshotActionScopeInstance() thread { try { - actionScope.enter(snapshot).use { + instance.inScope { assertThat(foo.get()).isEqualTo("seed-value and bar and foo!") } } catch (t: Throwable) { @@ -240,7 +240,7 @@ internal class ActionScopedTest { ) // exhibit A: trying to access scoped things in a new thread results in exceptions - scope.enter(seedData).use { _ -> + scope.create(seedData).inScope { var thrown: Throwable? = null thread { @@ -255,13 +255,13 @@ internal class ActionScopedTest { // exhibit B: trying to access scoped things in a new thread can work, if you take // a snapshot of the scope and use it to instantiate a scope in the new thread. - scope.enter(seedData).use { actionScope -> + scope.create(seedData).inScope { var thrown: Throwable? = null - val instance = actionScope.snapshotActionScopeInstance() + val instance = scope.snapshotActionScopeInstance() thread { try { - actionScope.enter(instance).use { + instance.inScope { assertThat(foo.get()).isEqualTo("seed-value and bar and foo!") } } catch (t: Throwable) { 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 e9a79f7d942..7d1f2d82bf4 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 @@ -35,7 +35,7 @@ internal class ActionScopedCoroutineTest { keyOf(Names.named("from-seed")) to "my seed data" ) - val value = scope.enter(seedData).use { + val value = scope.create(seedData).inScope { scope.runBlocking { tester.fooValue() } @@ -57,7 +57,7 @@ internal class ActionScopedCoroutineTest { keyOf(Names.named("from-seed")) to "my seed data" ) - val value = scope.enter(seedData).use { + val value = scope.create(seedData).inScope { scope.runBlocking(Dispatchers.IO) { tester.fooValue() } 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 0082193a10c..95a587d0aab 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 @@ -42,7 +42,7 @@ internal class ActionScopedExecutorServiceTest { keyOf(Names.named("from-seed")) to "my seed data" ) - val future = scope.enter(seedData).use { + val future = scope.create(seedData).inScope { executor.submit(Callable { tester.fooValue() }) } 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 a52008089c1..2e5552e4ba7 100644 --- a/misk-admin/src/test/kotlin/misk/web/v2/DashboardPageLayoutTest.kt +++ b/misk-admin/src/test/kotlin/misk/web/v2/DashboardPageLayoutTest.kt @@ -26,7 +26,7 @@ class DashboardPageLayoutTest { @Test fun `happy path`() { - actionScope.enter(mapOf(HttpCall::class.toKey() to fakeHttpCall)).use { + actionScope.create(mapOf(HttpCall::class.toKey() to fakeHttpCall)).inScope { // No exception thrown on correct usage layout.get().newBuilder().build() } @@ -34,7 +34,7 @@ class DashboardPageLayoutTest { @Test fun `no builder reuse permitted`() { - actionScope.enter(mapOf(HttpCall::class.toKey() to fakeHttpCall)).use { + actionScope.create(mapOf(HttpCall::class.toKey() to fakeHttpCall)).inScope { // Fresh builder must have newBuilder() called val e1 = assertFailsWith { layout.get().build() } assertEquals( diff --git a/misk-testing/src/main/kotlin/misk/web/MiskCallerExtension.kt b/misk-testing/src/main/kotlin/misk/web/MiskCallerExtension.kt index 92d3adfd2d6..c9697f175c5 100644 --- a/misk-testing/src/main/kotlin/misk/web/MiskCallerExtension.kt +++ b/misk-testing/src/main/kotlin/misk/web/MiskCallerExtension.kt @@ -15,11 +15,11 @@ class MiskCallerExtension : BeforeTestExecutionCallback, AfterTestExecutionCallb val injector = context.retrieve("injector") val actionScopeProvider = injector.getBinding(ActionScope::class.java) val actionScope = actionScopeProvider.provider.get() - actionScope.enter( + actionScope.create( mapOf( keyOf() to context.getPrincipal() ) - ) + ).enter() } override fun afterTestExecution(context: ExtensionContext) { diff --git a/misk/src/main/kotlin/misk/web/BoundAction.kt b/misk/src/main/kotlin/misk/web/BoundAction.kt index 31a989409da..1364473cd37 100644 --- a/misk/src/main/kotlin/misk/web/BoundAction.kt +++ b/misk/src/main/kotlin/misk/web/BoundAction.kt @@ -122,7 +122,7 @@ internal class BoundAction( MDC.clear() // MDC should already be empty, but clear it again just in case try { - scope.enter(seedData).use { + scope.create(seedData).inScope { handle(httpCall, pathMatcher) } } finally { diff --git a/misk/src/test/kotlin/misk/client/PropagatingScopeActionInInterceptorsTest.kt b/misk/src/test/kotlin/misk/client/PropagatingScopeActionInInterceptorsTest.kt index b6882c4f926..64de443dfbf 100644 --- a/misk/src/test/kotlin/misk/client/PropagatingScopeActionInInterceptorsTest.kt +++ b/misk/src/test/kotlin/misk/client/PropagatingScopeActionInInterceptorsTest.kt @@ -69,7 +69,7 @@ class PropagatingScopeActionInInterceptorsTest { @Test fun `propagate action scoped using typed http client`() { - val response = scope.enter(seedData).use { + val response = scope.create(seedData).inScope { client.getDinosaur(Dinosaur.Builder().name("trex").build()).execute() } assertThat(response.body()!!.name).isEqualTo("es-US") @@ -77,8 +77,8 @@ class PropagatingScopeActionInInterceptorsTest { @Test fun `propagate action scoped using typed http client with suspended calls`() { - val response = scope.enter(seedData).use { - runBlocking(Dispatchers.IO + it.asContextElement()) { + val response = scope.create(seedData).inScope { + runBlocking(Dispatchers.IO + scope.asContextElement()) { client.getDinosaur(Dinosaur.Builder().name("trex").build()).execute() } } @@ -87,7 +87,7 @@ class PropagatingScopeActionInInterceptorsTest { @Test fun `propagate action scoped using gRPC client`() { - val response = scope.enter(seedData).use { + val response = scope.create(seedData).inScope { dinoService.GetDinosour().executeBlocking(Dinosaur.Builder().name("trex").build()) } assertThat(response.name).isEqualTo("es-US") @@ -95,8 +95,8 @@ class PropagatingScopeActionInInterceptorsTest { @Test fun `propagate action scoped using gRPC client with suspended calls`() { - val response = scope.enter(seedData).use { - runBlocking(Dispatchers.IO + it.asContextElement()) { + val response = scope.create(seedData).inScope { + runBlocking(Dispatchers.IO + scope.asContextElement()) { dinoService.GetDinosour().execute(Dinosaur.Builder().name("trex").build()) } }