diff --git a/core/engine.go b/core/engine.go index 395ced6..c808bf1 100644 --- a/core/engine.go +++ b/core/engine.go @@ -147,15 +147,8 @@ func (e *Engine) JobManager() *JobManager { return e.jobManager } -// Logger returns current logger in goroutine-safe way +// Logger returns current logger func (e *Engine) Logger() LoggerInterface { - e.mutex.RLock() - defer e.mutex.RUnlock() - return e.logger -} - -// UnsafeLogger returns current logger in goroutine-unsafe way. Using this method you can cause race condition. -func (e *Engine) UnsafeLogger() LoggerInterface { return e.logger } diff --git a/core/engine_test.go b/core/engine_test.go index d97af66..8024604 100644 --- a/core/engine_test.go +++ b/core/engine_test.go @@ -187,17 +187,6 @@ func (e *EngineTest) Test_WithFilesystemSessions() { assert.NotNil(e.T(), e.engine.Sessions) } -func (e *EngineTest) Test_UnsafeLogger() { - origLogger := e.engine.logger - defer func() { - e.engine.logger = origLogger - }() - e.engine.logger = nil - assert.Nil(e.T(), e.engine.UnsafeLogger()) - e.engine.logger = &Logger{} - assert.IsType(e.T(), &Logger{}, e.engine.UnsafeLogger()) -} - func (e *EngineTest) Test_SetLogger() { origLogger := e.engine.logger defer func() { diff --git a/core/logger.go b/core/logger.go index 1c440b9..06275b3 100644 --- a/core/logger.go +++ b/core/logger.go @@ -28,10 +28,10 @@ type LoggerInterface interface { } // Logger component. Uses github.com/op/go-logging under the hood. -// This logger utilises sync.RWMutex functionality in order to avoid race conditions (in some cases it is useful). +// This logger can prevent any write operations (disabled by default, use .Exclusive() method to enable) type Logger struct { logger *logging.Logger - mutex sync.RWMutex + mutex *sync.RWMutex } // NewLogger will create new goroutine-safe logger with specified formatter. @@ -62,114 +62,137 @@ func DefaultLogFormatter() logging.Formatter { ) } +// Exclusive makes logger goroutine-safe +func (l *Logger) Exclusive() *Logger { + if l.mutex == nil { + l.mutex = &sync.RWMutex{} + } + + return l +} + +// lock locks logger +func (l *Logger) lock() { + if l.mutex != nil { + l.mutex.Lock() + } +} + +// unlock unlocks logger +func (l *Logger) unlock() { + if l.mutex != nil { + l.mutex.Unlock() + } +} + // Fatal is equivalent to l.Critical(fmt.Sprint()) followed by a call to os.Exit(1). func (l *Logger) Fatal(args ...interface{}) { - l.mutex.Lock() - defer l.mutex.Unlock() + l.lock() + defer l.unlock() l.logger.Fatal(args...) } // Fatalf is equivalent to l.Critical followed by a call to os.Exit(1). func (l *Logger) Fatalf(format string, args ...interface{}) { - l.mutex.Lock() - defer l.mutex.Unlock() + l.lock() + defer l.unlock() l.logger.Fatalf(format, args...) } // Panic is equivalent to l.Critical(fmt.Sprint()) followed by a call to panic(). func (l *Logger) Panic(args ...interface{}) { - l.mutex.Lock() - defer l.mutex.Unlock() + l.lock() + defer l.unlock() l.logger.Panic(args...) } // Panicf is equivalent to l.Critical followed by a call to panic(). func (l *Logger) Panicf(format string, args ...interface{}) { - l.mutex.Lock() - defer l.mutex.Unlock() + l.lock() + defer l.unlock() l.logger.Panicf(format, args...) } // Critical logs a message using CRITICAL as log level. func (l *Logger) Critical(args ...interface{}) { - l.mutex.Lock() - defer l.mutex.Unlock() + l.lock() + defer l.unlock() l.logger.Critical(args...) } // Criticalf logs a message using CRITICAL as log level. func (l *Logger) Criticalf(format string, args ...interface{}) { - l.mutex.Lock() - defer l.mutex.Unlock() + l.lock() + defer l.unlock() l.logger.Criticalf(format, args...) } // Error logs a message using ERROR as log level. func (l *Logger) Error(args ...interface{}) { - l.mutex.Lock() - defer l.mutex.Unlock() + l.lock() + defer l.unlock() l.logger.Error(args...) } // Errorf logs a message using ERROR as log level. func (l *Logger) Errorf(format string, args ...interface{}) { - l.mutex.Lock() - defer l.mutex.Unlock() + l.lock() + defer l.unlock() l.logger.Errorf(format, args...) } // Warning logs a message using WARNING as log level. func (l *Logger) Warning(args ...interface{}) { - l.mutex.Lock() - defer l.mutex.Unlock() + l.lock() + defer l.unlock() l.logger.Warning(args...) } // Warningf logs a message using WARNING as log level. func (l *Logger) Warningf(format string, args ...interface{}) { - l.mutex.Lock() - defer l.mutex.Unlock() + l.lock() + defer l.unlock() l.logger.Warningf(format, args...) } // Notice logs a message using NOTICE as log level. func (l *Logger) Notice(args ...interface{}) { - l.mutex.Lock() - defer l.mutex.Unlock() + l.lock() + defer l.unlock() l.logger.Notice(args...) } // Noticef logs a message using NOTICE as log level. func (l *Logger) Noticef(format string, args ...interface{}) { - l.mutex.Lock() - defer l.mutex.Unlock() + l.lock() + defer l.unlock() l.logger.Noticef(format, args...) } // Info logs a message using INFO as log level. func (l *Logger) Info(args ...interface{}) { - l.mutex.Lock() - defer l.mutex.Unlock() + l.lock() + defer l.unlock() l.logger.Info(args...) } // Infof logs a message using INFO as log level. func (l *Logger) Infof(format string, args ...interface{}) { - l.mutex.Lock() - defer l.mutex.Unlock() + l.lock() + defer l.unlock() l.logger.Infof(format, args...) } // Debug logs a message using DEBUG as log level. func (l *Logger) Debug(args ...interface{}) { - l.mutex.Lock() - defer l.mutex.Unlock() + l.lock() + defer l.unlock() l.logger.Debug(args...) } // Debugf logs a message using DEBUG as log level. func (l *Logger) Debugf(format string, args ...interface{}) { - l.mutex.Lock() - defer l.mutex.Unlock() + l.lock() + defer l.unlock() l.logger.Debugf(format, args...) } diff --git a/core/logger_test.go b/core/logger_test.go index 756bb62..b7333db 100644 --- a/core/logger_test.go +++ b/core/logger_test.go @@ -31,6 +31,10 @@ func Test_Logger(t *testing.T) { suite.Run(t, new(LoggerTest)) } +func (t *LoggerTest) SetupSuite() { + t.logger = NewLogger("code", logging.DEBUG, DefaultLogFormatter()).Exclusive() +} + // TODO Cover Fatal and Fatalf (implementation below is no-op) // func (t *LoggerTest) Test_Fatal() { // if os.Getenv("FLAG") == "1" { @@ -64,7 +68,7 @@ func (t *LoggerTest) Test_Panicf() { func (t *LoggerTest) Test_Critical() { defer func() { - if v := recover(); v == nil { + if v := recover(); v != nil { t.T().Fatal(v) } }() @@ -73,7 +77,7 @@ func (t *LoggerTest) Test_Critical() { func (t *LoggerTest) Test_Criticalf() { defer func() { - if v := recover(); v == nil { + if v := recover(); v != nil { t.T().Fatal(v) } }() @@ -82,7 +86,7 @@ func (t *LoggerTest) Test_Criticalf() { func (t *LoggerTest) Test_Warning() { defer func() { - if v := recover(); v == nil { + if v := recover(); v != nil { t.T().Fatal(v) } }() @@ -91,7 +95,7 @@ func (t *LoggerTest) Test_Warning() { func (t *LoggerTest) Test_Notice() { defer func() { - if v := recover(); v == nil { + if v := recover(); v != nil { t.T().Fatal(v) } }() @@ -100,7 +104,7 @@ func (t *LoggerTest) Test_Notice() { func (t *LoggerTest) Test_Info() { defer func() { - if v := recover(); v == nil { + if v := recover(); v != nil { t.T().Fatal(v) } }() @@ -109,7 +113,7 @@ func (t *LoggerTest) Test_Info() { func (t *LoggerTest) Test_Debug() { defer func() { - if v := recover(); v == nil { + if v := recover(); v != nil { t.T().Fatal(v) } }()