support/config: allow multiple flags to be defined with the same name across commands

go.mod

@@ -69,7 +69,7 @@ require (
 	github.com/spf13/cast v0.0.0-20150508191742-4d07383ffe94 // indirect
 	github.com/spf13/cobra v0.0.0-20160830174925-9c28e4bbd74e
 	github.com/spf13/jwalterweatherman v0.0.0-20141219030609-3d60171a6431 // indirect
-	github.com/spf13/pflag v0.0.0-20161005214240-4bd69631f475 // indirect
+	github.com/spf13/pflag v0.0.0-20161005214240-4bd69631f475
 	github.com/spf13/viper v0.0.0-20150621231900-db7ff930a189
 	github.com/stellar/go-xdr v0.0.0-20180917104419-0bc96f33a18e
 	github.com/stellar/throttled v2.2.3-0.20190823235211-89d75816f59d+incompatible

services/horizon/cmd/db.go

@@ -28,6 +28,9 @@ var dbInitCmd = &cobra.Command{
 	Short: "install schema",
 	Long:  "init initializes the postgres database used by horizon.",
 	Run: func(cmd *cobra.Command, args []string) {
+		dbURLConfigOption.Require()
+		dbURLConfigOption.SetValue()
+
 		db, err := sql.Open("postgres", viper.GetString("db-url"))
 		if err != nil {
 			log.Fatal(err)
@@ -72,6 +75,9 @@ var dbMigrateCmd = &cobra.Command{
 			}
 		}
 
+		dbURLConfigOption.Require()
+		dbURLConfigOption.SetValue()
+
 		db, err := sql.Open("postgres", viper.GetString("db-url"))
 		if err != nil {
 			log.Fatal(err)

services/horizon/cmd/root.go

@@ -95,17 +95,19 @@ func checkMigrations() {
 	}
 }
 
+var dbURLConfigOption = &support.ConfigOption{
+	Name:      "db-url",
+	EnvVar:    "DATABASE_URL",
+	ConfigKey: &config.DatabaseURL,
+	OptType:   types.String,
+	Required:  true,
+	Usage:     "horizon postgres database to connect with",
+}
+
 // configOpts defines the complete flag configuration for horizon.
 // Add a new entry here to connect a new field in the horizon.Config struct
 var configOpts = support.ConfigOptions{
-	&support.ConfigOption{
-		Name:      "db-url",
-		EnvVar:    "DATABASE_URL",
-		ConfigKey: &config.DatabaseURL,
-		OptType:   types.String,
-		Required:  true,
-		Usage:     "horizon postgres database to connect with",
-	},
+	dbURLConfigOption,
 	&support.ConfigOption{
 		Name:      "stellar-core-db-url",
 		EnvVar:    "STELLAR_CORE_DATABASE_URL",

support/config/config_option.go

@@ -8,6 +8,7 @@ import (
 	"time"
 
 	"github.com/spf13/cobra"
+	"github.com/spf13/pflag"
 	"github.com/spf13/viper"
 	"github.com/stellar/go/support/errors"
 	"github.com/stellar/go/support/strutils"
@@ -52,6 +53,7 @@ type ConfigOption struct {
 	Usage          string              // Help text
 	CustomSetValue func(*ConfigOption) // Optional function for custom validation/transformation
 	ConfigKey      interface{}         // Pointer to the final key in the linked Config struct
+	flag           *pflag.Flag         // The persistent flag that the config option is attached to
 }
 
 // Init handles initialisation steps, including configuring and binding the env variable name.
@@ -65,15 +67,24 @@ func (co *ConfigOption) Init(cmd *cobra.Command) error {
 	return co.setFlag(cmd)
 }
 
+// Bind binds the config option to viper.
+func (co *ConfigOption) Bind() {
+	viper.BindPFlag(co.Name, co.flag)
+	viper.BindEnv(co.Name, co.EnvVar)
+}
+
 // Require checks that a required string configuration option is not empty, raising a user error if it is.
 func (co *ConfigOption) Require() {
+	co.Bind()
 	if co.Required && viper.GetString(co.Name) == "" {
 		stdLog.Fatalf("Invalid config: %s is blank. Please specify --%s on the command line or set the %s environment variable.", co.Name, co.Name, co.EnvVar)
 	}
 }
 
 // SetValue sets a value in the global config, using a custom function, if one was provided.
 func (co *ConfigOption) SetValue() {
+	co.Bind()
+
 	// Use a custom setting function, if one is provided
 	if co.CustomSetValue != nil {
 		co.CustomSetValue(co)
@@ -129,13 +140,8 @@ func (co *ConfigOption) setFlag(cmd *cobra.Command) error {
 		return errors.New("Unexpected OptType")
 	}
 
-	if err := viper.BindPFlag(co.Name, cmd.PersistentFlags().Lookup(co.Name)); err != nil {
-		return err
-	}
+	co.flag = cmd.PersistentFlags().Lookup(co.Name)
 
-	if err := viper.BindEnv(co.Name, co.EnvVar); err != nil {
-		return err
-	}
 	return nil
 }
 

support/config/config_option_test.go

@@ -1,8 +1,11 @@
 package config
 
 import (
+	"go/types"
+	"os"
 	"testing"
 
+	"github.com/spf13/cobra"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -13,3 +16,241 @@ func TestConfigOption_UsageText(t *testing.T) {
 	}
 	assert.Equal(t, "Port to listen and serve on (PORT)", configOpt.UsageText())
 }
+
+type testOptions struct {
+	String string
+	Int    int
+	Bool   bool
+	Uint   uint
+	Uint32 uint32
+}
+
+// Test that when there are no args the defaults in the config options are
+// used.
+func TestConfigOption_getSimpleValue_defaults(t *testing.T) {
+	opts := testOptions{}
+	configOpts := ConfigOptions{
+		{Name: "string", OptType: types.String, ConfigKey: &opts.String, FlagDefault: "default"},
+		{Name: "int", OptType: types.Int, ConfigKey: &opts.Int, FlagDefault: 1},
+		{Name: "bool", OptType: types.Bool, ConfigKey: &opts.Bool, FlagDefault: true},
+		{Name: "uint", OptType: types.Uint, ConfigKey: &opts.Uint, FlagDefault: uint(2)},
+		{Name: "uint32", OptType: types.Uint32, ConfigKey: &opts.Uint32, FlagDefault: uint32(3)},
+	}
+	cmd := &cobra.Command{
+		Use: "doathing",
+		Run: func(_ *cobra.Command, _ []string) {
+			configOpts.Require()
+			configOpts.SetValues()
+		},
+	}
+	configOpts.Init(cmd)
+
+	cmd.SetArgs([]string{})
+	cmd.Execute()
+	assert.Equal(t, "default", opts.String)
+	assert.Equal(t, 1, opts.Int)
+	assert.Equal(t, true, opts.Bool)
+	assert.Equal(t, uint(2), opts.Uint)
+	assert.Equal(t, uint32(3), opts.Uint32)
+}
+
+// Test that when args are given, their values are used.
+func TestConfigOption_getSimpleValue_setFlag(t *testing.T) {
+	opts := testOptions{}
+	configOpts := ConfigOptions{
+		{Name: "string", OptType: types.String, ConfigKey: &opts.String, FlagDefault: "default"},
+		{Name: "int", OptType: types.Int, ConfigKey: &opts.Int, FlagDefault: 1},
+		{Name: "bool", OptType: types.Bool, ConfigKey: &opts.Bool, FlagDefault: false},
+		{Name: "uint", OptType: types.Uint, ConfigKey: &opts.Uint, FlagDefault: uint(2)},
+		{Name: "uint32", OptType: types.Uint32, ConfigKey: &opts.Uint32, FlagDefault: uint32(3)},
+	}
+	cmd := &cobra.Command{
+		Use: "doathing",
+		Run: func(_ *cobra.Command, _ []string) {
+			configOpts.Require()
+			configOpts.SetValues()
+		},
+	}
+	configOpts.Init(cmd)
+
+	cmd.SetArgs([]string{
+		"--string", "value",
+		"--int", "10",
+		"--bool",
+		"--uint", "20",
+		"--uint32", "30",
+	})
+	cmd.Execute()
+	assert.Equal(t, "value", opts.String)
+	assert.Equal(t, 10, opts.Int)
+	assert.Equal(t, true, opts.Bool)
+	assert.Equal(t, uint(20), opts.Uint)
+	assert.Equal(t, uint32(30), opts.Uint32)
+}
+
+// Test that when args are not given but env vars are, their values are used.
+func TestConfigOption_getSimpleValue_setEnv(t *testing.T) {
+	opts := testOptions{}
+	configOpts := ConfigOptions{
+		{Name: "string", OptType: types.String, ConfigKey: &opts.String, FlagDefault: "default"},
+		{Name: "int", OptType: types.Int, ConfigKey: &opts.Int, FlagDefault: 1},
+		{Name: "bool", OptType: types.Bool, ConfigKey: &opts.Bool, FlagDefault: false},
+		{Name: "uint", OptType: types.Uint, ConfigKey: &opts.Uint, FlagDefault: uint(2)},
+		{Name: "uint32", OptType: types.Uint32, ConfigKey: &opts.Uint32, FlagDefault: uint32(3)},
+	}
+	cmd := &cobra.Command{
+		Use: "doathing",
+		Run: func(_ *cobra.Command, _ []string) {
+			configOpts.Require()
+			configOpts.SetValues()
+		},
+	}
+	configOpts.Init(cmd)
+
+	defer os.Setenv("STRING", os.Getenv("STRING"))
+	defer os.Setenv("INT", os.Getenv("INT"))
+	defer os.Setenv("BOOL", os.Getenv("BOOL"))
+	defer os.Setenv("UINT", os.Getenv("UINT"))
+	defer os.Setenv("UINT32", os.Getenv("UINT32"))
+	os.Setenv("STRING", "value")
+	os.Setenv("INT", "10")
+	os.Setenv("BOOL", "true")
+	os.Setenv("UINT", "20")
+	os.Setenv("UINT32", "30")
+	cmd.Execute()
+	assert.Equal(t, "value", opts.String)
+	assert.Equal(t, 10, opts.Int)
+	assert.Equal(t, true, opts.Bool)
+	assert.Equal(t, uint(20), opts.Uint)
+	assert.Equal(t, uint32(30), opts.Uint32)
+}
+
+// Test that when multiple commands register the same option, they can be set
+// with flags.
+func TestConfigOption_getSimpleValue_setMultipleFlag(t *testing.T) {
+	opts1 := testOptions{}
+	configOpts1 := ConfigOptions{
+		{Name: "string", OptType: types.String, ConfigKey: &opts1.String, FlagDefault: "default1"},
+		{Name: "int", OptType: types.Int, ConfigKey: &opts1.Int, FlagDefault: 11},
+		{Name: "bool", OptType: types.Bool, ConfigKey: &opts1.Bool, FlagDefault: false},
+		{Name: "uint", OptType: types.Uint, ConfigKey: &opts1.Uint, FlagDefault: uint(12)},
+		{Name: "uint32", OptType: types.Uint32, ConfigKey: &opts1.Uint32, FlagDefault: uint32(13)},
+	}
+	cmd1 := &cobra.Command{
+		Use: "doathing1",
+		Run: func(_ *cobra.Command, _ []string) {
+			configOpts1.Require()
+			configOpts1.SetValues()
+		},
+	}
+	configOpts1.Init(cmd1)
+
+	opts2 := testOptions{}
+	configOpts2 := ConfigOptions{
+		{Name: "string", OptType: types.String, ConfigKey: &opts2.String, FlagDefault: "default2"},
+		{Name: "int", OptType: types.Int, ConfigKey: &opts2.Int, FlagDefault: 21},
+		{Name: "bool", OptType: types.Bool, ConfigKey: &opts2.Bool, FlagDefault: false},
+		{Name: "uint", OptType: types.Uint, ConfigKey: &opts2.Uint, FlagDefault: uint(22)},
+		{Name: "uint32", OptType: types.Uint32, ConfigKey: &opts2.Uint32, FlagDefault: uint32(23)},
+	}
+	cmd2 := &cobra.Command{
+		Use: "doathing2",
+		Run: func(_ *cobra.Command, _ []string) {
+			configOpts2.Require()
+			configOpts2.SetValues()
+		},
+	}
+	configOpts2.Init(cmd2)
+
+	cmd1.SetArgs([]string{
+		"--string", "value1",
+		"--int", "110",
+		"--bool",
+		"--uint", "120",
+		"--uint32", "130",
+	})
+	cmd1.Execute()
+	assert.Equal(t, "value1", opts1.String)
+	assert.Equal(t, 110, opts1.Int)
+	assert.Equal(t, true, opts1.Bool)
+	assert.Equal(t, uint(120), opts1.Uint)
+	assert.Equal(t, uint32(130), opts1.Uint32)
+
+	cmd2.SetArgs([]string{
+		"--string", "value2",
+		"--int", "210",
+		"--bool",
+		"--uint", "220",
+		"--uint32", "230",
+	})
+	cmd2.Execute()
+	assert.Equal(t, "value2", opts2.String)
+	assert.Equal(t, 210, opts2.Int)
+	assert.Equal(t, true, opts2.Bool)
+	assert.Equal(t, uint(220), opts2.Uint)
+	assert.Equal(t, uint32(230), opts2.Uint32)
+}
+
+// Test that when multiple commands register the same option, they can be set
+// with environment variables.
+func TestConfigOption_getSimpleValue_setMultipleEnv(t *testing.T) {
+	opts1 := testOptions{}
+	configOpts1 := ConfigOptions{
+		{Name: "string", OptType: types.String, ConfigKey: &opts1.String, FlagDefault: "default1"},
+		{Name: "int", OptType: types.Int, ConfigKey: &opts1.Int, FlagDefault: 11},
+		{Name: "bool", OptType: types.Bool, ConfigKey: &opts1.Bool, FlagDefault: false},
+		{Name: "uint", OptType: types.Uint, ConfigKey: &opts1.Uint, FlagDefault: uint(12)},
+		{Name: "uint32", OptType: types.Uint32, ConfigKey: &opts1.Uint32, FlagDefault: uint32(13)},
+	}
+	cmd1 := &cobra.Command{
+		Use: "doathing1",
+		Run: func(_ *cobra.Command, _ []string) {
+			configOpts1.Require()
+			configOpts1.SetValues()
+		},
+	}
+	configOpts1.Init(cmd1)
+
+	opts2 := testOptions{}
+	configOpts2 := ConfigOptions{
+		{Name: "string", OptType: types.String, ConfigKey: &opts2.String, FlagDefault: "default2"},
+		{Name: "int", OptType: types.Int, ConfigKey: &opts2.Int, FlagDefault: 21},
+		{Name: "bool", OptType: types.Bool, ConfigKey: &opts2.Bool, FlagDefault: false},
+		{Name: "uint", OptType: types.Uint, ConfigKey: &opts2.Uint, FlagDefault: uint(22)},
+		{Name: "uint32", OptType: types.Uint32, ConfigKey: &opts2.Uint32, FlagDefault: uint32(23)},
+	}
+	cmd2 := &cobra.Command{
+		Use: "doathing2",
+		Run: func(_ *cobra.Command, _ []string) {
+			configOpts2.Require()
+			configOpts2.SetValues()
+		},
+	}
+	configOpts2.Init(cmd2)
+
+	defer os.Setenv("STRING", os.Getenv("STRING"))
+	defer os.Setenv("INT", os.Getenv("INT"))
+	defer os.Setenv("BOOL", os.Getenv("BOOL"))
+	defer os.Setenv("UINT", os.Getenv("UINT"))
+	defer os.Setenv("UINT32", os.Getenv("UINT32"))
+
+	os.Setenv("STRING", "value1")
+	os.Setenv("INT", "110")
+	os.Setenv("BOOL", "true")
+	os.Setenv("UINT", "120")
+	os.Setenv("UINT32", "130")
+
+	cmd1.Execute()
+	assert.Equal(t, "value1", opts1.String)
+	assert.Equal(t, 110, opts1.Int)
+	assert.Equal(t, true, opts1.Bool)
+	assert.Equal(t, uint(120), opts1.Uint)
+	assert.Equal(t, uint32(130), opts1.Uint32)
+
+	cmd2.Execute()
+	assert.Equal(t, "value1", opts2.String)
+	assert.Equal(t, 110, opts2.Int)
+	assert.Equal(t, true, opts2.Bool)
+	assert.Equal(t, uint(120), opts2.Uint)
+	assert.Equal(t, uint32(130), opts2.Uint32)
+}