Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Empty model on BeforeInsert hook #24

Closed
jpascal opened this issue Jun 16, 2021 · 7 comments
Closed

Empty model on BeforeInsert hook #24

jpascal opened this issue Jun 16, 2021 · 7 comments

Comments

@jpascal
Copy link

jpascal commented Jun 16, 2021

My User model

type User struct {
	ID        uuid.UUID `gorm:"default:uuid_generate_v4();->"`
	CreatedAt time.Time
	UpdatedAt time.Time
	Admin          bool
}
func (u *User) BeforeInsert(ctx context.Context, query *bun.InsertQuery) error {
  // u.Email => "" not "user@domain.local"
  u.CreatedAt = time.Now()
  u.UpdatedAt = time.Now()
 return nil
}
var signUpUser = &User{
   Email: "user@domain.local"
}
tx.NewInsert().Model(signUpUser).Exec(ctx)
INSERT INTO "users" ("id", "created_at", "updated_at", "email") VALUES (DEFAULT, '0001-01-01 00:00:00+00:00', '0001-01-01 00:00:00+00:00', '', 'user@domain.local') RETURNING "id"
created_at => 0
updated_at => 0

Why I must use query.GetModel() all time instead use self object?

I have this code:

type ChainHookFn func(context.Context) error

func HooksChain(ctx context.Context, hookFns ...ChainHookFn) error {
	for _, hookFn := range hookFns {
		if err := hookFn(ctx); err != nil {
			return err
		}
	}
	return nil
}

I have hook chains in my models and all was good)

func (u *User) BeforeUpdate(ctx context.Context, query *bun.UpdateQuery) error {
	return HooksChain(ctx, u.updatePassword, u.validate)
}
func (u *User) updatePassword(_ context.Context) error {
	if len(u.Password) > 0 {
		if result, err := bcrypt.GenerateFromPassword([]byte(u.Password), bcrypt.DefaultCost); err != nil {
			return err
		} else {
			u.HashedPassword = string(result)
		}
	}
	return nil
}

func (u User) validate(ctx context.Context) error {
	validators := []*validator.Validator{
		validator.Validate("email", u.Email).Rules(
			rules.IsPresence(),
			rules.IsEmail()
		),
	}
	if len(u.HashedPassword) == 0 {
		validators = append(validators, validator.Validate("password", u.Password).Rules(
			rules.IsPresence(),
		))
	}
	return validator.Scope(validators...).Validate(ctx)
}

I can create long hook-chains for each models by logic.
After update, I must all time use query.Get Model().Value().(*SomeModel) boilerplate... :( and must thinking about type casting.

@vmihailenco
Copy link
Member

vmihailenco commented Jun 16, 2021

Why I must use query.GetModel() all time instead use self object?

Because Bun also works with slice-based models, e.g. *[]User. And in some cases it might be useful to have access to a whole slice.


Overall it is best to only use hooks as a last resort when everything else fails. In your case you should create InsertUser func and have everything there.

@vmihailenco
Copy link
Member

Also see https://bun.uptrace.dev/guide/hooks.html#disclaimer

@jpascal
Copy link
Author

jpascal commented Jun 16, 2021

Overall it is best to only use hooks as a last resort when everything else fails. In your case you should create InsertUser func and have everything there.

If I will create function Insert User and for other operations like this, then I didn't see some profit from model hooks overall. ) May be model hooks in this lib must not to be? :)

@jpascal
Copy link
Author

jpascal commented Jun 16, 2021

Another case...
I have models User and Profile.
One logic need to create user directly, other need create user and profile inside transaction. In this case I must realize InsertUser(bun.DB), InsertUserTx(bun.Tx), InsertProfile(bun.DB), InsertProfileTx(bun.Tx), ... a lot of code ... In old variant of hooks all work perfectly.

@jpascal
Copy link
Author

jpascal commented Jun 16, 2021

Better example for https://bun.uptrace.dev/guide/hooks.html#disclaimer

func UpdateUser(ctx context.Context, query bun.UpdateQuery, user *User) error { ... }
func InsertUser(ctx context.Context, query bun.InsertQuery, user *User) error { ... }

@vmihailenco
Copy link
Member

I am not sure sure how you managed transactions with hooks, but that does not sound right :)

A better solution would be to create a common interface for bun.DB and bun.Tx. They both have methods like NewSelect and NewInsert so you can pass a tx instead of db:

tx := db.Begin()
InsertUser(ctx, tx)
InsertProfile(ctx, tx)
tx.Commit()

I guess Bun must provide such interface.

May be model hooks in this lib must not to be? :)

Ideally, yes. Practically you should not use hooks for such common things like transactions and validation.

@vmihailenco
Copy link
Member

IDB was added in v0.2.1. See https://github.com/uptrace/bun/tree/master/example/tx-composition

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants