Remove deprecated ActionScope methods

Author: JGulbronson

.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<com.google.inject.Key<?>, 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<com.google.inject.Key<?>, 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<com.google.inject.Key<?>,\
+        \ ? 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"

misk-action-scopes/api/misk-action-scopes.api

@@ -19,15 +19,13 @@ 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;
 	public final fun propagate (Lkotlin/jvm/functions/Function0;)Lkotlin/jvm/functions/Function0;
 	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;
 }
 

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<Key<*>, 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<Key<*>, Any?>): ActionScope {
-    create(seedData).enter()
-    return this
-  }
-
   /** Creates a new scope on the current thread with the provided seed data */
   fun create(seedData: Map<Key<*>, Any?>): Instance {
     check(!inScope()) {
@@ -217,12 +202,6 @@ class ActionScope @Inject internal constructor(
       return lazyValues.getValue(key).value as T
     }
 
-    internal fun asImmediateValues(): Map<Key<*>, Any?> {
-      return lazyValues
-        .filterValues { it.isInitialized() }
-        .mapValues { it.value.value }
-    }
-
     fun <T> inScope(block: () -> T): T {
       return scope.enter(this).use {
         block()

misk-action-scopes/src/test/kotlin/misk/scope/ActionScopePropagationTest.kt

@@ -34,11 +34,11 @@ internal class ActionScopePropagationTest {
       keyOf<String>(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<String>(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<String> = 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<String> = 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() }
     }
 

misk-action-scopes/src/test/kotlin/misk/scope/ActionScopedTest.kt

@@ -58,7 +58,7 @@ internal class ActionScopedTest {
       keyOf<String>(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<Key<*>, Any> = mapOf(keyOf<String>(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<Key<*>, Any> = mapOf(
       optionalStringKey to Optional.empty<String>(),
     )
-    val emptyOptionalResult = scope.enter(emptyOptionalSeedData).use { optional.get() }
+    val emptyOptionalResult = scope.create(emptyOptionalSeedData).inScope { optional.get() }
     assertThat(emptyOptionalResult).isEqualTo("empty")
 
     val presentOptionalSeedData: Map<Key<*>, 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<Key<*>, Any> = mapOf(keyOf<String>(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<Key<*>, Any> = mapOf(
         keyOf<String>(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<String>(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) {

misk-action-scopes/src/test/kotlin/misk/scope/coroutine/ActionScopedCoroutineTest.kt

@@ -35,7 +35,7 @@ internal class ActionScopedCoroutineTest {
       keyOf<String>(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<String>(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()
       }

misk-action-scopes/src/test/kotlin/misk/scope/executor/ActionScopedExecutorServiceTest.kt

@@ -42,7 +42,7 @@ internal class ActionScopedExecutorServiceTest {
       keyOf<String>(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() })
     }
 

misk-admin/src/test/kotlin/misk/web/v2/DashboardPageLayoutTest.kt

@@ -26,15 +26,15 @@ 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()
     }
   }
 
   @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<IllegalStateException> { layout.get().build() }
       assertEquals(

misk-testing/src/main/kotlin/misk/web/MiskCallerExtension.kt

@@ -15,11 +15,11 @@ class MiskCallerExtension : BeforeTestExecutionCallback, AfterTestExecutionCallb
     val injector = context.retrieve<Injector>("injector")
     val actionScopeProvider = injector.getBinding(ActionScope::class.java)
     val actionScope = actionScopeProvider.provider.get()
-    actionScope.enter(
+    actionScope.create(
       mapOf(
         keyOf<MiskCaller>() to context.getPrincipal()
       )
-    )
+    ).enter()
   }
 
   override fun afterTestExecution(context: ExtensionContext) {

misk/src/main/kotlin/misk/web/BoundAction.kt

@@ -122,7 +122,7 @@ internal class BoundAction<A : WebAction>(
     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 {