From c9e9b01b1a0e64f39264a502e8da5e32a192f6ef Mon Sep 17 00:00:00 2001 From: Josh Klopfenstein Date: Tue, 26 Sep 2023 11:18:09 -0500 Subject: [PATCH] Set the default data directory correctly (#1249) We now use cwd/juno as the data directory. Previously, we set the default to the empty string "" and manually set the default directory in `node.New`. This meant the incorrect default was printed in two cases: 1. `--help` 2. When printing the config on boot. Finally, this centralizes all of our config management to the `cmd/juno` package; if `juno.NewCmd.Execute` returns without an error, the config is structurally valid. --- cmd/juno/juno.go | 18 +++++++++- cmd/juno/juno_test.go | 7 +++- node/node.go | 8 ----- node/node_test.go | 23 ------------ utils/datadir.go | 50 -------------------------- utils/datadir_test.go | 81 ------------------------------------------- 6 files changed, 23 insertions(+), 164 deletions(-) delete mode 100644 utils/datadir.go delete mode 100644 utils/datadir_test.go diff --git a/cmd/juno/juno.go b/cmd/juno/juno.go index 8ac9e3a871..7bb93413ac 100644 --- a/cmd/juno/juno.go +++ b/cmd/juno/juno.go @@ -5,6 +5,7 @@ import ( "fmt" "os" "os/signal" + "path/filepath" "syscall" "time" @@ -60,7 +61,6 @@ const ( defaultHTTPPort = 6060 defaultWS = false defaultWSPort = 6061 - defaultDBPath = "" defaultEthNode = "" defaultPprof = false defaultPprofPort = 6062 @@ -148,10 +148,17 @@ func NewCmd(config *node.Config, run func(*cobra.Command, []string) error) *cobr } var cfgFile string + var cwdErr error // PreRunE populates the configuration struct from the Cobra flags and Viper configuration. // This is called in step 3 of the process described above. junoCmd.PreRunE = func(cmd *cobra.Command, _ []string) error { + // If we couldn't find the current working directory and the database path is empty, + // return the error. + if cwdErr != nil && config.DatabasePath == "" { + return fmt.Errorf("find current working directory: %v", cwdErr) + } + v := viper.New() if cfgFile != "" { v.SetConfigType("yaml") @@ -171,6 +178,15 @@ func NewCmd(config *node.Config, run func(*cobra.Command, []string) error) *cobr mapstructure.TextUnmarshallerHookFunc(), mapstructure.StringToTimeDurationHookFunc()))) } + var defaultDBPath string + defaultDBPath, cwdErr = os.Getwd() + // Use empty string if we can't get the working directory. + // We don't want to return an error here since that would make `--help` fail. + // If the error is non-nil and a db path is not provided by the user, we'll return it in PreRunE. + if cwdErr == nil { + defaultDBPath = filepath.Join(defaultDBPath, "juno") + } + // For testing purposes, these variables cannot be declared outside the function because Cobra // may mutate their values. defaultLogLevel := utils.INFO diff --git a/cmd/juno/juno_test.go b/cmd/juno/juno_test.go index bdf4fcf4c7..798d9a09b6 100644 --- a/cmd/juno/juno_test.go +++ b/cmd/juno/juno_test.go @@ -3,6 +3,7 @@ package main_test import ( "context" "os" + "path/filepath" "testing" "time" @@ -15,6 +16,9 @@ import ( ) func TestConfigPrecedence(t *testing.T) { + pwd, err := os.Getwd() + require.NoError(t, err) + // The purpose of these tests are to ensure the precedence of our config // values is respected. Since viper offers this feature, it would be // redundant to enumerate all combinations. Thus, only a select few are @@ -27,7 +31,7 @@ func TestConfigPrecedence(t *testing.T) { defaultHTTPPort := uint16(6060) defaultWS := false defaultWSPort := uint16(6061) - defaultDBPath := "" + defaultDBPath := filepath.Join(pwd, "juno") defaultNetwork := utils.MAINNET defaultPprof := false defaultPprofPort := uint16(6062) @@ -122,6 +126,7 @@ func TestConfigPrecedence(t *testing.T) { Pprof: defaultPprof, PprofHost: defaultHost, PprofPort: defaultPprofPort, + DatabasePath: defaultDBPath, }, }, "config file with all settings but without any other flags": { diff --git a/node/node.go b/node/node.go index 8a631d7457..3410cc2e43 100644 --- a/node/node.go +++ b/node/node.go @@ -6,7 +6,6 @@ import ( "fmt" "net/url" "os" - "path/filepath" "reflect" "runtime" "time" @@ -85,13 +84,6 @@ type Node struct { // New sets the config and logger to the StarknetNode. // Any errors while parsing the config on creating logger will be returned. func New(cfg *Config, version string) (*Node, error) { //nolint:gocyclo,funlen - if cfg.DatabasePath == "" { - dirPrefix, err := utils.DefaultDataDir() - if err != nil { - return nil, err - } - cfg.DatabasePath = filepath.Join(dirPrefix, cfg.Network.String()) - } log, err := utils.NewZapLogger(cfg.LogLevel, cfg.Colour) if err != nil { return nil, err diff --git a/node/node_test.go b/node/node_test.go index 789ce66812..5a4d177ec9 100644 --- a/node/node_test.go +++ b/node/node_test.go @@ -1,37 +1,14 @@ package node_test import ( - "path/filepath" "testing" "time" "github.com/NethermindEth/juno/node" "github.com/NethermindEth/juno/utils" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -func TestDefaultDbPath(t *testing.T) { - defaultDataDir, err := utils.DefaultDataDir() - require.NoError(t, err) - - networks := []utils.Network{utils.GOERLI, utils.MAINNET, utils.GOERLI2, utils.INTEGRATION} - - for _, n := range networks { - t.Run(n.String(), func(t *testing.T) { - cfg := &node.Config{Network: n, DatabasePath: ""} - expectedCfg := node.Config{ - Network: n, - DatabasePath: filepath.Join(defaultDataDir, n.String()), - } - snNode, err := node.New(cfg, "1.2.3") - require.NoError(t, err) - - assert.Equal(t, expectedCfg, snNode.Config()) - }) - } -} - // Create a new node with all services enabled. func TestNewNode(t *testing.T) { config := &node.Config{ diff --git a/utils/datadir.go b/utils/datadir.go deleted file mode 100644 index 4aa163b3e5..0000000000 --- a/utils/datadir.go +++ /dev/null @@ -1,50 +0,0 @@ -package utils - -import ( - "errors" - "os" - "path/filepath" - "runtime" -) - -func DefaultDataDir() (string, error) { - userDataDir := os.Getenv("XDG_DATA_HOME") - if runtime.GOOS == "windows" { - configDir, err := os.UserConfigDir() - if err != nil { - return "", err - } - userDataDir = configDir - } - - userHomeDir, err := os.UserHomeDir() - if err != nil { - return "", err - } - - dataDir := DataDir(runtime.GOOS, userDataDir, userHomeDir) - if dataDir == "" { - return "", errors.New("data directory not found") - } - return dataDir, nil -} - -func DataDir(operatingSystem, userDataDir, homeDir string) string { - local, share, juno := ".local", "share", "juno" - - if operatingSystem == "" || (userDataDir == "" && homeDir == "") { - return "" - } - - if operatingSystem == "windows" { - if userDataDir == "" { - return "" - } - return filepath.Join(userDataDir, juno) - } - - if userDataDir != "" { - return filepath.Join(userDataDir, juno) - } - return filepath.Join(homeDir, local, share, juno) -} diff --git a/utils/datadir_test.go b/utils/datadir_test.go deleted file mode 100644 index 339d81612a..0000000000 --- a/utils/datadir_test.go +++ /dev/null @@ -1,81 +0,0 @@ -package utils_test - -import ( - "testing" - - "github.com/NethermindEth/juno/utils" - "github.com/stretchr/testify/assert" -) - -func TestDataDir(t *testing.T) { - tests := []struct { - os string - userDataDir string - userHomeDir string - expectedDir string - }{ - {"", "noos/dataDir", "noos/homeDir", ""}, - {"", "noos/dataDir", "", ""}, - {"", "", "noos/homeDir", ""}, - {"", "", "", ""}, - - {"unknown", "unknown/dataDir", "unknown/homeDir", "unknown/dataDir/juno"}, - {"unknown", "unknown/dataDir", "", "unknown/dataDir/juno"}, - {"unknown", "", "unknown/homeDir", "unknown/homeDir/.local/share/juno"}, - {"unknown", "", "", ""}, - - {"windows", "windows/dataDir", "windows/homeDir", "windows/dataDir/juno"}, - {"windows", "windows/dataDir", "", "windows/dataDir/juno"}, - {"windows", "", "windows/homeDir", ""}, - {"windows", "", "", ""}, - - {"dragonfly", "dragonfly/dataDir", "dragonfly/homeDir", "dragonfly/dataDir/juno"}, - {"dragonfly", "dragonfly/dataDir", "", "dragonfly/dataDir/juno"}, - {"dragonfly", "", "dragonfly/homeDir", "dragonfly/homeDir/.local/share/juno"}, - {"dragonfly", "", "", ""}, - - {"freebsd", "freebsd/dataDir", "freebsd/homeDir", "freebsd/dataDir/juno"}, - {"freebsd", "freebsd/dataDir", "", "freebsd/dataDir/juno"}, - {"freebsd", "", "freebsd/homeDir", "freebsd/homeDir/.local/share/juno"}, - {"freebsd", "", "", ""}, - - {"illumos", "illumos/dataDir", "illumos/homeDir", "illumos/dataDir/juno"}, - {"illumos", "illumos/dataDir", "", "illumos/dataDir/juno"}, - {"illumos", "", "illumos/homeDir", "illumos/homeDir/.local/share/juno"}, - {"illumos", "", "", ""}, - - {"ios", "ios/dataDir", "ios/homeDir", "ios/dataDir/juno"}, - {"ios", "ios/dataDir", "", "ios/dataDir/juno"}, - {"ios", "", "ios/homeDir", "ios/homeDir/.local/share/juno"}, - {"ios", "", "", ""}, - - {"linux", "linux/dataDir", "linux/homeDir", "linux/dataDir/juno"}, - {"linux", "linux/dataDir", "", "linux/dataDir/juno"}, - {"linux", "", "linux/homeDir", "linux/homeDir/.local/share/juno"}, - {"linux", "", "", ""}, - - {"netbsd", "netbsd/dataDir", "netbsd/homeDir", "netbsd/dataDir/juno"}, - {"netbsd", "netbsd/dataDir", "", "netbsd/dataDir/juno"}, - {"netbsd", "", "netbsd/homeDir", "netbsd/homeDir/.local/share/juno"}, - {"netbsd", "", "", ""}, - - {"openbsd", "openbsd/dataDir", "openbsd/homeDir", "openbsd/dataDir/juno"}, - {"openbsd", "openbsd/dataDir", "", "openbsd/dataDir/juno"}, - {"openbsd", "", "openbsd/homeDir", "openbsd/homeDir/.local/share/juno"}, - {"openbsd", "", "", ""}, - - {"solaris", "solaris/dataDir", "solaris/homeDir", "solaris/dataDir/juno"}, - {"solaris", "solaris/dataDir", "", "solaris/dataDir/juno"}, - {"solaris", "", "solaris/homeDir", "solaris/homeDir/.local/share/juno"}, - {"solaris", "", "", ""}, - - {"plan9", "plan9/dataDir", "plan9/homeDir", "plan9/dataDir/juno"}, - {"plan9", "plan9/dataDir", "", "plan9/dataDir/juno"}, - {"plan9", "", "plan9/homeDir", "plan9/homeDir/.local/share/juno"}, - {"plan9", "", "", ""}, - } - - for _, tc := range tests { - assert.Equal(t, tc.expectedDir, utils.DataDir(tc.os, tc.userDataDir, tc.userHomeDir)) - } -}