Fixing pagination on v3._ListSecurityGroupsRequest

[SaifuddinMerchant]

v3._ListSecurityGroupsRequest extends the v2.PaginatedRequest which is incorrect. It needs to extend the v3.PaginatedRequest. The code fixes the incorrect package referenced in v3._ListSecurityGroupsRequest and adds a integration test case

Issue Link: #1242

[Kehrlann]

I am unsure whether this test really showcases #1242 .

When I revert the change to have a v2 paginated request, and change the test to use .resultsPerPage(1), it still passes on against CC API 3.191.0.

With .resultsPerPage(1), does it fail on your CF deployment?

[SaifuddinMerchant]

The failure I've seen is - results-per-page is a invalid parameter for V3 API.
This matches what I expect given the specs
https://v3-apidocs.cloudfoundry.org/version/3.191.0/index.html#list-security-groups

org.cloudfoundry.client.v3.ClientV3Exception: CF-UnprocessableEntity(10008): Unknown query parameter(s): 'results-per-page'. Valid parameters are: 'page', 'per_page', 'order_by', 'created_ats', 'updated_ats', 'guids', 'names', 'running_space_guids', 'staging_space_guids', 'globally_enabled_running', 'globally_enabled_staging'
at org.cloudfoundry.reactor.util.ErrorPayloadMappers.lambda$null$2(ErrorPayloadMappers.java:62)

I haven't been able to run the integration test stand alone, but I do get the issue above using resultsPerPage(x) in my application

[Kehrlann]

Got it.

The test is incorrect, and is not testing what's intended, because when you do

this.securityGroup
        .map(...)

This returns a Mono<Flux<SecurityGroupResource>>. By subscribing to this, through .verify(...), only the this.securityGroup request is ever called. The subsequent listPage is not called because nothing subscribes to the Flux it returns.

Instead, you should do:

this.securityGroup
        .flux()
        .flatMap(/* ... */)
        .as(StepVerifier::create)
        // ...

This gives you a Flux<SecurityGroupResource>, and all calls are made. By doing that, I'm getting the error with the v2 pagination:

[ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.835 s <<< FAILURE! -- in org.cloudfoundry.client.v3.SecurityGroupsTest
[ERROR] org.cloudfoundry.client.v3.SecurityGroupsTest.listWithPagination -- Time elapsed: 0.830 s <<< FAILURE!
java.lang.AssertionError: expectation "expectNextCount(1)" failed (expected: count = 1; actual: counted = 0; signal: onError(org.cloudfoundry.client.v3.ClientV3Exception: CF-UnprocessableEntity(10008): Unknown query parameter(s): 'results-per-page'. Valid parameters are: 'page', 'per_page', 'order_by', 'created_ats', 'updated_ats', 'guids', 'names', 'running_space_guids', 'staging_space_guids', 'globally_enabled_running', 'globally_enabled_staging'))

Could you please update the test?

[SaifuddinMerchant]

Greatly Appreciate the feedback and help, I will make the change

[SaifuddinMerchant]

I'm also going to try and see if I can run the actual integration test for this method on my installation, so I might take some time to do that update

[SaifuddinMerchant]

I have fixed the test based on your feedback. No change from what you suggested. I can't run the integration test module but I was able to run just the specific test case with some modifications from my own module.

P.S I had based my test case on public void list() that I think suffers from a similar issue aka the listSecurityGroup is never called.

[Kehrlann]

Thank you for your contribution!

I'll take another look at the list() tests separately, thanks for flagging them.

[Kehrlann]

This is now in the SNAPSHOT repository: https://repo.spring.io/ui/native/snapshot/org/cloudfoundry/cloudfoundry-client-reactor/5.14.0.BUILD-SNAPSHOT/