fix: remove duplicated parameters during unification

src/errors.ts

@@ -8,4 +8,5 @@ export const ErrorMessage = {
   referenceNotAllowed: (ref: string) => `${JSON_SCHEMA_PROPERTY_REF} not allowed here: ${ref}`,
   refNotFound: (ref: string) => `${JSON_SCHEMA_PROPERTY_REF} can't be resolved: ${ref}`,
   refNotValidFormat: (ref: string) => `${JSON_SCHEMA_PROPERTY_REF} can't be parsed: ${ref}`,
+  duplicateParameter: (name: string, location: string) => `Duplicate parameter detected: name='${name}', in='${location}'`,
 } as const

src/origins.ts

@@ -100,6 +100,111 @@ export function concatenateArraysInProperty(source: Jso, target: Jso, propertyKe
   })
 }
 
+export function removeDuplicates<T>(
+  array: T[],
+  getUniqueKey: (item: T) => string | undefined,
+  onDuplicate?: (item: T, index: number, firstIndex: number) => void
+): T[] {
+  const seen = new Map<string, number>() // key -> first index
+
+  for (let i = 0; i < array.length; i++) {
+    const item = array[i]
+    const key = getUniqueKey(item)
+
+    if (key === undefined) {
+      continue // Skip items without valid keys
+    }
+
+    const firstIndex = seen.get(key)
+    if (firstIndex !== undefined) {
+      // Duplicate found
+      onDuplicate?.(item, i, firstIndex)
+      delete array[i] // Create sparse array
+    } else {
+      seen.set(key, i)
+    }
+  }
+
+  return array // Returns sparse array
+}
+
+export function densify<T>(sparseArray: T[], originsFlag: symbol | undefined): T[] {
+  const dense: T[] = []
+  const originsRecord: OriginsMetaRecord = {}
+
+  for (let i = 0; i < sparseArray.length; i++) {
+    if (i in sparseArray) { // Check if index exists (not deleted)
+      const newIndex = dense.length
+      dense[newIndex] = sparseArray[i]
+
+      // Copy origins if they exist
+      if (originsFlag) {
+        const sourceOrigins = resolveOrigins(sparseArray, i, originsFlag)
+        if (sourceOrigins) {
+          originsRecord[newIndex] = sourceOrigins
+        }
+      }
+    }
+  }
+
+  // Set origins for the dense array
+  if (originsFlag && Object.keys(originsRecord).length > 0) {
+    setJsoProperty(dense as any, originsFlag, originsRecord)
+  }
+
+  return dense
+}
+
+export function addUniqueElements(
+  source: Jso,
+  target: Jso,
+  propertyKey: PropertyKey,
+  originsFlag: symbol | undefined,
+  getUniqueKey: (item: any) => string | undefined
+): void {
+  const sourceArray = getJsoProperty(source, propertyKey)
+  if (!isArray(sourceArray)) {
+    return
+  }
+
+  let targetArray = getJsoProperty(target, propertyKey) as unknown[]
+  if (targetArray === undefined) {
+    targetArray = []
+    copyOrigins(source, target, propertyKey, propertyKey, originsFlag)
+  }
+
+  if (!isArray(targetArray)) {
+    return
+  }
+
+  setJsoProperty(target, propertyKey, targetArray)
+
+  // Build set of existing keys in target
+  const existingKeys = new Set<string>()
+  for (const item of targetArray) {
+    const key = getUniqueKey(item)
+    if (key !== undefined) {
+      existingKeys.add(key)
+    }
+  }
+
+  // Add source items that don't exist in target
+  for (let i = 0; i < sourceArray.length; i++) {
+    const item = sourceArray[i]
+    const key = getUniqueKey(item)
+
+    // Skip if already exists in target (operation overrides path) or key is undefined
+    if (key === undefined || existingKeys.has(key)) {
+      continue
+    }
+
+    // Add to target
+    const targetIndex = targetArray.length
+    targetArray[targetIndex] = item
+    copyOrigins(sourceArray, targetArray, i, targetIndex, originsFlag)
+  }
+}
+
 export function resolveOriginsMetaRecord(source: Jso, originsFlag: symbol | undefined): OriginsMetaRecord | undefined {
   if (!originsFlag) {
     return undefined
@@ -214,4 +319,4 @@ export function copyOriginsForArray(sourceArray: unknown[], targetArray: unknown
 
 export const createSelfOriginsCloneHook: <T extends HasSelfOriginsResolver, R extends {}>(originsFlag: symbol | undefined) => SyncCloneHook<T, R> = <State extends HasSelfOriginsResolver, Rules extends {}>(originsFlag: symbol | undefined) => {
   return createSelfMetaCloneHook<OriginLeafs, 'selfOriginResolver', State, Rules>('selfOriginResolver', originsFlag, [])
-}
+}

src/rules/openapi.ts

@@ -76,7 +76,7 @@ import {
   OPEN_API_PROPERTY_TAGS,
 } from './openapi.const'
 
-import { pathItemsUnification } from '../unifies/openapi'
+import { pathItemsUnification, deduplicateParameters } from '../unifies/openapi'
 import {
   calculateHeaderName,
   calculateHeaderPlace,
@@ -626,6 +626,7 @@ const openApiParametersRules = (version: OpenApiSpecVersion): NormalizationRules
     hashOwner: true,
   },
   validate: checkType(TYPE_ARRAY),
+  unify: deduplicateParameters,
 })
 
 const openApiRequestRules = (version: OpenApiSpecVersion): NormalizationRules => ({

src/unifies/openapi.ts

@@ -1,14 +1,52 @@
 import { UnifyFunction } from '../types'
-import { isObject } from '@netcracker/qubership-apihub-json-crawl'
+import { isArray, isObject } from '@netcracker/qubership-apihub-json-crawl'
 import { OpenAPIV3 } from 'openapi-types'
 import { OPEN_API_HTTP_METHODS } from '../rules/openapi.const'
-import { cleanSeveralOrigins, concatenateArraysInProperty, copyProperty } from '../origins'
+import {
+  addUniqueElements,
+  cleanSeveralOrigins,
+  concatenateArraysInProperty,
+  copyProperty,
+  densify,
+  removeDuplicates,
+} from '../origins'
+import { ErrorMessage } from '../errors'
 import PathItemObject = OpenAPIV3.PathItemObject
 
 const EXTENSION_PREFIX = 'x-'
 
+const getParameterUniqueKey = (param: any): string | undefined => {
+  if (!param || typeof param !== 'object') {
+    return undefined
+  }
+  const name = param.name
+  const inValue = param.in
+  if (name !== undefined && inValue !== undefined) {
+    return `${name}:${inValue}`
+  }
+  return undefined
+}
+
+export const deduplicateParameters: UnifyFunction = (jso, ctx) => {
+  if (!isArray(jso)) {
+    return jso
+  }
+
+  const sparseArray = removeDuplicates(
+    jso,
+    getParameterUniqueKey,
+    (item, index, firstIndex) => {
+      // Report duplicate error
+      const message = ErrorMessage.duplicateParameter(item?.name, item?.in)
+      ctx.options.onUnifyError?.(message, ctx.path, jso, new Error(message))
+    }
+  )
+
+  return densify(sparseArray, ctx.options.originsFlag)
+}
+
 export const pathItemsUnification: UnifyFunction = (value, { options }) => {
-  if (!isObject(value)) { return value}
+  if (!isObject(value)) { return value }
   const pathItem: PathItemObject = value
   const {
     parameters: pathParameters,
@@ -41,7 +79,7 @@ export const pathItemsUnification: UnifyFunction = (value, { options }) => {
     }
     result[method] = operation //origin copy already.
     // Deep copy not allowed!!! So only mutations :(
-    concatenateArraysInProperty(pathItem, operation, 'parameters', options.originsFlag)
+    addUniqueElements(pathItem, operation, 'parameters', options.originsFlag, getParameterUniqueKey)
     concatenateArraysInProperty(pathItem, operation, 'servers', options.originsFlag)
     if (operation.summary === undefined && pathSummary !== undefined) {
       copyProperty(pathItem, operation, 'summary', options.originsFlag)

src/utils.ts

@@ -383,4 +383,4 @@ export const removeDuplicatesWithMergeOrigins = <T>(array: T[], originFlag: symb
   setOriginsForArray(uniqueItems, originFlag, itemOrigins)
 
   return uniqueItems
-}
+}

test/oas/bugs.test.ts

@@ -2,7 +2,7 @@ import { TEST_DEFAULTS_FLAG, TEST_HASH_FLAG, TEST_SYNTHETIC_ALL_OF_FLAG } from '
 import { denormalize, JSON_SCHEMA_PROPERTY_DEPRECATED, normalize, OriginLeafs, resolveOrigins } from '../../src'
 import outOfMemoryCauseTooManyCombinations from '../resources/out-of-memory-cause-too-many-combinations.json'
 import bugWithWrongOrigins from '../resources/bug-with-wrong-origins.json'
-import bugWithSpearedArray from '../resources/bug-with-speared-array.json'
+import bugWithSparseArray from '../resources/bug-with-sparse-array.json'
 import { TEST_INLINE_REFS_FLAG, TEST_ORIGINS_FLAG, TEST_ORIGINS_FOR_DEFAULTS, TEST_SYNTHETIC_TITLE_FLAG } from '../helpers'
 import { isObject, syncCrawl } from '@netcracker/qubership-apihub-json-crawl'
 import 'jest-extended'
@@ -239,8 +239,8 @@ describe('Bugs', () => {
     expect(result.paths['customer'].get.parameters[0].schema).not.toHaveProperty([TEST_INLINE_REFS_FLAG])
   })
 
-  it('speared array', () => {
-    const result = normalize(bugWithSpearedArray, {
+  it('sparse array', () => {
+    const result = normalize(bugWithSparseArray, {
       validate: true,
       liftCombiners: true,
       unify: true,
@@ -250,6 +250,10 @@ describe('Bugs', () => {
       hashFlag: TEST_HASH_FLAG,
       inlineRefsFlag: TEST_INLINE_REFS_FLAG,
     }) as any
-    expect(result).toHaveProperty(['paths', '/datasets/events', 'get', 'parameters', 'length'], 10) /*1 and 3 index will be missing*/
+
+    expect(result).toHaveProperty(['paths', '/datasets/events', 'get', 'parameters', 'length'], 8)
+    // Verify all parameters are valid objects
+    const params = result.paths['/datasets/events'].get.parameters
+    expect(params.every((p: any) => p && typeof p === 'object')).toBe(true)
   })
 })

test/oas/unify.origins.test.ts

@@ -273,4 +273,43 @@ describe('unify origins', function () {
     })
 
   })
+
+  it('should preserve origins when unifying path item and deduplicating operation parameters', () => {
+    const spec = {
+      openapi: '3.0.0',
+      info: { title: 'Test API', version: '1.0.0' },
+      paths: {
+        'test': {
+          parameters: [
+            { name: 'id', in: 'query', schema: { type: 'string' } },
+            { name: 'id', in: 'query', schema: { type: 'string' } }, // duplicate
+            { name: 'limit', in: 'query', schema: { type: 'integer' } },
+          ],
+          get: {
+            parameters: [
+              { name: 'offset', in: 'query', schema: { type: 'string' } },
+              { name: 'offset', in: 'query', schema: { type: 'string' } }, // duplicate
+              { name: 'maxItems', in: 'query', schema: { type: 'integer' } },
+            ],
+            responses: {
+              '200': { description: 'OK' },
+            },
+          },
+        },
+      },
+    }
+
+    const result = normalize(spec, OPTIONS) as any
+
+    // Should have 2 parameters after deduplication
+    const params = result.paths?.['test']?.get?.parameters
+    expect(params).toHaveLength(4)
+
+    commonOriginsCheck(result, { source: spec })
+    const resultWithHmr = convertOriginToHumanReadable(result, TEST_ORIGINS_FLAG)
+    expect(resultWithHmr).toHaveProperty(['paths', 'test', 'get', 'parameters', TEST_ORIGINS_FLAG, 0], ['paths/test/get/parameters/0'])
+    expect(resultWithHmr).toHaveProperty(['paths', 'test', 'get', 'parameters', TEST_ORIGINS_FLAG, 1], ['paths/test/get/parameters/2'])
+    expect(resultWithHmr).toHaveProperty(['paths', 'test', 'get', 'parameters', TEST_ORIGINS_FLAG, 2], ['paths/test/parameters/0'])
+    expect(resultWithHmr).toHaveProperty(['paths', 'test', 'get', 'parameters', TEST_ORIGINS_FLAG, 3], ['paths/test/parameters/2'])
+  })
 })