Why does this test fail?

[Deleted user]

The following test fails on the last inequality check with:
demo_test.go:54: goonPutGet.Value should be 15 but is 0

package goon_test

import (
    "testing"

    "appengine/aetest"
    "appengine/datastore"

    "github.com/mjibson/goon"
)

type PutGet struct {
    ID    int64 `datastore:"-" goon:"id"`
    Value int32
}

func TestPutGet(t *testing.T) {
    c, err := aetest.NewContext(nil)
    if err != nil {
        t.Fatal(err.Error())
    }
    g := goon.FromContext(c)
    key, err := g.Put(&PutGet{ID: 12, Value: 15})
    if err != nil {
        t.Fatal(err.Error())
    }
    if key.IntID() != 12 {
        t.Fatal("ID should be 12 but is", key.IntID())
    }

    // Datastore Get
    dsPutGet := &PutGet{}
    err = datastore.Get(c,
        datastore.NewKey(c, "PutGet", "", 12, nil), dsPutGet)
    if err != nil {
        t.Fatal(err.Error())
    }
    if dsPutGet.Value != 15 {
        t.Fatal("dsPutGet.Value should be 15 but is",
            dsPutGet.Value)
    }

    // Goon Get
    goonPutGet := &PutGet{ID: 12}
    err = g.Get(goonPutGet)
    if err != nil {
        t.Fatal(err.Error())
    }
    if goonPutGet.ID != 12 {
        t.Fatal("goonPutGet.ID should be 12 but is", goonPutGet.ID)
    }
    if goonPutGet.Value != 15 {
        t.Fatal("goonPutGet.Value should be 15 but is",
            goonPutGet.Value)
    }
}

Looking at the code it appears that the in memory cached value of PutGet{ID:12} is not being transferred to goonPutGet. Note that if I create a new Goon struct for the g.Get(goonPutGet) everything works fine because the in memory cache is not used.

[maddyblue]

I've been looking at this and have learned a few things. Your test case is correctly identifying a bug. GetMulti, which is in charge of populating the result data, is correctly identifying this key as in memory (which is why it's returning 0 instead of 15, which it would if it had to go to memcache/datastore). However, instead of using the existing memory provided by goonPutGet, it is assigning over its location in the slice.

Get is a wrapper for GetMulti. I've modified your test to use v := []interface{}{goonPutGet} and call GetMulti(v). After the call, goonPutGet remains the same, but v now contains the data with a value of 15. So it's doing v[0] = <struct from memory cache> instead of populating the members of v[0] with the data from the cache.

The existing behavior has a nice speed and pointer benefit: the new v[0] points to the same data as is in the cache, which means modifying it in one place modifies it in all of them. I think that's good and expected. Doing something where we instead copy all of the data from the cached value into v[0] (and thus into goonPutGet) loses this feature. But, changing to the former behavior would mean changing the API (and drifting from the datastore API), which is worse.

So, the cache must always copy its contents to the destination instead of point the slice to the value. I'll work on that. Thanks for being a rubber duck.

[Deleted user]

Thanks for the explanation. I also learnt a few things!

[mzimmerman]

I had fixed this in my fork with revision 530e947
The key line is set.Elem().Set(reflect.ValueOf(dsts[0]).Elem()) which equates to *dst = *dsts[0]

[mzimmerman]

This was fixed in #13 and can be closed.