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

Parse multiple servers from Open API spec #80

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

aditya-67
Copy link

@aditya-67 aditya-67 commented Mar 14, 2022

This introduces a solution for the issue #62 and following #64.

Solution

This changes the response format of both getSnippets and getEndpointSnippets. As the response format is changed, test cases were also updated. I'm willing to brainstorm a simpler and straightforward solution but did this for an internal use-case.

getSnippets

Response will be an array of arrays rather than objects. Each child array corresponds to multiple server code snippets. If one server present in spec, the length of each child array will be one. Check the url in the response.

[
  [
    {
      "method": "GET",
      "url": "https://api.instagram.com/v1/users/{user-id}/relationship",
      "description": "Get information about a relationship to another user.",
      "resource": "relationship",
      "snippets": [
        {
          "id": "node",
          "mimeType": "application/json",  // Only set for methods with a request body
          "title": "Node + Native",
          "content": "var http = require(\"https\");\n\nvar options = {..."
        }
      ]
    },
    {
      "method": "GET",
      "url": "https://uat.api.instagram.com/v1/users/{user-id}/relationship",
      "description": "Get information about a relationship to another user in UAT.",
      "resource": "relationship",
      "snippets": [
        {
          "id": "node",
          "mimeType": "application/json",  // Only set for methods with a request body
          "title": "Node + Native",
          "content": "var http = require(\"https\");\n\nvar options = {..."
        }
      ]
    }
  ]
  //...
]

getEndpointSnippets

Response will be an array rather than an object. Response array corresponds to multiple server code snippets. If one server is present in spec, the length of array will be one. Check the url in the response.

[
  {
    method: 'GET',
    url: 'https://api.instagram.com/v1/locations/search',
    description: undefined,
    resource: 'search',
    snippets: [
      {
          "id": "node",
          "mimeType": "application/json",  // Only set for methods with a request body
          "title": "Node + Native",
          "content": "var http = require(\"https\");\n\nvar options = {..."
        }
    ]
  },
  {
    method: 'GET',
    url: 'https://uat.api.instagram.com/v1/locations/search',
    description: undefined,
    resource: 'search',
    snippets: [
      {
          "id": "node",
          "mimeType": "application/json",  // Only set for methods with a request body
          "title": "Node + Native",
          "content": "var http = require(\"https\");\n\nvar options = {..."
        }
    ]
  }
]

@ErikWittern
Copy link
Owner

Hi @aditya-67, thank you very much for this PR! I agree that it is an issue that this library does not support multiple servers, but I am uncertain about the solution you propose.

For one, it is a significant, breaking change, and I want to be mindful of existing users (even though semantic versioning can of course help avoid unintended issues). The changes this PR makes in test.js are concerning to me.

I am also unsure about the proposed data structure. I fear that it might be difficult to understand for users what the array nested in the array means. One would need to consult documentation to understand this. Using an object-based data-structure, where a key indicates semantics, might be easier to understand. This is especially relevant considering how common the use-case is. If most OpenAPIs have only one server, most users should not have to bother with nested arrays because of a less frequent use-case.

Would it not be possible to simply add the url field in the objects in the non-nested arrays, and leave it to the client to then filter the array using that field? This could also play nicely with adding an option to only produce snippets for specific servers, as proposed in #64. But maybe I am missing something?

What do you think?

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.

2 participants