Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Question] Handling empty query params #4010

Open
cptwunderlich opened this issue Sep 3, 2024 · 4 comments
Open

[Question] Handling empty query params #4010

cptwunderlich opened this issue Sep 3, 2024 · 4 comments

Comments

@cptwunderlich
Copy link
Contributor

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
Copy link
Member

adamw commented Sep 4, 2024

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
Copy link
Member

adamw commented Sep 4, 2024

@fmeriaux I think this is related to #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
Copy link
Contributor Author

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
Copy link
Contributor

fmeriaux commented Sep 4, 2024

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 :

  • None when query param is absent
  • List(Nil) when query param is present, without any character, (no validator of T called):
  • List(T1, T2...)) when query param is present with at least one character (validator of T passed)

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants