-
Notifications
You must be signed in to change notification settings - Fork 11
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
Swagger Integration #240
Swagger Integration #240
Conversation
src/xiana/swagger.clj
Outdated
(-> routes | ||
helpers/routes->routes' | ||
((if (not (false? render?)) | ||
#(helpers/routes->swagger-data % :route-opt-map route-opt-map) | ||
identity)) | ||
((cond | ||
(= type :json) json/write-value-as-string | ||
(= type :edn) identity | ||
:else identity)))) |
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.
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.
That's not what I was meant to write actually. If you don't have any strong opinions, I would rather go with this way on that.
src/xiana/swagger.clj
Outdated
type :type | ||
route-opt-map :route-opt-map}] | ||
(if (swagger-configs-there? config) | ||
(let [routes (or routes (-> config :routes)) |
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.
Why the macro?
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.
Fixed with more accurate version, get
.
src/xiana/swagger.clj
Outdated
config-key (-> config | ||
(get :xiana/swagger {}) | ||
(get :path :swagger.json))] |
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.
config-key (-> config | |
(get :xiana/swagger {}) | |
(get :path :swagger.json))] | |
config-key (get-in config [:xiana/swagger :path] :swagger.json)] |
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.
That was helpful but evantually, it will grow like this because of the potential of the BL.
Also, fixed, for now.
src/xiana/swagger.clj
Outdated
(->> config | ||
((juxt :xiana/swagger-ui :xiana/swagger)) | ||
(reduce #(and % %2) true) | ||
boolean)) |
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.
(apply and ((juxt :xiana/swagger-ui :xiana/swagger) config))
or
(every? some? ((juxt :xiana/swagger-ui :xiana/swagger) config))
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.
(apply and ((juxt :xiana/swagger-ui :xiana/swagger) config))
is the right reflection of that s-expression without overseeing any future changes. Fixed, thanks.
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.
Also, throwing an exception due to and
is a macro. Going with the second option for a while. Thanks.
src/xiana/route/helpers.clj
Outdated
@@ -16,3 +23,73 @@ | |||
(assoc :error e) | |||
(assoc :response | |||
{:status 500 :body "Internal Server error"}))))) | |||
|
|||
(defonce all-methods |
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.
It should be part of xiana.swagger
ns, isn't it?
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.
I was seeing the future again, and experimenting with more advanced and growth code-base. So, for now, fixed.
I'd like to see the default swagger endpoints added to the app routes on route initialization at https://github.com/Flexiana/framework/blob/main/src/xiana/route.clj#L11 when the swagger feature is enabled in configuration. |
I was thinking on that too, but you are still ignoring the people who might want to configure or tweak their enpoints. I think we can give them some configuration options by our configuration system but if you think carefully there is still some things that we can't configure with this way. For now, we provide basic functionality also, less configuration. Let me finish and push it. So we can examine it. Edit: your point is adding them silently, only by configuration, I believe. It seems not right to me. I would love to hear different perspectives over it, first. |
Removing unwanted, development s-expressions from the PR.
ece3066
to
7d2f9db
Compare
Co-authored-by: Gustavo Valente <gustavo.valente@flexiana.com>
Title: Swagger Integration
Description
In this PR, simply aimed to implement the Swagger Integration in Xiana-way. I'm fully open to critics
Tester info
The tests are so naive, so let me write more tests.
Completion Checklist
Nothing, now.