Skip to content

Commit fd63554

Browse files
voro015mromaszewiczclaude
authored
Fix: Query param deepObject return without assign on !required (#68)
* Fix: deepObject return without assign on !required * Add tests for deepObject required/optional query parameter binding Exercise the required and optional code paths for deepObject-style query parameters to verify no unintended side effects from PR #68. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Marcin Romaszewicz <marcinr@gmail.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent c9909ff commit fd63554

File tree

3 files changed

+83
-1
lines changed

3 files changed

+83
-1
lines changed

bindparam.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -565,7 +565,7 @@ func BindQueryParameterWithOptions(style string, explode bool, required bool, pa
565565
if !explode {
566566
return errors.New("deepObjects must be exploded")
567567
}
568-
return UnmarshalDeepObject(dest, paramName, queryParams)
568+
return unmarshalDeepObject(dest, paramName, queryParams, required)
569569
case "spaceDelimited", "pipeDelimited":
570570
return fmt.Errorf("query arguments of style '%s' aren't yet supported", style)
571571
default:

bindparam_test.go

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -428,6 +428,76 @@ func TestBindQueryParameter(t *testing.T) {
428428
err := BindQueryParameter("deepObject", true, false, paramName, queryParams, &actual)
429429
assert.NoError(t, err)
430430
assert.Equal(t, expectedDeepObject, actual)
431+
432+
// If we require values, we require errors when they're not present.
433+
err = BindQueryParameter("deepObject", true, true, "notfound", queryParams, &actual)
434+
assert.Error(t, err)
435+
})
436+
437+
t.Run("deepObject/required-and-optional", func(t *testing.T) {
438+
type SimpleObj struct {
439+
Name string `json:"name"`
440+
Age int `json:"age"`
441+
}
442+
443+
queryWithParam := url.Values{
444+
"obj[name]": {"Alice"},
445+
"obj[age]": {"30"},
446+
}
447+
emptyQuery := url.Values{}
448+
unrelatedQuery := url.Values{
449+
"other[name]": {"Bob"},
450+
}
451+
452+
t.Run("optional/present binds successfully", func(t *testing.T) {
453+
var dest SimpleObj
454+
err := BindQueryParameter("deepObject", true, false, "obj", queryWithParam, &dest)
455+
require.NoError(t, err)
456+
assert.Equal(t, SimpleObj{Name: "Alice", Age: 30}, dest)
457+
})
458+
459+
t.Run("optional/missing returns no error and does not modify dest", func(t *testing.T) {
460+
var dest SimpleObj
461+
err := BindQueryParameter("deepObject", true, false, "obj", emptyQuery, &dest)
462+
require.NoError(t, err)
463+
assert.Equal(t, SimpleObj{}, dest, "destination should remain zero-valued")
464+
})
465+
466+
t.Run("optional/missing with unrelated params returns no error", func(t *testing.T) {
467+
var dest SimpleObj
468+
err := BindQueryParameter("deepObject", true, false, "obj", unrelatedQuery, &dest)
469+
require.NoError(t, err)
470+
assert.Equal(t, SimpleObj{}, dest, "destination should remain zero-valued")
471+
})
472+
473+
t.Run("optional/missing preserves pre-populated dest", func(t *testing.T) {
474+
dest := SimpleObj{Name: "PreExisting", Age: 99}
475+
err := BindQueryParameter("deepObject", true, false, "obj", emptyQuery, &dest)
476+
require.NoError(t, err)
477+
assert.Equal(t, SimpleObj{Name: "PreExisting", Age: 99}, dest,
478+
"destination should not be zeroed out when optional param is absent")
479+
})
480+
481+
t.Run("required/present binds successfully", func(t *testing.T) {
482+
var dest SimpleObj
483+
err := BindQueryParameter("deepObject", true, true, "obj", queryWithParam, &dest)
484+
require.NoError(t, err)
485+
assert.Equal(t, SimpleObj{Name: "Alice", Age: 30}, dest)
486+
})
487+
488+
t.Run("required/missing returns error", func(t *testing.T) {
489+
var dest SimpleObj
490+
err := BindQueryParameter("deepObject", true, true, "obj", emptyQuery, &dest)
491+
require.Error(t, err)
492+
assert.Contains(t, err.Error(), "required")
493+
})
494+
495+
t.Run("required/missing with unrelated params returns error", func(t *testing.T) {
496+
var dest SimpleObj
497+
err := BindQueryParameter("deepObject", true, true, "obj", unrelatedQuery, &dest)
498+
require.Error(t, err)
499+
assert.Contains(t, err.Error(), "required")
500+
})
431501
})
432502

433503
t.Run("form", func(t *testing.T) {

deepobject.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,10 @@ func makeFieldOrValue(paths [][]string, values []string) fieldOrValue {
127127
}
128128

129129
func UnmarshalDeepObject(dst interface{}, paramName string, params url.Values) error {
130+
return unmarshalDeepObject(dst, paramName, params, false)
131+
}
132+
133+
func unmarshalDeepObject(dst interface{}, paramName string, params url.Values, required bool) error {
130134
// Params are all the query args, so we need those that look like
131135
// "paramName["...
132136
var fieldNames []string
@@ -149,6 +153,14 @@ func UnmarshalDeepObject(dst interface{}, paramName string, params url.Values) e
149153
}
150154
}
151155

156+
if len(fieldNames) == 0 {
157+
if required {
158+
return fmt.Errorf("query parameter '%s' is required", paramName)
159+
} else {
160+
return nil
161+
}
162+
}
163+
152164
// Now, for each field, reconstruct its subscript path and value
153165
paths := make([][]string, len(fieldNames))
154166
for i, path := range fieldNames {

0 commit comments

Comments
 (0)