Resolve several panics, incorrect reflection uses, and eager bugs

associations/association.go

@@ -160,5 +160,9 @@ func fieldIsNil(f reflect.Value) bool {
 
 // IsZeroOfUnderlyingType will check if the value of anything is the equal to the Zero value of that type.
 func IsZeroOfUnderlyingType(x interface{}) bool {
+	if x == nil {
+		return true
+	}
+
 	return reflect.DeepEqual(x, reflect.Zero(reflect.TypeOf(x)).Interface())
 }

associations/association_test.go

@@ -0,0 +1,37 @@
+package associations
+
+import (
+	"database/sql"
+	"fmt"
+	"github.com/gobuffalo/nulls"
+	"github.com/gofrs/uuid"
+	"github.com/stretchr/testify/assert"
+	"testing"
+)
+
+func Test_IsZeroOfUnderlyingType(t *testing.T) {
+	for k, tc := range []struct {
+		in   interface{}
+		zero bool
+	}{
+		{in: nil, zero: true},
+		{in: 0, zero: true},
+		{in: 1, zero: false},
+		{in: false, zero: true},
+		{in: "", zero: true},
+		{in: interface{}(nil), zero: true},
+		{in: uuid.NullUUID{}, zero: true},
+		{in: uuid.UUID{}, zero: true},
+		{in: uuid.NullUUID{Valid: true}, zero: false},
+		{in: nulls.Int{}, zero: true},
+		{in: nulls.String{}, zero: true},
+		{in: nulls.Bool{}, zero: true},
+		{in: nulls.Float64{}, zero: true},
+		{in: sql.NullString{}, zero: true},
+		{in: sql.NullString{Valid: true}, zero: false},
+	} {
+		t.Run(fmt.Sprintf("case=%d", k), func(t *testing.T) {
+			assert.EqualValues(t, tc.zero, IsZeroOfUnderlyingType(tc.in))
+		})
+	}
+}

associations/associations_for_struct.go

@@ -1,7 +1,6 @@
 package associations
 
 import (
-	"errors"
 	"fmt"
 	"reflect"
 	"regexp"
@@ -30,7 +29,7 @@ var associationBuilders = map[string]associationBuilder{}
 func ForStruct(s interface{}, fields ...string) (Associations, error) {
 	t, v := getModelDefinition(s)
 	if t.Kind() != reflect.Struct {
-		return nil, errors.New("could not get struct associations: not a struct")
+		return nil, fmt.Errorf("could not get struct associations: not a struct but %T", s)
 	}
 	fields = trimFields(fields)
 	associations := Associations{}

associations/belongs_to_association.go

@@ -128,15 +128,22 @@ func (b *belongsToAssociation) BeforeInterface() interface{} {
 
 func (b *belongsToAssociation) BeforeSetup() error {
 	ownerID := reflect.Indirect(reflect.ValueOf(b.ownerModel.Interface())).FieldByName("ID")
-	if b.ownerID.CanSet() {
-		if n := nulls.New(b.ownerID.Interface()); n != nil {
-			b.ownerID.Set(reflect.ValueOf(n.Parse(ownerID.Interface())))
-		} else if b.ownerID.Kind() == reflect.Ptr {
-			b.ownerID.Set(ownerID.Addr())
+	toSet := b.ownerID
+	switch b.ownerID.Type().Name() {
+	case "NullUUID":
+		b.ownerID.FieldByName("Valid").Set(reflect.ValueOf(true))
+		toSet = b.ownerID.FieldByName("UUID")
+	}
+
+	if toSet.CanSet() {
+		if n := nulls.New(toSet.Interface()); n != nil {
+			toSet.Set(reflect.ValueOf(n.Parse(ownerID.Interface())))
+		} else if toSet.Kind() == reflect.Ptr {
+			toSet.Set(ownerID.Addr())
 		} else {
-			b.ownerID.Set(ownerID)
+			toSet.Set(ownerID)
 		}
 		return nil
 	}
-	return fmt.Errorf("could not set '%s' to '%s'", ownerID, b.ownerID)
+	return fmt.Errorf("could not set '%s' to '%s'", ownerID, toSet)
 }

associations/belongs_to_association_test.go

@@ -22,6 +22,11 @@ type barBelongsTo struct {
 	Foo   fooBelongsTo `belongs_to:"foo"`
 }
 
+type barBelongsToNullable struct {
+	FooID uuid.NullUUID `db:"foo_id"`
+	Foo   *fooBelongsTo `belongs_to:"foo"`
+}
+
 func Test_Belongs_To_Association(t *testing.T) {
 	a := require.New(t)
 
@@ -50,3 +55,17 @@ func Test_Belongs_To_Association(t *testing.T) {
 		a.Equal(nil, before[index].BeforeInterface())
 	}
 }
+
+func Test_Belongs_To_Nullable_Association(t *testing.T) {
+	a := require.New(t)
+	id, _ := uuid.NewV1()
+
+	bar := barBelongsToNullable{Foo: &fooBelongsTo{id}}
+	as, err := associations.ForStruct(&bar, "Foo")
+	a.NoError(err)
+
+	before := as.AssociationsBeforeCreatable()
+	for index := range before {
+		a.Equal(nil, before[index].BeforeSetup())
+	}
+}

finders.go

@@ -282,7 +282,14 @@ func (q *Query) eagerDefaultAssociations(model interface{}) error {
 			v = reflect.Indirect(reflect.ValueOf(model)).FieldByName(inner.Name)
 			innerQuery := Q(query.Connection)
 			innerQuery.eagerFields = inner.Fields
-			err = innerQuery.eagerAssociations(v.Addr().Interface())
+
+			switch v.Kind() {
+			case reflect.Ptr:
+				err = innerQuery.eagerAssociations(v.Interface())
+			default:
+				err = innerQuery.eagerAssociations(v.Addr().Interface())
+			}
+
 			if err != nil {
 				return err
 			}

finders_test.go

@@ -314,6 +314,42 @@ func Test_Find_Eager_Has_One(t *testing.T) {
 	})
 }
 
+func Test_Find_Eager_Has_One_With_Inner_Associations_Pointer(t *testing.T) {
+	if PDB == nil {
+		t.Skip("skipping integration tests")
+	}
+	transaction(func(tx *Connection) {
+		r := require.New(t)
+
+		user := UserPointerAssocs{Name: nulls.NewString("Mark")}
+		err := tx.Create(&user)
+		r.NoError(err)
+
+		coolSong := Song{Title: "Hook - Blues Traveler", UserID: user.ID}
+		err = tx.Create(&coolSong)
+		r.NoError(err)
+
+		u := UserPointerAssocs{}
+		err = tx.Eager("FavoriteSong.ComposedBy").Find(&u, user.ID)
+		r.NoError(err)
+
+		r.NotEqual(u.ID, 0)
+		r.Equal(u.Name.String, "Mark")
+		r.Equal(u.FavoriteSong.ID, coolSong.ID)
+
+		// eager should work with rawquery
+		uid := u.ID
+		u = UserPointerAssocs{}
+		err = tx.RawQuery("select * from users where id=?", uid).First(&u)
+		r.NoError(err)
+		r.Nil(u.FavoriteSong)
+
+		err = tx.RawQuery("select * from users where id=?", uid).Eager("FavoriteSong").First(&u)
+		r.NoError(err)
+		r.Equal(u.FavoriteSong.ID, coolSong.ID)
+	})
+}
+
 func Test_Find_Eager_Has_One_With_Inner_Associations_Struct(t *testing.T) {
 	if PDB == nil {
 		t.Skip("skipping integration tests")

pop_test.go

@@ -106,6 +106,27 @@ type User struct {
 	Houses       Addresses     `many_to_many:"users_addresses"`
 }
 
+type UserPointerAssocs struct {
+	ID           int           `db:"id"`
+	UserName     string        `db:"user_name"`
+	Email        string        `db:"email"`
+	Name         nulls.String  `db:"name"`
+	Alive        nulls.Bool    `db:"alive"`
+	CreatedAt    time.Time     `db:"created_at"`
+	UpdatedAt    time.Time     `db:"updated_at"`
+	BirthDate    nulls.Time    `db:"birth_date"`
+	Bio          nulls.String  `db:"bio"`
+	Price        nulls.Float64 `db:"price"`
+	FullName     nulls.String  `db:"full_name" select:"name as full_name"`
+	Books        Books         `has_many:"books" order_by:"title asc" fk_id:"user_id"`
+	FavoriteSong *Song         `has_one:"song" fk_id:"u_id"`
+	Houses       Addresses     `many_to_many:"users_addresses"`
+}
+
+func (UserPointerAssocs) TableName() string {
+	return "users"
+}
+
 // Validate gets run every time you call a "Validate*" (ValidateAndSave, ValidateAndCreate, ValidateAndUpdate) method.
 // This method is not required and may be deleted.
 func (u *User) Validate(tx *Connection) (*validate.Errors, error) {
@@ -277,6 +298,23 @@ type CourseCode struct {
 	// Course Course `belongs_to:"course"`
 }
 
+type NetClient struct {
+	ID   uuid.UUID `json:"id" db:"id"`
+	Hops []Hop     `json:"hop_id" has_many:"hops"`
+}
+
+type Hop struct {
+	ID          uuid.UUID     `json:"id" db:"id"`
+	NetClient   *NetClient    `json:"net_client" belongs_to:"net_client" fk_id:"NetClientID"`
+	NetClientID uuid.UUID     `json:"net_client_id" db:"net_client_id"`
+	Server      *Server       `json:"course" belongs_to:"server" fk_id:"ServerID" oder_by:"id asc"`
+	ServerID    uuid.NullUUID `json:"server_id" db:"server_id"`
+}
+
+type Server struct {
+	ID uuid.UUID `json:"id" db:"id"`
+}
+
 type ValidatableCar struct {
 	ID        int64     `db:"id"`
 	Name      string    `db:"name"`

preload_associations.go

@@ -268,13 +268,18 @@ func preloadHasMany(tx *Connection, asoc *AssociationMetaInfo, mmi *ModelMetaInf
 	// 3) iterate over every model and fill it with the assoc.
 	foreignField := asoc.getDBFieldTaggedWith(fk)
 	mmi.iterate(func(mvalue reflect.Value) {
-		modelAssociationField := mmi.mapper.FieldByName(mvalue, asoc.Name)
 		for i := 0; i < slice.Elem().Len(); i++ {
 			asocValue := slice.Elem().Index(i)
 			valueField := reflect.Indirect(mmi.mapper.FieldByName(asocValue, foreignField.Path))
 			if mmi.mapper.FieldByName(mvalue, "ID").Interface() == valueField.Interface() ||
 				reflect.DeepEqual(mmi.mapper.FieldByName(mvalue, "ID"), valueField) {
-
+				// IMPORTANT
+				//
+				// FieldByName will initialize the value. It is important that this happens AFTER
+				// we checked whether the field should be set. Otherwise, we'll set a zero value!
+				//
+				// This is most likely the reason for https://github.com/gobuffalo/pop/issues/139
+				modelAssociationField := mmi.mapper.FieldByName(mvalue, asoc.Name)
 				switch {
 				case modelAssociationField.Kind() == reflect.Slice || modelAssociationField.Kind() == reflect.Array:
 					modelAssociationField.Set(reflect.Append(modelAssociationField, asocValue))
@@ -330,16 +335,25 @@ func preloadHasOne(tx *Connection, asoc *AssociationMetaInfo, mmi *ModelMetaInfo
 	//  3) iterate over every model and fill it with the assoc.
 	foreignField := asoc.getDBFieldTaggedWith(fk)
 	mmi.iterate(func(mvalue reflect.Value) {
-		modelAssociationField := mmi.mapper.FieldByName(mvalue, asoc.Name)
 		for i := 0; i < slice.Elem().Len(); i++ {
 			asocValue := slice.Elem().Index(i)
 			if mmi.mapper.FieldByName(mvalue, "ID").Interface() == mmi.mapper.FieldByName(asocValue, foreignField.Path).Interface() ||
 				reflect.DeepEqual(mmi.mapper.FieldByName(mvalue, "ID"), mmi.mapper.FieldByName(asocValue, foreignField.Path)) {
-				if modelAssociationField.Kind() == reflect.Slice || modelAssociationField.Kind() == reflect.Array {
+				// IMPORTANT
+				//
+				// FieldByName will initialize the value. It is important that this happens AFTER
+				// we checked whether the field should be set. Otherwise, we'll set a zero value!
+				//
+				// This is most likely the reason for https://github.com/gobuffalo/pop/issues/139
+				modelAssociationField := mmi.mapper.FieldByName(mvalue, asoc.Name)
+				switch {
+				case modelAssociationField.Kind() == reflect.Slice || modelAssociationField.Kind() == reflect.Array:
 					modelAssociationField.Set(reflect.Append(modelAssociationField, asocValue))
-					continue
+				case modelAssociationField.Kind() == reflect.Ptr:
+					modelAssociationField.Elem().Set(asocValue)
+				default:
+					modelAssociationField.Set(asocValue)
 				}
-				modelAssociationField.Set(asocValue)
 			}
 		}
 	})
@@ -392,13 +406,18 @@ func preloadBelongsTo(tx *Connection, asoc *AssociationMetaInfo, mmi *ModelMetaI
 		if isFieldNilPtr(mvalue, fi) {
 			return
 		}
-		modelAssociationField := mmi.mapper.FieldByName(mvalue, asoc.Name)
 		for i := 0; i < slice.Elem().Len(); i++ {
 			asocValue := slice.Elem().Index(i)
 			fkField := reflect.Indirect(mmi.mapper.FieldByName(mvalue, fi.Path))
-			if fkField.Interface() == mmi.mapper.FieldByName(asocValue, "ID").Interface() ||
-				reflect.DeepEqual(fkField, mmi.mapper.FieldByName(asocValue, "ID")) {
-
+			field := mmi.mapper.FieldByName(asocValue, "ID")
+			if fkField.Interface() == field.Interface() || reflect.DeepEqual(fkField, field) {
+				// IMPORTANT
+				//
+				// FieldByName will initialize the value. It is important that this happens AFTER
+				// we checked whether the field should be set. Otherwise, we'll set a zero value!
+				//
+				// This is most likely the reason for https://github.com/gobuffalo/pop/issues/139
+				modelAssociationField := mmi.mapper.FieldByName(mvalue, asoc.Name)
 				switch {
 				case modelAssociationField.Kind() == reflect.Slice || modelAssociationField.Kind() == reflect.Array:
 					modelAssociationField.Set(reflect.Append(modelAssociationField, asocValue))
@@ -497,11 +516,17 @@ func preloadManyToMany(tx *Connection, asoc *AssociationMetaInfo, mmi *ModelMeta
 	mmi.iterate(func(mvalue reflect.Value) {
 		id := mmi.mapper.FieldByName(mvalue, "ID").Interface()
 		if assocFkIds, ok := mapAssoc[fmt.Sprintf("%v", id)]; ok {
-			modelAssociationField := mmi.mapper.FieldByName(mvalue, asoc.Name)
 			for i := 0; i < slice.Elem().Len(); i++ {
 				asocValue := slice.Elem().Index(i)
 				for _, fkid := range assocFkIds {
 					if fmt.Sprintf("%v", fkid) == fmt.Sprintf("%v", mmi.mapper.FieldByName(asocValue, "ID").Interface()) {
+						// IMPORTANT
+						//
+						// FieldByName will initialize the value. It is important that this happens AFTER
+						// we checked whether the field should be set. Otherwise, we'll set a zero value!
+						//
+						// This is most likely the reason for https://github.com/gobuffalo/pop/issues/139
+						modelAssociationField := mmi.mapper.FieldByName(mvalue, asoc.Name)
 						modelAssociationField.Set(reflect.Append(modelAssociationField, asocValue))
 					}
 				}