Skip to content

Commit 01fe7e8

Browse files
b41exb41ex
committed
fix: use existing document pair (if available) to calculate diffs for added/removed operations
results in up to 3 times performance improvements for typical cases
1 parent 093cfa3 commit 01fe7e8

File tree

10 files changed

+195
-17
lines changed

10 files changed

+195
-17
lines changed

‎src/apitypes/graphql/graphql.changes.ts‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,6 @@ export const compareDocuments = async (
4545
const prevDocSchema = prevFile && buildSchema(await prevFile.text(), { noLocation: true })
4646
const currDocSchema = currFile && buildSchema(await currFile.text(), { noLocation: true })
4747

48-
const collectOnlyChangedOperations = Boolean(prevDoc && currDoc)
49-
5048
let prevDocData = prevDocSchema && (await buildGraphQLDocument({
5149
...prevDoc,
5250
source: prevFile,
@@ -104,10 +102,10 @@ export const compareDocuments = async (
104102
const operationAddedOrRemoved = !operationChanged
105103

106104
let operationDiffs: Diff[] = []
107-
if (operationChanged && collectOnlyChangedOperations) {
108-
operationDiffs = [...(methodData as WithAggregatedDiffs<GraphApiOperation>)[DIFFS_AGGREGATED_META_KEY]]
105+
if (operationChanged) {
106+
operationDiffs = [...(methodData as WithAggregatedDiffs<GraphApiOperation>)[DIFFS_AGGREGATED_META_KEY]??[]]
109107
}
110-
if (operationAddedOrRemoved && !collectOnlyChangedOperations) {
108+
if (operationAddedOrRemoved) {
111109
const operationAddedOrRemovedDiff = (merged[type] as WithDiffMetaRecord<Record<string, GraphApiOperation>>)[DIFF_META_KEY]?.[operationKey]
112110
operationAddedOrRemovedDiff && operationDiffs.push(operationAddedOrRemovedDiff)
113111
}

‎src/apitypes/rest/rest.changes.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,6 @@ export const compareDocuments = async (
8888
let prevDocData = prevFile && JSON.parse(await prevFile.text())
8989
let currDocData = currFile && JSON.parse(await currFile.text())
9090

91-
const collectOnlyChangedOperations = Boolean(prevDoc && currDoc)
92-
9391
if (prevDocData && previousGroup) {
9492
prevDocData = createCopyWithCurrentGroupOperationsOnly(prevDocData, previousGroup)
9593
}
@@ -152,17 +150,19 @@ export const compareDocuments = async (
152150
const operationAddedOrRemoved = !operationPotentiallyChanged
153151

154152
let operationDiffs: Diff[] = []
155-
if (operationPotentiallyChanged && collectOnlyChangedOperations) {
153+
if (operationPotentiallyChanged) {
156154
operationDiffs = [
157-
...(methodData as WithAggregatedDiffs<OpenAPIV3.OperationObject>)[DIFFS_AGGREGATED_META_KEY],
155+
...(methodData as WithAggregatedDiffs<OpenAPIV3.OperationObject>)[DIFFS_AGGREGATED_META_KEY]??[],
158156
...extractRootServersDiffs(merged),
159157
...extractRootSecurityDiffs(merged),
160158
...extractPathParamRenameDiff(merged, path),
161159
// parameters, servers, summary, description and extensionKeys are moved from path to method in pathItemsUnification during normalization in apiDiff, so no need to aggregate them here
162160
]
163161
}
164-
if (operationAddedOrRemoved && !collectOnlyChangedOperations) {
165-
const operationAddedOrRemovedDiff = (merged.paths[path] as WithDiffMetaRecord<OpenAPIV3.PathsObject>)[DIFF_META_KEY]?.[inferredMethod]
162+
if (operationAddedOrRemoved) {
163+
const operationAddedOrRemovedDiffFromSpecificPath = (merged.paths[path] as WithDiffMetaRecord<OpenAPIV3.PathsObject>)[DIFF_META_KEY]?.[inferredMethod]
164+
const operationAddedOrRemovedDiffFromPaths = (merged.paths as WithDiffMetaRecord<OpenAPIV3.PathsObject>)[DIFF_META_KEY]?.[path]
165+
const operationAddedOrRemovedDiff = operationAddedOrRemovedDiffFromSpecificPath ?? operationAddedOrRemovedDiffFromPaths
166166
operationDiffs = operationAddedOrRemovedDiff ? [operationAddedOrRemovedDiff] : []
167167
}
168168

‎src/components/compare/compare.operations.ts‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -141,11 +141,16 @@ async function compareCurrentApiType(
141141
const operationPairs = Object.values(operationsMap)
142142
const pairedDocs = await calculatePairedDocs(operationPairs, pairContext)
143143
const [operationChanges, tags] = await comparePairedDocs(operationsMap, pairedDocs, apiBuilder, pairContext)
144+
// Duplicates could happen in rare case when document for added/deleted operation was mapped to several documents in other version
145+
const uniqueOperationChanges = removeObjectDuplicates(
146+
operationChanges,
147+
(item) => `${item.apiType}:${item.operationId ?? ''}:${item.previousOperationId ?? ''}`,
148+
)
144149

145-
const dedupedChanges = removeObjectDuplicates(operationChanges.flatMap(({ diffs }) => diffs), calculateDiffId)
146-
const changesSummary = calculateChangeSummary(dedupedChanges)
150+
const uniqueDiffs = removeObjectDuplicates(uniqueOperationChanges.flatMap(({ diffs }) => diffs), calculateDiffId)
151+
const changesSummary = calculateChangeSummary(uniqueDiffs)
147152
const numberOfImpactedOperations = calculateTotalImpactedSummary(
148-
operationChanges.map(({ impactedSummary }) => impactedSummary),
153+
uniqueOperationChanges.map(({ impactedSummary }) => impactedSummary),
149154
)
150155

151156
const apiAudienceTransitions: ApiAudienceTransition[] = []
@@ -163,6 +168,6 @@ async function compareCurrentApiType(
163168
tags,
164169
apiAudienceTransitions,
165170
},
166-
operationChanges,
171+
uniqueOperationChanges,
167172
]
168173
}

‎src/components/compare/compare.utils.ts‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,32 @@ function dedupeTuples<T extends [object | undefined, object | undefined]>(
115115
return result
116116
}
117117

118+
function removeRedundantPartialPairs<T extends [object | undefined, object | undefined]>(
119+
tuples: T[],
120+
): T[] {
121+
const completeAtPosition = [new Set<object>(), new Set<object>()] as const
122+
123+
// First pass: identify all values that appear in complete pairs by position
124+
for (const [a, b] of tuples) {
125+
if (a !== undefined && b !== undefined) {
126+
completeAtPosition[0].add(a)
127+
completeAtPosition[1].add(b)
128+
}
129+
}
130+
131+
// Second pass: filter out partial pairs that are consumed by a complete pair
132+
return tuples.filter(([a, b]) => {
133+
const isPartial = a === undefined || b === undefined
134+
if (!isPartial) return true // Keep all complete pairs
135+
136+
// For partial pairs, only keep if there's no corresponding complete pair
137+
// with the defined value at corresponding position
138+
return a === undefined
139+
? !completeAtPosition[1].has(b!)
140+
: !completeAtPosition[0].has(a)
141+
})
142+
}
143+
118144
export function normalizeOperationIds(operations: ResolvedOperation[], apiBuilder: ApiBuilder, groupSlug: string): [(NormalizedOperationId | OperationId)[], Record<NormalizedOperationId | OperationId, ResolvedOperation>] {
119145
const normalizedOperationIdToOperation: Record<NormalizedOperationId | OperationId, ResolvedOperation> = {}
120146
operations.forEach(operation => {
@@ -200,7 +226,7 @@ export const calculatePairedDocs = async (
200226
const currDoc = current && currDocuments.find(document => document.slug === current.documentId)
201227
pairedDocs.push([prevDoc, currDoc])
202228
}
203-
return dedupeTuples(pairedDocs)
229+
return removeRedundantPartialPairs(dedupeTuples(pairedDocs))
204230
}
205231

206232
export const comparePairedDocs = async (

‎test/declarative-changes-in-rest-package-version.test.ts‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,4 +63,30 @@ describe('Number of declarative changes in rest package version test', () => {
6363
[RISKY_CHANGE_TYPE]: 1,
6464
}))
6565
})
66-
})
66+
67+
test('Uses synthetic document when there is no existing appropriate document pair', async () => {
68+
const result = await buildChangelogPackage(
69+
'declarative-changes-in-rest-package-version/whole-documents-added-removed',
70+
[{ fileId: 'before/spec1.yaml' }],
71+
[{ fileId: 'after/spec2.yaml' }],
72+
)
73+
expect(result).toEqual(changesSummaryMatcher({
74+
[BREAKING_CHANGE_TYPE]: 1,
75+
[NON_BREAKING_CHANGE_TYPE]: 1,
76+
}))
77+
expect(result).toEqual(numberOfImpactedOperationsMatcher({
78+
[BREAKING_CHANGE_TYPE]: 1,
79+
[NON_BREAKING_CHANGE_TYPE]: 1,
80+
}))
81+
})
82+
83+
test('Changes are not duplicated when deleted operation could be mapped to several documents', async () => {
84+
const result = await buildChangelogPackage(
85+
'declarative-changes-in-rest-package-version/deleted-operation-mapped-to-several-documents',
86+
[{ fileId: 'before/spec1.yaml' }],
87+
[{ fileId: 'after/spec2.yaml' }, { fileId: 'after/spec3.yaml' }],
88+
)
89+
expect(result).toEqual(changesSummaryMatcher({ [BREAKING_CHANGE_TYPE]: 1 }))
90+
expect(result).toEqual(numberOfImpactedOperationsMatcher({ [BREAKING_CHANGE_TYPE]: 1 }))
91+
})
92+
})

‎test/projects/declarative-changes-in-rest-package-version/deleted-operation-mapped-to-several-documents/after/spec2.yaml‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
openapi: 3.0.0
2+
info:
3+
title: Sample API
4+
description: API specification example
5+
version: 1.0.0
6+
paths:
7+
/packages:
8+
get:
9+
operationId: getPackage
10+
summary: get package
11+
responses:
12+
'200':
13+
description: OK
14+
content:
15+
application/json:
16+
schema: {}

‎test/projects/declarative-changes-in-rest-package-version/deleted-operation-mapped-to-several-documents/after/spec3.yaml‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
openapi: 3.0.0
2+
info:
3+
title: Sample API
4+
description: API specification example
5+
version: 1.0.0
6+
paths:
7+
/packages:
8+
post:
9+
summary: create package
10+
responses:
11+
'201':
12+
description: OK
13+
content:
14+
application/json:
15+
schema: {}

‎test/projects/declarative-changes-in-rest-package-version/deleted-operation-mapped-to-several-documents/before/spec1.yaml‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
openapi: 3.0.0
2+
info:
3+
title: Sample API
4+
description: API specification example
5+
version: 1.0.0
6+
paths:
7+
/packages:
8+
get:
9+
operationId: getPackage
10+
summary: get package
11+
responses:
12+
'200':
13+
description: OK
14+
content:
15+
application/json:
16+
schema: {}
17+
post:
18+
summary: create package
19+
responses:
20+
'201':
21+
description: OK
22+
content:
23+
application/json:
24+
schema: {}
25+
put:
26+
summary: update package
27+
responses:
28+
'200':
29+
description: OK
30+
content:
31+
application/json:
32+
schema: {}

‎test/projects/declarative-changes-in-rest-package-version/whole-documents-added-removed/after/spec2.yaml‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
openapi: 3.0.0
2+
info:
3+
title: Sample API
4+
description: API specification example
5+
version: 1.0.0
6+
paths:
7+
/packages:
8+
post:
9+
summary: create package
10+
requestBody:
11+
content:
12+
application/json:
13+
schema:
14+
type: object
15+
properties:
16+
name:
17+
type: string
18+
responses:
19+
'201':
20+
description: OK
21+
content:
22+
application/json:
23+
schema:
24+
type: object
25+
properties:
26+
packageId:
27+
$ref: '#/components/schemas/packageId'
28+
name:
29+
type: string
30+
parentPackageId:
31+
$ref: '#/components/schemas/packageId'
32+
components:
33+
schemas:
34+
packageId:
35+
type: string
36+
description: APIHUB package id

‎test/projects/declarative-changes-in-rest-package-version/whole-documents-added-removed/before/spec1.yaml‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
openapi: 3.0.0
2+
info:
3+
title: Sample API
4+
description: API specification example
5+
version: 1.0.0
6+
paths:
7+
/packages:
8+
get:
9+
operationId: getPackage
10+
summary: get package
11+
responses:
12+
'200':
13+
description: OK
14+
content:
15+
application/json:
16+
schema:
17+
$ref: '#/components/schemas/packageId'
18+
components:
19+
schemas:
20+
packageId:
21+
type: string
22+
description: CCI package id
23+
minLength: 1
24+
maxLength: 20

0 commit comments

Comments
 (0)