Re: [RFC PATCH v2 4/8] qapi: golang: Generate qapi's union types in Go

[Victor Toso <victortoso@redhat.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2214 bytes --]

Hi,

On Wed, Aug 17, 2022 at 06:25:56PM +0200, Victor Toso wrote:
> On Wed, Jul 06, 2022 at 10:48:06AM +0100, Daniel P. Berrangé wrote:
> > On Wed, Jul 06, 2022 at 10:37:54AM +0100, Daniel P. Berrangé wrote:
> > > On Wed, Jul 06, 2022 at 04:28:16AM -0500, Andrea Bolognani wrote:
> > > > You're right, that is undesirable. What about something like this?
> > > >
> > > >   type GuestPanicInformation struct {
> > > >       HyperV *GuestPanicInformationHyperV
> > > >       S390   *GuestPanicInformationS390
> > > >   }
> > > >
> > > >   type jsonGuestPanicInformation struct {
> > > >       Discriminator string                       `json:"type"`
> > > >       HyperV        *GuestPanicInformationHyperV `json:"hyper-v"`
> > > >       S390          *GuestPanicInformationS390   `json:"s390"`
> > > >   }
> > >
> > > It can possibly be even simpler with just embedding the real
> > > struct
> > >
> > >    type jsonGuestPanicInformation struct {
> > >        Discriminator string
> > >        GuestPanicInformation
> > >    }
>
> Similar to what I said in previous email (same thread) to Andrea,
> this would not work because the end result does not match with
> QAPI spec, where HyperV or S390 fields should be at the same
> level as 'type'.
>
> If we embed either HyperV or S390, then it should work, like:
>
>     tmp := struct {
>         GuestPanicInformationHyperV
>         Discriminator string "type"
>     }{}
>
> But I intend to try the json.RawMessage too as with description
> it seems like we can avoid looking the whole json data twice.

For the same reason, I could not use json.RawMessage, sadly.

As Andrea pointed out before, json.RawMessage is just an alias
for []byte. We need a field for that (so, it can't be embed) and
the contents of the JSON need to match that field's name.

I've removed the string manipulation and used a few anonymous
structs instead. I'm attaching a main.go program that tests this
behavior together with input checks that Andrea suggested. This
is more or less how the generated code will look like in the next
iteration but in case one want's to find a nicer/shorter
solution, I'm all ears :)

Cheers,
Victor

[-- Attachment #1.2: main.go --]
[-- Type: text/plain, Size: 3182 bytes --]

package main

import (
	"encoding/json"
	"errors"
	"fmt"
	"strings"
)

type QCryptoBlockOptionsQCow struct {
	KeySecret *string `json:"key-secret,omitempty"`
}

type QCryptoBlockOptionsLUKS struct {
	KeySecret *string `json:"key-secret,omitempty"`
}

type QCryptoBlockOpenOptions struct {
	// Variants fields
	Qcow *QCryptoBlockOptionsQCow `json:"-"`
	Luks *QCryptoBlockOptionsLUKS `json:"-"`
}

func (s QCryptoBlockOpenOptions) MarshalJSON() ([]byte, error) {
	var bytes []byte
	var err error
	if s.Qcow != nil {
		tmp := struct {
			QCryptoBlockOptionsQCow
			Discriminator string `json:"format"`
		}{
			QCryptoBlockOptionsQCow: *s.Qcow,
			Discriminator:           "qcow",
		}
		if bytes, err = json.Marshal(tmp); err != nil {
			return nil, err
		}
	}
	if s.Luks != nil {
		if len(bytes) != 0 {
			return nil, errors.New(`multiple fields set for QCryptoBlockOpenOptions`)
		}
		tmp := struct {
			QCryptoBlockOptionsLUKS
			Discriminator string `json:"format"`
		}{
			QCryptoBlockOptionsLUKS: *s.Luks,
			Discriminator:           "luks",
		}
		if bytes, err = json.Marshal(tmp); err != nil {
			return nil, err
		}
	}
	if len(bytes) == 0 {
		return nil, errors.New(`null not supported for QCryptoBlockOpenOptions`)
	}
	return bytes, nil
}

func (s *QCryptoBlockOpenOptions) UnmarshalJSON(data []byte) error {
	tmp := struct {
		Discriminator string `json:"format"`
	}{}
	if err := json.Unmarshal(data, &tmp); err != nil {
		return err
	}
	switch tmp.Discriminator {
	case "qcow":
		s.Qcow = &QCryptoBlockOptionsQCow{}
		if err := json.Unmarshal(data, s.Qcow); err != nil {
			s.Qcow = nil
			return err
		}
	case "luks":
		s.Luks = &QCryptoBlockOptionsLUKS{}
		if err := json.Unmarshal(data, s.Luks); err != nil {
			s.Luks = nil
			return err
		}
	}
	return nil
}

func main() {
	jsonLuks := `{"key-secret":"my luks secret is here","format":"luks"}`
	jsonQcow := `{"key-secret":"my qcow secret is here","format":"qcow"}`
	r := QCryptoBlockOpenOptions{}
	if err := json.Unmarshal([]byte(jsonLuks), &r); err != nil {
		panic(err)
	} else if r.Luks == nil || r.Qcow != nil {
		panic(fmt.Sprintf("Wrong: %v", r))
	} else if b, err := json.Marshal(&r); err != nil {
		panic(err)
	} else if string(b) != jsonLuks {
		panic(string(b))
	}

	r = QCryptoBlockOpenOptions{}
	if err := json.Unmarshal([]byte(jsonQcow), &r); err != nil {
		panic(err)
	} else if r.Luks != nil || r.Qcow == nil {
		panic(fmt.Sprintf("Wrong: %v", r))
	} else if b, err := json.Marshal(&r); err != nil {
		panic(err)
	} else if string(b) != jsonQcow {
		panic(string(b))
	}

	r = QCryptoBlockOpenOptions{}
	if _, err := json.Marshal(&r); err == nil {
		panic("No fields set should be an error")
	} else if !strings.Contains(err.Error(), "null not supported") {
		panic(err)
	}

	qcowSecret := "my-qcow-secret-is-here"
	luksSecret := "my-luks-secret-is-here"
	r = QCryptoBlockOpenOptions{
		Qcow: &QCryptoBlockOptionsQCow{
			KeySecret: &qcowSecret,
		},
		Luks: &QCryptoBlockOptionsLUKS{
			KeySecret: &luksSecret,
		},
	}

	if _, err := json.Marshal(&r); err == nil {
		panic("multiple fields set should be an error")
	} else if !strings.Contains(err.Error(), "multiple fields set for") {
		panic(err)
	}
}

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]