Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proposal: encoding/json: add FlexObject for encoding/decoding between JSON and flex Go types. #46065

Closed
vipally opened this issue May 9, 2021 · 19 comments

Comments

@vipally
Copy link

vipally commented May 9, 2021

What version of Go are you using (go version)?

$ go version
go version go1.16 windows/amd64

Does this issue reproduce with the latest release?

Yes

Sometimes we have to deal with JSON that with unknown data types.

Such as sample JSON below:

[
    {
        "kind":"dog",
        "attr":{
            "type":"Collie",
            "color":"black"
        }
    },
    {
        "kind":"duck",
        "attr":{
            "weight":1.2
        }
    }
]

The attr field maybe below Go types, which is not a certain Go type that is decided on the value of field "kind".

type DogAttr struct {
	Type  string `json:"type"`
	Color string `json:"color"`
}

type DuckAttr struct {
	Weight float64  `json:"weight"`
}

Currently, we may deal with this case as below:

func TestDecodeRaw(t *testing.T) {
	var factory = NewFactory()
	factory.MustReg("dog", (*DogAttr)(nil))
	factory.MustReg("duck", (*DuckAttr)(nil))

	type AnimalRaw struct {
		Kind string          `json:"kind"`
		Attr json.RawMessage `json:"attr"`
	}
	var animals []AnimalRaw
	json.Unmarshal(sampleJson, &animals)
	for i, v := range animals {
		d, _ := factory.Create(v.Kind)
		json.Unmarshal(v.Attr, d)
		fmt.Printf("index %d, kind=%s attr=%#v\n", i, v.Kind, d)
	}
	// Output:
	// index 0, kind=dog attr=&lab27.DogAttr{Type:"Collie", Color:"black"}
	// index 1, kind=duck attr=&lab27.DuckAttr{Weight:1.2}
}

But the way of generate a sample JSON in this case looks ugly:

func TestEncodeRaw(t *testing.T) {
	type AnimalRaw struct {
		Kind string          `json:"kind"`
		Attr json.RawMessage `json:"attr"`
	}
	var animals = []AnimalRaw{
		AnimalRaw{
			Kind: "dog",
			Attr: []byte(`{"type": "Collie","color": "white"}`), // ugly
		},
		AnimalRaw{
			Kind: "duck",
			Attr: []byte(`{"Weight": 2.34}`), // ugly
		},
	}
	b, _ := json.MarshalIndent(animals, "", "  ")
	fmt.Println(string(b))
	// Output:
	// [
	//  {
	//    "kind": "dog",
	//    "attr": {
	//      "type": "Collie",
	//      "color": "white"
	//    }
	//  },
	//  {
	//    "kind": "duck",
	//    "attr": {
	//      "Weight": 2.34
	//    }
	//  }
	// ]
}

An eleganter solution of this case maybe as below. Compare with json.RawMessage, FlexObject can delay JSON decoding
from field "Raw" into field "D" and can direct encoding JSON from field "D".

// FlexObject is an object that can encoding/decoding JSON between flex Go types.
// It implements Marshaler and Unmarshaler and can delay JSON decoding
// from field Raw into field D and can direct encoding from field D.
type FlexObject struct {
	Raw []byte     // raw bytes for delay JSON decoding into field D
	D   interface{} // flex object for JSON encoding
}

// MarshalJSON encoding field D as JSON.
func (f FlexObject) MarshalJSON() ([]byte, error) {
	return Marshal(f.D)
}

// UnmarshalJSON copy data into field Raw.
func (f *FlexObject) UnmarshalJSON(data []byte) error {
	f.Raw = append(f.Raw[0:0], data...)
	return nil
}

Coordinate with the flex object factory. The way to deal with this case maybe as below:

func TestFlexObjectFactory(t *testing.T) {
	var factory = NewFactory()
	factory.MustReg("dog", (*DogAttr)(nil))
	factory.MustReg("duck", (*DuckAttr)(nil))

	type Animal struct {
		Kind string          `json:"kind"`
		Attr json.FlexObject `json:"attr"`
	}
	var animals []Animal
	json.Unmarshal(sampleJson, &animals)
	for i, v := range animals {
		factory.UnmarshalJSONForFlexObj(v.Kind, &v.Attr)
		fmt.Printf("index %d, kind=%s attr=%#v\n", i, v.Kind, v.Attr.D)
	}
	// Output:
	// index 0, kind=dog attr=&lab27.DogAttr{Type:"Collie", Color:"black"}
	// index 1, kind=duck attr=&lab27.DuckAttr{Weight:1.2}
}

And the way to generate the sample JSON maybe as below:

func TestGenerateJsonByFlexObject(t *testing.T) {
	type Animal struct {
		Kind string          `json:"kind"`
		Attr json.FlexObject `json:"attr"`
	}
	var animals = []Animal{
		Animal{
			Kind: "dog",
			Attr: json.FlexObject{
				D: DogAttr{
					Type:  "Collie",
					Color: "white",
				},
			},
		},
		Animal{
			Kind: "duck",
			Attr: json.FlexObject{
				D: DuckAttr{
					Weight: 2.34,
				},
			},
		},
	}
	b, _ := json.MarshalIndent(animals, "", "  ")
	fmt.Println(string(b))
	// Ooutput:
	// [
	//  {
	//    "kind": "dog",
	//    "attr": {
	//      "type": "Collie",
	//      "color": "white"
	//    }
	//  },
	//  {
	//    "kind": "duck",
	//    "attr": {
	//      "Weight": 2.34
	//    }
	//  }
	// ]
}

As above shows, json.FlexObject coordinate with json.Factory makes the dealing with JSON flex object case eleganter and automatically.
Actually json.FlexObject is designed to replace json.RawMessage.But if remove json.RawMessage directly, it will break the consensus of Go 1. Maybe json.RawMessage can be removed from Go 2.

If the proposal is accepted, it will be my pleasure to push the PR.

@gopherbot gopherbot added this to the Proposal milestone May 9, 2021
@akupila
Copy link

akupila commented May 9, 2021

You don't have to use json.RawMessage to encode different values: https://play.golang.org/p/UAvrX99tZ18

@vipally
Copy link
Author

vipally commented May 9, 2021

You don't have to use json.RawMessage to encode different values: https://play.golang.org/p/UAvrX99tZ18

So please unmarshal sample JSON into proper type of xxAttr: https://play.golang.org/p/FxvATK57w14

@akupila
Copy link

akupila commented May 9, 2021

Sorry if my response came across as too direct, I was just providing an example that I believe solves the problem without adding anything to stdlib. For umarshalling, json.RawMessage is a good solution: https://play.golang.org/p/5utvAD2Rmi8

@vipally
Copy link
Author

vipally commented May 9, 2021

Sorry if my response came across as too direct, I was just providing an example that I believe solves the problem without adding anything to stdlib. For umarshalling, json.RawMessage is a good solution: https://play.golang.org/p/5utvAD2Rmi8

Do you notice that you use the same idea with FlexObject: json.RawMessage=>interface{} to delay JSON decoding into uncertain Go types, and interface{} to direct encoding to JSON.
But if you believe your JSON decoding code looks beautiful, I have nothing to say here.

@dsnet
Copy link
Member

dsnet commented May 10, 2021

If the FlexObject type can easily be implemented outside of the json pacakge, there needs to be much stronger justification for why it should be a first-class feature of the json package. The asymmetric nature of FlexObject is already a confusing aspect of it's semantic behavior. I suspect most users would be surprised that FlexObject.Raw only be used for decoding and FlexObject.D only be used for encoding.

@vipally
Copy link
Author

vipally commented May 10, 2021

If the FlexObject type can easily be implemented outside of the json pacakge, there needs to be much stronger justification for why it should be a first-class feature of the json package. The asymmetric nature of FlexObject is already a confusing aspect of it's semantic behavior. I suspect most users would be surprised that FlexObject.Raw only be used for decoding and FlexObject.D only be used for encoding.

  1. JSON was born from JavaScript, a dynamic laguage. In JS, describe an array with flex object is naturally. So deal with this case is common requirement, but not a rare case. But Go as a static typing programming language, it's difficult to describe this case.
  2. Currently, to encoding/decoding flex JSON object, we need at least two similar GO types. This makes the logic code confusing.
    So is this usage symmetrically?
// for JSON encoding only
type AnimalEncode struct {
	Kind string      `json:"kind"`
	Attr interface{} `json:"attr"` // for JSON encoding only
}

// for JSON decoding only
type AnimalDecode struct {
	Kind string          `json:"kind"`
	Attr json.RawMessage `json:"attr"` // for JSON decoding only
}
  1. About the asymmetric nature of FlexObject.
    The working flow of of FlexObject is:
  • decoding: JSON => FlexObject.Raw => FlexObject.D
  • encoding: FlexObject.D => JSON
    FlexObject is fundamentally convert between JSON <=> FlexObject.D
    FlexObject.Raw is just the temporary buffer to delay the decoding steps.
    So how does FlexObject takes asymmetric nature ?
  1. About why need implement FlexObject in stdlib instead of third party package.
    When we implements a Go programme we have following choices:
  • language inside feature
  • standard library
  • private packages
  • 3rd-party packages
    Language-inside and stdlib is always the first choice when we consider a feature rely, and 3rd-party package with the lowest priority.
    So the reason is:
  • This case is a common case when dealing with JSON in Go
  • stdlib lacks of feature to deal with this case elegantly
  • 3rd-party package increases the rely complexity

@dsnet
Copy link
Member

dsnet commented May 10, 2021

This case is a common case when dealing with JSON in Go

"Common" is a fairly subjective adjective and a lot of decision for proposals are driven by hard data. I believe the burden of proof is on the proposer to show that such a proposed pattern is sufficiently widely used. Can you provide data that this is the case?

@dsnet dsnet closed this as completed May 10, 2021
@dsnet dsnet reopened this May 10, 2021
@dsnet
Copy link
Member

dsnet commented May 10, 2021

I should also note that this can also be solved with #5901. Instead of a FlexObject type, you could just use interface{} such that you populate the interface{} with a concrete Go type for marshaling.

For unmarshaling, you can register custom unmarshaler that unmarshals into any interface{} type as a RawMessage instead of the usual map[string]interface{}:

dec.Register(func(b []byte, v *interface{}) error {
    *v = append(json.RawMessage(nil), b...)
    return nil
})

@rsc
Copy link
Contributor

rsc commented May 12, 2021

It sounds like this can be done with the existing API.

@rsc
Copy link
Contributor

rsc commented May 12, 2021

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@networkimprov
Copy link

cc @mvdan

@rsc
Copy link
Contributor

rsc commented May 19, 2021

Based on the discussion above, this proposal seems like a likely decline.
— rsc for the proposal review group

@vipally
Copy link
Author

vipally commented May 20, 2021

Maybe I have missing something.
In my understanding, #5901 enables change the behavior of marshaling/unmarshaling JSON for maybe 3rd-party types. Great job! @dsnet

I'am interested in how will gophers solve this case based on #5901
Maybe as follow:

var sampleJson = []byte(`
[
    {
        "kind":"dog",
        "attr":{
            "type":"Collie",
            "color":"black"
        }
    },
    {
        "kind":"duck",
        "attr":{
            "weight":1.2
        }
    }
]
`)

func TestCase5901(t *testing.T) {
	type AnimalRaw struct {
		Kind string      `json:"kind"`
		Attr interface{} `json:"attr"`
	}
	var animals []AnimalRaw
	var dec = json.NewDecoder(bytes.NewReader(sampleJson))
	dec.Register(func(b []byte, v *interface{}) error {
		*v = append(json.RawMessage(nil), b...)
		return nil
	})
	dec.Decode(&animals)
	fmt.Printf("%#v\n", animals)
}

And how about the behavior of the defalut API?

func TestCase5901_default(t *testing.T) {
	type AnimalRaw struct {
		Kind string      `json:"kind"`
		Attr interface{} `json:"attr"`
	}
	var animals []AnimalRaw
	json.Unmarshal(sampleJson, &animals) // the behavior of Attr?
	fmt.Printf("%#v\n", animals)
}

And the behavior of 3rd-party APIs?

import "github.com/gin-gonic/gin"
func TestCase5901_3rdParty(t *testing.T) {
	type AnimalRaw struct {
		Kind string      `json:"kind"`
		Attr interface{} `json:"attr"`
	}
	var animals []AnimalRaw
	gin.Context.BindJSON(&animals)  // the behavior of Attr?

	fmt.Printf("%#v\n", animals)
}

//---------git.luolix.top/gin-gonic/gin.BindJson-------------------------
func decodeJSON(r io.Reader, obj interface{}) error {
	decoder := json.NewDecoder(r)
	if EnableDecoderUseNumber {
		decoder.UseNumber()
	}
	if EnableDecoderDisallowUnknownFields {
		decoder.DisallowUnknownFields()
	}
	if err := decoder.Decode(obj); err != nil {
		return err
	}
	return validate(obj)
}

So @dsnet could you show more examples on how gophers will solve this case based on #5901 ?
Maybe we need another proposal?
#5901 : force unmarshal all interface{} to json.RawMessage.

@dsnet
Copy link
Member

dsnet commented May 20, 2021

I'm not sure I follow. The example shown with TestCase5901 seems to do what you want? I'm not sure what you're trying to get at with TestCase5901_default as I expect the default behavior to use the normal semantics when unmarshaling into a interface{}.

@vipally
Copy link
Author

vipally commented May 21, 2021

I'm not sure I follow. The example shown with TestCase5901 seems to do what you want? I'm not sure what you're trying to get at with TestCase5901_default as I expect the default behavior to use the normal semantics when unmarshaling into a interface{}.

Well, the basic demand of this case is:
Unmarshal interface{} into json.RawMessage instead of map[string]interface{}

As above shows, TestCase5901 can meet the demand because we can control the unmarshal behavior of interface{}

TestCase5901_default is the common usage when unmarshaling JSON. I think usually we don't care if the target type contains interface{}, but request the expected behavior of unmarshaling.

TestCase5901_3rdParty is the case that we can't control the unmarshaling behavior which depend on the behavior of 3rd-party package.

So I think #5901 can't meet this demand unless request below proposal:
#5901 : force unmarshal all interface{} into json.RawMessage instead of map[string]interface{} .

@vipally
Copy link
Author

vipally commented May 22, 2021

According to discussion above, update the implementation of FlexObject:

// FlexObject is an object that can encoding/decoding JSON between flex Go types.
// It implements Marshaler and Unmarshaler and can delay JSON decoding
// from []byte into flex object.
type FlexObject struct {
	D interface{} // flex object for JSON encoding and decoding
}

// MarshalJSON encoding field D as JSON.
func (f FlexObject) MarshalJSON() ([]byte, error) {
	return Marshal(f.D)
}

// UnmarshalJSON copy data into field D.
func (f *FlexObject) UnmarshalJSON(data []byte) error {
	f.D = append([]byte(nil), data...)
	return nil
}

// DelayedUnmarshalJSON unmarshal []byte into instance d.
// It will update field D if unmarshal OK.
func (f *FlexObject) DelayedUnmarshalJSON(d interface{}) error {
	b, ok := f.D.([]byte)
	if !ok {
		return errors.New("json.FlexObject: DelayedUnmarshalJSON on none []byte value")
	}
	if err := Unmarshal(b, d); err != nil {
		return err
	}
	f.D = d
	return nil
}

And the example of decoding code:

func TestDecodeFlexObject(t *testing.T) {
	type Animal struct {
		Kind string          `json:"kind"`
		Attr json.FlexObject `json:"attr"`
	}
	var animals []Animal
	json.Unmarshal(sampleJson, &animals)
	for i, v := range animals {
		var d interface{}
		switch v.Kind {
		case "dog":
			d = &DogAttr{}
		case "duck":
			d = &DuckAttr{}
		default:
			t.Fatalf("unsupport kind %s", v.Kind)
		}
		if err := v.Attr.DelayedUnmarshalJSON(d); err != nil { // delay decoding FlexObject
			t.Fatal(err)
		}
		fmt.Printf("index %d, kind=%s attr=%#v\n", i, v.Kind, v.Attr.D)
	}
	// Output:
	// index 0, kind=dog attr=&lab27.DogAttr{Type:"Collie", Color:"black"}
	// index 1, kind=duck attr=&lab27.DuckAttr{Weight:1.2}
}

@dsnet
Copy link
Member

dsnet commented May 22, 2021

@vipally, I mentioned as #5901 as one possible way to achieve what this issue seems to trying to do. I never claimed it was the only way nor perfect way (since it can only be done at the Unmarshal call site as you observed). I don't think it's helpful to keep focusing on #5901 as it's actually detracting from your proposal.

Given that there are several ways to accomplish this outside the standard json package, my original question still stands unanswered:

I believe the burden of proof is on the proposer to show that such a proposed pattern is sufficiently widely used. Can you provide data that this is the case?

Judging by the reaction by most others on this issue, it seems that this feature does not seem sufficiently useful to be included.

@vipally
Copy link
Author

vipally commented May 23, 2021

I mentioned as #5901 as one possible way to achieve what this issue seems to trying to do. I never claimed it was the only way nor perfect way (since it can only be done at the Unmarshal call site as you observed). I don't think it's helpful to keep focusing on #5901 as it's actually detracting from your proposal.

I assume that you have notice that #5901 can't meet this demand.
I'm looking forward to other solutions could achive this perfectly, but at least so far, nobody gives an example.

Given that there are several ways to accomplish this outside the standard json package, my original question still stands unanswered

I'm looking forward to other solutions could achive this perfectly, but at least so far, nobody gives an example.
It's truth that there are several ways to accomplish this by an outside package. And acctually I have achived my work by this way.
What I appeal here is to let the standard library to keep eyes on the clients code of this case.
jsom.RawMessage seems tried to achive this work but actually makes client code confusion.

I believe the burden of proof is on the proposer to show that such a proposed pattern is sufficiently widely used. Can you provide data that this is the case?

I can't list all use cases.
But do you notice that derivation feature in object-oriented theory matches well of this case.
I think none could gainsay OOP theory's widely used.
Here are some examples, and many of them were acctually born from my working abstraction.

  • []Animal{ Dog, Cat, Duck, ... }
  • []Shape{ Triangle, Square, Rectangle, ... }
  • []Automobile{Car, Truck, Bus, ... }
  • []People{ Teacher, Doctor, ... }
  • []Expression{ Number, String, Object, Array, ... }
  • []GameObject{ Actor, Monster, Pet, Npc...}
  • []Events{ Start, NetWork, Timing, ... }
  • ...

@rsc
Copy link
Contributor

rsc commented May 26, 2021

No change in consensus, so declined.
— rsc for the proposal review group

@rsc rsc closed this as completed May 26, 2021
@golang golang locked and limited conversation to collaborators May 26, 2022
@rsc rsc moved this to Declined in Proposals Aug 10, 2022
@rsc rsc added this to Proposals Aug 10, 2022
@rsc rsc removed this from Proposals Oct 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants