Merge pull request #384 from HyperloopUPV-H8/backend/logger

refactor(logger): improve structure, fix issues and add test coverage

Author: Humanoidear

backend/.gitignore

@@ -24,3 +24,8 @@ audience_static
 
 cmd/adj/
 cmd/config.toml
+
+
+# Test data
+c.out
+cover.out

backend/cmd/main.go

@@ -44,8 +44,6 @@ import (
 	"github.com/HyperloopUPV-H8/h9-backend/pkg/logger"
 	data_logger "github.com/HyperloopUPV-H8/h9-backend/pkg/logger/data"
 	order_logger "github.com/HyperloopUPV-H8/h9-backend/pkg/logger/order"
-	protection_logger "github.com/HyperloopUPV-H8/h9-backend/pkg/logger/protection"
-	state_logger "github.com/HyperloopUPV-H8/h9-backend/pkg/logger/state"
 	"github.com/HyperloopUPV-H8/h9-backend/pkg/transport"
 	"github.com/HyperloopUPV-H8/h9-backend/pkg/transport/network/sniffer"
 	"github.com/HyperloopUPV-H8/h9-backend/pkg/transport/network/tcp"
@@ -176,12 +174,9 @@ func main() {
 	updateFactory := update_factory.NewFactory(boardToPackets)
 
 	// <--- logger --->
-	var boardMap map[abstraction.BoardId]string
 	var subloggers = map[abstraction.LoggerName]abstraction.Logger{
-		data_logger.Name:       data_logger.NewLogger(),
-		protection_logger.Name: protection_logger.NewLogger(boardMap),
-		order_logger.Name:      order_logger.NewLogger(),
-		state_logger.Name:      state_logger.NewLogger(),
+		data_logger.Name:  data_logger.NewLogger(),
+		order_logger.Name: order_logger.NewLogger(),
 	}
 
 	logger.SetFormatTimestamp(logger.TimeUnit(config.Logging.TimeUnit)) // MUST be before creating subloggers

backend/pkg/logger/.gitignore

@@ -0,0 +1 @@
+logger/

backend/pkg/logger/base/logger.go

@@ -0,0 +1,96 @@
+package loggerbase
+
+import (
+	"fmt"
+	"os"
+	"path"
+	"sync/atomic"
+	"time"
+
+	"github.com/HyperloopUPV-H8/h9-backend/pkg/abstraction"
+	loggerHandler "github.com/HyperloopUPV-H8/h9-backend/pkg/logger"
+	"github.com/HyperloopUPV-H8/h9-backend/pkg/transport/packet/data"
+)
+
+/*********
+* Record *
+*********/
+
+type Record struct {
+	Packet    *data.Packet
+	From      string
+	To        string
+	Timestamp time.Time
+}
+
+/*************
+* BaseLogger *
+*************/
+
+type BaseLogger struct {
+
+	// An atomic boolean is used in order to use CompareAndSwap in the Start and Stop methods
+	Running *atomic.Bool
+
+	// save the starting time of the logger in Unix microseconds in order to log relative timestamps
+	StartTime int64
+
+	// Name of the logger
+	Name abstraction.LoggerName
+}
+
+// Function to start the base logger
+func (sublogger *BaseLogger) Start() error {
+	if !sublogger.Running.CompareAndSwap(false, true) {
+		fmt.Println("Logger already running")
+		return nil
+	}
+
+	sublogger.StartTime = loggerHandler.FormatTimestamp(time.Now()) // Update the start time
+
+	fmt.Println("Logger started " + string(sublogger.Name) + ".")
+	return nil
+}
+
+// Function to create the base file path
+
+func (sublogger *BaseLogger) CreateFile(filename string) (*os.File, error) {
+
+	err := os.MkdirAll(path.Dir(filename), os.ModePerm)
+	if err != nil {
+		return nil, loggerHandler.ErrCreatingAllDir{
+			Name:      sublogger.Name,
+			Timestamp: time.Now(),
+			Path:      filename,
+		}
+	}
+
+	return os.Create(path.Join(filename))
+}
+
+// Create a base Logger with default values
+func NewBaseLogger(name abstraction.LoggerName) *BaseLogger {
+	return &BaseLogger{
+		Running:   &atomic.Bool{},
+		StartTime: 0,
+		Name:      name,
+	}
+}
+
+func (sublogger *BaseLogger) PullRecord(abstraction.LoggerRequest) (abstraction.LoggerRecord, error) {
+	panic("TODO!")
+}
+
+// Function to stop the base logger
+// Param templateStop is a function that contains the specific stop actions of each logger
+func (sublogger *BaseLogger) Stop(templateStop func() error) error {
+	if !sublogger.Running.CompareAndSwap(true, false) {
+		fmt.Println("Logger already stopped" + string(sublogger.Name) + ".")
+		return nil
+	}
+
+	output := templateStop()
+
+	fmt.Println("Logger stopped " + string(sublogger.Name) + ".")
+	return output
+}

backend/pkg/logger/base/logger_test.go

@@ -0,0 +1,172 @@
+package loggerbase
+
+import (
+	"os"
+
+	"testing"
+)
+
+func createLoggerForTest(t *testing.T) *BaseLogger {
+
+	t.Helper() // Marks this function as a test helper, avoids cluttering test output
+
+	logger := NewBaseLogger("TEST")
+	return logger
+}
+
+// chdirTemp changes the current working directory to a temporary directory for the duration of the test.
+func chdirTemp(t *testing.T) string {
+	t.Helper()
+	tmp := t.TempDir()
+	old, err := os.Getwd()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := os.Chdir(tmp); err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(func() { _ = os.Chdir(old) })
+	return tmp
+}
+
+/*******
+* Start *
+*******/
+
+func TestStart(t *testing.T) {
+
+	_ = chdirTemp(t) // Change to a temporary directory
+
+	logger := createLoggerForTest(t)
+
+	// Sample data
+
+	var startTime int64
+
+	t.Run("Start (first time)", func(t *testing.T) {
+
+		// First start should succeed
+		out := logger.Start()
+
+		if out != nil {
+			t.Fatalf("expected nil, got %v", out)
+		}
+		// Capture start time
+		startTime = logger.StartTime
+
+	})
+
+	t.Run("Start (not first time)", func(t *testing.T) {
+
+		// El test Start (first time) debe haber tenido éxito y haber establecido startTime
+		if startTime == 0 {
+			t.Fatal("precondition failed: startTime should be set from the first start")
+		}
+
+		// Subsequent starts shouldn't change startTime
+		out := logger.Start()
+		if out != nil {
+			t.Fatalf("expected nil, got %v", out)
+		}
+		if logger.StartTime != startTime {
+			t.Fatalf("expected startTime to be %d, got %d", startTime, logger.StartTime)
+		}
+	})
+
+}
+
+/*************
+* Create File *
+*************/
+
+func TestCreateFile(t *testing.T) {
+
+	_ = chdirTemp(t) // Change to a temporary directory
+	logger := createLoggerForTest(t)
+
+	t.Run("Create File (valid path)", func(t *testing.T) {
+		filename := "logs/testlog.txt"
+		file, err := logger.CreateFile(filename)
+		if err != nil {
+			t.Fatalf("expected nil error, got %v", err)
+		}
+		defer file.Close()
+
+		// Verify file was created
+		if _, err := os.Stat(filename); os.IsNotExist(err) {
+			t.Fatalf("expected file %s to exist, but it does not", filename)
+		}
+	})
+
+	t.Run("Create File (invalid path)", func(t *testing.T) {
+		// Using an invalid path (e.g., a path with illegal characters)
+		invalidFilename := string([]byte{0}) + "invalidlog.txt"
+		_, err := logger.CreateFile(invalidFilename)
+		if err == nil {
+			t.Fatal("expected an error for invalid path, got nil")
+		}
+	})
+}
+
+/******
+* Stop *
+******/
+func TestStop(t *testing.T) {
+
+	_ = chdirTemp(t) // Change to a temporary directory
+	logger := createLoggerForTest(t)
+
+	// Sample stop function
+	sampleStopFunc := func() error {
+		// Simulate some stop actions
+		return nil
+	}
+
+	t.Run("Stop (not started)", func(t *testing.T) {
+
+		// Stopping without starting should be a no-op
+		out := logger.Stop(sampleStopFunc)
+		if out != nil {
+			t.Fatalf("expected nil, got %v", out)
+		}
+	})
+
+	t.Run("Stop (after start)", func(t *testing.T) {
+
+		// Start the logger first
+		_ = logger.Start()
+
+		// Now stop the logger
+		out := logger.Stop(sampleStopFunc)
+		if out != nil {
+			t.Fatalf("expected nil, got %v", out)
+		}
+	})
+
+	t.Run("Stop (already stopped)", func(t *testing.T) {
+
+		// Stopping again should be a no-op
+		out := logger.Stop(sampleStopFunc)
+		if out != nil {
+			t.Fatalf("expected nil, got %v", out)
+		}
+	})
+
+	// With a function that returns an error
+	errorStopFunc := func() error {
+		return os.ErrInvalid
+	}
+
+	t.Run("Stop (with error)", func(t *testing.T) {
+
+		// Start the logger first
+		_ = logger.Start()
+
+		// Now stop the logger with an error-producing function
+		out := logger.Stop(errorStopFunc)
+		if out != os.ErrInvalid {
+			t.Fatalf("expected os.ErrInvalid, got %v", out)
+		}
+	})
+
+}

backend/pkg/logger/data/.gitignore

@@ -0,0 +1 @@
+logger/