From ef9a89bfc4db1c59a1136e6f1f9a41f91a673030 Mon Sep 17 00:00:00 2001 From: Artem Voronin Date: Tue, 6 May 2025 21:19:34 -0700 Subject: [PATCH 1/2] Fix: deepObject return without assign on !required --- bindparam.go | 2 +- bindparam_test.go | 4 ++++ deepobject.go | 12 ++++++++++++ 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/bindparam.go b/bindparam.go index 2daf378..a92a0bd 100644 --- a/bindparam.go +++ b/bindparam.go @@ -469,7 +469,7 @@ func BindQueryParameter(style string, explode bool, required bool, paramName str if !explode { return errors.New("deepObjects must be exploded") } - return UnmarshalDeepObject(dest, paramName, queryParams) + return unmarshalDeepObject(dest, paramName, queryParams, required) case "spaceDelimited", "pipeDelimited": return fmt.Errorf("query arguments of style '%s' aren't yet supported", style) default: diff --git a/bindparam_test.go b/bindparam_test.go index 42da394..363c27a 100644 --- a/bindparam_test.go +++ b/bindparam_test.go @@ -322,6 +322,10 @@ func TestBindQueryParameter(t *testing.T) { err := BindQueryParameter("deepObject", true, false, paramName, queryParams, &actual) assert.NoError(t, err) assert.Equal(t, expectedDeepObject, actual) + + // If we require values, we require errors when they're not present. + err = BindQueryParameter("deepObject", true, true, "notfound", queryParams, &actual) + assert.Error(t, err) }) t.Run("form", func(t *testing.T) { diff --git a/deepobject.go b/deepobject.go index 7ec2f02..d06bf53 100644 --- a/deepobject.go +++ b/deepobject.go @@ -124,6 +124,10 @@ func makeFieldOrValue(paths [][]string, values []string) fieldOrValue { } func UnmarshalDeepObject(dst interface{}, paramName string, params url.Values) error { + return unmarshalDeepObject(dst, paramName, params, false) +} + +func unmarshalDeepObject(dst interface{}, paramName string, params url.Values, required bool) error { // Params are all the query args, so we need those that look like // "paramName["... var fieldNames []string @@ -141,6 +145,14 @@ func UnmarshalDeepObject(dst interface{}, paramName string, params url.Values) e } } + if len(fieldNames) == 0 { + if required { + return fmt.Errorf("query parameter '%s' is required", paramName) + } else { + return nil + } + } + // Now, for each field, reconstruct its subscript path and value paths := make([][]string, len(fieldNames)) for i, path := range fieldNames { From 5da5711e5bff7e847759690945548f94ad2d52fa Mon Sep 17 00:00:00 2001 From: Marcin Romaszewicz Date: Wed, 18 Mar 2026 06:51:10 -0700 Subject: [PATCH 2/2] 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) --- bindparam_test.go | 66 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/bindparam_test.go b/bindparam_test.go index 8ed7eae..ef6b76e 100644 --- a/bindparam_test.go +++ b/bindparam_test.go @@ -434,6 +434,72 @@ func TestBindQueryParameter(t *testing.T) { assert.Error(t, err) }) + t.Run("deepObject/required-and-optional", func(t *testing.T) { + type SimpleObj struct { + Name string `json:"name"` + Age int `json:"age"` + } + + queryWithParam := url.Values{ + "obj[name]": {"Alice"}, + "obj[age]": {"30"}, + } + emptyQuery := url.Values{} + unrelatedQuery := url.Values{ + "other[name]": {"Bob"}, + } + + t.Run("optional/present binds successfully", func(t *testing.T) { + var dest SimpleObj + err := BindQueryParameter("deepObject", true, false, "obj", queryWithParam, &dest) + require.NoError(t, err) + assert.Equal(t, SimpleObj{Name: "Alice", Age: 30}, dest) + }) + + t.Run("optional/missing returns no error and does not modify dest", func(t *testing.T) { + var dest SimpleObj + err := BindQueryParameter("deepObject", true, false, "obj", emptyQuery, &dest) + require.NoError(t, err) + assert.Equal(t, SimpleObj{}, dest, "destination should remain zero-valued") + }) + + t.Run("optional/missing with unrelated params returns no error", func(t *testing.T) { + var dest SimpleObj + err := BindQueryParameter("deepObject", true, false, "obj", unrelatedQuery, &dest) + require.NoError(t, err) + assert.Equal(t, SimpleObj{}, dest, "destination should remain zero-valued") + }) + + t.Run("optional/missing preserves pre-populated dest", func(t *testing.T) { + dest := SimpleObj{Name: "PreExisting", Age: 99} + err := BindQueryParameter("deepObject", true, false, "obj", emptyQuery, &dest) + require.NoError(t, err) + assert.Equal(t, SimpleObj{Name: "PreExisting", Age: 99}, dest, + "destination should not be zeroed out when optional param is absent") + }) + + t.Run("required/present binds successfully", func(t *testing.T) { + var dest SimpleObj + err := BindQueryParameter("deepObject", true, true, "obj", queryWithParam, &dest) + require.NoError(t, err) + assert.Equal(t, SimpleObj{Name: "Alice", Age: 30}, dest) + }) + + t.Run("required/missing returns error", func(t *testing.T) { + var dest SimpleObj + err := BindQueryParameter("deepObject", true, true, "obj", emptyQuery, &dest) + require.Error(t, err) + assert.Contains(t, err.Error(), "required") + }) + + t.Run("required/missing with unrelated params returns error", func(t *testing.T) { + var dest SimpleObj + err := BindQueryParameter("deepObject", true, true, "obj", unrelatedQuery, &dest) + require.Error(t, err) + assert.Contains(t, err.Error(), "required") + }) + }) + t.Run("form", func(t *testing.T) { expected := &MockBinder{Time: time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC)} birthday := &MockBinder{}