diff --git a/pkg/db/db.go b/pkg/db/db.go index 01d567b1d..1d8f331fa 100644 --- a/pkg/db/db.go +++ b/pkg/db/db.go @@ -185,24 +185,74 @@ func initPostgresEngine() (engine *xorm.Engine, err error) { return } +// DatabasePathConfig holds configuration for database path resolution. +// This struct allows the path resolution logic to be tested independently +// of the global config package. +type DatabasePathConfig struct { + ConfiguredPath string // The database.path config value + RootPath string // The service.rootpath config value + ExecutablePath string // Directory of the executable binary +} + +// resolveDatabasePath resolves a database path configuration to an absolute path. +// +// Resolution rules: +// 1. If ConfiguredPath is "memory", returns "memory" (special case for in-memory DB) +// 2. If ConfiguredPath is already absolute, returns it as-is (cleaned) +// 3. If ConfiguredPath is relative: +// a. If RootPath differs from ExecutablePath (explicitly configured), +// joins with RootPath +// b. Otherwise, joins with platform-specific user data directory +// +// The getUserDataDir parameter allows injecting a mock for testing. +func resolveDatabasePath(cfg DatabasePathConfig, getUserDataDir func() (string, error)) (string, error) { + if cfg.ConfiguredPath == "memory" { + return "memory", nil + } + + var path string + + switch { + case filepath.IsAbs(cfg.ConfiguredPath): + path = filepath.Clean(cfg.ConfiguredPath) + case cfg.RootPath != cfg.ExecutablePath: + path = filepath.Join(cfg.RootPath, cfg.ConfiguredPath) + default: + dataDir, err := getUserDataDir() + if err != nil { + log.Warningf("Could not get user data directory, falling back to rootpath: %v", err) + path = filepath.Join(cfg.RootPath, cfg.ConfiguredPath) + } else { + path = filepath.Join(dataDir, cfg.ConfiguredPath) + } + } + + return filepath.Abs(path) +} + func initSqliteEngine() (engine *xorm.Engine, err error) { - path := config.DatabasePath.GetString() + rootPath := config.ServiceRootpath.GetString() + + executablePath := rootPath + if execPath, err := os.Executable(); err == nil { + executablePath = filepath.Dir(execPath) + } + + cfg := DatabasePathConfig{ + ConfiguredPath: config.DatabasePath.GetString(), + RootPath: rootPath, + ExecutablePath: executablePath, + } + + path, err := resolveDatabasePath(cfg, getUserDataDir) + if err != nil { + return nil, fmt.Errorf("could not resolve database path: %w", err) + } if path == "memory" { return xorm.NewEngine("sqlite3", "file::memory:?cache=shared") } - // Resolve relative paths to a safe location instead of the current working directory - if !filepath.IsAbs(path) { - path = resolveDatabasePath(path) - } - - // Convert to absolute path for logging - path, err = filepath.Abs(path) - if err != nil { - return nil, fmt.Errorf("could not resolve database path to absolute path: %w", err) - } - // Log the resolved database path log.Infof("Using SQLite database at: %s", path) @@ -229,34 +279,6 @@ func initSqliteEngine() (engine *xorm.Engine, err error) { return xorm.NewEngine("sqlite3", path) } -// resolveDatabasePath resolves a relative database path to an appropriate location -// based on platform-specific conventions. This prevents SQLite databases from being -// created in system directories (like C:\Windows\System32 on Windows) when Vikunja -// runs as a service. -func resolveDatabasePath(relativePath string) string { - // First, check if a rootpath is explicitly configured and use that - rootPath := config.ServiceRootpath.GetString() - - // Determine if rootpath appears to be a user-configured value or a default - // If it looks like it was explicitly configured (not just the binary location), - // use it for backward compatibility - execPath, err := os.Executable() - if err == nil && rootPath != filepath.Dir(execPath) { - // rootpath was explicitly configured, use it - return filepath.Join(rootPath, relativePath) - } - - // Otherwise, use a platform-appropriate user data directory - dataDir, err := getUserDataDir() - if err != nil { - // Fall back to rootpath if we can't determine user data dir - log.Warningf("Could not determine user data directory, falling back to rootpath: %v", err) - return filepath.Join(rootPath, relativePath) - } - - return filepath.Join(dataDir, relativePath) -} - // getUserDataDir returns the platform-appropriate directory for application data func getUserDataDir() (string, error) { var dataDir string diff --git a/pkg/db/db_path_test.go b/pkg/db/db_path_test.go index 10f95cc05..c95ae08d3 100644 --- a/pkg/db/db_path_test.go +++ b/pkg/db/db_path_test.go @@ -17,76 +17,285 @@ package db import ( + "fmt" "os" "path/filepath" "runtime" "testing" - "code.vikunja.io/api/pkg/config" + "code.vikunja.io/api/pkg/log" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -func TestResolveDatabasePath(t *testing.T) { - // Save original config values - originalRootpath := config.ServiceRootpath.GetString() - defer config.ServiceRootpath.Set(originalRootpath) +func TestMain(m *testing.M) { + log.InitLogger() + os.Exit(m.Run()) +} +func Test_resolveDatabasePath(t *testing.T) { + mockGetUserDataDir := func(path string) func() (string, error) { + return func() (string, error) { + return path, nil + } + } + + mockGetUserDataDirError := func() (string, error) { + return "", fmt.Errorf("no home directory") + } + + tests := []struct { + name string + cfg DatabasePathConfig + getUserDataDir func() (string, error) + expected string + expectError bool + }{ + { + name: "memory database", + cfg: DatabasePathConfig{ + ConfiguredPath: "memory", + RootPath: "/opt/vikunja", + ExecutablePath: "/opt/vikunja", + }, + getUserDataDir: mockGetUserDataDir("/home/user/.local/share/vikunja"), + expected: "memory", + }, + + { + name: "absolute path should be used as-is", + cfg: DatabasePathConfig{ + ConfiguredPath: "/var/lib/vikunja/vikunja.db", + RootPath: "/opt/vikunja", + ExecutablePath: "/opt/vikunja", + }, + getUserDataDir: mockGetUserDataDir("/home/user/.local/share/vikunja"), + expected: "/var/lib/vikunja/vikunja.db", + }, + { + name: "absolute path with different rootpath still used as-is", + cfg: DatabasePathConfig{ + ConfiguredPath: "/data/mydb.db", + RootPath: "/custom/path", + ExecutablePath: "/opt/vikunja", + }, + getUserDataDir: mockGetUserDataDir("/home/user/.local/share/vikunja"), + expected: "/data/mydb.db", + }, + + { + name: "relative path with explicit rootpath", + cfg: DatabasePathConfig{ + ConfiguredPath: "vikunja.db", + RootPath: "/var/lib/vikunja", + ExecutablePath: "/opt/vikunja", + }, + getUserDataDir: mockGetUserDataDir("/home/user/.local/share/vikunja"), + expected: "/var/lib/vikunja/vikunja.db", + }, + { + name: "relative subdirectory path with explicit rootpath", + cfg: DatabasePathConfig{ + ConfiguredPath: "data/vikunja.db", + RootPath: "/var/lib/vikunja", + ExecutablePath: "/opt/vikunja", + }, + getUserDataDir: mockGetUserDataDir("/home/user/.local/share/vikunja"), + expected: "/var/lib/vikunja/data/vikunja.db", + }, + + { + name: "relative path with default rootpath uses user data dir", + cfg: DatabasePathConfig{ + ConfiguredPath: "vikunja.db", + RootPath: "/opt/vikunja", + ExecutablePath: "/opt/vikunja", + }, + getUserDataDir: mockGetUserDataDir("/home/user/.local/share/vikunja"), + expected: "/home/user/.local/share/vikunja/vikunja.db", + }, + + { + name: "os.Executable failure falls back to user data dir", + cfg: DatabasePathConfig{ + ConfiguredPath: "vikunja.db", + RootPath: "/opt/vikunja", + ExecutablePath: "/opt/vikunja", + }, + getUserDataDir: mockGetUserDataDir("/home/user/.local/share/vikunja"), + expected: "/home/user/.local/share/vikunja/vikunja.db", + }, + + { + name: "falls back to rootpath when getUserDataDir fails", + cfg: DatabasePathConfig{ + ConfiguredPath: "vikunja.db", + RootPath: "/opt/vikunja", + ExecutablePath: "/opt/vikunja", + }, + getUserDataDir: mockGetUserDataDirError, + expected: "/opt/vikunja/vikunja.db", + }, + + { + name: "empty configured path with explicit rootpath", + cfg: DatabasePathConfig{ + ConfiguredPath: "", + RootPath: "/var/lib/vikunja", + ExecutablePath: "/opt/vikunja", + }, + getUserDataDir: mockGetUserDataDir("/home/user/.local/share/vikunja"), + expected: "/var/lib/vikunja", + }, + { + name: "empty configured path with default rootpath", + cfg: DatabasePathConfig{ + ConfiguredPath: "", + RootPath: "/opt/vikunja", + ExecutablePath: "/opt/vikunja", + }, + getUserDataDir: mockGetUserDataDir("/home/user/.local/share/vikunja"), + expected: "/home/user/.local/share/vikunja", + }, + { + name: "path with dots normalized", + cfg: DatabasePathConfig{ + ConfiguredPath: "/var/lib/vikunja/../vikunja/./db.db", + RootPath: "/opt/vikunja", + ExecutablePath: "/opt/vikunja", + }, + getUserDataDir: mockGetUserDataDir("/home/user/.local/share/vikunja"), + expected: "/var/lib/vikunja/db.db", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := resolveDatabasePath(tt.cfg, tt.getUserDataDir) + + if tt.expectError { + require.Error(t, err) + return + } + + require.NoError(t, err) + assert.Equal(t, tt.expected, result) + }) + } +} + +func Test_resolveDatabasePath_Integration(t *testing.T) { t.Run("with explicitly configured rootpath", func(t *testing.T) { - // Set a rootpath that is different from the executable location - config.ServiceRootpath.Set("/custom/path") + cfg := DatabasePathConfig{ + ConfiguredPath: "vikunja.db", + RootPath: "/custom/path", + ExecutablePath: "/opt/vikunja", + } + + result, err := resolveDatabasePath(cfg, getUserDataDir) + require.NoError(t, err) - result := resolveDatabasePath("vikunja.db") expected := filepath.Join("/custom/path", "vikunja.db") - assert.Equal(t, expected, result) }) t.Run("with default rootpath uses user data directory", func(t *testing.T) { - // Get the actual executable path and set rootpath to it execPath, err := os.Executable() require.NoError(t, err) - config.ServiceRootpath.Set(filepath.Dir(execPath)) + execDir := filepath.Dir(execPath) - result := resolveDatabasePath("vikunja.db") + cfg := DatabasePathConfig{ + ConfiguredPath: "vikunja.db", + RootPath: execDir, + ExecutablePath: execDir, + } - // Result should contain the platform-specific user data directory - // and not be in the executable directory - assert.NotEqual(t, filepath.Join(filepath.Dir(execPath), "vikunja.db"), result) + result, err := resolveDatabasePath(cfg, getUserDataDir) + require.NoError(t, err) + + assert.NotEqual(t, filepath.Join(execDir, "vikunja.db"), result) assert.Contains(t, result, "vikunja.db") - // Verify it's using a platform-appropriate path switch runtime.GOOS { case "windows": - // Should be in %LOCALAPPDATA%\Vikunja or %USERPROFILE%\AppData\Local\Vikunja assert.Contains(t, result, "Vikunja") case "darwin": - // Should be in ~/Library/Application Support/Vikunja assert.Contains(t, result, "Library") assert.Contains(t, result, "Application Support") default: - // Should be in ~/.local/share/vikunja or $XDG_DATA_HOME/vikunja assert.NotEqual(t, filepath.Dir(result), - filepath.Dir(execPath), + execDir, "Database should not be in executable directory", ) } }) t.Run("with subdirectory path", func(t *testing.T) { - config.ServiceRootpath.Set("/custom/path") + cfg := DatabasePathConfig{ + ConfiguredPath: "data/vikunja.db", + RootPath: "/custom/path", + ExecutablePath: "/opt/vikunja", + } + + result, err := resolveDatabasePath(cfg, getUserDataDir) + require.NoError(t, err) - result := resolveDatabasePath("data/vikunja.db") expected := filepath.Join("/custom/path", "data", "vikunja.db") - assert.Equal(t, expected, result) }) } -func TestGetUserDataDir(t *testing.T) { +func Test_resolveDatabasePath_Windows(t *testing.T) { + if runtime.GOOS != "windows" { + t.Skip("Skipping Windows-specific test on non-Windows platform") + } + mockGetUserDataDir := func(path string) func() (string, error) { + return func() (string, error) { + return path, nil + } + } + + tests := []struct { + name string + cfg DatabasePathConfig + getUserDataDir func() (string, error) + expected string + }{ + { + name: "windows absolute path", + cfg: DatabasePathConfig{ + ConfiguredPath: "C:\\ProgramData\\Vikunja\\vikunja.db", + RootPath: "C:\\Program Files\\Vikunja", + ExecutablePath: "C:\\Program Files\\Vikunja", + }, + getUserDataDir: mockGetUserDataDir("C:\\Users\\test\\AppData\\Local\\Vikunja"), + expected: "C:\\ProgramData\\Vikunja\\vikunja.db", + }, + { + name: "windows relative path with explicit rootpath", + cfg: DatabasePathConfig{ + ConfiguredPath: "vikunja.db", + RootPath: "C:\\ProgramData\\Vikunja", + ExecutablePath: "C:\\Program Files\\Vikunja", + }, + getUserDataDir: mockGetUserDataDir("C:\\Users\\test\\AppData\\Local\\Vikunja"), + expected: "C:\\ProgramData\\Vikunja\\vikunja.db", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := resolveDatabasePath(tt.cfg, tt.getUserDataDir) + require.NoError(t, err) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestGetUserDataDir(t *testing.T) { test := func() string { dataDir, err := getUserDataDir() require.NoError(t, err)