Correctly prevent Put with an empty string id

entity.go

@@ -46,13 +46,17 @@ func fromGob(src interface{}, b []byte) error {
 	return nil
 }
 
-func (g *Goon) getStructKey(src interface{}) (*datastore.Key, error) {
+// getStructKey returns the key of the struct based in its reflected or
+// specified kind and id. The second return parameter is true if src has a
+// string id.
+func (g *Goon) getStructKey(src interface{}) (key *datastore.Key, hasStringId bool, err error) {
 	v := reflect.Indirect(reflect.ValueOf(src))
 	t := v.Type()
 	k := t.Kind()
 
 	if k != reflect.Struct {
-		return nil, fmt.Errorf("goon: Expected struct, got instead: %v", k)
+		err = fmt.Errorf("goon: Expected struct, got instead: %v", k)
+		return
 	}
 
 	var parent *datastore.Key
@@ -72,21 +76,26 @@ func (g *Goon) getStructKey(src interface{}) (*datastore.Key, error) {
 				switch vf.Kind() {
 				case reflect.Int64:
 					if intID != 0 || stringID != "" {
-						return nil, fmt.Errorf("goon: Only one field may be marked id")
+						err = fmt.Errorf("goon: Only one field may be marked id")
+						return
 					}
 					intID = vf.Int()
 				case reflect.String:
 					if intID != 0 || stringID != "" {
-						return nil, fmt.Errorf("goon: Only one field may be marked id")
+						err = fmt.Errorf("goon: Only one field may be marked id")
+						return
 					}
 					stringID = vf.String()
+					hasStringId = true
 				default:
-					return nil, fmt.Errorf("goon: ID field must be int64 or string in %v", t.Name())
+					err = fmt.Errorf("goon: ID field must be int64 or string in %v", t.Name())
+					return
 				}
 			} else if tagValue == "kind" {
 				if vf.Kind() == reflect.String {
 					if kind != "" {
-						return nil, fmt.Errorf("goon: Only one field may be marked kind")
+						err = fmt.Errorf("goon: Only one field may be marked kind")
+						return
 					}
 					kind = vf.String()
 					if kind == "" && len(tagValues) > 1 && tagValues[1] != "" {
@@ -96,7 +105,8 @@ func (g *Goon) getStructKey(src interface{}) (*datastore.Key, error) {
 			} else if tagValue == "parent" {
 				if vf.Type() == reflect.TypeOf(&datastore.Key{}) {
 					if parent != nil {
-						return nil, fmt.Errorf("goon: Only one field may be marked parent")
+						err = fmt.Errorf("goon: Only one field may be marked parent")
+						return
 					}
 					parent = vf.Interface().(*datastore.Key)
 				}
@@ -108,8 +118,8 @@ func (g *Goon) getStructKey(src interface{}) (*datastore.Key, error) {
 	if kind == "" {
 		kind = typeName(src)
 	}
-	// can be an incomplete Key but not for String Id objects
-	return datastore.NewKey(g.context, kind, stringID, intID, parent), nil
+	key = datastore.NewKey(g.context, kind, stringID, intID, parent)
+	return
 }
 
 func typeName(src interface{}) string {

goon.go

@@ -78,7 +78,7 @@ func (g *Goon) error(err error) {
 	}
 }
 
-func (g *Goon) extractKeys(src interface{}, allowIncomplete bool) ([]*datastore.Key, error) {
+func (g *Goon) extractKeys(src interface{}, putRequest bool) ([]*datastore.Key, error) {
 	v := reflect.Indirect(reflect.ValueOf(src))
 	if v.Kind() != reflect.Slice {
 		return nil, fmt.Errorf("goon: value must be a slice or pointer-to-slice")
@@ -88,12 +88,14 @@ func (g *Goon) extractKeys(src interface{}, allowIncomplete bool) ([]*datastore.
 	keys := make([]*datastore.Key, l)
 	for i := 0; i < l; i++ {
 		vi := v.Index(i)
-		key, err := g.getStructKey(vi.Interface())
+		key, hasStringId, err := g.getStructKey(vi.Interface())
 		if err != nil {
 			return nil, err
 		}
-		if !allowIncomplete && key.Incomplete() {
+		if !putRequest && key.Incomplete() {
 			return nil, fmt.Errorf("goon: cannot find a key for struct - %v", vi.Interface())
+		} else if putRequest && key.Incomplete() && hasStringId {
+			return nil, fmt.Errorf("goon: empty string id on put")
 		}
 		keys[i] = key
 	}
@@ -121,7 +123,8 @@ func (g *Goon) Kind(src interface{}) string {
 
 // KeyError returns the key of src based on its properties.
 func (g *Goon) KeyError(src interface{}) (*datastore.Key, error) {
-	return g.getStructKey(src)
+	key, _, err := g.getStructKey(src)
+	return key, err
 }
 
 // RunInTransaction runs f in a transaction. It calls f with a transaction
@@ -233,7 +236,7 @@ func (g *Goon) putMemoryMulti(src interface{}) {
 }
 
 func (g *Goon) putMemory(src interface{}) {
-	key, _ := g.getStructKey(src)
+	key, _, _ := g.getStructKey(src)
 	g.cacheLock.Lock()
 	defer g.cacheLock.Unlock()
 	g.cache[memkey(key)] = src
@@ -255,7 +258,7 @@ func (g *Goon) putMemcache(srcs []interface{}) error {
 			g.error(err)
 			return err
 		}
-		key, err := g.getStructKey(src)
+		key, _, err := g.getStructKey(src)
 		if err != nil {
 			return err
 		}