From 817ac36ce24af81b05e663a9d4f90e479b76d52a Mon Sep 17 00:00:00 2001 From: John Guo Date: Thu, 19 Dec 2024 10:11:55 +0800 Subject: [PATCH 1/5] ci: use latest go version for unit testing cases of contribution components (#4062) --- .github/workflows/ci-main.sh | 3 --- .github/workflows/ci-main.yml | 21 ++++++++++++++------- .github/workflows/ci-sub.yml | 11 ++++++++--- cmd/gf/go.work | 1 - 4 files changed, 22 insertions(+), 14 deletions(-) diff --git a/.github/workflows/ci-main.sh b/.github/workflows/ci-main.sh index 7171edab4e6..21891888024 100644 --- a/.github/workflows/ci-main.sh +++ b/.github/workflows/ci-main.sh @@ -1,8 +1,5 @@ #!/usr/bin/env bash -# Define the latest Go version requirement -LATEST_GO_VERSION="1.23" - coverage=$1 # find all path that contains go.mod. diff --git a/.github/workflows/ci-main.yml b/.github/workflows/ci-main.yml index ecbd084c67f..888476f77d9 100644 --- a/.github/workflows/ci-main.yml +++ b/.github/workflows/ci-main.yml @@ -28,10 +28,21 @@ concurrency: env: TZ: "Asia/Shanghai" - + # for unit testing cases of some components that only execute on the latest go version. + LATEST_GO_VERSION: "1.23" jobs: code-test: + strategy: + matrix: + # 🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥 + # When adding new go version to the list, make sure: + # 1. Update the `LATEST_GO_VERSION` env variable. + # 2. Update the `Report Coverage` action. + # 🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥 + go-version: [ "1.20", "1.21", "1.22", "1.23" ] + goarch: [ "386", "amd64" ] + runs-on: ubuntu-20.04 # Service containers to run with `code-test` @@ -185,11 +196,6 @@ jobs: ports: - 2181:2181 - strategy: - matrix: - go-version: [ "1.20", "1.21", "1.22", "1.23" ] - goarch: [ "386", "amd64" ] - steps: # TODO: szenius/set-timezone update to node16 # sudo ln -sf /usr/share/zoneinfo/Asia/Shanghai /etc/localtime @@ -256,7 +262,8 @@ jobs: - name: Report Coverage uses: codecov/codecov-action@v4 - if: ${{ github.event_name == 'push' && github.ref == 'refs/heads/master' }} + # Only report coverage on the latest go version and amd64 arch + if: ${{ github.event_name == 'push' && github.ref == 'refs/heads/master' && matrix.go-version == '1.23' && matrix.goarch == 'amd64' }} with: flags: go-${{ matrix.go-version }}-${{ matrix.goarch }} token: ${{ secrets.CODECOV_TOKEN }} diff --git a/.github/workflows/ci-sub.yml b/.github/workflows/ci-sub.yml index 56d9bb6eba3..5b98814ef36 100644 --- a/.github/workflows/ci-sub.yml +++ b/.github/workflows/ci-sub.yml @@ -29,17 +29,22 @@ concurrency: env: TZ: "Asia/Shanghai" - + # for unit testing cases of some components that only execute on the latest go version. + LATEST_GO_VERSION: "1.23" jobs: code-test: - runs-on: ubuntu-latest - strategy: matrix: + # 🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥 + # When adding new go version to the list, make sure: + # 1. Update the `LATEST_GO_VERSION` env variable. + # 🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥 go-version: [ "1.20", "1.21", "1.22", "1.23" ] goarch: [ "386", "amd64" ] + runs-on: ubuntu-latest + steps: - name: Setup Timezone uses: szenius/set-timezone@v2.0 diff --git a/cmd/gf/go.work b/cmd/gf/go.work index 1fbd6cb9a7c..19bee584aad 100644 --- a/cmd/gf/go.work +++ b/cmd/gf/go.work @@ -16,6 +16,5 @@ replace ( github.com/gogf/gf/contrib/drivers/oracle/v2 => ../../contrib/drivers/oracle github.com/gogf/gf/contrib/drivers/pgsql/v2 => ../../contrib/drivers/pgsql github.com/gogf/gf/contrib/drivers/sqlite/v2 => ../../contrib/drivers/sqlite - github.com/gogf/gf/contrib/drivers/dm/v2 => ../../contrib/drivers/dm github.com/gogf/gf/v2 => ../../ ) From 4c2a78b7bf23c48aaf17924a46822e452e6624e8 Mon Sep 17 00:00:00 2001 From: "Mr.Fan" <6213378@qq.com> Date: Thu, 19 Dec 2024 21:44:36 +0800 Subject: [PATCH 2/5] fix(database/gdb): regular expression pattern for link configuration to be compitable with tidbcloud (#4064) --- database/gdb/gdb.go | 2 +- database/gdb/gdb_z_mysql_internal_test.go | 34 +++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/database/gdb/gdb.go b/database/gdb/gdb.go index de3cdf3d369..558ca92c957 100644 --- a/database/gdb/gdb.go +++ b/database/gdb/gdb.go @@ -708,7 +708,7 @@ const ( ctxKeyCatchSQL gctx.StrKey = `CtxKeyCatchSQL` ctxKeyInternalProducedSQL gctx.StrKey = `CtxKeyInternalProducedSQL` - linkPattern = `(\w+):([\w\-\$]*):(.*?)@(\w+?)\((.+?)\)/{0,1}([^\?]*)\?{0,1}(.*)` + linkPattern = `^(\w+):(.*?):(.*?)@(\w+?)\((.+?)\)/{0,1}([^\?]*)\?{0,1}(.*?)$` linkPatternDescription = `type:username:password@protocol(host:port)/dbname?param1=value1&...¶mN=valueN` ) diff --git a/database/gdb/gdb_z_mysql_internal_test.go b/database/gdb/gdb_z_mysql_internal_test.go index ec70000b907..8539c2c9481 100644 --- a/database/gdb/gdb_z_mysql_internal_test.go +++ b/database/gdb/gdb_z_mysql_internal_test.go @@ -295,6 +295,40 @@ func Test_parseConfigNodeLink_WithType(t *testing.T) { t.Assert(newNode.Charset, `utf8`) t.Assert(newNode.Protocol, `unix`) }) + // https://github.com/gogf/gf/issues/4059 + gtest.C(t, func(t *gtest.T) { + node := &ConfigNode{ + Link: "tidb:2hcmRccccxxx9Fizz.root:wP3xxxxPIDc@tcp(xxxx.tidbcloud.com:4000)/db_name?tls=true", + } + newNode, err := parseConfigNodeLink(node) + t.AssertNil(err) + t.Assert(newNode.Type, `tidb`) + t.Assert(newNode.User, `2hcmRccccxxx9Fizz.root`) + t.Assert(newNode.Pass, `wP3xxxxPIDc`) + t.Assert(newNode.Host, `xxxx.tidbcloud.com`) + t.Assert(newNode.Port, `4000`) + t.Assert(newNode.Name, `db_name`) + t.Assert(newNode.Extra, `tls=true`) + t.Assert(newNode.Charset, `utf8`) + t.Assert(newNode.Protocol, `tcp`) + }) + gtest.C(t, func(t *gtest.T) { + node := &ConfigNode{ + Type: "tidb", + Link: "2hcmRccccxxx9Fizz.root:wP3xxxxPIDc@tcp(xxxx.tidbcloud.com:4000)/db_name?tls=true", + } + newNode, err := parseConfigNodeLink(node) + t.AssertNil(err) + t.Assert(newNode.Type, `tidb`) + t.Assert(newNode.User, `2hcmRccccxxx9Fizz.root`) + t.Assert(newNode.Pass, `wP3xxxxPIDc`) + t.Assert(newNode.Host, `xxxx.tidbcloud.com`) + t.Assert(newNode.Port, `4000`) + t.Assert(newNode.Name, `db_name`) + t.Assert(newNode.Extra, `tls=true`) + t.Assert(newNode.Charset, `utf8`) + t.Assert(newNode.Protocol, `tcp`) + }) } func Test_Func_doQuoteWord(t *testing.T) { From 594979c5afd22f01ca717247e6e243a0fc808d57 Mon Sep 17 00:00:00 2001 From: John Guo Date: Thu, 19 Dec 2024 22:31:17 +0800 Subject: [PATCH 3/5] fix(net/ghttp): nil pointer panic error when server logger set nil (#4055) --- net/ghttp/ghttp_server_config.go | 9 +++++++-- net/ghttp/ghttp_server_config_logging.go | 4 ++-- net/ghttp/ghttp_server_log.go | 4 ++-- net/ghttp/ghttp_z_unit_issue_test.go | 12 ++++++++++++ 4 files changed, 23 insertions(+), 6 deletions(-) diff --git a/net/ghttp/ghttp_server_config.go b/net/ghttp/ghttp_server_config.go index 450f9bb05fb..f5a6f4fe969 100644 --- a/net/ghttp/ghttp_server_config.go +++ b/net/ghttp/ghttp_server_config.go @@ -343,6 +343,9 @@ func (s *Server) SetConfigWithMap(m map[string]interface{}) error { if k, v := gutil.MapPossibleItemByKey(m, "FormParsingMemory"); k != "" { m[k] = gfile.StrToSize(gconv.String(v)) } + if _, v := gutil.MapPossibleItemByKey(m, "Logger"); v == nil { + intlog.Printf(context.TODO(), "SetConfigWithMap: set Logger nil") + } // Update the current configuration object. // It only updates the configured keys not all the object. if err := gconv.Struct(m, &s.config); err != nil { @@ -379,8 +382,10 @@ func (s *Server) SetConfig(c ServerConfig) error { return err } } - if err := s.config.Logger.SetLevelStr(s.config.LogLevel); err != nil { - intlog.Errorf(context.TODO(), `%+v`, err) + if s.config.Logger != nil { + if err := s.config.Logger.SetLevelStr(s.config.LogLevel); err != nil { + intlog.Errorf(context.TODO(), `%+v`, err) + } } gracefulEnabled = c.Graceful intlog.Printf(context.TODO(), "SetConfig: %+v", s.config) diff --git a/net/ghttp/ghttp_server_config_logging.go b/net/ghttp/ghttp_server_config_logging.go index f3e566b397e..c35edeb54eb 100644 --- a/net/ghttp/ghttp_server_config_logging.go +++ b/net/ghttp/ghttp_server_config_logging.go @@ -68,10 +68,10 @@ func (s *Server) GetLogPath() string { // IsAccessLogEnabled checks whether the access log enabled. func (s *Server) IsAccessLogEnabled() bool { - return s.config.AccessLogEnabled + return s.config.AccessLogEnabled && s.config.Logger != nil } // IsErrorLogEnabled checks whether the error log enabled. func (s *Server) IsErrorLogEnabled() bool { - return s.config.ErrorLogEnabled + return s.config.ErrorLogEnabled && s.config.Logger != nil } diff --git a/net/ghttp/ghttp_server_log.go b/net/ghttp/ghttp_server_log.go index 9f4e917dfbd..49b498ad09f 100644 --- a/net/ghttp/ghttp_server_log.go +++ b/net/ghttp/ghttp_server_log.go @@ -31,7 +31,7 @@ func (s *Server) handleAccessLog(r *Request) { r.GetClientIp(), r.Referer(), r.UserAgent(), ) logger := instance.GetOrSetFuncLock(loggerInstanceKey, func() interface{} { - l := s.Logger().Clone() + l := s.Logger() l.SetFile(s.config.AccessLogPattern) l.SetStdoutPrint(s.config.LogStdout) l.SetLevelPrint(false) @@ -73,7 +73,7 @@ func (s *Server) handleErrorLog(err error, r *Request) { content += ", " + err.Error() } logger := instance.GetOrSetFuncLock(loggerInstanceKey, func() interface{} { - l := s.Logger().Clone() + l := s.Logger() l.SetStack(false) l.SetFile(s.config.ErrorLogPattern) l.SetStdoutPrint(s.config.LogStdout) diff --git a/net/ghttp/ghttp_z_unit_issue_test.go b/net/ghttp/ghttp_z_unit_issue_test.go index d0d17b8471c..080597d0d52 100644 --- a/net/ghttp/ghttp_z_unit_issue_test.go +++ b/net/ghttp/ghttp_z_unit_issue_test.go @@ -618,3 +618,15 @@ func Test_Issue3789(t *testing.T) { t.Assert(c.GetContent(ctx, "/hello?id=&secondId=2&thirdId=3"), expect) }) } + +// https://github.com/gogf/gf/issues/4047 +func Test_Issue4047(t *testing.T) { + gtest.C(t, func(t *gtest.T) { + s := g.Server(guid.S()) + err := s.SetConfigWithMap(g.Map{ + "logger": nil, + }) + t.AssertNil(err) + t.Assert(s.Logger(), nil) + }) +} From 9ecf6f21a47c2480a594ece86bb2cb094b40461b Mon Sep 17 00:00:00 2001 From: wlynxg Date: Tue, 24 Dec 2024 10:17:47 +0800 Subject: [PATCH 4/5] feat: move plugin remove logic to Shutdown() && call Shutdown() when Run() exits --- net/ghttp/ghttp_server.go | 14 +++----------- net/ghttp/ghttp_server_admin.go | 13 ++++++++++++- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/net/ghttp/ghttp_server.go b/net/ghttp/ghttp_server.go index be567d068d7..6e84d9c952a 100644 --- a/net/ghttp/ghttp_server.go +++ b/net/ghttp/ghttp_server.go @@ -454,17 +454,9 @@ func (s *Server) Run() { // Blocking using the channel for graceful restart. <-s.closeChan - // Remove plugins. - if len(s.plugins) > 0 { - for _, p := range s.plugins { - intlog.Printf(ctx, `remove plugin: %s`, p.Name()) - if err := p.Remove(); err != nil { - intlog.Errorf(ctx, "%+v", err) - } - } - } - s.doServiceDeregister() - s.Logger().Infof(ctx, "pid[%d]: all servers shutdown", gproc.Pid()) + + // Shutdown the server + _ = s.Shutdown() } // Wait blocks to wait for all servers done. diff --git a/net/ghttp/ghttp_server_admin.go b/net/ghttp/ghttp_server_admin.go index 58f63575dec..df6540a83b9 100644 --- a/net/ghttp/ghttp_server_admin.go +++ b/net/ghttp/ghttp_server_admin.go @@ -88,7 +88,7 @@ func (s *Server) EnableAdmin(pattern ...string) { s.BindObject(p, &utilAdmin{}) } -// Shutdown shuts down current server. +// Shutdown shuts the current server. func (s *Server) Shutdown() error { var ctx = context.TODO() s.doServiceDeregister() @@ -97,5 +97,16 @@ func (s *Server) Shutdown() error { for _, v := range s.servers { v.Shutdown(ctx) } + s.Logger().Infof(ctx, "pid[%d]: all servers shutdown", gproc.Pid()) + + // Remove plugins. + if len(s.plugins) > 0 { + for _, p := range s.plugins { + s.Logger().Printf(ctx, `remove plugin: %s`, p.Name()) + if err := p.Remove(); err != nil { + s.Logger().Errorf(ctx, "%+v", err) + } + } + } return nil } From 6c4a3706ce78e4b5d64f0039571dd6430e415115 Mon Sep 17 00:00:00 2001 From: wlynxg Date: Wed, 25 Dec 2024 16:46:19 +0800 Subject: [PATCH 5/5] feat: execute the remove plugin operation first --- net/ghttp/ghttp_server_admin.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/net/ghttp/ghttp_server_admin.go b/net/ghttp/ghttp_server_admin.go index df6540a83b9..987e51a3ea4 100644 --- a/net/ghttp/ghttp_server_admin.go +++ b/net/ghttp/ghttp_server_admin.go @@ -91,14 +91,6 @@ func (s *Server) EnableAdmin(pattern ...string) { // Shutdown shuts the current server. func (s *Server) Shutdown() error { var ctx = context.TODO() - s.doServiceDeregister() - // Only shut down current servers. - // It may have multiple underlying http servers. - for _, v := range s.servers { - v.Shutdown(ctx) - } - s.Logger().Infof(ctx, "pid[%d]: all servers shutdown", gproc.Pid()) - // Remove plugins. if len(s.plugins) > 0 { for _, p := range s.plugins { @@ -108,5 +100,13 @@ func (s *Server) Shutdown() error { } } } + + s.doServiceDeregister() + // Only shut down current servers. + // It may have multiple underlying http servers. + for _, v := range s.servers { + v.Shutdown(ctx) + } + s.Logger().Infof(ctx, "pid[%d]: all servers shutdown", gproc.Pid()) return nil }