feat(openapi): specify spec path and type in success response (#480)

* feat(openapi): specify spec path in success response

* docs: corresponding docs update

* refactor: separate OAS prep into its own function

This way we can reuse it in our `validate` and `openapi` commands and allow for a shared way to specify whether the file is OpenAPI or Swagger. This also ended up fixing some inconsistencies with our debugging and error handling

* fix: remove old `oas` guidance

* feat: specify the spec type in another place

* docs: update docs accordingly

* fix(prepareOas): error handling + debugging

Author: kanadgupta

__tests__/cmds/__snapshots__/validate.test.js.snap

@@ -1,7 +1,7 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
 exports[`rdme validate error handling should throw an error if an in valid Swagger definition is supplied 1`] = `
-[Error: Swagger schema validation failed.
+[SyntaxError: Swagger schema validation failed.
 
 ADDITIONAL PROPERTY must NOT have additional properties
 
@@ -15,7 +15,7 @@ ADDITIONAL PROPERTY must NOT have additional properties
 `;
 
 exports[`rdme validate error handling should throw an error if an invalid OpenAPI 3.1 definition is supplied 1`] = `
-[Error: OpenAPI schema validation failed.
+[SyntaxError: OpenAPI schema validation failed.
 
 REQUIRED must have required property 'name'
 

__tests__/cmds/openapi.test.js

@@ -15,23 +15,25 @@ const key = 'API_KEY';
 const id = '5aa0409b7cf527a93bfb44df';
 const version = '1.0.0';
 const exampleRefLocation = `${config.get('host')}/project/example-project/1.0.1/refs/ex`;
-const successfulMessageBase = [
+const successfulMessageBase = (specPath, specType) => [
   '',
   `\t${chalk.green(exampleRefLocation)}`,
   '',
-  'To update your OpenAPI or Swagger definition, run the following:',
+  `To update your ${specType} definition, run the following:`,
   '',
-  `\t${chalk.green(`rdme openapi FILE --key=${key} --id=1`)}`,
+  `\t${chalk.green(`rdme openapi ${specPath} --key=${key} --id=1`)}`,
 ];
-const successfulUpload = [
-  "You've successfully uploaded a new OpenAPI file to your ReadMe project!",
-  ...successfulMessageBase,
-].join('\n');
-
-const successfulUpdate = [
-  "You've successfully updated an OpenAPI file on your ReadMe project!",
-  ...successfulMessageBase,
-].join('\n');
+const successfulUpload = (specPath, specType = 'OpenAPI') =>
+  [
+    `You've successfully uploaded a new ${specType} file to your ReadMe project!`,
+    ...successfulMessageBase(specPath, specType),
+  ].join('\n');
+
+const successfulUpdate = (specPath, specType = 'OpenAPI') =>
+  [
+    `You've successfully updated an existing ${specType} file on your ReadMe project!`,
+    ...successfulMessageBase(specPath, specType),
+  ].join('\n');
 
 const testWorkingDir = process.cwd();
 
@@ -60,13 +62,13 @@ describe('rdme openapi', () => {
 
   describe('upload', () => {
     it.each([
-      ['Swagger 2.0', 'json', '2.0'],
-      ['Swagger 2.0', 'yaml', '2.0'],
-      ['OpenAPI 3.0', 'json', '3.0'],
-      ['OpenAPI 3.0', 'yaml', '3.0'],
-      ['OpenAPI 3.1', 'json', '3.1'],
-      ['OpenAPI 3.1', 'yaml', '3.1'],
-    ])('should support uploading a %s definition (format: %s)', async (_, format, specVersion) => {
+      ['Swagger 2.0', 'json', '2.0', 'Swagger'],
+      ['Swagger 2.0', 'yaml', '2.0', 'Swagger'],
+      ['OpenAPI 3.0', 'json', '3.0', 'OpenAPI'],
+      ['OpenAPI 3.0', 'yaml', '3.0', 'OpenAPI'],
+      ['OpenAPI 3.1', 'json', '3.1', 'OpenAPI'],
+      ['OpenAPI 3.1', 'yaml', '3.1', 'OpenAPI'],
+    ])('should support uploading a %s definition (format: %s)', async (_, format, specVersion, type) => {
       const mock = getApiNock()
         .get('/api/v1/api-specification')
         .basicAuth({ user: key })
@@ -78,13 +80,15 @@ describe('rdme openapi', () => {
         .basicAuth({ user: key })
         .reply(201, { _id: 1 }, { location: exampleRefLocation });
 
+      const spec = require.resolve(`@readme/oas-examples/${specVersion}/${format}/petstore.${format}`);
+
       await expect(
         openapi.run({
-          spec: require.resolve(`@readme/oas-examples/${specVersion}/${format}/petstore.${format}`),
+          spec,
           key,
           version,
         })
-      ).resolves.toBe(successfulUpload);
+      ).resolves.toBe(successfulUpload(spec, type));
 
       expect(console.info).toHaveBeenCalledTimes(0);
 
@@ -114,7 +118,7 @@ describe('rdme openapi', () => {
       // to break.
       fs.copyFileSync(require.resolve('@readme/oas-examples/2.0/json/petstore.json'), './swagger.json');
 
-      await expect(openapi.run({ key })).resolves.toBe(successfulUpload);
+      await expect(openapi.run({ key })).resolves.toBe(successfulUpload('swagger.json', 'Swagger'));
 
       expect(console.info).toHaveBeenCalledTimes(1);
 
@@ -128,26 +132,28 @@ describe('rdme openapi', () => {
 
   describe('updates / resyncs', () => {
     it.each([
-      ['Swagger 2.0', 'json', '2.0'],
-      ['Swagger 2.0', 'yaml', '2.0'],
-      ['OpenAPI 3.0', 'json', '3.0'],
-      ['OpenAPI 3.0', 'yaml', '3.0'],
-      ['OpenAPI 3.1', 'json', '3.1'],
-      ['OpenAPI 3.1', 'yaml', '3.1'],
-    ])('should support updating a %s definition (format: %s)', async (_, format, specVersion) => {
+      ['Swagger 2.0', 'json', '2.0', 'Swagger'],
+      ['Swagger 2.0', 'yaml', '2.0', 'Swagger'],
+      ['OpenAPI 3.0', 'json', '3.0', 'OpenAPI'],
+      ['OpenAPI 3.0', 'yaml', '3.0', 'OpenAPI'],
+      ['OpenAPI 3.1', 'json', '3.1', 'OpenAPI'],
+      ['OpenAPI 3.1', 'yaml', '3.1', 'OpenAPI'],
+    ])('should support updating a %s definition (format: %s)', async (_, format, specVersion, type) => {
       const mock = getApiNock()
         .put(`/api/v1/api-specification/${id}`, body => body.match('form-data; name="spec"'))
         .basicAuth({ user: key })
         .reply(201, { _id: 1 }, { location: exampleRefLocation });
 
+      const spec = require.resolve(`@readme/oas-examples/${specVersion}/${format}/petstore.${format}`);
+
       await expect(
         openapi.run({
-          spec: require.resolve(`@readme/oas-examples/${specVersion}/${format}/petstore.${format}`),
+          spec,
           key,
           id,
           version,
         })
-      ).resolves.toBe(successfulUpdate);
+      ).resolves.toBe(successfulUpdate(spec, type));
 
       return mock.done();
     });
@@ -158,9 +164,9 @@ describe('rdme openapi', () => {
         .basicAuth({ user: key })
         .reply(201, { _id: 1 }, { location: exampleRefLocation });
 
-      await expect(
-        openapi.run({ spec: require.resolve('@readme/oas-examples/3.1/json/petstore.json'), key, id, version })
-      ).resolves.toBe(successfulUpdate);
+      const spec = require.resolve('@readme/oas-examples/3.1/json/petstore.json');
+
+      await expect(openapi.run({ spec, key, id, version })).resolves.toBe(successfulUpdate(spec));
 
       expect(console.warn).toHaveBeenCalledTimes(1);
       expect(console.info).toHaveBeenCalledTimes(0);
@@ -225,9 +231,9 @@ describe('rdme openapi', () => {
         .basicAuth({ user: key })
         .reply(201, { _id: 1 }, { location: exampleRefLocation });
 
-      await expect(
-        openapi.run({ spec: require.resolve('@readme/oas-examples/2.0/json/petstore.json'), key })
-      ).resolves.toBe(successfulUpload);
+      const spec = require.resolve('@readme/oas-examples/2.0/json/petstore.json');
+
+      await expect(openapi.run({ spec, key })).resolves.toBe(successfulUpload(spec, 'Swagger'));
 
       return mock.done();
     });
@@ -251,9 +257,9 @@ describe('rdme openapi', () => {
       .basicAuth({ user: key })
       .reply(201, { _id: 1 }, { location: exampleRefLocation });
 
-    await expect(openapi.run({ spec: './__tests__/__fixtures__/ref-oas/petstore.json', key, version })).resolves.toBe(
-      successfulUpload
-    );
+    const spec = './__tests__/__fixtures__/ref-oas/petstore.json';
+
+    await expect(openapi.run({ spec, key, version })).resolves.toBe(successfulUpload(spec));
 
     expect(console.info).toHaveBeenCalledTimes(0);
 
@@ -280,14 +286,16 @@ describe('rdme openapi', () => {
       .basicAuth({ user: key })
       .reply(201, { _id: 1 }, { location: exampleRefLocation });
 
+    const spec = 'petstore.json';
+
     await expect(
       openapi.run({
-        spec: 'petstore.json',
+        spec,
         key,
         version,
         workingDirectory: './__tests__/__fixtures__/relative-ref-oas',
       })
-    ).resolves.toBe(successfulUpload);
+    ).resolves.toBe(successfulUpload(spec));
 
     expect(console.info).toHaveBeenCalledTimes(0);
 
@@ -460,7 +468,7 @@ describe('rdme swagger', () => {
   it('should run `rdme openapi`', () => {
     return expect(swagger.run({ spec: '', key, id, version })).rejects.toThrow(
       "We couldn't find an OpenAPI or Swagger definition.\n\n" +
-        'Run `rdme openapi ./path/to/api/definition` to upload an existing definition or `rdme oas init` to create a fresh one!'
+        'Please specify the path to your definition with `rdme openapi ./path/to/api/definition`.'
     );
   });
 });

documentation/rdme.md

@@ -133,9 +133,9 @@ You've successfully updated an OpenAPI file on your ReadMe project!
 
         http://dash.readme.com/project/{your_project}/v1.0/refs/pet
 
-To update your OpenAPI or Swagger definition, run the following:
+To update your OpenAPI definition, run the following:
 
-        rdme openapi FILE --key=<<user>> --id=API_DEFINITION_ID
+        rdme openapi [path-to-file.json] --key=<<user>> --id=API_DEFINITION_ID
 ```
 
 </details>

src/cmds/openapi.js

@@ -2,7 +2,6 @@ const chalk = require('chalk');
 const fs = require('fs');
 const config = require('config');
 const { prompt } = require('enquirer');
-const OASNormalize = require('oas-normalize');
 const promptOpts = require('../lib/prompts');
 const APIError = require('../lib/apiError');
 const { getProjectVersion } = require('../lib/versionSelect');
@@ -12,6 +11,7 @@ const FormData = require('form-data');
 const parse = require('parse-link-header');
 const { file: tmpFile } = require('tmp-promise');
 const { debug } = require('../lib/logger');
+const prepareOas = require('../lib/prepareOas');
 
 module.exports = class OpenAPICommand {
   constructor() {
@@ -81,13 +81,12 @@ module.exports = class OpenAPICommand {
     }
 
     async function callApi(specPath, versionCleaned) {
-      debug(`bundling and validating spec located at ${specPath}`);
-      // @todo Tailor messaging to what is actually being handled here. If the user is uploading a Swagger file, never mention that they uploaded/updated an OpenAPI file.
+      const { bundledSpec, specType } = await prepareOas(specPath, true);
 
       async function success(data) {
         const message = !isUpdate
-          ? "You've successfully uploaded a new OpenAPI file to your ReadMe project!"
-          : "You've successfully updated an OpenAPI file on your ReadMe project!";
+          ? `You've successfully uploaded a new ${specType} file to your ReadMe project!`
+          : `You've successfully updated an existing ${specType} file on your ReadMe project!`;
 
         debug(`successful ${data.status} response`);
         const body = await data.json();
@@ -99,10 +98,10 @@ module.exports = class OpenAPICommand {
             '',
             `\t${chalk.green(`${data.headers.get('location')}`)}`,
             '',
-            'To update your OpenAPI or Swagger definition, run the following:',
+            `To update your ${specType} definition, run the following:`,
             '',
             // eslint-disable-next-line no-underscore-dangle
-            `\t${chalk.green(`rdme openapi FILE --key=${key} --id=${body._id}`)}`,
+            `\t${chalk.green(`rdme openapi ${specPath} --key=${key} --id=${body._id}`)}`,
           ].join('\n')
         );
       }
@@ -127,17 +126,6 @@ module.exports = class OpenAPICommand {
         }
       }
 
-      const oas = new OASNormalize(specPath, { colorizeErrors: true, enablePaths: true });
-      debug('spec normalized');
-
-      await oas.validate(false);
-      debug('spec validated');
-
-      const bundledSpec = await oas.bundle().then(res => {
-        return JSON.stringify(res);
-      });
-      debug('spec bundled');
-
       // Create a temporary file to write the bundled spec to,
       // which we will then stream into the form data body
       const { path } = await tmpFile({ prefix: 'rdme-openapi-', postfix: '.json' });
@@ -244,7 +232,7 @@ module.exports = class OpenAPICommand {
       reject(
         new Error(
           "We couldn't find an OpenAPI or Swagger definition.\n\n" +
-            'Run `rdme openapi ./path/to/api/definition` to upload an existing definition or `rdme oas init` to create a fresh one!'
+            'Please specify the path to your definition with `rdme openapi ./path/to/api/definition`.'
         )
       );
     });

src/cmds/validate.js

@@ -1,7 +1,7 @@
 const chalk = require('chalk');
 const fs = require('fs');
-const OASNormalize = require('oas-normalize');
 const { debug } = require('../lib/logger');
+const prepareOas = require('../lib/prepareOas');
 
 module.exports = class ValidateCommand {
   constructor() {
@@ -37,20 +37,8 @@ module.exports = class ValidateCommand {
     debug(`opts: ${JSON.stringify(opts)}`);
 
     async function validateSpec(specPath) {
-      const oas = new OASNormalize(specPath, { colorizeErrors: true, enablePaths: true });
-
-      return oas
-        .validate(false)
-        .then(api => {
-          if (api.swagger) {
-            return Promise.resolve(chalk.green(`${specPath} is a valid Swagger API definition!`));
-          }
-          return Promise.resolve(chalk.green(`${specPath} is a valid OpenAPI API definition!`));
-        })
-        .catch(err => {
-          debug(`raw validation error object: ${JSON.stringify(err)}`);
-          return Promise.reject(new Error(err.message));
-        });
+      const { specType } = await prepareOas(specPath);
+      return Promise.resolve(chalk.green(`${specPath} is a valid ${specType} API definition!`));
     }
 
     if (spec) {
@@ -73,7 +61,7 @@ module.exports = class ValidateCommand {
 
       reject(
         new Error(
-          "We couldn't find an OpenAPI or Swagger definition.\n\nIf you need help creating one run `rdme oas init`!"
+          "We couldn't find an OpenAPI or Swagger definition.\n\nPlease specify the path to your definition with `rdme validate ./path/to/api/definition`."
         )
       );
     });

src/lib/prepareOas.js

@@ -0,0 +1,43 @@
+const OASNormalize = require('oas-normalize');
+const { debug } = require('./logger');
+
+/**
+ * Normalizes, validates, and (optionally) bundles an OpenAPI definition.
+ *
+ * @param {String} path path to spec file
+ * @param {Boolean} bundle boolean to indicate whether or not to
+ * return a bundled spec (defaults to false)
+ */
+module.exports = async function prepare(path, bundle = false) {
+  debug(`about to normalize spec located at ${path}`);
+  const oas = new OASNormalize(path, { colorizeErrors: true, enablePaths: true });
+  debug('spec normalized');
+
+  const api = await oas.validate(false).catch(err => {
+    debug(`raw validation error object: ${JSON.stringify(err)}`);
+    throw err;
+  });
+  debug('👇👇👇👇👇 spec validated! logging spec below 👇👇👇👇👇');
+  debug(api);
+  debug('👆👆👆👆👆 finished logging spec 👆👆👆👆👆');
+
+  const specType = api.swagger ? 'Swagger' : 'OpenAPI';
+  debug(`spec type: ${specType}`);
+
+  let bundledSpec = '';
+
+  if (bundle) {
+    bundledSpec = await oas
+      .bundle()
+      .then(res => {
+        return JSON.stringify(res);
+      })
+      .catch(err => {
+        debug(`raw bundling error object: ${JSON.stringify(err)}`);
+        throw err;
+      });
+    debug('spec bundled');
+  }
+
+  return { bundledSpec, specType };
+};