Skip to content

Commit 86c9b86

Browse files
b41exb41ex
authored
fix: correctly map several media types which are the same in before/after but has wildcards (#19)
1 parent 9671994 commit 86c9b86

File tree

6 files changed

+567
-33
lines changed

6 files changed

+567
-33
lines changed

‎src/openapi/openapi3.mapping.ts‎

Lines changed: 72 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -92,38 +92,89 @@ export const contentMediaTypeMappingResolver: MappingResolver<string> = (before,
9292
const result: MapKeysResult<string> = { added: [], removed: [], mapped: {} }
9393

9494
const beforeKeys = objectKeys(before)
95-
const _beforeKeys = beforeKeys.map((key) => key.split(';')[0] ?? '')
9695
const afterKeys = objectKeys(after)
97-
const _afterKeys = afterKeys.map((key) => key.split(';')[0] ?? '')
9896

99-
const mappedIndex = new Set(afterKeys.keys())
100-
101-
for (let i = 0; i < beforeKeys.length; i++) {
102-
const _afterIndex = _afterKeys.findIndex((key) => {
103-
const [afterType, afterSubType] = key.split('/')
104-
const [beforeType, beforeSubType] = _beforeKeys[i].split('/')
97+
const unmappedAfterIndices = new Set(afterKeys.keys())
98+
const unmappedBeforeIndices = new Set(beforeKeys.keys())
99+
100+
function mapExactMatches(
101+
getComparisonKey: (key: string) => string
102+
): void {
103+
104+
for (const beforeIndex of unmappedBeforeIndices) {
105+
const beforeKey = getComparisonKey(beforeKeys[beforeIndex])
106+
107+
// Find matching after index by iterating over the after indices set
108+
let matchingAfterIndex: number | undefined
109+
for (const afterIndex of unmappedAfterIndices) {
110+
const afterKey = getComparisonKey(afterKeys[afterIndex])
111+
if (afterKey === beforeKey) {
112+
matchingAfterIndex = afterIndex
113+
break
114+
}
115+
}
105116

106-
if (afterType !== beforeType && afterType !== '*' && beforeType !== '*') { return false }
107-
if (afterSubType !== beforeSubType && afterSubType !== '*' && beforeSubType !== '*') { return false }
108-
return true
109-
})
117+
if (matchingAfterIndex !== undefined) {
118+
// Match found - create mapping and remove from unmapped sets
119+
result.mapped[beforeKeys[beforeIndex]] = afterKeys[matchingAfterIndex]
120+
unmappedAfterIndices.delete(matchingAfterIndex)
121+
unmappedBeforeIndices.delete(beforeIndex)
122+
}
123+
}
124+
}
110125

111-
if (_afterIndex < 0 || !mappedIndex.has(_afterIndex)) {
112-
// removed item
113-
result.removed.push(beforeKeys[i])
114-
} else {
115-
// mapped items
116-
result.mapped[beforeKeys[i]] = afterKeys[_afterIndex]
117-
mappedIndex.delete(_afterIndex)
126+
// First, map exact matches for full media type
127+
mapExactMatches((key) => key)
128+
129+
// After that, try to map media types by base type for remaining unmapped keys
130+
mapExactMatches(getMediaTypeBase)
131+
132+
// If exactly one unmapped item in both before and after, try wildcard matching
133+
if (unmappedBeforeIndices.size === 1 && unmappedAfterIndices.size === 1) {
134+
const beforeIndex = Array.from(unmappedBeforeIndices)[0]
135+
const afterIndex = Array.from(unmappedAfterIndices)[0]
136+
const beforeKey = beforeKeys[beforeIndex]
137+
const afterKey = afterKeys[afterIndex]
138+
const beforeBaseType = getMediaTypeBase(beforeKey)
139+
const afterBaseType = getMediaTypeBase(afterKey)
140+
141+
// Check if they are compatible using wildcard matching
142+
if (isWildcardCompatible(beforeBaseType, afterBaseType)) {
143+
// Map them together
144+
result.mapped[beforeKeys[beforeIndex]] = afterKeys[afterIndex]
145+
unmappedAfterIndices.delete(afterIndex)
146+
unmappedBeforeIndices.delete(beforeIndex)
118147
}
119148
}
120149

121-
// added items
122-
mappedIndex.forEach((i) => result.added.push(afterKeys[i]))
150+
// Mark remaining unmapped items as removed/added
151+
unmappedBeforeIndices.forEach((index) => result.removed.push(beforeKeys[index]))
152+
unmappedAfterIndices.forEach((index) => result.added.push(afterKeys[index]))
123153

124154
return result
125155
}
126156

157+
function getMediaTypeBase(mediaType: string): string {
158+
return mediaType.split(';')[0] ?? ''
159+
}
160+
161+
function isWildcardCompatible(beforeType: string, afterType: string): boolean {
162+
const [beforeMainType, beforeSubType] = beforeType.split('/')
163+
const [afterMainType, afterSubType] = afterType.split('/')
164+
165+
// Check main type compatibility
166+
if (beforeMainType !== afterMainType && beforeMainType !== '*' && afterMainType !== '*') {
167+
return false
168+
}
169+
170+
// Check sub type compatibility
171+
if (beforeSubType !== afterSubType && beforeSubType !== '*' && afterSubType !== '*') {
172+
return false
173+
}
174+
175+
return true
176+
}
177+
127178
export function hidePathParamNames(path: string): string {
128179
return path.replace(PATH_PARAMETER_REGEXP, PATH_PARAM_UNIFIED_PLACEHOLDER)
129180
}

‎test/bugs.test.ts‎

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@ import changeToNothingClassificationAfter from './helper/resources/change-to-not
2727
import spearedParamsBefore from './helper/resources/speared-parameters/before.json'
2828
import spearedParamsAfter from './helper/resources/speared-parameters/after.json'
2929

30+
import wildcardContentSchemaMediaTypeCombinedWithSpecificMediaTypeBefore from './helper/resources/wildcard-content-schema-media-type-combined-with-specific-media-type/before.json'
31+
import wildcardContentSchemaMediaTypeCombinedWithSpecificMediaTypeAfter from './helper/resources/wildcard-content-schema-media-type-combined-with-specific-media-type/after.json'
32+
3033
import { diffsMatcher } from './helper/matchers'
3134
import { TEST_DIFF_FLAG, TEST_ORIGINS_FLAG } from './helper'
3235
import { JSON_SCHEMA_NODE_SYNTHETIC_TYPE_NOTHING } from '@netcracker/qubership-apihub-api-unifier'
@@ -97,16 +100,7 @@ describe('Real Data', () => {
97100
const after: any = infinityAfter
98101
const { diffs } = apiDiff(before, after, OPTIONS)
99102
const responseContentPath = ['paths', '/api/v1/dictionaries/dictionary/item', 'get', 'responses', '200', 'content']
100-
expect(diffs).toEqual(diffsMatcher([
101-
expect.objectContaining({
102-
beforeDeclarationPaths: [[...responseContentPath, '*/*']],
103-
afterDeclarationPaths: [[...responseContentPath, 'application/json']],
104-
action: DiffAction.rename,
105-
beforeKey: '*/*',
106-
afterKey: 'application/json',
107-
type: nonBreaking,
108-
scope: 'response',
109-
}),
103+
expect(diffs).toEqual(diffsMatcher([
110104
expect.objectContaining({
111105
afterDeclarationPaths: [['components', 'schemas', 'DictionaryItem', 'x-entity']],
112106
afterValue: 'DictionaryItem',
@@ -210,10 +204,27 @@ describe('Real Data', () => {
210204
}),
211205
]))
212206
})
213-
it('spered parameters', () => {
207+
208+
it('speared parameters', () => {
214209
const before: any = spearedParamsBefore
215210
const after: any = spearedParamsAfter
216211
const { diffs } = apiDiff(before, after, OPTIONS)
217212
expect(diffs).toBeEmpty()
218213
})
214+
215+
// The original issue was that media type was reported as added/removed, when nothing actually changed
216+
it('wildcard content schema media type in combination with specific media type', () => {
217+
const before: any = wildcardContentSchemaMediaTypeCombinedWithSpecificMediaTypeBefore
218+
const after: any = wildcardContentSchemaMediaTypeCombinedWithSpecificMediaTypeAfter
219+
const { diffs } = apiDiff(before, after, OPTIONS)
220+
221+
expect(diffs).toEqual(diffsMatcher([
222+
expect.objectContaining({
223+
action: DiffAction.replace,
224+
beforeDeclarationPaths: [['servers', 0, 'url']],
225+
afterDeclarationPaths: [['servers', 0, 'url']],
226+
type: annotation,
227+
}),
228+
]))
229+
})
219230
})

‎test/helper/resources/ref-with-array-to-self/after.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
"200": {
88
"description": "OK",
99
"content": {
10-
"application/json": {
10+
"*/*": {
1111
"schema": {
1212
"type": "array",
1313
"items": {

‎test/helper/resources/wildcard-content-schema-media-type-combined-with-specific-media-type/after.json‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
{
2+
"openapi": "3.0.1",
3+
"servers": [
4+
{
5+
"url": "http://config-server.cbss-kue-coe-dv.cl.local:1111",
6+
"description": "Generated server url"
7+
}
8+
],
9+
"paths": {
10+
"/path1": {
11+
"get": {
12+
"operationId": "retrieve_1",
13+
"responses": {
14+
"200": {
15+
"description": "OK",
16+
"content": {
17+
"*/*": {
18+
"schema": {
19+
"type": "string"
20+
}
21+
},
22+
"application/octet-stream": {
23+
"schema": {
24+
"type": "string",
25+
"format": "byte"
26+
}
27+
}
28+
}
29+
},
30+
"404": {
31+
"description": "Not Found"
32+
}
33+
}
34+
}
35+
}
36+
},
37+
"components": {}
38+
}

‎test/helper/resources/wildcard-content-schema-media-type-combined-with-specific-media-type/before.json‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
{
2+
"openapi": "3.0.1",
3+
"servers": [
4+
{
5+
"url": "http://config-server:1111",
6+
"description": "Generated server url"
7+
}
8+
],
9+
"paths": {
10+
"/path1": {
11+
"get": {
12+
"operationId": "retrieve_1",
13+
"responses": {
14+
"200": {
15+
"description": "OK",
16+
"content": {
17+
"*/*": {
18+
"schema": {
19+
"type": "string"
20+
}
21+
},
22+
"application/octet-stream": {
23+
"schema": {
24+
"type": "string",
25+
"format": "byte"
26+
}
27+
}
28+
}
29+
},
30+
"404": {
31+
"description": "Not Found"
32+
}
33+
}
34+
}
35+
}
36+
},
37+
"components": {}
38+
}

0 commit comments

Comments
 (0)