feat(openapi): introduction of a new `openapi:inspect` command #698

erunion · 2022-12-03T01:43:03Z

🧰 Changes

This introduces a new ~~openapi:uses~~ openapi:inspect command that does a series of analyses on API definitions across a series of OpenAPI and ReadMe-speciifc datapoints.

💽 Datapoints

OpenAPI

Datapoint	What we're analyzing
additionalProperties	Does the API definition utilize `additionalProperties`?
callbacks	Are there any operation `callbacks`?
circularRefs	Are there any circular `$ref` pointers in the spec? Where are they?
discriminators	Does the API definition utilize `discriminator` in polymorphic schemas?
links	Are there any operation `links`?
operationTotal	How many total operations are there?
mediaTypes	What are all the media types that this API definition accepts or delivers?
style	Do any operation parameters have any serialization requirements?
polymorphism	Do any schemas utilize polymorphism (`anyof`, `allOf`, and `oneOf`) for composing themselves?
securityTypes	What forms of auth does this API support?
serverVariables	Any of the server URLs have variables setup?
webhooks	Does this API definition use the OpenAPI 3.1 `webhooks` feature for describing webhooks?
xml	Do any parameters, request bodies, or responses document XML payloads?

ReadMe

And we also look for usage of ReadMe's OpenAPI extensions:

x-default
x-readme.code-samples
x-readme.headers
x-readme.explorer-enabled
x-readme.proxy-enabled
x-readme.samples-languages

As well as some deprecated extensions that we've got:

x-readme.samples-enabled
RAW_BODY (not technically an extension but a quirk of our Manual API editor)

📽️ Demo

Screen.Recording.2022-12-02.at.5.28.49.PM.mov

🧬 QA & Testing

Easier to test this locally with a full build instead of using ts-node so run npm run build and then these commands:

Full report

./bin/rdme openapi:inspect https://raw.githubusercontent.com/readmeio/oas-examples/main/3.0/json/schema-circular.json

Feature report

Note

Note that the circular refs in this schema are being surfaced. 👀

./bin/rdme openapi:inspect https://raw.githubusercontent.com/readmeio/oas-examples/main/3.0/json/schema-circular.json --feature=additionalProperties --feature=style --feature=circularRefs --feature=readme

Feature report (fancy syntax)

./bin/rdme openapi:inspect https://raw.githubusercontent.com/readmeio/oas-examples/main/3.0/json/schema-circular.json --feature=additionalProperties --feature={style,circularRefs,readme}

src/cmds/openapi/uses.ts

erunion · 2022-12-03T01:48:45Z

src/cmds/openapi/index.ts

@@ -34,7 +34,7 @@ export default class OpenAPICommand extends Command {
    super();

    this.command = 'openapi';
-    this.usage = 'openapi [file] [options]';
+    this.usage = 'openapi [file|url] [options]';


oas-normalize, which all of these commands use for spec processing, handles URLs so we should document in these commands that you can supply them either of these.

erunion · 2022-12-06T06:13:57Z

@kanadgupta This is still a WIP but I'd love if you could take a look now before I start to write tests for the command. Here's the commands you can use to test:

Full report

npx ts-node src/cli.ts openapi:uses https://raw.githubusercontent.com/readmeio/oas-examples/main/3.0/json/schema-circular.json

Feature report

npx ts-node src/cli.ts openapi:uses https://raw.githubusercontent.com/readmeio/oas-examples/main/3.0/json/schema-circular.json --feature=additionalProperties --feature=style --feature=circularRefs

Feature report (fancy syntax)

npx ts-node src/cli.ts openapi:uses https://raw.githubusercontent.com/readmeio/oas-examples/main/3.0/json/schema-circular.json --feature={additionalProperties,style,circularRefs}

I don't much care for the way the feature report screen looks at the moment:

We talked earlier today about having it return a process.exit(1) exit code if the supplied feature(s) didn't exist in the spec but I'm not sure if we can actually write tests for that without killing Jest.
I'm not really sure how to best let people supply --feature args for our ReadMe extension features in the analysis report as they have pretty unwieldy names like x-readme.headers or x-readme.explorer-enabled. Doing --feature=x-readme.headers seems pretty shitty. Maybe there should just be a catchall "show all ReadMe extension usage" argument instead and let --feature be OpenAPI-specific?
Also the circularRefs report isn't totally accurate right now as it doesn't tell you which of those refs is circular, it just dumps out all $ref pointers that we didn't dereference. I have some ideas on how to improve this but those changes will happen upstream in oas when I get to that point.

kanadgupta · 2022-12-06T14:36:01Z

Just played around, really loving it!

We talked earlier today about having it return a process.exit(1) exit code if the supplied feature(s) didn't exist in the spec but I'm not sure if we can actually write tests for that without killing Jest.

What if this function returned a Promise.allSettled result (where each result is a fulfilled promise if it contains the feature and a rejected promise if not), and then this line returns a rejected promise if any of the results in the array contain result.status === 'rejected'? That way our top-level handler here will return the correct exit code.¹

I'm not really sure how to best let people supply --feature args for our ReadMe extension features in the analysis report as they have pretty unwieldy names like x-readme.headers or x-readme.explorer-enabled. Doing --feature=x-readme.headers seems pretty shitty. Maybe there should just be a catchall "show all ReadMe extension usage" argument instead and let --feature be OpenAPI-specific?

Yeah I like this "show all extension usage" approach! Our extensions may change over time too so I don't really think those are conventions our users should need to know. Now that I'm thinking about it, it'd be cool² if we had our extensions documented in the TS types in @readme/oas-extensions and then we were able to pull those into here.

Also the circularRefs report isn't totally accurate right now as it doesn't tell you which of those refs is circular, it just dumps out all $ref pointers that we didn't dereference. I have some ideas on how to improve this but those changes will happen upstream in oas when I get to that point.

👍🏽

if we do this, we should probably extend the Error object to include a flag that indicates whether or not to return a red output on this line. ↩
doesn't have to happen in this PR, obviously ↩

erunion · 2022-12-08T00:44:52Z

__tests__/cmds/openapi/__snapshots__/uses.test.ts.snap

+circularRefs:
+ · #/components/schemas/MultiPart/properties/parent
+ · #/components/schemas/ZoneOffset/properties/rules"


👀

This was made possible by some improvements to oas¹, @readme/openapi-parser² and @readme/json-schema-ref-parser³ to give us access to the $ref pointers in API specs that are circular. Previously all we had to go off on was "are there still $ref pointers remaining in a spec" after dereferencing and with the way that dereferencing works if it encounters a circular ref in a $ref tree it'll stop dereferencing that tree completely, leaving everything in tact.

Footnotes

https://github.com/readmeio/oas/pull/723 ↩

https://github.com/readmeio/openapi-parser/pull/157 ↩

https://github.com/readmeio/json-schema-ref-parser/pull/58 ↩

erunion · 2022-12-08T00:46:03Z

__tests__/cmds/openapi/uses.test.ts

@@ -0,0 +1,81 @@
+/* eslint-disable jest/no-conditional-expect */


eslint-plugin-jest doesn't like me doing expect() calls inside a try-catch and it wants me to instead write a wrapper for the command call and no thanks I'd rather not.

https://github.com/jest-community/eslint-plugin-jest/blob/v27.1.6/docs/rules/no-conditional-expect.md#how-to-catch-a-thrown-error-for-testing-without-violating-this-rule

erunion · 2022-12-08T00:51:09Z

package.json

@@ -47,17 +47,19 @@
    "form-data": "^4.0.0",
    "gray-matter": "^4.0.1",
    "ignore": "^5.2.0",
-    "jsonpath": "^1.1.1",
+    "jsonpath-plus": "^7.2.0",


I swapped this out because I'm using jsonpath-plus in oas for this new analyzer library and it doesn't make sense to have rdme ship both in users' node_modules/ directories.

Why did I use jsonpath-plus instead of jsonpath? jsonpath-plus has a nicer API, a larger install base, and has been recently updated within the past few months vs 2 years.

erunion · 2022-12-08T00:51:59Z

package.json

    "prompts": "^2.4.2",
    "semver": "^7.0.0",
    "simple-git": "^3.13.0",
+    "table": "^6.8.1",


We used to use cli-table but I prefer the API that table offers (it's also still being maintained).

erunion · 2022-12-08T00:53:17Z

src/cmds/openapi/uses.ts

+      if (hasUnusedFeature) {
+        // If we have any unused features we should reject the command with a soft error so we
+        // output the report as normal but return a `exit(1)` status code.
+        return Promise.reject(new SoftError(report));


We talked about doing this as a Promise.allSettled but it was much easier to have this.buildFeaturesReport return a boolean for this instead of overhauling the already complex report composition to fit into the structure of a Promise.

makes sense to me! really dig this SoftError pattern

erunion · 2022-12-08T00:53:43Z

src/lib/analyzeOas.ts

+
+const OPENAPI_FEATURE_DOCS: Record<keyof Analysis['openapi'], Pick<AnalyzedFeature, 'description' | 'url'>> = {
+  additionalProperties: {
+    description: 'additionalProperties allows you to document dictionaries where the keys are user-supplied strings.',


All of these descriptions could definitely use some work I'm sure.

erunion · 2022-12-08T00:55:07Z

src/lib/analyzeOas.ts

+    // OpenAPI features
+    ...Object.keys(OPENAPI_FEATURE_DOCS),
+
+    'readme', // A catch-all for ReadMe features and extensions. Will look for everything.


Because having people do --feature=x-readme.explorer-enable is gnar they can do --feature=readme and we'll just query all of their ReadMe extension and feature usage instead. They won't be able to do piecemeal lookups on these but they get a cleaner API instead looking for some of the data they want.

src/lib/prepareOas.ts

erunion · 2022-12-08T00:57:25Z

tsconfig.json

@@ -5,7 +5,7 @@
    "downlevelIteration": true,
    "esModuleInterop": true,
    "experimentalDecorators": true,
-    "lib": ["es2020"],
+    "lib": ["es2020", "ES2021.Intl"],


Pulling Intl in because I'm using Intl.ListFormat to make composing "feature, feature, and feature" sentence fragments a lot easier.

erunion · 2022-12-08T18:58:08Z

@kanadgupta @darrenyong @Dashron I found a bunch of small UI bugs last night while looking again at the screenshots I made which just pushed up a bunch of fixes for so this is finally ready for review now.

kanadgupta

mostly small nits, but this feels great when playing around locally and I'm super excited 😌

__tests__/__snapshots__/index.test.ts.snap

src/cli.ts

src/cmds/openapi/uses.ts

kanadgupta · 2022-12-08T21:41:32Z

src/cmds/openapi/uses.ts

+      if (hasUnusedFeature) {
+        // If we have any unused features we should reject the command with a soft error so we
+        // output the report as normal but return a `exit(1)` status code.
+        return Promise.reject(new SoftError(report));


makes sense to me! really dig this SoftError pattern

src/cmds/openapi/uses.ts

src/lib/prepareOas.ts

src/lib/softError.ts

src/lib/prepareOas.ts

src/cmds/openapi/uses.ts

Co-authored-by: Kanad Gupta <[email protected]>

* fix: file placeholders * docs: add docs on inspect

kanadgupta

🥳

feat: introduction of a new openapi:uses command

a630c8b

erunion added enhancement New feature or request command:openapi Issues pertaining to the `openapi`, `validate`, `reduce`, or `swagger` commands labels Dec 3, 2022

fix: clarifying that our openapi commands accept urls

179e93f

erunion commented Dec 3, 2022

View reviewed changes

src/cmds/openapi/uses.ts Outdated Show resolved Hide resolved

erunion commented Dec 3, 2022

View reviewed changes

erunion added 4 commits December 5, 2022 15:06

Merge branch 'main' into feat/openapi-uses-command

d5c3f8a

feat: swapping out the analysis work with it now living in oas

0f39147

fix: updating oas to move the reducer back to where it was

c30332e

feat: adding --feature args to openapi:uses

900ded0

erunion requested a review from kanadgupta December 6, 2022 06:09

erunion mentioned this pull request Dec 7, 2022

feat(analyzer): improvements to reporting and circular $ref discovery readmeio/oas#723

Merged

3 tasks

erunion added 2 commits December 7, 2022 14:42

feat: improvements to readme and circularRef reporting

ba007a9

test: adding tests

cab360e

erunion commented Dec 8, 2022

View reviewed changes

src/lib/prepareOas.ts Outdated Show resolved Hide resolved

erunion commented Dec 8, 2022

View reviewed changes

erunion marked this pull request as ready for review December 8, 2022 00:57

erunion requested review from darrenyong and Dashron December 8, 2022 00:57

erunion added 2 commits December 8, 2022 10:15

fix: fixing spacing bugs and adding tests

60b58be

Merge branch 'main' into feat/openapi-uses-command

1be488c

fix: check for features length

ded813b

kanadgupta requested changes Dec 8, 2022

View reviewed changes

erunion and others added 2 commits December 8, 2022 14:37

Update src/lib/softError.ts

0f3f744

Co-authored-by: Kanad Gupta <[email protected]>

Update src/cmds/openapi/uses.ts

bdfbc8b

Co-authored-by: Kanad Gupta <[email protected]>

erunion changed the title ~~feat(openapi): introduction of a new openapi:uses command~~ feat(openapi): introduction of a new openapi:inspect command Dec 8, 2022

erunion added 2 commits December 8, 2022 14:41

fix: pr feedback

8b3cb94

fix: renaming the command to openapi:uses

b5f4821

erunion requested a review from kanadgupta December 8, 2022 22:50

docs: openapi:inspect (#706)

7095282

* fix: file placeholders * docs: add docs on inspect

kanadgupta approved these changes Dec 8, 2022

View reviewed changes

erunion merged commit daa5346 into main Dec 8, 2022

erunion deleted the feat/openapi-uses-command branch December 8, 2022 23:30

r0hack mentioned this pull request Nov 30, 2023

[Snyk] Security upgrade oas from 1.5.2 to 20.0.0 r0hack/rdme#9

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(openapi): introduction of a new `openapi:inspect` command #698

feat(openapi): introduction of a new `openapi:inspect` command #698

erunion commented Dec 3, 2022 •

edited

Loading

erunion Dec 3, 2022

erunion commented Dec 6, 2022 •

edited

Loading

kanadgupta commented Dec 6, 2022 •

edited

Loading

erunion Dec 8, 2022 •

edited

Loading

erunion Dec 8, 2022

erunion Dec 8, 2022 •

edited

Loading

erunion Dec 8, 2022

erunion Dec 8, 2022

kanadgupta Dec 8, 2022

erunion Dec 8, 2022

erunion Dec 8, 2022

erunion Dec 8, 2022

erunion commented Dec 8, 2022

kanadgupta left a comment

kanadgupta Dec 8, 2022

kanadgupta left a comment

		@@ -0,0 +1,81 @@
		/* eslint-disable jest/no-conditional-expect */

feat(openapi): introduction of a new openapi:inspect command #698

feat(openapi): introduction of a new openapi:inspect command #698

Conversation

erunion commented Dec 3, 2022 • edited Loading

🧰 Changes

💽 Datapoints

OpenAPI

ReadMe

📽️ Demo

🧬 QA & Testing

Full report

Feature report

Feature report (fancy syntax)

Choose a reason for hiding this comment

erunion commented Dec 6, 2022 • edited Loading

kanadgupta commented Dec 6, 2022 • edited Loading

Footnotes

erunion Dec 8, 2022 • edited Loading

Choose a reason for hiding this comment

Footnotes

Choose a reason for hiding this comment

erunion Dec 8, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

erunion commented Dec 8, 2022

kanadgupta left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kanadgupta left a comment

Choose a reason for hiding this comment

feat(openapi): introduction of a new `openapi:inspect` command #698

feat(openapi): introduction of a new `openapi:inspect` command #698

erunion commented Dec 3, 2022 •

edited

Loading

erunion commented Dec 6, 2022 •

edited

Loading

kanadgupta commented Dec 6, 2022 •

edited

Loading

erunion Dec 8, 2022 •

edited

Loading

erunion Dec 8, 2022 •

edited

Loading