diff --git a/index/spec_index_test.go b/index/spec_index_test.go index 2c26ac3b..7f5298f7 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 @@ -1976,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") +} 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 +}