softwaremill / tapir

Rapid development of self-documenting APIs
https://tapir.softwaremill.com
Apache License 2.0
1.34k stars 405 forks source link

[Question] Handling empty query params #4010

Open cptwunderlich opened 1 week ago

cptwunderlich commented 1 week ago

I am using Tapir in combination with an HTMX frontend. Sending a GET request from an input field, HTMX will include the input as a query parameter, even if the value is empty, in some cases - it seems like this is purposefully replicating HTML form behavior. E.g., I have a date input and use the date pickers "clear" button to reset the value. HTMX will issue a request to mypath?date=. Tapir does not like this, I get a 400 - Invalid value for: query parameter date.

My endpoint looks something like this:

baseSiteEndpoint.get
      .in(header[Option[Boolean]]("HX-Request"))
      .in("mypath")
      .in(query[Option[LocalDate]]("date"))

I would have expected this to yield a None. My IDE suggests, that this query param uses Codec.listHeadOption and I assume with Codec.localDate as underlying codec. This seems to try to do something similar to strings.map(LocalDate.parse).headOption, in the sense that the LocalDate.parse fails bc. it might receive an empty string.

This does not work:

  implicit val localDateOpt: Codec[String, Option[LocalDate], TextPlain] =
    Codec.string.map(s => if (s.isEmpty) None else Some(LocalDate.parse(s)))(o => o.map(DateTimeFormatter.ISO_LOCAL_DATE.format).getOrElse(""))
      .schema(Schema.schemaForLocalDate.asOption)

I don't understand the codec selection fully, so I'm a bit at a loss. Also, I wonder if this behavior by Tapir is correct? It seems like many servers do accept query params of that sort?


Tapir version: 1.10.10 Scala version: 2.13.14

adamw commented 1 week ago

The problem here is that Tapir is quite simplistic in how it handles query parameters: a ?date= query parameter has the value of "" (an empty string). So it's there, but it's empty. And an empty string is a valid value. Hence the value passed to LocalDate.parse is an empty string - which results in an error. Hence the error result.

How to fix this? I see three options.

First, by providing an explicit codec. The query method requires the following codec:

def query[T: Codec[List[String], *, TextPlain]](name: String): EndpointInput.Query[T]

It needs a codec which converts a List[String] (query parameters might appear multiple times in an URL) to the target type T. So you've been close when you tried to provide the value implicit codec by hand. The one that compiler generates is Codec.listHeadOption(Codec.localDate).

In our custom one, we need to take the raw list of query parameters, filter out the empty ones, and then copy the logic from Codec.listHeadOption:

given Codec[List[String], Option[LocalDate], CodecFormat.TextPlain] = Codec
    .list[String, String, CodecFormat.TextPlain]
    .map(_.filter(_.nonEmpty))(identity)
    .mapDecode {
      case Nil     => DecodeResult.Value(None)
      case List(e) => Codec.localDate.decode(e).map(Some(_))
      case l       => DecodeResult.Multiple(l)
    }(v => v.map(Codec.localDate.encode).toList)
    .schema(Codec.localDate.schema.asOption)

The first .map filters out the empty query parameters, the second .mapDecode performs the parsing. Tested - works :)

The second solution would be more general, if there's a lot of such query parameters. You could create a RequestInterceptor which would override the query parameters, removing any query parameters with empty values. This, however, might be too broad - as sometimes you do want to differentiate between an empty query parameter and an absent query parameter.

Finally thirdly, I think we could add an attribute, so that you could instruct the server interpreter's decoder to treat empty values as absent. This would be sth like: query[Option[LocalDate]].attribute(EmptyAsAbsent, true). It would require changes in Tapir's codebase, though.

Let me know what you think!

adamw commented 1 week ago

@fmeriaux I think this is related to https://github.com/softwaremill/tapir/issues/3890, maybe the third option (query attribute) would fix your problem? Then an empty query parameter value could be treated the same as no-value.

cptwunderlich commented 1 week ago

Hi Adam, thank you for your quick and thorough response!

So, for my upcoming demo, the codec will do ^^ However, for the application as a whole, this might be error prone (accidentally not importing the custom codec and a date filter won't work).

I now better understand how the query param handling in Tapir works. This approach of building up codecs composes well, but unfortunately, it's blind to the final type. Bc. parsing an empty string "" fails - of course it does. But when the expected result is not a LocalDate, but a Option[LocalDate] I would expect None and not a parser error. After all, manually I might do something like Try(LocalDate.parse("")).toOption - but that would hide actually malformed date strings (?date=9999-9999-9999).

So yeah, not sure what the best design would be here. Or how other servers handle this. I would expect - for Option[LocalDate] - ?date= to result in None, ?date=2024-09-04 to give me Some(LocalDate("2024-09-04")) and ?date=afjadkfj to result in an error.

Attributes might be the easiest way to implement this. I saw that flag params got a special case with EndpointInput.Query.flagValue.

fmeriaux commented 1 week ago

There are indeed similarities with my issue, I'd have to see if the 3rd proposed solution would fix my issue, I don't know exactly what the behaviour will be with an Option[List[T]].

My need was to differentiate between the 3 possible cases :

A native codec that allows this is a flexible codec that allows the user to manage all cases according to their specific needs. It also means taking into account that semantically ‘absent’ and ‘empty’ are 2 different things. Today Tapir is confusing the two from my point of view.