Sorry, something went wrong.
| } | ||
|
|
||
| @Test | ||
| public void listWithPagination() { |
There was a problem hiding this comment.
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?
Sorry, something went wrong.
There was a problem hiding this comment.
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
Sorry, something went wrong.
There was a problem hiding this comment.
Got it.
The test is incorrect, and is not testing what's intended, because when you do
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 gives you a Flux<SecurityGroupResource>, and all calls are made. By doing that, I'm getting the error with the v2 pagination:
Could you please update the test?
Sorry, something went wrong.
There was a problem hiding this comment.
Greatly Appreciate the feedback and help, I will make the change
Sorry, something went wrong.
There was a problem hiding this comment.
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
Sorry, something went wrong.
There was a problem hiding this comment.
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.
Sorry, something went wrong.
There was a problem hiding this comment.
Thank you for your contribution!
I'll take another look at the list() tests separately, thanks for flagging them.
Sorry, something went wrong.
|
This is now in the SNAPSHOT repository: https://repo.spring.io/ui/native/snapshot/org/cloudfoundry/cloudfoundry-client-reactor/5.14.0.BUILD-SNAPSHOT/ |
Sorry, something went wrong.
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