Skip to content

Commit

Permalink
Fix association replace non-addressable panic (#7012)
Browse files Browse the repository at this point in the history
* Fix association replace non-addressable panic

* Fix tests

* Add has one panic test

---------

Co-authored-by: sgsv <->
  • Loading branch information
SergeiSadov committed Jun 12, 2024
1 parent 3fe7fcf commit 78c6dfd
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 0 deletions.
14 changes: 14 additions & 0 deletions association.go
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,10 @@ func (association *Association) saveAssociation(clear bool, values ...interface{
}
}
case reflect.Struct:
if !rv.CanAddr() {
association.Error = ErrInvalidValue
return
}
association.Error = association.Relationship.Field.Set(association.DB.Statement.Context, source, rv.Addr().Interface())

if association.Relationship.Field.FieldType.Kind() == reflect.Struct {
Expand Down Expand Up @@ -433,6 +437,10 @@ func (association *Association) saveAssociation(clear bool, values ...interface{
appendToFieldValues(reflect.Indirect(rv.Index(i)).Addr())
}
case reflect.Struct:
if !rv.CanAddr() {
association.Error = ErrInvalidValue
return
}
appendToFieldValues(rv.Addr())
}

Expand Down Expand Up @@ -510,6 +518,9 @@ func (association *Association) saveAssociation(clear bool, values ...interface{

for i := 0; i < reflectValue.Len(); i++ {
appendToRelations(reflectValue.Index(i), reflect.Indirect(reflect.ValueOf(values[i])), clear)
if association.Error != nil {
return
}

// TODO support save slice data, sql with case?
association.Error = associationDB.Updates(reflectValue.Index(i).Addr().Interface()).Error
Expand All @@ -531,6 +542,9 @@ func (association *Association) saveAssociation(clear bool, values ...interface{
for idx, value := range values {
rv := reflect.Indirect(reflect.ValueOf(value))
appendToRelations(reflectValue, rv, clear && idx == 0)
if association.Error != nil {
return
}
}

if len(values) > 0 {
Expand Down
12 changes: 12 additions & 0 deletions tests/associations_has_many_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -554,3 +554,15 @@ func TestHasManyAssociationUnscoped(t *testing.T) {
t.Errorf("expected %d contents, got %d", 0, len(contents))
}
}

func TestHasManyAssociationReplaceWithNonValidValue(t *testing.T) {
user := User{Name: "jinzhu", Languages: []Language{{Name: "EN"}}}

if err := DB.Create(&user).Error; err != nil {
t.Fatalf("errors happened when create: %v", err)
}

if err := DB.Model(&user).Association("Languages").Replace(Language{Name: "DE"}, Language{Name: "FR"}); err == nil {
t.Error("expected association error to be not nil")
}
}
12 changes: 12 additions & 0 deletions tests/associations_has_one_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -255,3 +255,15 @@ func TestPolymorphicHasOneAssociationForSlice(t *testing.T) {
DB.Model(&pets).Association("Toy").Clear()
AssertAssociationCount(t, pets, "Toy", 0, "After Clear")
}

func TestHasOneAssociationReplaceWithNonValidValue(t *testing.T) {
user := User{Name: "jinzhu", Account: Account{Number: "1"}}

if err := DB.Create(&user).Error; err != nil {
t.Fatalf("errors happened when create: %v", err)
}

if err := DB.Model(&user).Association("Languages").Replace(Account{Number: "2"}); err == nil {
t.Error("expected association error to be not nil")
}
}

0 comments on commit 78c6dfd

Please sign in to comment.