Allow binding custom StringConverters

## Description

Currently params had to be one of the supported types, or expose a
`valueOf` function. However at Faire, we want to offer better type
validation for our parameters. For example, we have `Token<T>` where `T`
could be something like `Brand` or `Product`. Each of these token types
has a unique prefix, so even if we have a `valueOf` function exposed we
lack the ability to validate the type is correct. With this change we
can inspect the type of the parameter, and validate it's the correct
type before converting the `String`.

Note that this is technically a breaking change because I've changed
`StringConverter` to an interface, and updated the public constructor of
`FeatureBindingFactory`, but they're small enough changes that I don't
think people will have any issues upgrading.

## Testing Strategy

I updated `QueryParamFeatureBindingTest` to show that a custom
`StringConverter` can be used to properly parse a string to a value
class, which otherwise would fail. I didn't update all the other binding
tests because the code path is the same, but can update them if people
feel strongly.

GitOrigin-RevId: ce50018634417ae91eef5dbba8d7e6d7c58c9b1f

Author: frojasg

.gitignore

@@ -21,6 +21,7 @@ lib
 dist
 .hash
 hooks.gradle
+.kotlin
 
 **/site/
 docs/0.x/*

misk/api/misk.api

@@ -1296,7 +1296,11 @@ public final class misk/web/FeatureBinding$DefaultImpls {
 }
 
 public abstract interface class misk/web/FeatureBinding$Factory {
-	public abstract fun create (Lmisk/Action;Lmisk/web/PathPattern;Lmisk/web/FeatureBinding$Claimer;)Lmisk/web/FeatureBinding;
+	public abstract fun create (Lmisk/Action;Lmisk/web/PathPattern;Lmisk/web/FeatureBinding$Claimer;Ljava/util/List;)Lmisk/web/FeatureBinding;
+}
+
+public final class misk/web/FeatureBinding$Factory$DefaultImpls {
+	public static synthetic fun create$default (Lmisk/web/FeatureBinding$Factory;Lmisk/Action;Lmisk/web/PathPattern;Lmisk/web/FeatureBinding$Claimer;Ljava/util/List;ILjava/lang/Object;)Lmisk/web/FeatureBinding;
 }
 
 public abstract interface class misk/web/FeatureBinding$Subject {
@@ -1857,8 +1861,17 @@ public final class misk/web/extractors/RequestBodyException : java/io/IOExceptio
 	public fun <init> (Ljava/lang/Throwable;)V
 }
 
+public abstract interface class misk/web/extractors/StringConverter {
+	public abstract fun convert (Ljava/lang/String;)Ljava/lang/Object;
+}
+
+public abstract interface class misk/web/extractors/StringConverter$Factory {
+	public abstract fun create (Lkotlin/reflect/KType;)Lmisk/web/extractors/StringConverter;
+}
+
 public final class misk/web/extractors/StringConverterKt {
-	public static final fun converterFor (Lkotlin/reflect/KType;)Lkotlin/jvm/functions/Function1;
+	public static final fun converterFor (Lkotlin/reflect/KType;Ljava/util/List;)Lmisk/web/extractors/StringConverter;
+	public static synthetic fun converterFor$default (Lkotlin/reflect/KType;Ljava/util/List;ILjava/lang/Object;)Lmisk/web/extractors/StringConverter;
 }
 
 public final class misk/web/formatter/ClassNameFormatter {

misk/src/main/kotlin/misk/grpc/GrpcFeatureBinding.kt

@@ -15,6 +15,7 @@ import misk.web.FeatureBinding.Claimer
 import misk.web.FeatureBinding.Subject
 import misk.web.PathPattern
 import misk.web.WebConfig
+import misk.web.extractors.StringConverter
 import misk.web.mediatype.MediaTypes
 import java.lang.reflect.Type
 import kotlin.coroutines.CoroutineContext
@@ -137,7 +138,8 @@ internal class GrpcFeatureBinding(
     override fun create(
       action: Action,
       pathPattern: PathPattern,
-      claimer: Claimer
+      claimer: Claimer,
+      stringConverterFactories: List<StringConverter.Factory>
     ): FeatureBinding? {
       if (action.dispatchMechanism != DispatchMechanism.GRPC) return null
 

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

@@ -2,6 +2,7 @@ package misk.web
 
 import misk.Action
 import misk.web.actions.WebAction
+import misk.web.extractors.StringConverter
 import okio.BufferedSink
 import okio.BufferedSource
 import java.util.regex.Matcher
@@ -64,7 +65,8 @@ interface FeatureBinding {
     fun create(
       action: Action,
       pathPattern: PathPattern,
-      claimer: Claimer
+      claimer: Claimer,
+      stringConverterFactories: List<StringConverter.Factory> = listOf(),
     ): FeatureBinding?
   }
 

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

@@ -58,6 +58,7 @@ import misk.web.extractors.RequestCookiesFeatureBinding
 import misk.web.extractors.RequestHeaderFeatureBinding
 import misk.web.extractors.RequestHeadersFeatureBinding
 import misk.web.extractors.ResponseBodyFeatureBinding
+import misk.web.extractors.StringConverter
 import misk.web.extractors.WebSocketFeatureBinding
 import misk.web.extractors.WebSocketListenerFeatureBinding
 import misk.web.interceptors.BeforeContentEncoding
@@ -167,6 +168,7 @@ class MiskWebModule @JvmOverloads constructor(
 
     // Initialize empty sets for our multibindings.
     newMultibinder<NetworkInterceptor.Factory>()
+    newMultibinder<StringConverter.Factory>()
     newMultibinder<ApplicationInterceptor.Factory>()
     newMultibinder<StaticResourceEntry>()
     newMultibinder<WebProxyEntry>()

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

@@ -6,6 +6,7 @@ import okio.BufferedSink
 import okio.BufferedSource
 import java.util.regex.Matcher
 import jakarta.inject.Inject
+import misk.web.extractors.StringConverter
 import kotlin.reflect.KParameter
 
 /** HTTP binding as specified by [FeatureBinding]. */
@@ -220,15 +221,16 @@ internal class WebActionBinding @Inject constructor(
 
   /** Creates feature bindings for use before and after a web call. */
   class Factory @Inject constructor(
-    private val featureBindingFactories: List<FeatureBinding.Factory>
+    private val featureBindingFactories: List<FeatureBinding.Factory>,
+    private val stringConverterFactories: List<StringConverter.Factory>,
   ) {
     fun create(
       action: Action,
       pathPattern: PathPattern
     ): WebActionBinding {
       val claimer = RealClaimer(action)
       for (factory in featureBindingFactories) {
-        val binding = factory.create(action, pathPattern, claimer)
+        val binding = factory.create(action, pathPattern, claimer, stringConverterFactories)
         claimer.commitClaims(factory, binding)
       }
       return claimer.newWebActionBinder()

misk/src/main/kotlin/misk/web/extractors/FormAdapter.kt

@@ -40,23 +40,28 @@ internal class FormAdapter<T : Any> private constructor(
   ) {
     fun valuesToParameter(values: Collection<String>): Any? {
       if (isList) {
-        return values.map { converter?.invoke(it) }.toList()
+        return values.map { converter?.convert(it) }.toList()
       } else {
         val first = values.firstOrNull() ?: return null
-        return converter?.invoke(first)
+        return converter?.convert(first)
       }
     }
   }
 
   companion object {
     /** Returns an adapter for [kClass], or null if it cannot be adapted. */
-    fun <T : Any> create(kClass: KClass<T>): FormAdapter<T>? {
+    fun <T : Any> create(
+      kClass: KClass<T>,
+      stringConverterFactories: List<StringConverter.Factory>,
+    ): FormAdapter<T>? {
       val constructor = kClass.primaryConstructor ?: return null
-      val fields = constructor.parameters.map { it.toField() }
+      val fields = constructor.parameters.map { it.toField(stringConverterFactories) }
       return FormAdapter(constructor, fields)
     }
 
-    private fun KParameter.toField(): Field {
+    private fun KParameter.toField(
+      stringConverterFactories: List<StringConverter.Factory>,
+    ): Field {
       val annotation = findAnnotation<FormField>()
       val name = annotation?.name?.lowercase()
         ?: name?.lowercase()
@@ -70,7 +75,10 @@ internal class FormAdapter<T : Any> private constructor(
         isOptional,
         type.isMarkedNullable,
         isList,
-        converterFor(if (isList) type.arguments.first().type!! else type)
+        converterFor(
+          if (isList) type.arguments.first().type!! else type,
+          stringConverterFactories,
+        )
       )
     }
   }

misk/src/main/kotlin/misk/web/extractors/FormValueFeatureBinding.kt

@@ -24,13 +24,14 @@ internal class FormValueFeatureBinding<T : Any>(
     override fun create(
       action: Action,
       pathPattern: PathPattern,
-      claimer: Claimer
+      claimer: Claimer,
+      stringConverterFactories: List<StringConverter.Factory>
     ): FeatureBinding? {
       val parameter = action.parameterAnnotatedOrNull<FormValue>() ?: return null
       if (parameter.type.classifier !is KClass<*>) return null
 
       val kClass = parameter.type.classifier as KClass<*>
-      val formAdapter = FormAdapter.create(kClass) ?: return null
+      val formAdapter = FormAdapter.create(kClass, stringConverterFactories) ?: return null
       claimer.claimRequestBody()
       claimer.claimParameter(parameter)
       return FormValueFeatureBinding(parameter, formAdapter)

misk/src/main/kotlin/misk/web/extractors/PathParamFeatureBinding.kt

@@ -26,17 +26,20 @@ internal class PathParamFeatureBinding private constructor(
   ) {
     fun bind(subject: Subject) {
       val pathParam = subject.pathMatcher.group(patternIndex + 1)
-      subject.setParameter(parameter, converter.invoke(pathParam))
+      subject.setParameter(parameter, converter.convert(pathParam))
     }
   }
 
   companion object Factory : FeatureBinding.Factory {
     override fun create(
       action: Action,
       pathPattern: PathPattern,
-      claimer: Claimer
+      claimer: Claimer,
+      stringConverterFactories: List<StringConverter.Factory>
     ): FeatureBinding? {
-      val bindings = action.parameters.mapNotNull { it.toParameterBinding(pathPattern) }
+      val bindings = action.parameters.mapNotNull {
+        it.toParameterBinding(pathPattern, stringConverterFactories)
+      }
       if (bindings.isEmpty()) return null
 
       for (binding in bindings) {
@@ -46,14 +49,17 @@ internal class PathParamFeatureBinding private constructor(
       return PathParamFeatureBinding(bindings)
     }
 
-    private fun KParameter.toParameterBinding(pathPattern: PathPattern): ParameterBinding? {
+    private fun KParameter.toParameterBinding(
+      pathPattern: PathPattern,
+      stringConverterFactories: List<StringConverter.Factory>
+    ): ParameterBinding? {
       val annotation = findAnnotation<PathParam>() ?: return null
       val name = if (annotation.value.isBlank()) name else annotation.value
 
       val patternIndex = pathPattern.variableNames.indexOf(name)
       if (patternIndex == -1) return null
 
-      val converter = converterFor(type)
+      val converter = converterFor(type, stringConverterFactories)
         ?: throw IllegalArgumentException("cannot convert path parameters to $type")
 
       return ParameterBinding(patternIndex, this, converter)

misk/src/main/kotlin/misk/web/extractors/QueryParamFeatureBinding.kt

@@ -43,8 +43,8 @@ internal class QueryParamFeatureBinding private constructor(
       if (values.isEmpty()) return null
 
       try {
-        return if (isList) values.map { converter(it) }
-        else converter(values.first())
+        return if (isList) values.map { converter.convert(it) }
+        else converter.convert(values.first())
       } catch (e: IllegalArgumentException) {
         throw BadRequestException("Invalid format for parameter: $name", e)
       }
@@ -55,9 +55,10 @@ internal class QueryParamFeatureBinding private constructor(
     override fun create(
       action: Action,
       pathPattern: PathPattern,
-      claimer: Claimer
+      claimer: Claimer,
+      stringConverterFactories: List<StringConverter.Factory>
     ): FeatureBinding? {
-      val bindings = action.parameters.mapNotNull { it.toQueryBinding() }
+      val bindings = action.parameters.mapNotNull { it.toQueryBinding(stringConverterFactories) }
       if (bindings.isEmpty()) return null
 
       for (binding in bindings) {
@@ -67,13 +68,15 @@ internal class QueryParamFeatureBinding private constructor(
       return QueryParamFeatureBinding(bindings)
     }
 
-    internal fun KParameter.toQueryBinding(): ParameterBinding? {
+    internal fun KParameter.toQueryBinding(
+      stringConverterFactories: List<StringConverter.Factory>,
+    ): ParameterBinding? {
       val annotation = findAnnotation<QueryParam>() ?: return null
       val name = if (annotation.value.isBlank()) name!! else annotation.value
 
       val isList = type.classifier?.equals(List::class) ?: false
       val elementType = if (isList) type.arguments.first().type!! else type
-      val stringConverter = converterFor(elementType)
+      val stringConverter = converterFor(elementType, stringConverterFactories)
         ?: throw IllegalArgumentException("Unable to create converter for $name")
 
       return ParameterBinding(this, isList, stringConverter, name)