Allow empty query params for V3 filtering & Fix build for Windows

[radito3]

When creating a V3 request with an empty string as a list element, it is ignored and the filter is not applied to the CC API request.
e.g.:

CloudFoundryClient client = /* ... */;
client.routesV3()
      .list(ListRoutesRequest.builder()
           .spaceGuid(/* ... */)
           .path("") //this is ignored
           .build())
      .block(); //returns all routes in the space, not filtered by path

The CC API allows filtering with empty strings, so it makes sense that the Java client should too.
e.g.:
The request <cf-api>/v3/routes?space_guids=<guid>&paths= is valid

This PR adds support for filtering with empty strings.

It also fixes building on Windows, as some tests were not platform-agnostic and/or broken for Windows.

[dmikusa]

Hi, thanks so much for the PR. A couple of questions.

1. I believe this change is not only going to allow setting an empty path, but it's also going to allow setting empty values for query params everywhere. If it were isolated to this path scenario, I'd merge this right away. I need to investigate this more because it has the potential to impact other areas, possibly allowing invalid queries to be generated by the client.

2. In regards to the Windows part of the PR. Can you expand on what happened there? I don't have a Windows environment to build on, so I can't see what was not working for you. I'd like it to build properly on Windows, but it's not an OS that we use. Locally I'm on a Mac and our CI is on Linux. What part of the PR addresses this issue? and why is that necessary to make it build on Windows?

Thanks

[dmikusa]

I was looking at #1 a little more and my thought would be to modify the FilterParameter annotation such that it has a property like "boolean allowEmpty". FilterBuilder is processing the annotations so it should be able to grab that metadata & use it.

The benefit with this approach is that you could at the property level indicate which values are allowed to be empty. You could retain the existing behavior for everything except the property where you need to allow this.

[radito3]

Hello,
Thanks for the quick response!

Regarding the first point, I agree with you. The boolean flag in the FilterParameter annotation would be a quick and clean solution. I did a quick test and found out that all (that I tested) fields, except for the metadata label_selector, permit empty query params.
Should I add this with a default of true and set the metadata to false or the other way around and only set it to true for the routes example which is the one I need?

As for the second point, when I was building with maven (with jdk 8), it failed on ApplicationManifestUtilsTest because, in one test, the manifest was incorrectly formed and in others, the mock path for an application was being compared in a platform-specific way (it was comparing /some-path with (resolved on windows) C:\some-path).
Fixing these small test issues enables the build to complete on Windows.

[radito3]

Upon further inspection, I found out that all query filter params that allow for multiple values (all that are plural, e.g. names, paths, space_guids) allow empty values. Or, in terms of code, all that return a Collection.
And all that allow a single value (or singular, e.g. label_selector) don't.
So, with that in mind, would you reconsider my current solution or would you prefer I add the allowsEmpty method?

[radito3]

@dmikusa-pivotal What do you think?

[dmikusa]

Sorry, I got pulled away to a different issue.

As for the second point, when I was building with maven (with jdk 8), it failed on ApplicationManifestUtilsTest because, in one test, the manifest was incorrectly formed and in others, the mock path for an application was being compared in a platform-specific way (it was comparing /some-path with (resolved on windows) C:\some-path).
Fixing these small test issues enables the build to complete on Windows.

Looks good to me.

Upon further inspection, I found out that all query filter params that allow for multiple values (all that are plural, e.g. names, paths, space_guids) allow empty values. Or, in terms of code, all that return a Collection.
And all that allow a single value (or singular, e.g. label_selector) don't.
So, with that in mind, would you reconsider my current solution or would you prefer I add the allowsEmpty method?

OK, perfect. I think that fits with what you submitted in the PR. I'll go ahead and merge this.