It seems the change is not in validation but in how the empty query param is treated
Before (in 3.17-), ?a= meant a="", in main it's the same as not having a
This makes the validation constraint fail since there is a default value.

I have another failing test on a similar case where ?e= and e is a list before it was a List.of("") and now it's a List.of().

THis started to fail on Nov 18 and was working on Nov 15

here is a new reproducer clearly showing that empty query params are now handled differently:

reproducer-empty-queryparam.zip

/cc @FroMage (rest), @stuartwdouglas (rest)

This change was intentional, and explained in #42468 (comment)

If you have use-cases that require the old behaviour, let us know, because we could not think of any.

Thanks for spotting that @FroMage!

I had completely forgot about that one...

@FroMage let's close this when this is in the migraition guide

Thanks @FroMage @geoand @cescoffier

@FroMage @geoand @cescoffier
I maintain a huge old legacy system... we actually use the empty string to signal something different than a null.

Think of this as a form of mutation that is submittet... null means no processing required here (ie leave it untouched)... empty string means delete it.. thats the logic that the system has implemented 😄

How this is totally broken with 3.18.x

This also is a result of using generated Rest Clients. As the methods generated have all params, and this forces the developers to fill something in.. so a lot of nulls normally for all the optional params. The REST clients dont send nulls, but they do send empty strings

I had a look at what initially triggered the issue. From what I could see, it wasn't related to strings and then the fix affected strings to improve consistency.

I agree it's not exactly easy to find a consistent way of doing things but I must admit I'm a bit worried about this change for strings.
Do we know how other REST layers behave? RESTEasy Classic and Spring?

Take these two examples:

path?myQueryParam=&anotherParam=foo
and
path?anotherParam=foo

For me there is a big difference in those two urls, with the first having the intention of submitting a value for that param. So myQueryParam should not be null in both (and IMO any @default should only kick in on the second one)

The original behaviour was mostly unintentional, non-specified, and not consistent.

You can still obtain the information that the query included one key with no value from the Vert.x request.

If there are multiple users who want to obtain this information, we could add a new API as a new feature to get that info, but so far I think this is the first time someone asks for that.

Using that Vert.x request as a workaround might be easy enough. But finding all places, to use this workaround, that now breaks in 100's of endpoints will be a major pain here

Sorry :(

You could also turn these String parameters into a new type OptionalString via a ParamConverter, and make that type have set/empty/not-set states. But that would still mean rewriting the endpoints, so not much more helpful.

Or… perhaps you can write a request filter that turns foo= into foo=<marker> and then you can refactor your calls from StringUtil.isEmpty(param) into one that treats <marker> specially?

Would it be possible with a config to enable legacy behavior and some kind of log message that we can look for and fix? I much rather fix the code correct than doing lots of workarounds.. and just make 3.19.x the cutoff?

In which case do you expect a log message? Runtime based on the values passed?

A flag to enable legacy behaviour, in itself, would not help us find all the places where this impacts us.

Any time a param would be null and not empty string, it should emit a log message. " will be null with new behaviour on endpoint ". We would still need to analyse the exact impact on that endpoint, but it will likely reduce the number of endpoints greatly that we have to go over

So let me rephrase this: are you looking for a build-time warning whenever we see a String parameter, or a runtime warning when we see an empty string parameter value?

When we encounter query parameters like this:
path?myQueryParam=&anotherParam=foo

It could issue a message that myQueryParam is no longer an empty string but null.. at runtime yes
This will help alot with finding all the places where this breaking change with query parameters have an effect at runtime. Because one of the big issues with this breaking change, is that the compiler cannot catch this change... so you get the suprise at runtime.. likely as a production issue for future me 😄

Now, I think, there should be a flag to enable this, so its not spamming these messages at default

@olivierbeltrandocintoo: find comments inline in bold:

The , Separator is across languages the separator.

Not for HTTP parameters, which REST is really standardised on. We do support it, but in addition to the HTTP semantics, not as a replacement for it.

As for your primitive table, I am not sure if you meant int (the primitive) or Integer (the boxed type).

In any case, deserialising foo= to something else than 0 (for primitives) or null (for non-String types) [which is what's happening ATM] would also break a lot of existing code, and we have to remember we're all here because I broke existing code and we should not fix this by breaking other existing code, especially when the reason is not very convincing.

Throwing a 400 when the type is incompatible (say a for an int or 23 for a Date) is fine. But throwing a 400 for an empty value (foo=) for either a primitive or a non-String type would be hard to justify.

Closed by accident.

I'm making progress on this. I have the non-parameter-container version ready already. Hopefully by the end of the week I can get parameter containers to also work. Perhaps.

This is a bit all over the place. I thought I could afford to copy the code for Optional since it's also a wrapper type, but it's special-cased to support types like set/list but for example, not arrays. So ATM Optional<String[]> does not work. I'm not fixing this, will wait for someone else to raise the issue, but the support for wrapper types is not recursive properly in that all the types that are wrapped by it are special-cased :(

Optional<String[]>

Nevermind, thinking about this, this type makes no sense to support 😅

We're hitting the same issue here, but with the over-complication that we can't change the API because it's auto-generated by the OpenApi extension. Let me explain further this use case:

• We have an API spec that uses a Query param which is an Enum type:

  /api/rhsm-subscriptions/v1/capacity/products/{product_id}/{metric_id}:
    description: 'Operations for capacity report for a given account and product'
    parameters:
      - name: product_id
        in: path
        required: true
        schema:
          $ref: "#/components/schemas/ProductId"
        description: "The ID for the product we wish to query"
    get:
      summary: "Fetch a capacity report for an account and product."
      operationId: getCapacityReportByMetricId
      parameters:
        - name: usage
          in: query
          schema:
            $ref: '#/components/schemas/UsageType'
          description: "Include only capacity for the specified usage level."

Where the UsageType is:

    UsageType:
      description: "Describes the usage that the report was made against. Not set if it wasn't
          specified as a query parameter."
      type: string
      enum: [ "", Production, Development/Test, Disaster Recovery, _ANY ]

The autogenerated API is:

@RegisterRestClient
@RegisterProvider(ApiExceptionMapper.class)
@Path("/api/rhsm-subscriptions/v1/capacity/products/{product_id}/{metric_id}")
public interface CapacityApi  {

    /**
     * Fetch a capacity report for an account and product.
     *
     */
    @GET
    
    @Produces({ "application/vnd.api+json" })
    CapacityReportByMetricId getCapacityReportByMetricId(@PathParam("product_id") ProductId productId, @QueryParam("usage") UsageType usage) throws ApiException, ProcessingException;
}

We use a custom param converter provider (see implementation here) but in Quarkus, this converter is not even called because of this line.

• We are migrating some services that use this API spec from Spring Boot to Quarkus. In Spring Boot, this worked fine perfectly because when the query "usage" is unset, it returns null, and when the query "usage" is set with empty value, it returns the UsageType.EMPTY.
• In Quarkus, it always return null.

Note that I don't follow how a query param that always returns null regardless if it was unset or empty goes in the direction of being more consistent... (I didn't follow all the conversation though), but unless there is a workaround that we could apply for all query params, this is blocking us to migrate our services to Quarkus :(

Thanks @Sgitario, I had not thought about openapi generation and custom parameter converters that actually do support empty values.

I was working hard to complete support for the Parameter<T> type which allows differentiating between absent/empty/set values, and I have it working for query parameters. I was going to add support for form parameters too, because that seems equally logical to allow it there (let's deal with cookies, matrix, path and headers in the future), but ran into issues due to multipart having specific code paths (as always). So I would not have enough time to fix this before I go on PTO.

Now, given what you said, I'm inclined to propose a different strategy:

• We revert the changes that led to removing empty values. So we'd be back to null | "" | "..." for strings, [], [""], ["..."] for collections of strings, and 0 (absent value) | 0 (empty value) | 0 (set value) for primitives (:puke:) and null (absent value) | <exception> (empty value) | 2025 (set value) for dates and other types whose converter does not allow empty values.
• We still introduce Parameter<T> for people who want to deal with empty values for any kind of type: string, primitives, dates in a sane and consistent manner
• We document the usage of Parameter if anybody wants to deal with empty values, making sure that regular users don't fall into the madness that is the JAX-RS default in those cases

The problem is that this will likely have to wait for my return in three weeks :-/

WDYT @geoand ?

We revert the changes that led to removing empty values

I am not sure this is a good thing to do TBH as I believe it will lead to even more confusion

The way I see it, we should just include the new API when we have it (the sooner the better) in a next release of 3.20 LTS and tell people that really rely on the old (undocumented) behavior to wait

I see your point. That's still a possibility.

However, I don't know how we can deal with openapi generators, it's likely they won't be able to target this new API. Although it depends on whether openapi can distinguish element types and whether they allow for empty values or not.

I used to think only String allowed empty values (as opposed to say primitives, or date/time) but the example with a custom parameter converter that does support empty values was not one I had thought about.

With the current behaviour, and even the addition of the Parameter<T> API, this sort of custom parameter converter won't be possible, because:

• With the current behaviour, we remove empty values, so the parameter converter isn't called
• With the Parameter<CustomParam> API, the framework will detect the empty parameter and make a special Parameter.empty() instance which won't call the parameter converter. So the fact that it's empty is actionable by the user. but it's further up the chain. Well, perhaps it doesn't matter too much.

I don't see a way to please everyone to be honest and I am -1 for causing even more confusion by flip-flopping

@FroMage many thanks for the explanation about what originally motivated the changes. I reverted the changes where we switched this API to Quarkus, so our team can wait for a fix for 3 weeks (also I'm happy to help here).

I don't see a way to please everyone to be honest and I am -1 for causing even more confusion by flip-flopping

@geoand since the original behaviour changed and there are no alternatives, I see this like a major regression issue. Note that many users might have not noticed this yet, but expect query params to be treated like null and empty values differently.

@geoand since the original behaviour changed and there are no alternatives, I see this like a major regression issue. Note that many users might have not noticed this yet, but expect query params to be treated like null and empty values differently.

But there will be soon.

Note that many users might have not noticed this yet, but expect query params to be treated like null and empty values differently.

I'm sympathetic to that, but we also had users that expected the exact opposite (which is why we changed it). The fact is that the old behavior was an emergent property of the implementation, not something that was thought out or specified.

not something that was thought out or specified.

Even when it was not thought or specified like this, it's how it was working before and how users have been using Quarkus so far.
To give more arguments, empty and null values really mean different things in an API viewpoint: Consider an API endpoint that filters users by name:

• ?name= (empty) might mean “search for users with no name.”
• ?name=null might mean “ignore the name filter.” or “use default behavior.”

Changing that, it's about forcing users/devs to start using optionals (the ones that noticed this critical change in behaviour) and it's against the consistency and stability :/

there are no alternatives

There are workarounds, currently. We've documented one here: #44885 (comment)

?name=null might mean “ignore the name filter.” or “use default behavior.”

I assume you mean …? (no query param name nor value)

Changing that, it's about forcing users/devs to start using optionals

I'm not sure what you mean about optionals here, but I don't think it's the java.lang.Optional type. I guess you refer to an extra parameter to specify whether you want an empty query parameter as opposed to an absent one.

I don't see a way to please everyone to be honest and I am -1 for causing even more confusion by flip-flopping

I am sympathetic to that, though I suppose it depends a bit on how this can be adapted for OpenAPI generators, perhaps @phillip-kruger knows if in open-api we can detect that some query parameter types allow for an empty value or not (as opposed to an absent state).

I would rather do as you suggest and move on from the previous behaviour. But I'm genuinely surprised by the number of reactions to this change. I did not expect that. I also did not expect that some parameter converters allowed for empty values.

So, I'm a bit torn between both options. And when torn, I would tend to trust @geoand's opinion. Who knows, perhaps when I come back in three weeks we'll even have a clear path forward :)

Changing that, it's about forcing users/devs to start using optionals

I undestood from the thread, that another solution was to start using java.lang.Optional for the query parameters that needed to differentiate between null and empty.

But say optionals, or say implement something like this: #44885 (comment). It's forcing users for an implementation change that they might not even be aware of that this changed.

I did not expect that

Same!

/cc @MikeEdgar

I undestood from the thread, that another solution was to start using java.lang.Optional for the query parameters that needed to differentiate between null and empty.

Ah OK. Well, that's the new Parameter<T> API. Optional only has two states: empty or set, we're not using null values for Optional types because that leads to madness. Parameter<T> has three states (and is never null itself, just like Optional values): absent/cleared/set.

You do need a new API, but on the other hand, the previous behaviour:

• did not work for non-String types (except the very special case of having a custom parameter converter which did handle empty values) such as primitives or date/time
• led to frankly silly things such as [""]: list with one element containing an empty string to indicate "cleared collection"
• was not used by a lot of use-cases: while many people showed up here to complain, far more users did not. The ability to differenciate between a cleared and absent value is not extremely common, hence why nobody even noticed you could not do that for non-String types (* minus the exception noted above)