Fix unmarshalling of registered types

cpuguy83 · cpuguy83 · commit a044d5393ea7 · 2024-11-06T13:40:42.000-08:00
This is a faithful backport of #53 --- In 7d3d258 I inadvterantly removed support for json unmarshalling for the case when a type implements a protobuf message but is a type that is registered. For types that are registered through the `Register` function typeurl is supposed to ignore the interfaces that the type implements and just use json. This change restores that behavior. Signed-off-by: Brian Goff <cpuguy83@gmail.com>
diff --git a/types.go b/types.go
@@ -39,7 +39,7 @@ type handler interface {
 	Marshaller(interface{}) func() ([]byte, error)
 	Unmarshaller(interface{}) func([]byte) error
 	TypeURL(interface{}) string
-	GetType(url string) reflect.Type
+	GetType(url string) (reflect.Type, bool)
 }
 
 // Definitions of common error types used throughout typeurl.
@@ -240,7 +240,7 @@ func MarshalAnyToProto(from interface{}) (*anypb.Any, error) {
 }
 
 func unmarshal(typeURL string, value []byte, v interface{}) (interface{}, error) {
-	t, err := getTypeByUrl(typeURL)
+	t, isProto, err := getTypeByUrl(typeURL)
 	if err != nil {
 		return nil, err
 	}
@@ -258,43 +258,45 @@ func unmarshal(typeURL string, value []byte, v interface{}) (interface{}, error)
 		}
 	}
 
-	pm, ok := v.(proto.Message)
-	if ok {
-		return v, proto.Unmarshal(value, pm)
-	}
+	if isProto {
+		pm, ok := v.(proto.Message)
+		if ok {
+			return v, proto.Unmarshal(value, pm)
+		}
 
-	for _, h := range handlers {
-		if unmarshal := h.Unmarshaller(v); unmarshal != nil {
-			return v, unmarshal(value)
+		for _, h := range handlers {
+			if unmarshal := h.Unmarshaller(v); unmarshal != nil {
+				return v, unmarshal(value)
+			}
 		}
 	}
 
 	// fallback to json unmarshaller
 	return v, json.Unmarshal(value, v)
 }
 
-func getTypeByUrl(url string) (reflect.Type, error) {
+func getTypeByUrl(url string) (_ reflect.Type, isProto bool, _ error) {
 	mu.RLock()
 	for t, u := range registry {
 		if u == url {
 			mu.RUnlock()
-			return t, nil
+			return t, false, nil
 		}
 	}
 	mu.RUnlock()
 	mt, err := protoregistry.GlobalTypes.FindMessageByURL(url)
 	if err != nil {
 		if errors.Is(err, protoregistry.NotFound) {
 			for _, h := range handlers {
-				if t := h.GetType(url); t != nil {
-					return t, nil
+				if t, isProto := h.GetType(url); t != nil {
+					return t, isProto, nil
 				}
 			}
 		}
-		return nil, fmt.Errorf("type with url %s: %w", url, ErrNotFound)
+		return nil, false, fmt.Errorf("type with url %s: %w", url, ErrNotFound)
 	}
 	empty := mt.New().Interface()
-	return reflect.TypeOf(empty).Elem(), nil
+	return reflect.TypeOf(empty).Elem(), true, nil
 }
 
 func tryDereference(v interface{}) reflect.Type {
diff --git a/types_gogo.go b/types_gogo.go
@@ -59,10 +59,10 @@ func (gogoHandler) TypeURL(v interface{}) string {
 	return gogoproto.MessageName(pm)
 }
 
-func (gogoHandler) GetType(url string) reflect.Type {
+func (gogoHandler) GetType(url string) (reflect.Type, bool) {
 	t := gogoproto.MessageType(url)
 	if t == nil {
-		return nil
+		return nil, false
 	}
-	return t.Elem()
+	return t.Elem(), true
 }
diff --git a/types_test.go b/types_test.go
@@ -18,6 +18,7 @@ package typeurl
 
 import (
 	"bytes"
+	"encoding/json"
 	"errors"
 	"reflect"
 	"testing"
@@ -241,3 +242,24 @@ func TestUnmarshalNotFound(t *testing.T) {
 		t.Fatalf("unexpected error unmarshalling type which does not exist: %v", err)
 	}
 }
+
+func TestUnmarshalJSON(t *testing.T) {
+	url := t.Name()
+	Register(&timestamppb.Timestamp{}, url)
+
+	expected := timestamppb.Now()
+
+	dt, err := json.Marshal(expected)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	var actual timestamppb.Timestamp
+	if err := UnmarshalToByTypeURL(url, dt, &actual); err != nil {
+		t.Fatal(err)
+	}
+
+	if !expected.AsTime().Equal(actual.AsTime()) {
+		t.Fatalf("expected value to be %q, got: %q", expected.AsTime(), actual.AsTime())
+	}
+}

Original file line number	Diff line number	Diff line change
`@@ -39,7 +39,7 @@ type handler interface {`
`39`	`39`	`Marshaller(interface{}) func() ([]byte, error)`
`40`	`40`	`Unmarshaller(interface{}) func([]byte) error`
`41`	`41`	`TypeURL(interface{}) string`
`42`		`- GetType(url string) reflect.Type`
	`42`	`+ GetType(url string) (reflect.Type, bool)`
`43`	`43`	`}`
`44`	`44`
`45`	`45`	`// Definitions of common error types used throughout typeurl.`
`@@ -240,7 +240,7 @@ func MarshalAnyToProto(from interface{}) (*anypb.Any, error) {`
`240`	`240`	`}`
`241`	`241`
`242`	`242`	`func unmarshal(typeURL string, value []byte, v interface{}) (interface{}, error) {`
`243`		`- t, err := getTypeByUrl(typeURL)`
	`243`	`+ t, isProto, err := getTypeByUrl(typeURL)`
`244`	`244`	`if err != nil {`
`245`	`245`	`return nil, err`
`246`	`246`	`}`
`@@ -258,43 +258,45 @@ func unmarshal(typeURL string, value []byte, v interface{}) (interface{}, error)`
`258`	`258`	`}`
`259`	`259`	`}`
`260`	`260`
`261`		`- pm, ok := v.(proto.Message)`
`262`		`- if ok {`
`263`		`- return v, proto.Unmarshal(value, pm)`
`264`		`- }`
	`261`	`+ if isProto {`
	`262`	`+ pm, ok := v.(proto.Message)`
	`263`	`+ if ok {`
	`264`	`+ return v, proto.Unmarshal(value, pm)`
	`265`	`+ }`
`265`	`266`
`266`		`- for _, h := range handlers {`
`267`		`- if unmarshal := h.Unmarshaller(v); unmarshal != nil {`
`268`		`- return v, unmarshal(value)`
	`267`	`+ for _, h := range handlers {`
	`268`	`+ if unmarshal := h.Unmarshaller(v); unmarshal != nil {`
	`269`	`+ return v, unmarshal(value)`
	`270`	`+ }`
`269`	`271`	`}`
`270`	`272`	`}`
`271`	`273`
`272`	`274`	`// fallback to json unmarshaller`
`273`	`275`	`return v, json.Unmarshal(value, v)`
`274`	`276`	`}`
`275`	`277`
`276`		`-func getTypeByUrl(url string) (reflect.Type, error) {`
	`278`	`+func getTypeByUrl(url string) (_ reflect.Type, isProto bool, _ error) {`
`277`	`279`	`mu.RLock()`
`278`	`280`	`for t, u := range registry {`
`279`	`281`	`if u == url {`
`280`	`282`	`mu.RUnlock()`
`281`		`- return t, nil`
	`283`	`+ return t, false, nil`
`282`	`284`	`}`
`283`	`285`	`}`
`284`	`286`	`mu.RUnlock()`
`285`	`287`	`mt, err := protoregistry.GlobalTypes.FindMessageByURL(url)`
`286`	`288`	`if err != nil {`
`287`	`289`	`if errors.Is(err, protoregistry.NotFound) {`
`288`	`290`	`for _, h := range handlers {`
`289`		`- if t := h.GetType(url); t != nil {`
`290`		`- return t, nil`
	`291`	`+ if t, isProto := h.GetType(url); t != nil {`
	`292`	`+ return t, isProto, nil`
`291`	`293`	`}`
`292`	`294`	`}`
`293`	`295`	`}`
`294`		`- return nil, fmt.Errorf("type with url %s: %w", url, ErrNotFound)`
	`296`	`+ return nil, false, fmt.Errorf("type with url %s: %w", url, ErrNotFound)`
`295`	`297`	`}`
`296`	`298`	`empty := mt.New().Interface()`
`297`		`- return reflect.TypeOf(empty).Elem(), nil`
	`299`	`+ return reflect.TypeOf(empty).Elem(), true, nil`
`298`	`300`	`}`
`299`	`301`
`300`	`302`	`func tryDereference(v interface{}) reflect.Type {`
Original file line number	Diff line number	Diff line change
`@@ -59,10 +59,10 @@ func (gogoHandler) TypeURL(v interface{}) string {`
`59`	`59`	`return gogoproto.MessageName(pm)`
`60`	`60`	`}`
`61`	`61`
`62`		`-func (gogoHandler) GetType(url string) reflect.Type {`
	`62`	`+func (gogoHandler) GetType(url string) (reflect.Type, bool) {`
`63`	`63`	`t := gogoproto.MessageType(url)`
`64`	`64`	`if t == nil {`
`65`		`- return nil`
	`65`	`+ return nil, false`
`66`	`66`	`}`
`67`		`- return t.Elem()`
	`67`	`+ return t.Elem(), true`
`68`	`68`	`}`