Skip to content
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

Honor variable defaults from servers declaration during OpenAPI3 import #2151

Merged

Conversation

develohpanda
Copy link
Contributor

@develohpanda develohpanda commented May 11, 2020

This PR adds functionality to honor variable defaults from the servers declaration during an OpenAPI3 spec import.

The openapi-2-kong package already does this, the functionality is simply adapted for insomnia-importers.

The openapi3 importer honors variable defaults, but if a variable exists in the URL and is missing in server.variables, an error is thrown and presented to the user.

Example spec
openapi: 3.0.2
info:
  description: This API is for administrating files and file storages.
  license:
    name: Some license
  title: File and Storage API
  version: 0.1.0
servers:
  - description: Devserver
    url: '{protocol}://{host}:{port}/{basePath}'
    variables:
      port:
        default: 8080
      host:
        default: localhost
      basePath:
        default: filemanagement
paths:
  /files:
    get:
      operationId: find_files

Insomnia Core
image

Insomnia Designer
Doesn't do anything during design -> debug navigation, silently swallows any errors caused by parsing an invalid spec. This is existing behavior, which we should improve.

Closes #1640

@develohpanda develohpanda requested a review from gschier May 11, 2020 01:18
@develohpanda develohpanda self-assigned this May 11, 2020
@netlify
Copy link

netlify bot commented May 11, 2020

Deploy preview for insomnia-storybook ready!

Built with commit ec2e8d5

https://deploy-preview-2151--insomnia-storybook.netlify.app

* @param {Object} api - openapi3 object
* @returns {UrlWithStringQuery} the resolved server URL
*/
function getDefaultServerUrl(api) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious why you used a different approach than openapi-2-kong does. Is it so you could replace vars with a default value if they don't have one defined? If so, can we make sure the fixture defined handles the case where there is no default?

export function fillServerVariables(server: OA3Server): string {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are two approaches (there should be a unified one) to fill server variables in openapi-2-kong:

  1. iterates over server.variables, throws if default missing, and replaces items in the url
  2. iterates over variables in the url, searches for a default in server.variables, defaults to http or .* if missing

I used a hybrid approach for the openapi3 importer, for several reasons:

  • To successfully parse the URL, we need a valid, resolved URL, and smart defaults are much more involved,
  • A truly valid OpenAPI spec should have variable defaults defined for a server url
  • o2k (kubernetes) can handle missing defaults (as per the requirements) using .* fallback

The openapi3 importer honors variable defaults, but if a variable exists in the URL and is missing in server.variables, an error is thrown and presented to the user.

@gschier

@develohpanda develohpanda force-pushed the feature/openapi-import-honor-server-default-variables branch from 0afb781 to ae894dd Compare August 5, 2020 04:59
@develohpanda develohpanda added this to Needs Review ❤ in Insomnia Kanban Aug 5, 2020
Copy link
Contributor

@gschier gschier left a comment

Choose a reason for hiding this comment

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

@develohpanda develohpanda merged commit 65b4838 into develop Aug 6, 2020
Insomnia Kanban automation moved this from Needs Review ❤ to Done 🎉 Aug 6, 2020
@develohpanda develohpanda deleted the feature/openapi-import-honor-server-default-variables branch August 6, 2020 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Insomnia Kanban
  
Done 🎉
Development

Successfully merging this pull request may close these issues.

[Bug] OpenAPI import does not seem to honor default variables in servers declaration
3 participants