From 96117c0a15bf3bd36f14fe7a99f1ad630a42acbf Mon Sep 17 00:00:00 2001
From: Rob Watson <rob@netflux.io>
Date: Mon, 10 Mar 2025 20:06:01 +0100
Subject: [PATCH] feat(config): logging

---
 internal/config/config.go                     | 12 ++---
 internal/config/service.go                    | 37 +++++++++++-----
 internal/config/service_test.go               | 22 ++++++++--
 internal/config/testdata/complete.yml         |  4 +-
 .../testdata/invalid-destination-url.yml      |  4 +-
 internal/config/testdata/logfile.yml          |  6 +++
 internal/config/testdata/no-logfile.yml       |  2 +
 internal/config/testdata/no-name.yml          |  4 +-
 internal/config/xdg.go                        | 44 +++++++++++++++++++
 main.go                                       | 20 +++++++--
 10 files changed, 129 insertions(+), 26 deletions(-)
 create mode 100644 internal/config/testdata/logfile.yml
 create mode 100644 internal/config/xdg.go

diff --git a/internal/config/config.go b/internal/config/config.go
index 19d9e63..b1170c5 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -1,17 +1,19 @@
 package config
 
-import "git.netflux.io/rob/octoplex/internal/domain"
-
-const defaultLogFile = domain.AppName + ".log"
-
 // Destination holds the configuration for a destination.
 type Destination struct {
 	Name string `yaml:"name"`
 	URL  string `yaml:"url"`
 }
 
+// LogFile holds the configuration for the log file.
+type LogFile struct {
+	Enabled bool   `yaml:"enabled"`
+	Path    string `yaml:"path"`
+}
+
 // Config holds the configuration for the application.
 type Config struct {
-	LogFile      string        `yaml:"logfile"`
+	LogFile      LogFile       `yaml:"logfile"`
 	Destinations []Destination `yaml:"destinations"`
 }
diff --git a/internal/config/service.go b/internal/config/service.go
index 87cf715..9efc9b5 100644
--- a/internal/config/service.go
+++ b/internal/config/service.go
@@ -13,7 +13,9 @@ import (
 
 // Service provides configuration services.
 type Service struct {
-	configDir string
+	userConfigDir string
+	appConfigDir  string
+	appStateDir   string
 }
 
 // ConfigDirFunc is a function that returns the user configuration directory.
@@ -26,14 +28,29 @@ func NewDefaultService() (*Service, error) {
 }
 
 // NewService creates a new service with provided ConfigDirFunc.
+//
+// The app data directories (config and state) are created if they do not
+// exist.
 func NewService(configDirFunc ConfigDirFunc) (*Service, error) {
-	userConfigDir, err := configDirFunc()
+	configDir, err := configDirFunc()
 	if err != nil {
 		return nil, fmt.Errorf("user config dir: %w", err)
 	}
 
+	appConfigDir, err := createAppConfigDir(configDir)
+	if err != nil {
+		return nil, fmt.Errorf("app config dir: %w", err)
+	}
+
+	appStateDir, err := createAppStateDir()
+	if err != nil {
+		return nil, fmt.Errorf("app state dir: %w", err)
+	}
+
 	return &Service{
-		configDir: filepath.Join(userConfigDir, domain.AppName),
+		userConfigDir: configDir,
+		appConfigDir:  appConfigDir,
+		appStateDir:   appStateDir,
 	}, nil
 }
 
@@ -59,7 +76,7 @@ func (s *Service) readConfig() (cfg Config, _ error) {
 		return cfg, fmt.Errorf("unmarshal: %w", err)
 	}
 
-	setDefaults(&cfg)
+	s.setDefaults(&cfg)
 
 	if err = validate(cfg); err != nil {
 		return cfg, err
@@ -69,11 +86,11 @@ func (s *Service) readConfig() (cfg Config, _ error) {
 }
 
 func (s *Service) createConfig() (cfg Config, _ error) {
-	if err := os.MkdirAll(s.configDir, 0744); err != nil {
+	if err := os.MkdirAll(s.appConfigDir, 0744); err != nil {
 		return cfg, fmt.Errorf("mkdir: %w", err)
 	}
 
-	setDefaults(&cfg)
+	s.setDefaults(&cfg)
 
 	yamlBytes, err := yaml.Marshal(cfg)
 	if err != nil {
@@ -88,12 +105,12 @@ func (s *Service) createConfig() (cfg Config, _ error) {
 }
 
 func (s *Service) Path() string {
-	return filepath.Join(s.configDir, "config.yaml")
+	return filepath.Join(s.appConfigDir, "config.yaml")
 }
 
-func setDefaults(cfg *Config) {
-	if cfg.LogFile == "" {
-		cfg.LogFile = defaultLogFile
+func (s *Service) setDefaults(cfg *Config) {
+	if cfg.LogFile.Enabled && cfg.LogFile.Path == "" {
+		cfg.LogFile.Path = filepath.Join(s.appStateDir, domain.AppName+".log")
 	}
 
 	for i := range cfg.Destinations {
diff --git a/internal/config/service_test.go b/internal/config/service_test.go
index 4f9187f..039a287 100644
--- a/internal/config/service_test.go
+++ b/internal/config/service_test.go
@@ -4,6 +4,7 @@ import (
 	_ "embed"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 
 	"git.netflux.io/rob/octoplex/internal/config"
@@ -18,6 +19,9 @@ var configComplete []byte
 //go:embed testdata/no-logfile.yml
 var configNoLogfile []byte
 
+//go:embed testdata/logfile.yml
+var configLogfile []byte
+
 //go:embed testdata/no-name.yml
 var configNoName []byte
 
@@ -34,7 +38,7 @@ func TestConfigServiceCreateConfig(t *testing.T) {
 
 	cfg, err := service.ReadOrCreateConfig()
 	require.NoError(t, err)
-	require.Equal(t, "octoplex.log", cfg.LogFile)
+	require.Empty(t, cfg.LogFile, "expected no log file")
 
 	p := filepath.Join(configDir(suffix), "config.yaml")
 	_, err = os.Stat(p)
@@ -55,7 +59,10 @@ func TestConfigServiceReadConfig(t *testing.T) {
 				require.Equal(
 					t,
 					config.Config{
-						LogFile: "test.log",
+						LogFile: config.LogFile{
+							Enabled: true,
+							Path:    "test.log",
+						},
 						Destinations: []config.Destination{
 							{
 								Name: "my stream",
@@ -66,10 +73,17 @@ func TestConfigServiceReadConfig(t *testing.T) {
 			},
 		},
 		{
-			name:        "no logfile",
+			name:        "logging enabled, no logfile",
 			configBytes: configNoLogfile,
 			want: func(t *testing.T, cfg config.Config) {
-				assert.Equal(t, "octoplex.log", cfg.LogFile)
+				assert.True(t, strings.HasSuffix(cfg.LogFile.Path, "/octoplex/octoplex.log"))
+			},
+		},
+		{
+			name:        "logging enabled, logfile",
+			configBytes: configLogfile,
+			want: func(t *testing.T, cfg config.Config) {
+				assert.Equal(t, "/tmp/octoplex.log", cfg.LogFile.Path)
 			},
 		},
 		{
diff --git a/internal/config/testdata/complete.yml b/internal/config/testdata/complete.yml
index 8dd6a86..6426993 100644
--- a/internal/config/testdata/complete.yml
+++ b/internal/config/testdata/complete.yml
@@ -1,5 +1,7 @@
 ---
-logfile: test.log
+logfile:
+  enabled: true
+  path: test.log
 destinations:
 - name: my stream
   url: rtmp://rtmp.example.com:1935/live
diff --git a/internal/config/testdata/invalid-destination-url.yml b/internal/config/testdata/invalid-destination-url.yml
index 037e169..24b3593 100644
--- a/internal/config/testdata/invalid-destination-url.yml
+++ b/internal/config/testdata/invalid-destination-url.yml
@@ -1,4 +1,6 @@
 ---
-logfile: test.log
+logfile:
+  enabled: true
+  path: test.log
 destinations:
 - url: http://nope.example.com:443/live
diff --git a/internal/config/testdata/logfile.yml b/internal/config/testdata/logfile.yml
new file mode 100644
index 0000000..5a642b2
--- /dev/null
+++ b/internal/config/testdata/logfile.yml
@@ -0,0 +1,6 @@
+---
+logfile:
+  enabled: true
+  path: /tmp/octoplex.log
+destinations:
+- url: rtmp://rtmp.example.com:1935/live
diff --git a/internal/config/testdata/no-logfile.yml b/internal/config/testdata/no-logfile.yml
index e392257..1825093 100644
--- a/internal/config/testdata/no-logfile.yml
+++ b/internal/config/testdata/no-logfile.yml
@@ -1,3 +1,5 @@
 ---
+logfile:
+  enabled: true
 destinations:
 - url: rtmp://rtmp.example.com:1935/live
diff --git a/internal/config/testdata/no-name.yml b/internal/config/testdata/no-name.yml
index 6542551..3b8c38a 100644
--- a/internal/config/testdata/no-name.yml
+++ b/internal/config/testdata/no-name.yml
@@ -1,4 +1,6 @@
 ---
-logfile: test.log
+logfile:
+  enabled: true
+  path: test.log
 destinations:
 - url: rtmp://rtmp.example.com:1935/live
diff --git a/internal/config/xdg.go b/internal/config/xdg.go
new file mode 100644
index 0000000..5e0e822
--- /dev/null
+++ b/internal/config/xdg.go
@@ -0,0 +1,44 @@
+package config
+
+import (
+	"errors"
+	"fmt"
+	"os"
+	"path/filepath"
+	"runtime"
+
+	"git.netflux.io/rob/octoplex/internal/domain"
+)
+
+func createAppConfigDir(configDir string) (string, error) {
+	path := filepath.Join(configDir, domain.AppName)
+	if err := os.MkdirAll(path, 0744); err != nil {
+		return "", fmt.Errorf("mkdir all: %w", err)
+	}
+
+	return path, nil
+}
+
+func createAppStateDir() (string, error) {
+	userHomeDir, err := os.UserHomeDir()
+	if err != nil {
+		return "", err
+	}
+
+	var dir string
+	switch runtime.GOOS {
+	case "darwin":
+		dir = filepath.Join(userHomeDir, "/Library", "Caches", domain.AppName)
+	case "windows":
+		// TODO: Windows support
+		return "", errors.New("not implemented")
+	default: // Unix-like
+		dir = filepath.Join(userHomeDir, ".state", domain.AppName)
+	}
+
+	if err := os.MkdirAll(dir, 0744); err != nil {
+		return "", fmt.Errorf("mkdir all: %w", err)
+	}
+
+	return dir, nil
+}
diff --git a/main.go b/main.go
index 879b0a2..2fc79ae 100644
--- a/main.go
+++ b/main.go
@@ -55,12 +55,10 @@ func run(ctx context.Context) error {
 	if err != nil {
 		return fmt.Errorf("read or create config: %w", err)
 	}
-
-	logFile, err := os.OpenFile(cfg.LogFile, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0666)
+	logger, err := buildLogger(cfg.LogFile)
 	if err != nil {
-		return fmt.Errorf("error opening log file: %w", err)
+		return fmt.Errorf("build logger: %w", err)
 	}
-	logger := slog.New(slog.NewTextHandler(logFile, nil))
 
 	var clipboardAvailable bool
 	if err = clipboard.Init(); err != nil {
@@ -127,3 +125,17 @@ func printVersion() error {
 	fmt.Fprintf(os.Stderr, "%s version %s\n", domain.AppName, "0.0.0")
 	return nil
 }
+
+// buildLogger builds the logger, which may be a no-op logger.
+func buildLogger(cfg config.LogFile) (*slog.Logger, error) {
+	if !cfg.Enabled {
+		return slog.New(slog.DiscardHandler), nil
+	}
+
+	fptr, err := os.OpenFile(cfg.Path, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0666)
+	if err != nil {
+		return nil, fmt.Errorf("error opening log file: %w", err)
+	}
+
+	return slog.New(slog.NewTextHandler(fptr, nil)), nil
+}