この記事では、Clean Architecture を中心に利用されるリポジトリパターンに関するよくあるミスや気をつけるべき点を紹介したいと思います。
少しテクニカルなところから話を始めて、抽象的な問題（設計・DX）へ移っていきたいと思います。
※ここで言う DX は、デベロッパーエクスペリエンスのことです。
不適切なパラメータバインディング
func (r *Repo) GetByName(name string) (*user.User, error) {
query := fmt.Sprintf("select * from users where name = '%s'", name)
_ = r.db.QueryRow(query).Scan(/* ... */)
}
こちらは完全にアウトです。このようなコードを書く人はめったにいないと思いますが、念の為に書いておきました。
正しくは以下の通りです。
func (r *Repo) GetByName(name string) (*user.User, error) {
_ = r.db.QueryRow("select * from users where name = ?", name).Scan(/* ... */)
}
context.Context を利用しない
row := tx.QueryRow("select * from users where id = ?", id)
こちらもご存知の方が多いかと思いますが、context.Context を使わないとタイムアウトなどを適用できないので、～Context がついているメソッドにしましょう。
row := tx.QueryRowContext(ctx, "select * from users where id = ?", id)
Context がついていないほうのメソッドは、もはや廃止だと考えて良いでしょう。使うべきではありません。
ロールバックエラーに対応しない
defer tx.Rollback()
こちらは結構見るコードです。Rollback は実はエラーを返しているのです。
しかもいつものエラーチェックを単純に行うと以下の様になるかと思いますが、
defer func() {
if err := tx.Rollback(); err != nil {
return fmt.Errorf("rollback: %w", err)
}
}()
これだと元のエラーを上書きしてしまいますね。
ログに出して、返却しないという選択肢も一応ありますが、エラーが発生したのに返却されないのはよろしくないと思います。この問題、実は割ときれいに解決できるのです。以下のコードを見てみてください。
func (r *Repo) CreateUser(ctx context.Context, usr user.User) (err error) {
err = r.db.BeginTx(ctx, nil)
if err != nil {
return fmt.Errorf("begin insert user: %w", err)
}
defer func() {
if err != nil {
if rollbackErr := tx.Rollback(); rollbackErr != nil {
err = errors.Join(err, fmt.Errorf("rollback insert user: %w", rollbackErr))
}
}
}()
// ...
return err
}
まずは、戻り値の error に変数名をつける（named return value にする）ことで defer の中で確認することができるようになります。そもそもエラーがなければロールバックはしなくて良いはずですね。
次に、ロールバックのエラーをチェックして、なにか問題があった場合に err とくっつけます。
こうすることで CreateUser は２つのエラーを一つの error として返すことができます。
BeginTx のエラーと同じように fmt.Errorf("%w, rollback: %w", err, rollbackErr) でラップすることもできますが、よく考えてみるとわかるように、ロールバックのエラーは元のエラーとは関係ありません。単純に全く関係のないエラーが別々に発生しただけなので、errors.Join のほうが相応しいでしょう。
注意点としては、named return を使っているため、若干、別関数にまとめ辛くなります。
リソースを開放しない
func (r *Repo) GetUsers(ctx context.Context, filter Filter) ([]user.User, error) {
rows, _ := r.db.Query("SELECT * FROM users where ...")
// 変換
return users, nil
}
こちらの落とし穴、気づかない方も多いと思います。特に ORM を使っている場合は、気にしなくても良いライブラリもあるかもしれませんが、database/sql をそのまま使うと rows を自分で Close しなければならないケースもあります。
func (r *Repo) GetUsers(ctx context.Context, filter Filter) ([]user.User, error) {
rows, _ := r.db.Query("SELECT * FROM users where ...")
defer rows.Close()
// 変換
return users, nil
}
ただ、Close も、トランザクションと同様、エラーが発生する可能性があるため、本来なら特別にラップしないと拾えません。
defer func() {
if closeErr := rows.Close(); closeErr != nil {
err = errors.Join(err, closeErr)
}
}
とはいえ、Close が失敗したとわかったところでどうにもできませんし、処理自体にも影響しないので、こちらのエラーは無視しても問題ありません。気になる方はログに出しても良いと思います。
コネクションプールの設定不備
db, _ := sql.Open(/* ... */)
return db
Open したDBコネクションプールの設定をせずにそのまま使ってしまうと、意外なところでエラーが発生したり、パフォーマンスが悪くなったりする可能性があります。
db, _ := sql.Open(/* ... */)
db.SetMaxOpenConns(25)
db.SetMaxIdleConns(25)
db.SetConnMaxLifetime(5 * time.Minute)
return db
もちろん、具体的な設定はサービスによって異なりますので、調査した上で設定値を決めてください。
sql.ErrNoRows をチェックしない
func (r *Repo) FindUserByID(id user.ID) (*user.User, error) {
var usr user.User
err := r.db.QueryRow("select * from users where id = ?", id).Scan(/*...*/)
if err != nil {
return nil, err
}
return &usr, nil
}
この実装では、ユーザが見つからなかった場合はエラーが返却されます。つまり、サービス層で sql.ErrNoRows をチェックする必要があるということは、リポジトリの内部仕様が外へ漏れてしまうということです。
この問題に対しては、２つのアプローチがあると思います。
- nil, nil を返却する
err := r.db.QueryRowContext(ctx, "select * from users where id = ?", id).Scan(/*...*/)
if err != nil {
if errors.Is(err, sql.ErrNoRows) {
return nil, nil
}
return nil, err
}
return &user, nil
- ドメインエラーを返却する
err := r.db.QueryRowContext(ctx, "select * from users where id = ?", id).Scan(/*...*/)
if err != nil {
if errors.Is(err, sql.ErrNoRows) {
return nil, user.NotFoundError
}
return nil, err
}
return &usr, nil
- (value, bool, error) を返却する
err := r.db.QueryRowContext(ctx, "select * from users where id = ?", id).Scan(/*...*/)
if err != nil {
if errors.Is(err, sql.ErrNoRows) {
return user.User{}, false, nil
}
return nil, false, err
}
return usr, true, nil
基本的にどちらでも構いませんが、1 の場合は panic の発生する可能性が高くなると思います。
usr, err := s.repo.GetUser(ctx, cmd.UserID)
if err != nil {
return fmt.Errorf("get user: %w", err)
}
usr.SetName(cmd.UserName) // nil panic
上記のコードはとても自然な書き方ですが、仕様として nil, nil の可能性を常に頭に入れておかなければなりません。
もちろん、返却されるデータがポインターであるということは、当然 nil の可能性もありますが、err が無い場合、データも nil にはなっていないような設計も割と一般的ではないでしょうか。
個人的には 2 のほうがおすすめです。
何でもポインターにする
FindUserByID(id user.ID) (*user.User, error)
前項と関連しますが、リポジトリメソッドを設計する際、本当に返却値をポインターにする必要性があるかどうかを考えてみてください。
一般的には、返却されるデータを変更したい場合はポインターで、変更しない場合は値型が良いと言われています。しかし、Go には直和型がないため、「データがない」ことを表現するために nil を利用する方も多いでしょう。
ただ、nil という概念を考え出した Tony Hoare 氏はそれを "Billion Dollar Mistake" と表現しました。この世から nil という概念を追放することを目的としたプログラミング言語さえあります。
nil は避けるべきだと思いませんか？
FindUserByID(id user.ID) (user.User, error)
前項のように「データがない」ことをエラーで表現できるので、デフォルトは上記のようなコードで始めても良いのではないでしょうか。必要に応じて後々ポインターにすることも可能です。
ビジネスロジックの混在
func (r *Repo) CreateUser(ctx context.Context, usr *user.User) error {
if usr.Age < 18 {
return errors.New("user must be 18 or older")
}
// ...
}
リポジトリはあくまでもデータを保存するための手段なので、ドメインロジック・ビジネスロジックを入れるべきではありません。
年齢チェックはサービス層、もしくはドメイン内で行うべきですね。
func (s *Service) CreateUser(ctx context.Context, cmd user.CreateCommand) error {
usr, err := user.New(
cmd.Name,
cmd.Age,
)
if err != nil {
return err // "user must be 18 or older" など
}
_ = s.repo.CreateUser(ctx, usr)
// ...
}
トランザクションを利用しない
func (s *Service) UpdateUserName(ctx context.Context, cmd user.UpdateNameCommand) error {
usr, _ := s.repo.GetUser(ctx, cmd.UserID)
_ = usr.SetName(cmd.UserName)
_ = repo.UpdateUser(ctx, cmd.UserID, usr)
return nil
}
はい、アウトです。トランザクションを張っていないので、アトミックな処理になっていません。
トランザクションをビジネスロジックで利用する
func (s *Service) UpdateUserName(ctx context.Context, cmd user.UpdateNameCommand) error {
tx, _ := s.repo.BeginTx(ctx)
// defer tx.Rollback ？？
usr, _ := tx.GetUser(ctx, cmd.UserID)
_ = usr.SetName(cmd.UserName)
_ = tx.UpdateUser(ctx, cmd.UserID, usr)
_ = tx.Commit()
return nil
}
これもアウトです。リポジトリの内部仕様が外へ漏れてしまっています。リポジトリがトランザクションに対応しているかどうかは、リポジトリの利用者が知らなくて良い情報です。
では、どうしたら良いでしょうか？
コールバックを用意すればアトミックなアップデートをきれいに書くことができます。コードは以下の通りです。
func (s *Service) UpdateUserName(ctx context.Context, cmd user.UpdateNameCommand) error {
_, _ = s.repo.GetAndUpdateUser(ctx, cmd.UserID, func(usr *user.User) error {
_ = usr.SetName(cmd.UserName)
return nil
})
return nil
}
リポジトリ側はこうなります。
func (r *Repo) GetAndUpdate(
ctx context.Context,
id user.ID,
updateFunc func(user *user.User) error,
) (err error) {
tx, _ := r.db.BeginTx(ctx)
// rollback
row := tx.QueryRowContext(
ctx,
"select * from users where id = ? for update",
id)
usr := toEntity(row)
err := updateFunc(&usr)
if err != nil {
// 呼び出し先が指定するエラーなので、ラップしない
return err
}
_ = tx.ExecContext("update users set ... where id = ?", id)
_ = tx.Commit()
return nil
}
このパターンはトランザクションをサポートしていないストレージにも利用できますし、カスタマイズ性も非常に高いので、使ってみてはいかがでしょうか。
以下はアウトボックスパターンのアレンジです。
func (s *Service) CreateUser(ctx context.Context, cmd user.CreateCommand) error {
// ...
_, _ = s.repo.CreateWithEvent(ctx, usr, func() error {
if err := event.Publish(user.CreatedEvent, usr.ID); err != nil {
// エラーがあれば自動ロールバック
return fmt.Errorf("publish user created event: %w", err)
}
return nil
})
return nil
}
使い方は色々あると思いますが、昔の JavaScript のようなコールバック地獄にならないように気をつけてください。
複雑なユースケース
単純なコールバックで表現できない複雑なトランザクションや、複数のリポジトリやサービスを跨いだトランザクションの場合は、ビジネスロジック内でトランザクションを明確にせざるを得ないケースもあると思います。その場合は、 Unit of Work パターンや Saga パターンを参考に設計してください。
テーブルごとにリポジトリを定義する
type UserRepository interface {/* ... */}
type UserDetailsRepository interface {/* ... */}
UserDetails という特殊なエンティティでなければ、これらのリポジトリは UserRepository にまとめるべきです。もう少し一般的に言うと、１つのリポジトリ = １つのテーブルにする必要は一切ありません。もちろん、あらゆるテーブルからデータを集めている場合はリポジトリと名乗ったクエリーサービスになりますが、基本的に１つのリポジトリ = １つのエンティティで設計したほうが後々楽になると思います。
以下の設計で十分です。
type UserRepository interface {/* ... */}
type UserQueryService interface {/* ... */}
終わりに
以上が、今まで出会ったことのあるリポジトリの落とし穴でした。割と当たり前なところもあると思いますが、意外と知られていないことも書いたつもりです。
「自分のコードを修正しないと」と思った方や、「こういう落とし穴もあるよ」と言った追加情報をお持ちの方、ぜひコメントをください！
Discussion