Skip to content

Commit bacd6bb

Browse files
b41exb41ex
authored
fix: remove duplicated parameters during unification (#39)
fix: densify parameters arrays during unification
1 parent aeeacb9 commit bacd6bb

File tree

9 files changed

+638
-11
lines changed

9 files changed

+638
-11
lines changed

‎src/errors.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,4 +8,5 @@ export const ErrorMessage = {
88
referenceNotAllowed: (ref: string) => `${JSON_SCHEMA_PROPERTY_REF} not allowed here: ${ref}`,
99
refNotFound: (ref: string) => `${JSON_SCHEMA_PROPERTY_REF} can't be resolved: ${ref}`,
1010
refNotValidFormat: (ref: string) => `${JSON_SCHEMA_PROPERTY_REF} can't be parsed: ${ref}`,
11+
duplicateParameter: (name: string, location: string) => `Duplicate parameter detected: name='${name}', in='${location}'`,
1112
} as const

‎src/origins.ts‎

Lines changed: 106 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,111 @@ export function concatenateArraysInProperty(source: Jso, target: Jso, propertyKe
100100
})
101101
}
102102

103+
export function removeDuplicates<T>(
104+
array: T[],
105+
getUniqueKey: (item: T) => string | undefined,
106+
onDuplicate?: (item: T, index: number, firstIndex: number) => void
107+
): T[] {
108+
const seen = new Map<string, number>() // key -> first index
109+
110+
for (let i = 0; i < array.length; i++) {
111+
const item = array[i]
112+
const key = getUniqueKey(item)
113+
114+
if (key === undefined) {
115+
continue // Skip items without valid keys
116+
}
117+
118+
const firstIndex = seen.get(key)
119+
if (firstIndex !== undefined) {
120+
// Duplicate found
121+
onDuplicate?.(item, i, firstIndex)
122+
delete array[i] // Create sparse array
123+
} else {
124+
seen.set(key, i)
125+
}
126+
}
127+
128+
return array // Returns sparse array
129+
}
130+
131+
export function densify<T>(sparseArray: T[], originsFlag: symbol | undefined): T[] {
132+
const dense: T[] = []
133+
const originsRecord: OriginsMetaRecord = {}
134+
135+
for (let i = 0; i < sparseArray.length; i++) {
136+
if (i in sparseArray) { // Check if index exists (not deleted)
137+
const newIndex = dense.length
138+
dense[newIndex] = sparseArray[i]
139+
140+
// Copy origins if they exist
141+
if (originsFlag) {
142+
const sourceOrigins = resolveOrigins(sparseArray, i, originsFlag)
143+
if (sourceOrigins) {
144+
originsRecord[newIndex] = sourceOrigins
145+
}
146+
}
147+
}
148+
}
149+
150+
// Set origins for the dense array
151+
if (originsFlag && Object.keys(originsRecord).length > 0) {
152+
setJsoProperty(dense as any, originsFlag, originsRecord)
153+
}
154+
155+
return dense
156+
}
157+
158+
export function addUniqueElements(
159+
source: Jso,
160+
target: Jso,
161+
propertyKey: PropertyKey,
162+
originsFlag: symbol | undefined,
163+
getUniqueKey: (item: any) => string | undefined
164+
): void {
165+
const sourceArray = getJsoProperty(source, propertyKey)
166+
if (!isArray(sourceArray)) {
167+
return
168+
}
169+
170+
let targetArray = getJsoProperty(target, propertyKey)
171+
if (targetArray === undefined) {
172+
targetArray = []
173+
copyOrigins(source, target, propertyKey, propertyKey, originsFlag)
174+
}
175+
176+
if (!isArray(targetArray)) {
177+
return
178+
}
179+
180+
setJsoProperty(target, propertyKey, targetArray)
181+
182+
// Build set of existing keys in target
183+
const existingKeys = new Set<string>()
184+
for (const item of targetArray) {
185+
const key = getUniqueKey(item)
186+
if (key !== undefined) {
187+
existingKeys.add(key)
188+
}
189+
}
190+
191+
// Add source items that don't exist in target
192+
for (let i = 0; i < sourceArray.length; i++) {
193+
const item = sourceArray[i]
194+
const key = getUniqueKey(item)
195+
196+
// Skip if already exists in target (operation overrides path) or key is undefined
197+
if (key === undefined || existingKeys.has(key)) {
198+
continue
199+
}
200+
201+
// Add to target
202+
const targetIndex = targetArray.length
203+
targetArray[targetIndex] = item
204+
copyOrigins(sourceArray, targetArray, i, targetIndex, originsFlag)
205+
}
206+
}
207+
103208
export function resolveOriginsMetaRecord(source: Jso, originsFlag: symbol | undefined): OriginsMetaRecord | undefined {
104209
if (!originsFlag) {
105210
return undefined
@@ -214,4 +319,4 @@ export function copyOriginsForArray(sourceArray: unknown[], targetArray: unknown
214319

215320
export const createSelfOriginsCloneHook: <T extends HasSelfOriginsResolver, R extends {}>(originsFlag: symbol | undefined) => SyncCloneHook<T, R> = <State extends HasSelfOriginsResolver, Rules extends {}>(originsFlag: symbol | undefined) => {
216321
return createSelfMetaCloneHook<OriginLeafs, 'selfOriginResolver', State, Rules>('selfOriginResolver', originsFlag, [])
217-
}
322+
}

‎src/rules/openapi.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ import {
7676
OPEN_API_PROPERTY_TAGS,
7777
} from './openapi.const'
7878

79-
import { pathItemsUnification } from '../unifies/openapi'
79+
import { pathItemsUnification, deduplicateParameters } from '../unifies/openapi'
8080
import {
8181
calculateHeaderName,
8282
calculateHeaderPlace,
@@ -626,6 +626,7 @@ const openApiParametersRules = (version: OpenApiSpecVersion): NormalizationRules
626626
hashOwner: true,
627627
},
628628
validate: checkType(TYPE_ARRAY),
629+
unify: deduplicateParameters,
629630
})
630631

631632
const openApiRequestRules = (version: OpenApiSpecVersion): NormalizationRules => ({

‎src/unifies/openapi.ts‎

Lines changed: 43 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,53 @@
11
import { UnifyFunction } from '../types'
2-
import { isObject } from '@netcracker/qubership-apihub-json-crawl'
2+
import { isArray, isObject } from '@netcracker/qubership-apihub-json-crawl'
33
import { OpenAPIV3 } from 'openapi-types'
44
import { OPEN_API_HTTP_METHODS } from '../rules/openapi.const'
5-
import { cleanSeveralOrigins, concatenateArraysInProperty, copyProperty } from '../origins'
5+
import {
6+
addUniqueElements,
7+
cleanSeveralOrigins,
8+
concatenateArraysInProperty,
9+
copyProperty,
10+
densify,
11+
removeDuplicates,
12+
} from '../origins'
13+
import { ErrorMessage } from '../errors'
614
import PathItemObject = OpenAPIV3.PathItemObject
15+
import ParameterObject = OpenAPIV3.ParameterObject
716

817
const EXTENSION_PREFIX = 'x-'
918

19+
const getParameterUniqueKey = (param: ParameterObject): string | undefined => {
20+
if (!isObject(param)) {
21+
return undefined
22+
}
23+
const name = param.name
24+
const inValue = param.in
25+
if (name !== undefined && inValue !== undefined) {
26+
return `${name}:${inValue}`
27+
}
28+
return undefined
29+
}
30+
31+
export const deduplicateParameters: UnifyFunction = (jso, ctx) => {
32+
if (!isArray(jso)) {
33+
return jso
34+
}
35+
36+
const sparseArray = removeDuplicates(
37+
jso,
38+
getParameterUniqueKey,
39+
(item, index, firstIndex) => {
40+
// Report duplicate error
41+
const message = ErrorMessage.duplicateParameter(item?.name, item?.in)
42+
ctx.options.onUnifyError?.(message, ctx.path, jso, new Error(message))
43+
}
44+
)
45+
46+
return densify(sparseArray, ctx.options.originsFlag)
47+
}
48+
1049
export const pathItemsUnification: UnifyFunction = (value, { options }) => {
11-
if (!isObject(value)) { return value}
50+
if (!isObject(value)) { return value }
1251
const pathItem: PathItemObject = value
1352
const {
1453
parameters: pathParameters,
@@ -41,7 +80,7 @@ export const pathItemsUnification: UnifyFunction = (value, { options }) => {
4180
}
4281
result[method] = operation //origin copy already.
4382
// Deep copy not allowed!!! So only mutations :(
44-
concatenateArraysInProperty(pathItem, operation, 'parameters', options.originsFlag)
83+
addUniqueElements(pathItem, operation, 'parameters', options.originsFlag, getParameterUniqueKey)
4584
concatenateArraysInProperty(pathItem, operation, 'servers', options.originsFlag)
4685
if (operation.summary === undefined && pathSummary !== undefined) {
4786
copyProperty(pathItem, operation, 'summary', options.originsFlag)

‎src/utils.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -383,4 +383,4 @@ export const removeDuplicatesWithMergeOrigins = <T>(array: T[], originFlag: symb
383383
setOriginsForArray(uniqueItems, originFlag, itemOrigins)
384384

385385
return uniqueItems
386-
}
386+
}

‎test/oas/bugs.test.ts‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { TEST_DEFAULTS_FLAG, TEST_HASH_FLAG, TEST_SYNTHETIC_ALL_OF_FLAG } from '
22
import { denormalize, JSON_SCHEMA_PROPERTY_DEPRECATED, normalize, OriginLeafs, resolveOrigins } from '../../src'
33
import outOfMemoryCauseTooManyCombinations from '../resources/out-of-memory-cause-too-many-combinations.json'
44
import bugWithWrongOrigins from '../resources/bug-with-wrong-origins.json'
5-
import bugWithSpearedArray from '../resources/bug-with-speared-array.json'
5+
import bugWithSparseArray from '../resources/bug-with-sparse-array.json'
66
import { TEST_INLINE_REFS_FLAG, TEST_ORIGINS_FLAG, TEST_ORIGINS_FOR_DEFAULTS, TEST_SYNTHETIC_TITLE_FLAG } from '../helpers'
77
import { isObject, syncCrawl } from '@netcracker/qubership-apihub-json-crawl'
88
import 'jest-extended'
@@ -239,8 +239,8 @@ describe('Bugs', () => {
239239
expect(result.paths['customer'].get.parameters[0].schema).not.toHaveProperty([TEST_INLINE_REFS_FLAG])
240240
})
241241

242-
it('speared array', () => {
243-
const result = normalize(bugWithSpearedArray, {
242+
it('sparse array', () => {
243+
const result = normalize(bugWithSparseArray, {
244244
validate: true,
245245
liftCombiners: true,
246246
unify: true,
@@ -250,6 +250,10 @@ describe('Bugs', () => {
250250
hashFlag: TEST_HASH_FLAG,
251251
inlineRefsFlag: TEST_INLINE_REFS_FLAG,
252252
}) as any
253-
expect(result).toHaveProperty(['paths', '/datasets/events', 'get', 'parameters', 'length'], 10) /*1 and 3 index will be missing*/
253+
254+
expect(result).toHaveProperty(['paths', '/datasets/events', 'get', 'parameters', 'length'], 8)
255+
// Verify all parameters are valid objects
256+
const params = result.paths['/datasets/events'].get.parameters
257+
expect(params.every((p: any) => p && typeof p === 'object')).toBe(true)
254258
})
255259
})

‎test/oas/unify.origins.test.ts‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -273,4 +273,43 @@ describe('unify origins', function () {
273273
})
274274

275275
})
276+
277+
it('should preserve origins when unifying path item and deduplicating operation parameters', () => {
278+
const spec = {
279+
openapi: '3.0.0',
280+
info: { title: 'Test API', version: '1.0.0' },
281+
paths: {
282+
'test': {
283+
parameters: [
284+
{ name: 'id', in: 'query', schema: { type: 'string' } },
285+
{ name: 'id', in: 'query', schema: { type: 'string' } }, // duplicate
286+
{ name: 'limit', in: 'query', schema: { type: 'integer' } },
287+
],
288+
get: {
289+
parameters: [
290+
{ name: 'offset', in: 'query', schema: { type: 'string' } },
291+
{ name: 'offset', in: 'query', schema: { type: 'string' } }, // duplicate
292+
{ name: 'maxItems', in: 'query', schema: { type: 'integer' } },
293+
],
294+
responses: {
295+
'200': { description: 'OK' },
296+
},
297+
},
298+
},
299+
},
300+
}
301+
302+
const result = normalize(spec, OPTIONS) as any
303+
304+
// Should have 2 parameters after deduplication
305+
const params = result.paths?.['test']?.get?.parameters
306+
expect(params).toHaveLength(4)
307+
308+
commonOriginsCheck(result, { source: spec })
309+
const resultWithHmr = convertOriginToHumanReadable(result, TEST_ORIGINS_FLAG)
310+
expect(resultWithHmr).toHaveProperty(['paths', 'test', 'get', 'parameters', TEST_ORIGINS_FLAG, 0], ['paths/test/get/parameters/0'])
311+
expect(resultWithHmr).toHaveProperty(['paths', 'test', 'get', 'parameters', TEST_ORIGINS_FLAG, 1], ['paths/test/get/parameters/2'])
312+
expect(resultWithHmr).toHaveProperty(['paths', 'test', 'get', 'parameters', TEST_ORIGINS_FLAG, 2], ['paths/test/parameters/0'])
313+
expect(resultWithHmr).toHaveProperty(['paths', 'test', 'get', 'parameters', TEST_ORIGINS_FLAG, 3], ['paths/test/parameters/2'])
314+
})
276315
})

0 commit comments

Comments
 (0)