Failure to guarantee table resolution for fixtures

[jjtg]

Bun version: v1.2.3

Issue

Addition of fixtures fails sporadically when model definitions that result in the same model name but which have different structures are registered in memory at the same time.

Replication

To replicate the issue, we first have to create two different structures with similar name (i.e. one capitalized and the other lowercased).

type User struct {
    bun.BaseModel `bun:"table:users,alias:u"`

    ID        int64     `bun:",pk,autoincrement"`
    Username  string    `bun:",notnull"`
    Email     string    `bun:",notnull,unique"`
    Password  string    `bun:",notnull"`
    CreatedAt time.Time `bun:",nullzero,default:current_timestamp"`
}

type user struct {
    ID        int64
    UpdatedAt time.Time `bun:",notnull"`
}

We’ll also need to create a fixtures.yaml since the issue can only be replicated with a combination of fixture tests and queries. We can use the following fixtures.

- model: User
  rows:
    - _id: smith
      username: smith
      email: [email protected]
      password: john1234
      created_at: '{{ now }}'
    - _id: doe
      username: doe
      email: [email protected]
      password: john1234
      created_at: '{{ now }}'

Having done this, the next step is to create a test that will reliably replicate the issue. Assuming you’ve setup postgres database in your local environment, create the table with this initial migration.

CREATE TABLE IF NOT EXISTS users(
    id SERIAL PRIMARY KEY,
    username VARCHAR (50) NOT NULL,
    email VARCHAR (100) NOT NULL UNIQUE,
    password VARCHAR (100) NOT NULL,
    created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP
);

Once the setup is complete, the below test should manage to replicate the issue fairly often.

func TestQueryRaw(t *testing.T) {
    type User struct {
        bun.BaseModel `bun:"table:users,alias:u"`

        ID        int64     `bun:",pk,autoincrement"`
        Username  string    `bun:",notnull"`
        Email     string    `bun:",notnull,unique"`
        Password  string    `bun:",notnull"`
        CreatedAt time.Time `bun:",nullzero,default:current_timestamp"`
    }

    type user struct {
        ID        int64
        UpdatedAt time.Time
    }

    dsn := "postgres://postgres:postgres@localhost:5432/postgres?sslmode=disable"
    sqldb := sql.OpenDB(pgdriver.NewConnector(pgdriver.WithDSN(dsn)))

    db := bun.NewDB(sqldb, pgdialect.New())
    db.RegisterModel((*User)(nil))

    t.Run("flaky test", func(t *testing.T) {
        var rows []*user

        ctx := context.Background()
        for i := 0; i < 100; i++ {
            fmt.Printf("Run number %d\n", i)
            fixture := dbfixture.New(db, dbfixture.WithTruncateTables())
            err := fixture.Load(ctx, os.DirFS("testdata"), "fixtures.yaml")
            if err != nil {
                log.Printf("failed to load fixtures: %v", err)
            }

            fmt.Println(fixture.MustRow("User.doe").(*User))

            err = db.NewRaw(
                "SELECT id, updated_at FROM (SELECT u1.id, u1.created_at as updated_at, ROW_NUMBER() over (PARTITION BY u1.id) as row_num FROM users AS u1 WHERE username is not null) sub WHERE row_num = 1",
                bun.Ident("users"), "username IS NOT NULL",
            ).Scan(ctx, &rows)

            for _, r := range rows {
                fmt.Println(r)
            }
        }

    })

}

To verify the error, simply run the tests a few times until you end up with an error like this

failed to load fixtures: bun: model=User does not have column=username

Cause

In the above scenario, both of the structures will result in the same TypeName due to the manner in which type names are normalized (see here for additional details). The paths towards storing the table definition are quite different however.

In the case of a standard fixture test the model is registered by calling db.RegisterModel which then calls t.Register only to finalize with t.Get. It is in the latter that the table is initialized and stored in a xsync.MapOf[reflect.Type, *Table]. When running a query however, the registration happens only when a receiver or destination reference is passed in for example the Scan function in a RawQuery . A representative call hierarchy would be the following:

db.NewRaw → Scan → q.scanOrExec → q.getModel → newModel → _newModel → newSliceTableModel → db.Table → db.dialect.Tables().Get

Once the call reaches the final point, a table gets initialized and stored in the xsync map. Once multiple tables with the same TypeName are registered, we suffer from the volatility of unordered maps when adding fixtures in our tests. This is because every time we register our fixtures with the Load function, the flow fetches the model based on the fixture model name which runs the following code:

func (t *Tables) ByModel(name string) *Table {
    var found *Table
    t.tables.Range(func(typ reflect.Type, table *Table) bool {
        if table.TypeName == name {
            found = table
            return false
        }
        return true
    })
    return found
}

The Range function provides a view into the stored key value pairs of reflect.Type to *Table , but it does not guarantee the order of the items returned which means that in some iterations we’ll have our last registered Table come first and vice versa. This, together with the permissive check of table.TypeName == name which will be true in cases where our model has the the same case insensitive name, causes the table resolution to fail sporadically when creating fixtures.

Proposal

There doesn’t seem to be a very elegant solution to this issue since the problem only arises when using fixtures. An idea could be to make the Range check more restrictive by verifying that all the fields of our fixture are included. This is perhaps the least intrusive way to solve the issue and would include the following changes:

fixture.go

func (f *Fixture) addFixture(ctx context.Context, data *fixtureData) error {
    // Send in the fields when fetching our model here.
    table := f.db.Dialect().Tables().ByModelAndFields(data.Model, data.geFields())
    
    // ... rest of the code remains the same

    return nil
}

// Gets all fields in a fixture row excluding fields starting with _
func (rs *fixtureData) geFields() []string {
    var fields []string
        if len(rs.Rows == 0) {
                return fields
        }

    for k := range rs.Rows[0] {
        if !strings.HasPrefix(k, "_") {
            fields = append(fields, k)
        }
        }
    return fields
}

tables.go

// Changed to be more specific
func (t *Tables) ByModelAndFields(name string, fields []string) *Table {
    var found *Table
    t.tables.Range(func(typ reflect.Type, table *Table) bool {
        if table.TypeName == name && containsAllFields(table.allFields, fields) {
            found = table
            return false
        }
        return true
    })
    return found
}

// Helper function to check if a slice of field names contains a subset of strings 
func containsAllFields(set []*Field, subset []string) bool {
    if len(set) > len(subset) {
        return false
    }
    if len(subset) == 0 {
        return true
    }

    setMap := make(map[string]struct{}, len(set))
    for _, s := range set {
        setMap[s.Name] = struct{}{}
    }

    for _, s := range subset {
        if _, ok := setMap[s]; !ok {
            return false
        }
    }
    return true
}

I'd be happy to discuss a solution and / or make a PR with a solution. 😄

[github-actions]

This issue has been automatically marked as stale because it has not had activity in the last 30 days. If there is no update within the next 7 days, this issue will be closed.