From 2874fdc59214ca13071d8550d57cdc8a5181c6e2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 5 Jun 2025 15:42:27 +0000 Subject: [PATCH 1/5] Initial plan for issue From ea7cae48b10916fdac17479d51f0b5d1d8172740 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 5 Jun 2025 15:46:35 +0000 Subject: [PATCH 2/5] Initial analysis: Add test showing DECKARD_MONGODB_DATABASE issue Co-authored-by: lucasoares <10624972+lucasoares@users.noreply.github.com> --- internal/config/mongo_database_test.go | 48 ++++++++++++++++++++++++++ internal/service/cert/ca-cert.srl | 1 + 2 files changed, 49 insertions(+) create mode 100644 internal/config/mongo_database_test.go create mode 100644 internal/service/cert/ca-cert.srl diff --git a/internal/config/mongo_database_test.go b/internal/config/mongo_database_test.go new file mode 100644 index 0000000..518a7e1 --- /dev/null +++ b/internal/config/mongo_database_test.go @@ -0,0 +1,48 @@ +package config + +import ( + "os" + "testing" + + "github.com/spf13/viper" + "github.com/stretchr/testify/require" +) + +func TestMongoDatabaseEnvironmentVariable(t *testing.T) { + // Import viper for debugging + defer func() { + // Clean up + os.Unsetenv("DECKARD_MONGODB_DATABASE") + os.Unsetenv("DECKARD_MONGO_DATABASE") + }() + + // Clean environment + os.Unsetenv("DECKARD_MONGODB_DATABASE") + os.Unsetenv("DECKARD_MONGO_DATABASE") + + // Test default value + Configure(true) + require.Equal(t, "deckard", MongoDatabase.Get()) + + // Test with DECKARD_MONGODB_DATABASE environment variable + os.Setenv("DECKARD_MONGODB_DATABASE", "custom_db_1") + Configure(true) + + // Debug: let's see what viper sees + t.Logf("MongoDatabase key: %s", MongoDatabase.GetKey()) + t.Logf("MongoDatabase aliases: %v", MongoDatabase.GetAliases()) + + // Let's check what viper sees for each key + t.Logf("viper.IsSet(mongo.database): %v", viper.IsSet("mongo.database")) + t.Logf("viper.IsSet(mongodb.database): %v", viper.IsSet("mongodb.database")) + t.Logf("viper.GetString(mongo.database): %s", viper.GetString("mongo.database")) + t.Logf("viper.GetString(mongodb.database): %s", viper.GetString("mongodb.database")) + + require.Equal(t, "custom_db_1", MongoDatabase.Get()) + + // Test with DECKARD_MONGO_DATABASE environment variable + os.Unsetenv("DECKARD_MONGODB_DATABASE") + os.Setenv("DECKARD_MONGO_DATABASE", "custom_db_2") + Configure(true) + require.Equal(t, "custom_db_2", MongoDatabase.Get()) +} \ No newline at end of file diff --git a/internal/service/cert/ca-cert.srl b/internal/service/cert/ca-cert.srl new file mode 100644 index 0000000..48b28e0 --- /dev/null +++ b/internal/service/cert/ca-cert.srl @@ -0,0 +1 @@ +05327763CD7DDF3F33B47C48B2116EE36472D01E From 668bf8cd76c8c01195a9de72d3ef08f68f236cb1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 5 Jun 2025 15:54:55 +0000 Subject: [PATCH 3/5] Fix ViperConfigKey methods to prioritize environment variables over defaults Co-authored-by: lucasoares <10624972+lucasoares@users.noreply.github.com> --- internal/config/mongo_database_test.go | 96 +++++++++++++++++++++---- internal/config/viper_config.go | 98 ++++++++++++++++++-------- 2 files changed, 150 insertions(+), 44 deletions(-) diff --git a/internal/config/mongo_database_test.go b/internal/config/mongo_database_test.go index 518a7e1..c7b2fd9 100644 --- a/internal/config/mongo_database_test.go +++ b/internal/config/mongo_database_test.go @@ -4,12 +4,10 @@ import ( "os" "testing" - "github.com/spf13/viper" "github.com/stretchr/testify/require" ) func TestMongoDatabaseEnvironmentVariable(t *testing.T) { - // Import viper for debugging defer func() { // Clean up os.Unsetenv("DECKARD_MONGODB_DATABASE") @@ -27,17 +25,6 @@ func TestMongoDatabaseEnvironmentVariable(t *testing.T) { // Test with DECKARD_MONGODB_DATABASE environment variable os.Setenv("DECKARD_MONGODB_DATABASE", "custom_db_1") Configure(true) - - // Debug: let's see what viper sees - t.Logf("MongoDatabase key: %s", MongoDatabase.GetKey()) - t.Logf("MongoDatabase aliases: %v", MongoDatabase.GetAliases()) - - // Let's check what viper sees for each key - t.Logf("viper.IsSet(mongo.database): %v", viper.IsSet("mongo.database")) - t.Logf("viper.IsSet(mongodb.database): %v", viper.IsSet("mongodb.database")) - t.Logf("viper.GetString(mongo.database): %s", viper.GetString("mongo.database")) - t.Logf("viper.GetString(mongodb.database): %s", viper.GetString("mongodb.database")) - require.Equal(t, "custom_db_1", MongoDatabase.Get()) // Test with DECKARD_MONGO_DATABASE environment variable @@ -45,4 +32,85 @@ func TestMongoDatabaseEnvironmentVariable(t *testing.T) { os.Setenv("DECKARD_MONGO_DATABASE", "custom_db_2") Configure(true) require.Equal(t, "custom_db_2", MongoDatabase.Get()) -} \ No newline at end of file +} + +func TestMongoCollectionEnvironmentVariable(t *testing.T) { + defer func() { + // Clean up + os.Unsetenv("DECKARD_MONGODB_COLLECTION") + os.Unsetenv("DECKARD_MONGO_COLLECTION") + }() + + // Clean environment + os.Unsetenv("DECKARD_MONGODB_COLLECTION") + os.Unsetenv("DECKARD_MONGO_COLLECTION") + + // Test default value + Configure(true) + require.Equal(t, "queue", MongoCollection.Get()) + + // Test with DECKARD_MONGODB_COLLECTION environment variable + os.Setenv("DECKARD_MONGODB_COLLECTION", "custom_collection") + Configure(true) + require.Equal(t, "custom_collection", MongoCollection.Get()) + + // Test with DECKARD_MONGO_COLLECTION environment variable + os.Unsetenv("DECKARD_MONGODB_COLLECTION") + os.Setenv("DECKARD_MONGO_COLLECTION", "custom_collection_2") + Configure(true) + require.Equal(t, "custom_collection_2", MongoCollection.Get()) +} + +func TestMongoBooleanEnvironmentVariable(t *testing.T) { + defer func() { + // Clean up + os.Unsetenv("DECKARD_MONGODB_SSL") + os.Unsetenv("DECKARD_MONGO_SSL") + }() + + // Clean environment + os.Unsetenv("DECKARD_MONGODB_SSL") + os.Unsetenv("DECKARD_MONGO_SSL") + + // Test default value + Configure(true) + require.Equal(t, false, MongoSsl.GetBool()) + + // Test with DECKARD_MONGODB_SSL environment variable + os.Setenv("DECKARD_MONGODB_SSL", "true") + Configure(true) + require.Equal(t, true, MongoSsl.GetBool()) + + // Test with DECKARD_MONGO_SSL environment variable + os.Unsetenv("DECKARD_MONGODB_SSL") + os.Setenv("DECKARD_MONGO_SSL", "true") + Configure(true) + require.Equal(t, true, MongoSsl.GetBool()) +} + +func TestMongoIntegerEnvironmentVariable(t *testing.T) { + defer func() { + // Clean up + os.Unsetenv("DECKARD_MONGODB_MAX_POOL_SIZE") + os.Unsetenv("DECKARD_MONGO_MAX_POOL_SIZE") + }() + + // Clean environment + os.Unsetenv("DECKARD_MONGODB_MAX_POOL_SIZE") + os.Unsetenv("DECKARD_MONGO_MAX_POOL_SIZE") + + // Test default value + Configure(true) + require.Equal(t, 100, MongoMaxPoolSize.GetInt()) + + // Test with DECKARD_MONGODB_MAX_POOL_SIZE environment variable + os.Setenv("DECKARD_MONGODB_MAX_POOL_SIZE", "250") + Configure(true) + require.Equal(t, 250, MongoMaxPoolSize.GetInt()) + + // Test with DECKARD_MONGO_MAX_POOL_SIZE environment variable + os.Unsetenv("DECKARD_MONGODB_MAX_POOL_SIZE") + os.Setenv("DECKARD_MONGO_MAX_POOL_SIZE", "300") + Configure(true) + require.Equal(t, 300, MongoMaxPoolSize.GetInt()) +} diff --git a/internal/config/viper_config.go b/internal/config/viper_config.go index 1d54118..622120f 100644 --- a/internal/config/viper_config.go +++ b/internal/config/viper_config.go @@ -34,78 +34,116 @@ func (config *ViperConfigKey) Set(value any) { // Should never be called before config is initialized using config.Configure() func (config *ViperConfigKey) Get() string { + defaultVal := "" + if val, ok := config.GetDefault().(string); ok { + defaultVal = val + } + + // Check main key - if it differs from default, use it (environment variable takes precedence) if viper.IsSet(config.Key) { - return viper.GetString(config.Key) + keyVal := viper.GetString(config.Key) + if keyVal != defaultVal { + return keyVal + } } + // Check aliases - if any differs from default, use it (environment variable takes precedence) for _, alias := range config.GetAliases() { if viper.IsSet(alias) { - return viper.GetString(alias) + aliasVal := viper.GetString(alias) + if aliasVal != defaultVal { + return aliasVal + } } } - if val, ok := config.GetDefault().(string); ok { - return val - } - - return "" + // Return default value + return defaultVal } // Should never be called before config is initialized using config.Configure() func (config *ViperConfigKey) GetDuration() time.Duration { + defaultVal := time.Duration(0) + if val, ok := config.GetDefault().(string); ok { + defaultVal, _ = time.ParseDuration(val) + } + + // Check main key - if it differs from default, use it (environment variable takes precedence) if viper.IsSet(config.Key) { - return viper.GetDuration(config.Key) + keyVal := viper.GetDuration(config.Key) + if keyVal != defaultVal { + return keyVal + } } + // Check aliases - if any differs from default, use it (environment variable takes precedence) for _, alias := range config.GetAliases() { if viper.IsSet(alias) { - return viper.GetDuration(alias) + aliasVal := viper.GetDuration(alias) + if aliasVal != defaultVal { + return aliasVal + } } } - if val, ok := config.GetDefault().(string); ok { - duration, _ := time.ParseDuration(val) - - return duration - } - - return 0 + // Return default value + return defaultVal } // Should never be called before config is initialized using config.Configure() func (config *ViperConfigKey) GetBool() bool { + defaultVal := false + if val, ok := config.GetDefault().(bool); ok { + defaultVal = val + } + + // Check main key - if it differs from default, use it (environment variable takes precedence) if viper.IsSet(config.Key) { - return viper.GetBool(config.Key) + keyVal := viper.GetBool(config.Key) + if keyVal != defaultVal { + return keyVal + } } + // Check aliases - if any differs from default, use it (environment variable takes precedence) for _, alias := range config.GetAliases() { if viper.IsSet(alias) { - return viper.GetBool(alias) + aliasVal := viper.GetBool(alias) + if aliasVal != defaultVal { + return aliasVal + } } } - if val, ok := config.GetDefault().(bool); ok { - return val - } - - return false + // Return default value + return defaultVal } // Should never be called before config is initialized using config.Configure() func (config *ViperConfigKey) GetInt() int { + defaultVal := 0 + if val, ok := config.GetDefault().(int); ok { + defaultVal = val + } + + // Check main key - if it differs from default, use it (environment variable takes precedence) if viper.IsSet(config.Key) { - return viper.GetInt(config.Key) + keyVal := viper.GetInt(config.Key) + if keyVal != defaultVal { + return keyVal + } } + // Check aliases - if any differs from default, use it (environment variable takes precedence) for _, alias := range config.GetAliases() { if viper.IsSet(alias) { - return viper.GetInt(alias) + aliasVal := viper.GetInt(alias) + if aliasVal != defaultVal { + return aliasVal + } } } - if val, ok := config.GetDefault().(int); ok { - return val - } - - return 0 + // Return default value + return defaultVal } From 766c4d0b70b0e583a9f6bcd12dc2c3199ecd8627 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 5 Jun 2025 16:08:53 +0000 Subject: [PATCH 4/5] Refactor: Extract helper function to reduce code duplication in getters Co-authored-by: lucasoares <10624972+lucasoares@users.noreply.github.com> --- internal/config/viper_config.go | 88 +++++++-------------------------- 1 file changed, 19 insertions(+), 69 deletions(-) diff --git a/internal/config/viper_config.go b/internal/config/viper_config.go index 622120f..8355014 100644 --- a/internal/config/viper_config.go +++ b/internal/config/viper_config.go @@ -32,16 +32,13 @@ func (config *ViperConfigKey) Set(value any) { } } -// Should never be called before config is initialized using config.Configure() -func (config *ViperConfigKey) Get() string { - defaultVal := "" - if val, ok := config.GetDefault().(string); ok { - defaultVal = val - } - +// getWithFallback is a helper that implements the common logic for all getter methods. +// It checks the main key and aliases for values different from the default, returning +// the first override found, or the default if no overrides exist. +func getWithFallback[T comparable](config *ViperConfigKey, defaultVal T, keyGetter func(string) T) T { // Check main key - if it differs from default, use it (environment variable takes precedence) if viper.IsSet(config.Key) { - keyVal := viper.GetString(config.Key) + keyVal := keyGetter(config.Key) if keyVal != defaultVal { return keyVal } @@ -50,7 +47,7 @@ func (config *ViperConfigKey) Get() string { // Check aliases - if any differs from default, use it (environment variable takes precedence) for _, alias := range config.GetAliases() { if viper.IsSet(alias) { - aliasVal := viper.GetString(alias) + aliasVal := keyGetter(alias) if aliasVal != defaultVal { return aliasVal } @@ -62,32 +59,23 @@ func (config *ViperConfigKey) Get() string { } // Should never be called before config is initialized using config.Configure() -func (config *ViperConfigKey) GetDuration() time.Duration { - defaultVal := time.Duration(0) +func (config *ViperConfigKey) Get() string { + defaultVal := "" if val, ok := config.GetDefault().(string); ok { - defaultVal, _ = time.ParseDuration(val) + defaultVal = val } - // Check main key - if it differs from default, use it (environment variable takes precedence) - if viper.IsSet(config.Key) { - keyVal := viper.GetDuration(config.Key) - if keyVal != defaultVal { - return keyVal - } - } + return getWithFallback(config, defaultVal, viper.GetString) +} - // Check aliases - if any differs from default, use it (environment variable takes precedence) - for _, alias := range config.GetAliases() { - if viper.IsSet(alias) { - aliasVal := viper.GetDuration(alias) - if aliasVal != defaultVal { - return aliasVal - } - } +// Should never be called before config is initialized using config.Configure() +func (config *ViperConfigKey) GetDuration() time.Duration { + defaultVal := time.Duration(0) + if val, ok := config.GetDefault().(string); ok { + defaultVal, _ = time.ParseDuration(val) } - // Return default value - return defaultVal + return getWithFallback(config, defaultVal, viper.GetDuration) } // Should never be called before config is initialized using config.Configure() @@ -97,26 +85,7 @@ func (config *ViperConfigKey) GetBool() bool { defaultVal = val } - // Check main key - if it differs from default, use it (environment variable takes precedence) - if viper.IsSet(config.Key) { - keyVal := viper.GetBool(config.Key) - if keyVal != defaultVal { - return keyVal - } - } - - // Check aliases - if any differs from default, use it (environment variable takes precedence) - for _, alias := range config.GetAliases() { - if viper.IsSet(alias) { - aliasVal := viper.GetBool(alias) - if aliasVal != defaultVal { - return aliasVal - } - } - } - - // Return default value - return defaultVal + return getWithFallback(config, defaultVal, viper.GetBool) } // Should never be called before config is initialized using config.Configure() @@ -126,24 +95,5 @@ func (config *ViperConfigKey) GetInt() int { defaultVal = val } - // Check main key - if it differs from default, use it (environment variable takes precedence) - if viper.IsSet(config.Key) { - keyVal := viper.GetInt(config.Key) - if keyVal != defaultVal { - return keyVal - } - } - - // Check aliases - if any differs from default, use it (environment variable takes precedence) - for _, alias := range config.GetAliases() { - if viper.IsSet(alias) { - aliasVal := viper.GetInt(alias) - if aliasVal != defaultVal { - return aliasVal - } - } - } - - // Return default value - return defaultVal + return getWithFallback(config, defaultVal, viper.GetInt) } From fb75746d28d0d376bbbd67dc956048e0f4735d35 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 5 Jun 2025 16:26:38 +0000 Subject: [PATCH 5/5] Fix duration parsing consistency and remove accidental cert file Co-authored-by: lucasoares <10624972+lucasoares@users.noreply.github.com> --- internal/config/viper_config.go | 24 ++++++++++++++++++++++-- internal/service/cert/.gitignore | 3 ++- internal/service/cert/ca-cert.srl | 1 - 3 files changed, 24 insertions(+), 4 deletions(-) delete mode 100644 internal/service/cert/ca-cert.srl diff --git a/internal/config/viper_config.go b/internal/config/viper_config.go index 8355014..899dc4c 100644 --- a/internal/config/viper_config.go +++ b/internal/config/viper_config.go @@ -72,10 +72,30 @@ func (config *ViperConfigKey) Get() string { func (config *ViperConfigKey) GetDuration() time.Duration { defaultVal := time.Duration(0) if val, ok := config.GetDefault().(string); ok { - defaultVal, _ = time.ParseDuration(val) + parsed, err := time.ParseDuration(val) + if err == nil { + defaultVal = parsed + } + } + + // Use a custom getter that ensures consistent parsing behavior + getDuration := func(key string) time.Duration { + if viper.IsSet(key) { + // Try viper's built-in parsing first + if duration := viper.GetDuration(key); duration != 0 { + return duration + } + // Fall back to manual parsing if viper returns 0 but key is set + if str := viper.GetString(key); str != "" { + if parsed, err := time.ParseDuration(str); err == nil { + return parsed + } + } + } + return 0 } - return getWithFallback(config, defaultVal, viper.GetDuration) + return getWithFallback(config, defaultVal, getDuration) } // Should never be called before config is initialized using config.Configure() diff --git a/internal/service/cert/.gitignore b/internal/service/cert/.gitignore index 612424a..35697cb 100644 --- a/internal/service/cert/.gitignore +++ b/internal/service/cert/.gitignore @@ -1 +1,2 @@ -*.pem \ No newline at end of file +*.pem +*.srl \ No newline at end of file diff --git a/internal/service/cert/ca-cert.srl b/internal/service/cert/ca-cert.srl deleted file mode 100644 index 48b28e0..0000000 --- a/internal/service/cert/ca-cert.srl +++ /dev/null @@ -1 +0,0 @@ -05327763CD7DDF3F33B47C48B2116EE36472D01E