datastore: load and save elements behind interfaces
Previously we didn't consider elements that
could be behind an interface field e.g.
type Foo struct {
Bar interface{}
}
&Foo{
Bar: "bar",
}
which would produce:
datastore: unsupported struct field type: interface {}
We now extract the underlying element, during both
Loads and Saves.
Fixes #1474
Change-Id: If675f3b9338c9f9f4d6d1c5a7032acce7250f651
Reviewed-on: https://code-review.googlesource.com/c/gocloud/+/42771
Reviewed-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Tyler Bui-Palsulich <tbp@google.com>
diff --git a/datastore/integration_test.go b/datastore/integration_test.go
index 2fd8e0d..2c20c2f 100644
--- a/datastore/integration_test.go
+++ b/datastore/integration_test.go
@@ -161,9 +161,10 @@
I int
S string
T time.Time
+ U interface{}
}
- x0 := X{66, "99", timeNow.Truncate(time.Millisecond)}
+ x0 := X{66, "99", timeNow.Truncate(time.Millisecond), "X"}
k, err := client.Put(ctx, IncompleteKey("BasicsX", nil), &x0)
if err != nil {
t.Fatalf("client.Put: %v", err)
diff --git a/datastore/load.go b/datastore/load.go
index e75ccef..ffed38b 100644
--- a/datastore/load.go
+++ b/datastore/load.go
@@ -292,6 +292,18 @@
return overflowReason(x, v)
}
v.SetFloat(x)
+
+ case reflect.Interface:
+ if !v.CanSet() {
+ return fmt.Sprintf("%v is unsettable", v.Type())
+ }
+
+ rpValue := reflect.ValueOf(pValue)
+ if !rpValue.Type().AssignableTo(v.Type()) {
+ return fmt.Sprintf("%q is not assignable to %q", rpValue.Type(), v.Type())
+ }
+ v.Set(rpValue)
+
case reflect.Ptr:
// v must be a pointer to either a Key, an Entity, or one of the supported basic types.
if v.Type() != typeOfKeyPtr && v.Type().Elem().Kind() != reflect.Struct && !isValidPointerType(v.Type().Elem()) {
diff --git a/datastore/load_test.go b/datastore/load_test.go
index f381cdf..24167ba 100644
--- a/datastore/load_test.go
+++ b/datastore/load_test.go
@@ -15,6 +15,7 @@
package datastore
import (
+ "fmt"
"reflect"
"testing"
"time"
@@ -425,6 +426,83 @@
type NestedSimple2 struct {
A *Simple
I int
+ U interface{}
+}
+
+type withTypedInterface struct {
+ Field fmt.Stringer
+}
+
+type withUntypedInterface struct {
+ Field interface{}
+}
+
+func TestLoadToInterface(t *testing.T) {
+ testCases := []struct {
+ name string
+ src *pb.Entity
+ dst interface{}
+ want interface{}
+ wantErr string
+ }{
+ {
+ name: "Typed interface",
+ src: &pb.Entity{
+ Key: keyToProto(testKey0),
+ Properties: map[string]*pb.Value{
+ "Field": {ValueType: &pb.Value_StringValue{
+ StringValue: "Foo",
+ }},
+ },
+ },
+ dst: &withTypedInterface{},
+ wantErr: `datastore: cannot load field "Field" into a "datastore.withTypedInterface": "string" is not assignable to "fmt.Stringer"`,
+ },
+ {
+ name: "Untyped interface, fresh struct",
+ src: &pb.Entity{
+ Key: keyToProto(testKey0),
+ Properties: map[string]*pb.Value{
+ "Field": {ValueType: &pb.Value_StringValue{
+ StringValue: "Foo",
+ }},
+ },
+ },
+ dst: &withUntypedInterface{},
+ want: &withUntypedInterface{Field: "Foo"},
+ },
+ {
+ name: "Untyped interface, already set",
+ src: &pb.Entity{
+ Key: keyToProto(testKey0),
+ Properties: map[string]*pb.Value{
+ "Field": {ValueType: &pb.Value_StringValue{
+ StringValue: "Newly set",
+ }},
+ },
+ },
+ dst: &withUntypedInterface{Field: 1e9},
+ want: &withUntypedInterface{Field: "Newly set"},
+ },
+ }
+
+ for _, tc := range testCases {
+ t.Run(tc.name, func(t *testing.T) {
+ err := loadEntityProto(tc.dst, tc.src)
+ if tc.wantErr != "" {
+ if err == nil || err.Error() != tc.wantErr {
+ t.Fatalf("Error mismatch\nGot: %s\nWant: %s", err, tc.wantErr)
+ }
+ } else {
+ if err != nil {
+ t.Fatalf("loadEntityProto: %v", err)
+ }
+ if diff := testutil.Diff(tc.dst, tc.want); diff != "" {
+ t.Fatalf("Mismatch: got - want +\n%s", diff)
+ }
+ }
+ })
+ }
}
func TestAlreadyPopulatedDst(t *testing.T) {
@@ -471,6 +549,7 @@
Properties: map[string]*pb.Value{
"A": {ValueType: &pb.Value_NullValue{}},
"I": {ValueType: &pb.Value_IntegerValue{IntegerValue: 2}},
+ "U": {ValueType: &pb.Value_StringValue{StringValue: "replaced"}},
},
},
}},
@@ -483,6 +562,7 @@
&NestedSimple2{
A: &Simple{I: 2},
/* I: 0 */
+ U: 1e9,
},
0,
},
@@ -492,6 +572,7 @@
&NestedSimple2{
/* A: nil, */
I: 2,
+ U: "replaced",
},
5,
},
diff --git a/datastore/save.go b/datastore/save.go
index 2a44469..d92aeff 100644
--- a/datastore/save.go
+++ b/datastore/save.go
@@ -47,27 +47,9 @@
return propertiesToProto(key, props)
}
-// TODO(djd): Convert this and below to return ([]Property, error).
-func saveStructProperty(props *[]Property, name string, opts saveOpts, v reflect.Value) error {
- p := Property{
- Name: name,
- NoIndex: opts.noIndex,
- }
-
- if opts.omitEmpty && isEmptyValue(v) {
- return nil
- }
-
- // First check if field type implements PLS. If so, use PLS to
- // save.
- ok, err := plsFieldSave(props, p, name, opts, v)
- if err != nil {
- return err
- }
- if ok {
- return nil
- }
-
+// reflectFieldSave extracts the underlying value of v by reflection,
+// and tries to extract a Property that'll be appended to props.
+func reflectFieldSave(props *[]Property, p Property, name string, opts saveOpts, v reflect.Value) error {
switch x := v.Interface().(type) {
case *Key, time.Time, GeoPoint:
p.Value = x
@@ -81,6 +63,13 @@
p.Value = v.String()
case reflect.Float32, reflect.Float64:
p.Value = v.Float()
+
+ case reflect.Interface:
+ // Extract the interface's underlying value and then retry the save.
+ // See issue https://github.com/googleapis/google-cloud-go/issues/1474.
+ v = v.Elem()
+ return reflectFieldSave(props, p, name, opts, v)
+
case reflect.Slice:
if v.Type().Elem().Kind() == reflect.Uint8 {
p.Value = v.Bytes()
@@ -141,6 +130,7 @@
}
}
}
+
if p.Value == nil {
return fmt.Errorf("datastore: unsupported struct field type: %v", v.Type())
}
@@ -148,6 +138,30 @@
return nil
}
+// TODO(djd): Convert this and below to return ([]Property, error).
+func saveStructProperty(props *[]Property, name string, opts saveOpts, v reflect.Value) error {
+ p := Property{
+ Name: name,
+ NoIndex: opts.noIndex,
+ }
+
+ if opts.omitEmpty && isEmptyValue(v) {
+ return nil
+ }
+
+ // First check if field type implements PLS. If so, use PLS to
+ // save.
+ ok, err := plsFieldSave(props, p, name, opts, v)
+ if err != nil {
+ return err
+ }
+ if ok {
+ return nil
+ }
+
+ return reflectFieldSave(props, p, name, opts, v)
+}
+
// plsFieldSave first tries to converts v's value to a PLS, then v's addressed
// value to a PLS. If neither succeeds, plsFieldSave returns false for first return
// value.
diff --git a/datastore/save_test.go b/datastore/save_test.go
index 36e9fb0..3ca1ca8 100644
--- a/datastore/save_test.go
+++ b/datastore/save_test.go
@@ -287,3 +287,88 @@
}
}
}
+
+func TestSaveFieldsWithInterface(t *testing.T) {
+ // We should be able to extract the underlying value behind an interface.
+ // See issue https://github.com/googleapis/google-cloud-go/issues/1474.
+
+ type n1 struct {
+ Inner interface{}
+ }
+
+ type n2 struct {
+ Inner2 *n1
+ }
+ type n3 struct {
+ N2 interface{}
+ }
+
+ cases := []struct {
+ name string
+ in interface{}
+ want []Property
+ }{
+ {
+ name: "Non-Nil value",
+ in: &struct {
+ Value interface{}
+ ID int
+ key interface{}
+ }{
+ Value: "this is a string",
+ ID: 17,
+ key: "key1",
+ },
+ want: []Property{
+ {Name: "Value", Value: "this is a string"},
+ {Name: "ID", Value: int64(17)},
+ },
+ },
+ {
+ name: "Nil value",
+ in: &struct {
+ foo interface{}
+ }{
+ foo: (*string)(nil),
+ },
+ want: nil,
+ },
+ {
+ name: "Nested",
+ in: &n3{
+ N2: &n2{
+ Inner2: &n1{
+ Inner: "Innest",
+ },
+ },
+ },
+ want: []Property{
+ {
+ Name: "N2",
+ Value: &Entity{
+ Properties: []Property{{
+ Name: "Inner2",
+ Value: &Entity{
+ Properties: []Property{{
+ Name: "Inner", Value: "Innest",
+ }},
+ },
+ }},
+ },
+ },
+ },
+ },
+ }
+
+ for _, tt := range cases {
+ t.Run(tt.name, func(t *testing.T) {
+ got, err := SaveStruct(tt.in)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if diff := testutil.Diff(got, tt.want); diff != "" {
+ t.Fatalf("got - want +\n%s", diff)
+ }
+ })
+ }
+}
