Skip to content

Commit 1650807

Browse files
mromaszewiczjuniorrantilaclaude
authored
fix: handle duplicate path parameters in OpenAPI specs (oapi-codegen#2220)
Some real-world OpenAPI specs (e.g. Keycloak) reuse the same path parameter more than once in a single URI, such as: /clients/{client-uuid}/roles/{role-name}/composites/clients/{client-uuid} Previously this caused a "has 4 positional parameters, but spec has 3 declared" error because SortParamsByPath compared raw URI placeholder count against unique declared parameters. Fix this by deduplicating the path parameters extracted from the URI (preserving first-occurrence order) before matching them against the spec declared parameters. This is the right level to fix the issue rather than scattering dedup logic across template helpers. Fixes oapi-codegen#1574 Supersedes oapi-codegen#2175 Co-authored-by: Junior Rantila <junior.rantila@gmail.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 01d4fc0 commit 1650807

File tree

3 files changed

+125
-4
lines changed

3 files changed

+125
-4
lines changed

pkg/codegen/codegen_test.go

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,5 +227,65 @@ func (t *ExampleSchema_Item) FromExternalRef0NewPet(v externalRef0.NewPet) error
227227
`)
228228
}
229229

230+
func TestDuplicatePathParameter(t *testing.T) {
231+
// Regression test for https://github.com/oapi-codegen/oapi-codegen/issues/1574
232+
// Some real-world specs (e.g. Keycloak) have paths where the same parameter
233+
// appears more than once: /clients/{client-uuid}/.../clients/{client-uuid}
234+
spec := `
235+
openapi: "3.0.0"
236+
info:
237+
version: 1.0.0
238+
title: Duplicate path param test
239+
paths:
240+
/admin/realms/{realm}/clients/{client-uuid}/roles/{role-name}/composites/clients/{client-uuid}:
241+
get:
242+
operationId: getCompositeRoles
243+
parameters:
244+
- name: realm
245+
in: path
246+
required: true
247+
schema:
248+
type: string
249+
- name: client-uuid
250+
in: path
251+
required: true
252+
schema:
253+
type: string
254+
- name: role-name
255+
in: path
256+
required: true
257+
schema:
258+
type: string
259+
responses:
260+
'200':
261+
description: Success
262+
`
263+
loader := openapi3.NewLoader()
264+
swagger, err := loader.LoadFromData([]byte(spec))
265+
require.NoError(t, err)
266+
267+
opts := Configuration{
268+
PackageName: "api",
269+
Generate: GenerateOptions{
270+
EchoServer: true,
271+
Client: true,
272+
Models: true,
273+
},
274+
}
275+
276+
code, err := Generate(swagger, opts)
277+
require.NoError(t, err)
278+
assert.NotEmpty(t, code)
279+
280+
// Verify the generated code is valid Go.
281+
_, err = format.Source([]byte(code))
282+
require.NoError(t, err)
283+
284+
// The path params should appear exactly once in the function signature.
285+
assert.Contains(t, code, "realm string")
286+
assert.Contains(t, code, "clientUuid string")
287+
assert.Contains(t, code, "roleName string")
288+
}
289+
230290
//go:embed test_spec.yaml
231291
var testOpenAPIDefinition string

pkg/codegen/utils.go

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -654,15 +654,29 @@ func ReplacePathParamsWithStr(uri string) string {
654654
}
655655

656656
// SortParamsByPath reorders the given parameter definitions to match those in the path URI.
657+
// If a parameter appears more than once in the path (e.g. Keycloak's
658+
// /clients/{client-uuid}/roles/{role-name}/composites/clients/{client-uuid}),
659+
// duplicates are removed and only the first occurrence determines the order.
657660
func SortParamsByPath(path string, in []ParameterDefinition) ([]ParameterDefinition, error) {
658661
pathParams := OrderedParamsFromUri(path)
662+
663+
// Deduplicate, preserving first-occurrence order.
664+
seen := make(map[string]struct{}, len(pathParams))
665+
uniqueParams := make([]string, 0, len(pathParams))
666+
for _, name := range pathParams {
667+
if _, exists := seen[name]; !exists {
668+
seen[name] = struct{}{}
669+
uniqueParams = append(uniqueParams, name)
670+
}
671+
}
672+
659673
n := len(in)
660-
if len(pathParams) != n {
674+
if len(uniqueParams) != n {
661675
return nil, fmt.Errorf("path '%s' has %d positional parameters, but spec has %d declared",
662-
path, len(pathParams), n)
676+
path, len(uniqueParams), n)
663677
}
664-
out := make([]ParameterDefinition, len(in))
665-
for i, name := range pathParams {
678+
out := make([]ParameterDefinition, n)
679+
for i, name := range uniqueParams {
666680
p := ParameterDefinitions(in).FindByName(name)
667681
if p == nil {
668682
return nil, fmt.Errorf("path '%s' refers to parameter '%s', which doesn't exist in specification",

pkg/codegen/utils_test.go

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -462,6 +462,53 @@ func TestOrderedParamsFromUri(t *testing.T) {
462462

463463
result = OrderedParamsFromUri("/path/foo")
464464
assert.EqualValues(t, []string{}, result)
465+
466+
// A parameter can appear more than once in the URI (e.g. Keycloak API).
467+
// OrderedParamsFromUri faithfully returns all occurrences.
468+
result = OrderedParamsFromUri("/admin/realms/{realm}/clients/{client-uuid}/roles/{role-name}/composites/clients/{client-uuid}")
469+
assert.EqualValues(t, []string{"realm", "client-uuid", "role-name", "client-uuid"}, result)
470+
}
471+
472+
func TestSortParamsByPath(t *testing.T) {
473+
strSchema := &openapi3.Schema{Type: &openapi3.Types{"string"}}
474+
475+
t.Run("reorders params to match path order", func(t *testing.T) {
476+
params := []ParameterDefinition{
477+
{ParamName: "b", In: "path", Spec: &openapi3.Parameter{Name: "b", Schema: &openapi3.SchemaRef{Value: strSchema}}},
478+
{ParamName: "a", In: "path", Spec: &openapi3.Parameter{Name: "a", Schema: &openapi3.SchemaRef{Value: strSchema}}},
479+
}
480+
sorted, err := SortParamsByPath("/foo/{a}/bar/{b}", params)
481+
require.NoError(t, err)
482+
require.Len(t, sorted, 2)
483+
assert.Equal(t, "a", sorted[0].ParamName)
484+
assert.Equal(t, "b", sorted[1].ParamName)
485+
})
486+
487+
t.Run("errors on missing parameter", func(t *testing.T) {
488+
params := []ParameterDefinition{
489+
{ParamName: "a", In: "path", Spec: &openapi3.Parameter{Name: "a", Schema: &openapi3.SchemaRef{Value: strSchema}}},
490+
}
491+
_, err := SortParamsByPath("/foo/{a}/bar/{b}", params)
492+
assert.Error(t, err)
493+
})
494+
495+
t.Run("handles duplicate path parameters", func(t *testing.T) {
496+
// This is the Keycloak-style path where {client-uuid} appears twice.
497+
// The spec only declares 3 unique parameters.
498+
params := []ParameterDefinition{
499+
{ParamName: "realm", In: "path", Spec: &openapi3.Parameter{Name: "realm", Schema: &openapi3.SchemaRef{Value: strSchema}}},
500+
{ParamName: "client-uuid", In: "path", Spec: &openapi3.Parameter{Name: "client-uuid", Schema: &openapi3.SchemaRef{Value: strSchema}}},
501+
{ParamName: "role-name", In: "path", Spec: &openapi3.Parameter{Name: "role-name", Schema: &openapi3.SchemaRef{Value: strSchema}}},
502+
}
503+
path := "/admin/realms/{realm}/clients/{client-uuid}/roles/{role-name}/composites/clients/{client-uuid}"
504+
sorted, err := SortParamsByPath(path, params)
505+
require.NoError(t, err)
506+
// Should return 3 unique params in first-occurrence order
507+
require.Len(t, sorted, 3)
508+
assert.Equal(t, "realm", sorted[0].ParamName)
509+
assert.Equal(t, "client-uuid", sorted[1].ParamName)
510+
assert.Equal(t, "role-name", sorted[2].ParamName)
511+
})
465512
}
466513

467514
func TestReplacePathParamsWithStr(t *testing.T) {

0 commit comments

Comments
 (0)