Implement projection queries correctly.

impl/prod/raw_datastore_type_converter.go

Index: impl/prod/raw_datastore_type_converter.go
diff --git a/impl/prod/raw_datastore_type_converter.go b/impl/prod/raw_datastore_type_converter.go
index 0543326492c450ebe6e665b180d17ed741b82f1c..9d7b479beea467ec3faf0e159130200356711a89 100644
--- a/impl/prod/raw_datastore_type_converter.go
+++ b/impl/prod/raw_datastore_type_converter.go
@@ -5,10 +5,13 @@
 package prod
 
 import (
+	"fmt"
+	"reflect"
 	"time"
 
 	bs "github.com/luci/gae/service/blobstore"
 	ds "github.com/luci/gae/service/datastore"
+	"github.com/luci/gae/service/datastore/dskey"
 	"google.golang.org/appengine"
 	"google.golang.org/appengine/datastore"
 )
@@ -19,13 +22,89 @@ type typeFilter struct {
 
 var _ datastore.PropertyLoadSaver = &typeFilter{}
 
+func maybeIndexValue(val interface{}) interface{} {

[iannucci, 2015/09/10 03:56:58]

This bit of grossness is to extract values from this sucker
https://github.com/golang/appengine/blob/master/datastore/load.go#L324 which
actually just holds a value pointing at
https://github.com/golang/appengine/blob/master/internal/datastore/datastore_...

It would be nice if the SDK exposed a method on this to just get its essential
value. At least the SDK doesn't actively block Projection queries loading into a
PropertyList.

+	// It may be the SDK's datastore.indexValue structure (in datastore/load.go).
+	//
+	// Since this is a private type with no methods, we need to use reflection
+	// to get the data out. Ick.
+	rv := reflect.ValueOf(val)
+	if rv.Kind() == reflect.Struct && rv.Type().String() == "datastore.indexValue" {
+		rv = rv.FieldByName("value")
+		if rv.IsValid() && rv.Kind() == reflect.Ptr {
+			// TODO(riannucci): determine if this is how nil IndexValues are stored.
+			// Maybe they're encoded as a PropertyValue with all-nil fields instead?
+			if rv.IsNil() {
+				return nil
+			}
+			rv = rv.Elem()
+			// we're in protobuf land now.
+			if rv.Type().Name() == "PropertyValue" {
+				for i := 0; i < rv.NumField(); i++ {
+					field := rv.Field(i)
+					if field.Kind() == reflect.Ptr {
+						if field.IsNil() {
+							continue
+						}
+						field = field.Elem()
+						switch field.Kind() {
+						case reflect.Int64:
+							return field.Int()
+						case reflect.String:
+							return field.String()
+						case reflect.Bool:
+							return field.Bool()
+						case reflect.Float64:
+							return field.Float()
+						}
+						switch field.Type().Name() {
+						case "PropertyValue_PointValue":
+							// Lat == X, Lng == Y b/c historical resons.
+							return ds.GeoPoint{
+								Lat: field.FieldByName("X").Float(),
+								Lng: field.FieldByName("Y").Float()}
+						case "PropertyValue_ReferenceValue":
+							aid := field.FieldByName("App").Elem().String()
+							ns := ""
+							if nsf := field.FieldByName("NameSpace"); !nsf.IsNil() {
+								ns = nsf.Elem().String()
+							}
+							elems := field.FieldByName("Pathelement")
+							toks := make([]ds.KeyTok, elems.Len())
+							for i := range toks {
+								e := elems.Index(i).Elem()
+								toks[i].Kind = e.FieldByName("Type").Elem().String()
+								if iid := e.FieldByName("Id"); !iid.IsNil() {
+									toks[i].IntID = iid.Elem().Int()
+								}
+								if sid := e.FieldByName("Name"); !sid.IsNil() {
+									toks[i].StringID = sid.Elem().String()
+								}
+							}
+							return dskey.NewToks(aid, ns, toks)
+						}
+						panic(fmt.Errorf(
+							"UNKNOWN datastore.indexValue field type: %s", field.Type()))
+					}
+					// there's also the `XXX_unrecognized []byte` field, so don't panic
+					// here.
+				}
+				panic(fmt.Errorf("cannot decode datastore.indexValue (no recognize field): %v", val))
+			}
+			panic(fmt.Errorf("cannot decode datastore.indexValue (wrong inner type): %v", val))
+		}
+		panic(fmt.Errorf("cannot decode datastore.indexValue: %v", val))
+	} else {
+		return val
+	}
+}
+
 func (tf *typeFilter) Load(props []datastore.Property) error {
 	tf.pm = make(ds.PropertyMap, len(props))
 	for _, p := range props {
 		val := p.Value
 		switch x := val.(type) {
 		case datastore.ByteString:
-			val = ds.ByteString(x)
+			val = []byte(x)
 		case *datastore.Key:
 			val = dsR2F(x)
 		case appengine.BlobKey:
@@ -35,6 +114,8 @@ func (tf *typeFilter) Load(props []datastore.Property) error {
 		case time.Time:
 			// "appengine" layer instantiates with Local timezone.
 			val = x.UTC()
+		default:
+			val = maybeIndexValue(val)
 		}
 		prop := ds.Property{}
 		is := ds.ShouldIndex
@@ -62,17 +143,22 @@ func (tf *typeFilter) Save() ([]datastore.Property, error) {
 				Multiple: multiple,
 				NoIndex:  prop.IndexSetting() == ds.NoIndex,
 			}
-			switch x := prop.Value().(type) {
-			case ds.ByteString:
-				toAdd.Value = datastore.ByteString(x)
-			case ds.Key:
-				toAdd.Value = dsF2R(x)
-			case bs.Key:
-				toAdd.Value = appengine.BlobKey(x)
-			case ds.GeoPoint:
-				toAdd.Value = appengine.GeoPoint(x)
+			switch prop.Type() {
+			case ds.PTBytes:
+				v := prop.Value().([]byte)
+				if prop.IndexSetting() == ds.ShouldIndex {
+					toAdd.Value = datastore.ByteString(v)
+				} else {
+					toAdd.Value = v
+				}

[iannucci, 2015/09/10 03:56:58]

This is now the only place where we have to deal with the ByteString v. []byte
split. I think it "seemed" like []byte being noindex by default would be
'convenient', but really it's just a pain in the butt (and it makes structs
harder to read, because the exception is easy to forget).

+			case ds.PTKey:
+				toAdd.Value = dsF2R(prop.Value().(ds.Key))
+			case ds.PTBlobKey:
+				toAdd.Value = appengine.BlobKey(prop.Value().(bs.Key))
+			case ds.PTGeoPoint:
+				toAdd.Value = appengine.GeoPoint(prop.Value().(ds.GeoPoint))
 			default:
-				toAdd.Value = x
+				toAdd.Value = prop.Value()
 			}
 			props = append(props, toAdd)
 		}