Switch Metadata to mapbinder and expose module to wrap install (#3231)

* Switch Metadata to mapbinder and expose module to wrap install

* Add MetadataModule to wrap installation

* apiDump

* Fix API breaks

* Fix broken API

.palantir/revapi.yml

@@ -6234,13 +6234,6 @@ acceptedBreaks:
         \ extends java.lang.annotation.Annotation>, java.util.List<java.lang.String>,\
         \ java.util.List<java.lang.String>, boolean, boolean)"
       justification: "New parameters are optional"
-  "2024.04.10.210352-9854ef3":
-    com.squareup.misk:misk-aws:
-    - code: "java.method.numberOfParametersChanged"
-      old: "method void misk.jobqueue.sqs.SqsJob::delayForFailure(int)"
-      new: "method void misk.jobqueue.sqs.SqsJob::delayForFailure()"
-      justification: "{SqsJob is an internal class and there seem to be no references\
-        \ to this function}"
   "2024.04.12.171745-81ea336":
     com.squareup.misk:misk-aws:
     - code: "java.method.numberOfParametersChanged"
@@ -6267,6 +6260,46 @@ acceptedBreaks:
         \ com.amazonaws.services.sqs.AmazonSQS, int)"
       justification: "SQSJob is an internal class and is not exposed to the clients\
         \ of Misk"
+  "2024.04.21.232758-20ad049":
+    com.squareup.misk:misk:
+    - code: "java.method.removed"
+      old: "method java.lang.String misk.web.metadata.Metadata::getId() @ misk.web.metadata.webaction.WebActionsMetadata"
+      justification: "API is unused so change is safe"
+    com.squareup.misk:misk-admin:
+    - code: "java.method.parameterTypeChanged"
+      old: "parameter misk.web.metadata.all.AllMetadataAction.Response misk.web.metadata.all.AllMetadataAction.Response::copy(===java.util.List<?\
+        \ extends misk.web.metadata.Metadata>===)"
+      new: "parameter misk.web.metadata.all.AllMetadataAction.Response misk.web.metadata.all.AllMetadataAction.Response::copy(===java.util.Map<java.lang.String,\
+        \ ? extends misk.web.metadata.Metadata>===)"
+      justification: "API is unused so change is safe"
+    - code: "java.method.parameterTypeChanged"
+      old: "parameter void misk.web.metadata.all.AllMetadataAction.Response::<init>(===java.util.List<?\
+        \ extends misk.web.metadata.Metadata>===)"
+      new: "parameter void misk.web.metadata.all.AllMetadataAction.Response::<init>(===java.util.Map<java.lang.String,\
+        \ ? extends misk.web.metadata.Metadata>===)"
+      justification: "API is unused so change is safe"
+    - code: "java.method.parameterTypeChanged"
+      old: "parameter void misk.web.metadata.all.AllMetadataAction::<init>(===java.util.List<?\
+        \ extends misk.web.metadata.Metadata>===)"
+      new: "parameter void misk.web.metadata.all.AllMetadataAction::<init>(===java.util.Map<java.lang.String,\
+        \ ? extends misk.web.metadata.Metadata>===)"
+      justification: "API is unused so change is safe"
+    - code: "java.method.returnTypeChanged"
+      old: "method java.util.List<misk.web.metadata.Metadata> misk.web.metadata.all.AllMetadataAction.Response::component1()"
+      new: "method java.util.Map<java.lang.String, misk.web.metadata.Metadata> misk.web.metadata.all.AllMetadataAction.Response::component1()"
+      justification: "API is unused so change is safe"
+    - code: "java.method.returnTypeChanged"
+      old: "method java.util.List<misk.web.metadata.Metadata> misk.web.metadata.all.AllMetadataAction.Response::getAll()"
+      new: "method java.util.Map<java.lang.String, misk.web.metadata.Metadata> misk.web.metadata.all.AllMetadataAction.Response::getAll()"
+      justification: "API is unused so change is safe"
+    com.squareup.misk:misk-config:
+    - code: "java.method.numberOfParametersChanged"
+      old: "method void misk.web.metadata.Metadata::<init>(java.lang.String, java.lang.Object)"
+      new: "method void misk.web.metadata.Metadata::<init>(java.lang.Object)"
+      justification: "API is unused so change is safe"
+    - code: "java.method.removed"
+      old: "method java.lang.String misk.web.metadata.Metadata::getId()"
+      justification: "API is unused so change is safe"
   misk-0.18.0:
     com.squareup.misk:misk-gcp:
     - code: "java.method.numberOfParametersChanged"

misk-admin/api/misk-admin.api

@@ -630,17 +630,17 @@ public abstract interface annotation class misk/web/metadata/all/AllMetadataAcce
 }
 
 public final class misk/web/metadata/all/AllMetadataAction : misk/web/actions/WebAction {
-	public fun <init> (Ljava/util/List;)V
+	public fun <init> (Ljava/util/Map;)V
 	public final fun getAll ()Lmisk/web/metadata/all/AllMetadataAction$Response;
 }
 
 public final class misk/web/metadata/all/AllMetadataAction$Response {
-	public fun <init> (Ljava/util/List;)V
-	public final fun component1 ()Ljava/util/List;
-	public final fun copy (Ljava/util/List;)Lmisk/web/metadata/all/AllMetadataAction$Response;
-	public static synthetic fun copy$default (Lmisk/web/metadata/all/AllMetadataAction$Response;Ljava/util/List;ILjava/lang/Object;)Lmisk/web/metadata/all/AllMetadataAction$Response;
+	public fun <init> (Ljava/util/Map;)V
+	public final fun component1 ()Ljava/util/Map;
+	public final fun copy (Ljava/util/Map;)Lmisk/web/metadata/all/AllMetadataAction$Response;
+	public static synthetic fun copy$default (Lmisk/web/metadata/all/AllMetadataAction$Response;Ljava/util/Map;ILjava/lang/Object;)Lmisk/web/metadata/all/AllMetadataAction$Response;
 	public fun equals (Ljava/lang/Object;)Z
-	public final fun getAll ()Ljava/util/List;
+	public final fun getAll ()Ljava/util/Map;
 	public fun hashCode ()I
 	public fun toString ()Ljava/lang/String;
 }
@@ -685,10 +685,11 @@ public final class misk/web/metadata/config/ConfigMetadataAction$Response {
 	public fun toString ()Ljava/lang/String;
 }
 
-public final class misk/web/metadata/config/ConfigMetadataProvider : com/google/inject/Provider {
+public final class misk/web/metadata/config/ConfigMetadataProvider : misk/web/metadata/MetadataProvider {
 	public fun <init> ()V
 	public synthetic fun get ()Ljava/lang/Object;
 	public fun get ()Lmisk/web/metadata/config/ConfigMetadata;
+	public fun getId ()Ljava/lang/String;
 }
 
 public final class misk/web/metadata/database/DatabaseHibernateMetadata : misk/web/metadata/Metadata {
@@ -702,11 +703,12 @@ public final class misk/web/metadata/database/DatabaseHibernateMetadata : misk/w
 	public fun toString ()Ljava/lang/String;
 }
 
-public final class misk/web/metadata/database/DatabaseHibernateMetadataProvider : com/google/inject/Provider {
+public final class misk/web/metadata/database/DatabaseHibernateMetadataProvider : misk/web/metadata/MetadataProvider {
 	public field metadata Ljava/util/List;
 	public fun <init> ()V
 	public synthetic fun get ()Ljava/lang/Object;
 	public fun get ()Lmisk/web/metadata/database/DatabaseHibernateMetadata;
+	public fun getId ()Ljava/lang/String;
 	public final fun getMetadata ()Ljava/util/List;
 	public final fun setMetadata (Ljava/util/List;)V
 }

misk-admin/src/main/kotlin/misk/web/metadata/all/AllMetadataAction.kt

@@ -11,7 +11,7 @@ import misk.web.metadata.Metadata
 
 @Singleton
 class AllMetadataAction @Inject constructor(
-  private val metadata: List<Metadata>
+  private val metadata: Map<String, Metadata>
 ) : WebAction {
   @Get("/api/all/metadata")
   @RequestContentType(MediaTypes.APPLICATION_JSON)
@@ -21,5 +21,5 @@ class AllMetadataAction @Inject constructor(
     return Response(all = metadata)
   }
 
-  data class Response(val all: List<Metadata>)
+  data class Response(val all: Map<String, Metadata>)
 }

misk-admin/src/main/kotlin/misk/web/metadata/all/AllMetadataModule.kt

@@ -2,7 +2,7 @@ package misk.web.metadata.all
 
 import misk.inject.KAbstractModule
 import misk.web.WebActionModule
-import misk.web.metadata.Metadata
+import misk.web.metadata.MetadataModule
 import misk.web.metadata.config.ConfigMetadataProvider
 import misk.web.metadata.database.DatabaseHibernateMetadataProvider
 import misk.web.metadata.webaction.WebActionsMetadataProvider
@@ -21,12 +21,9 @@ class AllMetadataModule : KAbstractModule() {
   override fun configure() {
     install(WebActionModule.create<AllMetadataAction>())
 
-    // Any module can bind metadata to be exposed by the AllMetadataAction
-    newMultibinder<Metadata>()
-
     // Built in metadata
-    multibind<Metadata>().toProvider(ConfigMetadataProvider())
-    multibind<Metadata>().toProvider(DatabaseHibernateMetadataProvider())
-    multibind<Metadata>().toProvider(WebActionsMetadataProvider())
+    install(MetadataModule(ConfigMetadataProvider()))
+    install(MetadataModule(DatabaseHibernateMetadataProvider()))
+    install(MetadataModule(WebActionsMetadataProvider()))
   }
 }

misk-admin/src/main/kotlin/misk/web/metadata/config/ConfigMetadata.kt

@@ -1,25 +1,27 @@
 package misk.web.metadata.config
 
-import com.google.inject.Provider
 import jakarta.inject.Inject
 import misk.config.AppName
 import misk.config.MiskConfig
 import misk.resources.ResourceLoader
 import misk.web.metadata.Metadata
+import misk.web.metadata.MetadataProvider
 import misk.web.metadata.jvm.JvmMetadataAction
 import wisp.deployment.Deployment
 
 data class ConfigMetadata(
   val resources: Map<String, String?>
-) : Metadata(id = "config", metadata = resources)
+) : Metadata(metadata = resources)
 
-class ConfigMetadataProvider : Provider<ConfigMetadata> {
+class ConfigMetadataProvider : MetadataProvider<ConfigMetadata> {
   @Inject @AppName private lateinit var appName: String
   @Inject private lateinit var deployment: Deployment
   @Inject private lateinit var config: wisp.config.Config
   @Inject private lateinit var jvmMetadataAction: JvmMetadataAction
   @Inject private lateinit var mode: ConfigMetadataAction.ConfigTabMode
 
+  override val id: String = "config"
+
   override fun get() = ConfigMetadata(
     resources = generateConfigResources(appName, deployment, config)
   )

misk-admin/src/main/kotlin/misk/web/metadata/database/DatabaseHibernateMetadata.kt

@@ -1,14 +1,17 @@
 package misk.web.metadata.database
 
-import com.google.inject.Provider
 import jakarta.inject.Inject
 import misk.web.metadata.Metadata
+import misk.web.metadata.MetadataProvider
 
 data class DatabaseHibernateMetadata(
   val hibernate: List<DatabaseQueryMetadata>
-) : Metadata(id = "database-hibernate", metadata = hibernate)
+) : Metadata(metadata = hibernate)
 
-class DatabaseHibernateMetadataProvider : Provider<DatabaseHibernateMetadata> {
+class DatabaseHibernateMetadataProvider : MetadataProvider<DatabaseHibernateMetadata> {
   @Inject lateinit var metadata: List<DatabaseQueryMetadata>
+
+  override val id: String = "database-hibernate"
+
   override fun get() = DatabaseHibernateMetadata(hibernate = metadata)
 }

misk-admin/src/test/kotlin/misk/web/metadata/all/AllMetadataActionTest.kt

@@ -19,11 +19,11 @@ class AllMetadataActionTest {
   @Test
   fun `happy path`() {
     val actual = action.getAll()
-    val actualIds = actual.all.map { it.id }
-    assertEquals(listOf("config", "database-hibernate", "web-actions"), actualIds)
+    val actualIds = actual.all.keys
+    assertEquals(setOf("config", "database-hibernate", "web-actions"), actualIds)
 
     // Config
-    val actualConfig = actual.all.single { it.id == "config" }
+    val actualConfig = actual.all["config"]!!
     assert((actualConfig.metadata as Map<String, String?>).contains("JVM"))
     assert((actualConfig.metadata as Map<String, String?>).contains("Effective Config"))
     assertEquals(
@@ -86,11 +86,11 @@ class AllMetadataActionTest {
 
     // Database Hibernate Metadata
     // TODO maybe add a DB to the test so that assertions can confirm Database Hibernate metadata is included
-    val actualDatabaseHibernate = actual.all.single { it.id == "database-hibernate" }
+    val actualDatabaseHibernate = actual.all["database-hibernate"]!!
     assertEquals(listOf(), (actualDatabaseHibernate.metadata as List<DatabaseQueryMetadata>))
 
     // Web Action Metadata
-    val actualWebActionsMetadata = actual.all.single { it.id == "web-actions" }
+    val actualWebActionsMetadata = actual.all["web-actions"]!!
     assertEquals(
       """
         WebActionMetadata(name=StatusAction, function=fun misk.web.actions.StatusAction.getStatus(): misk.web.actions.StatusAction.ServerStatus, packageName=misk.web.actions, description=null, functionAnnotations=[@misk.web.Get(pathPattern="/_status"), @misk.web.ResponseContentType({"application/json;charset=utf-8"}), @misk.security.authz.Unauthenticated(), @misk.web.AvailableWhenDegraded()], requestMediaTypes=[*/*], responseMediaType=application/json;charset=utf-8, parameterTypes=[], parameters=[], requestType=null, returnType=misk.web.actions.StatusAction.ServerStatus, responseType=null, types={}, responseTypes={}, pathPattern=/_status, applicationInterceptors=[], networkInterceptors=[misk.web.interceptors.GunzipRequestBodyInterceptor, misk.web.interceptors.InternalErrorInterceptorFactory${'$'}Companion${'$'}INTERCEPTOR${'$'}1, misk.web.interceptors.RequestLogContextInterceptor, misk.web.interceptors.MetricsInterceptor, misk.web.exceptions.ExceptionHandlingInterceptor], httpMethod=GET, allowedServices=[], allowedCapabilities=[])

misk-config/api/misk-config.api

@@ -94,8 +94,15 @@ public final class misk/resources/TestingResourceLoaderModule : misk/inject/KAbs
 }
 
 public class misk/web/metadata/Metadata {
-	public fun <init> (Ljava/lang/String;Ljava/lang/Object;)V
-	public final fun getId ()Ljava/lang/String;
+	public fun <init> (Ljava/lang/Object;)V
 	public final fun getMetadata ()Ljava/lang/Object;
 }
 
+public final class misk/web/metadata/MetadataModule : misk/inject/KAbstractModule {
+	public fun <init> (Lmisk/web/metadata/MetadataProvider;)V
+}
+
+public abstract interface class misk/web/metadata/MetadataProvider : com/google/inject/Provider {
+	public abstract fun getId ()Ljava/lang/String;
+}
+

misk-config/build.gradle.kts

@@ -9,6 +9,7 @@ plugins {
 }
 
 dependencies {
+  api(libs.guice)
   api(libs.jacksonAnotations)
   api(libs.jacksonDatabind)
   api(libs.jakartaInject)
@@ -18,7 +19,6 @@ dependencies {
   api(project(":misk-inject"))
   implementation(libs.apacheCommonsLang3)
   implementation(libs.guava)
-  implementation(libs.guice)
   implementation(libs.jacksonCore)
   implementation(libs.jacksonDataformatYaml)
   implementation(libs.jacksonJsr310)

misk-config/src/main/kotlin/misk/web/metadata/Metadata.kt

@@ -1,8 +1,6 @@
 package misk.web.metadata
 
 open class Metadata(
-  /** Unique identifier for the type of metadata. Ie. "web-actions" or "config". */
-  val id: String,
   /** Metadata object, should be a data class for easy built-in serialization to JSON. */
   val metadata: Any,
 )

misk-config/src/main/kotlin/misk/web/metadata/MetadataModule.kt

@@ -0,0 +1,11 @@
+package misk.web.metadata
+
+import misk.inject.KAbstractModule
+
+/** Installs a new Metadata type with associated provider to expose in [AllMetadataAction]. */
+class MetadataModule<T: Metadata>(private val provider: MetadataProvider<T>): KAbstractModule() {
+  override fun configure() {
+    val binder = newMapBinder<String, Metadata>()
+    binder.addBinding(provider.id).toProvider(provider)
+  }
+}

misk-config/src/main/kotlin/misk/web/metadata/MetadataProvider.kt

@@ -0,0 +1,8 @@
+package misk.web.metadata
+
+import com.google.inject.Provider
+
+interface MetadataProvider<T: Metadata> : Provider<T> {
+  /** Unique identifier for the type of metadata. Ie. "web-actions" or "config". */
+  val id: String
+}

misk-rate-limiting-bucket4j-mysql/src/main/kotlin/misk/ratelimiting/bucket4j/mysql/MySQLBucket4jRateLimiterModule.kt

@@ -37,7 +37,7 @@ class MySQLBucket4jRateLimiterModule @JvmOverloads constructor(
   fun providedRateLimiter(
     clock: Clock,
     injector: Injector,
-    meterRegistry: MeterRegistry
+    meterRegistry: MeterRegistry,
   ): RateLimiter {
     val dataSourceService = injector.getInstance(keyOf<DataSourceService>(qualifier))
     val sqlConfiguration = SQLProxyConfiguration.builder()

misk/api/misk.api

@@ -2189,10 +2189,11 @@ public final class misk/web/metadata/webaction/WebActionsMetadata : misk/web/met
 	public fun toString ()Ljava/lang/String;
 }
 
-public final class misk/web/metadata/webaction/WebActionsMetadataProvider : com/google/inject/Provider {
+public final class misk/web/metadata/webaction/WebActionsMetadataProvider : misk/web/metadata/MetadataProvider {
 	public fun <init> ()V
 	public synthetic fun get ()Ljava/lang/Object;
 	public fun get ()Lmisk/web/metadata/webaction/WebActionsMetadata;
+	public fun getId ()Ljava/lang/String;
 }
 
 public final class misk/web/proxy/OptionalBinder {

misk/src/main/kotlin/misk/web/metadata/webaction/WebActionsMetadataProvider.kt

@@ -5,13 +5,17 @@ import jakarta.inject.Inject
 import jakarta.inject.Singleton
 import misk.web.jetty.WebActionsServlet
 import misk.web.metadata.Metadata
+import misk.web.metadata.MetadataProvider
 
 data class WebActionsMetadata(
   val webActions: List<WebActionMetadata>
-) : Metadata(id = "web-actions", metadata = webActions)
+) : Metadata(metadata = webActions)
 
 @Singleton
-class WebActionsMetadataProvider : Provider<WebActionsMetadata> {
+class WebActionsMetadataProvider : MetadataProvider<WebActionsMetadata> {
   @Inject private lateinit var servletProvider: Provider<WebActionsServlet>
+
+  override val id: String = "web-actions"
+
   override fun get() = WebActionsMetadata(servletProvider.get().webActionsMetadata)
 }