From c29c06c5fa23a8cc5c21dabb5956db6a32906109 Mon Sep 17 00:00:00 2001 From: Antonio Pagano <645522+paganotoni@users.noreply.github.com> Date: Sun, 17 Apr 2022 18:37:29 -0500 Subject: [PATCH] v6.0.2 (#704) * fix: improve model ID field customization (#604) Updates places where `"id"` was hardcoded instead of using `model.IDField()`. * Ensure uninitialized map is initialized when unmarshaling json Add tests for this scenario * exclude migration_table_name from connection string * add test for OptionsString * Add support for pointer FKs when preloading a belongs_to association (#602) * feat: support context-aware tablenames (#614) This patch adds a feature which enables pop to pass down the connection context to the model's TableName() function by implementing TableName(ctx context.Context) string. The context can be used to dynamically generate tablenames which can be important for prefixed or generic tables and other use cases. * Bump pg deps (#616) * Reset to development * bumping pgx and pgconn versions Co-authored-by: Stanislas Michalak Co-authored-by: Larry M Jordan * Latest from master (#620) * Latest from development (#617) * fix: improve model ID field customization (#604) Updates places where `"id"` was hardcoded instead of using `model.IDField()`. * Ensure uninitialized map is initialized when unmarshaling json Add tests for this scenario * exclude migration_table_name from connection string * add test for OptionsString * Add support for pointer FKs when preloading a belongs_to association (#602) * feat: support context-aware tablenames (#614) This patch adds a feature which enables pop to pass down the connection context to the model's TableName() function by implementing TableName(ctx context.Context) string. The context can be used to dynamically generate tablenames which can be important for prefixed or generic tables and other use cases. * Bump pg deps (#616) * Reset to development * bumping pgx and pgconn versions Co-authored-by: Stanislas Michalak Co-authored-by: Larry M Jordan Co-authored-by: Patrik Co-authored-by: Michael Montgomery Co-authored-by: kyrozetera Co-authored-by: Reggie Riser <4960757+reggieriser@users.noreply.github.com> Co-authored-by: hackerman <3372410+aeneasr@users.noreply.github.com> Co-authored-by: Stanislas Michalak Co-authored-by: Larry M Jordan * adding goreleaser syntaz (#619) Co-authored-by: Patrik Co-authored-by: Michael Montgomery Co-authored-by: kyrozetera Co-authored-by: Reggie Riser <4960757+reggieriser@users.noreply.github.com> Co-authored-by: hackerman <3372410+aeneasr@users.noreply.github.com> Co-authored-by: Stanislas Michalak Co-authored-by: Larry M Jordan * Resolve issues in UPDATE and DELETE when using schemas (#618) * Resolve MySQL issues and improve test migrations * Bump CockroachDB to maintained and supported versions Version 2.1 has reached EoL in 2019 Signed-off-by: aeneasr <3372410+aeneasr@users.noreply.github.com> * Use `PaginatorPageKey` and `PaginatorPerPageKey` variables (#615) * update pagination_test * Pass Time structure into timestamp update functions. (#625) Closes #624 * Allow nullable JSONB and resolve MySQL regression (#639) * Allow passing args to `Order` (#630) * Added connection maximum idle time configuration (#635) This PR add the possibility to configure the connection maximum idle time (https://golang.org/pkg/database/sql/#DB.SetConnMaxIdleTime). Closes #632 BREAKING CHANGE: Requires Go 1.15 from now on. * Bump sqlite to 3.35.4 / 1.14.7 (#642) * Update pg, pgx, sqlx (#643) - `jackc/pgx` to version `v4.11.0`. - `jmoiron/sqlx` to version`v1.3.3` - `lib/pq` to version`v1.10.1` * Fix Inner has many associations when passing on multiple arguments (#633) * Fix Inner has many associations when passing on multiple arguments for inner fields * Fix broken tests * clean up extractFieldAndInnerFields function Co-authored-by: hackerman <3372410+aeneasr@users.noreply.github.com> Co-authored-by: Antonio Pagano <645522+paganotoni@users.noreply.github.com> * Remove many to many TX condition for EagerPreload (#645) * Remove the need to use Tx when loading many to many associations * replace TX access to create a new tx.Store.Transaction() object * Added fix/tests for has_many with pointer foreign key (#647) Co-authored-by: Antonio Pagano <645522+paganotoni@users.noreply.github.com> * Export WhereID, Alias, WhereNamedID (#637) This patch export some model convenience functions which are useful when constructing queries outside of pop: custom updates, deletes, inserts, ... Signed-off-by: aeneasr <3372410+aeneasr@users.noreply.github.com> Co-authored-by: Antonio Pagano <645522+paganotoni@users.noreply.github.com> * fix: log model values everywhere (#656) Some SQL logs were missing the values as argument. This adds all places * Add delete to query builder (#658) This allows writing delete queries without knowing the exact primary key or for composite keys. `Destroy` only allows to delete by primary key, but there are many cases where you want to delete multiple rows or by some other query than the ID. See #29 * Sort down migrations (#657) Basically, just reversing the up migration order does not work, as that puts "all" migrations before specific ones. Therefore, I added implemented the proper `Less` function for down migrations explicitly. Related #533 Co-authored-by: hackerman <3372410+aeneasr@users.noreply.github.com> * Preserve eager information when validating models (#664) (#665) Co-authored-by: Karl Haas * Migrate from packr to fs (#667) * Updating Pgx (#660) * adding goreleaser syntaz * updating pgx now really * Migrate from packr to fs * Migrate to v6 * Use old build tags * Update error handling * Fix error after rebase * Fix error handling * Fix filenames for embed Go 1.16 usage Co-authored-by: Antonio Pagano <645522+paganotoni@users.noreply.github.com> * Task merging master (#669) * v5.3.4 (#644) * fix: improve model ID field customization (#604) Updates places where `"id"` was hardcoded instead of using `model.IDField()`. * Ensure uninitialized map is initialized when unmarshaling json Add tests for this scenario * exclude migration_table_name from connection string * add test for OptionsString * Add support for pointer FKs when preloading a belongs_to association (#602) * feat: support context-aware tablenames (#614) This patch adds a feature which enables pop to pass down the connection context to the model's TableName() function by implementing TableName(ctx context.Context) string. The context can be used to dynamically generate tablenames which can be important for prefixed or generic tables and other use cases. * Bump pg deps (#616) * Reset to development * bumping pgx and pgconn versions Co-authored-by: Stanislas Michalak Co-authored-by: Larry M Jordan * Latest from master (#620) * Latest from development (#617) * fix: improve model ID field customization (#604) Updates places where `"id"` was hardcoded instead of using `model.IDField()`. * Ensure uninitialized map is initialized when unmarshaling json Add tests for this scenario * exclude migration_table_name from connection string * add test for OptionsString * Add support for pointer FKs when preloading a belongs_to association (#602) * feat: support context-aware tablenames (#614) This patch adds a feature which enables pop to pass down the connection context to the model's TableName() function by implementing TableName(ctx context.Context) string. The context can be used to dynamically generate tablenames which can be important for prefixed or generic tables and other use cases. * Bump pg deps (#616) * Reset to development * bumping pgx and pgconn versions Co-authored-by: Stanislas Michalak Co-authored-by: Larry M Jordan Co-authored-by: Patrik Co-authored-by: Michael Montgomery Co-authored-by: kyrozetera Co-authored-by: Reggie Riser <4960757+reggieriser@users.noreply.github.com> Co-authored-by: hackerman <3372410+aeneasr@users.noreply.github.com> Co-authored-by: Stanislas Michalak Co-authored-by: Larry M Jordan * adding goreleaser syntaz (#619) Co-authored-by: Patrik Co-authored-by: Michael Montgomery Co-authored-by: kyrozetera Co-authored-by: Reggie Riser <4960757+reggieriser@users.noreply.github.com> Co-authored-by: hackerman <3372410+aeneasr@users.noreply.github.com> Co-authored-by: Stanislas Michalak Co-authored-by: Larry M Jordan * Resolve issues in UPDATE and DELETE when using schemas (#618) * Resolve MySQL issues and improve test migrations * Bump CockroachDB to maintained and supported versions Version 2.1 has reached EoL in 2019 Signed-off-by: aeneasr <3372410+aeneasr@users.noreply.github.com> * Use `PaginatorPageKey` and `PaginatorPerPageKey` variables (#615) * update pagination_test * Pass Time structure into timestamp update functions. (#625) Closes #624 * Allow nullable JSONB and resolve MySQL regression (#639) * Allow passing args to `Order` (#630) * Added connection maximum idle time configuration (#635) This PR add the possibility to configure the connection maximum idle time (https://golang.org/pkg/database/sql/#DB.SetConnMaxIdleTime). Closes #632 BREAKING CHANGE: Requires Go 1.15 from now on. * Bump sqlite to 3.35.4 / 1.14.7 (#642) * Update pg, pgx, sqlx (#643) - `jackc/pgx` to version `v4.11.0`. - `jmoiron/sqlx` to version`v1.3.3` - `lib/pq` to version`v1.10.1` * Fix Inner has many associations when passing on multiple arguments (#633) * Fix Inner has many associations when passing on multiple arguments for inner fields * Fix broken tests * clean up extractFieldAndInnerFields function Co-authored-by: hackerman <3372410+aeneasr@users.noreply.github.com> Co-authored-by: Antonio Pagano <645522+paganotoni@users.noreply.github.com> * Remove many to many TX condition for EagerPreload (#645) * Remove the need to use Tx when loading many to many associations * replace TX access to create a new tx.Store.Transaction() object * Added fix/tests for has_many with pointer foreign key (#647) Co-authored-by: Antonio Pagano <645522+paganotoni@users.noreply.github.com> Co-authored-by: Patrik Co-authored-by: Michael Montgomery Co-authored-by: kyrozetera Co-authored-by: Reggie Riser <4960757+reggieriser@users.noreply.github.com> Co-authored-by: hackerman <3372410+aeneasr@users.noreply.github.com> Co-authored-by: Stanislas Michalak Co-authored-by: Larry M Jordan Co-authored-by: Brian Buchholz <4773480+bhb603@users.noreply.github.com> Co-authored-by: Mike Pontillo Co-authored-by: Benjamin Blattberg Co-authored-by: Jonathan Duck Co-authored-by: Arthur Knoepflin * Updating Pgx (#660) * adding goreleaser syntaz * updating pgx now really * tidying Co-authored-by: Patrik Co-authored-by: Michael Montgomery Co-authored-by: kyrozetera Co-authored-by: Reggie Riser <4960757+reggieriser@users.noreply.github.com> Co-authored-by: hackerman <3372410+aeneasr@users.noreply.github.com> Co-authored-by: Stanislas Michalak Co-authored-by: Larry M Jordan Co-authored-by: Brian Buchholz <4773480+bhb603@users.noreply.github.com> Co-authored-by: Mike Pontillo Co-authored-by: Benjamin Blattberg Co-authored-by: Jonathan Duck Co-authored-by: Arthur Knoepflin * Replace removed command The command has been removed from the CLI. This patch introduces a new mechanism to reliably dump the SQL schema for CockroachDB. * Resolve `EagerPreload` panic caused for pointer references Resolves a panic where `EagerPreload` tried to set `reflect.Struct` for a `reflect.Pointer` on 1.. associations. Signed-off-by: aeneasr <3372410+aeneasr@users.noreply.github.com> * Resolve `EagerPreload` panic caused by NullUUID Resolves a panic where `EagerPreload` was trying to set UUID into NullUUID. Signed-off-by: aeneasr <3372410+aeneasr@users.noreply.github.com> * Support pointers in n+1 `Eager` loading Resolves an issue where n+1 eager associations would error with a double pointer in `associations.ForStruct`. Signed-off-by: aeneasr <3372410+aeneasr@users.noreply.github.com> * Improve error message of associations.ForStruct Signed-off-by: aeneasr <3372410+aeneasr@users.noreply.github.com> * Add test cases for `IsZeroOfUnderlyingType` Signed-off-by: aeneasr <3372410+aeneasr@users.noreply.github.com> * Resolve an obscure bug where empty structs got loaded for NULL foreign keys Closes https://github.com/gobuffalo/pop/issues/139 Signed-off-by: aeneasr <3372410+aeneasr@users.noreply.github.com> * Resolve association regression in finders.go Signed-off-by: aeneasr <3372410+aeneasr@users.noreply.github.com> * Use dedicated migrations for preloading regression test Signed-off-by: aeneasr <3372410+aeneasr@users.noreply.github.com> * Fix code regression Signed-off-by: aeneasr <3372410+aeneasr@users.noreply.github.com> * Fix test code regressions Signed-off-by: aeneasr <3372410+aeneasr@users.noreply.github.com> * Fix sql migration order Signed-off-by: aeneasr <3372410+aeneasr@users.noreply.github.com> * Resolve order issue in test * Ignore order when testing for nil values in test * Pass Context during exec in create. (#688) * feat: support embedded struct fields (#691) * test: use `T.TempDir` to create temporary test directory This commit replaces `ioutil.TempDir` with `t.TempDir` in tests. The directory created by `t.TempDir` is automatically removed when the test and all its subtests complete. Prior to this commit, temporary directory created using `ioutil.TempDir` needs to be removed manually by calling `os.RemoveAll`, which is omitted in some tests. The error handling boilerplate e.g. defer func() { if err := os.RemoveAll(dir); err != nil { t.Fatal(err) } } is also tedious, but `t.TempDir` handles this for us nicely. Reference: https://pkg.go.dev/testing#T.TempDir Signed-off-by: Eng Zer Jun * task: adding next version number Co-authored-by: Patrik Co-authored-by: Michael Montgomery Co-authored-by: kyrozetera Co-authored-by: Reggie Riser <4960757+reggieriser@users.noreply.github.com> Co-authored-by: hackerman <3372410+aeneasr@users.noreply.github.com> Co-authored-by: Stanislas Michalak Co-authored-by: Larry M Jordan Co-authored-by: Brian Buchholz <4773480+bhb603@users.noreply.github.com> Co-authored-by: Mike Pontillo Co-authored-by: Benjamin Blattberg Co-authored-by: Jonathan Duck Co-authored-by: Arthur Knoepflin Co-authored-by: karlhaas Co-authored-by: Karl Haas Co-authored-by: Matthias Fasching Co-authored-by: Martin Eigenbrodt Co-authored-by: Eng Zer Jun --- associations/association.go | 4 + associations/association_test.go | 37 +++++++++ associations/associations_for_struct.go | 13 +++- associations/belongs_to_association.go | 21 +++-- associations/belongs_to_association_test.go | 19 +++++ columns/columns_for_struct.go | 50 +++++++----- dialect_cockroach.go | 35 ++++++++- dialect_common.go | 2 +- dialect_sqlite.go | 3 +- dialect_sqlite_test.go | 10 +-- executors_test.go | 27 +++++++ finders.go | 9 ++- finders_test.go | 36 +++++++++ pop_test.go | 49 ++++++++++++ preload_associations.go | 47 ++++++++--- preload_associations_test.go | 77 ++++++++++++++++++- soda/cmd/generate/config_cmd_test.go | 5 +- soda/cmd/generate/fizz_cmd_test.go | 5 +- soda/cmd/generate/model_cmd_test.go | 9 +-- soda/cmd/migrate.go | 2 +- soda/cmd/version.go | 2 +- test.sh | 2 +- .../20210104145902_network.down.fizz | 3 + .../migrations/20210104145902_network.up.fizz | 18 +++++ .../20220214113659_embedded_struct.down.fizz | 1 + .../20220214113659_embedded_struct.up.fizz | 7 ++ 26 files changed, 422 insertions(+), 71 deletions(-) create mode 100644 associations/association_test.go create mode 100644 testdata/migrations/20210104145902_network.down.fizz create mode 100644 testdata/migrations/20210104145902_network.up.fizz create mode 100644 testdata/migrations/20220214113659_embedded_struct.down.fizz create mode 100644 testdata/migrations/20220214113659_embedded_struct.up.fizz diff --git a/associations/association.go b/associations/association.go index 436c4581..8ed165a4 100644 --- a/associations/association.go +++ b/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()) } diff --git a/associations/association_test.go b/associations/association_test.go new file mode 100644 index 00000000..504c52e9 --- /dev/null +++ b/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)) + }) + } +} diff --git a/associations/associations_for_struct.go b/associations/associations_for_struct.go index 67c8ca76..bdd7e180 100644 --- a/associations/associations_for_struct.go +++ b/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{} @@ -73,6 +72,16 @@ func ForStruct(s interface{}, fields ...string) (Associations, error) { for i := 0; i < t.NumField(); i++ { f := t.Field(i) + // inline embedded field + if f.Anonymous { + innerAssociations, err := ForStruct(v.Field(i).Interface(), fields...) + if err != nil { + return nil, err + } + associations = append(associations, innerAssociations...) + continue + } + // ignores those fields not included in fields list. if len(fields) > 0 && fieldIgnoredIn(fields, f.Name) { continue diff --git a/associations/belongs_to_association.go b/associations/belongs_to_association.go index fce161d0..c64d7208 100644 --- a/associations/belongs_to_association.go +++ b/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) } diff --git a/associations/belongs_to_association_test.go b/associations/belongs_to_association_test.go index f7a09ba3..ca72c137 100644 --- a/associations/belongs_to_association_test.go +++ b/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()) + } +} diff --git a/columns/columns_for_struct.go b/columns/columns_for_struct.go index a20cd426..5e09f69f 100644 --- a/columns/columns_for_struct.go +++ b/columns/columns_for_struct.go @@ -31,33 +31,47 @@ func ForStructWithAlias(s interface{}, tableName, tableAlias, idField string) (c } } - fieldCount := st.NumField() + // recursive functions to also find and add embedded struct fields + var findColumns func(st reflect.Type) + findColumns = func(t reflect.Type) { + if t.Kind() == reflect.Ptr { + t = t.Elem() + } - for i := 0; i < fieldCount; i++ { - field := st.Field(i) + fc := t.NumField() + for i := 0; i < fc; i++ { + field := t.Field(i) - popTags := TagsFor(field) - tag := popTags.Find("db") + if field.Anonymous { + findColumns(field.Type) + continue + } - if !tag.Ignored() && !tag.Empty() { - col := tag.Value + popTags := TagsFor(field) + tag := popTags.Find("db") - // add writable or readable. - tag := popTags.Find("rw") - if !tag.Empty() { - col = col + "," + tag.Value - } + if !tag.Ignored() && !tag.Empty() { + col := tag.Value - cs := columns.Add(col) + // add writable or readable. + tag := popTags.Find("rw") + if !tag.Empty() { + col = col + "," + tag.Value + } - // add select clause. - tag = popTags.Find("select") - if !tag.Empty() { - c := cs[0] - c.SetSelectSQL(tag.Value) + cs := columns.Add(col) + + // add select clause. + tag = popTags.Find("select") + if !tag.Empty() { + c := cs[0] + c.SetSelectSQL(tag.Value) + } } } } + findColumns(st) + return columns } diff --git a/dialect_cockroach.go b/dialect_cockroach.go index 2ea10cb3..16d4d96b 100644 --- a/dialect_cockroach.go +++ b/dialect_cockroach.go @@ -1,11 +1,13 @@ package pop import ( + "bytes" "fmt" "io" "os" "os/exec" "path/filepath" + "regexp" "strings" "sync" @@ -200,13 +202,42 @@ func (p *cockroach) FizzTranslator() fizz.Translator { } func (p *cockroach) DumpSchema(w io.Writer) error { - cmd := exec.Command("cockroach", "dump", p.Details().Database, "--dump-mode=schema") + cmd := exec.Command("cockroach", "sql", "-e", "SHOW CREATE ALL TABLES", "-d", p.Details().Database, "--format", "raw") c := p.ConnectionDetails if defaults.String(c.Options["sslmode"], "disable") == "disable" || strings.Contains(c.RawOptions, "sslmode=disable") { cmd.Args = append(cmd.Args, "--insecure") } - return genericDumpSchema(p.Details(), cmd, w) + return cockroachDumpSchema(p.Details(), cmd, w) +} + +func cockroachDumpSchema(deets *ConnectionDetails, cmd *exec.Cmd, w io.Writer) error { + log(logging.SQL, strings.Join(cmd.Args, " ")) + + var bb bytes.Buffer + + cmd.Stdout = &bb + cmd.Stderr = os.Stderr + + err := cmd.Run() + if err != nil { + return err + } + + // --format raw returns comments prefixed with # which is invalid, so we make it a valid SQL comment. + result := regexp.MustCompile("(?m)^#").ReplaceAll(bb.Bytes(), []byte("-- #")) + + if _, err := w.Write(result); err != nil { + return err + } + + x := bytes.TrimSpace(result) + if len(x) == 0 { + return fmt.Errorf("unable to dump schema for %s", deets.Database) + } + + log(logging.Info, "dumped schema for %s", deets.Database) + return nil } func (p *cockroach) LoadSchema(r io.Reader) error { diff --git a/dialect_common.go b/dialect_common.go index 00e9e552..d0876295 100644 --- a/dialect_common.go +++ b/dialect_common.go @@ -85,7 +85,7 @@ func genericCreate(s store, model *Model, cols columns.Columns, quoter quotable) if err != nil { return err } - _, err = stmt.Exec(model.Value) + _, err = stmt.ExecContext(model.ctx, model.Value) if err != nil { if closeErr := stmt.Close(); closeErr != nil { return fmt.Errorf("failed to close prepared statement: %s: %w", closeErr, err) diff --git a/dialect_sqlite.go b/dialect_sqlite.go index 49306c30..1ac8691f 100644 --- a/dialect_sqlite.go +++ b/dialect_sqlite.go @@ -169,10 +169,11 @@ func (m *sqlite) CreateDB() error { if err != nil { return fmt.Errorf("could not create SQLite database '%s': %w", m.ConnectionDetails.Database, err) } - _, err = os.Create(m.ConnectionDetails.Database) + f, err := os.Create(m.ConnectionDetails.Database) if err != nil { return fmt.Errorf("could not create SQLite database '%s': %w", m.ConnectionDetails.Database, err) } + _ = f.Close() log(logging.Info, "created database '%s'", m.ConnectionDetails.Database) return nil diff --git a/dialect_sqlite_test.go b/dialect_sqlite_test.go index 16796f6d..83a0294a 100644 --- a/dialect_sqlite_test.go +++ b/dialect_sqlite_test.go @@ -4,8 +4,6 @@ package pop import ( "fmt" - "io/ioutil" - "os" "path/filepath" "testing" @@ -133,10 +131,8 @@ func Test_ConnectionDetails_Finalize_SQLite_OverrideOptions_Synonym_Path(t *test func Test_ConnectionDetails_FinalizeOSPath(t *testing.T) { r := require.New(t) - d, err := ioutil.TempDir("", "") - r.NoError(err) + d := t.TempDir() p := filepath.Join(d, "testdb.sqlite") - defer os.RemoveAll(p) cd := &ConnectionDetails{ Dialect: "sqlite", Database: p, @@ -148,10 +144,8 @@ func Test_ConnectionDetails_FinalizeOSPath(t *testing.T) { func TestSqlite_CreateDB(t *testing.T) { r := require.New(t) - dir, err := ioutil.TempDir("", "") - r.NoError(err) + dir := t.TempDir() p := filepath.Join(dir, "testdb.sqlite") - defer os.RemoveAll(p) cd := &ConnectionDetails{ Dialect: "sqlite", Database: p, diff --git a/executors_test.go b/executors_test.go index 3202bfad..d428e364 100644 --- a/executors_test.go +++ b/executors_test.go @@ -533,6 +533,33 @@ func Test_Create_Non_PK_ID(t *testing.T) { }) } +func Test_Embedded_Struct(t *testing.T) { + if PDB == nil { + t.Skip("skipping integration tests") + } + transaction(func(tx *Connection) { + r := require.New(t) + + entry := &EmbeddingStruct{ + InnerStruct: InnerStruct{}, + AdditionalField: "I am also important!", + } + r.NoError(tx.Create(entry)) + + var actual EmbeddingStruct + r.NoError(tx.Find(&actual, entry.ID)) + r.Equal(entry.AdditionalField, actual.AdditionalField) + + entry.AdditionalField = entry.AdditionalField + " updated" + r.NoError(tx.Update(entry)) + + r.NoError(tx.Find(&actual, entry.ID)) + r.Equal(entry.AdditionalField, actual.AdditionalField) + + r.NoError(tx.Destroy(entry)) + }) +} + func Test_Eager_Create_Has_Many(t *testing.T) { if PDB == nil { t.Skip("skipping integration tests") diff --git a/finders.go b/finders.go index c90d1032..ccd4bbd5 100644 --- a/finders.go +++ b/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 } diff --git a/finders_test.go b/finders_test.go index 7e2daa02..cc5bd925 100644 --- a/finders_test.go +++ b/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") diff --git a/pop_test.go b/pop_test.go index bfe7ea78..629ea35a 100644 --- a/pop_test.go +++ b/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"` @@ -438,3 +476,14 @@ type NonStandardID struct { ID int `db:"pk"` OutfacingID string `db:"id"` } + +type InnerStruct struct { + ID int `db:"id"` + CreatedAt time.Time `db:"created_at"` + UpdatedAt time.Time `db:"updated_at"` +} + +type EmbeddingStruct struct { + InnerStruct + AdditionalField string `db:"additional_field"` +} diff --git a/preload_associations.go b/preload_associations.go index 72a7f4c3..6e8fec10 100644 --- a/preload_associations.go +++ b/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)) } } diff --git a/preload_associations_test.go b/preload_associations_test.go index 879531c2..3d70bad9 100644 --- a/preload_associations_test.go +++ b/preload_associations_test.go @@ -1,10 +1,9 @@ package pop import ( - "testing" - "github.com/gobuffalo/nulls" "github.com/stretchr/testify/require" + "testing" ) func Test_New_Implementation_For_Nplus1(t *testing.T) { @@ -53,6 +52,18 @@ func Test_New_Implementation_For_Nplus1(t *testing.T) { a.Len(book.Writers, 1) a.Equal("Larry", book.Writers[0].Name) a.Equal("Mark", book.User.Name.String) + + usersWithPointers := []UserPointerAssocs{} + a.NoError(tx.All(&usersWithPointers)) + + // FILL THE HAS-MANY and HAS_ONE + a.NoError(preload(tx, &usersWithPointers)) + + a.Len(usersWithPointers[0].Books, 1) + a.Len(usersWithPointers[1].Books, 1) + a.Len(usersWithPointers[2].Books, 1) + a.Equal(usersWithPointers[0].FavoriteSong.UserID, users[0].ID) + a.Len(usersWithPointers[0].Houses, 1) }) } @@ -101,6 +112,68 @@ func Test_New_Implementation_For_Nplus1_With_UUID(t *testing.T) { }) } +func Test_New_Implementation_For_Nplus1_With_NullUUIDs_And_FK_ID(t *testing.T) { + if PDB == nil { + t.Skip("skipping integration tests") + } + + // This test suite prevents regressions of an obscure bug in the preload code which caused + // pointer values to be set with their empty values when relations did not exist. + // + // See also: https://github.com/gobuffalo/pop/issues/139 + transaction(func(tx *Connection) { + a := require.New(t) + + var server Server + a.NoError(tx.Create(&server)) + + class := &NetClient{ + // The bug only appears when we have two elements in the slice where + // one has a relation and the other one has no such relation. + Hops: []Hop{ + {Server: &server}, + {}, + }} + + // This code basically just sets up + a.NoError(tx.Eager().Create(class)) + + var expected NetClient + a.NoError(tx.EagerPreload("Hops.Server").First(&expected)) + + // What would happen before the patch resolved this issue is that: + // + // Classes.CourseCodes[0].Course would be the correct value (a filled struct) + // + // "server": { + // "id": "fa51f71f-e884-4641-8005-923258b814f9", + // "created_at": "2021-12-09T23:20:10.208019+01:00", + // "updated_at": "2021-12-09T23:20:10.208019+01:00" + // }, + // + // Classes.CourseCodes[1].Course would an "empty" struct of Course even though there is no relation set up: + // + // "server": { + // "id": "00000000-0000-0000-0000-000000000000", + // "created_at": "0001-01-01T00:00:00Z", + // "updated_at": "0001-01-01T00:00:00Z" + // }, + var foundValid, foundEmpty int + for _, hop := range expected.Hops { + if hop.ServerID.Valid { + foundValid++ + a.NotNil(hop.Server, "%+v", hop) + } else { + foundEmpty++ + a.Nil(hop.Server, "%+v", hop) + } + } + + a.Equal(1, foundValid) + a.Equal(1, foundEmpty) + }) +} + func Test_New_Implementation_For_Nplus1_Single(t *testing.T) { if PDB == nil { t.Skip("skipping integration tests") diff --git a/soda/cmd/generate/config_cmd_test.go b/soda/cmd/generate/config_cmd_test.go index 51f21d7c..309121fb 100644 --- a/soda/cmd/generate/config_cmd_test.go +++ b/soda/cmd/generate/config_cmd_test.go @@ -1,7 +1,6 @@ package generate import ( - "io/ioutil" "os" "testing" @@ -13,9 +12,7 @@ func Test_ConfigCmd_NoArg(t *testing.T) { c := ConfigCmd c.SetArgs([]string{}) - tdir, err := ioutil.TempDir("", "testapp") - r.NoError(err) - defer os.RemoveAll(tdir) + tdir := t.TempDir() pwd, err := os.Getwd() r.NoError(err) diff --git a/soda/cmd/generate/fizz_cmd_test.go b/soda/cmd/generate/fizz_cmd_test.go index 59d3bfb1..04b7e6a3 100644 --- a/soda/cmd/generate/fizz_cmd_test.go +++ b/soda/cmd/generate/fizz_cmd_test.go @@ -1,7 +1,6 @@ package generate import ( - "io/ioutil" "os" "testing" @@ -13,9 +12,7 @@ func Test_FizzCmd_NoArg(t *testing.T) { c := FizzCmd c.SetArgs([]string{}) - tdir, err := ioutil.TempDir("", "testapp") - r.NoError(err) - defer os.RemoveAll(tdir) + tdir := t.TempDir() pwd, err := os.Getwd() r.NoError(err) diff --git a/soda/cmd/generate/model_cmd_test.go b/soda/cmd/generate/model_cmd_test.go index a70bc2e5..5dcb3de7 100644 --- a/soda/cmd/generate/model_cmd_test.go +++ b/soda/cmd/generate/model_cmd_test.go @@ -1,7 +1,6 @@ package generate import ( - "io/ioutil" "os" "path/filepath" "testing" @@ -14,9 +13,7 @@ func Test_ModelCmd_NoArg(t *testing.T) { c := ModelCmd c.SetArgs([]string{}) - tdir, err := ioutil.TempDir("", "testapp") - r.NoError(err) - defer os.RemoveAll(tdir) + tdir := t.TempDir() pwd, err := os.Getwd() r.NoError(err) @@ -32,9 +29,7 @@ func Test_ModelCmd_NameOnly(t *testing.T) { c := ModelCmd c.SetArgs([]string{"users"}) - tdir, err := ioutil.TempDir("", "testapp") - r.NoError(err) - defer os.RemoveAll(tdir) + tdir := t.TempDir() pwd, err := os.Getwd() r.NoError(err) diff --git a/soda/cmd/migrate.go b/soda/cmd/migrate.go index 70f35534..8fa25b70 100644 --- a/soda/cmd/migrate.go +++ b/soda/cmd/migrate.go @@ -1,8 +1,8 @@ package cmd import ( - "os" "errors" + "os" "github.com/gobuffalo/pop/v6" "github.com/spf13/cobra" diff --git a/soda/cmd/version.go b/soda/cmd/version.go index 37ba4f67..148d5663 100644 --- a/soda/cmd/version.go +++ b/soda/cmd/version.go @@ -2,4 +2,4 @@ package cmd // Version defines the current Pop version. // this version will also be used by soda(also buffalo-pop) -const Version = "v6.0.1" +const Version = "v6.0.2" diff --git a/test.sh b/test.sh index 4ea9a5f0..d99c6744 100755 --- a/test.sh +++ b/test.sh @@ -58,7 +58,7 @@ function test { ./tsoda create -e $SODA_DIALECT -c ./database.yml -p ./testdata/migrations ./tsoda migrate -e $SODA_DIALECT -c ./database.yml -p ./testdata/migrations echo "Test..." - go test -race -tags sqlite $VERBOSE ./... -count=1 + go test -race -tags sqlite $VERBOSE -count=1 ./... } function debug_test { diff --git a/testdata/migrations/20210104145902_network.down.fizz b/testdata/migrations/20210104145902_network.down.fizz new file mode 100644 index 00000000..151bae5c --- /dev/null +++ b/testdata/migrations/20210104145902_network.down.fizz @@ -0,0 +1,3 @@ +drop_table("clients") +drop_table("hops") +drop_table("servers") diff --git a/testdata/migrations/20210104145902_network.up.fizz b/testdata/migrations/20210104145902_network.up.fizz new file mode 100644 index 00000000..dc624e6c --- /dev/null +++ b/testdata/migrations/20210104145902_network.up.fizz @@ -0,0 +1,18 @@ +create_table("net_clients") { + t.Column("id", "uuid", {"primary": true}) + t.DisableTimestamps() +} + +create_table("servers") { + t.Column("id", "uuid", {"primary": true}) + t.DisableTimestamps() +} + +create_table("hops") { + t.Column("id", "uuid", {"primary": true}) + t.Column("server_id", "uuid", {"null":true}) + t.ForeignKey("server_id", {"servers": ["id"]}, {}) + t.Column("net_client_id", "uuid") + t.ForeignKey("net_client_id", {"net_clients": ["id"]}, {}) + t.DisableTimestamps() +} diff --git a/testdata/migrations/20220214113659_embedded_struct.down.fizz b/testdata/migrations/20220214113659_embedded_struct.down.fizz new file mode 100644 index 00000000..ec1651dd --- /dev/null +++ b/testdata/migrations/20220214113659_embedded_struct.down.fizz @@ -0,0 +1 @@ +drop_table("embedding_structs") diff --git a/testdata/migrations/20220214113659_embedded_struct.up.fizz b/testdata/migrations/20220214113659_embedded_struct.up.fizz new file mode 100644 index 00000000..90473246 --- /dev/null +++ b/testdata/migrations/20220214113659_embedded_struct.up.fizz @@ -0,0 +1,7 @@ +create_table("embedding_structs") { + t.Column("id", "int", { "primary": true }) + + t.Column("additional_field", "string", {}) + + t.Timestamps() +} \ No newline at end of file