review

avito-tech · Jul 28, 2024 · 5baa881 · 5baa881
1 parent d16c62d
commit 5baa881
Show file tree

Hide file tree

Showing 5 changed files with 40 additions and 38 deletions.
diff --git a/.golangci.yml b/.golangci.yml
@@ -29,7 +29,7 @@ linters:
     - gocritic
     - gocyclo
     - godot
-    - goerr113
+    - err113
     - gofmt
     - gofumpt
     - goimports
@@ -129,7 +129,7 @@ issues:
         - goconst
         - gomnd
         - containedctx
-        - goerr113
+        - err113
         - errcheck
         - nolintlint
         - forcetypeassert

diff --git a/drivers/pgxv4/transaction.go b/drivers/pgxv4/transaction.go
@@ -17,6 +17,14 @@ type Transaction struct {
 	isClosed *drivers.IsClosed
 }
 
+func newDefaultTransaction(tx pgx.Tx) *Transaction {
+	return &Transaction{
+		mu:       sync.Mutex{},
+		tx:       tx,
+		isClosed: drivers.NewIsClosed(),
+	}
+}
+
 // NewTransaction creates trm.Transaction for pgx.Tx.
 func NewTransaction(
 	ctx context.Context,
@@ -28,11 +36,7 @@ func NewTransaction(
 		return ctx, nil, err
 	}
 
-	tr := &Transaction{
-		mu:       sync.Mutex{},
-		tx:       tx,
-		isClosed: drivers.NewIsClosed(),
-	}
+	tr := newDefaultTransaction(tx)
 
 	go tr.awaitDone(ctx)
 
@@ -63,29 +67,25 @@ func (t *Transaction) Begin(ctx context.Context, _ trm.Settings) (context.Contex
 		return ctx, nil, err
 	}
 
-	tr := &Transaction{
-		mu:       sync.Mutex{},
-		tx:       tx,
-		isClosed: drivers.NewIsClosed(),
-	}
+	tr := newDefaultTransaction(tx)
 
 	return ctx, tr, nil
 }
 
 // Commit the trm.Transaction.
 func (t *Transaction) Commit(ctx context.Context) error {
 	t.mu.Lock()
-	defer t.isClosed.Close()
 	defer t.mu.Unlock()
+	defer t.isClosed.Close()
 
 	return t.tx.Commit(ctx)
 }
 
 // Rollback the trm.Transaction.
 func (t *Transaction) Rollback(ctx context.Context) error {
 	t.mu.Lock()
-	defer t.isClosed.Close()
 	defer t.mu.Unlock()
+	defer t.isClosed.Close()
 
 	return t.tx.Rollback(ctx)
 }

diff --git a/drivers/pgxv4/transaction_integration_test.go b/drivers/pgxv4/transaction_integration_test.go
@@ -20,7 +20,7 @@ import (
 
 func db(ctx context.Context) (*pgxpool.Pool, error) {
 	uri := fmt.Sprintf("postgres://%s:%s@%s:%d/%s",
-		"postgres", "postgres", "localhost", 5432, "service-db",
+		"user", "pass", "localhost", 5432, "db",
 	)
 
 	pool, err := pgxpool.Connect(ctx, uri)
@@ -55,13 +55,14 @@ func TestTransaction_WithRealDB(t *testing.T) {
 	require.ErrorIs(t, tr.Rollback(ctx), pgx.ErrTxClosed)
 }
 
+// transaction should release all resources if context is cancelled
+// otherwise pool.Close() is blocked forever
 func TestTransaction_WithRealDB_RollbackOnContextCancel(t *testing.T) {
-	// transaction should release all resources if context is cancelled
-	// otherwise pool.Close() is blocked forever
-
 	t.Parallel()
 
-	pool, err := db(context.Background())
+	ctx := context.Background()
+
+	pool, err := db(ctx)
 	require.NoError(t, err)
 
 	defer func() {
@@ -70,7 +71,7 @@ func TestTransaction_WithRealDB_RollbackOnContextCancel(t *testing.T) {
 
 	f := pgxv4.NewDefaultFactory(pool)
 
-	ctx, cancel := context.WithCancel(context.Background())
+	ctx, cancel := context.WithCancel(ctx)
 
 	_, tr, err := f(ctx, settings.Must())
 	require.NoError(t, err)

diff --git a/drivers/pgxv5/transaction.go b/drivers/pgxv5/transaction.go
@@ -17,6 +17,14 @@ type Transaction struct {
 	isClosed *drivers.IsClosed
 }
 
+func newDefaultTransaction(tx pgx.Tx) *Transaction {
+	return &Transaction{
+		mu:       sync.Mutex{},
+		tx:       tx,
+		isClosed: drivers.NewIsClosed(),
+	}
+}
+
 // NewTransaction creates trm.Transaction for pgx.Tx.
 func NewTransaction(
 	ctx context.Context,
@@ -28,11 +36,7 @@ func NewTransaction(
 		return ctx, nil, err
 	}
 
-	tr := &Transaction{
-		mu:       sync.Mutex{},
-		tx:       tx,
-		isClosed: drivers.NewIsClosed(),
-	}
+	tr := newDefaultTransaction(tx)
 
 	go tr.awaitDone(ctx)
 
@@ -63,29 +67,25 @@ func (t *Transaction) Begin(ctx context.Context, _ trm.Settings) (context.Contex
 		return ctx, nil, err
 	}
 
-	tr := &Transaction{
-		mu:       sync.Mutex{},
-		tx:       tx,
-		isClosed: drivers.NewIsClosed(),
-	}
+	tr := newDefaultTransaction(tx)
 
 	return ctx, tr, nil
 }
 
 // Commit the trm.Transaction.
 func (t *Transaction) Commit(ctx context.Context) error {
 	t.mu.Lock()
-	defer t.isClosed.Close()
 	defer t.mu.Unlock()
+	defer t.isClosed.Close()
 
 	return t.tx.Commit(ctx)
 }
 
 // Rollback the trm.Transaction.
 func (t *Transaction) Rollback(ctx context.Context) error {
 	t.mu.Lock()
-	defer t.isClosed.Close()
 	defer t.mu.Unlock()
+	defer t.isClosed.Close()
 
 	return t.tx.Rollback(ctx)
 }

diff --git a/drivers/pgxv5/transaction_integration_test.go b/drivers/pgxv5/transaction_integration_test.go
@@ -20,7 +20,7 @@ import (
 
 func db(ctx context.Context) (*pgxpool.Pool, error) {
 	uri := fmt.Sprintf("postgres://%s:%s@%s:%d/%s",
-		"postgres", "postgres", "localhost", 5432, "service-db",
+		"user", "pass", "localhost", 5432, "db",
 	)
 
 	pool, err := pgxpool.New(ctx, uri)
@@ -55,13 +55,14 @@ func TestTransaction_WithRealDB(t *testing.T) {
 	require.ErrorIs(t, tr.Rollback(ctx), pgx.ErrTxClosed)
 }
 
+// transaction should release all resources if context is cancelled
+// otherwise pool.Close() is blocked forever
 func TestTransaction_WithRealDB_RollbackOnContextCancel(t *testing.T) {
-	// transaction should release all resources if context is cancelled
-	// otherwise pool.Close() is blocked forever
-
 	t.Parallel()
 
-	pool, err := db(context.Background())
+	ctx := context.Background()
+
+	pool, err := db(ctx)
 	require.NoError(t, err)
 
 	defer func() {
@@ -70,7 +71,7 @@ func TestTransaction_WithRealDB_RollbackOnContextCancel(t *testing.T) {
 
 	f := pgxv5.NewDefaultFactory(pool)
 
-	ctx, cancel := context.WithCancel(context.Background())
+	ctx, cancel := context.WithCancel(ctx)
 
 	_, tr, err := f(ctx, settings.Must())
 	require.NoError(t, err)