Repository: go-gorm/gorm
Pull request #7086 (2024-06-25 17:48:42)

feat: Modernize Docker Compose File
<!--
Make sure these boxes checked before submitting your pull request.

For significant changes, please open an issue to make an agreement on an implementation design/plan first before starting it.
-->

- [x] Do only one thing
- [x] Non breaking API changes
- [x] Tested

### What did this pull request do?

This pull request did the following
- [x] Rename the file from docker-compose.yml to compose.yml .
- [x] Remove version top-level element on compose.yml .
- [x] ~~Changed command used from `docker-compose` to `docker compose`.~~
- [x] ~~Change related files.~~

#### Rename the file from docker-compose.yml to compose.yml .

The file name `compose.yml` is preferred.
https://docs.docker.com/compose/compose-application-model/#the-compose-file

#### Remove version top-level element on compose.yml .

Version top-level element is obsoleted.
https://docs.docker.com/compose/compose-file/04-version-and-name/#version-top-level-element-obsolete

When run test, system shows warning.

```sh
tests % ./tests_all.sh
WARN[0000] /Users/john/gorm/tests/docker-compose.yml: `version` is obsolete
...
```

#### Changed command used from `docker-compose` to `docker compose`.

The recommended command-line syntax is `docker compose`.
https://docs.docker.com/compose/migrate/#what-are-the-differences-between-compose-v1-and-compose-v2

#### Change related files.

`docker-compose start` => `docker compose start`
`docker-compose up` => `docker compose up`

### User Case Description

<!-- Your use case -->


jinzhu: It seems that docker-compose is still an alias of docker compose in the new version. Let's keep it for better compatibility.
isso-719: > It seems that docker-compose is still an alias of docker compose in the new version. Let's keep it for better compatibility.

@jinzhu 
Thank you for your reply.
For compatibility, I reverted from the `docker compose` command to the `docker-compose` command.

Please check Files changed.
Thank you.

Patch:
diff --git a/callbacks/query.go b/callbacks/query.go
index 9b2b17e..bbf238a 100644
--- a/callbacks/query.go
+++ b/callbacks/query.go
@@ -288,7 +288,7 @@ func AfterQuery(db *gorm.DB) {
 	// clear the joins after query because preload need it
 	if v, ok := db.Statement.Clauses["FROM"].Expression.(clause.From); ok {
 		fromClause := db.Statement.Clauses["FROM"]
-		fromClause.Expression = clause.From{Tables: v.Tables, Joins: v.Joins[:len(v.Joins)-len(db.Statement.Joins)]} // keep the original From Joins
+		fromClause.Expression = clause.From{Tables: v.Tables, Joins: utils.RTrimSlice(v.Joins, len(db.Statement.Joins))} // keep the original From Joins
 		db.Statement.Clauses["FROM"] = fromClause
 	}
 	if db.Error == nil && db.Statement.Schema != nil && !db.Statement.SkipHooks && db.Statement.Schema.AfterFind && db.RowsAffected > 0 {
diff --git a/finisher_api.go b/finisher_api.go
index f97571e..6802945 100644
--- a/finisher_api.go
+++ b/finisher_api.go
@@ -4,6 +4,7 @@ import (
 	"database/sql"
 	"errors"
 	"fmt"
+	"hash/maphash"
 	"reflect"
 	"strings"
 
@@ -623,14 +624,15 @@ func (db *DB) Transaction(fc func(tx *DB) error, opts ...*sql.TxOptions) (err er
 	if committer, ok := db.Statement.ConnPool.(TxCommitter); ok && committer != nil {
 		// nested transaction
 		if !db.DisableNestedTransaction {
-			err = db.SavePoint(fmt.Sprintf("sp%p", fc)).Error
+			spID := new(maphash.Hash).Sum64()
+			err = db.SavePoint(fmt.Sprintf("sp%d", spID)).Error
 			if err != nil {
 				return
 			}
 			defer func() {
 				// Make sure to rollback when panic, Block error or Commit error
 				if panicked || err != nil {
-					db.RollbackTo(fmt.Sprintf("sp%p", fc))
+					db.RollbackTo(fmt.Sprintf("sp%d", spID))
 				}
 			}()
 		}
diff --git a/prepare_stmt.go b/prepare_stmt.go
index 4d53388..094bb47 100644
--- a/prepare_stmt.go
+++ b/prepare_stmt.go
@@ -17,18 +17,16 @@ type Stmt struct {
 }
 
 type PreparedStmtDB struct {
-	Stmts       map[string]*Stmt
-	PreparedSQL []string
-	Mux         *sync.RWMutex
+	Stmts map[string]*Stmt
+	Mux   *sync.RWMutex
 	ConnPool
 }
 
 func NewPreparedStmtDB(connPool ConnPool) *PreparedStmtDB {
 	return &PreparedStmtDB{
-		ConnPool:    connPool,
-		Stmts:       make(map[string]*Stmt),
-		Mux:         &sync.RWMutex{},
-		PreparedSQL: make([]string, 0, 100),
+		ConnPool: connPool,
+		Stmts:    make(map[string]*Stmt),
+		Mux:      &sync.RWMutex{},
 	}
 }
 
@@ -48,12 +46,17 @@ func (db *PreparedStmtDB) Close() {
 	db.Mux.Lock()
 	defer db.Mux.Unlock()
 
-	for _, query := range db.PreparedSQL {
-		if stmt, ok := db.Stmts[query]; ok {
-			delete(db.Stmts, query)
-			go stmt.Close()
-		}
+	for _, stmt := range db.Stmts {
+		go func(s *Stmt) {
+			// make sure the stmt must finish preparation first
+			<-s.prepared
+			if s.Stmt != nil {
+				_ = s.Close()
+			}
+		}(stmt)
 	}
+	// setting db.Stmts to nil to avoid further using
+	db.Stmts = nil
 }
 
 func (sdb *PreparedStmtDB) Reset() {
@@ -61,9 +64,14 @@ func (sdb *PreparedStmtDB) Reset() {
 	defer sdb.Mux.Unlock()
 
 	for _, stmt := range sdb.Stmts {
-		go stmt.Close()
+		go func(s *Stmt) {
+			// make sure the stmt must finish preparation first
+			<-s.prepared
+			if s.Stmt != nil {
+				_ = s.Close()
+			}
+		}(stmt)
 	}
-	sdb.PreparedSQL = make([]string, 0, 100)
 	sdb.Stmts = make(map[string]*Stmt)
 }
 
@@ -93,7 +101,12 @@ func (db *PreparedStmtDB) prepare(ctx context.Context, conn ConnPool, isTransact
 
 		return *stmt, nil
 	}
-
+	// check db.Stmts first to avoid Segmentation Fault(setting value to nil map)
+	// which cause by calling Close and executing SQL concurrently
+	if db.Stmts == nil {
+		db.Mux.Unlock()
+		return Stmt{}, ErrInvalidDB
+	}
 	// cache preparing stmt first
 	cacheStmt := Stmt{Transaction: isTransaction, prepared: make(chan struct{})}
 	db.Stmts[query] = &cacheStmt
@@ -118,7 +131,6 @@ func (db *PreparedStmtDB) prepare(ctx context.Context, conn ConnPool, isTransact
 
 	db.Mux.Lock()
 	cacheStmt.Stmt = stmt
-	db.PreparedSQL = append(db.PreparedSQL, query)
 	db.Mux.Unlock()
 
 	return cacheStmt, nil
diff --git a/utils/utils.go b/utils/utils.go
index b8d30b3..fc615d7 100644
--- a/utils/utils.go
+++ b/utils/utils.go
@@ -166,3 +166,14 @@ func SplitNestedRelationName(name string) []string {
 func JoinNestedRelationNames(relationNames []string) string {
 	return strings.Join(relationNames, nestedRelationSplit)
 }
+
+// RTrimSlice Right trims the given slice by given length
+func RTrimSlice[T any](v []T, trimLen int) []T {
+	if trimLen >= len(v) { // trimLen greater than slice len means fully sliced
+		return v[:0]
+	}
+	if trimLen < 0 { // negative trimLen is ignored
+		return v[:]
+	}
+	return v[:len(v)-trimLen]
+}


Test patch:
diff --git a/tests/docker-compose.yml b/tests/compose.yml
similarity index 98%
rename from tests/docker-compose.yml
rename to tests/compose.yml
index 5a3a10f..666b1fb 100644
--- a/tests/docker-compose.yml
+++ b/tests/compose.yml
@@ -1,5 +1,3 @@
-version: '3'
-
 services:
   mysql:
     image: 'mysql/mysql-server:latest'
diff --git a/tests/go.mod b/tests/go.mod
index 350d179..8eeab51 100644
--- a/tests/go.mod
+++ b/tests/go.mod
@@ -29,8 +29,8 @@ require (
 	github.com/microsoft/go-mssqldb v1.7.2 // indirect
 	github.com/pmezard/go-difflib v1.0.0 // indirect
 	github.com/rogpeppe/go-internal v1.12.0 // indirect
-	golang.org/x/crypto v0.24.0 // indirect
-	golang.org/x/text v0.16.0 // indirect
+	golang.org/x/crypto v0.26.0 // indirect
+	golang.org/x/text v0.17.0 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 )
 
diff --git a/tests/prepared_stmt_test.go b/tests/prepared_stmt_test.go
index b86bc3d..20a4f73 100644
--- a/tests/prepared_stmt_test.go
+++ b/tests/prepared_stmt_test.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"sync"
+	"sync/atomic"
 	"testing"
 	"time"
 
@@ -167,3 +168,149 @@ func TestPreparedStmtReset(t *testing.T) {
 		t.Fatalf("prepared stmt should be empty")
 	}
 }
+
+func isUsingClosedConnError(err error) bool {
+	// https://github.com/golang/go/blob/e705a2d16e4ece77e08e80c168382cdb02890f5b/src/database/sql/sql.go#L2717
+	return err.Error() == "sql: statement is closed"
+}
+
+// TestPreparedStmtConcurrentReset test calling reset and executing SQL concurrently
+// this test making sure that the gorm would not get a Segmentation Fault, and the only error cause by this is using a closed Stmt
+func TestPreparedStmtConcurrentReset(t *testing.T) {
+	name := "prepared_stmt_concurrent_reset"
+	user := *GetUser(name, Config{})
+	createTx := DB.Session(&gorm.Session{}).Create(&user)
+	if createTx.Error != nil {
+		t.Fatalf("failed to prepare record due to %s, test cannot be continue", createTx.Error)
+	}
+
+	// create a new connection to keep away from other tests
+	tx, err := OpenTestConnection(&gorm.Config{PrepareStmt: true})
+	if err != nil {
+		t.Fatalf("failed to open test connection due to %s", err)
+	}
+	pdb, ok := tx.ConnPool.(*gorm.PreparedStmtDB)
+	if !ok {
+		t.Fatalf("should assign PreparedStatement Manager back to database when using PrepareStmt mode")
+	}
+
+	loopCount := 100
+	var wg sync.WaitGroup
+	var unexpectedError bool
+	writerFinish := make(chan struct{})
+
+	wg.Add(1)
+	go func(id uint) {
+		defer wg.Done()
+		defer close(writerFinish)
+
+		for j := 0; j < loopCount; j++ {
+			var tmp User
+			err := tx.Session(&gorm.Session{}).First(&tmp, id).Error
+			if err == nil || isUsingClosedConnError(err) {
+				continue
+			}
+			t.Errorf("failed to read user of id %d due to %s, there should not be error", id, err)
+			unexpectedError = true
+			break
+		}
+	}(user.ID)
+
+	wg.Add(1)
+	go func() {
+		defer wg.Done()
+		<-writerFinish
+		pdb.Reset()
+	}()
+
+	wg.Wait()
+
+	if unexpectedError {
+		t.Fatalf("should is a unexpected error")
+	}
+}
+
+// TestPreparedStmtConcurrentClose test calling close and executing SQL concurrently
+// for example: one goroutine found error and just close the database, and others are executing SQL
+// this test making sure that the gorm would not get a Segmentation Fault,
+// and the only error cause by this is using a closed Stmt or gorm.ErrInvalidDB
+// and all of the goroutine must got gorm.ErrInvalidDB after database close
+func TestPreparedStmtConcurrentClose(t *testing.T) {
+	name := "prepared_stmt_concurrent_close"
+	user := *GetUser(name, Config{})
+	createTx := DB.Session(&gorm.Session{}).Create(&user)
+	if createTx.Error != nil {
+		t.Fatalf("failed to prepare record due to %s, test cannot be continue", createTx.Error)
+	}
+
+	// create a new connection to keep away from other tests
+	tx, err := OpenTestConnection(&gorm.Config{PrepareStmt: true})
+	if err != nil {
+		t.Fatalf("failed to open test connection due to %s", err)
+	}
+	pdb, ok := tx.ConnPool.(*gorm.PreparedStmtDB)
+	if !ok {
+		t.Fatalf("should assign PreparedStatement Manager back to database when using PrepareStmt mode")
+	}
+
+	loopCount := 100
+	var wg sync.WaitGroup
+	var lastErr error
+	closeValid := make(chan struct{}, loopCount)
+	closeStartIdx := loopCount / 2 // close the database at the middle of the execution
+	var lastRunIndex int
+	var closeFinishedAt int64
+
+	wg.Add(1)
+	go func(id uint) {
+		defer wg.Done()
+		defer close(closeValid)
+		for lastRunIndex = 1; lastRunIndex <= loopCount; lastRunIndex++ {
+			if lastRunIndex == closeStartIdx {
+				closeValid <- struct{}{}
+			}
+			var tmp User
+			now := time.Now().UnixNano()
+			err := tx.Session(&gorm.Session{}).First(&tmp, id).Error
+			if err == nil {
+				closeFinishedAt := atomic.LoadInt64(&closeFinishedAt)
+				if (closeFinishedAt != 0) && (now > closeFinishedAt) {
+					lastErr = errors.New("must got error after database closed")
+					break
+				}
+				continue
+			}
+			lastErr = err
+			break
+		}
+	}(user.ID)
+
+	wg.Add(1)
+	go func() {
+		defer wg.Done()
+		for range closeValid {
+			for i := 0; i < loopCount; i++ {
+				pdb.Close() // the Close method must can be call multiple times
+				atomic.CompareAndSwapInt64(&closeFinishedAt, 0, time.Now().UnixNano())
+			}
+		}
+	}()
+
+	wg.Wait()
+	var tmp User
+	err = tx.Session(&gorm.Session{}).First(&tmp, user.ID).Error
+	if err != gorm.ErrInvalidDB {
+		t.Fatalf("must got a gorm.ErrInvalidDB while execution after db close, got %+v instead", err)
+	}
+
+	// must be error
+	if lastErr != gorm.ErrInvalidDB && !isUsingClosedConnError(lastErr) {
+		t.Fatalf("exp error gorm.ErrInvalidDB, got %+v instead", lastErr)
+	}
+	if lastRunIndex >= loopCount || lastRunIndex < closeStartIdx {
+		t.Fatalf("exp loop times between (closeStartIdx %d <=) and (< loopCount %d), got %d instead", closeStartIdx, loopCount, lastRunIndex)
+	}
+	if pdb.Stmts != nil {
+		t.Fatalf("stmts must be nil")
+	}
+}
diff --git a/tests/transaction_test.go b/tests/transaction_test.go
index d2cbc9a..9f0f067 100644
--- a/tests/transaction_test.go
+++ b/tests/transaction_test.go
@@ -297,6 +297,74 @@ func TestNestedTransactionWithBlock(t *testing.T) {
 	}
 }
 
+func TestDeeplyNestedTransactionWithBlockAndWrappedCallback(t *testing.T) {
+	transaction := func(ctx context.Context, db *gorm.DB, callback func(ctx context.Context, db *gorm.DB) error) error {
+		return db.WithContext(ctx).Transaction(func(tx *gorm.DB) error {
+			return callback(ctx, tx)
+		})
+	}
+	var (
+		user  = *GetUser("transaction-nested", Config{})
+		user1 = *GetUser("transaction-nested-1", Config{})
+		user2 = *GetUser("transaction-nested-2", Config{})
+	)
+
+	if err := transaction(context.Background(), DB, func(ctx context.Context, tx *gorm.DB) error {
+		tx.Create(&user)
+
+		if err := tx.First(&User{}, "name = ?", user.Name).Error; err != nil {
+			t.Fatalf("Should find saved record")
+		}
+
+		if err := transaction(ctx, tx, func(ctx context.Context, tx1 *gorm.DB) error {
+			tx1.Create(&user1)
+
+			if err := tx1.First(&User{}, "name = ?", user1.Name).Error; err != nil {
+				t.Fatalf("Should find saved record")
+			}
+
+			if err := transaction(ctx, tx1, func(ctx context.Context, tx2 *gorm.DB) error {
+				tx2.Create(&user2)
+
+				if err := tx2.First(&User{}, "name = ?", user2.Name).Error; err != nil {
+					t.Fatalf("Should find saved record")
+				}
+
+				return errors.New("inner rollback")
+			}); err == nil {
+				t.Fatalf("nested transaction has no error")
+			}
+
+			return errors.New("rollback")
+		}); err == nil {
+			t.Fatalf("nested transaction should returns error")
+		}
+
+		if err := tx.First(&User{}, "name = ?", user1.Name).Error; err == nil {
+			t.Fatalf("Should not find rollbacked record")
+		}
+
+		if err := tx.First(&User{}, "name = ?", user2.Name).Error; err != nil {
+			t.Fatalf("Should find saved record")
+		}
+		return nil
+	}); err != nil {
+		t.Fatalf("no error should return, but got %v", err)
+	}
+
+	if err := DB.First(&User{}, "name = ?", user.Name).Error; err != nil {
+		t.Fatalf("Should find saved record")
+	}
+
+	if err := DB.First(&User{}, "name = ?", user1.Name).Error; err == nil {
+		t.Fatalf("Should not find rollbacked parent record")
+	}
+
+	if err := DB.First(&User{}, "name = ?", user2.Name).Error; err != nil {
+		t.Fatalf("Should not find rollbacked nested record")
+	}
+}
+
 func TestDisabledNestedTransaction(t *testing.T) {
 	var (
 		user  = *GetUser("transaction-nested", Config{})
diff --git a/utils/utils_test.go b/utils/utils_test.go
index 8ff42af..089cc4c 100644
--- a/utils/utils_test.go
+++ b/utils/utils_test.go
@@ -138,3 +138,64 @@ func TestToString(t *testing.T) {
 		})
 	}
 }
+
+func TestRTrimSlice(t *testing.T) {
+	tests := []struct {
+		name     string
+		input    []int
+		trimLen  int
+		expected []int
+	}{
+		{
+			name:     "Trim two elements from end",
+			input:    []int{1, 2, 3, 4, 5},
+			trimLen:  2,
+			expected: []int{1, 2, 3},
+		},
+		{
+			name:     "Trim entire slice",
+			input:    []int{1, 2, 3},
+			trimLen:  3,
+			expected: []int{},
+		},
+		{
+			name:     "Trim length greater than slice length",
+			input:    []int{1, 2, 3},
+			trimLen:  5,
+			expected: []int{},
+		},
+		{
+			name:     "Zero trim length",
+			input:    []int{1, 2, 3},
+			trimLen:  0,
+			expected: []int{1, 2, 3},
+		},
+		{
+			name:     "Trim one element from end",
+			input:    []int{1, 2, 3},
+			trimLen:  1,
+			expected: []int{1, 2},
+		},
+		{
+			name:     "Empty slice",
+			input:    []int{},
+			trimLen:  2,
+			expected: []int{},
+		},
+		{
+			name:     "Negative trim length (should be treated as zero)",
+			input:    []int{1, 2, 3},
+			trimLen:  -1,
+			expected: []int{1, 2, 3},
+		},
+	}
+
+	for _, testcase := range tests {
+		t.Run(testcase.name, func(t *testing.T) {
+			result := RTrimSlice(testcase.input, testcase.trimLen)
+			if !AssertEqual(result, testcase.expected) {
+				t.Errorf("RTrimSlice(%v, %d) = %v; want %v", testcase.input, testcase.trimLen, result, testcase.expected)
+			}
+		})
+	}
+}