datastore: save is unable to extract underlying values behind fields of type interface

[Sheshagiri]

I'm not sure if this is expected to work, do let me know if I'm missing anything here.

The issue is an unmarshalled proto message with a field of type google.protobuf.Value is not being persisted to cloud datastore.

Expected Behavior

unmarshalled protobuf object should be saved in the datastore.

Actual Behavior

following is the error

2019/06/24 21:46:11 failed to save the user: datastore: unsupported struct field type: structpb.isValue_Kind

proto file can be found here
https://github.com/Sheshagiri/go-protobuf-cloud-datastore/blob/master/models/user.proto

However for requests like

$ curl --request POST \
    --url http://localhost:8080/api/v1/users \
    --header 'content-type: application/json' \
    --data '{
  	"username":"Steve",
  	"first_name":"Steve",
  	"last_name":"Jobs"
  }'

things work fine. I see the error only when I inclue the location field

$ curl --request POST \
    --url http://localhost:8080/api/v1/users \
    --header 'content-type: application/json' \
    --data '{
	"username": "tim",
	"first_name": "Tim",
	"last_name": "Cook",
	"location": {
		"objectType": "place",
		"displayName": "Denver, CO"
	}
}'

[jeanbza]

Sorry, just to back up a bit: are you using one of the clients defined in this package? And if so, could you post the Go code showing your usage, and a complete Go stacktrace of the error?

If not using the Go clients defined in this package, I'm happy to direct you to the relevant Datastore location for filing questions.

[Sheshagiri]

@jadekler yes the datastore client present in this repo(AFAIK).
my go code is available here
https://github.com/Sheshagiri/go-protobuf-cloud-datastore/blob/70c9cdfdab7140b1089bf514b1191f24ccbd0573/models/user.go#L15

and this is the only error I see

datastore: unsupported struct field type: structpb.isValue_Kind

[jeanbza]

Thank you! I vaguely recall something about protos not working, but someone should go look and see what's going on. Un-assigning myself til either I or someone else finds time to do so.

[odeke-em]

Thank you for reporting this issue @Sheshagiri and for the response @jadekler!

So the proto not being supported is just a misnomer, the real issue is that when we encounter a field with that is an interface, we aren't trying to derefence the underlying value thus don't support fields of type interface even when the underlying value is saveable e.g. string, and here is a minimal repro

package main

import (
        "context"
        "log"

        "cloud.google.com/go/datastore"
)

func main() {
        ctx := context.Background()
        projectID := "odeke-sandbox"
        client, err := datastore.NewClient(ctx, projectID)
        if err != nil {
                log.Fatalf("Failed to create datastore client: %v", err)
        }
        defer client.Close()

        type Spot struct {
                Name     string      `datastore:"name"`
                Location interface{} `datastore:"location"`
        }

        newKey := datastore.IncompleteKey("Spot", nil)
        pk, err := client.Put(ctx, newKey, &Spot{
                Name:     "foo",
                Location: "bar",
        })
        if err != nil {
                log.Printf("Failed to insert key: %v", err)
                return
        }
        log.Printf("Inserted key: %v\n", pk)
}

which will log

2019/07/12 14:35:25 Failed to insert key: datastore: unsupported struct field type: interface {}

I believe the fix for this issue requires a case of considering reflect.Interface

diff --git a/datastore/load.go b/datastore/load.go
index e75ccef4..7b752919 100644
--- a/datastore/load.go
+++ b/datastore/load.go
@@ -292,6 +292,11 @@ func setVal(v reflect.Value, p Property) (s string) {
                        return overflowReason(x, v)
                }
                v.SetFloat(x)
+
+       case reflect.Interface:
+               // An interface can be set to any value regardless of the type.
+               v.Set(reflect.ValueOf(pValue))
+
        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/save.go b/datastore/save.go
index 2a444699..4f8ca81a 100644
--- a/datastore/save.go
+++ b/datastore/save.go
@@ -68,6 +68,7 @@ func saveStructProperty(props *[]Property, name string, opts saveOpts, v reflect
                return nil
        }
 
+extract:
        switch x := v.Interface().(type) {
        case *Key, time.Time, GeoPoint:
                p.Value = x
@@ -81,6 +82,14 @@ func saveStructProperty(props *[]Property, name string, opts saveOpts, v reflect
                        p.Value = v.String()
                case reflect.Float32, reflect.Float64:
                        p.Value = v.Float()
+
+               case reflect.Interface:
+                       // Extract the underlying value and
+                       // then try dereferencing again.
+                       // See Issue https://github.com/googleapis/google-cloud-go/issues/1474
+                       v = v.Elem()
+                       goto extract
+
                case reflect.Slice:
                        if v.Type().Elem().Kind() == reflect.Uint8 {
                                p.Value = v.Bytes()

in datastore/save.go in function saveStructProperty to extract the underlying value.

[odeke-em]

So while the issue of interfaces not being extracted is trivially solvable, @Sheshagiri the one that'll block your issue is that _struct.StructValue.Fields is a map and maps can't be serialized directly to datastore and would require your struct to manually implement the PropertyLoadSaver interface https://godoc.org/cloud.google.com/go/datastore#PropertyLoadSaver to marshal and unmarshal the data which is nested inside

[Sheshagiri]

@odeke-em thanks for picking this. To give you some background we were exploring the option of persisting protobuf's to google cloud datastore hence this issue of

proto message with a field of type google.protobuf.Value is not being persisted to cloud datastore.

After looking at the datastore protobuf's and the datastore go client library I noticed that some pieces were missing, so I did a hackish implementation here https://github.com/Sheshagiri/go-protobuf-cloud-datastore-entity-translator. I did in 2 steps

1. a translator that takes any proto.Message and translates it to datastore.Entity and vice-versa
2. I noticed that the datastore go client library doesn't expose functions to put and get the entities directly so I added those functions in my fork here https://github.com/Sheshagiri/google-cloud-go

the translator helps avoid implementing https://godoc.org/cloud.google.com/go/datastore#PropertyLoadSaver for each protobuf.

[odeke-em]

@Sheshagiri the CL that I mailed for unfurling the interface is at https://code-review.googlesource.com/c/gocloud/+/42771 but the other part which we don't support is marshaling and unmarshaling maps.