Skip to content
Open
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,13 @@
"update-lock-file": "update-lock-file @netcracker"
},
"dependencies": {
"@netcracker/qubership-apihub-api-unifier": "dev",
"@netcracker/qubership-apihub-api-unifier": "feature-performance-optimization",
"@netcracker/qubership-apihub-json-crawl": "1.0.4",
"fast-equals": "4.0.3"
},
"devDependencies": {
"@netcracker/qubership-apihub-compatibility-suites": "dev",
"@netcracker/qubership-apihub-graphapi": "1.0.8",
"@netcracker/qubership-apihub-graphapi": "feature-performance-optimization",
"@netcracker/qubership-apihub-npm-gitflow": "3.1.0",
"@types/jest": "29.5.11",
"@types/node": "20.11.6",
Expand Down
1 change: 1 addition & 0 deletions src/core/constants.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { ClassifyRule } from '../types'

export const DIFF_META_KEY = Symbol('$diff')
export const DIFFS_AGGREGATED_META_KEY = Symbol('$diffs-aggregated')
export const DEFAULT_NORMALIZED_RESULT = false
export const DEFAULT_OPTION_DEFAULTS_META_KEY = Symbol('$defaults')
export const DEFAULT_OPTION_ORIGINS_META_KEY = Symbol('$origins')
Expand Down
14 changes: 13 additions & 1 deletion src/index.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,16 @@
export { COMPARE_MODE_DEFAULT, COMPARE_MODE_OPERATION } from './types'

export {
ClassifierType, DiffAction, DIFF_META_KEY, breaking, nonBreaking, unclassified, annotation, deprecated, risky,
ClassifierType,
DiffAction,
DIFFS_AGGREGATED_META_KEY,
DIFF_META_KEY,
breaking,
nonBreaking,
unclassified,
annotation,
deprecated,
risky,
} from './core'

export { apiDiff } from './api'
Expand All @@ -24,4 +33,7 @@ export {
isDiffRename,
isDiffReplace,
} from './utils'

export { onlyExistedArrayIndexes } from './utils'

export { aggregateDiffsWithRollup } from './utils'
10 changes: 5 additions & 5 deletions src/openapi/openapi3.classify.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,16 @@ import {
breakingIfAfterTrue,
nonBreaking,
PARENT_JUMP,
strictResolveValueFromContext,
reverseClassifyRule,
strictResolveValueFromContext,
transformClassifyRule,
unclassified,
} from '../core'
import { getKeyValue, isExist, isNotEmptyArray } from '../utils'
import { emptySecurity, includeSecurity } from './openapi3.utils'
import type { ClassifyRule, CompareContext } from '../types'
import { DiffType } from '../types'
import { hidePathParamNames } from './openapi3.mapping'
import { createPathUnifier } from './openapi3.mapping'

export const paramClassifyRule: ClassifyRule = [
({ after }) => {
Expand Down Expand Up @@ -143,9 +143,9 @@ export const pathChangeClassifyRule: ClassifyRule = [
({ before, after }) => {
const beforePath = before.key as string
const afterPath = after.key as string
const unifiedBeforePath = hidePathParamNames(beforePath)
const unifiedAfterPath = hidePathParamNames(afterPath)
const unifiedBeforePath = createPathUnifier(before)(beforePath)
const unifiedAfterPath = createPathUnifier(after)(afterPath)

// If unified paths are the same, it means only parameter names changed
return unifiedBeforePath === unifiedAfterPath ? annotation : breaking
}
Expand Down
83 changes: 62 additions & 21 deletions src/openapi/openapi3.mapping.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
import type { MapKeysResult, MappingResolver } from '../types'
import { getStringValue, objectKeys, onlyExistedArrayIndexes } from '../utils'
import { MapKeysResult, MappingResolver, NodeContext } from '../types'
import {
difference,
getStringValue,
intersection,
objectKeys,
onlyExistedArrayIndexes,
removeSlashes,
} from '../utils'
import { mapPathParams } from './openapi3.utils'
import { OpenAPIV3 } from 'openapi-types'

export const singleOperationPathMappingResolver: MappingResolver<string> = (before, after) => {

Expand All @@ -23,32 +31,40 @@ export const singleOperationPathMappingResolver: MappingResolver<string> = (befo
return result
}

export const pathMappingResolver: MappingResolver<string> = (before, after) => {
export const pathMappingResolver: MappingResolver<string> = (before, after, ctx) => {

const result: MapKeysResult<string> = { added: [], removed: [], mapped: {} }

const originalBeforeKeys = objectKeys(before)
const originalAfterKeys = objectKeys(after)
const unifiedAfterKeys = originalAfterKeys.map(hidePathParamNames)
const unifyBeforePath = createPathUnifier(ctx.before)
const unifyAfterPath = createPathUnifier(ctx.after)

const notMappedAfterIndices = new Set(originalAfterKeys.keys())
const unifiedBeforeKeyToKey = Object.fromEntries(objectKeys(before).map(key => [unifyBeforePath(key), key]))
const unifiedAfterKeyToKey = Object.fromEntries(objectKeys(after).map(key => [unifyAfterPath(key), key]))

originalBeforeKeys.forEach(beforeKey => {
const unifiedBeforePath = hidePathParamNames(beforeKey)
const index = unifiedAfterKeys.indexOf(unifiedBeforePath)
const unifiedBeforeKeys = Object.keys(unifiedBeforeKeyToKey)
const unifiedAfterKeys = Object.keys(unifiedAfterKeyToKey)

if (index < 0) {
// removed item
result.removed.push(beforeKey)
} else {
// mapped items
result.mapped[beforeKey] = originalAfterKeys[index]
notMappedAfterIndices.delete(index)
}
})
result.added = difference(unifiedAfterKeys, unifiedBeforeKeys).map(key => unifiedAfterKeyToKey[key])
result.removed = difference(unifiedBeforeKeys, unifiedAfterKeys).map(key => unifiedBeforeKeyToKey[key])
result.mapped = Object.fromEntries(
intersection(unifiedBeforeKeys, unifiedAfterKeys).map(key => [unifiedBeforeKeyToKey[key], unifiedAfterKeyToKey[key]]),
)

return result
}

export const methodMappingResolver: MappingResolver<string> = (before, after) => {

const result: MapKeysResult<string> = { added: [], removed: [], mapped: {} }

const beforeKeys = objectKeys(before)
const afterKeys = objectKeys(after)

// added items
notMappedAfterIndices.forEach((notMappedIndex) => result.added.push(originalAfterKeys[notMappedIndex]))
result.added = difference(afterKeys, beforeKeys)
result.removed = difference(beforeKeys, afterKeys)

const mapped = intersection(beforeKeys, afterKeys)
mapped.forEach(key => result.mapped[key] = key)

return result
}
Expand Down Expand Up @@ -175,6 +191,31 @@ function isWildcardCompatible(beforeType: string, afterType: string): boolean {
return true
}

// todo copy-paste from api-processor
export const extractOperationBasePath = (servers?: OpenAPIV3.ServerObject[]): string => {
if (!Array.isArray(servers) || !servers.length) { return '' }

try {
const [firstServer] = servers
let serverUrl = firstServer.url
const { variables = {} } = firstServer

for (const param of Object.keys(variables)) {
serverUrl = serverUrl.replace(new RegExp(`{${param}}`, 'g'), variables[param].default)
}

const { pathname } = new URL(serverUrl, 'https://localhost')
return pathname.slice(-1) === '/' ? pathname.slice(0, -1) : pathname
} catch (error) {
return ''
}
}

export function createPathUnifier(nodeContext: NodeContext): (path: string) => string {
const serverPrefix = extractOperationBasePath((nodeContext.root as OpenAPIV3.Document).servers) // /api/v2
return (path) => removeSlashes(`${serverPrefix}${hidePathParamNames(path)}`)
}

export function hidePathParamNames(path: string): string {
return path.replace(PATH_PARAMETER_REGEXP, PATH_PARAM_UNIFIED_PLACEHOLDER)
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no comments but because all these manipulations like magic. So I would like to request some documentation for these operations because there a lot of utility functions calling each other and it's difficult to understand whole the algorithm

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added jsdoc for path mapper.

Expand Down
3 changes: 2 additions & 1 deletion src/openapi/openapi3.rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ import {
} from './openapi3.classify'
import {
contentMediaTypeMappingResolver,
methodMappingResolver,
paramMappingResolver,
pathMappingResolver,
singleOperationPathMappingResolver,
Expand Down Expand Up @@ -390,7 +391,7 @@ export const openApi3Rules = (options: OpenApi3RulesOptions): CompareRules => {

const pathItemObjectRules = (options: OpenApi3RulesOptions): CompareRules => ({
$: pathChangeClassifyRule,
mapping: options.mode === COMPARE_MODE_OPERATION ? singleOperationPathMappingResolver : pathMappingResolver,
mapping: options.mode === COMPARE_MODE_OPERATION ? singleOperationPathMappingResolver : methodMappingResolver,
'/description': { $: allAnnotation },
'/parameters': {
$: [nonBreaking, breaking, breaking],
Expand Down
85 changes: 85 additions & 0 deletions src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,3 +221,88 @@ export const checkPrimitiveType = (value: unknown): PrimitiveType | undefined =>
}
return undefined
}

export function intersection(array1: string[], array2: string[]): string[] {
const set2 = new Set(array2)
return [...new Set(array1.filter(x => set2.has(x)))]
}

export function difference(array1: string[], array2: string[]): string[] {
const set2 = new Set(array2)
return [...new Set(array1.filter(x => !set2.has(x)))]
}

export function removeSlashes(input: string): string {
return input.replace(/\//g, '')
}

/**
* Traverses the merged document starting from given obj to the bottom and aggregates the diffs with rollup from the bottom up.
* Each object in the tree will have aggregatedDiffProperty only if there are diffs in the object or in the children,
* otherwise the aggregatedDiffProperty is not added.
* Note, that adding/removing the object itself is not included in the aggregation for this object,
* you need retrieve this diffs from parent object if you need them.
* Supports cycled JSO, nested objects and arrays.
* @param obj - The object to aggregate the diffs of.
* @param diffProperty - The property of the object to aggregate the diffs of.
* @param aggregatedDiffProperty - The property of the object to store the aggregated diffs in.
* @returns The aggregated diffs of the given object.
*/

// TODO: generalize to other use cases (like collecting deprecated)
export function aggregateDiffsWithRollup(obj: any, diffProperty: any, aggregatedDiffProperty: any): Set<Diff> | undefined {

const visited = new Set<any>()

function _aggregateDiffsWithRollup(obj: any): Set<Diff> | undefined {
if (obj === null || typeof obj !== 'object' ) {
return undefined
}

if (visited.has(obj)) {
return obj[aggregatedDiffProperty]
}

visited.add(obj)

// Process all children and collect their diffs
const childrenDiffs = new Array<Set<Diff>>()
if (Array.isArray(obj)) {
for (const item of obj) {
const childDiffs = _aggregateDiffsWithRollup(item)
childDiffs && childDiffs.size > 0 && childrenDiffs.push(childDiffs)
}
} else {
for (const [_, value] of Object.entries(obj)) {
const childDiffs = _aggregateDiffsWithRollup(value)
childDiffs && childDiffs.size > 0 && childrenDiffs.push(childDiffs)
}
}

const hasOwnDiffs = diffProperty in obj

if (hasOwnDiffs || childrenDiffs.length > 1) {
// obj aggregated diffs are different from children diffs
const aggregatedDiffs = new Set<Diff>()
for (const childDiffs of childrenDiffs) {
childDiffs.forEach(diff => aggregatedDiffs.add(diff))
}
const diffs = obj[diffProperty]
for (const key in diffs) {
aggregatedDiffs.add(diffs[key])
}
// Store the aggregated diffs in the object
obj[aggregatedDiffProperty] = aggregatedDiffs
}else if (childrenDiffs.length === 1) {
// could reuse a child diffs if there is only one
[obj[aggregatedDiffProperty]] = childrenDiffs
}else{
// no diffs- no aggregated diffs get assigned
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we really need empty else?
and you may format the code

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Else is not empty, it contains comment:) This is to emphasize/remind the reader that in this case to aggregated diffs will be added.

Formatted code.


return obj[aggregatedDiffProperty]
}

return _aggregateDiffsWithRollup(obj)
}

89 changes: 89 additions & 0 deletions test/openapi.pathAndMethodMapping.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
import { annotation, apiDiff, DiffAction } from '../src'
import { OpenapiBuilder } from './helper'
import { diffsMatcher } from './helper/matchers'

describe('Path and method mapping', () => {
let openapiBuilder: OpenapiBuilder

beforeEach(() => {
openapiBuilder = new OpenapiBuilder()
})

it('Move prefix from server to path', () => {
const before = openapiBuilder
.addServer('https://example1.com/api/v2')
.addPath({
path: '/path1',
responses: {
'200': {
description: 'OK',
},
},
})
.getSpec()

openapiBuilder.reset()

const after = openapiBuilder
.addServer('https://example1.com')
.addPath({
path: '/api/v2/path1', responses: {
'200': {
description: 'not OK',
},
},
})
.getSpec()

const { diffs } = apiDiff(before, after)

expect(diffs).toEqual(diffsMatcher([
expect.objectContaining({
beforeDeclarationPaths: [['paths', '/path1']],
afterDeclarationPaths: [['paths', '/api/v2/path1']],
action: DiffAction.rename,
type: annotation,
}),
expect.objectContaining({
beforeDeclarationPaths: [['paths', '/path1', 'get', 'responses', '200', 'description']],
afterDeclarationPaths: [['paths', '/api/v2/path1', 'get', 'responses', '200', 'description']],
action: DiffAction.replace,
type: annotation,
}),
expect.objectContaining({
beforeDeclarationPaths: [['servers', 0, 'url']],
action: DiffAction.replace,
type: annotation,
}),
]))
})

it('Remove mistyped slashes', () => {
const before = openapiBuilder
.addPath({
path: '//path1/',
responses: { '200': { description: 'OK' } },
})
.getSpec()

openapiBuilder.reset()

const after = openapiBuilder
.addPath({
path: '/path1',
responses: { '200': { description: 'OK' } },
})
.getSpec()

const { diffs } = apiDiff(before, after)

expect(diffs).toEqual(diffsMatcher([
expect.objectContaining({
beforeDeclarationPaths: [['paths', '//path1/']],
afterDeclarationPaths: [['paths', '/path1']],
action: DiffAction.rename,
type: annotation,
})
]))
})
})
Loading