googleapis / google-cloud-go

Google Cloud Client Libraries for Go.
https://cloud.google.com/go/docs/reference
Apache License 2.0
3.72k stars 1.28k forks source link

datastore: load and write string representation of int enum #4196

Open scorsi opened 3 years ago

scorsi commented 3 years ago

Is your feature request related to a problem? Please describe. I have issues loading enums from/saving enums to datastore.

Describe the solution you'd like I would be great to use the String method if it exists on enums ? Or give a property like datastore:",string" to specify we want the string repr. Maybe there is a solution I didn't found ?

Describe alternatives you've considered I make it working by changing my enum to string but loose good (automatic) enum checks on my API.

Additional context For example:

//go:generate enum Role -text -nosql -transform=kebab

type Role int

const (
    Admin Role = iota
    Standard
)

type User struct {
    Name       string
    Role        Role
}

This will save 1 for admin and 2 for standard. The issue is that I have Dataflow, and other stuffs, working with admin or standard string representation instead of the int value.

crwilcox commented 3 years ago

Can you convert the enum value to the string in before using it in the other instances?

role := Role(2)
scorsi commented 3 years ago

Sorry for the delay I missed the notification. A lot of other systems are reading/writing enum as strings, only my Go service can't, I won't change a lot of systems and risk a system outage :)

Enums in Go are conventionnaly coded as int to lightweight the memory usage (I suppose) and read/written as string from api request/database.

scorsi commented 3 years ago

I am making a PR to resolve that

crwilcox commented 3 years ago

If you need to store and hydrate from a string version, have you tried using the PropertyLoaderSaver interface. Before adding additional support I'd like to understand if this is style, something that couldn't be done, etc. :)

scorsi commented 3 years ago

Hello,

Sorry for the delay to respond...

Judge by yourself:

//go:generate enum MyEnum -text -nosql -transform=kebab

package main

import (
    "cloud.google.com/go/datastore"
    "context"
    "fmt"
    "time"
)

type MyEnum int

const (
    MyEnumA MyEnum = iota
    MyEnumB
    MyEnumC
)

type MyStruct struct {
    MyEnum MyEnum `datastore:"my_enum"`
    MyInt  int    `datastore:"my_int"`
}

func (x *MyStruct) Load(ps []datastore.Property) error {
    var err error
    for _, p := range ps {
        switch p.Name {
        case "my_enum":
            if s, ok := p.Value.(string); !ok {
                err = fmt.Errorf("error")
                break
            } else {
                if err_ := x.MyEnum.UnmarshalText([]byte(s)); err_ != nil {
                    err = err_
                    break
                }
            }
        case "my_int":
            if i, ok := p.Value.(int64); !ok {
                err = fmt.Errorf("error")
                break
            } else {
                x.MyInt = int(i)
            }
        }
    }
    return err
}

func (x *MyStruct) Save() ([]datastore.Property, error) {
    return []datastore.Property{
        {
            Name:
            "my_enum",
            Value: x.MyEnum.String(),
        },
        {
            Name:
            "my_int",
            Value: int64(x.MyInt),
        },
    }, nil
}

func main() {
    ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
    defer cancel()

    dsClient, err := datastore.NewClient(ctx, "my-project")
    if err != nil {
        panic(err)
    }
    defer func(dsClient *datastore.Client) {
        err := dsClient.Close()
        if err != nil {
            panic(err)
        }
    }(dsClient)

    key := datastore.NameKey("MyStruct", "stringID", nil)

    structToWrite := &MyStruct{
        MyEnum: MyEnumB,
        MyInt:  42,
    }

    if _, err := dsClient.Put(ctx, key, structToWrite); err != nil {
        panic(err)
    }

    structToRead := new(MyStruct)

    if err := dsClient.Get(ctx, key, structToRead); err != nil {
        panic(err)
    }

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

This code is the only one I succeed to make it work. This is not conceivable to manually write the load/save methods... In addition, here to simplify the code, I didn't check if all fields are correctly set/save so the code should contains much more boilerplate.

scorsi commented 3 years ago

I finally found a way to lightweight a little bit the code by using LoadStruct and SaveStruct:

type MyStruct struct {
    MyEnum MyEnum `datastore:"-"` // we exclude for LoadStruct/SaveStruct
    MyInt  int    `datastore:"my_int"`
}

func (x *MyStruct) Load(ps []datastore.Property) error {
    var newPs []datastore.Property
    var myEnumP datastore.Property
    for _, p := range ps {
        if p.Name != "my_enum" {
            newPs = append(newPs, p)
        } else {
            myEnumP = p
        }
    }
    if err := datastore.LoadStruct(x, newPs); err != nil {
        return err
    }
    if s, ok := myEnumP.Value.(string); !ok {
        return fmt.Errorf("error")
    } else {
        if err := x.MyEnum.UnmarshalText([]byte(s)); err != nil {
            return err
        }
    }
    return nil
}

func (x *MyStruct) Save() ([]datastore.Property, error) {
    props, err := datastore.SaveStruct(x)
    if err != nil {
        return nil, err
    }
    props = append(props, datastore.Property{
        Name:  "my_enum",
        Value: x.MyEnum.String(),
    })
    return props, nil
}

It's still to much code for just this but it works.

scorsi commented 3 years ago

@crwilcox did you take the time to look at the snippet before ? :)

scorsi commented 3 years ago

@crwilcox did you find a solution for my problem ? It is "simple" when the element is at the root of the type but when we work with embedded structs in arrays... It's starting to be very difficult to handle it.

zamicol commented 3 years ago

We're trying to do PropertyLoadSaver on a single type (which we use as an enum). The enum is an int, we want to save a string in the database.

Enums don't exists in Go as they do in other languages and the int value may change in the future, whereas a short string will remain constant which we can easily marshal/unmarshal into its enum type. A string is also more compatible across systems. And finally, the datastore int type uses 8 bytes, a string of 7 character is the same size in the database. There's no space advantage in using int over a 7 character string.

PropertyLoadSaver's Load works fine on a single field. Save will not save a single field. It will only save a struct. On a type instead of a struct, Save() saves an embedded entity into the field in the database instead of simply a single property.

In summary, although Load() works well for PropertyLoadSaver on individual custom types, Save() appears broken.

For example, Use.Save() will save an embedded {} in the database. There's no way to save "use" as a string in the database on a type. However, Use.Load() works as expected.

type Use int64

func (u *Use) Save() (ps []datastore.Property, err error) {
    return nil, nil
}

func (u *Use) Load(p []datastore.Property) error {
    s, ok := p[0].Value.(string)
    if ok {
        *u = Use.Parse(s)
    }
    return nil
}

Or alternatively, if storing enums as ints in the database, this works:

func (u *Use) Load(p []datastore.Property) error {
    s, ok := p[0].Value.(int)
    if ok {
        *u = Use.(i)
    }
    return nil
}
meredithslota commented 2 years ago

Reclassifying this as a feature request — we will have to consider it against other proposed features/solutions. Thanks for filing!

alon-ne commented 1 year ago

Hey @meredithslota, I also came across the same exact situation, upvote 👍 for the feature request, thanks!