[BUG] Option of CommaSeparated has a strange behavior when no value is provided

[fmeriaux]

Tapir version: *** 1.10.10

Scala version: *** 2.13

Describe the bug

I need to have an optional list of parameters. This list comes from a query parameter that is either not present in the query at all, defined without a value, or defined with a comma-separated list of values.

Use case:

For example, let's take the parameter "sort" and "desc", which controls the sorting of an API result. Sort indicates the list of fields to be sorted, in order, and desc indicates the direction. If desc is:

1. absent from the query, then sorting is ascending.
2. present without value, then the sort is descending on all fields.
3. present with value, then sorting is ascending except on fields listed in desc.

This use case is in fact an implementation of a design proposed by Octo on the sorting of a REST API.

I then expect to have to declare something like:

 query[Option[CommaSeparated[String]]]("desc")
          .map(_.map(_.values))(values => values.map(Delimited.apply))

In reality I have to do:

 query[Option[CommaSeparated[String]]]("desc")
          .map(_.map(_.values.filterNot(_.isBlank)))(values => values.map(Delimited.apply))

What is the problem?

The problem is that in the case where the parameter is supplied without a value, I should have Some(Nil) as the decoded value, but I actually have Some(List("")). And if the codec type is not a string but an enumeration or something else, then the decoding return an error for invalid value.

For my opinion, this result is not what we can expect when we declare a query input as "option of list".

What do you think about that ?

[adamw]

I think the current design is consistent with how non-comma-separated parameters are represented. That is, if you have a query[String]("x"), and the server receives ?x=, I think it's quite natural that the value here is the empty string. I don't think there could be another design choice, in fact.

If "empty value" is a valid input it should be properly handled by the server logic - in your case, it's discarding/using a default value, in others it might be quite different.

[fmeriaux]

I agree with your example of a non-list value. It's actually quite simple, I imagine, with Tapir, to validate the String and set a minimum length if necessary.

However, in the case of an argument with a list, I don't think it's the responsibility of the logic, but of the codec.
When we declare as input, via Tapir, that we want a List Option, we don't expect to have Some(List("")) as real input.
If we take the case of CommaSeparated with an enum, we don't get the empty string, but a parsing error (it tried to parse the empty string as an enum).

That's why I think that managing non-blank in a codec option list string, in the comma separated, or a little earlier (I don't know tapir well enough), seems to me to make sense and be of value to the library.

This would make it possible to manage my case, but also the case with an enum, because you won't get a parsing error if you supply ?x=. If you want an error in this case, you can do it cleanly, by declaring a validator that checks that at least one value is supplied.

That's my opinion, but after all it's up to you :)

Thank you for your reply.

[adamw]

I'd say that if I declare that I want a list of comma-separated values parsed as an enum, and the user passes ?x=, that's an invalid parameter on their part? The default-value-if-empty-parameter-provided seems to be quite a special case