Merge pull request 'Fixed WithinTx and improved test coverage' (#46) from dev into main

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,

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) {
+func (m *mockListRepository) CreateList(ctx context.Context, name string) (*List, error) {
-	args := m.Called(ctx, name, userIDs)
+	args := m.Called(ctx, name)
-	var list *domain.List
+	list, _ := args.Get(0).(*List)
-	if l := args.Get(0); l != nil {
-		list = l.(*domain.List)
-	}
 	return list, args.Error(1)
 }
 
-func TestListService_Create_Success(t *testing.T) {
+func (m *mockListRepository) DeleteList(ctx context.Context, listID string) error {
-	expectedList := &domain.List{
+	args := m.Called(ctx, listID)
-		ID:         "1",
+	return args.Error(0)
-		Name:       "My List",
-		CreatedAt:  time.Now(),
-		ModifiedAt: time.Now(),
 }
 
-	repo := new(mockListRepo)
+func (m *mockListRepository) GetLists(ctx context.Context, userID string) ([]List, error) {
-	repo.On("CreateList", mock.Anything, "My List", []string{"user1", "user2"}).Return(expectedList, nil)
+	args := m.Called(ctx, userID)
-
+	lists, _ := args.Get(0).([]List)
-	//service := domain.NewListService(repo)
+	return lists, args.Error(1)
-	//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) {
+func (m *mockListRepository) AddUserToList(ctx context.Context, listID string, userID string) error {
-	repo := new(mockListRepo)
+	args := m.Called(ctx, listID, userID)
-	//service := domain.NewListService(repo)
+	return args.Error(0)
-
-	//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) {
+func (m *mockListRepository) RemoveUserFromList(ctx context.Context, listID string, userID string) error {
-	repo := new(mockListRepo)
+	args := m.Called(ctx, listID, userID)
-	//service := domain.NewListService(repo)
+	return args.Error(0)
-
-	//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) {
+func (m *mockListRepository) IsUserInList(ctx context.Context, listID string, userID string) (bool, error) {
-	expectedErr := errors.New("database error")
+	args := m.Called(ctx, listID, userID)
-	repo := new(mockListRepo)
+	return args.Bool(0), args.Error(1)
-	repo.On("CreateList", mock.Anything, "My List", []string{"user1"}).Return(nil, expectedErr)
+}
 
-	//service := domain.NewListService(repo)
+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)
+}
 
-	//list, err := service.CreateList(context.Background(), "My List", []string{"user1"})
+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)
+}
 
-	//require.Error(t, err)
+func (m *mockListRepository) DeleteListItem(ctx context.Context, listItemID string) error {
-	//assert.ErrorIs(t, err, expectedErr)
+	args := m.Called(ctx, listItemID)
-	//assert.Nil(t, list)
+	return args.Error(0)
-	//repo.AssertExpectations(t)
+}
+
+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,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			svc, _, _ := newListService(t)
+
+			assert.ErrorIs(t, tt.call(svc), tt.wantErr)
+		})
+	}
 }

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()

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)
+	})
+}

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()
+	t.Run("success", func(t *testing.T) {
-	assert.NoError(t, err)
+		repo, mock := newUserRepo(t)
-	defer db.Close()
 
-	//repo := NewUserRepo(db)
+		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),
+			)
 
-	//ctx := context.Background()
+		user, err := repo.CreateUser(context.Background(), "robin@dittmar.dev", "Robin", "$2a$10$hash")
-	email := "test@example.com"
-	name := "Test User"
-	passwordHash := "hashedpassword123"
 
-	now := time.Now()
+		require.NoError(t, err)
-	expectedUser := &domain.User{
+		require.NotNil(t, user)
-		ID:        "1",
+		assert.Equal(t, &domain.User{
-		Email:     email,
+			ID:        "2f1c...",
-		Name:      name,
+			Email:     "robin@dittmar.dev",
-		CreatedAt: now,
+			Name:      "Robin",
+			CreatedAt: createdAt,
+		}, user)
+	})
+
+	t.Run("db error is wrapped", func(t *testing.T) {
+		repo, mock := newUserRepo(t)
+
+		dbErr := errors.New("duplicate key value violates unique constraint")
+		mock.ExpectQuery(regexp.QuoteMeta(insertUserQuery)).
+			WithArgs("robin@dittmar.dev", "Robin", "$2a$10$hash").
+			WillReturnError(dbErr)
+
+		user, err := repo.CreateUser(context.Background(), "robin@dittmar.dev", "Robin", "$2a$10$hash")
+
+		assert.Nil(t, user)
+		assert.ErrorIs(t, err, dbErr)
+		assert.ErrorContains(t, err, "failed to insert user")
+	})
 }
 
+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)).
-		mock.ExpectQuery(`^INSERT INTO users \(email, name, password_hash\) VALUES \(\$1, \$2, \$3\) RETURNING id, created_at$`).
+		WithArgs("user-1").
-			WithArgs(email, name, passwordHash).
+		WillReturnResult(sqlmock.NewResult(0, 1))
-			WillReturnRows(sqlmock.NewRows([]string{"id", "created_at"}).AddRow(expectedUser.ID, expectedUser.CreatedAt))
-
 	mock.ExpectCommit()
 
-		//user, err := repo.CreateUser(ctx, email, name, passwordHash)
+	tx, err := repo.db.BeginTx(context.Background(), nil)
-		//assert.NoError(t, err)
+	require.NoError(t, err)
-		//assert.Equal(t, expectedUser, user)
-		//assert.NoError(t, mock.ExpectationsWereMet())
-	})
 
-	t.Run("begin_tx_error", func(t *testing.T) {
+	ctx := context.WithValue(context.Background(), txKey{}, tx)
-		mock.ExpectBegin().WillReturnError(fmt.Errorf("tx error"))
+	require.NoError(t, repo.DeleteUser(ctx, "user-1"))
-
+	require.NoError(t, tx.Commit())
-		//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())
-	})
-
-	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(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())
-	})
 }
 
-//func TestUserRepo_CreateUser2(t *testing.T) {
+func TestUserRepo_GetUserByEmail(t *testing.T) {
-//	email := "test@example.com"
+	t.Run("success", func(t *testing.T) {
-//	name := "Test User"
+		repo, mock := newUserRepo(t)
-//	passwordHash := "hashedpassword123"
+
-//	now := time.Now()
+		mock.ExpectQuery(regexp.QuoteMeta(selectUserQuery)).
-//	expectedID := "42"
+			WithArgs("robin@dittmar.dev").
-//
+			WillReturnRows(
-//	insertQuery := regexp.QuoteMeta(
+				sqlmock.NewRows([]string{"id", "email", "name"}).
-//		"INSERT INTO users (email, name, password_hash) VALUES ($1, $2, $3) RETURNING id, created_at",
+					AddRow("user-1", "robin@dittmar.dev", "Robin"),
-//	)
+			)
-//
+
-//	testCases := []struct {
+		user, err := repo.GetUserByEmail(context.Background(), "robin@dittmar.dev")
-//		name          string
+
-//		setupMock     func(mock sqlmock.Sqlmock)
+		require.NoError(t, err)
-//		expectedError string
+		require.NotNil(t, user)
-//	}{
+		assert.Equal(t, "user-1", user.ID)
-//		{
+		assert.Equal(t, "robin@dittmar.dev", user.Email)
-//			name: "Success: User created perfectly",
+		assert.Equal(t, "Robin", user.Name)
-//			setupMock: func(mock sqlmock.Sqlmock) {
+		assert.Zero(t, user.CreatedAt) // not selected by this query
-//				mock.ExpectBegin()
+	})
-//
+
-//				rows := sqlmock.NewRows([]string{"id", "created_at"}).
+	t.Run("not found stays matchable via errors.Is", func(t *testing.T) {
-//					AddRow(expectedID, now)
+		repo, mock := newUserRepo(t)
-//
+
-//				mock.ExpectQuery(insertQuery).
+		mock.ExpectQuery(regexp.QuoteMeta(selectUserQuery)).
-//					WithArgs(email, name, passwordHash).
+			WithArgs("nobody@dittmar.dev").
-//					WillReturnRows(rows)
+			WillReturnError(sql.ErrNoRows)
-//
+
-//				mock.ExpectCommit()
+		user, err := repo.GetUserByEmail(context.Background(), "nobody@dittmar.dev")
-//			},
+
-//			expectedError: "",
+		assert.Nil(t, user)
-//		},
+		assert.ErrorIs(t, err, sql.ErrNoRows)
-//		{
+		assert.ErrorContains(t, err, "failed to get user")
-//			name: "Failure: Database connection fails on BeginTx",
+	})
-//			setupMock: func(mock sqlmock.Sqlmock) {
+
-//				mock.ExpectBegin().WillReturnError(errors.New("db connection failed"))
+	t.Run("scan error on type mismatch", func(t *testing.T) {
-//			},
+		repo, mock := newUserRepo(t)
-//			expectedError: "begin transaction: db connection failed",
+
-//		},
+		mock.ExpectQuery(regexp.QuoteMeta(selectUserQuery)).
-//		{
+			WithArgs("robin@dittmar.dev").
-//			name: "Failure: Query fails (e.g., duplicate email)",
+			WillReturnRows(
-//			setupMock: func(mock sqlmock.Sqlmock) {
+				sqlmock.NewRows([]string{"id", "email", "name"}).
-//				mock.ExpectBegin()
+					AddRow(nil, "robin@dittmar.dev", "Robin"),
-//
+			)
-//				mock.ExpectQuery(insertQuery).
+
-//					WithArgs(email, name, passwordHash).
+		user, err := repo.GetUserByEmail(context.Background(), "robin@dittmar.dev")
-//					WillReturnError(errors.New("unique constraint violation"))
+
-//
+		assert.Nil(t, user)
-//				mock.ExpectRollback()
+		assert.Error(t, err)
-//			},
+	})
-//			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())
-//		})
-//	}
-//}