-
Notifications
You must be signed in to change notification settings - Fork 4
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
feat: handle root security requirements on root DSL #101
feat: handle root security requirements on root DSL #101
Conversation
d456b6d
to
17df020
Compare
edf6910
to
4a37ddd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Just minor comments, could you also have a look at the CI failure?
I'll take care of updating the docs once this is merged :)
...l-openapi/tegral-openapi-dsl/src/main/kotlin/guru/zoroark/tegral/openapi/dsl/OperationDsl.kt
Outdated
Show resolved
Hide resolved
tegral-openapi/tegral-openapi-dsl/src/main/kotlin/guru/zoroark/tegral/openapi/dsl/PathDsl.kt
Outdated
Show resolved
Hide resolved
...l-openapi/tegral-openapi-ktor/src/test/kotlin/guru/zoroark/tegral/openapi/ktor/PluginTest.kt
Outdated
Show resolved
Hide resolved
Hi ! sorry for the delay. |
Done! You can also run |
No problem ! awesome work you did here already. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #101 +/- ##
============================================
+ Coverage 89.64% 89.73% +0.08%
Complexity 24 24
============================================
Files 202 203 +1
Lines 4568 4587 +19
Branches 441 442 +1
============================================
+ Hits 4095 4116 +21
+ Misses 310 308 -2
Partials 163 163 ☔ View full report in Codecov by Sentry. |
269552d
to
59bf2b8
Compare
Ah, didn't know the "long method" check was enabled in tests as well. You can go ahead and |
I added the annotation. I don't know why I didn't get it when running locally though. Maybe because it failed fast due the certificate error. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks a lot for your contribution! I'll update the docs in a follow-up PR.
I didn't find how to add security requirements at the root Open API definition as the spec expect it.
I extracted a
SecurityDsl
in order to reuse it in theRootDsl
and theOperationDsl
as security requirements can be defined at root level and at operation level.I also added the possibility to make AND/OR security requirements combinations (as documented here).
Fixes #106