add error handling in JSONDataHandler (#28)

CHANGELOG.md

@@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [[1.1.5](https://github.com/PerimeterX/marshmallow/compare/v1.1.4...v1.1.5)] - 2023-07-03
+
+### Added
+
+- Support for reporting errors from `HandleJSONData` - [info](https://github.com/PerimeterX/marshmallow/issues/27).
+
 ## [[1.1.4](https://github.com/PerimeterX/marshmallow/compare/v1.1.3...v1.1.4)] - 2022-11-10
 
 ### Fixed

README.md

@@ -6,7 +6,7 @@
 [![Run Tests](https://img.shields.io/github/actions/workflow/status/perimeterx/marshmallow/go.yml?branch=main&logo=github&label=Run%20Tests)](https://github.com/PerimeterX/marshmallow/actions/workflows/go.yml?query=branch%3Amain)
 [![Dependency Review](https://img.shields.io/github/actions/workflow/status/perimeterx/marshmallow/dependency-review.yml?logo=github&label=Dependency%20Review)](https://github.com/PerimeterX/marshmallow/actions/workflows/dependency-review.yml?query=branch%3Amain)
 [![Go Report Card](https://goreportcard.com/badge/github.com/perimeterx/marshmallow)](https://goreportcard.com/report/github.com/perimeterx/marshmallow)
-![Manual Code Coverage](https://img.shields.io/badge/coverage-92.4%25-green)
+![Manual Code Coverage](https://img.shields.io/badge/coverage-92.6%25-green)
 [![Go Reference](https://pkg.go.dev/badge/github.com/perimeterx/marshmallow.svg)](https://pkg.go.dev/github.com/perimeterx/marshmallow)
 [![Licence](https://img.shields.io/github/license/perimeterx/marshmallow)](LICENSE)
 [![Latest Release](https://img.shields.io/github/v/release/perimeterx/marshmallow)](https://github.com/PerimeterX/marshmallow/releases)
@@ -177,7 +177,7 @@ While unmarshalling, marshmallow supports the following optional options:
 * Excluding known fields from the result map using the [WithExcludeKnownFieldsFromMap](https://github.com/PerimeterX/marshmallow/blob/457669ae9973895584f2636eabfc104140d3b700/options.go#L50) function. 
 * Skipping struct population to boost performance using the [WithSkipPopulateStruct](https://github.com/PerimeterX/marshmallow/blob/0e0218ab860be8a4b5f57f5ff239f281c250c5da/options.go#L41) function.
 
-In order to capture unknown nested fields, structs must implement [JSONDataHandler](https://github.com/PerimeterX/marshmallow/blob/2d254bf2ed5f9b02cafb8ba6eaa726cba38bc92b/options.go#L65).
+In order to capture unknown nested fields, structs must implement [JSONDataErrorHandler](https://github.com/PerimeterX/marshmallow/blob/195c994aa6e3e0852601ad9cf65bcddef0dd7479/options.go#L76).
 More info [here](https://github.com/PerimeterX/marshmallow/issues/15). 
 
 Marshmallow also supports caching of refection information using 

example_test.go

@@ -248,6 +248,7 @@ type childStruct struct {
 	Data map[string]interface{} `json:"-"`
 }
 
-func (c *childStruct) HandleJSONData(data map[string]interface{}) {
+func (c *childStruct) HandleJSONData(data map[string]interface{}) error {
 	c.Data = data
+	return nil
 }

options.go

@@ -69,9 +69,28 @@ func buildUnmarshalOptions(options []UnmarshalOption) *unmarshalOptions {
 	return result
 }
 
-// JSONDataHandler allow types to handle JSON data as maps.
+// JSONDataErrorHandler allow types to handle JSON data as maps.
 // Types should implement this interface if they wish to act on the map representation of parsed JSON input.
 // This is mainly used to allow nested objects to capture unknown fields and leverage marshmallow's abilities.
+// If HandleJSONData returns an error, it will be propagated as an unmarshal error
+type JSONDataErrorHandler interface {
+	HandleJSONData(data map[string]interface{}) error
+}
+
+// Deprecated: use JSONDataErrorHandler instead
 type JSONDataHandler interface {
 	HandleJSONData(data map[string]interface{})
 }
+
+func asJSONDataHandler(value interface{}) (func(map[string]interface{}) error, bool) {
+	if handler, ok := value.(JSONDataErrorHandler); ok {
+		return handler.HandleJSONData, true
+	}
+	if handler, ok := value.(JSONDataHandler); ok {
+		return func(m map[string]interface{}) error {
+			handler.HandleJSONData(m)
+			return nil
+		}, true
+	}
+	return nil, false
+}

unmarshal.go

@@ -310,16 +310,21 @@ func (d *decoder) buildStruct(structType reflect.Type) (interface{}, bool) {
 		return d.lexer.Interface(), false
 	}
 	value := reflect.New(structType).Interface()
-	handler, ok := value.(JSONDataHandler)
+	handler, ok := asJSONDataHandler(value)
 	if !ok {
 		return d.populateStruct(true, value, nil)
 	}
 	data := make(map[string]interface{})
 	result, valid := d.populateStruct(true, value, data)
-	if valid {
-		handler.HandleJSONData(data)
+	if !valid {
+		return result, false
 	}
-	return result, valid
+	err := handler(data)
+	if err != nil {
+		d.lexer.AddNonFatalError(err)
+		return result, false
+	}
+	return result, true
 }
 
 func (d *decoder) valueFromCustomUnmarshaler(unmarshaler json.Unmarshaler) {

unmarshal_from_json_map.go

@@ -262,16 +262,21 @@ func (m *mapDecoder) buildStruct(path []string, v interface{}, structType reflec
 		return v, false
 	}
 	value := reflect.New(structType).Interface()
-	handler, ok := value.(JSONDataHandler)
+	handler, ok := asJSONDataHandler(value)
 	if !ok {
 		return m.populateStruct(true, path, mp, value, nil)
 	}
 	data := make(map[string]interface{})
 	result, valid := m.populateStruct(true, path, mp, value, data)
-	if valid {
-		handler.HandleJSONData(data)
+	if !valid {
+		return result, false
 	}
-	return result, valid
+	err := handler(data)
+	if err != nil {
+		m.addError(err)
+		return result, false
+	}
+	return result, true
 }
 
 func (m *mapDecoder) valueFromCustomUnmarshaler(data interface{}, unmarshaler UnmarshalerFromJSONMap) {

unmarshal_from_json_map_test.go

@@ -2279,7 +2279,11 @@ func TestUnmarshalFromJSONMapJSONDataHandler(t *testing.T) {
 		data := map[string]interface{}{
 			"known":   "foo",
 			"unknown": "boo",
-			"nested": map[string]interface{}{
+			"nested1": map[string]interface{}{
+				"known":   "goo",
+				"unknown": "doo",
+			},
+			"nested2": map[string]interface{}{
 				"known":   "goo",
 				"unknown": "doo",
 			},
@@ -2289,7 +2293,80 @@ func TestUnmarshalFromJSONMapJSONDataHandler(t *testing.T) {
 		if err != nil {
 			t.Errorf("unexpected error %v", err)
 		}
-		_, ok := result["nested"].(handleJSONDataChild)
+		_, ok := result["nested1"].(handleJSONDataChild)
+		if !ok {
+			t.Error("invalid map value")
+		}
+		if p.Nested1.Data == nil {
+			t.Error("HandleJSONData not called")
+		}
+		if len(p.Nested1.Data) != 2 || p.Nested1.Data["known"] != "goo" || p.Nested1.Data["unknown"] != "doo" {
+			t.Error("invalid JSON data")
+		}
+		_, ok = result["nested2"].(handleJSONDataChild)
+		if !ok {
+			t.Error("invalid map value")
+		}
+		if p.Nested2.Data == nil {
+			t.Error("HandleJSONData not called")
+		}
+		if len(p.Nested2.Data) != 2 || p.Nested2.Data["known"] != "goo" || p.Nested2.Data["unknown"] != "doo" {
+			t.Error("invalid JSON data")
+		}
+	})
+	t.Run("test_JSONDataHandler_single_error", func(t *testing.T) {
+		data := map[string]interface{}{
+			"known":   "foo",
+			"unknown": "boo",
+			"nested1": map[string]interface{}{"known": "goo", "unknown": "doo", "fail": true},
+			"nested2": map[string]interface{}{"known": "goo", "unknown": "doo", "fail": true},
+		}
+		p := &handleJSONDataParent{}
+		_, err := UnmarshalFromJSONMap(data, p)
+		if err == nil {
+			t.Errorf("expected JSONDataHandler error %v", err)
+		}
+		if err.Error() != "HandleJSONData failure" {
+			t.Errorf("unexpected JSONDataHandler error type %v", err)
+		}
+	})
+	t.Run("test_JSONDataHandler_multiple_error", func(t *testing.T) {
+		data := map[string]interface{}{
+			"known":   "foo",
+			"unknown": "boo",
+			"nested1": map[string]interface{}{"known": "goo", "unknown": "doo", "fail": true},
+			"nested2": map[string]interface{}{"known": "goo", "unknown": "doo", "fail": true},
+		}
+		p := &handleJSONDataParent{}
+		_, err := UnmarshalFromJSONMap(data, p, WithMode(ModeAllowMultipleErrors))
+		if err == nil {
+			t.Errorf("expected JSONDataHandler error %v", err)
+		}
+		e, ok := err.(*MultipleError)
+		if !ok {
+			t.Errorf("unexpected JSONDataHandler error type %v", err)
+		}
+		for _, currentError := range e.Errors {
+			if currentError.Error() != "HandleJSONData failure" {
+				t.Errorf("unexpected JSONDataHandler error type %v", err)
+			}
+		}
+	})
+	t.Run("test_JSONDataHandler_deprecated", func(t *testing.T) {
+		data := map[string]interface{}{
+			"known":   "foo",
+			"unknown": "boo",
+			"nested": map[string]interface{}{
+				"known":   "goo",
+				"unknown": "doo",
+			},
+		}
+		p := &handleJSONDataDeprecatedParent{}
+		result, err := UnmarshalFromJSONMap(data, p)
+		if err != nil {
+			t.Errorf("unexpected error %v", err)
+		}
+		_, ok := result["nested"].(handleJSONDataDeprecatedChild)
 		if !ok {
 			t.Error("invalid map value")
 		}

unmarshal_test.go

@@ -2306,8 +2306,9 @@ func TestEmbedding(t *testing.T) {
 }
 
 type handleJSONDataParent struct {
-	Known  string              `json:"known"`
-	Nested handleJSONDataChild `json:"nested"`
+	Known   string              `json:"known"`
+	Nested1 handleJSONDataChild `json:"nested1"`
+	Nested2 handleJSONDataChild `json:"nested2"`
 }
 
 type handleJSONDataChild struct {
@@ -2316,19 +2317,110 @@ type handleJSONDataChild struct {
 	Data map[string]interface{} `json:"-"`
 }
 
-func (c *handleJSONDataChild) HandleJSONData(data map[string]interface{}) {
+func (c *handleJSONDataChild) HandleJSONData(data map[string]interface{}) error {
+	if _, exists := data["fail"]; exists {
+		return errors.New("HandleJSONData failure")
+	}
+	c.Data = data
+	return nil
+}
+
+type handleJSONDataDeprecatedParent struct {
+	Known  string                        `json:"known"`
+	Nested handleJSONDataDeprecatedChild `json:"nested"`
+}
+
+type handleJSONDataDeprecatedChild struct {
+	Known string `json:"known"`
+
+	Data map[string]interface{} `json:"-"`
+}
+
+func (c *handleJSONDataDeprecatedChild) HandleJSONData(data map[string]interface{}) {
 	c.Data = data
 }
 
 func TestJSONDataHandler(t *testing.T) {
 	t.Run("test_JSONDataHandler", func(t *testing.T) {
-		data := []byte(`{"known": "foo","unknown": "boo","nested": {"known": "goo","unknown": "doo"}}`)
+		data := []byte(`{
+			"known": "foo",
+			"unknown":"boo",
+			"nested1": {"known": "goo","unknown": "doo"},
+			"nested2": {"known": "goo","unknown": "doo"}
+		}`)
+		p := &handleJSONDataParent{}
+		result, err := Unmarshal(data, p)
+		if err != nil {
+			t.Errorf("unexpected error %v", err)
+		}
+		_, ok := result["nested1"].(handleJSONDataChild)
+		if !ok {
+			t.Error("invalid map value")
+		}
+		if p.Nested1.Data == nil {
+			t.Error("Nested1 HandleJSONData not called")
+		}
+		if len(p.Nested1.Data) != 2 || p.Nested1.Data["known"] != "goo" || p.Nested1.Data["unknown"] != "doo" {
+			t.Error("Nested1 invalid JSON data")
+		}
+		_, ok = result["nested2"].(handleJSONDataChild)
+		if !ok {
+			t.Error("invalid map value")
+		}
+		if p.Nested2.Data == nil {
+			t.Error("Nested2 HandleJSONData not called")
+		}
+		if len(p.Nested2.Data) != 2 || p.Nested2.Data["known"] != "goo" || p.Nested2.Data["unknown"] != "doo" {
+			t.Error("Nested2 invalid JSON data")
+		}
+	})
+	t.Run("test_JSONDataHandler_single_error", func(t *testing.T) {
+		data := []byte(`{
+			"known": "foo",
+			"unknown":"boo",
+			"nested1": {"known": "goo","unknown": "doo", "fail": true},
+			"nested2": {"known": "goo","unknown": "doo", "fail": true}
+		}`)
 		p := &handleJSONDataParent{}
+		_, err := Unmarshal(data, p)
+		if err == nil {
+			t.Errorf("expected JSONDataHandler error %v", err)
+		}
+		e, ok := err.(*jlexer.LexerError)
+		if !ok || e.Reason != "HandleJSONData failure" {
+			t.Errorf("unexpected JSONDataHandler error type %v", err)
+		}
+	})
+	t.Run("test_JSONDataHandler_multiple_error", func(t *testing.T) {
+		data := []byte(`{
+			"known": "foo",
+			"unknown":"boo",
+			"nested1": {"known": "goo","unknown": "doo", "fail": true},
+			"nested2": {"known": "goo","unknown": "doo", "fail": true}
+		}`)
+		p := &handleJSONDataParent{}
+		_, err := Unmarshal(data, p, WithMode(ModeAllowMultipleErrors))
+		if err == nil {
+			t.Errorf("expected JSONDataHandler error %v", err)
+		}
+		e, ok := err.(*MultipleLexerError)
+		if !ok {
+			t.Errorf("unexpected JSONDataHandler error type %v", err)
+		}
+		for _, lexerError := range e.Errors {
+			if lexerError.Reason != "HandleJSONData failure" {
+				t.Errorf("unexpected JSONDataHandler error type %v", err)
+			}
+		}
+	})
+	t.Run("test_JSONDataHandler_deprecated", func(t *testing.T) {
+		data := []byte(`{"known": "foo","unknown": "boo","nested": {"known": "goo","unknown": "doo"}}`)
+		p := &handleJSONDataDeprecatedParent{}
 		result, err := Unmarshal(data, p)
 		if err != nil {
 			t.Errorf("unexpected error %v", err)
 		}
-		_, ok := result["nested"].(handleJSONDataChild)
+		_, ok := result["nested"].(handleJSONDataDeprecatedChild)
 		if !ok {
 			t.Error("invalid map value")
 		}
@@ -2477,7 +2569,9 @@ type nestedSkipPopulateChild struct {
 	Foo string `json:"foo"`
 }
 
-func (c *nestedSkipPopulateChild) HandleJSONData(map[string]interface{}) {}
+func (c *nestedSkipPopulateChild) HandleJSONData(map[string]interface{}) error {
+	return nil
+}
 
 var extraData = map[string]interface{}{
 	"extra1": "foo",
@@ -2496,7 +2590,9 @@ type failOverStruct struct {
 	C string `json:"c"`
 }
 
-func (f *failOverStruct) HandleJSONData(map[string]interface{}) {}
+func (f *failOverStruct) HandleJSONData(map[string]interface{}) error {
+	return nil
+}
 
 func buildParentStruct() *parentStruct {
 	return &parentStruct{