Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cloudfoundry-client: V3.securitygroups _ListSecurityGroupsRequest extends the wrong pagination request #1242

Open
SaifuddinMerchant opened this issue Oct 7, 2024 · 6 comments

Comments

@SaifuddinMerchant
Copy link

Version 3 List Security Groups is extending the wrong version of pagination request
import org.cloudfoundry.client.v2.PaginatedRequest;

See _ListSecurityGroupsRequest.java#L17

Error when using the client to make a ListSecurityGroupRequest

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)

The generated class ListSecurityGroupsRequest also has the wrong imports, but I am guessing if we fix the import in _ListSecurityGroupsRequest.java that will automatically fix the generated code

CF Java Client Version: 5.12.2.RELEASE
CF API version: 3.166.0

@Lokowandtg
Copy link

Will look into this.

@Lokowandtg
Copy link

Hi @SaifuddinMerchant,
can you provide some code to reproduce the error? It would best fit into
integration-test/src/test/java/org/cloudfoundry/client/v3/SecurityGroupsTest.java

At least some (failing) skeleton would be great, as I was not able to reproduce your error.

Thanks, Georg

@SaifuddinMerchant
Copy link
Author

SaifuddinMerchant commented Jan 16, 2025

Hi,

I will provide another comment that will have the test case in the Integration Test format. Here a general Rest API way to reproduce the error


package xyz;

import net.jpmchase.gaia.usage.client.CloudFoundryClientFactory;
import org.cloudfoundry.client.CloudFoundryClient;
import org.cloudfoundry.client.v3.securitygroups.ListSecurityGroupsRequest;
import org.cloudfoundry.client.v3.securitygroups.ListSecurityGroupsResponse;
import org.cloudfoundry.client.v3.securitygroups.SecurityGroupResource;
import org.cloudfoundry.util.PaginationUtils;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.RestController;
import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;

@RestController
public class SecurityGroupPaginationTest {

  public final CloudFoundryClient cloudFoundryClient;

  public SecurityGroupPaginationTest(final CloudFoundryClientFactory cloudFoundryClientFactory) {
    this.cloudFoundryClient = cloudFoundryClientFactory.cfClient();
  }

  @GetMapping("/listNoPagination")  //This works
  public String listNoPagination() {
    final Mono<ListSecurityGroupsResponse> list =
        cloudFoundryClient.securityGroupsV3().list(ListSecurityGroupsRequest.builder().build());
    System.out.println(list.block());
    return "Done";
  }

  @GetMapping("/listWithPagination") //This does not work
  public String listWithPagination() {

    final Flux<SecurityGroupResource> securityGroupResourceFlux =
        PaginationUtils.requestClientV3Resources(
            page -> {
              return cloudFoundryClient
                  .securityGroupsV3()
                  .list(ListSecurityGroupsRequest.builder().page(page).resultsPerPage(100).build());
            });
    System.out.println(securityGroupResourceFlux.blockFirst());
    return "Done";
  }
}

// Exception message for listWithPagination is ...
/*
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'] with root cause

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'
*/

@SaifuddinMerchant
Copy link
Author

SaifuddinMerchant commented Jan 16, 2025

Here is a stab at the failing integration test. I have validated this code compiles, I have not run the integration test as I can't get the code to compile locally

import org.cloudfoundry.util.PaginationUtils;

@Test
  public void listWithPagination() {
    this.securityGroup
        .map(
            securityGroup ->
                PaginationUtils.requestClientV3Resources(
                    page ->
                        cloudFoundryClient
                            .securityGroupsV3()
                            .list(
                                ListSecurityGroupsRequest.builder()
                                    .page(page)
                                    .resultsPerPage(1)
                                    .names(Arrays.asList(securityGroup.getName()))
                                    .build())))
        .as(StepVerifier::create)
        .expectNextCount(1)
        .expectComplete()
        .verify(Duration.ofMinutes(5));
  }

@SaifuddinMerchant
Copy link
Author

SaifuddinMerchant commented Jan 16, 2025

If possible, I would like to submit the fix (one line change in import) and the updated integration test via a pull request. However I do not have an empty cloud foundry installation. I am able to verify the fix with the Rest End point test that I added to my own code base.

Could I submit a PR without running the integration test locally? I'm hoping to understand the process so I can contribute other changes in the further

@Lokowandtg
Copy link

You can create a pull request without testing the code beforehand. Just label / comment the pull request accordingly. If the purpose is to transfer some code sample, you should clearly state that you do not want this change to be merged.
In other projects I have seen a "do not merge" label for this purpose. Here I would simply write it to the comment.

Thanks, Georg

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants