Skip to content

Commit

Permalink
fix: fix gorm issue with reading values which are defaulted to null
Browse files Browse the repository at this point in the history
Added RW.clearDefaultNullResourceFields which will clear fields in the
resource which are defaulted to a null value.  This addresses the
unfixed issue in gorm: go-gorm/gorm#6351
  • Loading branch information
jimlambrt committed May 31, 2023
1 parent 56bb75f commit 9a5a532
Show file tree
Hide file tree
Showing 6 changed files with 60 additions and 2 deletions.
2 changes: 1 addition & 1 deletion go.mod
Expand Up @@ -18,7 +18,7 @@ require (
google.golang.org/protobuf v1.27.1
gorm.io/driver/postgres v1.5.0
gorm.io/driver/sqlite v1.4.4
gorm.io/gorm v1.24.7-0.20230306060331-85eaf9eeda11
gorm.io/gorm v1.25.1
mvdan.cc/gofumpt v0.2.0
)

Expand Down
2 changes: 2 additions & 0 deletions go.sum
Expand Up @@ -933,6 +933,8 @@ gorm.io/gorm v1.24.5 h1:g6OPREKqqlWq4kh/3MCQbZKImeB9e6Xgc4zD+JgNZGE=
gorm.io/gorm v1.24.5/go.mod h1:DVrVomtaYTbqs7gB/x2uVvqnXzv0nqjB396B8cG4dBA=
gorm.io/gorm v1.24.7-0.20230306060331-85eaf9eeda11 h1:9qNbmu21nNThCNnF5i2R3kw2aL27U8ZwbzccNjOmW0g=
gorm.io/gorm v1.24.7-0.20230306060331-85eaf9eeda11/go.mod h1:L4uxeKpfBml98NYqVqwAdmV1a2nBtAec/cf3fpucW/k=
gorm.io/gorm v1.25.1 h1:nsSALe5Pr+cM3V1qwwQ7rOkw+6UeLrX5O4v3llhHa64=
gorm.io/gorm v1.25.1/go.mod h1:L4uxeKpfBml98NYqVqwAdmV1a2nBtAec/cf3fpucW/k=
honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
honnef.co/go/tools v0.0.0-20190106161140-3f1c8253044a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
honnef.co/go/tools v0.0.0-20190418001031-e561f6794a2a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
Expand Down
1 change: 1 addition & 0 deletions lookup.go
Expand Up @@ -39,6 +39,7 @@ func (rw *RW) LookupBy(ctx context.Context, resourceWithIder interface{}, opt ..
if opts.WithDebug {
db = db.Debug()
}
rw.clearDefaultNullResourceFields(ctx, resourceWithIder)
if err := db.Where(where, keys...).First(resourceWithIder).Error; err != nil {
if err == gorm.ErrRecordNotFound {
return fmt.Errorf("%s: %w", op, ErrRecordNotFound)
Expand Down
18 changes: 17 additions & 1 deletion lookup_test.go
Expand Up @@ -7,11 +7,13 @@ import (
"context"
"testing"

"github.com/google/go-cmp/cmp"
"github.com/hashicorp/go-dbw"
"github.com/hashicorp/go-dbw/internal/dbtest"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"google.golang.org/protobuf/proto"
"google.golang.org/protobuf/testing/protocmp"
)

func TestDb_LookupBy(t *testing.T) {
Expand Down Expand Up @@ -54,6 +56,20 @@ func TestDb_LookupBy(t *testing.T) {
wantErr: false,
want: user,
},
{
name: "with-null-values-set",
rw: testRw,
args: args{
resource: &dbtest.TestScooter{
StoreTestScooter: &dbtest.StoreTestScooter{
PrivateId: scooter.GetPrivateId(),
Model: "model",
Mpg: 10,
},
},
},
want: scooter,
},
{
name: "with-table",
rw: testRw,
Expand Down Expand Up @@ -161,7 +177,7 @@ func TestDb_LookupBy(t *testing.T) {
return
}
require.NoError(err)
assert.True(proto.Equal(tt.want, cp.(proto.Message)))
assert.Empty(cmp.Diff(tt.want, cp.(proto.Message), protocmp.Transform()))
})
}
t.Run("not-ptr", func(t *testing.T) {
Expand Down
31 changes: 31 additions & 0 deletions rw.go
Expand Up @@ -204,6 +204,37 @@ func (rw *RW) whereClausesFromOpts(_ context.Context, i interface{}, opts Option
return strings.Join(where, " and "), args, nil
}

// clearDefaultNullResourceFields will clear fields in the resource which are
// defaulted to a null value. This addresses the unfixed issue in gorm:
// https://github.com/go-gorm/gorm/issues/6351
func (rw *RW) clearDefaultNullResourceFields(ctx context.Context, i interface{}) error {
const op = "dbw.ClearResourceFields"
stmt := rw.underlying.wrapped.Model(i).Statement
if err := stmt.Parse(i); err != nil {
return fmt.Errorf("%s: %w", op, err)
}
v := reflect.ValueOf(i)
for _, f := range stmt.Schema.Fields {
switch {
case f.PrimaryKey:
// seems a bit redundant, with the test for null, but it's very
// important to not clear the primary fields, so we'll make an
// explicit test
continue
case !strings.EqualFold(f.DefaultValue, "null"):
continue
default:
_, isZero := f.ValueOf(ctx, v)
if isZero {
continue
}
if err := f.Set(stmt.Context, v, f.DefaultValueInterface); err != nil {
return fmt.Errorf("%s: unable to set value of non-zero field: %w", op, err)
}
}
}
return nil
}
func (rw *RW) primaryKeysWhere(ctx context.Context, i interface{}) (string, []interface{}, error) {
const op = "dbw.primaryKeysWhere"
var fieldNames []string
Expand Down
8 changes: 8 additions & 0 deletions update_test.go
Expand Up @@ -38,6 +38,14 @@ func TestDb_UpdateUnsetField(t *testing.T) {
assert.Equal(1, cnt)
assert.Equal("", updatedTu.Email)
assert.Equal("updated", updatedTu.Name)

found := dbtest.AllocTestUser()
found.PublicId = tu.PublicId
found.Email = "ignore"
err = rw.LookupByPublicId(testCtx, &found)
require.NoError(err)
assert.Equal("", found.Email)
assert.Equal("updated", found.Name)
}

func TestDb_Update(t *testing.T) {
Expand Down

0 comments on commit 9a5a532

Please sign in to comment.