refactor: simplify comparison logic

Author: CountRedClaw

src/apitypes/graphql/graphql.changes.ts

@@ -23,25 +23,23 @@ import { buildGraphQLDocument } from './graphql.document'
 import {
   CompareOperationsPairContext,
   FILE_KIND,
-  NormalizedOperationId,
   OperationChanges,
-  OperationsApiType,
-  ResolvedOperation,
   ResolvedVersionDocument,
   WithAggregatedDiffs,
   WithDiffMetaRecord,
 } from '../../types'
 import { GRAPHQL_TYPE, GRAPHQL_TYPE_KEYS } from './graphql.consts'
-import { createOperationChange, getOperationTags } from '../../components'
+import { createOperationChange, getOperationTags, OperationsMap } from '../../components'
 
-export const compareDocuments = async (apiType: OperationsApiType, operationsMap: Record<NormalizedOperationId, {
-  previous?: ResolvedOperation
-  current?: ResolvedOperation
-}>, prevDoc: ResolvedVersionDocument | undefined, currDoc: ResolvedVersionDocument | undefined, ctx: CompareOperationsPairContext): Promise<{
+export const compareDocuments = async (
+  operationsMap: OperationsMap,
+  prevDoc: ResolvedVersionDocument | undefined,
+  currDoc: ResolvedVersionDocument | undefined,
+  ctx: CompareOperationsPairContext): Promise<{
   operationChanges: OperationChanges[]
   tags: string[]
 }> => {
-  const { rawDocumentResolver, previousVersion, currentVersion, previousPackageId, currentPackageId } = ctx
+  const { apiType, rawDocumentResolver, previousVersion, currentVersion, previousPackageId, currentPackageId } = ctx
   const prevFile = prevDoc && await rawDocumentResolver(previousVersion, previousPackageId, prevDoc.slug)
   const currFile = currDoc && await rawDocumentResolver(currentVersion, currentPackageId, currDoc.slug)
   const prevDocSchema = prevFile && buildSchema(await prevFile.text(), { noLocation: true })
@@ -78,7 +76,7 @@ export const compareDocuments = async (apiType: OperationsApiType, operationsMap
       // mode: COMPARE_MODE_OPERATION,
       normalizedResult: true,
     },
-  ) as {merged: GraphApiSchema; diffs: Diff[]}
+  ) as { merged: GraphApiSchema; diffs: Diff[] }
 
   if (isEmpty(diffs)) {
     return { operationChanges: [], tags: [] }

src/apitypes/rest/rest.changes.ts

@@ -17,6 +17,7 @@
 import { RestOperationData } from './rest.types'
 import {
   areDeprecatedOriginsNotEmpty,
+  convertToSlug,
   IGNORE_PATH_PARAM_UNIFIED_PLACEHOLDER,
   isEmpty,
   isOperationRemove,
@@ -38,10 +39,7 @@ import { MESSAGE_SEVERITY, NORMALIZE_OPTIONS, ORIGINS_SYMBOL } from '../../const
 import {
   BREAKING_CHANGE_TYPE,
   CompareOperationsPairContext,
-  NormalizedOperationId,
   OperationChanges,
-  OperationsApiType,
-  ResolvedOperation,
   ResolvedVersionDocument,
   RISKY_CHANGE_TYPE,
   WithAggregatedDiffs,
@@ -59,16 +57,27 @@ import { calculateObjectHash } from '../../utils/hashes'
 import { REST_API_TYPE } from './rest.consts'
 import { OpenAPIV3 } from 'openapi-types'
 import { extractServersDiffs, getOperationBasePath } from './rest.utils'
-import { createOperationChange, getOperationTags } from '../../components'
+import { createOperationChange, getOperationTags, OperationsMap } from '../../components'
 
-export const compareDocuments = async (apiType: OperationsApiType, operationsMap: Record<NormalizedOperationId, {
-  previous?: ResolvedOperation
-  current?: ResolvedOperation
-}>, prevDoc: ResolvedVersionDocument | undefined, currDoc: ResolvedVersionDocument | undefined, ctx: CompareOperationsPairContext): Promise<{
+export const compareDocuments = async (
+  operationsMap: OperationsMap,
+  prevDoc: ResolvedVersionDocument | undefined,
+  currDoc: ResolvedVersionDocument | undefined,
+  ctx: CompareOperationsPairContext,
+): Promise<{
   operationChanges: OperationChanges[]
   tags: string[]
 }> => {
-  const { rawDocumentResolver, previousVersion, currentVersion, previousPackageId, currentPackageId, currentGroup, previousGroup } = ctx
+  const {
+    apiType,
+    rawDocumentResolver,
+    previousVersion,
+    currentVersion,
+    previousPackageId,
+    currentPackageId,
+    currentGroup,
+    previousGroup,
+  } = ctx
   const prevFile = prevDoc && await rawDocumentResolver(previousVersion, previousPackageId, prevDoc.slug)
   const currFile = currDoc && await rawDocumentResolver(currentVersion, currentPackageId, currDoc.slug)
   let prevDocData = prevFile && JSON.parse(await prevFile.text())
@@ -108,8 +117,8 @@ export const compareDocuments = async (apiType: OperationsApiType, operationsMap
     return { operationChanges: [], tags: [] }
   }
 
-  const currGroupSlug = slugify(removeFirstSlash(currentGroup || ''))
-  const prevGroupSlug = slugify(removeFirstSlash(previousGroup || ''))
+  const currGroupSlug = convertToSlug(currentGroup || '')
+  const prevGroupSlug = convertToSlug(previousGroup || '')
 
   const tags = new Set<string>()
   const changedOperations: OperationChanges[] = []
@@ -127,6 +136,7 @@ export const compareDocuments = async (apiType: OperationsApiType, operationsMap
       }
 
       const methodData = pathData[inferredMethod]
+      // todo if there were actually servers here, we wouldn't have handle it, add a test
       const basePath = getOperationBasePath(methodData?.servers || pathData?.servers || merged.servers || [])
       const operationPath = basePath + path
       const operationId = slugify(`${operationPath}-${inferredMethod}`)

src/builder.ts

@@ -475,7 +475,7 @@ export class PackageVersionBuilder implements IPackageVersionBuilder {
     apiType?: OperationsApiType,
   ): Promise<ResolvedVersionDocuments | null> {
     packageId = packageId ?? this.config.packageId
-    // this is a case when the version has been built just now, and there's nothing to fetch yet, so
+    // this is the case when a version has been built just now, and there's nothing to fetch yet, so
     // the only way to get the docs is to get them from buildResult, but the referenced packages map will be empty (packages: {})
     if (this.canBeResolvedLocally(version, packageId)) {
       const apiBuilder = this.apiBuilders.find(apiBuilder => apiBuilder.apiType === apiType)

src/components/compare/compare.operations.ts

@@ -18,35 +18,28 @@ import {
   CompareContext,
   CompareOperationsPairContext,
   OperationChanges,
-  OperationId,
   OperationsApiType,
   OperationType,
-  ResolvedVersionDocument,
   VersionCache,
   VersionParams,
   VersionsComparison,
 } from '../../types'
 import {
+  calculatePairedDocs,
   calculateTotalImpactedSummary,
+  comparePairedDocs,
   createPairOperationsMap,
-  dedupePairs,
   getUniqueApiTypesFromVersions,
-  normalizeOperationIds,
 } from './compare.utils'
 import {
   calculateApiAudienceTransitions,
   calculateChangeSummary,
   calculateDiffId,
   convertToSlug,
-  difference,
-  executeInBatches,
   getSplittedVersionKey,
-  intersection,
-  removeFirstSlash,
   removeObjectDuplicates,
-  slugify,
 } from '../../utils'
-import { asyncDebugPerformance, DebugPerformanceContext } from '../../utils/logs'
+import { asyncDebugPerformance, DebugPerformanceContext, syncDebugPerformance } from '../../utils/logs'
 
 export async function compareVersionsOperations(
   prev: VersionParams,
@@ -109,125 +102,59 @@ async function compareCurrentApiType(
   debugCtx?: DebugPerformanceContext,
 ): Promise<[OperationType, OperationChanges[]] | null> {
   const {
-    batchSize,
     versionOperationsResolver,
-    versionDocumentsResolver,
     rawDocumentResolver,
     config: { currentGroup = '', previousGroup = '' },
   } = ctx
   const apiBuilder = ctx.apiBuilders.find((builder) => apiType === builder.apiType)
   if (!apiBuilder) { return null }
 
-  const { operations: prevOperations = [] } = await versionOperationsResolver(apiType, prev?.version ?? '', prev?.packageId ?? '', undefined, false) || {}
-  const { operations: currOperations = [] } = await versionOperationsResolver(apiType, curr?.version ?? '', curr?.packageId ?? '', undefined, false) || {}
-
-  const previousGroupSlug = convertToSlug(previousGroup)
-  const prevOperationsWithPrefix = previousGroupSlug ? prevOperations.filter(operation => operation.operationId.startsWith(previousGroupSlug)) : prevOperations
-
-  const currentGroupSlug = convertToSlug(currentGroup)
-  const currOperationsWithPrefix = currentGroupSlug ? currOperations.filter(operation => operation.operationId.startsWith(currentGroupSlug)) : currOperations
-
-  const [prevNormalizedOperationIds, prevNormalizedOperationIdToOperation] = normalizeOperationIds(prevOperationsWithPrefix, apiBuilder, previousGroup)
-  const [currNormalizedOperationIds, currNormalizedOperationIdToOperation] = normalizeOperationIds(currOperationsWithPrefix, apiBuilder, currentGroup)
-
-  const added = difference(currNormalizedOperationIds, prevNormalizedOperationIds)
-  const removed = difference(prevNormalizedOperationIds, currNormalizedOperationIds)
-  const potentiallyChanged = intersection(prevNormalizedOperationIds, currNormalizedOperationIds)
-
   const { version: prevVersion, packageId: prevPackageId } = prev ?? { version: '', packageId: '' }
   const { version: currVersion, packageId: currPackageId } = curr ?? { version: '', packageId: '' }
 
-  const { documents: prevDocuments } = await versionDocumentsResolver(prev?.version ?? '', prev?.packageId ?? '', apiType) ?? { documents: [] }
-  const { documents: unfilteredCurrDocuments } = await versionDocumentsResolver(curr?.version ?? '', curr?.packageId ?? '', apiType) ?? { documents: [] }
-  // todo this filters out files that are not in build config, but config.files is stored between tests and that breaks the '[Response] Should be non-breaking if response code changed in case' test
-  // const currDocuments = ctx.config.files ? unfilteredCurrDocuments.filter(({fileId}) => ctx.config.files?.find((file) => file.fileId === fileId)) : unfilteredCurrDocuments
-  const currDocuments = unfilteredCurrDocuments
-
-  const pairedRestDocs: [ResolvedVersionDocument, ResolvedVersionDocument][] = []
-  for (const normalizedOperationId of potentiallyChanged) {
-    const prevDocumentId = prevNormalizedOperationIdToOperation[normalizedOperationId]?.documentId
-    const currDocumentId = currNormalizedOperationIdToOperation[normalizedOperationId]?.documentId
-    const prevDoc = prevDocuments.find(document => document.slug === prevDocumentId)
-    const currDoc = currDocuments.find(document => document.slug === currDocumentId)
-
-    if (!prevDoc || !currDoc) {
-      throw new Error('should not happen')
-    }
-
-    pairedRestDocs.push([prevDoc, currDoc])
-  }
-
-  const pairedDocs: [ResolvedVersionDocument | undefined, ResolvedVersionDocument | undefined][] = dedupePairs(pairedRestDocs)
-  const uniqueCurrentDocuments = [...new Set(added.map((normalizedOperationId) => {
-    const currDocumentId = currNormalizedOperationIdToOperation[normalizedOperationId]?.documentId
-    return currDocuments.find(document => document.slug === currDocumentId)
-  }))]
-  for (const currDoc of uniqueCurrentDocuments) {
-    pairedDocs.push([undefined, currDoc])
-  }
-
-  const asd = [...new Set(removed.map((normalizedOperationId) => {
-    const prevDocumentId = prevNormalizedOperationIdToOperation[normalizedOperationId]?.documentId
-    return prevDocuments.find(document => document.slug === prevDocumentId)
-  }))]
-  for (const prevDoc of asd) {
-    pairedDocs.push([prevDoc, undefined])
-  }
-
-  const currGroupSlug = slugify(removeFirstSlash(currentGroup || ''))
-  const prevGroupSlug = slugify(removeFirstSlash(previousGroup || ''))
+  const { operations: prevOperations = [] } = await versionOperationsResolver(apiType, prevVersion, prevPackageId, undefined, false) || {}
+  const { operations: currOperations = [] } = await versionOperationsResolver(apiType, currVersion, currPackageId, undefined, false) || {}
 
-  const operationsMap = createPairOperationsMap(prevGroupSlug, currGroupSlug, prevOperationsWithPrefix, currOperationsWithPrefix, apiBuilder)
+  const previousGroupSlug = convertToSlug(previousGroup)
+  const currentGroupSlug = convertToSlug(currentGroup)
 
-  const operationChanges: OperationChanges[] = []
-  const tags: string[] = []
+  const prevOperationsWithPrefix = previousGroupSlug ? prevOperations.filter(operation => operation.operationId.startsWith(`${previousGroupSlug}-`)) : prevOperations
+  const currOperationsWithPrefix = currentGroupSlug ? currOperations.filter(operation => operation.operationId.startsWith(`${currentGroupSlug}-`)) : currOperations
 
   const pairContext: CompareOperationsPairContext = {
+    apiType: apiType,
     notifications: ctx.notifications,
     rawDocumentResolver,
     versionDeprecatedResolver: ctx.versionDeprecatedResolver,
+    versionDocumentsResolver: ctx.versionDocumentsResolver,
     previousVersion: prevVersion,
     currentVersion: currVersion,
-    previousPackageId: prevPackageId,
     currentPackageId: currPackageId,
-    currentGroup: currentGroup,
+    previousPackageId: prevPackageId,
     previousGroup: previousGroup,
+    currentGroup: currentGroup,
+    previousGroupSlug: previousGroupSlug,
+    currentGroupSlug: currentGroupSlug,
   }
 
-  for (const [prevDoc, currDoc] of pairedDocs) {
-    const {
-      operationChanges: docsPairOperationChanges,
-      tags: docsPairTags,
-    } = await apiBuilder.compareDocuments!(apiType, operationsMap, prevDoc, currDoc, pairContext)
-
-    operationChanges.push(...docsPairOperationChanges)
-    tags.push(...docsPairTags)
-  }
+  const operationsMap = createPairOperationsMap(previousGroupSlug, currentGroupSlug, prevOperationsWithPrefix, currOperationsWithPrefix, apiBuilder)
+  const operationPairs = Object.values(operationsMap)
+  const pairedDocs = await calculatePairedDocs(operationPairs, pairContext)
+  const [operationChanges, tags] = await comparePairedDocs(operationsMap, pairedDocs, apiBuilder, pairContext)
 
   const dedupedChanges = removeObjectDuplicates(operationChanges.flatMap(({ diffs }) => diffs), calculateDiffId)
   const changesSummary = calculateChangeSummary(dedupedChanges)
   const numberOfImpactedOperations = calculateTotalImpactedSummary(
     operationChanges.map(({ impactedSummary }) => impactedSummary),
   )
 
-  const pairedOperationIds =
-    Object.entries(operationsMap)
-      .filter(([, { previous, current }]) => previous && current)
-      .map(([, { previous, current }]) => [previous?.operationId, current?.operationId] as [OperationId, OperationId])
-
   const apiAudienceTransitions: ApiAudienceTransition[] = []
   // todo: convert from objects analysis to apihub-diff result analysis after the "info" section participates in the comparison of operations
-  await asyncDebugPerformance('[ApiAudience]', async () => await executeInBatches(pairedOperationIds, async (operationsBatch) => {
-    const previousBatch = operationsBatch.map(([prevOperationId]) => prevOperationId)
-    const currentBatch = operationsBatch.map(([, currOperationId]) => currOperationId)
-    const { operations: currOperationsWithoutData = [] } = await versionOperationsResolver(apiType, currVersion, currPackageId, previousBatch, false) || {}
-    const { operations: prevOperationsWithoutData = [] } = await versionOperationsResolver(apiType, prevVersion, prevPackageId, currentBatch, false) || {}
-
-    const pairOperationsMap = createPairOperationsMap(prevGroupSlug, currGroupSlug, prevOperationsWithoutData, currOperationsWithoutData, apiBuilder)
-    Object.values(pairOperationsMap).forEach((pair) => {
-      calculateApiAudienceTransitions(pair.current, pair.previous, apiAudienceTransitions)
+  syncDebugPerformance('[ApiAudience]', () => {
+    operationPairs.forEach(({current, previous}) => {
+      calculateApiAudienceTransitions(current, previous, apiAudienceTransitions)
     })
-  }, batchSize), debugCtx)
+  }, debugCtx)
 
   return [
     {