feat: Implement parsing and generation of OpenAPI security schemes and security requirements#342
feat: Implement parsing and generation of OpenAPI security schemes and security requirements#342NicolasThierion wants to merge 4 commits intokbuntrock:devfrom
Conversation
…d requirements from annotations.
kbuntrock
left a comment
kbuntrock
left a comment
There was a problem hiding this comment.
Thank you very much, this MR looks pretty good to me!
I left a few comments on your MR, but nothing significant! 😉
| | | `required` | `boolean` | Whether the parameter is required | | ||
| | | `schema` | `Schema` | Parameter schema | | ||
| | | `example` | `String` | Example value | | ||
| | `SecurityScheme` | `name` | `String` | Security Scheme name | |
There was a problem hiding this comment.
If you want you can also add this in the french version here :
docs/i18n/fr/docusaurus-plugin-content-docs/current/advanced_topics/swagger_annotations.md
(since I suspect you're also pretty fluent in french 😊)
| // in is an enum, so we use its toString() method | ||
| // only set it if it's not the DEFAULT or empty enum value | ||
| String inStr = inOpt.get().toString(); | ||
| if(!StringUtils.isEmpty(inStr) && !"DEFAULT".equals(inStr)) { |
There was a problem hiding this comment.
Looking at the implementation of the enumeration (v2.2.30), the enum DEFAULT is linked to the empty string value.
Unless you are aware of a different implementation in another version, "DEFAULT" will never be equal to the value returned by toString(). Better to choose here one of the two method : comparaison of the value or the name.
| .filter(e -> methodIdentifier.equals(e.getIdentifier())) | ||
| .collect(Collectors.toList()); | ||
| for(Endpoint endpoint : endpointsForMethod) { | ||
| readSecurityRequirements(mergedAnnotations, endpoint.getSecurityRequirements()); |
There was a problem hiding this comment.
To avoid matching things a posteriori, I would prefer that the reading of securityRequirement be done at the same time the other endpoint information are collected.
I suggest moving the readSecurityRequirements function to the AbstractLibraryReader class so that it can be called for the class from JavaClassAnalyser (as it is now), and inside the readers for each endpoint.
I think its call could be added to the AbstractLibraryReader#setSwaggerAnnotatedEndpointProperties function.
And this block would disappear.
| import com.fasterxml.jackson.annotation.JsonInclude; | ||
|
|
||
| @JsonInclude(JsonInclude.Include.NON_NULL) | ||
| public class SecurityScheme { |
There was a problem hiding this comment.
SecurityScheme is use in a couple of Set but do not define an equals function.
I guess comparing instance on the name is what we want?
Or let it like this but use List instead of Set in the readSecuritySchemes functions.
| } | ||
|
|
||
| private void readSecuritySchemes(MergedAnnotations mergedAnnotations, Set<SecurityScheme> securitySchemes) { | ||
| Map<String, SecurityScheme> map = new LinkedHashMap<>(); |
There was a problem hiding this comment.
In this function, you silently secure the addition of multiple SecurityScheme with the same name. But later you throw a runtime exception on merging the schemes.
Better to homogenise how we handle this.
But an exception is a bit extreme for this functionality in my opinion. I would prefer a warning log since I guess Spring do not care and launch the app anyway.
You can still use the result of the map addition to log a warn, or use directly the destination Set addition if you opted to implement the equals function on SecurityScheme.
|
|
||
| public void addSecurityScheme(SecurityScheme scheme) { | ||
| if(securitySchemes.containsKey(scheme.getName())) { | ||
| throw new RuntimeException("Multiple SecurityScheme with the same name (" + scheme.getName() + ") are defined."); |
There was a problem hiding this comment.
I would prefer a warning log here instead of the exception.
|
Hello @NicolasThierion Would you happen to know when you might have time to continue this MR? (No pressure at all. 😅) |
|
Hello Kévin.
I had no time recently to work neither on my own project, nor this PR, but I'll look into it as soon as possible, and I hope it will be ready before your next release.
It likely won't happen until this sunday. I'll keep you in touch.
…________________________________
De : Kévin Buntrock ***@***.***>
Envoyé : lundi 16 mars 2026 22:36
À : kbuntrock/openapi-maven-plugin ***@***.***>
Cc : Nicolas THIERION ***@***.***>; Mention ***@***.***>
Objet : Re: [kbuntrock/openapi-maven-plugin] feat: Implement parsing and generation of OpenAPI security schemes and security requirements (PR #342)
[https://avatars.githubusercontent.com/u/15209500?s=20&v=4]kbuntrock left a comment (kbuntrock/openapi-maven-plugin#342)<#342 (comment)>
Hello @NicolasThierion<https://github.com/NicolasThierion>
Would you happen to know when you might have time to continue this MR? (No pressure at all. 😅)
It’s just that I’m considering making a release and possibly including this feature.
—
Reply to this email directly, view it on GitHub<#342 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABOUX227IRK6PQ44WZKFDWD4RBXXFAVCNFSM6AAAAACWJNKWFCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DANZQG4ZTEMJTGE>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
2 participants
Use this checklist to help us review and merge your contribution quickly and smoothly:
mvn verifyto confirm that the basic checks pass.A more thorough validation will be executed automatically on your pull request.
English documentation files can be found in
docs/docs.Disclamer: I’m not very familiar with this project yet, so I used some AI assistance to help implement this feature. I have thoroughly tested the results and everything looks good on my end, but please let me know if any parts of the code seem out of place or inconsistent with the project's standards.