Skip to content

updated swagger ui#5

Closed
nhardman wants to merge 2 commits intomasterfrom
update-swaggerui
Closed

updated swagger ui#5
nhardman wants to merge 2 commits intomasterfrom
update-swaggerui

Conversation

@nhardman
Copy link
Copy Markdown

Include a up to date version of the swagger ui into Kitura-OpenAPI.
The swagger ui has been given a Kitura look with new colours and icons
Changes to the config to ensure that api and swaggerui locations can be altered whilst allowing the swaggerui to still find the correct api location.

@nhardman nhardman requested a review from ianpartridge June 29, 2018 12:27
Copy link
Copy Markdown
Contributor

@ianpartridge ianpartridge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to split up the update to SwaggerUI and the other fixes into separate PRs.

let indexPath = "file://" + swaggerUIInstallation + "/index.html"
if let indexLocation = URL(string: indexPath) {
do {
try rendered.write(to: indexLocation, atomically: true, encoding: .utf8)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this is a viable option - what if we are running in the cloud and can't write over the file?

}

router.get(path) {
var apipath = path
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of this you can do guard var above.

let template = "index.stencil"
let swaggerUIInstallation = sourcesDirectory + "/swaggerui"

let fsLoader = FileSystemLoader(paths: [Path(swaggerUIInstallation)])
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of using PathKit, would FileManager from Foundation do? That would be preferable.


if let rendered = try? environment.renderTemplate(name: template, context: context) {
router.get(uPath) { request, response, next in
response.send(String(describing: rendered))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use the new Codable templating we added in Kitura 2.4? https://developer.ibm.com/swift/2018/05/31/type-safe-templating/

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a play with this. There's a couple of problems:

  1. The template we are trying to serve is within the Kitura-OpenAPI dependency, not within the user's application, so the template is not in the usual place (./Views). And we don't want to mess with the user's Router.viewsPath since that would break their own templating.
  2. By using the KituraStencil templating (where we register a template engine), we're required to register a StencilTemplateEngine with the user's Router - which could conflict with their own code.

@@ -0,0 +1,299 @@
/**
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugh do we really have to copy and paste all this?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The alternative is to make KituraTest and KituraTestBuilder part of a separate project which is imported as a test dependency.

Neil and I discussed this, but because SPM does not have 'test dependencies', it would have the side-effect of introducing a common dependency to Kitura and (potentially) many other repos that depend on Kitura. Unlike other common dependencies (eg. LoggerAPI), we'd probably find ourselves needing to change the base Kitura test classes relatively frequently - eg. to add new functionality - which I anticipate would cause headaches for keeping our packages in a resolvable state.

@djones6
Copy link
Copy Markdown
Contributor

djones6 commented Aug 10, 2018

This will be delivered incrementally in #9 and #10

@djones6 djones6 closed this Aug 10, 2018
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.

3 participants