Skip to content

Commit cf4fcce

Browse files
committed
feat: use mask (*) instead of parameter names while matching rest operations during comparison
1 parent 4e42de4 commit cf4fcce

File tree

13 files changed

+159
-91
lines changed

13 files changed

+159
-91
lines changed

src/apitypes/rest/rest.types.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,14 @@ import type { OpenAPIV3 } from 'openapi-types'
1818

1919
import type { ApiOperation, NotificationMessage, VersionDocument } from '../../types'
2020
import { REST_DOCUMENT_TYPE, REST_KIND_KEY, REST_SCOPES } from './rest.consts'
21+
import { NormalizedPath } from '../../utils'
2122

2223
export type RestScopeType = keyof typeof REST_SCOPES
2324
export type RestDocumentType = (typeof REST_DOCUMENT_TYPE)[keyof typeof REST_DOCUMENT_TYPE]
2425
export type CustomTags = Record<string, unknown>
2526

2627
export interface RestOperationMeta {
27-
path: string // `/packages/*/version/*`
28+
path: NormalizedPath // `/packages/*/version/*`
2829
originalPath: string // `/packages/{packageId}/version/{version}`
2930
method: OpenAPIV3.HttpMethods // `get` | `post` | ...
3031
tags?: string[] // operations tags

src/builder.ts

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,6 @@ import {
2727
ResolvedDeprecatedOperations,
2828
ResolvedDocuments,
2929
ResolvedOperations,
30-
ResolvedVersionOperationsHashMap,
31-
VALIDATION_RULES_SEVERITY_LEVEL_WARNING,
3230
VersionId,
3331
} from './types'
3432
import {
@@ -492,10 +490,9 @@ export class PackageVersionBuilder implements IPackageVersionBuilder {
492490
const operationsTypes: OperationTypes[] = []
493491

494492
for (const apiType of this.existingOperationsApiTypes) {
495-
const operationsHashMap = this.operationsHashMapByApiType(apiType)
496493
operationsTypes.push({
497494
apiType: apiType,
498-
operations: operationsHashMap,
495+
operationsCount: this.operations.size,
499496
})
500497
}
501498

@@ -508,18 +505,6 @@ export class PackageVersionBuilder implements IPackageVersionBuilder {
508505
return new Set(apiTypes)
509506
}
510507

511-
private operationsHashMapByApiType(operationsApiType: OperationsApiType): ResolvedVersionOperationsHashMap {
512-
const hashMap: ResolvedVersionOperationsHashMap = {}
513-
514-
for (const { apiType, operationId, dataHash } of this.operations.values()) {
515-
if (apiType === operationsApiType) {
516-
hashMap[operationId] = dataHash
517-
}
518-
}
519-
520-
return hashMap
521-
}
522-
523508
async parseFile(fileId: string, source: Blob): Promise<File | null> {
524509
if (this.parsedFiles.has(fileId)) {
525510
return this.parsedFiles.get(fileId) ?? null

src/components/compare/compare.operations.ts

Lines changed: 55 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ import {
3535
getOperationTags,
3636
getOperationTypesFromTwoVersions,
3737
getUniqueApiTypesFromVersions,
38-
OperationIdentityMap,
3938
takeSubstringIf,
4039
totalChanges,
4140
} from './compare.utils'
@@ -50,9 +49,9 @@ import {
5049
removeObjectDuplicates,
5150
slugify,
5251
} from '../../utils'
53-
import { asyncFunction } from '../../utils/async'
54-
import { asyncDebugPerformance, DebugPerformanceContext, logLongBuild, syncDebugPerformance } from '../../utils/logs'
52+
import { asyncDebugPerformance, DebugPerformanceContext } from '../../utils/logs'
5553
import { validateBwcBreakingChanges } from './bwc.validation'
54+
import { REST_API_TYPE } from '../../apitypes'
5655

5756
export async function compareVersionsOperations(
5857
prev: VersionParams,
@@ -107,6 +106,18 @@ export async function compareVersionsOperations(
107106
}
108107
}
109108

109+
const HANDLE_TYPE_FULLY_ADDED = 'added'
110+
const HANDLE_TYPE_FULLY_REMOVED = 'removed'
111+
const HANDLE_TYPE_FULLY_CHANGED = 'changed'
112+
113+
type HandleType = typeof HANDLE_TYPE_FULLY_ADDED | typeof HANDLE_TYPE_FULLY_REMOVED | typeof HANDLE_TYPE_FULLY_CHANGED
114+
115+
export interface MappingResult<T extends PropertyKey> {
116+
[HANDLE_TYPE_FULLY_ADDED]: T[]
117+
[HANDLE_TYPE_FULLY_REMOVED]: T[]
118+
[HANDLE_TYPE_FULLY_CHANGED]: Record<T, T>
119+
}
120+
110121
async function compareCurrentApiType(
111122
apiType: OperationsApiType,
112123
prev: VersionCache | null,
@@ -124,30 +135,37 @@ async function compareCurrentApiType(
124135

125136
const [prevOperationTypesData, currOperationTypesData] = getOperationTypesFromTwoVersions(prev, curr)
126137

127-
const changedIdToOriginal: OperationIdentityMap = {}
128-
const prevOperationHashMap = getOperationsHashMapByApiType(apiType, prevOperationTypesData, changedIdToOriginal, ctx)
129-
const currOperationHashMap = getOperationsHashMapByApiType(apiType, currOperationTypesData, changedIdToOriginal, ctx, true)
138+
const prevOperationsCount = prevOperationTypesData.find(data => apiType === data.apiType)?.operationsCount
139+
const currOperationsCount = currOperationTypesData.find(data => apiType === data.apiType)?.operationsCount
130140

131-
const operationIds = new Set([...Object.keys(prevOperationHashMap), ...Object.keys(currOperationHashMap)])
132-
const operationsMapping: Record<string, Array<string>> = { added: [], removed: [], changed: [] }
133-
const apiAudienceTransitions: ApiAudienceTransition[] = []
134-
const pairedOperationIds: Array<string> = []
141+
const { operations: prevOperations = [] } = await versionOperationsResolver(apiType, prev?.version ?? '', prev?.packageId ?? '', undefined, false, prevOperationsCount) || {}
142+
const { operations: currOperations = [] } = await versionOperationsResolver(apiType, curr?.version ?? '', curr?.packageId ?? '', undefined, false, currOperationsCount) || {}
135143

136-
for (const operationId of operationIds) {
137-
const v1OperationHash = prevOperationHashMap[operationId]
138-
const v2OperationHash = currOperationHashMap[operationId]
139-
if (v1OperationHash && v2OperationHash) {
140-
pairedOperationIds.push(changedIdToOriginal[operationId] || operationId)
144+
const [prevReducedOperationIdToHashMap, prevReducedOperationIdToOriginal] = getOperationsHashMapByApiType(apiType, prevOperations, ctx)
145+
const [currReducedOperationIdToHashMap, currReducedOperationIdToOriginal] = getOperationsHashMapByApiType(apiType, currOperations, ctx, true)
146+
147+
const reducedOperationIds = new Set([...Object.keys(prevReducedOperationIdToHashMap), ...Object.keys(currReducedOperationIdToHashMap)])
148+
const operationsMapping: MappingResult<OperationId> = { [HANDLE_TYPE_FULLY_ADDED]: [], [HANDLE_TYPE_FULLY_REMOVED]: [], [HANDLE_TYPE_FULLY_CHANGED]: {} }
149+
const apiAudienceTransitions: ApiAudienceTransition[] = []
150+
const pairedOperationIds: Record<OperationId, OperationId> = {}
151+
152+
for (const reducedOperationId of reducedOperationIds) {
153+
const prevOperationHash = prevReducedOperationIdToHashMap[reducedOperationId]
154+
const currOperationHash = currReducedOperationIdToHashMap[reducedOperationId]
155+
const prevOperationId = prevReducedOperationIdToOriginal[reducedOperationId]
156+
const currOperationId = currReducedOperationIdToOriginal[reducedOperationId]
157+
if (prevOperationHash && currOperationHash) {
158+
pairedOperationIds[prevOperationId] = currOperationId
141159
// operation not changed
142-
if (v1OperationHash === v2OperationHash) { continue }
160+
if (prevOperationHash === currOperationHash) { continue }
143161
// operation changed
144-
operationsMapping.changed.push(changedIdToOriginal[operationId] || operationId)
145-
} else if (v1OperationHash) {
162+
operationsMapping[HANDLE_TYPE_FULLY_CHANGED][prevOperationId] = currOperationId
163+
} else if (prevOperationHash) {
146164
// operation removed
147-
operationsMapping.removed.push(changedIdToOriginal[operationId] || operationId)
148-
} else if (v2OperationHash) {
165+
operationsMapping[HANDLE_TYPE_FULLY_REMOVED].push(prevOperationId)
166+
} else if (currOperationHash) {
149167
// operation added
150-
operationsMapping.added.push(changedIdToOriginal[operationId] || operationId)
168+
operationsMapping[HANDLE_TYPE_FULLY_ADDED].push(currOperationId)
151169
}
152170
}
153171

@@ -229,9 +247,11 @@ async function compareCurrentApiType(
229247
}
230248

231249
// todo: convert from objects analysis to apihub-diff result analysis after the "info" section participates in the comparison of operations
232-
await asyncDebugPerformance('[ApiAudience]', async () => await executeInBatches(pairedOperationIds, async (operationsBatch) => {
233-
const { operations: prevOperationsWithoutData = [] } = await versionOperationsResolver(apiType, prevVersion, prevPackageId, operationsBatch, false) || {}
234-
const { operations: currOperationsWithoutData = [] } = await versionOperationsResolver(apiType, currVersion, currPackageId, operationsBatch, false) || {}
250+
await asyncDebugPerformance('[ApiAudience]', async () => await executeInBatches(Object.entries(pairedOperationIds), async (operationsBatch) => {
251+
const previousBatch = operationsBatch.map(([prevOperationId]) => prevOperationId)
252+
const currentBatch = operationsBatch.map(([, currOperationId]) => currOperationId)
253+
const { operations: currOperationsWithoutData = [] } = await versionOperationsResolver(apiType, currVersion, currPackageId, previousBatch, false) || {}
254+
const { operations: prevOperationsWithoutData = [] } = await versionOperationsResolver(apiType, prevVersion, prevPackageId, currentBatch, false) || {}
235255

236256
const pairOperationsMap = createPairOperationsMap(currGroupSlug, prevGroupSlug, currOperationsWithoutData, prevOperationsWithoutData)
237257
Object.values(pairOperationsMap).forEach((pair) => {
@@ -262,12 +282,11 @@ async function compareCurrentApiType(
262282
if (!apiBuilder.compareOperationsData) { return null }
263283

264284
await asyncDebugPerformance('[Changed]', async (innerDebugCtx) =>
265-
await executeInBatches(operationsMapping.changed, async (operationsBatch) => {
266-
const currentBatch = currentGroup ? operationsBatch.map(operationId => currGroupSlug + operationId.substring(currGroupSlug.length)) : operationsBatch
267-
const previousBatch = previousGroup ? operationsBatch.map(operationId => prevGroupSlug + operationId.substring(prevGroupSlug.length)) : operationsBatch
268-
269-
const { operations: prevOperationsWithData = [] } = await versionOperationsResolver(apiType, prevVersion!, prevPackageId!, previousBatch) || {}
270-
const { operations: currOperationsWithData = [] } = await versionOperationsResolver(apiType, currVersion!, currPackageId!, currentBatch) || {}
285+
await executeInBatches(Object.entries(operationsMapping.changed), async (operationsBatch) => {
286+
const previousBatch = operationsBatch.map(([prevOperationId]) => prevOperationId)
287+
const currentBatch = operationsBatch.map(([, currOperationId]) => currOperationId)
288+
const { operations: prevOperationsWithData = [] } = await versionOperationsResolver(apiType, prevVersion, prevPackageId, previousBatch) || {}
289+
const { operations: currOperationsWithData = [] } = await versionOperationsResolver(apiType, currVersion, currPackageId, currentBatch) || {}
271290

272291
const operationsMap = createPairOperationsMap(currGroupSlug, prevGroupSlug, currOperationsWithData, prevOperationsWithData)
273292

@@ -343,21 +362,20 @@ async function compareCurrentApiType(
343362
]
344363
}
345364

346-
const HANDLE_TYPE_FULLY_ADDED = 'added'
347-
const HANDLE_TYPE_FULLY_REMOVED = 'removed'
348-
349-
type HandleType = typeof HANDLE_TYPE_FULLY_ADDED | typeof HANDLE_TYPE_FULLY_REMOVED
350-
351365
const createPairOperationsMap = (currGroupSlug: string, prevGroupSlug: string, currentOperations: ResolvedOperation[], previousOperations: ResolvedOperation[]): Record<string, { previous?: ResolvedOperation; current: ResolvedOperation }> => {
352366

353367
const operationsMap: Record<string, { previous?: ResolvedOperation; current: ResolvedOperation }> = {}
354368

355369
for (const currentOperation of currentOperations) {
356-
operationsMap[takeSubstringIf(!!currGroupSlug, currentOperation.operationId, currGroupSlug.length)] = { current: currentOperation }
370+
// todo
371+
const normalizedOperationId = currentOperation.apiType === REST_API_TYPE ? slugify(`${removeFirstSlash(`${currentOperation.metadata.path}`)}-${currentOperation.metadata.method}`) : currentOperation.operationId
372+
operationsMap[takeSubstringIf(!!currGroupSlug, normalizedOperationId, currGroupSlug.length)] = { current: currentOperation }
357373
}
358374

359375
for (const previousOperation of previousOperations) {
360-
const prevOperationId = takeSubstringIf(!!prevGroupSlug, previousOperation.operationId, prevGroupSlug.length)
376+
// todo
377+
const normalizedOperationId = previousOperation.apiType === REST_API_TYPE ? slugify(`${removeFirstSlash(`${previousOperation.metadata.path}`)}-${previousOperation.metadata.method}`) : previousOperation.operationId
378+
const prevOperationId = takeSubstringIf(!!prevGroupSlug, normalizedOperationId, prevGroupSlug.length)
361379
const operationsMappingElement = operationsMap[prevOperationId]
362380
if (operationsMappingElement) {
363381
operationsMap[prevOperationId] = {

src/components/compare/compare.utils.ts

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,10 @@ import {
2828
VersionCache,
2929
} from '../../types'
3030
import { BUILD_TYPE, EMPTY_CHANGE_SUMMARY, MESSAGE_SEVERITY } from '../../consts'
31-
import { convertToSlug, takeIfDefined } from '../../utils'
31+
import { convertToSlug, NormalizedPath, PATH_PARAM_UNIFIED_PLACEHOLDER, slugify, takeIfDefined } from '../../utils'
32+
import { REST_API_TYPE } from '../../apitypes'
33+
import { OpenAPIV3 } from 'openapi-types'
34+
import { CharMap } from 'slug'
3235

3336
export const totalChanges = (changeSummary?: ChangeSummary): number => {
3437
return changeSummary
@@ -86,28 +89,42 @@ export function getOperationTypesFromTwoVersions(
8689
type OperationIdWithoutGroupPrefix = string
8790
export type OperationIdentityMap = Record<OperationIdWithoutGroupPrefix, OperationId>
8891

92+
type NormalizedOperationId = `${NormalizedPath}-${OpenAPIV3.HttpMethods}`
93+
94+
const IGNORE_PATH_PARAM_UNIFIED_PLACEHOLDER: CharMap = { [PATH_PARAM_UNIFIED_PLACEHOLDER]: PATH_PARAM_UNIFIED_PLACEHOLDER }
95+
8996
export function getOperationsHashMapByApiType(
9097
currentApiType: OperationsApiType,
91-
operationTypes: OperationTypes[],
92-
operationIdentityMap: OperationIdentityMap,
98+
operations: ResolvedOperation[],
9399
ctx: CompareContext,
94100
areOperationsFromCurrentVersion: boolean = false,
95-
): ResolvedVersionOperationsHashMap {
96-
const resolvedHashMap = { ...operationTypes.find(({ apiType: type }) => type === currentApiType)?.operations || {} }
101+
): [ResolvedVersionOperationsHashMap, OperationIdentityMap] {
97102
const { buildType, currentGroup, previousGroup } = ctx.config
103+
const currentApiTypeOperations = operations.filter(({ apiType }) => apiType === currentApiType)
104+
105+
const resolvedHashMap: ResolvedVersionOperationsHashMap = {}
106+
const normalizedToOriginalOperationIdMap: Record<NormalizedOperationId | OperationId, OperationId> = {}
107+
108+
for (const { operationId, apiType, metadata, dataHash } of currentApiTypeOperations) {
109+
const normalizedOperationId = apiType === REST_API_TYPE ? slugify(`${metadata.path}-${metadata.method}`, [], IGNORE_PATH_PARAM_UNIFIED_PLACEHOLDER) : operationId
110+
resolvedHashMap[normalizedOperationId] = dataHash
111+
normalizedToOriginalOperationIdMap[normalizedOperationId] = operationId
112+
}
98113

99114
if (buildType !== BUILD_TYPE.PREFIX_GROUPS_CHANGELOG) {
100-
return resolvedHashMap
115+
return [resolvedHashMap, normalizedToOriginalOperationIdMap]
101116
}
102117

103118
if (!currentGroup || !previousGroup) {
104119
ctx.notifications.push({
105120
severity: MESSAGE_SEVERITY.Warning,
106121
message: `Build type is prefix group changelog, but one of the groups is not provided: currentGroup=${currentGroup}, previousGroup=${previousGroup}`,
107122
})
108-
return resolvedHashMap
123+
return [resolvedHashMap, normalizedToOriginalOperationIdMap]
109124
}
110125

126+
const changedIdToOriginal: OperationIdentityMap = {}
127+
111128
for (const [operationId, dataHash] of Object.entries(resolvedHashMap)) {
112129
Reflect.deleteProperty(resolvedHashMap, operationId)
113130

@@ -116,11 +133,11 @@ export function getOperationsHashMapByApiType(
116133
if (operationId.startsWith(groupSlug)) {
117134
const changedOperationId = operationId.substring(groupSlug.length)
118135
resolvedHashMap[changedOperationId] = dataHash
119-
operationIdentityMap[changedOperationId] = operationId
136+
changedIdToOriginal[changedOperationId] = normalizedToOriginalOperationIdMap[operationId]
120137
}
121138
}
122139

123-
return resolvedHashMap
140+
return [resolvedHashMap, changedIdToOriginal]
124141
}
125142

126143
export function getOperationMetadata(operation: ResolvedOperation): OperationChangesMetadata {

src/types/external/operations.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ export type VersionOperationsResolver = (
2727
packageId: PackageId,
2828
operationsIds?: OperationId[],
2929
includeData?: boolean,
30+
operationsCount?: number,
3031
) => Promise<ResolvedOperations | null>
3132

3233
export interface ResolvedOperations {
@@ -35,7 +36,7 @@ export interface ResolvedOperations {
3536
}
3637

3738
export interface ResolvedOperation<M = any> {
38-
operationId: string
39+
operationId: OperationId
3940
title: string
4041
dataHash: string
4142
apiType: OperationsApiType

src/types/external/version.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ export type VersionResolver = (
2121
packageId: PackageId,
2222
version: VersionId,
2323
includeOperations?: boolean,
24+
includeSummary?: boolean,
2425
) => Promise<ResolvedVersion | null>
2526

2627
export type ResolvedVersion = {
@@ -43,7 +44,6 @@ export type OperationTypes = {
4344
changesSummary?: ChangeSummary
4445
operationsCount?: number
4546
deprecatedCount?: number
46-
operations?: ResolvedVersionOperationsHashMap
4747
}
4848

4949
export interface ResolvedVersionOperationsHashMap {

src/utils/builder.ts

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,15 @@
1515
*/
1616

1717
import { ApiAudienceTransition } from './../types/external/comparison'
18-
import { ApiKind, ChangeSummary, ResolvedOperation } from '../types'
1918
import {
2019
ANNOTATION_CHANGE_TYPE,
20+
ApiKind,
2121
BREAKING_CHANGE_TYPE,
22+
ChangeSummary,
2223
DEPRECATED_CHANGE_TYPE,
2324
ImpactedOperationSummary,
2425
NON_BREAKING_CHANGE_TYPE,
26+
ResolvedOperation,
2527
SEMI_BREAKING_CHANGE_TYPE,
2628
UNCLASSIFIED_CHANGE_TYPE,
2729
} from '../types'
@@ -41,10 +43,19 @@ export const removeFirstSlash = (input: string): string => {
4143
return input.startsWith('/') ? input.substring(1) : input
4244
}
4345

44-
export const normalizePath = (path: string): string => {
45-
return path.replace(new RegExp('{.*?}', 'g'), '*')
46+
export type NormalizedPath = string
47+
48+
export const normalizePath = (path: string): NormalizedPath => {
49+
return hidePathParamNames(path)
50+
}
51+
52+
export function hidePathParamNames(path: string): string {
53+
return path.replace(PATH_PARAMETER_REGEXP, PATH_PARAM_UNIFIED_PLACEHOLDER)
4654
}
4755

56+
const PATH_PARAMETER_REGEXP = /\{.*?\}/g
57+
export const PATH_PARAM_UNIFIED_PLACEHOLDER = '*'
58+
4859
export const filesDiff = (files1: { fileId: string }[], files2: { fileId: string }[]): { fileId: string }[] => {
4960
return files1.filter((f1) => !files2.find((f2) => f1.fileId === f2.fileId))
5061
}

src/utils/document.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
* limitations under the License.
1515
*/
1616

17-
import createSlug from 'slug'
17+
import createSlug, { CharMap } from 'slug'
1818
import {
1919
_ParsedFileResolver,
2020
ApiOperation,
@@ -87,6 +87,7 @@ createSlug.extend({ '_': '_' })
8787
createSlug.extend({ '.': '-' })
8888
createSlug.extend({ '(': '-' })
8989
createSlug.extend({ ')': '-' })
90+
// createSlug.extend({ '*': '*' })
9091

9192
export const findSharedPath = (fileIds: string[]): string => {
9293
if (!fileIds.length) { return '' }
@@ -99,12 +100,12 @@ export const findSharedPath = (fileIds: string[]): string => {
99100
return first.slice(0, i).join('/') + (i ? '/' : '')
100101
}
101102

102-
export const slugify = (text: string, slugs: string[] = []): string => {
103+
export const slugify = (text: string, slugs: string[] = [], charMapEntry?: CharMap): string => {
103104
if (!text) {
104105
return ''
105106
}
106107

107-
const slug = createSlug(text)
108+
const slug = createSlug(text, { charmap: { ...createSlug.charmap, ...charMapEntry } })
108109
let suffix: string = ''
109110
// add suffix if not unique
110111
while (slugs.includes(slug + suffix)) { suffix = String(+suffix + 1) }

0 commit comments

Comments
 (0)