Implement projection queries correctly.

service/datastore/pls_impl.go

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // HEAVILY adapted from github.com/golang/appengine/datastore
 
 package datastore
 
 import (
         "fmt"
         "reflect"
         "strconv"
         "strings"
         "sync"
-        "time"
         "unicode"
 
-        "github.com/luci/gae/service/blobstore"
         "github.com/luci/luci-go/common/errors"
 )
 
 // Entities with more than this many indexed properties will not be saved.
 const maxIndexedProperties = 20000
 
 type structTag struct {
         name           string
         idxSetting     IndexSetting
         isSlice        bool
@@ skipping 102 matching lines @@
         }
 
         var slice reflect.Value
         if v.Kind() == reflect.Slice && v.Type().Elem().Kind() != reflect.Uint8 {
                 slice = v
                 v = reflect.New(v.Type().Elem()).Elem()
         } else if requireSlice {
                 return "multiple-valued property requires a slice field type"
         }
 
-        pVal := p.Value()
-
         if ret, ok := doConversion(v); ok {
                 if ret != "" {
                         return ret
                 }
         } else {
                 knd := v.Kind()
                 if v.Type().Implements(typeOfKey) {
                         knd = reflect.Interface
                 }
+
+                project := PTNull
+                overflow := (func(interface{}) bool)(nil)
+                set := (func(interface{}))(nil)
+
                 switch knd {
                 case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
-                        x, ok := pVal.(int64)
-                        if !ok && pVal != nil {
-                                return typeMismatchReason(pVal, v)
-                        }
-                        if v.OverflowInt(x) {
-                                return fmt.Sprintf("value %v overflows struct field of type %v", x, v.Type())
-                        }
-                        v.SetInt(x)
+                        project = PTInt
+                        overflow = func(x interface{}) bool { return v.OverflowInt(x.(int64)) }
+                        set = func(x interface{}) { v.SetInt(x.(int64)) }
                 case reflect.Bool:
-                        x, ok := pVal.(bool)
-                        if !ok && pVal != nil {
-                                return typeMismatchReason(pVal, v)
-                        }
-                        v.SetBool(x)
+                        project = PTBool
+                        set = func(x interface{}) { v.SetBool(x.(bool)) }
                 case reflect.String:
-                        switch x := pVal.(type) {
-                        case blobstore.Key:
-                                v.SetString(string(x))
-                        case string:
-                                v.SetString(x)
-                        default:
-                                if pVal != nil {
-                                        return typeMismatchReason(pVal, v)
+                        project = PTString
+                        set = func(x interface{}) { v.SetString(x.(string)) }
+                case reflect.Float32, reflect.Float64:
+                        project = PTFloat
+                        overflow = func(x interface{}) bool { return v.OverflowFloat(x.(float64)) }
+                        set = func(x interface{}) { v.SetFloat(x.(float64)) }
+                case reflect.Interface:

[dnj (Google), 2015/09/10 16:26:11]

Before you were checking if the type was Key. Is being a reflect.Interface
sufficient to assume it's a Key now, or should that check be re-added?

[iannucci, 2015/09/10 17:29:21]

No I wasn't. I was casting the property value to a Key. Projecting to PTKey is
the same thing. If this interface was anything else, the codec would have a
permanent error (or this field would have been handled above by doConversion).

+                        project = PTKey
+                        set = func(x interface{}) {
+                                if k, ok := x.(Key); ok {
+                                        v.Set(reflect.ValueOf(k))
                                 }
                         }
-                case reflect.Float32, reflect.Float64:
-                        x, ok := pVal.(float64)
-                        if !ok && pVal != nil {
-                                return typeMismatchReason(pVal, v)
-                        }
-                        if v.OverflowFloat(x) {
-                                return fmt.Sprintf("value %v overflows struct field of type %v", x, v.Type())
-                        }
-                        v.SetFloat(x)
-                case reflect.Interface:
-                        x, ok := pVal.(Key)
-                        if !ok && pVal != nil {
-                                return typeMismatchReason(pVal, v)
-                        }
-                        if x != nil {
-                                v.Set(reflect.ValueOf(x))
-                        }
                 case reflect.Struct:
                         switch v.Type() {
                         case typeOfTime:
-                                x, ok := pVal.(time.Time)
-                                if !ok && pVal != nil {
-                                        return typeMismatchReason(pVal, v)
-                                }
-                                v.Set(reflect.ValueOf(x))
+                                project = PTTime
+                                set = func(x interface{}) { v.Set(reflect.ValueOf(x)) }
                         case typeOfGeoPoint:
-                                x, ok := pVal.(GeoPoint)
-                                if !ok && pVal != nil {
-                                        return typeMismatchReason(pVal, v)
-                                }
-                                v.Set(reflect.ValueOf(x))
+                                project = PTGeoPoint
+                                set = func(x interface{}) { v.Set(reflect.ValueOf(x)) }
                         default:
-                                panic(fmt.Errorf("helper: impossible: %s", typeMismatchReason(pVal, v)))
+                                panic(fmt.Errorf("helper: impossible: %s", typeMismatchReason(p.value, v)))
                         }
                 case reflect.Slice:
-                        switch x := pVal.(type) {
-                        case []byte:
-                                v.SetBytes(x)
-                        case ByteString:
-                                v.SetBytes([]byte(x))
-                        default:
-                                panic(fmt.Errorf("helper: impossible: %s", typeMismatchReason(pVal, v)))
+                        project = PTBytes
+                        set = func(x interface{}) {
+                                v.SetBytes(reflect.ValueOf(x).Bytes())
                         }
                 default:
-                        panic(fmt.Errorf("helper: impossible: %s", typeMismatchReason(pVal, v)))
+                        panic(fmt.Errorf("helper: impossible: %s", typeMismatchReason(p.value, v)))
                 }
+
+                pVal, err := p.Project(project)
+                if err != nil {
+                        return typeMismatchReason(p.value, v)
+                }
+                if overflow != nil && overflow(pVal) {
+                        return fmt.Sprintf("value %v overflows struct field of type %v", pVal, v.Type())
+                }
+                set(pVal)
         }
         if slice.IsValid() {
                 slice.Set(reflect.Append(slice, v))
         }
         return ""
 }
 
 func (p *structPLS) Save(withMeta bool) (PropertyMap, error) {
         size := len(p.c.byName)
         if withMeta {
@@ skipping 360 matching lines @@
                 switch val {
                 case "on", "On", "true":
                         return true, nil
                 case "off", "Off", "false":
                         return false, nil
                 }
                 return nil, fmt.Errorf("Toggle field has bad/missing default, got %q", val)
         }
         return nil, fmt.Errorf("helper: meta field with bad type/value %s/%q", t, val)
 }