refactor: cleanup

Author: CountRedClaw

src/apitypes/graphql/graphql.changes.ts

@@ -14,15 +14,8 @@
  * limitations under the License.
  */
 
-import { VersionGraphQLOperation } from './graphql.types'
-import { isEmpty, removeComponents, removeFirstSlash, slugify, takeIf } from '../../utils'
-import {
-  apiDiff,
-  COMPARE_MODE_OPERATION,
-  Diff,
-  DIFF_META_KEY,
-  DIFFS_AGGREGATED_META_KEY,
-} from '@netcracker/qubership-apihub-api-diff'
+import { isEmpty, removeFirstSlash, slugify, takeIf } from '../../utils'
+import { apiDiff, Diff, DIFF_META_KEY, DIFFS_AGGREGATED_META_KEY } from '@netcracker/qubership-apihub-api-diff'
 import { NORMALIZE_OPTIONS, ORIGINS_SYMBOL } from '../../consts'
 import { GraphApiOperation, GraphApiSchema } from '@netcracker/qubership-apihub-graphapi'
 import { buildSchema } from 'graphql/utilities'
@@ -134,33 +127,6 @@ export const compareDocuments = async (apiType: OperationsApiType, operationsMap
   return { operationChanges: changedOperations, tags: [...tags.values()] }
 }
 
-/** @deprecated */
-export const graphqlOperationsCompare = async (current: VersionGraphQLOperation | undefined, previous: VersionGraphQLOperation | undefined): Promise<Diff[]> => {
-  let previousOperation = removeComponents(previous?.data)
-  let currentOperation = removeComponents(current?.data)
-  if (!previousOperation && currentOperation) {
-    previousOperation = getCopyWithEmptyOperations(currentOperation as GraphApiSchema)
-  }
-
-  if (previousOperation && !currentOperation) {
-    currentOperation = getCopyWithEmptyOperations(previousOperation as GraphApiSchema)
-  }
-
-  //todo think about normalize options
-  const { diffs } = apiDiff(
-    previousOperation,
-    currentOperation,
-    {
-      ...NORMALIZE_OPTIONS,
-      mode: COMPARE_MODE_OPERATION,
-      normalizedResult: true,
-      beforeSource: previous?.data,
-      afterSource: current?.data,
-    },
-  )
-  return diffs
-}
-
 function getCopyWithEmptyOperations(template: GraphApiSchema): GraphApiSchema {
   // eslint-disable-next-line @typescript-eslint/no-unused-vars
   const { queries, mutations, subscriptions, ...rest } = template

src/apitypes/graphql/index.ts

@@ -21,7 +21,7 @@ import { buildGraphQLOperations } from './graphql.operations'
 import { GRAPHQL_API_TYPE, GRAPHQL_DOCUMENT_TYPE } from './graphql.consts'
 import { parseGraphQLFile } from './graphql.parser'
 import { ApiBuilder } from '../../types'
-import { compareDocuments, graphqlOperationsCompare } from './graphql.changes'
+import { compareDocuments } from './graphql.changes'
 
 export * from './graphql.consts'
 
@@ -32,6 +32,5 @@ export const graphqlApiBuilder: ApiBuilder<GraphApiSchema> = {
   buildDocument: buildGraphQLDocument,
   buildOperations: buildGraphQLOperations,
   dumpDocument: dumpGraphQLDocument,
-  compareOperationsData: graphqlOperationsCompare,
   compareDocuments: compareDocuments,
 }

src/apitypes/rest/index.ts

@@ -18,7 +18,7 @@ import { OpenAPIV3 } from 'openapi-types'
 
 import { buildRestDocument, createRestExportDocument, dumpRestDocument } from './rest.document'
 import { REST_API_TYPE, REST_DOCUMENT_TYPE } from './rest.consts'
-import { compareDocuments, compareRestOperationsData } from './rest.changes'
+import { compareDocuments } from './rest.changes'
 import { buildRestOperations, createNormalizedOperationId } from './rest.operations'
 import { parseRestFile } from './rest.parser'
 
@@ -33,7 +33,6 @@ export const restApiBuilder: ApiBuilder<OpenAPIV3.Document> = {
   buildDocument: buildRestDocument,
   buildOperations: buildRestOperations,
   dumpDocument: dumpRestDocument,
-  compareOperationsData: compareRestOperationsData,
   compareDocuments: compareDocuments,
   createNormalizedOperationId: createNormalizedOperationId,
   createExportDocument: createRestExportDocument,

src/apitypes/rest/rest.changes.ts

@@ -14,22 +14,20 @@
  * limitations under the License.
  */
 
-import { RestOperationData, VersionRestOperation } from './rest.types'
+import { RestOperationData } from './rest.types'
 import {
   areDeprecatedOriginsNotEmpty,
   IGNORE_PATH_PARAM_UNIFIED_PLACEHOLDER,
   isEmpty,
   isOperationRemove,
   isPathParamRenameDiff,
   normalizePath,
-  removeComponents,
   removeFirstSlash,
   slugify,
 } from '../../utils'
 import {
   apiDiff,
   breaking,
-  COMPARE_MODE_OPERATION,
   Diff,
   DIFF_META_KEY,
   DiffAction,
@@ -79,12 +77,10 @@ export const compareDocuments = async (apiType: OperationsApiType, operationsMap
   const isChangedOperations = prevDoc && currDoc
 
   if (prevDocData && previousGroup) {
-    // todo what's with components? why don't remove?
     prevDocData = createCopyWithCurrentGroupOperationsOnly(prevDocData, previousGroup)
   }
 
   if (currDocData && currentGroup) {
-    // todo what's with components? why don't remove?
     currDocData = createCopyWithCurrentGroupOperationsOnly(currDocData, currentGroup)
   }
 
@@ -158,21 +154,14 @@ export const compareDocuments = async (apiType: OperationsApiType, operationsMap
           // ignore removed and added operations, they'll be handled in a separate docs comparison
           continue
         }
-        // const deprecatedInVersionsCount = previousVersionDeprecations?.operations.find((operation) => operation.operationId === operationId)?.deprecatedInPreviousVersions?.length ?? 0
-        // if (isOperationRemove(operationDiff) && deprecatedInVersionsCount > 1) {
-        //   operationDiff.type = risky
-        // }
         operationDiffs.push(operationDiff)
       }
 
       if (isEmpty(operationDiffs)) {
         continue
       }
 
-      const onlyBreaking = operationDiffs.filter((diff) => diff.type === breaking)
-      if (onlyBreaking.length > 0 && previous?.operationId) {
-        await reclassifyBreakingChanges(previous.operationId, merged, onlyBreaking, ctx)
-      }
+      await reclassifyBreakingChanges(previous?.operationId, merged, operationDiffs, ctx)
 
       changedOperations.push(createOperationChange(apiType, operationDiffs, previous, current, currGroupSlug, prevGroupSlug, currentGroup, previousGroup))
       getOperationTags(current ?? previous).forEach(tag => tags.add(tag))
@@ -182,57 +171,31 @@ export const compareDocuments = async (apiType: OperationsApiType, operationsMap
   return { operationChanges: changedOperations, tags: [...tags.values()] }
 }
 
-/** @deprecated */
-export const compareRestOperationsData = async (current: VersionRestOperation | undefined, previous: VersionRestOperation | undefined, ctx: CompareOperationsPairContext): Promise<Diff[]> => {
-
-  let previousOperation = removeComponents(previous?.data)
-  let currentOperation = removeComponents(current?.data)
-  if (!previousOperation && currentOperation) {
-    previousOperation = createCopyWithEmptyPathItems(currentOperation as RestOperationData)
-  }
-
-  if (previousOperation && !currentOperation) {
-    currentOperation = createCopyWithEmptyPathItems(previousOperation as RestOperationData)
-  }
-
-  const diffResult = apiDiff(
-    previousOperation,
-    currentOperation,
-    {
-      ...NORMALIZE_OPTIONS,
-      originsFlag: ORIGINS_SYMBOL,
-      mode: COMPARE_MODE_OPERATION,
-      normalizedResult: true,
-      beforeSource: previous?.data,
-      afterSource: current?.data,
-    },
-  )
-  const olnyBreaking = diffResult.diffs.filter((diff) => diff.type === breaking)
-  if (olnyBreaking.length > 0 && previous?.operationId) {
-    await reclassifyBreakingChanges(previous.operationId, diffResult.merged, olnyBreaking, ctx)
-  }
-  return diffResult.diffs
-}
-
 async function reclassifyBreakingChanges(
-  operationId: string,
+  previousOperationId: string | undefined,
   mergedJso: unknown,
   diffs: Diff[],
   ctx: CompareOperationsPairContext,
 ): Promise<void> {
-  if (!ctx.previousVersion || !ctx.previousPackageId) {
+  if (!previousOperationId || !ctx.previousVersion || !ctx.previousPackageId) {
+    return
+  }
+
+  const onlyBreaking = diffs.filter((diff) => diff.type === breaking)
+  if (isEmpty(onlyBreaking)) {
     return
   }
-  const previosVersionDeprecations = await ctx.versionDeprecatedResolver(REST_API_TYPE, ctx.previousVersion, ctx.previousPackageId, [operationId])
-  if (!previosVersionDeprecations) {
+
+  const previousVersionDeprecations = await ctx.versionDeprecatedResolver(REST_API_TYPE, ctx.previousVersion, ctx.previousPackageId, [previousOperationId])
+  if (!previousVersionDeprecations) {
     return
   }
 
-  const previousOperation = previosVersionDeprecations.operations[0]
+  const [previousOperation] = previousVersionDeprecations.operations
 
   if (!previousOperation?.deprecatedItems) { return }
 
-  for (const diff of diffs) {
+  for (const diff of onlyBreaking) {
     if (diff.type !== breaking) {
       continue
     }
@@ -286,15 +249,13 @@ async function reclassifyBreakingChanges(
     }
   }
   // mark removed required status of the property as risky
-  if (diffs.length) {
-    const requiredProperties = findRequiredRemovedProperties(mergedJso, diffs)
+  const requiredProperties = findRequiredRemovedProperties(mergedJso, onlyBreaking)
 
-    requiredProperties?.forEach(prop => {
-      if (prop.propDiff.type === RISKY_CHANGE_TYPE && prop.requiredDiff?.type === BREAKING_CHANGE_TYPE) {
-        prop.requiredDiff.type = risky
-      }
-    })
-  }
+  requiredProperties?.forEach(prop => {
+    if (prop.propDiff.type === RISKY_CHANGE_TYPE && prop.requiredDiff?.type === BREAKING_CHANGE_TYPE) {
+      prop.requiredDiff.type = risky
+    }
+  })
 }
 
 export function createCopyWithEmptyPathItems(template: RestOperationData): RestOperationData {

src/builder.ts

@@ -476,7 +476,7 @@ export class PackageVersionBuilder implements IPackageVersionBuilder {
   ): 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
-    // the only way possible is to get the docs from buildResult, but the referenced packages map will be empty
+    // 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)
       const currentApiTypeDocuments = this.documentList.filter(({ type }) => apiBuilder?.types.includes(type))

src/components/compare/bwc.validation.ts

@@ -14,29 +14,22 @@
  * limitations under the License.
  */
 
-import { BREAKING_CHANGE_TYPE, OperationChanges, RISKY_CHANGE_TYPE } from '../../types'
+import { BREAKING_CHANGE_TYPE, ResolvedOperation, RISKY_CHANGE_TYPE } from '../../types'
 import { API_KIND } from '../../consts'
+import { Diff } from '@netcracker/qubership-apihub-api-diff'
 
-export function validateBwcBreakingChanges(
-  operationChanges: OperationChanges,
-): void {
-  if (!operationChanges.changeSummary?.breaking) {
-    return
+export function reclassifyNoBwcBreakingChanges(
+  operationDiffs: Diff[],
+  previous?: ResolvedOperation,
+  current?: ResolvedOperation,
+): Diff[] {
+  if (current?.apiKind === API_KIND.NO_BWC || previous?.apiKind === API_KIND.NO_BWC) {
+    return operationDiffs?.map((diff) => {
+      if (diff.type !== BREAKING_CHANGE_TYPE) {
+        return diff
+      }
+      return { ...diff, type: RISKY_CHANGE_TYPE }
+    })
   }
-  if (operationChanges.apiKind !== API_KIND.NO_BWC && operationChanges.previousApiKind !== API_KIND.NO_BWC) {
-    return
-  }
-
-  operationChanges.changeSummary[RISKY_CHANGE_TYPE] = operationChanges.changeSummary[BREAKING_CHANGE_TYPE]
-  operationChanges.changeSummary[BREAKING_CHANGE_TYPE] = 0
-
-  operationChanges.impactedSummary[BREAKING_CHANGE_TYPE] = false
-  operationChanges.impactedSummary[RISKY_CHANGE_TYPE] = true
-
-  operationChanges.diffs = operationChanges.diffs?.map((diff) => {
-    if (diff.type !== BREAKING_CHANGE_TYPE) {
-      return { ...diff }
-    }
-    return { ...diff, type: RISKY_CHANGE_TYPE }
-  })
+  return operationDiffs
 }

src/components/compare/compare.utils.ts

@@ -17,7 +17,6 @@
 import {
   ApiBuilder,
   ChangeSummary,
-  CompareContext,
   DIFF_TYPES,
   ImpactedOperationSummary,
   NormalizedOperationId,
@@ -27,25 +26,18 @@ import {
   OperationsApiType,
   OperationTypes,
   ResolvedOperation,
-  ResolvedVersionOperationsHashMap,
   VersionCache,
 } from '../../types'
-import { BUILD_TYPE, EMPTY_CHANGE_SUMMARY, MESSAGE_SEVERITY } from '../../consts'
+import { EMPTY_CHANGE_SUMMARY } from '../../consts'
 import {
   calculateChangeSummary,
   calculateImpactedSummary,
   convertToSlug,
   removeFirstSlash,
   takeIfDefined,
 } from '../../utils'
-import { validateBwcBreakingChanges } from './bwc.validation'
 import { Diff } from '@netcracker/qubership-apihub-api-diff'
-
-export const totalChanges = (changeSummary?: ChangeSummary): number => {
-  return changeSummary
-    ? Object.values(changeSummary).reduce((acc, current) => acc + current, 0)
-    : 0
-}
+import { reclassifyNoBwcBreakingChanges } from './bwc.validation'
 
 export function calculateTotalChangeSummary(
   summaries: ChangeSummary[],
@@ -171,6 +163,7 @@ export const createPairOperationsMap = (
   for (const previousOperation of previousOperations) {
     const normalizedOperationId = apiBuilder.createNormalizedOperationId?.(previousOperation) ?? previousOperation.operationId
     const prevOperationId = takeSubstringIf(!!prevGroupSlug, normalizedOperationId, prevGroupSlug.length + '-'.length)
+    // todo fix
     // todo it won't work if groups have different length (add test with /api/v1/ and /api/abc/)
     const operationsMappingElement = operationsMap[prevOperationId]
     operationsMap[prevOperationId] = {
@@ -182,11 +175,22 @@ export const createPairOperationsMap = (
   return operationsMap
 }
 
-export function createOperationChange(apiType: OperationsApiType, operationDiffs: Diff[], previous?: ResolvedOperation, current?: ResolvedOperation, currGroupSlug?: string, prevGroupSlug?: string, currentGroup?: string, previousGroup?: string): OperationChanges {
-  const changeSummary = calculateChangeSummary(operationDiffs)
+export function createOperationChange(
+  apiType: OperationsApiType,
+  operationDiffs: Diff[],
+  previous?: ResolvedOperation,
+  current?: ResolvedOperation,
+  currGroupSlug?: string,
+  prevGroupSlug?: string,
+  currentGroup?: string,
+  previousGroup?: string,
+): OperationChanges {
+  const reclassifiedDiffs = reclassifyNoBwcBreakingChanges(operationDiffs, previous, current)
+  const changeSummary = calculateChangeSummary(reclassifiedDiffs)
   const impactedSummary = calculateImpactedSummary([changeSummary])
 
   const currentOperationFields = current && {
+    // todo remove one that is excessive: slug or group
     operationId: takeSubstringIf(!!currGroupSlug, current.operationId, removeFirstSlash(currentGroup ?? '').length),
     apiKind: current.apiKind,
     metadata: getOperationMetadata(current),
@@ -198,15 +202,12 @@ export function createOperationChange(apiType: OperationsApiType, operationDiffs
     previousMetadata: getOperationMetadata(previous),
   }
 
-  const operationChange = {
+  return {
     apiType,
-    diffs: operationDiffs,
+    diffs: reclassifiedDiffs,
     changeSummary: changeSummary,
     impactedSummary: impactedSummary,
     ...currentOperationFields,
     ...previousOperationFields,
   }
-  validateBwcBreakingChanges(operationChange)
-
-  return operationChange
 }