From 76053b1c6325086cd2aa7789dc6088c4c4ea0288 Mon Sep 17 00:00:00 2001 From: Calin Gilan Date: Mon, 25 Aug 2025 12:42:46 -0400 Subject: [PATCH 1/2] Fix parameter counting inconsistency in GetAllParametersFromOperations Resolves issue where parameters with same name but different 'in' types were counted inconsistently based on their order in the specification. The bug caused duplicate parameter entries when checking existing parameters. - Extract duplicate detection logic to hasDuplicateInType helper function - Fix loop logic that was adding parameters multiple times - Add test case to verify consistent parameter counting regardless of order --- index/spec_index_test.go | 79 ++++++++++++++++++++++++++++++++++++++++ index/utility_methods.go | 42 +++++++++++---------- 2 files changed, 102 insertions(+), 19 deletions(-) diff --git a/index/spec_index_test.go b/index/spec_index_test.go index 2c26ac3b..5428a002 100644 --- a/index/spec_index_test.go +++ b/index/spec_index_test.go @@ -731,6 +731,85 @@ paths: assert.Equal(t, 1, len(index.GetOperationParametersIndexErrors())) } +func TestSpecIndex_ParametersWithSameNameDifferentIn(t *testing.T) { + spec1 := `openapi: 3.1.0 +info: + title: Test API + version: 1.0.0 +paths: + /test: + get: + parameters: + - name: random_id + in: path + required: true + schema: + type: string + - name: random_id + in: query + required: false + schema: + type: integer + - name: random_id + in: header + required: false + schema: + type: boolean` + + spec2 := `openapi: 3.1.0 +info: + title: Test API + version: 1.0.0 +paths: + /test: + get: + parameters: + - name: random_id + in: query + required: false + schema: + type: integer + - name: random_id + in: path + required: true + schema: + type: string + - name: random_id + in: header + required: false + schema: + type: boolean` + + var rootNode1 yaml.Node + _ = yaml.Unmarshal([]byte(spec1), &rootNode1) + index1 := NewSpecIndexWithConfig(&rootNode1, CreateOpenAPIIndexConfig()) + params1 := index1.GetAllParametersFromOperations() + + var rootNode2 yaml.Node + _ = yaml.Unmarshal([]byte(spec2), &rootNode2) + index2 := NewSpecIndexWithConfig(&rootNode2, CreateOpenAPIIndexConfig()) + params2 := index2.GetAllParametersFromOperations() + + count1 := countParametersFromOperations(params1) + count2 := countParametersFromOperations(params2) + + assert.Equal(t, 3, count1) + assert.Equal(t, 3, count2) + assert.Equal(t, count1, count2) +} + +func countParametersFromOperations(params map[string]map[string]map[string][]*Reference) int { + total := 0 + for _, pathParams := range params { + for _, methodParams := range pathParams { + for _, refs := range methodParams { + total += len(refs) + } + } + } + return total +} + func TestSpecIndex_BurgerShop_AllTheComponents(t *testing.T) { burgershop, _ := os.ReadFile("../test_specs/all-the-components.yaml") var rootNode yaml.Node diff --git a/index/utility_methods.go b/index/utility_methods.go index 6201962b..774b5acf 100644 --- a/index/utility_methods.go +++ b/index/utility_methods.go @@ -510,26 +510,20 @@ func (index *SpecIndex) scanOperationParams(params []*yaml.Node, keyNode, pathIt checkNodes := index.paramOpRefs[pathItemNode.Value][method][ref.Name] _, currentIn := utils.FindKeyNodeTop("in", currentNode.Content) - for _, checkNode := range checkNodes { - - _, checkIn := utils.FindKeyNodeTop("in", checkNode.Node.Content) - - if currentIn != nil && checkIn != nil && currentIn.Value == checkIn.Value { - - path := fmt.Sprintf("$.paths['%s'].%s.parameters[%d]", pathItemNode.Value, method, i) - if method == "top" { - path = fmt.Sprintf("$.paths['%s'].parameters[%d]", pathItemNode.Value, i) - } - - index.operationParamErrors = append(index.operationParamErrors, &IndexingError{ - Err: fmt.Errorf("the `%s` operation parameter at path `%s`, "+ - "index %d has a duplicate name `%s` and `in` type", strings.ToUpper(method), pathItemNode.Value, i, vn.Value), - Node: param, - Path: path, - }) - } else { - index.paramOpRefs[pathItemNode.Value][method][ref.Name] = append(index.paramOpRefs[pathItemNode.Value][method][ref.Name], ref) + if hasDuplicateInType(currentIn, checkNodes) { + path := fmt.Sprintf("$.paths['%s'].%s.parameters[%d]", pathItemNode.Value, method, i) + if method == "top" { + path = fmt.Sprintf("$.paths['%s'].parameters[%d]", pathItemNode.Value, i) } + + index.operationParamErrors = append(index.operationParamErrors, &IndexingError{ + Err: fmt.Errorf("the `%s` operation parameter at path `%s`, "+ + "index %d has a duplicate name `%s` and `in` type", strings.ToUpper(method), pathItemNode.Value, i, vn.Value), + Node: param, + Path: path, + }) + } else { + index.paramOpRefs[pathItemNode.Value][method][ref.Name] = append(index.paramOpRefs[pathItemNode.Value][method][ref.Name], ref) } } else { index.paramOpRefs[pathItemNode.Value][method][ref.Name] = append(index.paramOpRefs[pathItemNode.Value][method][ref.Name], ref) @@ -813,3 +807,13 @@ func hashNodeSimple(n *yaml.Node, h hash.Hash, depth int) { } } } + +func hasDuplicateInType(currentIn *yaml.Node, existingRefs []*Reference) bool { + for _, ref := range existingRefs { + _, existingIn := utils.FindKeyNodeTop("in", ref.Node.Content) + if currentIn != nil && existingIn != nil && currentIn.Value == existingIn.Value { + return true + } + } + return false +} From fc0c19abdc1638da5b721e4aea02d95bf28f61d3 Mon Sep 17 00:00:00 2001 From: Calin Gilan Date: Wed, 27 Aug 2025 14:16:48 -0400 Subject: [PATCH 2/2] Fix Test --- index/spec_index_test.go | 83 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/index/spec_index_test.go b/index/spec_index_test.go index 5428a002..7f5298f7 100644 --- a/index/spec_index_test.go +++ b/index/spec_index_test.go @@ -2055,3 +2055,86 @@ components: assert.NotNil(t, index.GetRolodex()) } + +func TestSpecIndex_DuplicateParameterHandling(t *testing.T) { + // Test duplicate parameter detection: two query params + one path param with same name + // Should return 2 parameters (1 path + 1 query) and record 1 duplicate error + spec1 := `openapi: 3.1.0 +info: + title: Test API + version: 1.0.0 +paths: + /items/{random_id}: + get: + parameters: + - name: random_id + in: path + required: true + schema: + type: boolean + - name: random_id + in: query + required: true + schema: + type: integer + - name: random_id + in: query + required: true + schema: + type: boolean` + + // Test with different parameter order to ensure consistency + spec2 := `openapi: 3.1.0 +info: + title: Test API + version: 1.0.0 +paths: + /items/{random_id}: + get: + parameters: + - name: random_id + in: query + required: true + schema: + type: integer + - name: random_id + in: query + required: true + schema: + type: boolean + - name: random_id + in: path + required: true + schema: + type: boolean` + + var rootNode1 yaml.Node + _ = yaml.Unmarshal([]byte(spec1), &rootNode1) + index1 := NewSpecIndexWithConfig(&rootNode1, CreateOpenAPIIndexConfig()) + params1 := index1.GetAllParametersFromOperations() + errors1 := index1.GetOperationParametersIndexErrors() + + var rootNode2 yaml.Node + _ = yaml.Unmarshal([]byte(spec2), &rootNode2) + index2 := NewSpecIndexWithConfig(&rootNode2, CreateOpenAPIIndexConfig()) + params2 := index2.GetAllParametersFromOperations() + errors2 := index2.GetOperationParametersIndexErrors() + + count1 := countParametersFromOperations(params1) + count2 := countParametersFromOperations(params2) + + // Should have 2 parameters total (1 path + 1 query, second query rejected as duplicate) + assert.Equal(t, 2, count1, "Spec1 should have 2 parameters (1 path + 1 query), not 3") + assert.Equal(t, 2, count2, "Spec2 should have 2 parameters (1 path + 1 query), not 3") + assert.Equal(t, count1, count2, "Both specs should return same parameter count regardless of order") + + // Should have exactly 1 duplicate error for the second query parameter + assert.Equal(t, 1, len(errors1), "Spec1 should have 1 duplicate parameter error") + assert.Equal(t, 1, len(errors2), "Spec2 should have 1 duplicate parameter error") + + // Verify the error message mentions duplicate name and in type + assert.Contains(t, errors1[0].Error(), "duplicate name") + assert.Contains(t, errors1[0].Error(), "random_id") + assert.Contains(t, errors2[0].Error(), "duplicate name") + assert.Contains(t, errors2[0].Error(), "random_id") +}