When there is one QueryParam and you don't sent any QueryParam the router doesn't find the route

[cristiangarciascmspain]

example:

@Path(value = "/api/meme", method = HttpMethod.GET)
    public Observable<Void> getMeme(
            HttpServerRequest<ByteBuf> request,
            HttpServerResponse<ByteBuf> response,
            @QueryParam(value = "text", defaultValue = "", required = true) String text) {

it works with:
http://..../api/meme?text=hello

but NOT with:
http://..../api/meme

[cristiangarciascmspain]

It fails also when you have 2 QueryParam, if you don't put both on the request it doesn't find the route.

[victuxbb]

Thanks @cristiangarciascmspain, tomorrow I will fix it and I will take care of the tests :)

[cristiangarciascmspain]

The key of the problem seems to be the defaultValue="", when we used other value it worked.

[ghost]

It seems that if you set both the QueryParam as required and defaultValue="" it fails and it's OK IMO.

You set that QueryParam as required but you don't set a defaultValue (empty string is not a valid required value I think) so it expects to fail, isn't it?

[cristiangarciascmspain]

IMHO an empty string is a valid value.

It's common to have a default with an empty string, because if not you'll have a null.

The other thing is the incoherence having both default and required, the behaviour should be documented, because to me, if I have a default it should work, and for the library if it's required the default doesn't matter.

[ghost]

It seems strange for me to declare a required parameter that it accepts empty string as valid value, but maybe it's me :P

[cristiangarciascmspain]

Remove the required, can't I set a defaultValue="" ?
In that case you are forcing me to handle the null.

[cristiangarciascmspain]

Maybe this issue should be split into:

• Document the behaviour when you have both required and defaultValue.
• Accept an empty String as defaultValue.

[ghost]

If the param is not required for me defaultValue="" is valid.
If the param is required I think defaultValue="" is wrong.

[cristiangarciascmspain]

Strange to have two different behaviours, but anyway without the required is sending a null instead of an empty String.

[enriclluelles]

IMO these are two different issues:

• "" should be handled the same as other strings, it shouldn't be special
• When a parameter is required, defaultValue doesn't mean anything(in fact it shouldn't be used, but it's hard to check for that right now), so if the request doesn't contain the parameter, it is a bad request

[victuxbb]

Ok then, hands up for the next solution:

If param required is set and given queryparam doesn't exist in the uri -> 400 bad request
If param required is not set and given queryparam doesn't exist in the uri -> set default value

Currently we only need to fix a null value in query param when default=""

Ok?

[enriclluelles]

I agree with the solution proposed, but I fail to understand this:

Currently we only need to fix a null value in query param when default=""

If the defaultValue is set to "" it shouldn't be null like it was when I debugged the code, do we agree on that?

[victuxbb]

Yes, exactly, it shouldn't be null, currently we are returning null because of this:

@Override
  public String getParameterValue() throws ParamAnnotationException {
    List<String> values = queryParams.get(queryParam.value());

    String queryParamValue = null;
    if (values != null && !values.isEmpty()) {
      queryParamValue = values.get(0);
    }
    if (queryParamValue == null) {
      if (!QueryParam.DEFAULT_VALUE.equals(queryParam.defaultValue())) {
        queryParamValue = queryParam.defaultValue();
      } else if (queryParam.required()) {
        throw new QueryParamRequiredNotFoundException(queryParam.value());
      }
    }

    return queryParamValue;
  }

I'm working on fix it...

[enriclluelles]

👏 👏

[victuxbb]

PR #5