diff --git a/projects/optic/src/__tests__/integration/__snapshots__/diff-all.test.ts.snap b/projects/optic/src/__tests__/integration/__snapshots__/diff-all.test.ts.snap index 0c349562ce..f9cfe4490a 100644 --- a/projects/optic/src/__tests__/integration/__snapshots__/diff-all.test.ts.snap +++ b/projects/optic/src/__tests__/integration/__snapshots__/diff-all.test.ts.snap @@ -1,5 +1,38 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`diff-all diff all against a cloud tag 1`] = ` +"Diffing cloud:main to spec.json + +specification details: +- /x-optic-url added +- /openapi changed + +GET /api/users: removed + +GET /example: added + +POST /example: added + +PUT /example: added + +PATCH /example: added + +Checks + +  FAIL  GET /api/users + removed rule [error]: prevent operation removal + x cannot remove an operation. This is a breaking change. + at paths > /api/users > get (empty.json:1:45) + + + +4 operations added, 1 removed +0 passed +1 errors + +" +`; + exports[`diff-all diff all not in the root of a repo 1`] = ` "Diffing HEAD~1:folder/in-folder.yml to in-folder.yml No changes were detected @@ -141,11 +174,12 @@ Checks 2 errors ✔ Uploaded results of diff to http://localhost:3001/organizations/org-id/apis/api-id/runs/run-id -Warning - the following OpenAPI specs were detected but did not have valid x-optic-url keys. \`optic diff-all\` only runs on specs that include \`x-optic-url\` keys that point to specs uploaded to optic. +Warning - the following OpenAPI specs were detected but did not have valid x-optic-url keys. 'optic diff-all --upload' can only runs on specs that have been added to optic + Run the \`optic api add\` command to add these specs to optic -../spec-with-invalid-url.yml -../specwithoutkey.json -../specwithoutkey.yml +../spec-with-invalid-url.yml (untracked) +../specwithoutkey.json (untracked) +../specwithoutkey.yml (untracked) " `; @@ -337,11 +371,12 @@ Checks 2 errors ✔ Uploaded results of diff to http://localhost:3001/organizations/org-id/apis/api-id/runs/run-id -Warning - the following OpenAPI specs were detected but did not have valid x-optic-url keys. \`optic diff-all\` only runs on specs that include \`x-optic-url\` keys that point to specs uploaded to optic. +Warning - the following OpenAPI specs were detected but did not have valid x-optic-url keys. 'optic diff-all --upload' can only runs on specs that have been added to optic + Run the \`optic api add\` command to add these specs to optic -spec-with-invalid-url.yml -specwithoutkey.json -specwithoutkey.yml +spec-with-invalid-url.yml (untracked) +specwithoutkey.json (untracked) +specwithoutkey.yml (untracked) " `; diff --git a/projects/optic/src/__tests__/integration/__snapshots__/diff.test.ts.snap b/projects/optic/src/__tests__/integration/__snapshots__/diff.test.ts.snap index 9f36c7df46..56cf9bf85e 100644 --- a/projects/optic/src/__tests__/integration/__snapshots__/diff.test.ts.snap +++ b/projects/optic/src/__tests__/integration/__snapshots__/diff.test.ts.snap @@ -690,6 +690,39 @@ Checks " `; +exports[`diff with mock server with --base cloud:tag 1`] = ` +"Rerun this command with the --web flag to open a visual changelog your browser + +specification details: +- /x-optic-url added +- /openapi changed + +GET /api/users: removed + +GET /example: added + +POST /example: added + +PUT /example: added + +PATCH /example: added + +Checks + +  FAIL  GET /api/users + removed rule [error]: prevent operation removal + x cannot remove an operation. This is a breaking change. + at paths > /api/users > get (empty.json:1:45) + + + +4 operations added, 1 removed +0 passed +1 errors + +" +`; + exports[`diff with mock server with cloud tag ref 1`] = ` "Rerun this command with the --web flag to open a visual changelog your browser diff --git a/projects/optic/src/__tests__/integration/diff-all.test.ts b/projects/optic/src/__tests__/integration/diff-all.test.ts index ff80dfcb55..31276b5705 100644 --- a/projects/optic/src/__tests__/integration/diff-all.test.ts +++ b/projects/optic/src/__tests__/integration/diff-all.test.ts @@ -32,6 +32,16 @@ setupTestServer(({ url, method }) => { return JSON.stringify({ id: 'run-id', }); + } else if (method === 'GET' && /spec$/.test(url)) { + return `{"openapi":"3.1.0","paths":{ "/api/users": { "get": { "responses":{} }}},"info":{"version":"0.0.0","title":"Empty"}}`; + } else if (method === 'GET' && /sourcemap$/.test(url)) { + return `{"rootFilePath":"empty.json","files":[{"path":"empty.json","sha256":"815b8e5491a1f491765084f236c741d5073e10fcece23436f2db84a8c788db09","contents":"{'openapi':'3.1.0','paths':{ '/api/users': { 'get': { 'responses':{} }}},'info':{'version':'0.0.0','title':'Empty'}}","index":0}],"refMappings":{}}`; + } else if (method === 'GET' && /api\/apis\/.*\/specs\/.*$/.test(url)) { + return JSON.stringify({ + id: 'run-id', + specUrl: `${process.env.BWTS_HOST_OVERRIDE}/spec`, + sourcemapUrl: `${process.env.BWTS_HOST_OVERRIDE}/sourcemap`, + }); } return JSON.stringify({}); }); @@ -158,4 +168,20 @@ describe('diff-all', () => { expect(normalizeWorkspace(workspace, combined)).toMatchSnapshot(); expect(code).toBe(1); }); + + test('diff all against a cloud tag', async () => { + const workspace = await setupWorkspace('diff-all/cloud-diff', { + repo: true, + commit: true, + }); + process.env.OPTIC_TOKEN = '123'; + + const { combined, code } = await runOptic( + workspace, + 'diff-all --compare-from cloud:main --check' + ); + + expect(code).toBe(1); + expect(normalizeWorkspace(workspace, combined)).toMatchSnapshot(); + }); }); diff --git a/projects/optic/src/__tests__/integration/diff.test.ts b/projects/optic/src/__tests__/integration/diff.test.ts index e22e206f20..e208596426 100644 --- a/projects/optic/src/__tests__/integration/diff.test.ts +++ b/projects/optic/src/__tests__/integration/diff.test.ts @@ -137,7 +137,7 @@ paths: {}`; } else if (method === 'GET' && /spec$/.test(url)) { return `{"openapi":"3.1.0","paths":{ "/api/users": { "get": { "responses":{} }}},"info":{"version":"0.0.0","title":"Empty"}}`; } else if (method === 'GET' && /sourcemap$/.test(url)) { - return `{"rootFilePath":"empty.json","files":[{"path":"empty.json","index":0,"contents":{"openapi": "3.1.0","paths": {},"info": {"version": "0.0.0","title": "Empty"},"x-optic-ci-empty-spec": true},"sha256":"841ad837d9488cb03837e695fd2d7dfacacc708465ba8b4e3d2d811428915016"}],"refMappings":{}}`; + return `{"rootFilePath":"empty.json","files":[{"path":"empty.json","sha256":"815b8e5491a1f491765084f236c741d5073e10fcece23436f2db84a8c788db09","contents":"{'openapi':'3.1.0','paths':{ '/api/users': { 'get': { 'responses':{} }}},'info':{'version':'0.0.0','title':'Empty'}}","index":0}],"refMappings":{}}`; } else if (method === 'GET' && /api\/apis\/.*\/specs\/.*$/.test(url)) { return JSON.stringify({ id: 'run-id', @@ -239,5 +239,18 @@ paths: {}`; expect(code).toBe(0); expect(normalizeWorkspace(workspace, combined)).toMatchSnapshot(); }); + + test('with --base cloud:tag', async () => { + const workspace = await setupWorkspace('diff/upload', { + repo: false, + }); + const { combined, code } = await runOptic( + workspace, + `diff spec.json --base cloud:main --check` + ); + + expect(code).toBe(1); + expect(normalizeWorkspace(workspace, combined)).toMatchSnapshot(); + }); }); }); diff --git a/projects/optic/src/__tests__/integration/workspaces/diff-all/cloud-diff/spec.json b/projects/optic/src/__tests__/integration/workspaces/diff-all/cloud-diff/spec.json new file mode 100644 index 0000000000..22517743d4 --- /dev/null +++ b/projects/optic/src/__tests__/integration/workspaces/diff-all/cloud-diff/spec.json @@ -0,0 +1,40 @@ +{ + "openapi": "3.0.1", + "x-optic-url": "https://app.useoptic.com/organizations/org-id/apis/api-id", + "paths": { + "/example": { + "get": { + "operationId": "my-op", + "responses": {} + }, + "post": { + "operationId": "postOriginalaa", + "responses": {} + }, + "put": { + "operationId": "putOriginalaa", + "responses": {} + }, + "patch": { + "operationId": "putOriginaaal", + "responses": { + "200": { + "description": "hello", + "content": { + "application/json": { + "example": "hello", + "schema": { + "type": "string" + } + } + } + } + } + } + } + }, + "info": { + "version": "0.0.0", + "title": "Empty" + } +} diff --git a/projects/optic/src/commands/diff/diff-all.ts b/projects/optic/src/commands/diff/diff-all.ts index d85989165b..9b68939d28 100644 --- a/projects/optic/src/commands/diff/diff-all.ts +++ b/projects/optic/src/commands/diff/diff-all.ts @@ -56,7 +56,7 @@ export const registerDiffAll = (cli: Command, config: OpticCliConfig) => { ) .option( '--compare-from ', - 'the base ref to compare against. Defaults to HEAD~1', + 'the base ref to compare against. Defaults to HEAD~1. Also supports optic cloud tags (cloud:tag_name)', 'HEAD~1' ) .option( @@ -121,6 +121,35 @@ type DiffAllActionOptions = { severity: 'info' | 'warn' | 'error'; }; +type CandidateMap = Map< + string, + { + from?: string; + to?: string; + } +>; + +function getCandidatesFromCloudTag( + tag: string, + to: { ref?: string; paths: string[] }, + root: string +): CandidateMap { + const results: CandidateMap = new Map(); + for (const toPath of to.paths) { + const hasRef = to.ref && toPath.startsWith(`${to.ref}:`); + const strippedPath = hasRef ? toPath.replace(`${to.ref}:`, '') : toPath; + const pathFromRoot = path.relative(root, strippedPath); + const refAndPathFromRoot = hasRef ? `${to.ref}:${pathFromRoot}` : toPath; + + results.set(pathFromRoot, { + from: tag, + to: refAndPathFromRoot, + }); + } + + return results; +} + // Match up the to and from candidates // This will return the comparisons we can try to run function matchCandidates( @@ -130,14 +159,8 @@ function matchCandidates( }, to: { ref?: string; paths: string[] }, root: string -): Map< - string, - { - from?: string; - to?: string; - } -> { - const results = new Map(); +): CandidateMap { + const results: CandidateMap = new Map(); for (const fromPath of from.paths) { const strippedPath = fromPath.replace(`${from.ref}:`, ''); const pathFromRoot = path.relative(root, strippedPath); @@ -166,7 +189,7 @@ function matchCandidates( } async function computeAll( - candidatesMap: ReturnType, + candidateMap: CandidateMap, config: OpticCliConfig, options: DiffAllActionOptions ): Promise<{ @@ -181,11 +204,15 @@ async function computeAll( const results: Result[] = []; - for await (const [_, candidate] of candidatesMap) { + for await (const [_, candidate] of candidateMap) { // We load the raw spec and discard the comparison if there is no optic url or is in an invalid version // Cases we run the comparison: // - if to spec has x-optic-url // - if from spec has x-optic-url AND to spec is empty + const cloudTag: string | null = + !!candidate.from && /^cloud:/.test(candidate.from) + ? candidate.from.replace(/^cloud:/, '') + : null; const specPathToLoad = candidate.to ?? candidate.from; if (!specPathToLoad) { logger.debug( @@ -211,10 +238,12 @@ async function computeAll( continue; } + let fromRef = candidate.from; + try { - const hasOpticUrl = getApiFromOpticUrl(rawSpec[OPTIC_URL_KEY]); + const opticApi = getApiFromOpticUrl(rawSpec[OPTIC_URL_KEY]); checkOpenAPIVersion(rawSpec); - if (!hasOpticUrl && options.upload) { + if (!opticApi && (options.upload || cloudTag)) { logger.debug( `Skipping comparison from ${candidate.from} to ${candidate.to} because there was no x-optic-url` ); @@ -222,6 +251,8 @@ async function computeAll( path: candidate.to!, }); continue; + } else if (opticApi && cloudTag) { + fromRef = `cloud:${opticApi.apiId}@${cloudTag}`; } } catch (e) { logger.debug( @@ -236,7 +267,7 @@ async function computeAll( let fromParseResults: ParseResult; let toParseResults: ParseResult; try { - fromParseResults = await loadSpec(candidate.from, config, { + fromParseResults = await loadSpec(fromRef, config, { strict: false, denormalize: true, }); @@ -367,15 +398,28 @@ type Warnings = { unparseableToSpec: { path: string; error: unknown }[]; }; -function handleWarnings(warnings: Warnings, options: DiffAllActionOptions) { +function handleWarnings( + warnings: Warnings, + options: DiffAllActionOptions, + isCloudDiff: boolean +) { if (warnings.missingOpticUrl.length > 0) { logger.info( chalk.yellow( - 'Warning - the following OpenAPI specs were detected but did not have valid x-optic-url keys. `optic diff-all` only runs on specs that include `x-optic-url` keys that point to specs uploaded to optic.' + `Warning - the following OpenAPI specs were detected but did not have valid x-optic-url keys. ${ + isCloudDiff + ? `optic diff-all --compare-from cloud:{tag}' can only runs on specs that have been added to optic` + : `'optic diff-all --upload' can only runs on specs that have been added to optic` + }` ) ); + logger.info(''); logger.info('Run the `optic api add` command to add these specs to optic'); - logger.info(warnings.missingOpticUrl.map((f) => f.path).join('\n')); + logger.info( + warnings.missingOpticUrl + .map((f) => `${f.path} ${chalk.red('(untracked)')}`) + .join('\n') + ); logger.info(''); if (options.failOnUntrackedOpenapi) { @@ -499,9 +543,8 @@ const getDiffAllAction = logger.setLevel('silent'); } + let candidateMap: CandidateMap; let compareToCandidates: string[]; - let compareFromCandidates: string[]; - try { compareToCandidates = await findOpenApiSpecsCandidates(options.compareTo); } catch (e) { @@ -513,39 +556,55 @@ const getDiffAllAction = return; } - try { - compareFromCandidates = await findOpenApiSpecsCandidates( - options.compareFrom + if (/^cloud:/.test(options.compareFrom)) { + candidateMap = getCandidatesFromCloudTag( + options.compareFrom, + { + ref: options.compareTo, + paths: applyGlobFilter(compareToCandidates, { + matches: options.match, + ignores: options.ignore, + }), + }, + config.root ); - } catch (e) { - logger.error( - `Error reading files from git history for --compare-from ${options.compareFrom}` + } else { + let compareFromCandidates: string[]; + + try { + compareFromCandidates = await findOpenApiSpecsCandidates( + options.compareFrom + ); + } catch (e) { + logger.error( + `Error reading files from git history for --compare-from ${options.compareFrom}` + ); + logger.error(e); + process.exitCode = 1; + return; + } + + candidateMap = matchCandidates( + { + ref: options.compareFrom, + paths: applyGlobFilter(compareFromCandidates, { + matches: options.match, + ignores: options.ignore, + }), + }, + { + ref: options.compareTo, + paths: applyGlobFilter(compareToCandidates, { + matches: options.match, + ignores: options.ignore, + }), + }, + config.root ); - logger.error(e); - process.exitCode = 1; - return; } - const candidatesMap = matchCandidates( - { - ref: options.compareFrom, - paths: applyGlobFilter(compareFromCandidates, { - matches: options.match, - ignores: options.ignore, - }), - }, - { - ref: options.compareTo, - paths: applyGlobFilter(compareToCandidates, { - matches: options.match, - ignores: options.ignore, - }), - }, - config.root - ); - const { warnings, results } = await computeAll( - candidatesMap, + candidateMap, config, options ); @@ -560,13 +619,11 @@ const getDiffAllAction = openWebpage(changelogUrl, result, config); } } - - handleWarnings(warnings, options); + const isCloudDiff = /^cloud:/.test(options.compareFrom); + handleWarnings(warnings, options, isCloudDiff); if (results.length === 0) { - logger.info( - 'No comparisons were run between specs - `optic diff-all` will run comparisons on any spec that has an `x-optic-url` key' - ); + logger.info('No comparisons were run between specs'); logger.info( 'Get started by running `optic api add` and making a change to an API spec' ); diff --git a/projects/optic/src/commands/diff/diff.ts b/projects/optic/src/commands/diff/diff.ts index f42a142271..18e93d8c89 100644 --- a/projects/optic/src/commands/diff/diff.ts +++ b/projects/optic/src/commands/diff/diff.ts @@ -13,6 +13,7 @@ import { parseFilesFromRef, ParseResult, loadSpec, + parseFilesFromCloud, } from '../../utils/spec-loaders'; import { OpticCliConfig, VCS } from '../../config'; import chalk from 'chalk'; @@ -62,7 +63,7 @@ export const registerDiff = (cli: Command, config: OpticCliConfig) => { .argument('[file_to_compare_against]', 'path to file to compare with') .option( '--base ', - 'the base ref to compare against. Defaults to HEAD', + 'the base ref to compare against. Defaults to HEAD. Also supports optic cloud tags (cloud:tag_name)', 'HEAD' ) .option( @@ -122,14 +123,24 @@ const getBaseAndHeadFromFileAndBase = async ( headStrict: boolean ): Promise<[ParseResult, ParseResult]> => { try { - const { baseFile, headFile } = await parseFilesFromRef( - file1, - base, - root, - config, - { denormalize: true, headStrict: headStrict } - ); - return [baseFile, headFile]; + if (/^cloud:/.test(base)) { + const { baseFile, headFile } = await parseFilesFromCloud( + file1, + base.replace(/^cloud:/, ''), + config, + { denormalize: true, headStrict: headStrict } + ); + return [baseFile, headFile]; + } else { + const { baseFile, headFile } = await parseFilesFromRef( + file1, + base, + root, + config, + { denormalize: true, headStrict: headStrict } + ); + return [baseFile, headFile]; + } } catch (e) { console.error(e instanceof Error ? e.message : e); throw new UserError(); diff --git a/projects/optic/src/utils/cloud-urls.ts b/projects/optic/src/utils/cloud-urls.ts index 2212791ea3..fcf0c8bc58 100644 --- a/projects/optic/src/utils/cloud-urls.ts +++ b/projects/optic/src/utils/cloud-urls.ts @@ -7,8 +7,9 @@ const PATH_NAME_REGEXP = /^\/organizations\/([a-zA-Z0-9-_]+)\/apis\/([a-zA-Z0-9-_]+)$/i; export function getApiFromOpticUrl( - opticUrl: string + opticUrl: string | undefined ): { apiId: string; orgId: string } | null { + if (!opticUrl) return null; try { const url = new URL(opticUrl); const match = url.pathname.match(PATH_NAME_REGEXP); diff --git a/projects/optic/src/utils/spec-loaders.ts b/projects/optic/src/utils/spec-loaders.ts index 3adcc0dc08..4256099ecf 100644 --- a/projects/optic/src/utils/spec-loaders.ts +++ b/projects/optic/src/utils/spec-loaders.ts @@ -19,6 +19,9 @@ import * as Git from './git-utils'; import { createNullSpec, createNullSpecSourcemap } from './specs'; import { downloadSpec } from './cloud-specs'; import { OpticBackendClient } from '../client'; +import { getApiFromOpticUrl } from './cloud-urls'; +import { OPTIC_URL_KEY } from '../constants'; +import chalk from 'chalk'; const exec = promisify(callbackExec); @@ -343,6 +346,45 @@ export const parseFilesFromRef = async ( }; }; +export const parseFilesFromCloud = async ( + filePath: string, + cloudTag: string, + config: OpticCliConfig, + options: { + denormalize: boolean; + headStrict: boolean; + } +): Promise<{ + baseFile: ParseResult; + headFile: ParseResult; +}> => { + const headFile = await loadSpec(filePath, config, { + denormalize: options.denormalize, + strict: options.headStrict, + }); + + const maybeApi = getApiFromOpticUrl(headFile.jsonLike[OPTIC_URL_KEY]); + + if (!maybeApi) { + throw new Error( + `${chalk.bold.red( + "Must have an 'x-optic-url' in your OpenAPI spec file to be able to compare against a cloud base." + )}. + +${chalk.gray(`Get started by running 'optic api add ${filePath}'`)}` + ); + } + + const baseFile = await parseSpecAndDereference( + `cloud:${maybeApi.apiId}@${cloudTag}`, + config + ); + return { + baseFile, + headFile, + }; +}; + export const specHasUncommittedChanges = ( sourcemap: JsonSchemaSourcemap, diffSet: Set