On Fri, Jun 17, 2022 at 02:19:28PM +0200, Victor Toso wrote:
> qapi:
> | { 'union': 'SetPasswordOptions',
> | 'base': { 'protocol': 'DisplayProtocol',
> | 'password': 'str',
> | '*connected': 'SetPasswordAction' },
> | 'discriminator': 'protocol',
> | 'data': { 'vnc': 'SetPasswordOptionsVnc' } }
>
> go:
> | type SetPasswordOptions struct {
> | // Base fields
> | Password string `json:"password"`
> | Connected *SetPasswordAction `json:"connected,omitempty"`
> |
> | // Variants fields
> | Vnc *SetPasswordOptionsVnc `json:"-"`
> | }
Generated code:
> type GuestPanicInformation struct {
> // Variants fields
> HyperV *GuestPanicInformationHyperV `json:"-"`
> S390 *GuestPanicInformationS390 `json:"-"`
> }
>
> func (s GuestPanicInformation) MarshalJSON() ([]byte, error) {
> type Alias GuestPanicInformation
> alias := Alias(s)
> base, err := json.Marshal(alias)
> if err != nil {
> return nil, err
> }
>
> driver := ""
> payload := []byte{}
> if s.HyperV != nil {
> driver = "hyper-v"
> payload, err = json.Marshal(s.HyperV)
> } else if s.S390 != nil {
> driver = "s390"
> payload, err = json.Marshal(s.S390)
> }
>
> if err != nil {
> return nil, err
> }
>
> if len(base) == len("{}") {
> base = []byte("")
> } else {
> base = append([]byte{','}, base[1:len(base)-1]...)
> }
>
> if len(payload) == 0 || len(payload) == len("{}") {
> payload = []byte("")
> } else {
> payload = append([]byte{','}, payload[1:len(payload)-1]...)
> }
>
> result := fmt.Sprintf(`{"type":"%s"%s%s}`, driver, base, payload)
> return []byte(result), nil
All this string manipulation looks sketchy. Is there some reason that
I'm not seeing preventing you for doing something like the untested
code below?
func (s GuestPanicInformation) MarshalJSON() ([]byte, error) {
if s.HyperV != nil {
type union struct {
Discriminator string `json:"type"`
HyperV GuestPanicInformationHyperV `json:"hyper-v"`
}
tmp := union {
Discriminator: "hyper-v",
HyperV: s.HyperV,
}
return json.Marshal(tmp)
} else if s.S390 != nil {
type union struct {
Discriminator string `json:"type"`
S390 GuestPanicInformationHyperV `json:"s390"`
}
tmp := union {
Discriminator: "s390",
S390: s.S390,
}
return json.Marshal(tmp)
}
return nil, errors.New("...")
}
> func (s *GuestPanicInformation) UnmarshalJSON(data []byte) error {
> type Alias GuestPanicInformation
> peek := struct {
> Alias
> Driver string `json:"type"`
> }{}
>
> if err := json.Unmarshal(data, &peek); err != nil {
> return err
> }
> *s = GuestPanicInformation(peek.Alias)
>
> switch peek.Driver {
>
> case "hyper-v":
> s.HyperV = new(GuestPanicInformationHyperV)
> if err := json.Unmarshal(data, s.HyperV); err != nil {
> s.HyperV = nil
> return err
> }
> case "s390":
> s.S390 = new(GuestPanicInformationS390)
> if err := json.Unmarshal(data, s.S390); err != nil {
> s.S390 = nil
> return err
> }
> }
> // Unrecognizer drivers are silently ignored.
> return nil
This looks pretty reasonable, but since you're only using "peek" to
look at the discriminator you should be able to leave out the Alias
type entirely and perform the initial Unmarshal operation while
ignoring all other fields.
The comments made elsewhere about potentially being more strict and
rejecting invalid JSON also apply here.
--
Andrea Bolognani / Red Hat / Virtualization
