fix: graphql operations handling

Author: CountRedClaw

src/apitypes/graphql/graphql.changes.ts

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import { isEmpty, removeFirstSlash, slugify, takeIf } from '../../utils'
+import { isEmpty, 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'
@@ -45,6 +45,8 @@ export const compareDocuments = async (
   const prevDocSchema = prevFile && buildSchema(await prevFile.text(), { noLocation: true })
   const currDocSchema = currFile && buildSchema(await currFile.text(), { noLocation: true })
 
+  const collectOnlyChangedOperations = Boolean(prevDoc && currDoc)
+
   let prevDocData = prevDocSchema && (await buildGraphQLDocument({
     ...prevDoc,
     source: prevFile,
@@ -81,8 +83,6 @@ export const compareDocuments = async (
     return { operationChanges: [], tags: new Set() }
   }
 
-  let operationDiffs: Diff[] = []
-
   const { currentGroup, previousGroup } = ctx
 
   const tags = new Set<string>()
@@ -97,17 +97,19 @@ export const compareDocuments = async (
       const methodData = operationsByType[operationKey]
 
       const { current, previous } = operationsMap[operationId] ?? {}
-      if (current && previous) {
+      if (!current && !previous) {
+        throw new Error(`Can't find the ${operationId} operation from documents pair ${prevDoc?.fileId} and ${currDoc?.fileId}`)
+      }
+      const operationChanged = Boolean(current && previous)
+      const operationAddedOrRemoved = !operationChanged
+
+      let operationDiffs: Diff[] = []
+      if (operationChanged && collectOnlyChangedOperations) {
         operationDiffs = [...(methodData as WithAggregatedDiffs<GraphApiOperation>)[DIFFS_AGGREGATED_META_KEY]]
-      } else if (current || previous) {
-        for (const type of GRAPHQL_TYPE_KEYS) {
-          const operationsByType = (merged[type] as WithDiffMetaRecord<Record<string, GraphApiOperation>>)?.[DIFF_META_KEY]
-          if (!operationsByType) { continue }
-          operationDiffs.push(...Object.values(operationsByType))
-        }
-        if (isEmpty(operationDiffs)) {
-          throw new Error('should not happen')
-        }
+      }
+      if (operationAddedOrRemoved && !collectOnlyChangedOperations) {
+        const operationAddedOrRemovedDiff = (merged[type] as WithDiffMetaRecord<Record<string, GraphApiOperation>>)[DIFF_META_KEY]?.[operationKey]
+        operationAddedOrRemovedDiff && operationDiffs.push(operationAddedOrRemovedDiff)
       }
 
       if (isEmpty(operationDiffs)) {

src/apitypes/rest/rest.changes.ts

@@ -143,7 +143,7 @@ export const compareDocuments = async (
       const prevNormalizedOperationId = slugify(`${normalizePath(previousBasePath + path)}-${inferredMethod}`, [], IGNORE_PATH_PARAM_UNIFIED_PLACEHOLDER)
       const currNormalizedOperationId = slugify(`${normalizePath(currentBasePath + path)}-${inferredMethod}`, [], IGNORE_PATH_PARAM_UNIFIED_PLACEHOLDER)
 
-      const { current, previous } = operationsMap[prevNormalizedOperationId] ?? operationsMap[currNormalizedOperationId] /*?? operationsMap[operationId]*/ ?? {}
+      const { current, previous } = operationsMap[prevNormalizedOperationId] ?? operationsMap[currNormalizedOperationId] ?? {}
       if (!current && !previous) {
         const missingOperations = prevNormalizedOperationId === currNormalizedOperationId ? `the ${prevNormalizedOperationId} operation` : `the ${prevNormalizedOperationId} and ${currNormalizedOperationId} operations`
         throw new Error(`Can't find ${missingOperations} from documents pair ${prevDoc?.fileId} and ${currDoc?.fileId}`)

test/changes.test.ts

@@ -73,20 +73,20 @@ describe('Changes test', () => {
   })
 
   describe('Added/removed/changed operations handling', () => {
-    test('Add method', async () => {
-      const result = await buildChangelogPackage('changelog/add-method')
+    test('Add operation', async () => {
+      const result = await buildChangelogPackage('changelog/add-operation')
       expect(result).toEqual(changesSummaryMatcher({ [NON_BREAKING_CHANGE_TYPE]: 1 }))
       expect(result).toEqual(numberOfImpactedOperationsMatcher({ [NON_BREAKING_CHANGE_TYPE]: 1 }))
     })
 
-    test('Remove method', async () => {
-      const result = await buildChangelogPackage('changelog/remove-method')
+    test('Remove operation', async () => {
+      const result = await buildChangelogPackage('changelog/remove-operation')
       expect(result).toEqual(changesSummaryMatcher({ [BREAKING_CHANGE_TYPE]: 1 }))
       expect(result).toEqual(numberOfImpactedOperationsMatcher({ [BREAKING_CHANGE_TYPE]: 1 }))
     })
 
-    test('Change method content', async () => {
-      const result = await buildChangelogPackage('changelog/change-inside-method')
+    test('Change operation content', async () => {
+      const result = await buildChangelogPackage('changelog/change-inside-operation')
 
       expect(result).toEqual(changesSummaryMatcher({
         [BREAKING_CHANGE_TYPE]: 1,
@@ -226,8 +226,8 @@ describe('Changes test', () => {
     })
   })
 
-  test('Operation changes fields are correct (REST)', async () => {
-    const result = await buildChangelogPackage('changelog/operation-changes-fields-rest')
+  test('Operation changes fields are correct', async () => {
+    const result = await buildChangelogPackage('changelog/operation-changes-fields')
 
     expect(result).toEqual(operationChangesMatcher([
       expect.objectContaining({
@@ -250,30 +250,6 @@ describe('Changes test', () => {
     ]))
   })
 
-  test('Operation changes fields are correct (GQL)', async () => {
-    const result = await buildChangelogPackage('changelog/operation-changes-fields-gql', [{ fileId: 'before.graphql' }], [{ fileId: 'after.graphql' }])
-
-    expect(result).toEqual(operationChangesMatcher([
-      expect.objectContaining({
-        previousOperationId: 'query-fruits',
-        operationId: 'query-fruits',
-        previousMetadata: {
-          'title': 'Fruits',
-          'tags': ['queries'],
-          'method': 'fruits',
-          'type': 'query',
-        },
-        metadata: {
-          'title': 'Fruits',
-          'tags': ['queries'],
-          'method': 'fruits',
-          'type': 'query',
-        },
-        // rest of the fields are covered by dedicated tests
-      }),
-    ]))
-  })
-
   test('Tags are not duplicated', async () => {
     const result = await buildChangelogPackage('changelog/tags')
 

test/graphql-changes.test.ts

@@ -14,23 +14,56 @@
  * limitations under the License.
  */
 
-import { buildChangelogPackage, changesSummaryMatcher, numberOfImpactedOperationsMatcher } from './helpers'
+import {
+  buildGqlChangelogPackage,
+  changesSummaryMatcher,
+  numberOfImpactedOperationsMatcher,
+  operationChangesMatcher,
+} from './helpers'
 import { BREAKING_CHANGE_TYPE, GRAPHQL_API_TYPE, NON_BREAKING_CHANGE_TYPE } from '../src'
 
 describe('Graphql changes test', () => {
-  test('add mutation', async () => {
-    const result = await buildChangelogPackage(
-      'graphql-changes-test/add-mutation', [{ fileId: 'before.gql' }], [{ fileId: 'after.gql' }],
-    )
-    expect(result).toEqual(changesSummaryMatcher({ [NON_BREAKING_CHANGE_TYPE]: 1 }, GRAPHQL_API_TYPE))
-    expect(result).toEqual(numberOfImpactedOperationsMatcher({ [NON_BREAKING_CHANGE_TYPE]: 1 }, GRAPHQL_API_TYPE))
+  describe('Added/removed/changed operations handling', () => {
+    test('Add operation', async () => {
+      const result = await buildGqlChangelogPackage('graphql-changes/add-operation')
+      expect(result).toEqual(changesSummaryMatcher({ [NON_BREAKING_CHANGE_TYPE]: 1 }, GRAPHQL_API_TYPE))
+      expect(result).toEqual(numberOfImpactedOperationsMatcher({ [NON_BREAKING_CHANGE_TYPE]: 1 }, GRAPHQL_API_TYPE))
+    })
+
+    test('Remove operation', async () => {
+      const result = await buildGqlChangelogPackage('graphql-changes/remove-operation')
+      expect(result).toEqual(changesSummaryMatcher({ [BREAKING_CHANGE_TYPE]: 1 }, GRAPHQL_API_TYPE))
+      expect(result).toEqual(numberOfImpactedOperationsMatcher({ [BREAKING_CHANGE_TYPE]: 1 }, GRAPHQL_API_TYPE))
+    })
+
+    test('Change operation content', async () => {
+      const result = await buildGqlChangelogPackage('graphql-changes/change-inside-operation')
+      expect(result).toEqual(changesSummaryMatcher({ [BREAKING_CHANGE_TYPE]: 1 }, GRAPHQL_API_TYPE))
+      expect(result).toEqual(numberOfImpactedOperationsMatcher({ [BREAKING_CHANGE_TYPE]: 1 }, GRAPHQL_API_TYPE))
+    })
   })
 
-  test('remove mutation', async () => {
-    const result = await buildChangelogPackage(
-      'graphql-changes-test/remove-mutation', [{ fileId: 'before.gql' }], [{ fileId: 'after.gql' }],
-    )
-    expect(result).toEqual(changesSummaryMatcher({ [BREAKING_CHANGE_TYPE]: 1 }, GRAPHQL_API_TYPE))
-    expect(result).toEqual(numberOfImpactedOperationsMatcher({ [BREAKING_CHANGE_TYPE]: 1 }, GRAPHQL_API_TYPE))
+  test('Operation changes fields are correct', async () => {
+    const result = await buildGqlChangelogPackage('graphql-changes/operation-changes-fields')
+
+    expect(result).toEqual(operationChangesMatcher([
+      expect.objectContaining({
+        previousOperationId: 'query-fruits',
+        operationId: 'query-fruits',
+        previousMetadata: {
+          'title': 'Fruits',
+          'tags': ['queries'],
+          'method': 'fruits',
+          'type': 'query',
+        },
+        metadata: {
+          'title': 'Fruits',
+          'tags': ['queries'],
+          'method': 'fruits',
+          'type': 'query',
+        },
+        // rest of the fields are covered by dedicated tests
+      }),
+    ]))
   })
 })