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

Add PathFilter processor to filter paths #1613

Merged
merged 2 commits into from
Jul 2, 2024

Conversation

DerManoMann
Copy link
Collaborator

@DerManoMann DerManoMann commented Jun 30, 2024

Fixes #1609

Allows to filter on either tags or paths via configurable regexps.

Example using PHP:

     // ...
     (new Generator())
         ->setConfig([
             'pathInfo' => [
                 'tags' => ['/pet/', '/store/'],
                 'paths => ['/api\/v2/'],
             ]
         ])

Command line would look something like

./bin/openapi -c pathFilter.tags[]=/pet/ -c cleanUnusedComponents.enabled=true app/*

@DerManoMann DerManoMann marked this pull request as draft June 30, 2024 09:31
@DerManoMann DerManoMann marked this pull request as ready for review July 2, 2024 01:22
@DerManoMann DerManoMann merged commit b308ec0 into zircote:master Jul 2, 2024
21 checks passed
@DerManoMann DerManoMann deleted the path-filter-ii branch July 2, 2024 03:04
@caixingyue
Copy link

caixingyue commented Jul 5, 2024

@DerManoMann

Hi,There are still several problems with the filter

  1. When all interfaces are filtered out, their labels still exist, but they are empty after being expanded.

  2. The same class name cannot coexist. For example, if I have WebhookResource in both Admin and Api responses, there will be a conflict.

@DerManoMann
Copy link
Collaborator Author

  1. What do you mean with interfaces? Maybe an example would help. Sample code is always good as it can be easily used to build new tests...
  2. Not sure there is an easy solution for this. I suppose filtering would have to happen a lot earlier for this and that would be a completely new pattern in swagger-php; might be a useful addition, though.

@caixingyue
Copy link

caixingyue commented Jul 5, 2024

  1. What do you mean with interfaces? Maybe an example would help. Sample code is always good as it can be easily used to build new tests...
image
new Processors\PathFilter([], ['/api/']),
new Processors\CleanUnusedComponents(true),

Please look at the blue line. It is expected that these two lines will not be displayed because the related interfaces have been filtered out due to filtering.

@caixingyue
Copy link

caixingyue commented Jul 5, 2024

  1. Not sure there is an easy solution for this. I suppose filtering would have to happen a lot earlier for this and that would be a completely new pattern in swagger-php; might be a useful addition, though.

At present, I have found such a method to replace it.
#[OA\Response(response: 200, description: 'Success', content: new OA\JsonContent(ref: WebhookResource::class))]
#[Schema(schema: 'ApiMerchantWebhookResource', title: 'WebhookResource')]

If you can filter it before generating the document, there will be only one of the two, and no exception will occur.

@caixingyue
Copy link

@DerManoMann There is still a problem. In our actual use, we may have different names and version numbers for different interface documents. Currently, only the following unified comments are supported:

#[OA\Info(version: '1.0', description: 'This is provided for partners to use, and is not supported for merchants and other roles', title: 'Partner API Documentation')]

@DerManoMann
Copy link
Collaborator Author

What does unified comments mean? How does the info attribute have a relationship with path items or tags?
Do you mean you have also multiple version attributes?
What are the rules about which one to pick then?
Sounds like a lot more than just filtering...

@caixingyue
Copy link

I wrote a JAVA Swagger configuration file for you, I think this should help me express

@Configuration
public class SwaggerConfiguration {

    @Bean
    public GroupedOpenApi partnerApi() {
        return GroupedOpenApi.builder()
                .group("partner")
                .pathsToMatch("/api/**")
                .addOpenApiCustomizer(openApi -> {
                    openApi.getInfo().setTitle("Partner API");
                    openApi.getInfo().setVersion("1.1");
                    openApi.getInfo().setDescription("This interface document is only provided to Xingrui's internal partners. Some interfaces require relevant permissions to be opened.");
                })
                .addOperationCustomizer((operation, handlerMethod) -> {
                    operation.addParametersItem(new Parameter()
                            .in(ParameterIn.HEADER.toString())
                            .name(HttpHeaders.AUTHORIZATION)
                            .description("Authorization token")
                            .required(true)
                            .schema(new StringSchema())
                    );

                    return operation;
                })
                .addOperationCustomizer(globalResponseMessages())
                .build();
    }

    @Bean
    public GroupedOpenApi merchantApi() {
        return GroupedOpenApi.builder()
                .group("merchant")
                .pathsToMatch("/merchantApi/**")
                .addOpenApiCustomizer(openApi -> {
                    openApi.getInfo().setTitle("Merchant API");
                    openApi.getInfo().setVersion("1.0");
                    openApi.getInfo().setDescription("This interface document is provided to all merchants. Some interfaces require relevant permissions to be enabled.");
                })
                .addOperationCustomizer((operation, handlerMethod) -> {
                    operation.addParametersItem(new Parameter()
                            .in(ParameterIn.HEADER.toString())
                            .name(HttpHeaders.AUTHORIZATION)
                            .description("Authorization token")
                            .required(true)
                            .schema(new StringSchema())
                    );

                    return operation;
                })
                .addOperationCustomizer(globalResponseMessages())
                .build();
    }

    @Bean
    public OperationCustomizer globalResponseMessages() {
        return (operation, handlerMethod) -> {
            operation.getResponses().addApiResponse("404", new ApiResponse().description("Resource not found").content(new Content().addMediaType(
                    MediaType.APPLICATION_JSON_VALUE,
                    new io.swagger.v3.oas.models.media.MediaType().schema(new StringSchema()).example("Resource not found")
            )));

            return operation;
        };
    }
}

@DerManoMann
Copy link
Collaborator Author

Yeah, I was wondering if things would get there :)

Something like this is currently way out of scope of what swagger-php does. It is, for the most part, just a annotation/attribute processor to convert those into an OpenAPI spec.

Programmatic feature like the above would be a new paradigm. Having said that, you could add the actual OA\Info programmatically, I suppose. Although, I think you'd have to jump through some hoops right now.

As for the other questions - maybe it is worth filing new, separate issues for that. In particular the one about the filtering not working as expected.

Finally, your duplicate WebhookResource - I am still unclear how the code would know which version to pick. However, you could experiment with manually moving the two processors to the top of the pipeline so see if it makes a difference.

@caixingyue
Copy link

Yeah, I was wondering if things would get there :)

Something like this is currently way out of scope of what swagger-php does. It is, for the most part, just a annotation/attribute processor to convert those into an OpenAPI spec.

Programmatic feature like the above would be a new paradigm. Having said that, you could add the actual OA\Info programmatically, I suppose. Although, I think you'd have to jump through some hoops right now.

As for the other questions - maybe it is worth filing new, separate issues for that. In particular the one about the filtering not working as expected.

Finally, your duplicate WebhookResource - I am still unclear how the code would know which version to pick. However, you could experiment with manually moving the two processors to the top of the pipeline so see if it makes a difference.

Hi~

I think this may be quite different from the existing swagger-php in global configuration, but I think excellent evolution is necessary, and a global configuration file can be added while retaining the original solution.

For this I created a new issue

@caixingyue
Copy link

The filtering did not meet my expectations, so I created the corresponding issue

@caixingyue
Copy link

caixingyue commented Jul 8, 2024

Finally, your duplicate WebhookResource - I am still unclear how the code would know which version to pick. However, you could experiment with manually moving the two processors to the top of the pipeline so see if it makes a difference.

When you don't know which WebhookResource to choose, if you provide a class, there will be a corresponding namespace. You can filter based on this. If both are used, then an exception will continue to be thrown.

Because the documents are different, only one will be left.

#[OA\Response(response: 200, description: 'Success', content: new OA\JsonContent(ref: App\Http\Controllers\Merchant\WebhookResource::class))]

#[OA\Response(response: 200, description: 'Success', content: new OA\JsonContent(ref: App\Http\Controllers\Partner\WebhookResource::class))]

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

Successfully merging this pull request may close these issues.

Unable to group
2 participants