Skip to content

Support to customize path #1675

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

Closed
seoul-afternoon opened this issue May 25, 2022 · 9 comments
Closed

Support to customize path #1675

seoul-afternoon opened this issue May 25, 2022 · 9 comments
Labels
wontfix This will not be worked on

Comments

@seoul-afternoon
Copy link

I look forward to create each path for each query parameter although it is the same path.
However, the path is overwritten and omitted.

I want to create 4 various paths

  • /v1/security/credentials?action=changePassword
  • /v1/security/credentials?action=requestPasswordReset
  • /v1/security/credentials?action=resetPassword
  • /v1/security/credentials?action=remindUsername

But I created 1 various path

  • /v1/security/credentials

I think this is expected behavior for current open api spec.

But several open api spec users have been already experiencing this problem.
And it's been considered for a long time to be reflected in the next version.

In order to solve this problem, I would like to have an extension point to customize path.
I suggest adding PathCustomizer.

If you don't mind, I'll propose a PR about this.

I hope you consider this plan.

Thanks in advance for answering.

@bnasslahsen
Copy link
Collaborator

@seoul-afternoon,

You already have OpenApiCustomizer, where you can customize all the attributes of the OpenAPI object. (Path included).

@bnasslahsen bnasslahsen added the wontfix This will not be worked on label May 26, 2022
@seoul-afternoon
Copy link
Author

@bnasslahsen

Hi, I tried using OpenApiCustomizer.
This is not enough to customize I want.

When I have two methods

@GetMapping("/members/{memberNo}")
fun findMember(@PathVariable memberNo: Long, userName: String, age: Int)

@GetMapping("/members/{memberNo}", params = "filter=admin")
fun findAdmin(@PathVariable memberNo: Long, admin : Boolean)

I expected to get a document for two paths like this.

/members/{memberNo}
/members/{memberNo}?filter=admin

But I got a document for one path.

/members/{memberNo}

When I try to customize using OpenApiCustomizer,
the path is already combined, so it is not possible to customize as I want.

What should I do to be able to customize it as I want?

@bnasslahsen
Copy link
Collaborator

@seoul-afternoon,

What is your expected OpenAPI description, for the code you provided ?

@seoul-afternoon
Copy link
Author

seoul-afternoon commented Jun 8, 2022

@bnasslahsen

Actually, it's not OpenAPI's specification yet.
But API developers sometimes face that case.
In this issue, I linked the issue being discussed in the OpenAPI spec.
So, I want to points that can be customized.

@bnasslahsen
Copy link
Collaborator

bnasslahsen commented Jun 8, 2022

@seoul-afternoon,

If you want to customize the path, make sure you put what is exactly your expcted OpenAPI description. Otherwise, we have no way to help you.
Remember, contributions to this repository should follow its contributing guidelines and code of conduct.

@seoul-afternoon
Copy link
Author

@bnasslahsen

String[] produces = requestMappingInfo.getProducesCondition().getProducibleMediaTypes().stream().map(MimeType::toString).toArray(String[]::new);
String[] consumes = requestMappingInfo.getConsumesCondition().getConsumableMediaTypes().stream().map(MimeType::toString).toArray(String[]::new);
String[] headers = requestMappingInfo.getHeadersCondition().getExpressions().stream().map(Object::toString).toArray(String[]::new);
if ((isRestController(restControllers, handlerMethod, operationPath) || isActuatorRestController(operationPath, handlerMethod))
&& isFilterCondition(handlerMethod, operationPath, produces, consumes, headers)) {
Set<RequestMethod> requestMethods = requestMappingInfo.getMethodsCondition().getMethods();
// default allowed requestmethods
if (requestMethods.isEmpty())
requestMethods = this.getDefaultAllowedHttpMethods();
calculatePath(handlerMethod, operationPath, requestMethods, consumes, produces, headers, locale, openAPI);
}

I think we can extract String[] params here and bind them to RouterOperation.

@FunctionalInterface
public interface PathCustomizer {
    String customize(HandlerMethod handlerMethod, RouterOperation routerOperation, Locale locale);
}

And we can create a PathCustomizer interface and provide it to the user.

protected void calculatePath(HandlerMethod handlerMethod,
RouterOperation routerOperation, Locale locale, OpenAPI openAPI) {
String operationPath = routerOperation.getPath();
Set<RequestMethod> requestMethods = new HashSet<>(Arrays.asList(routerOperation.getMethods()));
io.swagger.v3.oas.annotations.Operation apiOperation = routerOperation.getOperation();
String[] methodConsumes = routerOperation.getConsumes();
String[] methodProduces = routerOperation.getProduces();
String[] headers = routerOperation.getHeaders();
Map<String, String> queryParams = routerOperation.getQueryParams();
Components components = openAPI.getComponents();
Paths paths = openAPI.getPaths();
Map<HttpMethod, Operation> operationMap = null;

And if there is a PathCustomizer implementation, We can pass it to the customized path when we build the operation path.

@bnasslahsen
Copy link
Collaborator

@seoul-afternoon,

You are not describing the test case why you need this change.
As explained earlier, you need to provide the expected OpenAPI description or minimal sample with a test case, that shows why this change is really needed.

Without these elements, there is no need to insist.

@junoyoon
Copy link

junoyoon commented Jun 9, 2022

@bnasslahsen

@seoul-afternoon will make PR for this soon.
We're not demanding this for u to make this.
We'll make the PR for this issue and ask u review if u don't mind.
Is it OK?

@bnasslahsen
Copy link
Collaborator

@junoyoon,

Feel free to propose a PR with the related unit tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

3 participants