googleapis / google-cloud-go

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

datastore: entity fields not stored/loaded when embedded struct implements PropertyLoadSaver and entity does not #5824

Open evanwht opened 2 years ago

evanwht commented 2 years ago

Client

Datastore v1.6.0

Environment

MacOS

Go Environment

go1.17.2 darwin/amd64 cloud.google.com/do/datastore v.1.16.0

Code

package main

type (
    MyStruct struct {
        Field string
        EmbeddedStruct
    }
    EmbeddedStruct struct {
        EmbeddedField string
    }
)

func (es *EmbeddedStruct) Load(properties []datastore.Property) error {
    return datastore.LoadStruct(es, properties)
}

func (es *EmbeddedStruct) Save() ([]datastore.Property, error) {
    return datastore.SaveStruct(es)
}

func main() {
    ctx := context.Background()
    c, err := datastore.NewClient(ctx, "my-project")
    if err != nil {
        panic(err)
    }

    ms := MyStruct{
        Field: "field",
        EmbeddedStruct: EmbeddedStruct{
            EmbeddedField: "embedded-field",
        },
    }

    fmt.Printf("%+v\n", ms)
    // only stores EmbeddedField to datastore entity
    key, err := c.Put(ctx, datastore.IncompleteKey("test", nil), &ms)
    if err != nil {
        panic(err)
    }

    var s MyStruct
    // only loads EmbeddedField from a datastore entity that has a value for Field
    if err = c.Get(ctx, key, &s); err != nil {
        panic(err)
    }
    fmt.Printf("%+v\n", s)
}

Expected behavior

I'm not actually sure if this behavior is what is intended but my thought would be that all fields are saved to datastore and loaded from datastore.

Actual behavior

EmbeddedField is saved to datastore but Field is not. Additionally if an entity in datastore were to have both fields saved correctly only the EmbeddedField is loaded into the struct correctly. Field is left blank.

If you implement PropertyLoadSaver on MyStruct then Field is saved and loaded correctly.

cnbuff410 commented 2 months ago

Is there any plan to address this issue soon?

Right now, because of this issue, whenever developer add Load and Save to a struct, they have to check all the other struct that contain this struct as embedded field to see if they have Load and Save override as well. Since this is not checked by compiler, it's very easy to forget this check and have the bug leaked into production that only people who knows this issue can diagnose.

It's extremely annoying for a codebase with many different data models that interact with each other.