Skip to content

Commit e820f6b

Browse files
committed
feat: support prefix groups comparison
1 parent 6f736a2 commit e820f6b

File tree

5 files changed

+106
-64
lines changed

5 files changed

+106
-64
lines changed

src/apitypes/rest/rest.changes.ts

Lines changed: 58 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {
2020
IGNORE_PATH_PARAM_UNIFIED_PLACEHOLDER,
2121
isEmpty,
2222
isOperationRemove,
23+
isPathParamRenameDiff,
2324
normalizePath,
2425
removeComponents,
2526
removeFirstSlash,
@@ -60,7 +61,7 @@ import { calculateObjectHash } from '../../utils/hashes'
6061
import { REST_API_TYPE } from './rest.consts'
6162
import { OpenAPIV3 } from 'openapi-types'
6263
import { extractServersDiffs, getOperationBasePath } from './rest.utils'
63-
import { createOperationChange, getOperationTags, takeSubstringIf } from '../../components'
64+
import { createOperationChange, getOperationTags } from '../../components'
6465

6566
export const compareDocuments = async (apiType: OperationsApiType, operationsMap: Record<NormalizedOperationId, {
6667
previous?: ResolvedOperation
@@ -69,17 +70,29 @@ export const compareDocuments = async (apiType: OperationsApiType, operationsMap
6970
operationChanges: OperationChanges[]
7071
tags: string[]
7172
}> => {
72-
const { rawDocumentResolver, previousVersion, currentVersion, previousPackageId, currentPackageId } = ctx
73+
const { rawDocumentResolver, previousVersion, currentVersion, previousPackageId, currentPackageId, currentGroup, previousGroup } = ctx
7374
const prevFile = prevDoc && await rawDocumentResolver(previousVersion, previousPackageId, prevDoc.slug)
7475
const currFile = currDoc && await rawDocumentResolver(currentVersion, currentPackageId, currDoc.slug)
7576
let prevDocData = prevFile && JSON.parse(await prevFile.text())
7677
let currDocData = currFile && JSON.parse(await currFile.text())
7778

79+
const isChangedOperations = prevDoc && currDoc
80+
81+
if (prevDocData && previousGroup) {
82+
// todo what's with components? why don't remove?
83+
prevDocData = createCopyWithCurrentGroupOperationsOnly(prevDocData, previousGroup)
84+
}
85+
86+
if (currDocData && currentGroup) {
87+
// todo what's with components? why don't remove?
88+
currDocData = createCopyWithCurrentGroupOperationsOnly(currDocData, currentGroup)
89+
}
90+
7891
if (!prevDocData && currDocData) {
79-
prevDocData = createCopyWithEmptyPath(currDocData)
92+
prevDocData = createCopyWithEmptyPathItems(currDocData)
8093
}
8194
if (prevDocData && !currDocData) {
82-
currDocData = createCopyWithEmptyPath(prevDocData)
95+
currDocData = createCopyWithEmptyPathItems(prevDocData)
8396
}
8497

8598
const { merged, diffs } = apiDiff(
@@ -99,13 +112,6 @@ export const compareDocuments = async (apiType: OperationsApiType, operationsMap
99112
return { operationChanges: [], tags: [] }
100113
}
101114

102-
// todo reclassify
103-
// const olnyBreaking = diffs.filter((diff) => diff.type === breaking)
104-
// if (olnyBreaking.length > 0 && previous?.operationId) {
105-
// await reclassifyBreakingChanges(previous.operationId, diffResult.merged, olnyBreaking, ctx)
106-
// }
107-
108-
const { currentGroup, previousGroup } = ctx.config
109115
const currGroupSlug = slugify(removeFirstSlash(currentGroup || ''))
110116
const prevGroupSlug = slugify(removeFirstSlash(previousGroup || ''))
111117

@@ -127,40 +133,47 @@ export const compareDocuments = async (apiType: OperationsApiType, operationsMap
127133
const methodData = pathData[inferredMethod]
128134
const basePath = getOperationBasePath(methodData?.servers || pathData?.servers || merged.servers || [])
129135
const operationPath = basePath + path
130-
const operationId = slugify(`${removeFirstSlash(operationPath)}-${inferredMethod}`)
136+
const operationId = slugify(`${operationPath}-${inferredMethod}`)
131137
const normalizedOperationId = slugify(`${normalizePath(basePath + path)}-${inferredMethod}`, [], IGNORE_PATH_PARAM_UNIFIED_PLACEHOLDER)
132-
// todo what's with prevslug? which tests affected? which slug to slice prev or curr?
133-
const qwe = takeSubstringIf(!!currGroupSlug, normalizedOperationId, currGroupSlug.length)
134138

135-
const { current, previous } = operationsMap[qwe] ?? operationsMap[operationId] ?? {}
139+
const { current, previous } = operationsMap[normalizedOperationId] ?? operationsMap[operationId] ?? {}
136140

137141
let operationDiffs: Diff[] = []
138-
if (current && previous) {
142+
if (current && previous && isChangedOperations) {
139143
operationDiffs = [
140144
...(methodData as WithAggregatedDiffs<OpenAPIV3.OperationObject>)[DIFFS_AGGREGATED_META_KEY],
141145
// todo what about security? add test
142146
...extractServersDiffs(merged),
143147
]
144148

145-
const pathParamRenameDiff = (merged.paths as WithDiffMetaRecord<OpenAPIV3.PathsObject>)[DIFF_META_KEY]?.[path]
146-
pathParamRenameDiff && operationDiffs.push(pathParamRenameDiff)
147-
} else if (current || previous) {
149+
const diff = (merged.paths as WithDiffMetaRecord<OpenAPIV3.PathsObject>)[DIFF_META_KEY]?.[path]
150+
if (diff && isPathParamRenameDiff(diff)) {
151+
// ignore removed and added operations, they'll be handled in a separate docs comparison???
152+
operationDiffs.push(diff)
153+
}
154+
}
155+
if ((!!current !== !!previous) && !isChangedOperations) {
148156
const operationDiff = (merged.paths[path] as WithDiffMetaRecord<OpenAPIV3.PathsObject>)[DIFF_META_KEY]?.[inferredMethod]
149157
if (!operationDiff) {
150158
// ignore removed and added operations, they'll be handled in a separate docs comparison
151159
continue
152160
}
153-
const deprecatedInVersionsCount = previousVersionDeprecations?.operations.find((operation) => operation.operationId === operationId)?.deprecatedInPreviousVersions?.length ?? 0
154-
if (isOperationRemove(operationDiff) && deprecatedInVersionsCount > 1) {
155-
operationDiff.type = risky
156-
}
161+
// const deprecatedInVersionsCount = previousVersionDeprecations?.operations.find((operation) => operation.operationId === operationId)?.deprecatedInPreviousVersions?.length ?? 0
162+
// if (isOperationRemove(operationDiff) && deprecatedInVersionsCount > 1) {
163+
// operationDiff.type = risky
164+
// }
157165
operationDiffs.push(operationDiff)
158166
}
159167

160168
if (isEmpty(operationDiffs)) {
161169
continue
162170
}
163171

172+
const onlyBreaking = operationDiffs.filter((diff) => diff.type === breaking)
173+
if (onlyBreaking.length > 0 && previous?.operationId) {
174+
await reclassifyBreakingChanges(previous.operationId, merged, onlyBreaking, ctx)
175+
}
176+
164177
// todo operationDiffs can be [undefined] in 'type error must not appear during build'
165178
changedOperations.push(createOperationChange(apiType, operationDiffs, previous, current, currGroupSlug, prevGroupSlug, currentGroup, previousGroup))
166179
getOperationTags(current ?? previous).forEach(tag => tags.add(tag))
@@ -176,11 +189,11 @@ export const compareRestOperationsData = async (current: VersionRestOperation |
176189
let previousOperation = removeComponents(previous?.data)
177190
let currentOperation = removeComponents(current?.data)
178191
if (!previousOperation && currentOperation) {
179-
previousOperation = createCopyWithEmptyPath(currentOperation as RestOperationData)
192+
previousOperation = createCopyWithEmptyPathItems(currentOperation as RestOperationData)
180193
}
181194

182195
if (previousOperation && !currentOperation) {
183-
currentOperation = createCopyWithEmptyPath(previousOperation as RestOperationData)
196+
currentOperation = createCopyWithEmptyPathItems(previousOperation as RestOperationData)
184197
}
185198

186199
const diffResult = apiDiff(
@@ -285,13 +298,30 @@ async function reclassifyBreakingChanges(
285298
}
286299
}
287300

288-
export function createCopyWithEmptyPath(template: RestOperationData): RestOperationData {
289-
// eslint-disable-next-line @typescript-eslint/no-unused-vars
301+
export function createCopyWithEmptyPathItems(template: RestOperationData): RestOperationData {
302+
const { paths, ...rest } = template
303+
304+
return {
305+
paths: {
306+
...Object.fromEntries(Object.keys(paths).map(key => [key, {}])),
307+
},
308+
...rest,
309+
}
310+
}
311+
312+
export function createCopyWithCurrentGroupOperationsOnly(template: RestOperationData, group: string): RestOperationData {
290313
const { paths, ...rest } = template
314+
const groupWithoutLeadingSlash = removeFirstSlash(group)
291315

292316
return {
293317
paths: {
294-
...Object.fromEntries(Object.keys(template.paths).map(key => [key, {}])),
318+
...Object.fromEntries(
319+
Object.entries(paths)
320+
.filter(([key]) => removeFirstSlash(key).startsWith(groupWithoutLeadingSlash))
321+
// remove prefix group for correct path mapping in apiDiff
322+
// todo support the most common case when a group is in servers instead of hardcoded in path, add a test
323+
.map(([key, value]) => [removeFirstSlash(key).substring(groupWithoutLeadingSlash.length), value]),
324+
),
295325
},
296326
...rest,
297327
}

src/components/compare/compare.operations.ts

Lines changed: 35 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,15 @@ import {
2929
import {
3030
calculateTotalImpactedSummary,
3131
createPairOperationsMap,
32+
dedupePairs,
3233
getUniqueApiTypesFromVersions,
3334
normalizeOperationIds,
3435
} from './compare.utils'
3536
import {
3637
calculateApiAudienceTransitions,
3738
calculateChangeSummary,
3839
calculateDiffId,
40+
convertToSlug,
3941
difference,
4042
executeInBatches,
4143
getSplittedVersionKey,
@@ -106,20 +108,31 @@ async function compareCurrentApiType(
106108
ctx: CompareContext,
107109
debugCtx?: DebugPerformanceContext,
108110
): Promise<[OperationType, OperationChanges[]] | null> {
109-
const { batchSize, versionOperationsResolver, versionDocumentsResolver, rawDocumentResolver } = ctx
110-
111+
const {
112+
batchSize,
113+
versionOperationsResolver,
114+
versionDocumentsResolver,
115+
rawDocumentResolver,
116+
config: { currentGroup = '', previousGroup = '' },
117+
} = ctx
111118
const apiBuilder = ctx.apiBuilders.find((builder) => apiType === builder.apiType)
112119
if (!apiBuilder) { return null }
113120

114121
const { operations: prevOperations = [] } = await versionOperationsResolver(apiType, prev?.version ?? '', prev?.packageId ?? '', undefined, false) || {}
115122
const { operations: currOperations = [] } = await versionOperationsResolver(apiType, curr?.version ?? '', curr?.packageId ?? '', undefined, false) || {}
116123

117-
const [prevOperationIds, prevNormalizedOperationIdToOperation] = normalizeOperationIds(prevOperations, apiBuilder)
118-
const [currOperationIds, currNormalizedOperationIdToOperation] = normalizeOperationIds(currOperations, apiBuilder)
124+
const previousGroupSlug = convertToSlug(previousGroup)
125+
const prevOperationsWithPrefix = previousGroupSlug ? prevOperations.filter(operation => operation.operationId.startsWith(previousGroupSlug)) : prevOperations
126+
127+
const currentGroupSlug = convertToSlug(currentGroup)
128+
const currOperationsWithPrefix = currentGroupSlug ? currOperations.filter(operation => operation.operationId.startsWith(currentGroupSlug)) : currOperations
119129

120-
const added = difference(currOperationIds, prevOperationIds)
121-
const removed = difference(prevOperationIds, currOperationIds)
122-
const rest = intersection(prevOperationIds, currOperationIds)
130+
const [prevNormalizedOperationIds, prevNormalizedOperationIdToOperation] = normalizeOperationIds(prevOperationsWithPrefix, apiBuilder, previousGroup)
131+
const [currNormalizedOperationIds, currNormalizedOperationIdToOperation] = normalizeOperationIds(currOperationsWithPrefix, apiBuilder, currentGroup)
132+
133+
const added = difference(currNormalizedOperationIds, prevNormalizedOperationIds)
134+
const removed = difference(prevNormalizedOperationIds, currNormalizedOperationIds)
135+
const potentiallyChanged = intersection(prevNormalizedOperationIds, currNormalizedOperationIds)
123136

124137
const { version: prevVersion, packageId: prevPackageId } = prev ?? { version: '', packageId: '' }
125138
const { version: currVersion, packageId: currPackageId } = curr ?? { version: '', packageId: '' }
@@ -131,7 +144,7 @@ async function compareCurrentApiType(
131144
const currDocuments = unfilteredCurrDocuments
132145

133146
const pairedRestDocs: [ResolvedVersionDocument, ResolvedVersionDocument][] = []
134-
for (const normalizedOperationId of rest) {
147+
for (const normalizedOperationId of potentiallyChanged) {
135148
const prevDocumentId = prevNormalizedOperationIdToOperation[normalizedOperationId]?.documentId
136149
const currDocumentId = currNormalizedOperationIdToOperation[normalizedOperationId]?.documentId
137150
const prevDoc = prevDocuments.find(document => document.slug === prevDocumentId)
@@ -161,31 +174,27 @@ async function compareCurrentApiType(
161174
pairedDocs.push([prevDoc, undefined])
162175
}
163176

164-
// todo > 1 is a hack for before\after files naming, docs matching approach is not yet defined
165-
// const pairedDocs = prevDocuments.map((prevDoc) => [prevDoc, currDocuments.length > 1 ? currDocuments.find(({ fileId }) => fileId === prevDoc.fileId) : currDocuments[0]])
166-
167-
const { currentGroup, previousGroup } = ctx.config
168177
const currGroupSlug = slugify(removeFirstSlash(currentGroup || ''))
169178
const prevGroupSlug = slugify(removeFirstSlash(previousGroup || ''))
170179

171-
const operationsMap = createPairOperationsMap(currGroupSlug, prevGroupSlug, currOperations, prevOperations, apiBuilder)
180+
const operationsMap = createPairOperationsMap(prevGroupSlug, currGroupSlug, prevOperationsWithPrefix, currOperationsWithPrefix, apiBuilder)
172181

173182
const operationChanges: OperationChanges[] = []
174183
const tags: string[] = []
175184

176-
for (const [prevDoc, currDoc] of pairedDocs) {
177-
const pairContext: CompareOperationsPairContext = {
178-
notifications: ctx.notifications,
179-
rawDocumentResolver,
180-
versionDeprecatedResolver: ctx.versionDeprecatedResolver,
181-
previousVersion: prevVersion,
182-
currentVersion: currVersion,
183-
previousPackageId: prevPackageId,
184-
currentPackageId: currPackageId,
185-
currentGroup: currentGroup,
186-
previousGroup: previousGroup,
187-
}
185+
const pairContext: CompareOperationsPairContext = {
186+
notifications: ctx.notifications,
187+
rawDocumentResolver,
188+
versionDeprecatedResolver: ctx.versionDeprecatedResolver,
189+
previousVersion: prevVersion,
190+
currentVersion: currVersion,
191+
previousPackageId: prevPackageId,
192+
currentPackageId: currPackageId,
193+
currentGroup: currentGroup,
194+
previousGroup: previousGroup,
195+
}
188196

197+
for (const [prevDoc, currDoc] of pairedDocs) {
189198
const {
190199
operationChanges: docsPairOperationChanges,
191200
tags: docsPairTags,
@@ -214,7 +223,7 @@ async function compareCurrentApiType(
214223
const { operations: currOperationsWithoutData = [] } = await versionOperationsResolver(apiType, currVersion, currPackageId, previousBatch, false) || {}
215224
const { operations: prevOperationsWithoutData = [] } = await versionOperationsResolver(apiType, prevVersion, prevPackageId, currentBatch, false) || {}
216225

217-
const pairOperationsMap = createPairOperationsMap(currGroupSlug, prevGroupSlug, currOperationsWithoutData, prevOperationsWithoutData, apiBuilder)
226+
const pairOperationsMap = createPairOperationsMap(prevGroupSlug, currGroupSlug, prevOperationsWithoutData, currOperationsWithoutData, apiBuilder)
218227
Object.values(pairOperationsMap).forEach((pair) => {
219228
calculateApiAudienceTransitions(pair.current, pair.previous, apiAudienceTransitions)
220229
})

src/components/compare/compare.utils.ts

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -117,12 +117,13 @@ export function dedupePairs<A extends object, B extends object>(
117117
return out
118118
}
119119

120-
export function normalizeOperationIds(operations: ResolvedOperation[], apiBuilder: ApiBuilder): [OperationId[], Record<NormalizedOperationId | OperationId, ResolvedOperation>] {
120+
export function normalizeOperationIds(operations: ResolvedOperation[], apiBuilder: ApiBuilder, group: string): [OperationId[], Record<NormalizedOperationId | OperationId, ResolvedOperation>] {
121121
const normalizedOperationIdToOperation: Record<NormalizedOperationId | OperationId, ResolvedOperation> = {}
122-
for (const operation of operations) {
122+
const groupSlug = convertToSlug(group)
123+
operations.forEach(operation => {
123124
const normalizedOperationId = apiBuilder.createNormalizedOperationId?.(operation) ?? operation.operationId
124-
normalizedOperationIdToOperation[normalizedOperationId] = operation
125-
}
125+
normalizedOperationIdToOperation[takeSubstringIf(!!groupSlug, normalizedOperationId, groupSlug.length)] = operation
126+
})
126127
return [Object.keys(normalizedOperationIdToOperation), normalizedOperationIdToOperation]
127128
}
128129

@@ -193,10 +194,10 @@ export function takeSubstringIf(condition: boolean, value: string, startIndex: n
193194
}
194195

195196
export const createPairOperationsMap = (
196-
currGroupSlug: string,
197197
prevGroupSlug: string,
198-
currentOperations: ResolvedOperation[],
198+
currGroupSlug: string,
199199
previousOperations: ResolvedOperation[],
200+
currentOperations: ResolvedOperation[],
200201
apiBuilder: ApiBuilder,
201202
): Record<NormalizedOperationId, {
202203
previous?: ResolvedOperation
@@ -210,12 +211,12 @@ export const createPairOperationsMap = (
210211

211212
for (const currentOperation of currentOperations) {
212213
const normalizedOperationId = apiBuilder.createNormalizedOperationId?.(currentOperation) ?? currentOperation.operationId
213-
operationsMap[takeSubstringIf(!!currGroupSlug, normalizedOperationId, currGroupSlug.length)] = { current: currentOperation }
214+
operationsMap[takeSubstringIf(!!currGroupSlug, normalizedOperationId, currGroupSlug.length + '-'.length)] = { current: currentOperation }
214215
}
215216

216217
for (const previousOperation of previousOperations) {
217218
const normalizedOperationId = apiBuilder.createNormalizedOperationId?.(previousOperation) ?? previousOperation.operationId
218-
const prevOperationId = takeSubstringIf(!!prevGroupSlug, normalizedOperationId, prevGroupSlug.length)
219+
const prevOperationId = takeSubstringIf(!!prevGroupSlug, normalizedOperationId, prevGroupSlug.length + '-'.length)
219220
// todo it won't work if groups have different length (add test with /api/v1/ and /api/abc/)
220221
const operationsMappingElement = operationsMap[prevOperationId]
221222
operationsMap[prevOperationId] = {

src/types/external/config.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -156,8 +156,6 @@ export interface PrefixGroupsChangelogBuildConfig extends BuildConfigBase {
156156
packageId: PackageId
157157
version: VersionId
158158
status: VersionStatus
159-
previousVersion?: VersionId
160-
previousVersionPackageId?: PackageId
161159
currentGroup?: string
162160
previousGroup?: string
163161

src/utils/operations.utils.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,3 +82,7 @@ export function isOperationRemove(operationDiff: Diff): boolean {
8282
// length is 2 because json path has view like ['paths', '/test/element']
8383
return operationDiff.action === DiffAction.remove && !!matchPaths(operationDiff.beforeDeclarationPaths, [[OPEN_API_PROPERTY_PATHS, PREDICATE_ANY_VALUE, PREDICATE_ANY_VALUE]])
8484
}
85+
86+
export function isPathParamRenameDiff(diff: Diff): boolean {
87+
return diff.action === DiffAction.rename && !!matchPaths(diff.beforeDeclarationPaths, [[OPEN_API_PROPERTY_PATHS, PREDICATE_ANY_VALUE]])
88+
}

0 commit comments

Comments
 (0)