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

feat: openapi v3 import #578

Merged
merged 3 commits into from
Oct 22, 2023
Merged

Conversation

bplatta
Copy link
Contributor

@bplatta bplatta commented Oct 13, 2023

First pass at adding import for OpenAPI collections. Currently this only supports v3. Likely still missing some things.

Some assumptions worth considering from product standpoint and deserve discussion:

  • It uses the operationId as the "name" of the request
  • It groups the requests by tags (the first tag)
  • it uses the first security scheme configured globally (if an operation overrides the security config, it uses the first one defined under the operation)
  • For basic auth, it adds {{username}} and {{password}} for the values so that one wouldn't have to run through each request to add it. It similarly uses {{apiKey}} if API Key (in a header) security scheme is used
  • For request bodies, it uses the first content type defined in the spec
  • it uses the first "server" in the servers list configuration of the openapi spec to construct a base url. For example, this was useful to create collections that already had "localhost" in

Does NOT support currently:

  • OpenAPI spec v2
feature-openapi-import.mp4

@bplatta bplatta changed the title Feature/import openapi v3 feat: openapi v3 import Oct 13, 2023
@@ -42,6 +51,9 @@ const ImportCollection = ({ onClose, handleSubmit }) => {
<div className="text-link hover:underline cursor-pointer mt-2" onClick={handleImportInsomniaCollection}>
Insomnia Collection
</div>
<div className="text-link hover:underline cursor-pointer mt-2" onClick={handleImportOpenapiCollection}>
OpenAPI Collection
Copy link
Contributor

Choose a reason for hiding this comment

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

More precise it is OpenAPI Specification

Copy link
Contributor Author

@bplatta bplatta Oct 14, 2023

Choose a reason for hiding this comment

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

Happy to add v3 here if that is the consensus! The question here is, should there be separate options for v2 and v3 openapi specs in the Import list?

Pros for current approach:

  • unburden user with version selection, as it can be read from the spec. let Bruno just handle it (eventually will support v2 as well)
  • Fewer list options in Import modal, especially as future spec versions are released

Cons:

  • perhaps unclear to the user whether their Openapi spec is supported
  • Less visibly declarative as to version being imported (reliant on error reporting on import validation)

Will defer to decision from community/product owners but thought I'd raise this point first.

Copy link

Choose a reason for hiding this comment

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

If it's not to ugly maybe we could show which version are supported:
"OpenAPI Specification V3"
and then later on:
"OpenAPI Specification V2 & V3"

That way we could have Bruno handle it and we tell the users early on if they will have success in importing their spec fil.

@survivant
Copy link
Contributor

here few comments.
You should support yaml file as input too. Often the openapi schemas will be in yaml (generated by swagger).

I try the Pet store example from swagger (https://editor.swagger.io/). I had to convert it to json and when I try to import it I got an error
image

@survivant
Copy link
Contributor

I started to work also for that feature :) But my code is so ugly for now (never really works in React before) You can take a look here : https://github.com/survivant/bruno/tree/feature/openapi-collection

I added also a payload in the xml and json section for each request based on the schema. We could also use the examples if they are provided in the yaml file, but I prefer auto-generated.

@bplatta
Copy link
Contributor Author

bplatta commented Oct 14, 2023

here few comments. You should support yaml file as input too. Often the openapi schemas will be in yaml (generated by swagger).

I try the Pet store example from swagger (https://editor.swagger.io/). I had to convert it to json and when I try to import it I got an error image

Ok, nice catch! Will add yaml support if we want to install new library for yaml. Otherwise I will add validation error ("only json supported for now"). I went with Json only to avoid the new library install question. Question: If product owners are ok with new library being installed, please advise on which, I see a couple options in npm

I started to work also for that feature :) But my code is so ugly for now (never really works in React before) You can take a look here : https://github.com/survivant/bruno/tree/feature/openapi-collection

I added also a payload in the xml and json section for each request based on the schema. We could also use the examples if they are provided in the yaml file, but I prefer auto-generated.

Awesome, thank you @survivant ! Thought I'd hammer out something quick for my local install to get things going. Currently, the request payloads for json are constructed from the schema of the request body but nothing for xml yet. Will look at your branch. If you want to colab, feel free to PR into https://github.com/bplatta/bruno. Otherwise, I'll get to this early next week!

@survivant
Copy link
Contributor

@bplatta I have no issue at all the work together, or you take you want need from my branch. PS. like I said, I'm not a React dev so maybe you will make a heart attack when you will see it :) But the core is there. and for the Import of OpenApi.. you should just use the version in the schema, let concentrate on V3.x and 2.x won't be hard to add just after. We could just have a temporary message "Version 2.x not yet implemented..coming soon" and we add it just after. Like that we will be able to get feedback.

@thelinuxlich
Copy link

Hyped for this one!

@helloanoop
Copy link
Contributor

I am going to merge this and have this shipped in our upcoming release.

This does not directly break anything that already works, and bugs (if any) will be contained only in the importer.
Getting this merged also allows the larger community to test this out and report things that are breaking or not working as expected.

@bplatta Thanks for the amazing work on this!

@helloanoop helloanoop merged commit 49ea7f3 into usebruno:main Oct 22, 2023
1 check passed
@survivant
Copy link
Contributor

@bplatta can you fix the issue

TypeError: request.global.security.getScheme is not a function at transformOpenapiRequestItem (webpack-internal:///./src/utils/importers/openapi-collection.js:130:36) at Array.map (<anonymous>) at eval (webpack-internal:///./src/utils/importers/openapi-collection.js:402:33) at Array.map (<anonymous>) at eval (webpack-internal:///./src/utils/importers/openapi-collection.js:397:33) at new Promise (<anonymous>) at parseOpenApiCollection (webpack-internal:///./src/utils/importers/openapi-collection.js:346:10) {stack: "TypeError: request.global.security.getScheme is no…src/utils/importers/openapi-collection.js:346:10)", message: "request.global.security.getScheme is not a function"}
stack = "TypeError: request.global.security.getScheme is not a function\n at transformOpenapiRequestItem (webpack-internal:///./src/utils/importers/openapi-collection.js:130:36)\n at Array.map (<anonymous>)\n at eval (webpack-internal:///./src/utils/importers/openapi-collection.js:402:33)\n at Array.map (<anonymous>)\n at eval (webpack-internal:///./src/utils/importers/openapi-collection.js:397:33)\n at new Promise (<anonymous>)\n at parseOpenApiCollection (webpack-internal:///./src/utils/importers/openapi-collection.js:346:10)"
message = "request.global.security.getScheme is not a function"

the code is not in the main branch and the import OpenApi is not working.

thanks. PS I created my PR to add yaml support.

@GideonRoose
Copy link

When I try to import our openapi spec, I keep getting the following errors:

ValidationError: name is required
    at b (_app-edd8a6697a85c40e.js:1:1102435)
    at _app-edd8a6697a85c40e.js:1:1102991
BrunoError: The Collection file is corrupted
    at 764-7f30ab9a719fb6c4.js:1:9300

I created a minimal json file for reproduction:

{
  "openapi": "3.0.1",
  "info": {
    "title": "Example api",
    "version": "v1"
  },
  "paths": {
    "/users": {
      "get": {
        "description": "Returns users",
        "responses": {
          "200": {
            "description": "Success"
          }
        }
      }
    }
  }
}

The problem seems to be in the /users object. Using "/users": {} works fine, but adding anything in it breaks the import and I'm not sure what's wrong.

@bplatta bplatta deleted the feature/import-openapi-v3 branch October 23, 2023 16:24
@bplatta bplatta mentioned this pull request Oct 23, 2023
5 tasks
@bplatta
Copy link
Contributor Author

bplatta commented Oct 23, 2023

Ok, @survivant your issue was because the spec had no OpenAPI security block defined and there was a bug in the code for handling that! Fixed in #742.

@GideonRoose Issue was that the "operationId" field in operation objects is used for the bruno request item "name". I've added some other defaults now, testing with your spec (it uses the description field if exists). Fixed in #742

@GideonRoose
Copy link

Thanks! The import now 'succeeds', however, my collection is completely empty. Would it be possible for you (@bplatta) to share the openapi.spec.json file you use in the showcase video? Assuming that that one works as expected, I can more easily debug what is causing my issues.

@survivant
Copy link
Contributor

@GideonRoose go here and export the json https://editor.swagger.io/

@aviskase
Copy link
Contributor

Thanks! The import now 'succeeds', however, my collection is completely empty. Would it be possible for you (@bplatta) to share the openapi.spec.json file you use in the showcase video? Assuming that that one works as expected, I can more easily debug what is causing my issues.

I fixed that in #758

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.

8 participants