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

[Victor Toso]

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.

> > >   func (s GuestPanicInformation) MarshalJSON() ([]byte, error) {
> > >       if (s.HyperV != nil && s.S390 != nil) ||
> > >           (s.HyperV == nil && s.S390 == nil) {
> > >           // client hasn't filled in the struct properly
> > >           return nil, errors.New("...")
> > >       }
> > > 
> > >       tmp := jsonGuestPanicInformation{}
> > > 
> > >       if s.HyperV != nil {
> > >           tmp.Discriminator = "hyper-v"
> > >           tmp.HyperV = s.HyperV
> > >       } else if s.S390 != nil {
> > >           tmp.Discriminator = "s390"
> > >           tmp.S390 = s.S390
> > >       }
> > > 
> > >       return json.Marshal(tmp)
> > >   }
> > 
> > And...
> > 
> >        var discriminator string
> >        if s.HyperV != nil {
> >            discriminator = "hyper-v"
> >        } else if s.S390 != nil {
> >            discriminator = "s390"
> >        }
> > 
> >        tmp := jsonGuestPanicInformation{ discriminator, s}
> >        return json.Marshal(tmp)
> > 
> > > 
> > >   func (s *GuestPanicInformation) UnmarshalJSON(data []byte) error {
> > >       tmp := jsonGuestPanicInformation{}
> > > 
> > >       err := json.Unmarshal(data, &tmp)
> > >       if err != nil {
> > >           return err
> > >       }
> > > 
> > >       switch tmp.Discriminator {
> > >       case "hyper-v":
> > >           if tmp.HyperV == nil {
> > >               return errors.New("...")
> > >           }
> > >           s.HyperV = tmp.HyperV
> > >       case "s390":
> > >           if tmp.S390 == nil {
> > >               return errors.New("...")
> > >           }
> > >           s.S390 = tmp.S390
> > >       }
> >
> > I'm not actually sure this works, because the first json.Unmarshal
> > call won't know which branch to try unmarhsalling. So it might be
> > unavoidable to parse twice.  With the XML parser this wouldn't be
> > a problem as it has separated the parse phase and then fills the
> > struct after.
> 
> Right afer sending, I remember how this is supposed to be done. It
> involves use of 'json.RawMessage' eg examples at:
> 
>   https://pkg.go.dev/encoding/json#example-RawMessage-Unmarshal
> 
> So it would look like:
> 
>    type GuestPanicInformation struct {
>        HyperV *GuestPanicInformationHyperV
>        S390   *GuestPanicInformationS390
>    }
>  
>    type jsonGuestPanicInformation struct {
>        Discriminator string   `json:"type"`
>        Payload *json.RawMessage
>    }
> 
> 
>     func (s GuestPanicInformation) MarshalJSON() ([]byte, error) {
>         var p *json.RawMesage
>         var err error
>         if s.HyperV != nil {
>             d = "hyper-v"
>             p, err = json.Marshal(s.HyperV)
>         } else if s.S390 != nil {
>             d = "s390"
>             p, err = json.Marshal(s.S390)
>         } else {
>           err = fmt.Errorf("No payload defined")
>       }
>         if err != nil {
>             return []byte{}, err
>         }
>   
>         return json.Marshal(jsonGuestPanicInformation{d, p}), nil
>     }
> 
> 
>  
>    func (s *GuestPanicInformation) UnmarshalJSON(data []byte) error {
>        tmp := jsonGuestPanicInformation{}
>  
>        err := json.Unmarshal(data, &tmp)
>        if err != nil {
>            return err
>        }
>  
>        switch tmp.Discriminator {
>        case "hyper-v":
>            s.HyperV := GuestPanicInformationHyperV{}
>            err := json.Unmarshal(tmp.Payload, s.HyperV)
>            if err != nil {
>               return err
>            }
>        case "s390":
>            s.S390 := GuestPanicInformationS390{}
>            err := json.Unmarshal(tmp.Payload, s.S390)
>            if err != nil {
>               return err
>            }
>        }
> 
>        return fmt.Errorf("Unknown type '%s'", tmp.Discriminator)
>   }
> 
> With regards,
> Daniel
> -- 
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Cheers,
Victor

signature.asc
Description: PGP signature