From e3cee2c51869ef0786bd22c59a053d4e82ad70dc Mon Sep 17 00:00:00 2001 From: Nicholas Lim <18374483+niclim@users.noreply.github.com> Date: Wed, 26 Apr 2023 11:40:06 -0400 Subject: [PATCH 01/11] Add uncommited changes flag to diff and diff-all --- projects/optic/src/commands/diff/diff-all.ts | 7 +++++ projects/optic/src/commands/diff/diff.ts | 32 +++++++++++++++----- projects/optic/src/utils/spec-loaders.ts | 19 +++++++++--- 3 files changed, 47 insertions(+), 11 deletions(-) diff --git a/projects/optic/src/commands/diff/diff-all.ts b/projects/optic/src/commands/diff/diff-all.ts index d85989165b..d07f8cac2b 100644 --- a/projects/optic/src/commands/diff/diff-all.ts +++ b/projects/optic/src/commands/diff/diff-all.ts @@ -97,6 +97,11 @@ comma separated values (e.g. "**/*.yml,**/*.json")' .option('--upload', 'upload specs', false) .option('--web', 'view the diff in the optic changelog web view', false) .option('--json', 'output as json', false) + .option( + '--include-uncommited-changes', + 'include uncommitted changes and tag it against this spec. Use this if you generate specs in CI to upload. This option is generally not recommended for other cases as it means your uploaded spec may not match your git history.', + false + ) .option( '--fail-on-untracked-openapi', 'fail with exit code 1 if there are detected untracked apis', @@ -113,6 +118,7 @@ type DiffAllActionOptions = { ignore?: string; headTag?: string; check: boolean; + includeUncommittedChanges: boolean; web: boolean; upload: boolean; json: boolean; @@ -252,6 +258,7 @@ async function computeAll( toParseResults = await loadSpec(candidate.to, config, { strict: options.validation === 'strict', denormalize: true, + includeUncommittedChanges: options.includeUncommittedChanges, }); } catch (e) { allWarnings.unparseableToSpec.push({ diff --git a/projects/optic/src/commands/diff/diff.ts b/projects/optic/src/commands/diff/diff.ts index f42a142271..23f51c101f 100644 --- a/projects/optic/src/commands/diff/diff.ts +++ b/projects/optic/src/commands/diff/diff.ts @@ -94,6 +94,11 @@ export const registerDiff = (cli: Command, config: OpticCliConfig) => { .option('--upload', 'upload run to cloud', false) .option('--web', 'view the diff in the optic changelog web view', false) .option('--json', 'output as json', false) + .option( + '--include-uncommited-changes', + 'include uncommitted changes and tag it against this spec. Use this if you generate specs in CI to upload. This option is generally not recommended for other cases as it means your uploaded spec may not match your git history.', + false + ) .action(errorHandler(getDiffAction(config))); }; @@ -101,12 +106,20 @@ const getBaseAndHeadFromFiles = async ( file1: string, file2: string, config: OpticCliConfig, - strict: boolean + options: DiffActionOptions ): Promise<[ParseResult, ParseResult]> => { try { return await Promise.all([ - loadSpec(file1, config, { strict, denormalize: true }), - loadSpec(file2, config, { strict, denormalize: true }), + loadSpec(file1, config, { + strict: options.validation === 'strict', + denormalize: true, + includeUncommittedChanges: options.includeUncommittedChanges, + }), + loadSpec(file2, config, { + strict: options.validation === 'strict', + denormalize: true, + includeUncommittedChanges: options.includeUncommittedChanges, + }), ]); } catch (e) { console.error(e instanceof Error ? e.message : e); @@ -119,7 +132,7 @@ const getBaseAndHeadFromFileAndBase = async ( base: string, root: string, config: OpticCliConfig, - headStrict: boolean + options: DiffActionOptions ): Promise<[ParseResult, ParseResult]> => { try { const { baseFile, headFile } = await parseFilesFromRef( @@ -127,7 +140,11 @@ const getBaseAndHeadFromFileAndBase = async ( base, root, config, - { denormalize: true, headStrict: headStrict } + { + denormalize: true, + headStrict: options.validation === 'strict', + includeUncommittedChanges: options.includeUncommittedChanges, + } ); return [baseFile, headFile]; } catch (e) { @@ -229,6 +246,7 @@ type DiffActionOptions = { web: boolean; upload: boolean; json: boolean; + includeUncommittedChanges: boolean; standard?: string; ruleset?: string; headTag?: string; @@ -271,7 +289,7 @@ const getDiffAction = file1, file2, config, - options.validation === 'strict' + options ); } else if (file1) { if (config.vcs?.type !== VCS.Git) { @@ -287,7 +305,7 @@ const getDiffAction = options.base, config.root, config, - options.validation === 'strict' + options ); } else { logger.error( diff --git a/projects/optic/src/utils/spec-loaders.ts b/projects/optic/src/utils/spec-loaders.ts index 3adcc0dc08..7b2d721603 100644 --- a/projects/optic/src/utils/spec-loaders.ts +++ b/projects/optic/src/utils/spec-loaders.ts @@ -160,7 +160,10 @@ export async function loadRaw( async function parseSpecAndDereference( filePathOrRef: string | undefined, - config: OpticCliConfig + config: OpticCliConfig, + options: { + includeUncommittedChanges: boolean; + } = { includeUncommittedChanges: false } ): Promise { const workingDir = process.cwd(); const input = parseOpticRef(filePathOrRef); @@ -235,7 +238,8 @@ async function parseSpecAndDereference( if ( config.vcs?.type === VCS.Git && - !specHasUncommittedChanges(parseResult.sourcemap, config.vcs.diffSet) + (options.includeUncommittedChanges || + !specHasUncommittedChanges(parseResult.sourcemap, config.vcs.diffSet)) ) { const commitMeta = await Git.commitMeta(config.vcs.sha); @@ -284,9 +288,12 @@ export const loadSpec = async ( options: { strict: boolean; denormalize: boolean; + includeUncommittedChanges?: boolean; } ): Promise => { - const file = await parseSpecAndDereference(opticRef, config); + const file = await parseSpecAndDereference(opticRef, config, { + includeUncommittedChanges: options.includeUncommittedChanges ?? false, + }); return validateAndDenormalize(file, options); }; @@ -299,6 +306,7 @@ export const parseFilesFromRef = async ( options: { denormalize: boolean; headStrict: boolean; + includeUncommittedChanges: boolean; } ): Promise<{ baseFile: ParseResult; @@ -332,7 +340,10 @@ export const parseFilesFromRef = async ( }), headFile: await parseSpecAndDereference( existsOnHead ? absolutePath : undefined, - config + config, + { + includeUncommittedChanges: options.includeUncommittedChanges, + } ).then((file) => { return validateAndDenormalize(file, { denormalize: options.denormalize, From 68b3e9180b2dd6f2453ccf1cf93daa372bf65fad Mon Sep 17 00:00:00 2001 From: Nicholas Lim <18374483+niclim@users.noreply.github.com> Date: Wed, 26 Apr 2023 11:57:21 -0400 Subject: [PATCH 02/11] update diff to point to existing spec if referring to a cloud spec --- projects/optic/src/commands/api/add.ts | 6 ++- .../optic/src/commands/diff/upload-diff.ts | 45 ++++++++++--------- projects/optic/src/config.ts | 1 + projects/optic/src/utils/cloud-specs.ts | 27 ++++++++--- projects/optic/src/utils/spec-loaders.ts | 29 +++++++----- 5 files changed, 71 insertions(+), 37 deletions(-) diff --git a/projects/optic/src/commands/api/add.ts b/projects/optic/src/commands/api/add.ts index 0fc4b6eb6b..ec8e515554 100644 --- a/projects/optic/src/commands/api/add.ts +++ b/projects/optic/src/commands/api/add.ts @@ -194,7 +194,11 @@ async function crawlCandidateSpecs( orgId, forward_effective_at_to_tags: true, }); - specsToTag.push([specId, sha, parseResult.context?.effective_at]); + const effective_at = + parseResult.context?.vcs === 'git' + ? parseResult.context.effective_at + : undefined; + specsToTag.push([specId, sha, effective_at]); } if (!alreadyTracked) { diff --git a/projects/optic/src/commands/diff/upload-diff.ts b/projects/optic/src/commands/diff/upload-diff.ts index 24d67bf3a8..91f0ff3b84 100644 --- a/projects/optic/src/commands/diff/upload-diff.ts +++ b/projects/optic/src/commands/diff/upload-diff.ts @@ -39,25 +39,26 @@ export async function uploadDiff( let baseSpecId: string | null = null; let headSpecId: string | null = null; if (specs.from.context && specDetails) { - const tags = - specs.from.context.vcs === VCS.Git - ? [`git:${specs.from.context.sha}`] - : []; - baseSpecId = await uploadSpec(specDetails.apiId, { - spec: specs.from, - client: config.client, - tags, - orgId: specDetails.orgId, - }); + if (specs.from.context.vcs === VCS.Git) { + const tags = [`git:${specs.from.context.sha}`]; + baseSpecId = await uploadSpec(specDetails.apiId, { + spec: specs.from, + client: config.client, + tags, + orgId: specDetails.orgId, + }); + } else if (specs.from.context.vcs === VCS.Cloud) { + baseSpecId = specs.from.context.specId; + } } else if (specs.from.isEmptySpec) { baseSpecId = EMPTY_SPEC_ID; } if (specs.to.context && specDetails) { - let tags: string[] = []; - const tagsFromOptions = getTagsFromOptions(options.headTag); - tags.push(...tagsFromOptions); if (specs.to.context.vcs === VCS.Git) { + let tags: string[] = []; + const tagsFromOptions = getTagsFromOptions(options.headTag); + tags.push(...tagsFromOptions); tags.push(`git:${specs.to.context.sha}`); // If no gitbranch is set, try to add own git branch if (!tagsFromOptions.some((tag) => /^gitbranch\:/.test(tag))) { @@ -73,15 +74,17 @@ export async function uploadDiff( ); } } - } - tags = getUniqueTags(tags); - headSpecId = await uploadSpec(specDetails.apiId, { - spec: specs.to, - client: config.client, - tags, - orgId: specDetails.orgId, - }); + tags = getUniqueTags(tags); + headSpecId = await uploadSpec(specDetails.apiId, { + spec: specs.to, + client: config.client, + tags, + orgId: specDetails.orgId, + }); + } else if (specs.to.context.vcs === VCS.Cloud) { + headSpecId = specs.to.context.specId; + } } else if (specs.to.isEmptySpec) { headSpecId = EMPTY_SPEC_ID; } diff --git a/projects/optic/src/config.ts b/projects/optic/src/config.ts index 691984b0f5..03e883ec5a 100644 --- a/projects/optic/src/config.ts +++ b/projects/optic/src/config.ts @@ -10,6 +10,7 @@ import { logger } from './logger'; export enum VCS { Git = 'git', + Cloud = 'cloud', // hosted in optic cloud } export const OPTIC_YML_NAME = 'optic.yml'; diff --git a/projects/optic/src/utils/cloud-specs.ts b/projects/optic/src/utils/cloud-specs.ts index 3297e2091e..5674d758f8 100644 --- a/projects/optic/src/utils/cloud-specs.ts +++ b/projects/optic/src/utils/cloud-specs.ts @@ -3,7 +3,6 @@ import { CompareSpecResults, UserError, ApiCoverage, - sourcemapReader, } from '@useoptic/openapi-utilities'; import { OpticBackendClient } from '../client'; import { computeChecksumForAws } from './checksum'; @@ -24,6 +23,9 @@ export async function downloadSpec( ): Promise<{ jsonLike: ParseResult['jsonLike']; sourcemap: ParseResult['sourcemap']; + spec: { + id: string; + }; }> { const response = await opts.client.getSpec(spec.apiId, spec.tag); @@ -34,6 +36,9 @@ export async function downloadSpec( return { jsonLike: spec, sourcemap, + spec: { + id: response.id, + }, }; } else { // fetch from cloud @@ -46,6 +51,9 @@ export async function downloadSpec( sourcemap: JsonSchemaSourcemap.fromSerializedSourcemap( JSON.parse(sourcemapStr) ), + spec: { + id: response.id, + }, }; } } @@ -99,10 +107,19 @@ export async function uploadSpec( }), ]); - const effective_at = opts.spec.context?.effective_at; - const git_name = opts.spec.context?.name; - const git_email = opts.spec.context?.email; - const commit_message = opts.spec.context?.message; + let effective_at: Date | undefined = undefined; + let git_name: string | undefined = undefined; + let git_email: string | undefined = undefined; + let commit_message: string | undefined = undefined; + + if (opts.spec.context?.vcs === 'git') { + ({ + effective_at, + name: git_name, + email: git_email, + message: commit_message, + } = opts.spec.context); + } const { id } = await opts.client.createSpec({ upload_id: result.upload_id, diff --git a/projects/optic/src/utils/spec-loaders.ts b/projects/optic/src/utils/spec-loaders.ts index 7b2d721603..28a775855e 100644 --- a/projects/optic/src/utils/spec-loaders.ts +++ b/projects/optic/src/utils/spec-loaders.ts @@ -22,14 +22,20 @@ import { OpticBackendClient } from '../client'; const exec = promisify(callbackExec); -export type ParseResultContext = { - vcs: 'git'; - sha: string; - effective_at?: Date; - name: string; - email: string; - message: string; -} | null; +export type ParseResultContext = + | { + vcs: 'git'; + sha: string; + effective_at?: Date; + name: string; + email: string; + message: string; + } + | { + vcs: 'cloud'; + specId: string; + } + | null; export type ParseResult = ParseOpenAPIResult & { isEmptySpec: boolean; @@ -183,7 +189,7 @@ async function parseSpecAndDereference( case 'cloud': { // try fetch from cloud, if 404 return an error // todo handle empty spec case - const { jsonLike, sourcemap } = await downloadSpec( + const { jsonLike, sourcemap, spec } = await downloadSpec( { apiId: input.apiId, tag: input.tag }, config ); @@ -192,7 +198,10 @@ async function parseSpecAndDereference( sourcemap, from: 'cloud', isEmptySpec: false, - context: null, + context: { + vcs: 'cloud', + specId: spec.id, + }, }; } case 'git': { From 6c213b5d247b5721748d4c1a6b682edf23b650c3 Mon Sep 17 00:00:00 2001 From: Nicholas Lim <18374483+niclim@users.noreply.github.com> Date: Thu, 27 Apr 2023 14:47:26 -0400 Subject: [PATCH 03/11] Change to use generated flag --- projects/optic/src/commands/diff/diff-all.ts | 10 +++------- projects/optic/src/commands/diff/diff.ts | 16 ++++++---------- 2 files changed, 9 insertions(+), 17 deletions(-) diff --git a/projects/optic/src/commands/diff/diff-all.ts b/projects/optic/src/commands/diff/diff-all.ts index a66666d7d3..88a89463c9 100644 --- a/projects/optic/src/commands/diff/diff-all.ts +++ b/projects/optic/src/commands/diff/diff-all.ts @@ -97,11 +97,7 @@ comma separated values (e.g. "**/*.yml,**/*.json")' .option('--upload', 'upload specs', false) .option('--web', 'view the diff in the optic changelog web view', false) .option('--json', 'output as json', false) - .option( - '--include-uncommited-changes', - 'include uncommitted changes and tag it against this spec. Use this if you generate specs in CI to upload. This option is generally not recommended for other cases as it means your uploaded spec may not match your git history.', - false - ) + .option('--generated', 'use with --upload with a generated spec', false) .option( '--fail-on-untracked-openapi', 'fail with exit code 1 if there are detected untracked apis', @@ -118,7 +114,7 @@ type DiffAllActionOptions = { ignore?: string; headTag?: string; check: boolean; - includeUncommittedChanges: boolean; + generated: boolean; web: boolean; upload: boolean; json: boolean; @@ -289,7 +285,7 @@ async function computeAll( toParseResults = await loadSpec(candidate.to, config, { strict: options.validation === 'strict', denormalize: true, - includeUncommittedChanges: options.includeUncommittedChanges, + includeUncommittedChanges: options.generated, }); } catch (e) { allWarnings.unparseableToSpec.push({ diff --git a/projects/optic/src/commands/diff/diff.ts b/projects/optic/src/commands/diff/diff.ts index a0ab31925b..bd56fe7c2e 100644 --- a/projects/optic/src/commands/diff/diff.ts +++ b/projects/optic/src/commands/diff/diff.ts @@ -95,11 +95,7 @@ export const registerDiff = (cli: Command, config: OpticCliConfig) => { .option('--upload', 'upload run to cloud', false) .option('--web', 'view the diff in the optic changelog web view', false) .option('--json', 'output as json', false) - .option( - '--include-uncommited-changes', - 'include uncommitted changes and tag it against this spec. Use this if you generate specs in CI to upload. This option is generally not recommended for other cases as it means your uploaded spec may not match your git history.', - false - ) + .option('--generated', 'use with --upload with a generated spec', false) .action(errorHandler(getDiffAction(config))); }; @@ -114,12 +110,12 @@ const getBaseAndHeadFromFiles = async ( loadSpec(file1, config, { strict: options.validation === 'strict', denormalize: true, - includeUncommittedChanges: options.includeUncommittedChanges, + includeUncommittedChanges: options.generated, }), loadSpec(file2, config, { strict: options.validation === 'strict', denormalize: true, - includeUncommittedChanges: options.includeUncommittedChanges, + includeUncommittedChanges: options.generated, }), ]); } catch (e) { @@ -144,7 +140,7 @@ const getBaseAndHeadFromFileAndBase = async ( { denormalize: true, headStrict: options.validation === 'strict', - includeUncommittedChanges: options.includeUncommittedChanges, + includeUncommittedChanges: options.generated, } ); return [baseFile, headFile]; @@ -157,7 +153,7 @@ const getBaseAndHeadFromFileAndBase = async ( { denormalize: true, headStrict: options.validation === 'strict', - includeUncommittedChanges: options.includeUncommittedChanges, + includeUncommittedChanges: options.generated, } ); return [baseFile, headFile]; @@ -261,7 +257,7 @@ type DiffActionOptions = { web: boolean; upload: boolean; json: boolean; - includeUncommittedChanges: boolean; + generated: boolean; standard?: string; ruleset?: string; headTag?: string; From 3b11620173da3bc0d7b8e07fbc5b43a0ead4bc61 Mon Sep 17 00:00:00 2001 From: Nicholas Lim <18374483+niclim@users.noreply.github.com> Date: Thu, 27 Apr 2023 16:37:01 -0400 Subject: [PATCH 04/11] Update diff-all to handle generated specs --- .../optic/src/client/optic-backend-types.ts | 6 + projects/optic/src/client/optic-backend.ts | 11 ++ projects/optic/src/commands/api/add.ts | 2 +- projects/optic/src/commands/diff/diff-all.ts | 134 +++++++++++++++--- projects/optic/src/utils/organization.ts | 32 +++-- 5 files changed, 148 insertions(+), 37 deletions(-) diff --git a/projects/optic/src/client/optic-backend-types.ts b/projects/optic/src/client/optic-backend-types.ts index 8904509d24..0607758feb 100644 --- a/projects/optic/src/client/optic-backend-types.ts +++ b/projects/optic/src/client/optic-backend-types.ts @@ -11,3 +11,9 @@ export type Standard = { }; export type StandardConfig = { name: string; config: any }[]; + +export type Api = { + api_id: string; + organization_id: string; + path: string; +}; diff --git a/projects/optic/src/client/optic-backend.ts b/projects/optic/src/client/optic-backend.ts index eb338130d4..6170d16efd 100644 --- a/projects/optic/src/client/optic-backend.ts +++ b/projects/optic/src/client/optic-backend.ts @@ -178,6 +178,17 @@ export class OpticBackendClient extends JsonHttpClient { return this.postJson(`/api/runs2`, run); } + public async getApis( + paths: string[], + web_url: string + ): Promise<{ apis: (Types.Api | null)[] }> { + return this.getJson<{ apis: (Types.Api | null)[] }>( + `/api/apis?paths=${paths + .map((p) => encodeURIComponent(p)) + .join(',')}&web_url=${encodeURIComponent(web_url)}` + ); + } + public async createApi( organizationId: string, opts: { diff --git a/projects/optic/src/commands/api/add.ts b/projects/optic/src/commands/api/add.ts index ec8e515554..11e4db1177 100644 --- a/projects/optic/src/commands/api/add.ts +++ b/projects/optic/src/commands/api/add.ts @@ -1,7 +1,7 @@ import { Command } from 'commander'; import prompts from 'prompts'; import open from 'open'; -import path, { parse } from 'path'; +import path from 'path'; import fs from 'node:fs/promises'; import ora from 'ora'; import { OpticCliConfig, VCS } from '../../config'; diff --git a/projects/optic/src/commands/diff/diff-all.ts b/projects/optic/src/commands/diff/diff-all.ts index 88a89463c9..4ddb42d63b 100644 --- a/projects/optic/src/commands/diff/diff-all.ts +++ b/projects/optic/src/commands/diff/diff-all.ts @@ -25,6 +25,9 @@ import { errorHandler } from '../../error-handler'; import { checkOpenAPIVersion } from '@useoptic/openapi-io'; import { generateComparisonLogsV2 } from '../../utils/diff-renderer'; import path from 'path'; +import * as Git from '../../utils/git-utils'; +import { getApiUrl } from '../../utils/cloud-urls'; +import { getOrganizationFromToken } from '../../utils/organization'; const usage = () => ` optic diff-all @@ -205,6 +208,15 @@ async function computeAll( }; const results: Result[] = []; + const comparisons: Map< + string, + { + from?: string; + to?: string; + opticUrl?: string; + cloudTag: string | null; + } + > = new Map(); 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 @@ -241,19 +253,13 @@ async function computeAll( } let fromRef = candidate.from; + const opticUrl = rawSpec[OPTIC_URL_KEY]; try { - const opticApi = getApiFromOpticUrl(rawSpec[OPTIC_URL_KEY]); + const opticApi = getApiFromOpticUrl(opticUrl); + checkOpenAPIVersion(rawSpec); - if (!opticApi && (options.upload || cloudTag)) { - logger.debug( - `Skipping comparison from ${candidate.from} to ${candidate.to} because there was no x-optic-url` - ); - allWarnings.missingOpticUrl.push({ - path: candidate.to!, - }); - continue; - } else if (opticApi && cloudTag) { + if (opticApi && cloudTag) { fromRef = `cloud:${opticApi.apiId}@${cloudTag}`; } } catch (e) { @@ -265,47 +271,131 @@ async function computeAll( continue; } + const path = candidate.to ?? candidate.from; + // should never happen + if (!path) continue; + + comparisons.set(path, { + from: fromRef, + to: candidate.to, + opticUrl, + cloudTag, + }); + } + + if (options.generated) { + let default_branch: string = 'main'; + let default_tag: string = 'gitbranch:main'; + const maybeOrigin = await Git.guessRemoteOrigin(); + const orgRes = await getOrganizationFromToken(config.client, false); + const maybeDefaultBranch = await Git.getDefaultBranchName(); + if (maybeDefaultBranch) { + default_branch = maybeDefaultBranch; + default_tag = `gitbranch:${default_branch}`; + } + if (maybeOrigin && orgRes.ok) { + const pathToUrl: Record = {}; + for (const [path, comparison] of comparisons.entries()) { + if (!comparison.opticUrl) { + pathToUrl[path] = null; + } + } + + const { apis } = await config.client.getApis( + Object.keys(pathToUrl), + maybeOrigin.web_url + ); + + for (const api of apis) { + if (api) { + pathToUrl[api.path] = getApiUrl( + config.client.getWebBase(), + api.organization_id, + api.api_id + ); + } + } + for (const [path, url] of Object.entries(pathToUrl)) { + if (!url) { + const api = await config.client.createApi(orgRes.org.id, { + name: path, + web_url: maybeOrigin.web_url, + default_branch, + default_tag, + }); + pathToUrl[path] = getApiUrl( + config.client.getWebBase(), + orgRes.org.id, + api.id + ); + } + } + } else if (!maybeOrigin) { + logger.warn( + chalk.yellow( + 'Could not guess the git remote origin - cannot automatically connect untracked apis with optic cloud' + ) + ); + logger.warn( + `To fix this, ensure that the git remote is set, or manually add x-optic-url to the specs you want to track.` + ); + } else if (!orgRes.ok) { + logger.error(orgRes.error); + logger.error( + 'skipping automatically connect untracked apis with optic cloud' + ); + } + } + + for (const { from, to, opticUrl, cloudTag } of comparisons.values()) { + const opticApi = getApiFromOpticUrl(opticUrl); + + if (!opticApi && (options.upload || cloudTag)) { + logger.debug( + `Skipping comparison from ${from} to ${to} because there was no x-optic-url` + ); + allWarnings.missingOpticUrl.push({ + path: to!, + }); + continue; + } // try load both from + to spec let fromParseResults: ParseResult; let toParseResults: ParseResult; try { - fromParseResults = await loadSpec(fromRef, config, { + fromParseResults = await loadSpec(from, config, { strict: false, denormalize: true, }); } catch (e) { allWarnings.unparseableFromSpec.push({ - path: candidate.from!, + path: from!, error: e, }); continue; } try { - toParseResults = await loadSpec(candidate.to, config, { + toParseResults = await loadSpec(to, config, { strict: options.validation === 'strict', denormalize: true, includeUncommittedChanges: options.generated, }); } catch (e) { allWarnings.unparseableToSpec.push({ - path: candidate.to!, + path: to!, error: e, }); continue; } logger.info( - chalk.blue( - `Diffing ${candidate.from ?? 'empty spec'} to ${ - candidate.to ?? 'empty spec' - }` - ) + chalk.blue(`Diffing ${from ?? 'empty spec'} to ${to ?? 'empty spec'}`) ); const { specResults, checks, changelogData, warnings } = await compute( [fromParseResults, toParseResults], config, - { ...options, path: candidate.to ?? candidate.from ?? null } + { ...options, path: to ?? from ?? null } ); for (const warning of warnings) { @@ -369,8 +459,8 @@ async function computeAll( specResults, checks, changelogData, - from: candidate.from, - to: candidate.to, + from, + to, changelogUrl, specUrl, }); diff --git a/projects/optic/src/utils/organization.ts b/projects/optic/src/utils/organization.ts index 487b498a4e..bc96cb978a 100644 --- a/projects/optic/src/utils/organization.ts +++ b/projects/optic/src/utils/organization.ts @@ -3,7 +3,7 @@ import { OpticBackendClient } from '../client'; export async function getOrganizationFromToken( client: OpticBackendClient, - message: string + message: string | false ): Promise< | { ok: true; @@ -18,19 +18,23 @@ export async function getOrganizationFromToken( const { organizations } = await client.getTokenOrgs(); if (organizations.length > 1) { - const response = await prompts( - { - type: 'select', - name: 'orgId', - message, - choices: organizations.map((org) => ({ - title: org.name, - value: org.id, - })), - }, - { onCancel: () => process.exit(1) } - ); - org = organizations.find((o) => o.id === response.orgId)!; + if (message) { + const response = await prompts( + { + type: 'select', + name: 'orgId', + message, + choices: organizations.map((org) => ({ + title: org.name, + value: org.id, + })), + }, + { onCancel: () => process.exit(1) } + ); + org = organizations.find((o) => o.id === response.orgId)!; + } else { + org = organizations[0]; + } } else if (organizations.length === 0) { process.exitCode = 1; return { From b3338c1d0bd702c473fed8ba364b08ea580df5bc Mon Sep 17 00:00:00 2001 From: Nicholas Lim <18374483+niclim@users.noreply.github.com> Date: Thu, 27 Apr 2023 16:54:14 -0400 Subject: [PATCH 05/11] Support generated in diff --- projects/optic/src/commands/diff/diff-all.ts | 42 +++++----------- projects/optic/src/commands/diff/diff.ts | 44 +++++++++++++++++ .../optic/src/commands/diff/upload-diff.ts | 16 ++----- projects/optic/src/utils/cloud-specs.ts | 2 + projects/optic/src/utils/generated.ts | 48 +++++++++++++++++++ 5 files changed, 111 insertions(+), 41 deletions(-) create mode 100644 projects/optic/src/utils/generated.ts diff --git a/projects/optic/src/commands/diff/diff-all.ts b/projects/optic/src/commands/diff/diff-all.ts index 4ddb42d63b..3f73a3bed4 100644 --- a/projects/optic/src/commands/diff/diff-all.ts +++ b/projects/optic/src/commands/diff/diff-all.ts @@ -28,6 +28,7 @@ import path from 'path'; import * as Git from '../../utils/git-utils'; import { getApiUrl } from '../../utils/cloud-urls'; import { getOrganizationFromToken } from '../../utils/organization'; +import { getDetailsForGeneration } from '../../utils/generated'; const usage = () => ` optic diff-all @@ -284,16 +285,10 @@ async function computeAll( } if (options.generated) { - let default_branch: string = 'main'; - let default_tag: string = 'gitbranch:main'; - const maybeOrigin = await Git.guessRemoteOrigin(); - const orgRes = await getOrganizationFromToken(config.client, false); - const maybeDefaultBranch = await Git.getDefaultBranchName(); - if (maybeDefaultBranch) { - default_branch = maybeDefaultBranch; - default_tag = `gitbranch:${default_branch}`; - } - if (maybeOrigin && orgRes.ok) { + const generatedDetails = await getDetailsForGeneration(config); + if (generatedDetails) { + const { web_url, organization_id, default_branch, default_tag } = + generatedDetails; const pathToUrl: Record = {}; for (const [path, comparison] of comparisons.entries()) { if (!comparison.opticUrl) { @@ -303,7 +298,7 @@ async function computeAll( const { apis } = await config.client.getApis( Object.keys(pathToUrl), - maybeOrigin.web_url + web_url ); for (const api of apis) { @@ -317,40 +312,26 @@ async function computeAll( } for (const [path, url] of Object.entries(pathToUrl)) { if (!url) { - const api = await config.client.createApi(orgRes.org.id, { + const api = await config.client.createApi(organization_id, { name: path, - web_url: maybeOrigin.web_url, + web_url: web_url, default_branch, default_tag, }); pathToUrl[path] = getApiUrl( config.client.getWebBase(), - orgRes.org.id, + organization_id, api.id ); } } - } else if (!maybeOrigin) { - logger.warn( - chalk.yellow( - 'Could not guess the git remote origin - cannot automatically connect untracked apis with optic cloud' - ) - ); - logger.warn( - `To fix this, ensure that the git remote is set, or manually add x-optic-url to the specs you want to track.` - ); - } else if (!orgRes.ok) { - logger.error(orgRes.error); - logger.error( - 'skipping automatically connect untracked apis with optic cloud' - ); } } for (const { from, to, opticUrl, cloudTag } of comparisons.values()) { - const opticApi = getApiFromOpticUrl(opticUrl); + const specDetails = getApiFromOpticUrl(opticUrl); - if (!opticApi && (options.upload || cloudTag)) { + if (!specDetails && (options.upload || cloudTag)) { logger.debug( `Skipping comparison from ${from} to ${to} because there was no x-optic-url` ); @@ -446,6 +427,7 @@ async function computeAll( }, specResults, config, + specDetails, options ); specUrl = uploadResults?.headSpecUrl ?? null; diff --git a/projects/optic/src/commands/diff/diff.ts b/projects/optic/src/commands/diff/diff.ts index bd56fe7c2e..b0ea2d26d7 100644 --- a/projects/optic/src/commands/diff/diff.ts +++ b/projects/optic/src/commands/diff/diff.ts @@ -28,6 +28,11 @@ import { writeDataForCi } from '../../utils/ci-data'; import { logger } from '../../logger'; import { errorHandler } from '../../error-handler'; import path from 'path'; +import { OPTIC_URL_KEY } from '../../constants'; +import { getApiFromOpticUrl, getApiUrl } from '../../utils/cloud-urls'; +import * as Git from '../../utils/git-utils'; +import { getOrganizationFromToken } from '../../utils/organization'; +import { getDetailsForGeneration } from '../../utils/generated'; const description = `run a diff between two API specs`; @@ -332,6 +337,44 @@ const getDiffAction = const [baseParseResult, headParseResult] = parsedFiles; if (options.upload) { + const opticUrl: string | null = + headParseResult.jsonLike[OPTIC_URL_KEY] ?? + baseParseResult.jsonLike[OPTIC_URL_KEY] ?? + null; + let specDetails = opticUrl ? getApiFromOpticUrl(opticUrl) : null; + + if (options.generated && !specDetails) { + const path = file1; + const generatedDetails = await getDetailsForGeneration(config); + if (generatedDetails) { + const { web_url, organization_id, default_branch, default_tag } = + generatedDetails; + + const { apis } = await config.client.getApis([path], web_url); + let url: string; + if (!apis[0]) { + const api = await config.client.createApi(organization_id, { + name: path, + web_url: web_url, + default_branch, + default_tag, + }); + url = getApiUrl( + config.client.getWebBase(), + organization_id, + api.id + ); + } else { + url = getApiUrl( + config.client.getWebBase(), + organization_id, + apis[0].api_id + ); + } + specDetails = getApiFromOpticUrl(url); + } + } + const uploadResults = await uploadDiff( { from: baseParseResult, @@ -339,6 +382,7 @@ const getDiffAction = }, diffResult.specResults, config, + specDetails, options ); specUrl = uploadResults?.headSpecUrl ?? null; diff --git a/projects/optic/src/commands/diff/upload-diff.ts b/projects/optic/src/commands/diff/upload-diff.ts index 91f0ff3b84..049f8ef809 100644 --- a/projects/optic/src/commands/diff/upload-diff.ts +++ b/projects/optic/src/commands/diff/upload-diff.ts @@ -1,11 +1,6 @@ import ora from 'ora'; import { OpticCliConfig, VCS } from '../../config'; -import { OPTIC_URL_KEY } from '../../constants'; -import { - getApiFromOpticUrl, - getRunUrl, - getSpecUrl, -} from '../../utils/cloud-urls'; +import { getRunUrl, getSpecUrl } from '../../utils/cloud-urls'; import { ParseResult } from '../../utils/spec-loaders'; import { EMPTY_SPEC_ID, uploadRun, uploadSpec } from '../../utils/cloud-specs'; import * as Git from '../../utils/git-utils'; @@ -17,6 +12,10 @@ export async function uploadDiff( specs: { from: ParseResult; to: ParseResult }, specResults: Parameters['1']['specResults'], config: OpticCliConfig, + specDetails: { + apiId: string; + orgId: string; + } | null, options: { headTag?: string; } = {} @@ -30,11 +29,6 @@ export async function uploadDiff( ? ora({ text: `Uploading diff...`, color: 'blue' }) : null; - const opticUrl: string | null = - specs.to.jsonLike[OPTIC_URL_KEY] ?? - specs.from.jsonLike[OPTIC_URL_KEY] ?? - null; - const specDetails = opticUrl ? getApiFromOpticUrl(opticUrl) : null; // We upload a spec if it is unchanged in git and there is an API id on the spec let baseSpecId: string | null = null; let headSpecId: string | null = null; diff --git a/projects/optic/src/utils/cloud-specs.ts b/projects/optic/src/utils/cloud-specs.ts index 5674d758f8..55fb1b367a 100644 --- a/projects/optic/src/utils/cloud-specs.ts +++ b/projects/optic/src/utils/cloud-specs.ts @@ -17,6 +17,8 @@ import { JsonSchemaSourcemap } from '@useoptic/openapi-io'; export const EMPTY_SPEC_ID = 'EMPTY'; +export async function getOrCreateApis() {} + export async function downloadSpec( spec: { apiId: string; tag: string }, opts: { client: OpticBackendClient } diff --git a/projects/optic/src/utils/generated.ts b/projects/optic/src/utils/generated.ts new file mode 100644 index 0000000000..bc7e06a87c --- /dev/null +++ b/projects/optic/src/utils/generated.ts @@ -0,0 +1,48 @@ +import * as Git from './git-utils'; +import { getOrganizationFromToken } from './organization'; +import { OpticCliConfig } from '../config'; +import { logger } from '../logger'; +import chalk from 'chalk'; + +export async function getDetailsForGeneration(config: OpticCliConfig): Promise<{ + default_branch: string; + default_tag: string; + web_url: string; + organization_id: string; +} | null> { + let default_branch: string = 'main'; + let default_tag: string = 'gitbranch:main'; + const maybeOrigin = await Git.guessRemoteOrigin(); + const orgRes = await getOrganizationFromToken(config.client, false); + const maybeDefaultBranch = await Git.getDefaultBranchName(); + if (maybeDefaultBranch) { + default_branch = maybeDefaultBranch; + default_tag = `gitbranch:${default_branch}`; + } + + if (maybeOrigin && orgRes.ok) { + return { + default_branch, + default_tag, + web_url: maybeOrigin.web_url, + organization_id: orgRes.org.id, + }; + } else if (!maybeOrigin) { + logger.warn( + chalk.yellow( + 'Could not guess the git remote origin - cannot automatically connect untracked apis with optic cloud' + ) + ); + logger.warn( + `To fix this, ensure that the git remote is set, or manually add x-optic-url to the specs you want to track.` + ); + return null; + } else if (!orgRes.ok) { + logger.error(orgRes.error); + logger.error( + 'skipping automatically connect untracked apis with optic cloud' + ); + return null; + } + return null; +} From c4d6b38ff3f925eb5930242e2005fb18a58475eb Mon Sep 17 00:00:00 2001 From: Nicholas Lim <18374483+niclim@users.noreply.github.com> Date: Thu, 27 Apr 2023 17:03:48 -0400 Subject: [PATCH 06/11] rm unused code --- projects/optic/src/utils/cloud-specs.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/projects/optic/src/utils/cloud-specs.ts b/projects/optic/src/utils/cloud-specs.ts index 55fb1b367a..5674d758f8 100644 --- a/projects/optic/src/utils/cloud-specs.ts +++ b/projects/optic/src/utils/cloud-specs.ts @@ -17,8 +17,6 @@ import { JsonSchemaSourcemap } from '@useoptic/openapi-io'; export const EMPTY_SPEC_ID = 'EMPTY'; -export async function getOrCreateApis() {} - export async function downloadSpec( spec: { apiId: string; tag: string }, opts: { client: OpticBackendClient } From 5f7fccd8a98043df19068a7ee7a89f913d3f6704 Mon Sep 17 00:00:00 2001 From: Nicholas Lim <18374483+niclim@users.noreply.github.com> Date: Thu, 27 Apr 2023 17:45:24 -0400 Subject: [PATCH 07/11] write test for diff-all --- .../__snapshots__/diff-all.test.ts.snap | 70 ++++++++++++++++++- .../__tests__/integration/diff-all.test.ts | 34 +++++++++ .../diff-all/cloud-diff/spec-no-url.json | 39 +++++++++++ projects/optic/src/commands/diff/diff-all.ts | 36 ++++------ 4 files changed, 156 insertions(+), 23 deletions(-) create mode 100644 projects/optic/src/__tests__/integration/workspaces/diff-all/cloud-diff/spec-no-url.json 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 f9cfe4490a..494b89c647 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,7 +1,7 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`diff-all diff all against a cloud tag 1`] = ` -"Diffing cloud:main to spec.json +"Diffing cloud:api-id@main to spec.json specification details: - /x-optic-url added @@ -30,6 +30,74 @@ Checks 0 passed 1 errors +Warning - the following OpenAPI specs were detected but did not have valid x-optic-url keys. optic diff-all --compare-from cloud:{tag}' can only runs on specs that have been added to optic + +Run the \`optic api add\` command to add these specs to optic +spec-no-url.json (untracked) + +" +`; + +exports[`diff-all diff all in --generated 1`] = ` +"Diffing cloud:generated-api@main to spec-no-url.json + +specification details: +- /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 + +✔ Uploaded results of diff to http://localhost:3001/organizations/org-id/apis/generated-api/runs/run-id +Diffing cloud:api-id@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 + +✔ Uploaded results of diff to http://localhost:3001/organizations/org-id/apis/api-id/runs/run-id " `; diff --git a/projects/optic/src/__tests__/integration/diff-all.test.ts b/projects/optic/src/__tests__/integration/diff-all.test.ts index 31276b5705..ec96959dfb 100644 --- a/projects/optic/src/__tests__/integration/diff-all.test.ts +++ b/projects/optic/src/__tests__/integration/diff-all.test.ts @@ -42,6 +42,18 @@ setupTestServer(({ url, method }) => { specUrl: `${process.env.BWTS_HOST_OVERRIDE}/spec`, sourcemapUrl: `${process.env.BWTS_HOST_OVERRIDE}/sourcemap`, }); + } else if (method === 'GET' && /\/api\/apis/.test(url)) { + return JSON.stringify({ + apis: [null], + }); + } else if (method === 'POST' && /\/api\/api$/.test(url)) { + return JSON.stringify({ + id: 'generated-api', + }); + } else if (method === 'GET' && /\/api\/token\/orgs/.test(url)) { + return JSON.stringify({ + organizations: [{ id: 'org-id', name: 'org-blah' }], + }); } return JSON.stringify({}); }); @@ -184,4 +196,26 @@ describe('diff-all', () => { expect(code).toBe(1); expect(normalizeWorkspace(workspace, combined)).toMatchSnapshot(); }); + + test('diff all in --generated', async () => { + const workspace = await setupWorkspace('diff-all/cloud-diff', { + repo: true, + commit: true, + }); + + await run( + `sed -i.bak 's/string/number/' spec-no-url.json spec-no-url.json`, + false, + workspace + ); + process.env.OPTIC_TOKEN = '123'; + + const { combined, code } = await runOptic( + workspace, + 'diff-all --compare-from cloud:main --check --upload --generated' + ); + + expect(code).toBe(1); + expect(normalizeWorkspace(workspace, combined)).toMatchSnapshot(); + }); }); diff --git a/projects/optic/src/__tests__/integration/workspaces/diff-all/cloud-diff/spec-no-url.json b/projects/optic/src/__tests__/integration/workspaces/diff-all/cloud-diff/spec-no-url.json new file mode 100644 index 0000000000..1248e7d5e7 --- /dev/null +++ b/projects/optic/src/__tests__/integration/workspaces/diff-all/cloud-diff/spec-no-url.json @@ -0,0 +1,39 @@ +{ + "openapi": "3.0.1", + "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 b70bc2f274..d51bd61d66 100644 --- a/projects/optic/src/commands/diff/diff-all.ts +++ b/projects/optic/src/commands/diff/diff-all.ts @@ -25,9 +25,7 @@ import { errorHandler } from '../../error-handler'; import { checkOpenAPIVersion } from '@useoptic/openapi-io'; import { generateComparisonLogsV2 } from '../../utils/diff-renderer'; import path from 'path'; -import * as Git from '../../utils/git-utils'; import { getApiUrl } from '../../utils/cloud-urls'; -import { getOrganizationFromToken } from '../../utils/organization'; import { getDetailsForGeneration } from '../../utils/generated'; const usage = () => ` @@ -215,7 +213,6 @@ async function computeAll( from?: string; to?: string; opticUrl?: string; - cloudTag: string | null; } > = new Map(); @@ -224,10 +221,6 @@ async function computeAll( // 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( @@ -253,16 +246,10 @@ async function computeAll( continue; } - let fromRef = candidate.from; const opticUrl = rawSpec[OPTIC_URL_KEY]; try { - const opticApi = getApiFromOpticUrl(opticUrl); - checkOpenAPIVersion(rawSpec); - if (opticApi && cloudTag) { - fromRef = `cloud:${opticApi.apiId}@${cloudTag}`; - } } catch (e) { logger.debug( `Skipping comparison from ${candidate.from} to ${candidate.to} because of error: ` @@ -277,10 +264,9 @@ async function computeAll( if (!path) continue; comparisons.set(path, { - from: fromRef, + from: candidate.from, to: candidate.to, opticUrl, - cloudTag, }); } @@ -289,6 +275,7 @@ async function computeAll( if (generatedDetails) { const { web_url, organization_id, default_branch, default_tag } = generatedDetails; + const pathToUrl: Record = {}; for (const [path, comparison] of comparisons.entries()) { if (!comparison.opticUrl) { @@ -310,7 +297,8 @@ async function computeAll( ); } } - for (const [path, url] of Object.entries(pathToUrl)) { + + for (let [path, url] of Object.entries(pathToUrl)) { if (!url) { const api = await config.client.createApi(organization_id, { name: path, @@ -318,17 +306,18 @@ async function computeAll( default_branch, default_tag, }); - pathToUrl[path] = getApiUrl( - config.client.getWebBase(), - organization_id, - api.id - ); + url = getApiUrl(config.client.getWebBase(), organization_id, api.id); } + const comparison = comparisons.get(path); + if (comparison) comparison.opticUrl = url; } } } - for (const { from, to, opticUrl, cloudTag } of comparisons.values()) { + for (let { from, to, opticUrl } of comparisons.values()) { + const cloudTag: string | null = + !!from && /^cloud:/.test(from) ? from.replace(/^cloud:/, '') : null; + const specDetails = getApiFromOpticUrl(opticUrl); if (!specDetails && (options.upload || cloudTag)) { @@ -339,7 +328,10 @@ async function computeAll( path: to!, }); continue; + } else if (specDetails && cloudTag) { + from = `cloud:${specDetails.apiId}@${cloudTag}`; } + // try load both from + to spec let fromParseResults: ParseResult; let toParseResults: ParseResult; From 424b24737eea43d57ef2bd864e12647b69c5107b Mon Sep 17 00:00:00 2001 From: Nicholas Lim <18374483+niclim@users.noreply.github.com> Date: Thu, 27 Apr 2023 18:01:55 -0400 Subject: [PATCH 08/11] fix pass path to create api --- projects/optic/src/client/optic-backend.ts | 1 + projects/optic/src/commands/diff/diff-all.ts | 13 +++++++------ 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/projects/optic/src/client/optic-backend.ts b/projects/optic/src/client/optic-backend.ts index 7158b9799c..d47b069c04 100644 --- a/projects/optic/src/client/optic-backend.ts +++ b/projects/optic/src/client/optic-backend.ts @@ -193,6 +193,7 @@ export class OpticBackendClient extends JsonHttpClient { organizationId: string, opts: { name: string; + path?: string; web_url?: string; default_branch: string; default_tag?: string; diff --git a/projects/optic/src/commands/diff/diff-all.ts b/projects/optic/src/commands/diff/diff-all.ts index d51bd61d66..3a14cc80e6 100644 --- a/projects/optic/src/commands/diff/diff-all.ts +++ b/projects/optic/src/commands/diff/diff-all.ts @@ -259,11 +259,12 @@ async function computeAll( continue; } - const path = candidate.to ?? candidate.from; + const p = candidate.to ?? candidate.from; // should never happen - if (!path) continue; + if (!p) continue; + const relativePath = path.relative(config.root, path.resolve(p)); - comparisons.set(path, { + comparisons.set(relativePath, { from: candidate.from, to: candidate.to, opticUrl, @@ -277,12 +278,11 @@ async function computeAll( generatedDetails; const pathToUrl: Record = {}; - for (const [path, comparison] of comparisons.entries()) { + for (const [p, comparison] of comparisons.entries()) { if (!comparison.opticUrl) { - pathToUrl[path] = null; + pathToUrl[p] = null; } } - const { apis } = await config.client.getApis( Object.keys(pathToUrl), web_url @@ -302,6 +302,7 @@ async function computeAll( if (!url) { const api = await config.client.createApi(organization_id, { name: path, + path, web_url: web_url, default_branch, default_tag, From 25cf6d53045126cd4f77aee965eb0eca8adc32ad Mon Sep 17 00:00:00 2001 From: Nicholas Lim <18374483+niclim@users.noreply.github.com> Date: Thu, 27 Apr 2023 18:04:45 -0400 Subject: [PATCH 09/11] Fix first generate caase where no spec uploaded --- projects/optic/src/utils/cloud-specs.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/projects/optic/src/utils/cloud-specs.ts b/projects/optic/src/utils/cloud-specs.ts index b61dfcaa7c..1341014c89 100644 --- a/projects/optic/src/utils/cloud-specs.ts +++ b/projects/optic/src/utils/cloud-specs.ts @@ -28,7 +28,14 @@ export async function downloadSpec( id: string; }; }> { - const response = await opts.client.getSpec(spec.apiId, spec.tag); + const response = await opts.client + .getSpec(spec.apiId, spec.tag) + .catch((e) => { + if (e instanceof Error && /spec does not exist/i.test(e.message)) { + return { id: EMPTY_SPEC_ID, specUrl: null, sourcemapUrl: null }; + } + throw e; + }); if (response.id === EMPTY_SPEC_ID) { const spec = createNullSpec(); From 4804610a74f766f0fb9bdfec9588e6f540e87bf6 Mon Sep 17 00:00:00 2001 From: Nicholas Lim <18374483+niclim@users.noreply.github.com> Date: Thu, 27 Apr 2023 18:17:50 -0400 Subject: [PATCH 10/11] fix diff --generated with --base cloud: --- projects/optic/src/commands/diff/diff.ts | 76 ++++++++---------------- projects/optic/src/utils/spec-loaders.ts | 49 +++++++++++---- 2 files changed, 62 insertions(+), 63 deletions(-) diff --git a/projects/optic/src/commands/diff/diff.ts b/projects/optic/src/commands/diff/diff.ts index f0876b124c..7fd4714cdf 100644 --- a/projects/optic/src/commands/diff/diff.ts +++ b/projects/optic/src/commands/diff/diff.ts @@ -29,10 +29,7 @@ import { logger } from '../../logger'; import { errorHandler } from '../../error-handler'; import path from 'path'; import { OPTIC_URL_KEY } from '../../constants'; -import { getApiFromOpticUrl, getApiUrl } from '../../utils/cloud-urls'; -import * as Git from '../../utils/git-utils'; -import { getOrganizationFromToken } from '../../utils/organization'; -import { getDetailsForGeneration } from '../../utils/generated'; +import { getApiFromOpticUrl } from '../../utils/cloud-urls'; const description = `run a diff between two API specs`; @@ -104,14 +101,16 @@ export const registerDiff = (cli: Command, config: OpticCliConfig) => { .action(errorHandler(getDiffAction(config))); }; +type SpecDetails = { apiId: string; orgId: string } | null; + const getBaseAndHeadFromFiles = async ( file1: string, file2: string, config: OpticCliConfig, options: DiffActionOptions -): Promise<[ParseResult, ParseResult]> => { +): Promise<[ParseResult, ParseResult, SpecDetails]> => { try { - return await Promise.all([ + const [baseFile, headFile] = await Promise.all([ loadSpec(file1, config, { strict: options.validation === 'strict', denormalize: true, @@ -123,6 +122,12 @@ const getBaseAndHeadFromFiles = async ( includeUncommittedChanges: options.generated, }), ]); + const opticUrl: string | null = + headFile.jsonLike[OPTIC_URL_KEY] ?? + baseFile.jsonLike[OPTIC_URL_KEY] ?? + null; + const specDetails = opticUrl ? getApiFromOpticUrl(opticUrl) : null; + return [baseFile, headFile, specDetails]; } catch (e) { console.error(e instanceof Error ? e.message : e); throw new UserError(); @@ -135,20 +140,20 @@ const getBaseAndHeadFromFileAndBase = async ( root: string, config: OpticCliConfig, options: DiffActionOptions -): Promise<[ParseResult, ParseResult]> => { +): Promise<[ParseResult, ParseResult, SpecDetails]> => { try { if (/^cloud:/.test(base)) { - const { baseFile, headFile } = await parseFilesFromCloud( + const { baseFile, headFile, specDetails } = await parseFilesFromCloud( file1, base.replace(/^cloud:/, ''), config, { denormalize: true, headStrict: options.validation === 'strict', - includeUncommittedChanges: options.generated, + generated: options.generated, } ); - return [baseFile, headFile]; + return [baseFile, headFile, specDetails]; } else { const { baseFile, headFile } = await parseFilesFromRef( file1, @@ -161,7 +166,12 @@ const getBaseAndHeadFromFileAndBase = async ( includeUncommittedChanges: options.generated, } ); - return [baseFile, headFile]; + const opticUrl: string | null = + headFile.jsonLike[OPTIC_URL_KEY] ?? + baseFile.jsonLike[OPTIC_URL_KEY] ?? + null; + const specDetails = opticUrl ? getApiFromOpticUrl(opticUrl) : null; + return [baseFile, headFile, specDetails]; } } catch (e) { console.error(e instanceof Error ? e.message : e); @@ -170,7 +180,7 @@ const getBaseAndHeadFromFileAndBase = async ( }; const runDiff = async ( - [baseFile, headFile]: [ParseResult, ParseResult], + [baseFile, headFile]: [ParseResult, ParseResult, SpecDetails], config: OpticCliConfig, options: DiffActionOptions, filepath: string @@ -303,7 +313,7 @@ const getDiffAction = if (options.ruleset && !options.standard) { options.standard = options.ruleset; } - let parsedFiles: [ParseResult, ParseResult]; + let parsedFiles: [ParseResult, ParseResult, SpecDetails]; if (file1 && file2) { parsedFiles = await getBaseAndHeadFromFiles( file1, @@ -339,46 +349,8 @@ const getDiffAction = let maybeChangelogUrl: string | null = null; let specUrl: string | null = null; - const [baseParseResult, headParseResult] = parsedFiles; + let [baseParseResult, headParseResult, specDetails] = parsedFiles; if (options.upload) { - const opticUrl: string | null = - headParseResult.jsonLike[OPTIC_URL_KEY] ?? - baseParseResult.jsonLike[OPTIC_URL_KEY] ?? - null; - let specDetails = opticUrl ? getApiFromOpticUrl(opticUrl) : null; - - if (options.generated && !specDetails) { - const path = file1; - const generatedDetails = await getDetailsForGeneration(config); - if (generatedDetails) { - const { web_url, organization_id, default_branch, default_tag } = - generatedDetails; - - const { apis } = await config.client.getApis([path], web_url); - let url: string; - if (!apis[0]) { - const api = await config.client.createApi(organization_id, { - name: path, - web_url: web_url, - default_branch, - default_tag, - }); - url = getApiUrl( - config.client.getWebBase(), - organization_id, - api.id - ); - } else { - url = getApiUrl( - config.client.getWebBase(), - organization_id, - apis[0].api_id - ); - } - specDetails = getApiFromOpticUrl(url); - } - } - const uploadResults = await uploadDiff( { from: baseParseResult, diff --git a/projects/optic/src/utils/spec-loaders.ts b/projects/optic/src/utils/spec-loaders.ts index 516bec23a5..8fc0492edc 100644 --- a/projects/optic/src/utils/spec-loaders.ts +++ b/projects/optic/src/utils/spec-loaders.ts @@ -19,9 +19,10 @@ 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 { getApiFromOpticUrl, getApiUrl } from './cloud-urls'; import { OPTIC_URL_KEY } from '../constants'; import chalk from 'chalk'; +import { getDetailsForGeneration } from './generated'; const exec = promisify(callbackExec); @@ -373,21 +374,47 @@ export const parseFilesFromCloud = async ( options: { denormalize: boolean; headStrict: boolean; - includeUncommittedChanges: boolean; + generated: boolean; } -): Promise<{ - baseFile: ParseResult; - headFile: ParseResult; -}> => { +) => { const headFile = await loadSpec(filePath, config, { denormalize: options.denormalize, strict: options.headStrict, - includeUncommittedChanges: options.includeUncommittedChanges, + includeUncommittedChanges: options.generated, }); - const maybeApi = getApiFromOpticUrl(headFile.jsonLike[OPTIC_URL_KEY]); + let specDetails = getApiFromOpticUrl(headFile.jsonLike[OPTIC_URL_KEY]); + + if (options.generated) { + const relativePath = path.relative(config.root, path.resolve(filePath)); + const generatedDetails = await getDetailsForGeneration(config); + if (generatedDetails) { + const { web_url, organization_id, default_branch, default_tag } = + generatedDetails; - if (!maybeApi) { + const { apis } = await config.client.getApis([relativePath], web_url); + let url: string; + if (!apis[0]) { + const api = await config.client.createApi(organization_id, { + name: relativePath, + path: relativePath, + web_url: web_url, + default_branch, + default_tag, + }); + url = getApiUrl(config.client.getWebBase(), organization_id, api.id); + } else { + url = getApiUrl( + config.client.getWebBase(), + organization_id, + apis[0].api_id + ); + } + specDetails = getApiFromOpticUrl(url); + } + } + + if (!specDetails) { 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." @@ -396,14 +423,14 @@ export const parseFilesFromCloud = async ( ${chalk.gray(`Get started by running 'optic api add ${filePath}'`)}` ); } - const baseFile = await parseSpecAndDereference( - `cloud:${maybeApi.apiId}@${cloudTag}`, + `cloud:${specDetails.apiId}@${cloudTag}`, config ); return { baseFile, headFile, + specDetails, }; }; From 61fe69133a3890d85db89d1a310fc0334ccfc26f Mon Sep 17 00:00:00 2001 From: Nicholas Lim <18374483+niclim@users.noreply.github.com> Date: Fri, 28 Apr 2023 15:07:40 -0400 Subject: [PATCH 11/11] bump patch --- package.json | 2 +- projects/json-pointer-helpers/package.json | 2 +- projects/openapi-io/package.json | 2 +- projects/openapi-utilities/package.json | 2 +- projects/optic-ci/package.json | 2 +- projects/optic/package.json | 2 +- projects/rulesets-base/package.json | 2 +- projects/standard-rulesets/package.json | 2 +- 8 files changed, 8 insertions(+), 8 deletions(-) diff --git a/package.json b/package.json index 88550bca58..0f185cacd6 100644 --- a/package.json +++ b/package.json @@ -2,7 +2,7 @@ "name": "openapi-workspaces", "license": "MIT", "private": true, - "version": "0.42.15", + "version": "0.42.16", "workspaces": [ "projects/json-pointer-helpers", "projects/openapi-io", diff --git a/projects/json-pointer-helpers/package.json b/projects/json-pointer-helpers/package.json index 32edb24556..52db027d23 100644 --- a/projects/json-pointer-helpers/package.json +++ b/projects/json-pointer-helpers/package.json @@ -2,7 +2,7 @@ "name": "@useoptic/json-pointer-helpers", "license": "MIT", "packageManager": "yarn@3.5.0", - "version": "0.42.15", + "version": "0.42.16", "main": "build/index.js", "types": "build/index.d.ts", "files": [ diff --git a/projects/openapi-io/package.json b/projects/openapi-io/package.json index 418e93cd3e..cd96ca251c 100644 --- a/projects/openapi-io/package.json +++ b/projects/openapi-io/package.json @@ -2,7 +2,7 @@ "name": "@useoptic/openapi-io", "license": "MIT", "packageManager": "yarn@3.5.0", - "version": "0.42.15", + "version": "0.42.16", "main": "build/index.js", "types": "build/index.d.ts", "files": [ diff --git a/projects/openapi-utilities/package.json b/projects/openapi-utilities/package.json index c4817f0920..c46e13bea1 100644 --- a/projects/openapi-utilities/package.json +++ b/projects/openapi-utilities/package.json @@ -2,7 +2,7 @@ "name": "@useoptic/openapi-utilities", "license": "MIT", "packageManager": "yarn@3.5.0", - "version": "0.42.15", + "version": "0.42.16", "main": "build/index.js", "types": "build/index.d.ts", "files": [ diff --git a/projects/optic-ci/package.json b/projects/optic-ci/package.json index 054b2dc31c..754c91467d 100644 --- a/projects/optic-ci/package.json +++ b/projects/optic-ci/package.json @@ -2,7 +2,7 @@ "name": "@useoptic/optic-ci", "license": "MIT", "packageManager": "yarn@3.5.0", - "version": "0.42.15", + "version": "0.42.16", "main": "build/index.js", "types": "build/index.d.ts", "files": [ diff --git a/projects/optic/package.json b/projects/optic/package.json index bf88ef7229..bb1c5b96c3 100644 --- a/projects/optic/package.json +++ b/projects/optic/package.json @@ -2,7 +2,7 @@ "name": "@useoptic/optic", "license": "MIT", "packageManager": "yarn@3.5.0", - "version": "0.42.15", + "version": "0.42.16", "main": "build/index.js", "types": "build/index.d.ts", "files": [ diff --git a/projects/rulesets-base/package.json b/projects/rulesets-base/package.json index 0055b5e964..b84b8b4e0d 100644 --- a/projects/rulesets-base/package.json +++ b/projects/rulesets-base/package.json @@ -2,7 +2,7 @@ "name": "@useoptic/rulesets-base", "license": "MIT", "packageManager": "yarn@3.5.0", - "version": "0.42.15", + "version": "0.42.16", "main": "build/index.js", "types": "build/index.d.ts", "files": [ diff --git a/projects/standard-rulesets/package.json b/projects/standard-rulesets/package.json index 6e5669c0bf..3a757d9ab9 100644 --- a/projects/standard-rulesets/package.json +++ b/projects/standard-rulesets/package.json @@ -2,7 +2,7 @@ "name": "@useoptic/standard-rulesets", "license": "MIT", "packageManager": "yarn@3.5.0", - "version": "0.42.15", + "version": "0.42.16", "main": "build/index.js", "types": "build/index.d.ts", "files": [