From 366386c8bfc32f63f7e86e3f5115aaa6b95eb835 Mon Sep 17 00:00:00 2001 From: Robin Dittmar Date: Wed, 9 Sep 2026 16:25:48 +0200 Subject: [PATCH 1/5] test: re-worked user repo tests --- internal/api/handler/version.go | 6 +- internal/repository/user_test.go | 347 ++++++++++++++++--------------- 2 files changed, 188 insertions(+), 165 deletions(-) diff --git a/internal/api/handler/version.go b/internal/api/handler/version.go index b6104ce..bb30fd6 100644 --- a/internal/api/handler/version.go +++ b/internal/api/handler/version.go @@ -6,7 +6,7 @@ import ( "git.dittmar.dev/robin/dttmr-api/internal/api/response" ) -type VersionResponse struct { +type versionResponse struct { Version string `json:"version"` Commit string `json:"commit"` BuildTime string `json:"buildTime"` @@ -19,11 +19,11 @@ type VersionResponse struct { // @Tags Version // @Accept json // @Produce json -// @Success 200 {object} VersionResponse +// @Success 200 {object} versionResponse // @Router /version [get] func VersionHandler(version string, commit string, buildTime string) func(http.ResponseWriter, *http.Request) { return func(w http.ResponseWriter, r *http.Request) { - response.JSON(r.Context(), w, http.StatusOK, VersionResponse{ + response.JSON(r.Context(), w, http.StatusOK, versionResponse{ Version: version, Commit: commit, BuildTime: buildTime, diff --git a/internal/repository/user_test.go b/internal/repository/user_test.go index 3adb480..43a2a87 100644 --- a/internal/repository/user_test.go +++ b/internal/repository/user_test.go @@ -1,185 +1,208 @@ package repository import ( - "fmt" + "context" + "database/sql" + "errors" + "regexp" "testing" "time" - "git.dittmar.dev/robin/dttmr-api/internal/domain" "github.com/DATA-DOG/go-sqlmock" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "git.dittmar.dev/robin/dttmr-api/internal/domain" +) + +func newUserRepo(t *testing.T) (*UserRepo, sqlmock.Sqlmock) { + t.Helper() + + db, mock, err := sqlmock.New() + require.NoError(t, err) + + t.Cleanup(func() { + assert.NoError(t, mock.ExpectationsWereMet()) + _ = db.Close() + }) + + return &UserRepo{Repo: NewRepo(NewTransactor(db))}, mock +} + +const ( + insertUserQuery = `INSERT INTO users (email, name, password_hash) VALUES ($1, $2, $3) RETURNING id, created_at` + deleteUserQuery = `DELETE FROM users WHERE id = $1` + updatePassQuery = `UPDATE users SET password_hash = $1 WHERE id = $2` + selectUserQuery = `SELECT id, email, name FROM users WHERE email = $1` ) func TestUserRepo_CreateUser(t *testing.T) { - db, mock, err := sqlmock.New() - assert.NoError(t, err) - defer db.Close() - - //repo := NewUserRepo(db) - - //ctx := context.Background() - email := "test@example.com" - name := "Test User" - passwordHash := "hashedpassword123" - - now := time.Now() - expectedUser := &domain.User{ - ID: "1", - Email: email, - Name: name, - CreatedAt: now, - } - t.Run("success", func(t *testing.T) { - mock.ExpectBegin() + repo, mock := newUserRepo(t) - mock.ExpectQuery(`^INSERT INTO users \(email, name, password_hash\) VALUES \(\$1, \$2, \$3\) RETURNING id, created_at$`). - WithArgs(email, name, passwordHash). - WillReturnRows(sqlmock.NewRows([]string{"id", "created_at"}).AddRow(expectedUser.ID, expectedUser.CreatedAt)) + createdAt := time.Date(2026, 9, 9, 10, 0, 0, 0, time.UTC) + mock.ExpectQuery(regexp.QuoteMeta(insertUserQuery)). + WithArgs("robin@dittmar.dev", "Robin", "$2a$10$hash"). + WillReturnRows( + sqlmock.NewRows([]string{"id", "created_at"}). + AddRow("2f1c...", createdAt), + ) - mock.ExpectCommit() + user, err := repo.CreateUser(context.Background(), "robin@dittmar.dev", "Robin", "$2a$10$hash") - //user, err := repo.CreateUser(ctx, email, name, passwordHash) - //assert.NoError(t, err) - //assert.Equal(t, expectedUser, user) - //assert.NoError(t, mock.ExpectationsWereMet()) + require.NoError(t, err) + require.NotNil(t, user) + assert.Equal(t, &domain.User{ + ID: "2f1c...", + Email: "robin@dittmar.dev", + Name: "Robin", + CreatedAt: createdAt, + }, user) }) - t.Run("begin_tx_error", func(t *testing.T) { - mock.ExpectBegin().WillReturnError(fmt.Errorf("tx error")) + t.Run("db error is wrapped", func(t *testing.T) { + repo, mock := newUserRepo(t) - //user, err := repo.CreateUser(ctx, email, name, passwordHash) - //assert.Error(t, err) - //assert.Contains(t, err.Error(), "begin transaction") - //assert.Nil(t, user) - //assert.NoError(t, mock.ExpectationsWereMet()) - }) + dbErr := errors.New("duplicate key value violates unique constraint") + mock.ExpectQuery(regexp.QuoteMeta(insertUserQuery)). + WithArgs("robin@dittmar.dev", "Robin", "$2a$10$hash"). + WillReturnError(dbErr) - t.Run("insert_error", func(t *testing.T) { - mock.ExpectBegin() - mock.ExpectQuery(`^INSERT INTO users \(email, name, password_hash\) VALUES \(\$1, \$2, \$3\) RETURNING id, created_at$`). - WithArgs(email, name, passwordHash). - WillReturnError(fmt.Errorf("insert error")) - mock.ExpectRollback() + user, err := repo.CreateUser(context.Background(), "robin@dittmar.dev", "Robin", "$2a$10$hash") - //user, err := repo.CreateUser(ctx, email, name, passwordHash) - //assert.Error(t, err) - //assert.Contains(t, err.Error(), "failed to insert user") - //assert.Nil(t, user) - //assert.NoError(t, mock.ExpectationsWereMet()) - }) - - t.Run("commit_error", func(t *testing.T) { - mock.ExpectBegin() - mock.ExpectQuery(`^INSERT INTO users \(email, name, password_hash\) VALUES \(\$1, \$2, \$3\) RETURNING id, created_at$`). - WithArgs(email, name, passwordHash). - WillReturnRows(sqlmock.NewRows([]string{"id", "created_at"}).AddRow(expectedUser.ID, expectedUser.CreatedAt)) - mock.ExpectCommit().WillReturnError(fmt.Errorf("commit error")) - - //user, err := repo.CreateUser(ctx, email, name, passwordHash) - //assert.Error(t, err) - //assert.Contains(t, err.Error(), "commit transaction") - //assert.Nil(t, user) - //assert.NoError(t, mock.ExpectationsWereMet()) + assert.Nil(t, user) + assert.ErrorIs(t, err, dbErr) + assert.ErrorContains(t, err, "failed to insert user") }) } -//func TestUserRepo_CreateUser2(t *testing.T) { -// email := "test@example.com" -// name := "Test User" -// passwordHash := "hashedpassword123" -// now := time.Now() -// expectedID := "42" -// -// insertQuery := regexp.QuoteMeta( -// "INSERT INTO users (email, name, password_hash) VALUES ($1, $2, $3) RETURNING id, created_at", -// ) -// -// testCases := []struct { -// name string -// setupMock func(mock sqlmock.Sqlmock) -// expectedError string -// }{ -// { -// name: "Success: User created perfectly", -// setupMock: func(mock sqlmock.Sqlmock) { -// mock.ExpectBegin() -// -// rows := sqlmock.NewRows([]string{"id", "created_at"}). -// AddRow(expectedID, now) -// -// mock.ExpectQuery(insertQuery). -// WithArgs(email, name, passwordHash). -// WillReturnRows(rows) -// -// mock.ExpectCommit() -// }, -// expectedError: "", -// }, -// { -// name: "Failure: Database connection fails on BeginTx", -// setupMock: func(mock sqlmock.Sqlmock) { -// mock.ExpectBegin().WillReturnError(errors.New("db connection failed")) -// }, -// expectedError: "begin transaction: db connection failed", -// }, -// { -// name: "Failure: Query fails (e.g., duplicate email)", -// setupMock: func(mock sqlmock.Sqlmock) { -// mock.ExpectBegin() -// -// mock.ExpectQuery(insertQuery). -// WithArgs(email, name, passwordHash). -// WillReturnError(errors.New("unique constraint violation")) -// -// mock.ExpectRollback() -// }, -// expectedError: "failed to insert user: unique constraint violation", -// }, -// { -// name: "Failure: Commit fails (e.g., network timeout)", -// setupMock: func(mock sqlmock.Sqlmock) { -// mock.ExpectBegin() -// -// rows := sqlmock.NewRows([]string{"id", "created_at"}). -// AddRow(expectedID, now) -// -// mock.ExpectQuery(insertQuery). -// WithArgs(email, name, passwordHash). -// WillReturnRows(rows) -// -// mock.ExpectCommit().WillReturnError(errors.New("commit timeout")) -// }, -// expectedError: "commit transaction: commit timeout", -// }, -// } -// -// for _, tc := range testCases { -// t.Run(tc.name, func(t *testing.T) { -// db, mock, err := sqlmock.New() -// require.NoError(t, err) -// defer db.Close() -// -// tc.setupMock(mock) -// -// //repo := NewUserRepo(db) -// // -// //user, err := repo.CreateUser(context.Background(), email, name, passwordHash) -// -// if tc.expectedError != "" { -// require.Error(t, err) -// assert.Contains(t, err.Error(), tc.expectedError) -// //assert.Nil(t, user) -// } else { -// require.NoError(t, err) -// //require.NotNil(t, user) -// //assert.Equal(t, expectedID, user.ID) -// //assert.Equal(t, email, user.Email) -// //assert.Equal(t, name, user.Name) -// //assert.Equal(t, now, user.CreatedAt) -// } -// -// assert.NoError(t, mock.ExpectationsWereMet()) -// }) -// } -//} +func TestUserRepo_DeleteUser(t *testing.T) { + t.Run("success", func(t *testing.T) { + repo, mock := newUserRepo(t) + + mock.ExpectExec(regexp.QuoteMeta(deleteUserQuery)). + WithArgs("user-1"). + WillReturnResult(sqlmock.NewResult(0, 1)) + + assert.NoError(t, repo.DeleteUser(context.Background(), "user-1")) + }) + + t.Run("unknown id is not reported", func(t *testing.T) { + repo, mock := newUserRepo(t) + + mock.ExpectExec(regexp.QuoteMeta(deleteUserQuery)). + WithArgs("does-not-exist"). + WillReturnResult(sqlmock.NewResult(0, 0)) + + assert.NoError(t, repo.DeleteUser(context.Background(), "does-not-exist")) + }) + + t.Run("db error is wrapped", func(t *testing.T) { + repo, mock := newUserRepo(t) + + dbErr := errors.New("connection reset") + mock.ExpectExec(regexp.QuoteMeta(deleteUserQuery)). + WithArgs("user-1"). + WillReturnError(dbErr) + + err := repo.DeleteUser(context.Background(), "user-1") + + assert.ErrorIs(t, err, dbErr) + assert.ErrorContains(t, err, "failed to delete user") + }) +} + +func TestUserRepo_ChangePassword(t *testing.T) { + t.Run("success", func(t *testing.T) { + repo, mock := newUserRepo(t) + + mock.ExpectExec(regexp.QuoteMeta(updatePassQuery)). + WithArgs("$2a$10$newhash", "user-1"). + WillReturnResult(sqlmock.NewResult(0, 1)) + + assert.NoError(t, repo.ChangePassword(context.Background(), "user-1", "$2a$10$newhash")) + }) + + t.Run("db error is wrapped", func(t *testing.T) { + repo, mock := newUserRepo(t) + + dbErr := errors.New("deadlock detected") + mock.ExpectExec(regexp.QuoteMeta(updatePassQuery)). + WithArgs("$2a$10$newhash", "user-1"). + WillReturnError(dbErr) + + err := repo.ChangePassword(context.Background(), "user-1", "$2a$10$newhash") + + assert.ErrorIs(t, err, dbErr) + assert.ErrorContains(t, err, "failed to update user") + }) +} + +func TestUserRepo_UsesTransactionFromContext(t *testing.T) { + repo, mock := newUserRepo(t) + + mock.ExpectBegin() + mock.ExpectExec(regexp.QuoteMeta(deleteUserQuery)). + WithArgs("user-1"). + WillReturnResult(sqlmock.NewResult(0, 1)) + mock.ExpectCommit() + + tx, err := repo.db.BeginTx(context.Background(), nil) + require.NoError(t, err) + + ctx := context.WithValue(context.Background(), txKey{}, tx) + require.NoError(t, repo.DeleteUser(ctx, "user-1")) + require.NoError(t, tx.Commit()) +} + +func TestUserRepo_GetUserByEmail(t *testing.T) { + t.Run("success", func(t *testing.T) { + repo, mock := newUserRepo(t) + + mock.ExpectQuery(regexp.QuoteMeta(selectUserQuery)). + WithArgs("robin@dittmar.dev"). + WillReturnRows( + sqlmock.NewRows([]string{"id", "email", "name"}). + AddRow("user-1", "robin@dittmar.dev", "Robin"), + ) + + user, err := repo.GetUserByEmail(context.Background(), "robin@dittmar.dev") + + require.NoError(t, err) + require.NotNil(t, user) + assert.Equal(t, "user-1", user.ID) + assert.Equal(t, "robin@dittmar.dev", user.Email) + assert.Equal(t, "Robin", user.Name) + assert.Zero(t, user.CreatedAt) // not selected by this query + }) + + t.Run("not found stays matchable via errors.Is", func(t *testing.T) { + repo, mock := newUserRepo(t) + + mock.ExpectQuery(regexp.QuoteMeta(selectUserQuery)). + WithArgs("nobody@dittmar.dev"). + WillReturnError(sql.ErrNoRows) + + user, err := repo.GetUserByEmail(context.Background(), "nobody@dittmar.dev") + + assert.Nil(t, user) + assert.ErrorIs(t, err, sql.ErrNoRows) + assert.ErrorContains(t, err, "failed to get user") + }) + + t.Run("scan error on type mismatch", func(t *testing.T) { + repo, mock := newUserRepo(t) + + mock.ExpectQuery(regexp.QuoteMeta(selectUserQuery)). + WithArgs("robin@dittmar.dev"). + WillReturnRows( + sqlmock.NewRows([]string{"id", "email", "name"}). + AddRow(nil, "robin@dittmar.dev", "Robin"), + ) + + user, err := repo.GetUserByEmail(context.Background(), "robin@dittmar.dev") + + assert.Nil(t, user) + assert.Error(t, err) + }) +} -- 2.54.0 From 4a967ef002b0c5d82fd78f10e7a3cbe7ed22db8b Mon Sep 17 00:00:00 2001 From: Robin Dittmar Date: Wed, 9 Sep 2026 16:30:43 +0200 Subject: [PATCH 2/5] fix: WithinTx now actually places the transaction into the context --- internal/repository/repo.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/repository/repo.go b/internal/repository/repo.go index b5cefb9..16a3c46 100644 --- a/internal/repository/repo.go +++ b/internal/repository/repo.go @@ -49,9 +49,9 @@ func (t *Transactor) WithinTx(ctx context.Context, fn func(context.Context) erro if err != nil { return err } - defer tx.Rollback() + defer func() { _ = tx.Rollback() }() - if err = fn(ctx); err != nil { + if err = fn(context.WithValue(ctx, txKey{}, tx)); err != nil { return err } return tx.Commit() -- 2.54.0 From 77a4ccc520186e0a9fbdd9cea1254cc29c5baf8a Mon Sep 17 00:00:00 2001 From: Robin Dittmar Date: Wed, 9 Sep 2026 16:33:47 +0200 Subject: [PATCH 3/5] fix: added missing spaces for tx savepoints --- internal/repository/repo.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/repository/repo.go b/internal/repository/repo.go index 16a3c46..2936993 100644 --- a/internal/repository/repo.go +++ b/internal/repository/repo.go @@ -64,10 +64,10 @@ func (t *Transactor) withinSavepoint(ctx context.Context, tx *sql.Tx, fn func(co } if err := fn(ctx); err != nil { - _, _ = tx.ExecContext(ctx, "ROLLBACK TO SAVEPOINT"+name) + _, _ = tx.ExecContext(ctx, "ROLLBACK TO SAVEPOINT "+name) return err } - _, err := tx.ExecContext(ctx, "RELEASE SAVEPOINT"+name) + _, err := tx.ExecContext(ctx, "RELEASE SAVEPOINT "+name) return err } -- 2.54.0 From c8a5da61ef0f865c67c93ae7e792726ebf5354f2 Mon Sep 17 00:00:00 2001 From: Robin Dittmar Date: Wed, 9 Sep 2026 16:34:26 +0200 Subject: [PATCH 4/5] fix: re-worked list tests go --- internal/domain/list_test.go | 384 +++++++++++++++++++++++++++++------ 1 file changed, 322 insertions(+), 62 deletions(-) diff --git a/internal/domain/list_test.go b/internal/domain/list_test.go index 28685b7..eeee2cd 100644 --- a/internal/domain/list_test.go +++ b/internal/domain/list_test.go @@ -1,4 +1,4 @@ -package domain_test +package domain import ( "context" @@ -6,77 +6,337 @@ import ( "testing" "time" - "git.dittmar.dev/robin/dttmr-api/internal/domain" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" ) -type mockListRepo struct { +type fakeTransactor struct { + calls int + err error +} + +func (f *fakeTransactor) WithinTx(ctx context.Context, fn func(ctx context.Context) error) error { + f.calls++ + if f.err != nil { + return f.err + } + return fn(ctx) +} + +type mockListRepository struct { mock.Mock } -func (m *mockListRepo) CreateList(ctx context.Context, name string, userIDs []string) (*domain.List, error) { - args := m.Called(ctx, name, userIDs) - var list *domain.List - if l := args.Get(0); l != nil { - list = l.(*domain.List) - } +func (m *mockListRepository) CreateList(ctx context.Context, name string) (*List, error) { + args := m.Called(ctx, name) + list, _ := args.Get(0).(*List) return list, args.Error(1) } -func TestListService_Create_Success(t *testing.T) { - expectedList := &domain.List{ - ID: "1", - Name: "My List", - CreatedAt: time.Now(), - ModifiedAt: time.Now(), +func (m *mockListRepository) DeleteList(ctx context.Context, listID string) error { + args := m.Called(ctx, listID) + return args.Error(0) +} + +func (m *mockListRepository) GetLists(ctx context.Context, userID string) ([]List, error) { + args := m.Called(ctx, userID) + lists, _ := args.Get(0).([]List) + return lists, args.Error(1) +} + +func (m *mockListRepository) AddUserToList(ctx context.Context, listID string, userID string) error { + args := m.Called(ctx, listID, userID) + return args.Error(0) +} + +func (m *mockListRepository) RemoveUserFromList(ctx context.Context, listID string, userID string) error { + args := m.Called(ctx, listID, userID) + return args.Error(0) +} + +func (m *mockListRepository) IsUserInList(ctx context.Context, listID string, userID string) (bool, error) { + args := m.Called(ctx, listID, userID) + return args.Bool(0), args.Error(1) +} + +func (m *mockListRepository) IsUserInListByItemID(ctx context.Context, listItemID string, userID string) (bool, error) { + args := m.Called(ctx, listItemID, userID) + return args.Bool(0), args.Error(1) +} + +func (m *mockListRepository) CreateListItem(ctx context.Context, listID string, title string) (*ListItem, error) { + args := m.Called(ctx, listID, title) + item, _ := args.Get(0).(*ListItem) + return item, args.Error(1) +} + +func (m *mockListRepository) DeleteListItem(ctx context.Context, listItemID string) error { + args := m.Called(ctx, listItemID) + return args.Error(0) +} + +func (m *mockListRepository) UpdateListItem(ctx context.Context, listItemID string, title string, isCompleted bool) error { + args := m.Called(ctx, listItemID, title, isCompleted) + return args.Error(0) +} + +func (m *mockListRepository) SetListItemTitle(ctx context.Context, listItemID string, title string) error { + args := m.Called(ctx, listItemID, title) + return args.Error(0) +} + +func (m *mockListRepository) SetListItemCompleted(ctx context.Context, listItemID string, isCompleted bool) error { + args := m.Called(ctx, listItemID, isCompleted) + return args.Error(0) +} + +func (m *mockListRepository) GetListItems(ctx context.Context, listID string) ([]ListItem, error) { + args := m.Called(ctx, listID) + items, _ := args.Get(0).([]ListItem) + return items, args.Error(1) +} + +func newListService(t *testing.T) (*ListService, *mockListRepository, *fakeTransactor) { + t.Helper() + + repo := &mockListRepository{} + t.Cleanup(func() { repo.AssertExpectations(t) }) + + tx := &fakeTransactor{} + + return NewListService(tx, repo), repo, tx +} + +func TestListService_CreateList(t *testing.T) { + t.Run("creates the list and adds the owner", func(t *testing.T) { + svc, repo, tx := newListService(t) + + created := &List{ID: "list-1", Name: "Groceries", CreatedAt: time.Now()} + repo.On("CreateList", mock.Anything, "Groceries").Return(created, nil) + repo.On("AddUserToList", mock.Anything, "list-1", "user-1").Return(nil) + + list, err := svc.CreateList(context.Background(), "user-1", "Groceries") + + require.NoError(t, err) + assert.Equal(t, created, list) + assert.Equal(t, 1, tx.calls) + }) + + t.Run("empty name is rejected before any repo call", func(t *testing.T) { + svc, _, tx := newListService(t) + + list, err := svc.CreateList(context.Background(), "user-1", "") + + assert.Nil(t, list) + assert.ErrorIs(t, err, ErrListNameMissing) + assert.Zero(t, tx.calls) + }) + + t.Run("insert error aborts before adding the user", func(t *testing.T) { + svc, repo, _ := newListService(t) + + repoErr := errors.New("insert failed") + repo.On("CreateList", mock.Anything, "Groceries").Return(nil, repoErr) + + list, err := svc.CreateList(context.Background(), "user-1", "Groceries") + + assert.Nil(t, list) + assert.ErrorIs(t, err, repoErr) + repo.AssertNotCalled(t, "AddUserToList", mock.Anything, mock.Anything, mock.Anything) + }) + + t.Run("transaction error is returned", func(t *testing.T) { + svc, _, tx := newListService(t) + + tx.err = errors.New("could not begin transaction") + + list, err := svc.CreateList(context.Background(), "user-1", "Groceries") + + assert.Nil(t, list) + assert.ErrorIs(t, err, tx.err) + }) +} + +func TestListService_DeleteList(t *testing.T) { + t.Run("deletes when the user is a member", func(t *testing.T) { + svc, repo, _ := newListService(t) + + repo.On("IsUserInList", mock.Anything, "list-1", "user-1").Return(true, nil) + repo.On("DeleteList", mock.Anything, "list-1").Return(nil) + + assert.NoError(t, svc.DeleteList(context.Background(), "user-1", "list-1")) + }) + + t.Run("refuses when the user is not a member", func(t *testing.T) { + svc, repo, _ := newListService(t) + + repo.On("IsUserInList", mock.Anything, "list-1", "intruder").Return(false, nil) + + err := svc.DeleteList(context.Background(), "intruder", "list-1") + + assert.ErrorIs(t, err, ErrUserNotInList) + repo.AssertNotCalled(t, "DeleteList", mock.Anything, mock.Anything) + }) + + t.Run("membership check error is propagated", func(t *testing.T) { + svc, repo, _ := newListService(t) + + repoErr := errors.New("connection reset") + repo.On("IsUserInList", mock.Anything, "list-1", "user-1").Return(false, repoErr) + + err := svc.DeleteList(context.Background(), "user-1", "list-1") + + assert.ErrorIs(t, err, repoErr) + assert.NotErrorIs(t, err, ErrUserNotInList) + repo.AssertNotCalled(t, "DeleteList", mock.Anything, mock.Anything) + }) +} + +func TestListService_GetLists(t *testing.T) { + svc, repo, _ := newListService(t) + + want := []List{{ID: "list-1", Name: "Groceries"}, {ID: "list-2", Name: "Reading"}} + repo.On("GetLists", mock.Anything, "user-1").Return(want, nil) + + lists, err := svc.GetLists(context.Background(), "user-1") + + require.NoError(t, err) + assert.Equal(t, want, lists) +} + +func TestListService_CreateListItem(t *testing.T) { + t.Run("creates the item for a member", func(t *testing.T) { + svc, repo, _ := newListService(t) + + created := &ListItem{ID: "item-1", ListID: "list-1", Title: "Milk"} + repo.On("IsUserInList", mock.Anything, "list-1", "user-1").Return(true, nil) + repo.On("CreateListItem", mock.Anything, "list-1", "Milk").Return(created, nil) + + item, err := svc.CreateListItem(context.Background(), "user-1", "list-1", "Milk") + + require.NoError(t, err) + assert.Equal(t, created, item) + }) + + t.Run("refuses for a non-member", func(t *testing.T) { + svc, repo, _ := newListService(t) + + repo.On("IsUserInList", mock.Anything, "list-1", "intruder").Return(false, nil) + + item, err := svc.CreateListItem(context.Background(), "intruder", "list-1", "Milk") + + assert.Nil(t, item) + assert.ErrorIs(t, err, ErrUserNotInList) + repo.AssertNotCalled(t, "CreateListItem", mock.Anything, mock.Anything, mock.Anything) + }) +} + +func TestListService_SetListItemCompleted(t *testing.T) { + t.Run("checks membership through the item id", func(t *testing.T) { + svc, repo, _ := newListService(t) + + repo.On("IsUserInListByItemID", mock.Anything, "item-1", "user-1").Return(true, nil) + repo.On("SetListItemCompleted", mock.Anything, "item-1", true).Return(nil) + + assert.NoError(t, svc.SetListItemCompleted(context.Background(), "user-1", "item-1", true)) + }) + + t.Run("refuses for a non-member", func(t *testing.T) { + svc, repo, _ := newListService(t) + + repo.On("IsUserInListByItemID", mock.Anything, "item-1", "intruder").Return(false, nil) + + err := svc.SetListItemCompleted(context.Background(), "intruder", "item-1", false) + + assert.ErrorIs(t, err, ErrUserNotInList) + repo.AssertNotCalled(t, "SetListItemCompleted", mock.Anything, mock.Anything, mock.Anything) + }) +} + +func TestListService_ValidationErrors(t *testing.T) { + tests := []struct { + name string + call func(svc *ListService) error + wantErr error + }{ + { + name: "DeleteList without list id", + call: func(svc *ListService) error { return svc.DeleteList(context.Background(), "user-1", "") }, + wantErr: ErrListIDMissing, + }, + { + name: "AddUserToList without list id", + call: func(svc *ListService) error { + return svc.AddUserToList(context.Background(), "user-1", "", "user-2") + }, + wantErr: ErrListIDMissing, + }, + { + name: "AddUserToList without user id", + call: func(svc *ListService) error { + return svc.AddUserToList(context.Background(), "user-1", "list-1", "") + }, + wantErr: ErrUserIDMissing, + }, + { + name: "RemoveUserFromList without user id", + call: func(svc *ListService) error { + return svc.RemoveUserFromList(context.Background(), "user-1", "list-1", "") + }, + wantErr: ErrUserIDMissing, + }, + { + name: "CreateListItem without title", + call: func(svc *ListService) error { + _, err := svc.CreateListItem(context.Background(), "user-1", "list-1", "") + return err + }, + wantErr: ErrListItemTitleMissing, + }, + { + name: "DeleteListItem without item id", + call: func(svc *ListService) error { + return svc.DeleteListItem(context.Background(), "user-1", "") + }, + wantErr: ErrListItemIDMissing, + }, + { + name: "UpdateListItem without title", + call: func(svc *ListService) error { + return svc.UpdateListItem(context.Background(), "user-1", "item-1", "", false) + }, + wantErr: ErrListItemTitleMissing, + }, + { + name: "SetListItemTitle without item id", + call: func(svc *ListService) error { + return svc.SetListItemTitle(context.Background(), "user-1", "", "Milk") + }, + wantErr: ErrListItemIDMissing, + }, + { + name: "SetListItemCompleted without item id", + call: func(svc *ListService) error { + return svc.SetListItemCompleted(context.Background(), "user-1", "", true) + }, + wantErr: ErrListItemIDMissing, + }, + { + name: "GetListItems without list id", + call: func(svc *ListService) error { + _, err := svc.GetListItems(context.Background(), "user-1", "") + return err + }, + wantErr: ErrListIDMissing, + }, } - repo := new(mockListRepo) - repo.On("CreateList", mock.Anything, "My List", []string{"user1", "user2"}).Return(expectedList, nil) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + svc, _, _ := newListService(t) - //service := domain.NewListService(repo) - //list, err := service.CreateList(context.Background(), "My List", []string{"user1", "user2"}) - - //require.NoError(t, err) - //assert.Equal(t, expectedList, list) - //repo.AssertExpectations(t) -} - -func TestListService_Create_EmptyName(t *testing.T) { - repo := new(mockListRepo) - //service := domain.NewListService(repo) - - //list, err := service.CreateList(context.Background(), "", []string{"user1"}) - - //require.Error(t, err) - //assert.EqualError(t, err, "list name must not be empty") - //assert.Nil(t, list) - repo.AssertExpectations(t) -} - -func TestListService_Create_EmptyUsers(t *testing.T) { - repo := new(mockListRepo) - //service := domain.NewListService(repo) - - //list, err := service.CreateList(context.Background(), "My List", []string{}) - - //require.Error(t, err) - //assert.EqualError(t, err, "users must have at least one associated user") - //assert.Nil(t, list) - repo.AssertExpectations(t) -} - -func TestListService_Create_RepoError(t *testing.T) { - expectedErr := errors.New("database error") - repo := new(mockListRepo) - repo.On("CreateList", mock.Anything, "My List", []string{"user1"}).Return(nil, expectedErr) - - //service := domain.NewListService(repo) - - //list, err := service.CreateList(context.Background(), "My List", []string{"user1"}) - - //require.Error(t, err) - //assert.ErrorIs(t, err, expectedErr) - //assert.Nil(t, list) - //repo.AssertExpectations(t) + assert.ErrorIs(t, tt.call(svc), tt.wantErr) + }) + } } -- 2.54.0 From 726c3554eb0adb1561e6f31e33e237a98ca1d17d Mon Sep 17 00:00:00 2001 From: Robin Dittmar Date: Wed, 9 Sep 2026 16:41:30 +0200 Subject: [PATCH 5/5] feat: added repo_test.go --- internal/repository/repo_test.go | 302 +++++++++++++++++++++++++++++++ 1 file changed, 302 insertions(+) create mode 100644 internal/repository/repo_test.go diff --git a/internal/repository/repo_test.go b/internal/repository/repo_test.go new file mode 100644 index 0000000..552cc2e --- /dev/null +++ b/internal/repository/repo_test.go @@ -0,0 +1,302 @@ +package repository + +import ( + "context" + "database/sql" + "errors" + "testing" + + "github.com/DATA-DOG/go-sqlmock" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type unrelatedKey struct{} + +func newTransactor(t *testing.T) (*Transactor, sqlmock.Sqlmock) { + t.Helper() + + db, mock, err := sqlmock.New(sqlmock.QueryMatcherOption(sqlmock.QueryMatcherEqual)) + require.NoError(t, err) + + t.Cleanup(func() { + assert.NoError(t, mock.ExpectationsWereMet()) + _ = db.Close() + }) + + return NewTransactor(db), mock +} + +func TestNewRepo(t *testing.T) { + tr, _ := newTransactor(t) + + repo := NewRepo(tr) + + assert.Same(t, tr, repo.Transactor) +} + +func TestRepo_Conn(t *testing.T) { + t.Run("falls back to the pool without a transaction", func(t *testing.T) { + tr, mock := newTransactor(t) + repo := NewRepo(tr) + + mock.ExpectExec("DELETE FROM users WHERE id = $1"). + WithArgs("user-1"). + WillReturnResult(sqlmock.NewResult(0, 1)) + + ctx := context.Background() + assert.Same(t, tr.db, repo.conn(ctx)) + + _, err := repo.conn(ctx).ExecContext(ctx, "DELETE FROM users WHERE id = $1", "user-1") + require.NoError(t, err) + }) + + t.Run("returns the transaction carried by the context", func(t *testing.T) { + tr, mock := newTransactor(t) + repo := NewRepo(tr) + + mock.ExpectBegin() + mock.ExpectRollback() + + tx, err := tr.db.BeginTx(context.Background(), nil) + require.NoError(t, err) + + ctx := context.WithValue(context.Background(), txKey{}, tx) + assert.Same(t, tx, repo.conn(ctx)) + + require.NoError(t, tx.Rollback()) + }) + + t.Run("ignores values stored under other keys", func(t *testing.T) { + tr, _ := newTransactor(t) + repo := NewRepo(tr) + + ctx := context.WithValue(context.Background(), unrelatedKey{}, "irrelevant") + + assert.Same(t, tr.db, repo.conn(ctx)) + }) + + t.Run("ignores a value of the wrong type under txKey", func(t *testing.T) { + tr, _ := newTransactor(t) + repo := NewRepo(tr) + + ctx := context.WithValue(context.Background(), txKey{}, "not a transaction") + + assert.Same(t, tr.db, repo.conn(ctx)) + }) +} + +func TestRepo_WithinTxIsPromoted(t *testing.T) { + tr, mock := newTransactor(t) + repo := NewRepo(tr) + + mock.ExpectBegin() + mock.ExpectExec("DELETE FROM users WHERE id = $1"). + WithArgs("user-1"). + WillReturnResult(sqlmock.NewResult(0, 1)) + mock.ExpectCommit() + + err := repo.WithinTx(context.Background(), func(ctx context.Context) error { + _, execErr := repo.conn(ctx).ExecContext(ctx, "DELETE FROM users WHERE id = $1", "user-1") + return execErr + }) + + require.NoError(t, err) +} + +func TestRepo_SiblingReposShareTheTransaction(t *testing.T) { + tr, mock := newTransactor(t) + users := NewRepo(tr) + lists := NewRepo(tr) + + mock.ExpectBegin() + mock.ExpectExec("INSERT INTO lists (name) VALUES ($1)"). + WithArgs("Groceries"). + WillReturnResult(sqlmock.NewResult(1, 1)) + mock.ExpectExec("DELETE FROM users WHERE id = $1"). + WithArgs("user-1"). + WillReturnResult(sqlmock.NewResult(0, 1)) + mock.ExpectCommit() + + err := tr.WithinTx(context.Background(), func(ctx context.Context) error { + assert.Same(t, lists.conn(ctx), users.conn(ctx)) + + if _, err := lists.conn(ctx).ExecContext(ctx, "INSERT INTO lists (name) VALUES ($1)", "Groceries"); err != nil { + return err + } + + _, err := users.conn(ctx).ExecContext(ctx, "DELETE FROM users WHERE id = $1", "user-1") + return err + }) + + require.NoError(t, err) +} + +func TestTransactor_WithinTx(t *testing.T) { + t.Run("commits and routes statements through the tx", func(t *testing.T) { + tr, mock := newTransactor(t) + repo := NewRepo(tr) + + mock.ExpectBegin() + mock.ExpectExec("DELETE FROM users WHERE id = $1"). + WithArgs("user-1"). + WillReturnResult(sqlmock.NewResult(0, 1)) + mock.ExpectCommit() + + err := tr.WithinTx(context.Background(), func(ctx context.Context) error { + tx, ok := ctx.Value(txKey{}).(*sql.Tx) + require.True(t, ok) + assert.Same(t, tx, repo.conn(ctx)) + + _, execErr := repo.conn(ctx).ExecContext(ctx, "DELETE FROM users WHERE id = $1", "user-1") + return execErr + }) + + require.NoError(t, err) + }) + + t.Run("callback error rolls back and propagates", func(t *testing.T) { + tr, mock := newTransactor(t) + + mock.ExpectBegin() + mock.ExpectRollback() + + fnErr := errors.New("business rule violated") + err := tr.WithinTx(context.Background(), func(ctx context.Context) error { + return fnErr + }) + + assert.ErrorIs(t, err, fnErr) + }) + + t.Run("begin error skips the callback", func(t *testing.T) { + tr, mock := newTransactor(t) + + beginErr := errors.New("too many connections") + mock.ExpectBegin().WillReturnError(beginErr) + + called := false + err := tr.WithinTx(context.Background(), func(ctx context.Context) error { + called = true + return nil + }) + + assert.ErrorIs(t, err, beginErr) + assert.False(t, called) + }) + + t.Run("commit error is returned", func(t *testing.T) { + tr, mock := newTransactor(t) + + commitErr := errors.New("could not serialize access") + mock.ExpectBegin() + mock.ExpectCommit().WillReturnError(commitErr) + + err := tr.WithinTx(context.Background(), func(ctx context.Context) error { + return nil + }) + + assert.ErrorIs(t, err, commitErr) + }) +} + +func TestTransactor_WithinTxNested(t *testing.T) { + t.Run("reuses the outer tx and releases the savepoint", func(t *testing.T) { + tr, mock := newTransactor(t) + + mock.ExpectBegin() + mock.ExpectExec("SAVEPOINT sp_1").WillReturnResult(sqlmock.NewResult(0, 0)) + mock.ExpectExec("RELEASE SAVEPOINT sp_1").WillReturnResult(sqlmock.NewResult(0, 0)) + mock.ExpectCommit() + + var outerTx *sql.Tx + err := tr.WithinTx(context.Background(), func(ctx context.Context) error { + outerTx = ctx.Value(txKey{}).(*sql.Tx) + + return tr.WithinTx(ctx, func(ctx context.Context) error { + assert.Same(t, outerTx, ctx.Value(txKey{}).(*sql.Tx)) + return nil + }) + }) + + require.NoError(t, err) + }) + + t.Run("inner error rolls back to the savepoint", func(t *testing.T) { + tr, mock := newTransactor(t) + + mock.ExpectBegin() + mock.ExpectExec("SAVEPOINT sp_1").WillReturnResult(sqlmock.NewResult(0, 0)) + mock.ExpectExec("ROLLBACK TO SAVEPOINT sp_1").WillReturnResult(sqlmock.NewResult(0, 0)) + mock.ExpectRollback() + + innerErr := errors.New("nested failure") + err := tr.WithinTx(context.Background(), func(ctx context.Context) error { + return tr.WithinTx(ctx, func(ctx context.Context) error { + return innerErr + }) + }) + + assert.ErrorIs(t, err, innerErr) + }) + + t.Run("savepoint creation error skips the callback", func(t *testing.T) { + tr, mock := newTransactor(t) + + spErr := errors.New("savepoint failed") + mock.ExpectBegin() + mock.ExpectExec("SAVEPOINT sp_1").WillReturnError(spErr) + mock.ExpectRollback() + + called := false + err := tr.WithinTx(context.Background(), func(ctx context.Context) error { + return tr.WithinTx(ctx, func(ctx context.Context) error { + called = true + return nil + }) + }) + + assert.ErrorIs(t, err, spErr) + assert.False(t, called) + }) + + t.Run("release error is returned", func(t *testing.T) { + tr, mock := newTransactor(t) + + releaseErr := errors.New("no such savepoint") + mock.ExpectBegin() + mock.ExpectExec("SAVEPOINT sp_1").WillReturnResult(sqlmock.NewResult(0, 0)) + mock.ExpectExec("RELEASE SAVEPOINT sp_1").WillReturnError(releaseErr) + mock.ExpectRollback() + + err := tr.WithinTx(context.Background(), func(ctx context.Context) error { + return tr.WithinTx(ctx, func(ctx context.Context) error { + return nil + }) + }) + + assert.ErrorIs(t, err, releaseErr) + }) + + t.Run("each savepoint gets its own name", func(t *testing.T) { + tr, mock := newTransactor(t) + + mock.ExpectBegin() + mock.ExpectExec("SAVEPOINT sp_1").WillReturnResult(sqlmock.NewResult(0, 0)) + mock.ExpectExec("RELEASE SAVEPOINT sp_1").WillReturnResult(sqlmock.NewResult(0, 0)) + mock.ExpectExec("SAVEPOINT sp_2").WillReturnResult(sqlmock.NewResult(0, 0)) + mock.ExpectExec("RELEASE SAVEPOINT sp_2").WillReturnResult(sqlmock.NewResult(0, 0)) + mock.ExpectCommit() + + noop := func(ctx context.Context) error { return nil } + + err := tr.WithinTx(context.Background(), func(ctx context.Context) error { + if err := tr.WithinTx(ctx, noop); err != nil { + return err + } + return tr.WithinTx(ctx, noop) + }) + + require.NoError(t, err) + }) +} -- 2.54.0