From db80eaead1620c977c4d7a245b8736f8142c0c55 Mon Sep 17 00:00:00 2001 From: Ming Deng Date: Sun, 22 Sep 2019 23:45:48 +0800 Subject: [PATCH] Fix issue for review --- filter/impl/access_log_filter.go | 61 ++++++++++++++------------- filter/impl/access_log_filter_test.go | 27 +++++++++--- 2 files changed, 54 insertions(+), 34 deletions(-) diff --git a/filter/impl/access_log_filter.go b/filter/impl/access_log_filter.go index 52dde6e901..1b5aa403bc 100644 --- a/filter/impl/access_log_filter.go +++ b/filter/impl/access_log_filter.go @@ -17,20 +17,23 @@ package impl +import ( + "os" + "reflect" + "strings" + "time" +) + import ( "github.com/apache/dubbo-go/common/constant" "github.com/apache/dubbo-go/common/extension" "github.com/apache/dubbo-go/common/logger" "github.com/apache/dubbo-go/filter" "github.com/apache/dubbo-go/protocol" - "os" - "reflect" - "strings" - "time" ) const ( - //usd in URL. + //used in URL. AccessLogKey = "accesslog" FileDateFormat = "2006-01-02" MessageDateLayout = "2006-01-02 15:04:05" @@ -72,7 +75,7 @@ func (ef *AccessLogFilter) logIntoChannel(accessLogData AccessLogData) { } func (ef *AccessLogFilter) buildAccessLogData(invoker protocol.Invoker, invocation protocol.Invocation) map[string]string { - dataMap := make(map[string]string) + dataMap := make(map[string]string, 16) attachments := invocation.Attachments() dataMap[constant.INTERFACE_KEY] = attachments[constant.INTERFACE_KEY] dataMap[constant.METHOD_KEY] = invocation.MethodName() @@ -86,15 +89,15 @@ func (ef *AccessLogFilter) buildAccessLogData(invoker protocol.Invoker, invocati builder := strings.Builder{} // todo(after the paramTypes were set to the invocation. we should change this implementation) typeBuilder := strings.Builder{} - first := true - for _, arg := range invocation.Arguments() { - if first { - first = false - } else { - builder.WriteString(",") - typeBuilder.WriteString(",") - } + + builder.WriteString(reflect.ValueOf(invocation.Arguments()[0]).String()) + typeBuilder.WriteString(reflect.TypeOf(invocation.Arguments()[0]).Name()) + for idx := 1; idx < len(invocation.Arguments()); idx++ { + arg := invocation.Arguments()[idx] + builder.WriteString(",") builder.WriteString(reflect.ValueOf(arg).String()) + + typeBuilder.WriteString(",") typeBuilder.WriteString(reflect.TypeOf(arg).Name()) } dataMap[Arguments] = builder.String() @@ -112,19 +115,20 @@ func (ef *AccessLogFilter) writeLogToFile(data AccessLogData) { accessLog := data.accessLog if isDefault(accessLog) { logger.Info(data.toLogMessage()) - } else { - logFile, err := ef.openLogFile(accessLog) - if err != nil { - logger.Warnf("Can not open the access log file: %s, %v", accessLog, err) - return - } - logger.Debugf("Append log to %s", accessLog) - message := data.toLogMessage() - message = message + "\n" - _, err = logFile.WriteString(message) - if err != nil { - logger.Warnf("Can not write the log into access log file: %s, %v", accessLog, err) - } + return + } + + logFile, err := ef.openLogFile(accessLog) + if err != nil { + logger.Warnf("Can not open the access log file: %s, %v", accessLog, err) + return + } + logger.Debugf("Append log to %s", accessLog) + message := data.toLogMessage() + message = message + "\n" + _, err = logFile.WriteString(message) + if err != nil { + logger.Warnf("Can not write the log into access log file: %s, %v", accessLog, err) } } @@ -146,9 +150,8 @@ func (ef *AccessLogFilter) openLogFile(accessLog string) (*os.File, error) { if err != nil { logger.Warnf("Can not rename access log file: %s, %v", fileInfo.Name(), err) return nil, err - } else { - logFile, err = os.OpenFile(accessLog, os.O_CREATE|os.O_APPEND|os.O_RDWR, LogFileMode) } + logFile, err = os.OpenFile(accessLog, os.O_CREATE|os.O_APPEND|os.O_RDWR, LogFileMode) } return logFile, err } diff --git a/filter/impl/access_log_filter_test.go b/filter/impl/access_log_filter_test.go index 662c991a7c..9d77d07cbd 100644 --- a/filter/impl/access_log_filter_test.go +++ b/filter/impl/access_log_filter_test.go @@ -19,12 +19,20 @@ package impl import ( "context" + "testing" + + "github.com/apache/dubbo-go/common/constant" +) + +import ( + "github.com/golang/mock/gomock" + "github.com/stretchr/testify/assert" +) + +import ( "github.com/apache/dubbo-go/common" "github.com/apache/dubbo-go/protocol" "github.com/apache/dubbo-go/protocol/invocation" - "github.com/golang/mock/gomock" - "github.com/stretchr/testify/assert" - "testing" ) func TestAccessLogFilter_Invoke_Not_Config(t *testing.T) { @@ -39,7 +47,7 @@ func TestAccessLogFilter_Invoke_Not_Config(t *testing.T) { invoker := protocol.NewBaseInvoker(url) attach := make(map[string]string, 10) - inv := invocation.NewRPCInvocation("MethodName", []interface{}{"OK"}, attach) + inv := invocation.NewRPCInvocation("MethodName", []interface{}{"OK", "Hello"}, attach) accessLogFilter := GetAccessLogFilter() result := accessLogFilter.Invoke(invoker, inv) @@ -58,9 +66,18 @@ func TestAccessLogFilter_Invoke_Default_Config(t *testing.T) { invoker := protocol.NewBaseInvoker(url) attach := make(map[string]string, 10) - inv := invocation.NewRPCInvocation("MethodName", []interface{}{"OK"}, attach) + attach[constant.VERSION_KEY] = "1.0" + attach[constant.GROUP_KEY] = "MyGroup" + inv := invocation.NewRPCInvocation("MethodName", []interface{}{"OK", "Hello"}, attach) accessLogFilter := GetAccessLogFilter() result := accessLogFilter.Invoke(invoker, inv) assert.Nil(t, result.Error()) } + +func TestAccessLogFilter_OnResponse(t *testing.T) { + result := &protocol.RPCResult{} + accessLogFilter := GetAccessLogFilter() + response := accessLogFilter.OnResponse(result, nil, nil) + assert.Equal(t, result, response) +}