Repository: openimsdk/open-im-server
Pull request #1285 (2023-10-26 05:38:39)

fix: create database name
<br>

<!--
#### 🫰Thanks for sending a pull request!  Here are some tips for you:

1. If this is your first time, please read our contributor guidelines: 
📇 https://github.com/OpenIMSDK/Open-IM-Server/blob/main/CONTRIBUTING.md
2. Ensure you have added or ran the appropriate tests for your PR.
-->
#### 🔍 What type of PR is this?
<!--
We need to tag this PR, which you should learn about in the contributor guide.

Add one of the following kinds:
/kind bug
/kind cleanup
/kind documentation
/kind feature

Optionally add one or more of the following kinds if applicable:
/kind api-change
/kind deprecation
/kind failing-test
/kind flake
/kind regression
-->
/kind bug

#### 👀 What this PR does / why we need it:
<!-- What this PR does? -->
<!-- Make sure your pr passes the CI checks and do check the following fields as needed -->
- [ ] My pull request adheres to the code style of this project
- [ ] My code requires changes to the documentation
- [ ] I have updated the documentation as required
- [ ] All the tests have passed

<!--Why do we need this PR?-->
It will failed when you init app with a custom database name which contains '-' (eg: "im-db").

```
Error 1064 (42000): You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '-db default charset utf8mb4 COLLATE utf8mb4_unicode_ci' at line 1
[0.236ms] [rows:0] CREATE DATABASE IF NOT EXISTS im-db default charset utf8mb4 COLLATE utf8mb4_unicode_ci;
```

Screenshot:
![image](https://github.com/openimsdk/open-im-server/assets/56432636/0ba24c6b-0553-4812-9ce6-e69976af018c)

#### 🅰 Which issue(s) this PR fixes:
<!--
*Automatically closes linked issue when PR is merged https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
If there are multiple PRS, use `Resolves #10, resolves #123, resolves octo-org/octo-repo#100`
If there is a relevant PR, use `octo-org/octo-repo#123`
-->

Fixes #

#### 📝 Special notes for your reviewer:

<!-- What else would you tell someone who reviews your code -->
* Added a new test.
* Added a new type `option`.
* Note `defer sqlDB.Close()` has been moved.

#### 🎯 Describe how to verify it

<!-- Make sure to execute it `make all` locally, and it passed the test.-->
`go test -run ^TestMaybeCreateTable$ github.com/openimsdk/open-im-server/v3/pkg/common/db/relation`

#### 📑 Additional documentation e.g., RFC, notion, Google docs, usage docs, etc.:


<!--
This section can be blank if this pull request does not require a release note.

When adding links which point to resources within git repositories, like
KEPs or supporting documentation, please reference a specific commit and avoid
linking directly to the master branch. This ensures that links reference a
specific point in time, rather than a document that may change over time.

In the sharers Guide, we recommend the following documents:
1. Using GitHub RFCs template: https://github.com/OpenIMSDK/community/blob/main/0000-template.md
2. Use Google Docs OR Notion and share it with the community.
-->


kubbot: **CLA Assistant Lite bot:** 💕 Thank you for your contribution and please kindly read and sign our [🎯https://github.com/openim-sigs/cla/blob/main/README.md](https://github.com/openim-sigs/cla/blob/main/README.md). <br> If you wish to sign the CRA, **Please copy and comment on the following sentence:**
   - - -
   I have read the CLA Document and I hereby sign the CLA
   - - -
   <sub>You can retrigger this bot by commenting **recheck** in this Pull Request</sub>
sweep-ai[bot]: ## Apply [Sweep Rules](https://docs.sweep.dev/usage/config#rules) to your PR?
- [ ] **Apply:**  Leftover TODOs in the code should be handled.
- [ ] **Apply:**  All new business logic should have corresponding unit tests in the tests/ directory.
- [ ] **Apply:**  Any clearly inefficient or repeated code should be optimized or refactored.
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/OpenIMSDK/Open-IM-Server/pull/1285?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=OpenIMSDK) Report
> Merging [#1285](https://app.codecov.io/gh/OpenIMSDK/Open-IM-Server/pull/1285?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=OpenIMSDK) (ce4f5da) into [main](https://app.codecov.io/gh/OpenIMSDK/Open-IM-Server/commit/cb0394392b84e617e851d55ab55221e887e09bbc?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=OpenIMSDK) (cb03943) will **increase** coverage by `2.44%`.
> Report is 1 commits behind head on main.
> The diff coverage is `38.00%`.

```diff
@@           Coverage Diff           @@
##           main   #1285      +/-   ##
=======================================
+ Coverage      0   2.44%   +2.44%     
=======================================
  Files         0      13      +13     
  Lines         0     778     +778     
=======================================
+ Hits          0      19      +19     
- Misses        0     756     +756     
- Partials      0       3       +3     
```


| [Files](https://app.codecov.io/gh/OpenIMSDK/Open-IM-Server/pull/1285?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=OpenIMSDK) | Coverage Δ | |
|---|---|---|
| [pkg/common/db/relation/mysql\_init.go](https://app.codecov.io/gh/OpenIMSDK/Open-IM-Server/pull/1285?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=OpenIMSDK#diff-cGtnL2NvbW1vbi9kYi9yZWxhdGlvbi9teXNxbF9pbml0Lmdv) | `20.87% <38.00%> (ø)` | |

... and [12 files with indirect coverage changes](https://app.codecov.io/gh/OpenIMSDK/Open-IM-Server/pull/1285/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=OpenIMSDK)


kvii: I have read the CLA Document and I hereby sign the CLA
cubxxw: Thank you for your contribution to the OpenIM community. This is your first PR and we truly appreciate your help.

Patch:
diff --git a/pkg/common/db/relation/mysql_init.go b/pkg/common/db/relation/mysql_init.go
index 0e5ea5e43..16b8c99fa 100644
--- a/pkg/common/db/relation/mysql_init.go
+++ b/pkg/common/db/relation/mysql_init.go
@@ -18,15 +18,12 @@ import (
 	"fmt"
 	"time"
 
-	mysqldriver "github.com/go-sql-driver/mysql"
-	"gorm.io/driver/mysql"
-
 	"github.com/OpenIMSDK/tools/errs"
 	"github.com/OpenIMSDK/tools/log"
 	"github.com/OpenIMSDK/tools/mw/specialerror"
-
+	mysqldriver "github.com/go-sql-driver/mysql"
 	"github.com/openimsdk/open-im-server/v3/pkg/common/config"
-
+	"gorm.io/driver/mysql"
 	"gorm.io/gorm"
 	"gorm.io/gorm/logger"
 )
@@ -35,56 +32,80 @@ const (
 	maxRetry = 100 // number of retries
 )
 
-// newMysqlGormDB Initialize the database connection.
-func newMysqlGormDB() (*gorm.DB, error) {
-	dsn := fmt.Sprintf("%s:%s@tcp(%s)/%s?charset=utf8mb4&parseTime=true&loc=Local",
-		config.Config.Mysql.Username, config.Config.Mysql.Password, config.Config.Mysql.Address[0], "mysql")
+type option struct {
+	Username      string
+	Password      string
+	Address       []string
+	Database      string
+	LogLevel      int
+	SlowThreshold int
+	MaxLifeTime   int
+	MaxOpenConn   int
+	MaxIdleConn   int
+	Connect       func(dsn string, maxRetry int) (*gorm.DB, error)
+}
 
-	db, err := connectToDatabase(dsn, maxRetry)
-	if err != nil {
-		panic(err.Error() + " Open failed " + dsn)
-	}
-	sqlDB, err := db.DB()
+// newMysqlGormDB Initialize the database connection.
+func newMysqlGormDB(o *option) (*gorm.DB, error) {
+	err := maybeCreateTable(o)
 	if err != nil {
 		return nil, err
 	}
-	defer sqlDB.Close()
-	sql := fmt.Sprintf(
-		"CREATE DATABASE IF NOT EXISTS %s default charset utf8mb4 COLLATE utf8mb4_unicode_ci;",
-		config.Config.Mysql.Database,
-	)
-	err = db.Exec(sql).Error
-	if err != nil {
-		return nil, fmt.Errorf("init db %w", err)
-	}
-	dsn = fmt.Sprintf(
-		"%s:%s@tcp(%s)/%s?charset=utf8mb4&parseTime=true&loc=Local",
-		config.Config.Mysql.Username,
-		config.Config.Mysql.Password,
-		config.Config.Mysql.Address[0],
-		config.Config.Mysql.Database,
-	)
+	dsn := fmt.Sprintf("%s:%s@tcp(%s)/%s?charset=utf8mb4&parseTime=true&loc=Local",
+		o.Username, o.Password, o.Address[0], o.Database)
 	sqlLogger := log.NewSqlLogger(
-		logger.LogLevel(config.Config.Mysql.LogLevel),
+		logger.LogLevel(o.LogLevel),
 		true,
-		time.Duration(config.Config.Mysql.SlowThreshold)*time.Millisecond,
+		time.Duration(o.SlowThreshold)*time.Millisecond,
 	)
-	db, err = gorm.Open(mysql.Open(dsn), &gorm.Config{
+	db, err := gorm.Open(mysql.Open(dsn), &gorm.Config{
 		Logger: sqlLogger,
 	})
 	if err != nil {
 		return nil, err
 	}
-	sqlDB, err = db.DB()
+	sqlDB, err := db.DB()
 	if err != nil {
 		return nil, err
 	}
-	sqlDB.SetConnMaxLifetime(time.Second * time.Duration(config.Config.Mysql.MaxLifeTime))
-	sqlDB.SetMaxOpenConns(config.Config.Mysql.MaxOpenConn)
-	sqlDB.SetMaxIdleConns(config.Config.Mysql.MaxIdleConn)
+	sqlDB.SetConnMaxLifetime(time.Second * time.Duration(o.MaxLifeTime))
+	sqlDB.SetMaxOpenConns(o.MaxOpenConn)
+	sqlDB.SetMaxIdleConns(o.MaxIdleConn)
 	return db, nil
 }
 
+// maybeCreateTable creates a database if it does not exists.
+func maybeCreateTable(o *option) error {
+	dsn := fmt.Sprintf("%s:%s@tcp(%s)/%s?charset=utf8mb4&parseTime=true&loc=Local",
+		o.Username, o.Password, o.Address[0], "mysql")
+
+	var db *gorm.DB
+	var err error
+	if f := o.Connect; f != nil {
+		db, err = f(dsn, maxRetry)
+	} else {
+		db, err = connectToDatabase(dsn, maxRetry)
+	}
+	if err != nil {
+		panic(err.Error() + " Open failed " + dsn)
+	}
+
+	sqlDB, err := db.DB()
+	if err != nil {
+		return err
+	}
+	defer sqlDB.Close()
+	sql := fmt.Sprintf(
+		"CREATE DATABASE IF NOT EXISTS `%s` default charset utf8mb4 COLLATE utf8mb4_unicode_ci",
+		o.Database,
+	)
+	err = db.Exec(sql).Error
+	if err != nil {
+		return fmt.Errorf("init db %w", err)
+	}
+	return nil
+}
+
 // connectToDatabase Connection retry for mysql.
 func connectToDatabase(dsn string, maxRetry int) (*gorm.DB, error) {
 	var db *gorm.DB
@@ -106,7 +127,18 @@ func connectToDatabase(dsn string, maxRetry int) (*gorm.DB, error) {
 func NewGormDB() (*gorm.DB, error) {
 	specialerror.AddReplace(gorm.ErrRecordNotFound, errs.ErrRecordNotFound)
 	specialerror.AddErrHandler(replaceDuplicateKey)
-	return newMysqlGormDB()
+
+	return newMysqlGormDB(&option{
+		Username:      config.Config.Mysql.Username,
+		Password:      config.Config.Mysql.Password,
+		Address:       config.Config.Mysql.Address,
+		Database:      config.Config.Mysql.Database,
+		LogLevel:      config.Config.Mysql.LogLevel,
+		SlowThreshold: config.Config.Mysql.SlowThreshold,
+		MaxLifeTime:   config.Config.Mysql.MaxLifeTime,
+		MaxOpenConn:   config.Config.Mysql.MaxOpenConn,
+		MaxIdleConn:   config.Config.Mysql.MaxIdleConn,
+	})
 }
 
 func replaceDuplicateKey(err error) errs.CodeError {


Test patch:
diff --git a/pkg/common/db/relation/mysql_init_test.go b/pkg/common/db/relation/mysql_init_test.go
new file mode 100644
index 000000000..c321dfd9f
--- /dev/null
+++ b/pkg/common/db/relation/mysql_init_test.go
@@ -0,0 +1,121 @@
+package relation
+
+import (
+	"context"
+	"database/sql"
+	"database/sql/driver"
+	"errors"
+	"fmt"
+	"reflect"
+	"testing"
+
+	"gorm.io/driver/mysql"
+	"gorm.io/gorm"
+	"gorm.io/gorm/logger"
+)
+
+func TestMaybeCreateTable(t *testing.T) {
+	t.Run("normal", func(t *testing.T) {
+		err := maybeCreateTable(&option{
+			Username:      "root",
+			Password:      "openIM123",
+			Address:       []string{"172.28.0.1:13306"},
+			Database:      "openIM_v3",
+			LogLevel:      4,
+			SlowThreshold: 500,
+			MaxOpenConn:   1000,
+			MaxIdleConn:   100,
+			MaxLifeTime:   60,
+			Connect: connect(expectExec{
+				query: "CREATE DATABASE IF NOT EXISTS `openIM_v3` default charset utf8mb4 COLLATE utf8mb4_unicode_ci",
+				args:  nil,
+			}),
+		})
+		if err != nil {
+			t.Fatal(err)
+		}
+	})
+
+	t.Run("im-db", func(t *testing.T) {
+		err := maybeCreateTable(&option{
+			Username:      "root",
+			Password:      "openIM123",
+			Address:       []string{"172.28.0.1:13306"},
+			Database:      "im-db",
+			LogLevel:      4,
+			SlowThreshold: 500,
+			MaxOpenConn:   1000,
+			MaxIdleConn:   100,
+			MaxLifeTime:   60,
+			Connect: connect(expectExec{
+				query: "CREATE DATABASE IF NOT EXISTS `im-db` default charset utf8mb4 COLLATE utf8mb4_unicode_ci",
+				args:  nil,
+			}),
+		})
+		if err != nil {
+			t.Fatal(err)
+		}
+	})
+
+	t.Run("err", func(t *testing.T) {
+		e := errors.New("e")
+		err := maybeCreateTable(&option{
+			Username:      "root",
+			Password:      "openIM123",
+			Address:       []string{"172.28.0.1:13306"},
+			Database:      "openIM_v3",
+			LogLevel:      4,
+			SlowThreshold: 500,
+			MaxOpenConn:   1000,
+			MaxIdleConn:   100,
+			MaxLifeTime:   60,
+			Connect: connect(expectExec{
+				err: e,
+			}),
+		})
+		if !errors.Is(err, e) {
+			t.Fatalf("err not is e: %v", err)
+		}
+	})
+}
+
+func connect(e expectExec) func(string, int) (*gorm.DB, error) {
+	return func(string, int) (*gorm.DB, error) {
+		return gorm.Open(mysql.New(mysql.Config{
+			SkipInitializeWithVersion: true,
+			Conn:                      sql.OpenDB(e),
+		}), &gorm.Config{
+			Logger: logger.Discard,
+		})
+	}
+}
+
+type expectExec struct {
+	err   error
+	query string
+	args  []driver.NamedValue
+}
+
+func (c expectExec) ExecContext(ctx context.Context, query string, args []driver.NamedValue) (driver.Result, error) {
+	if c.err != nil {
+		return nil, c.err
+	}
+	if query != c.query {
+		return nil, fmt.Errorf("query mismatch. expect: %s, got: %s", c.query, query)
+	}
+	if reflect.DeepEqual(args, c.args) {
+		return nil, fmt.Errorf("args mismatch. expect: %v, got: %v", c.args, args)
+	}
+	return noEffectResult{}, nil
+}
+
+func (e expectExec) Connect(context.Context) (driver.Conn, error) { return e, nil }
+func (expectExec) Driver() driver.Driver                          { panic("not implemented") }
+func (expectExec) Prepare(query string) (driver.Stmt, error)      { panic("not implemented") }
+func (expectExec) Close() (e error)                               { return }
+func (expectExec) Begin() (driver.Tx, error)                      { panic("not implemented") }
+
+type noEffectResult struct{}
+
+func (noEffectResult) LastInsertId() (i int64, e error) { return }
+func (noEffectResult) RowsAffected() (i int64, e error) { return }