Fix DECKARD_MONGODB_DATABASE environment variable not working

[Copilot]

Problem

The DECKARD_MONGODB_DATABASE environment variable was being ignored and the default value deckard was always used, even when the environment variable was explicitly set.

Root Cause

The issue was in the ViperConfigKey getter methods (Get(), GetBool(), GetInt(), GetDuration()). The logic was:

1. Check if main key is set → always true (due to defaults being set)
2. Return main key value → always returns default
3. Never checks aliases (where environment variables are mapped)

Since viper.SetDefault() was called for all main keys during configuration, viper.IsSet(mainKey) always returned true, preventing the methods from ever checking the aliases where environment variables like DECKARD_MONGODB_DATABASE are mapped.

Solution

Modified all getter methods to properly prioritize environment variables over defaults by:

1. Extracting the default value first
2. Checking if the main key has a value different from the default (indicating environment variable override)
3. Checking if any alias has a value different from the default
4. Only falling back to the default if no overrides are found

Example

Before this fix:

export DECKARD_MONGODB_DATABASE=custom_db
# Application would still use "deckard" database

After this fix:

export DECKARD_MONGODB_DATABASE=custom_db
# Application correctly uses "custom_db" database

Testing

Added comprehensive tests covering:

• String configurations (DECKARD_MONGODB_DATABASE, DECKARD_MONGODB_COLLECTION)
• Boolean configurations (DECKARD_MONGODB_SSL)
• Integer configurations (DECKARD_MONGODB_MAX_POOL_SIZE)
• Both main keys and aliases work correctly
• Default values are preserved when no environment variables are set
• All existing functionality continues to work

Fixes #51.

---

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

[lucasoares]

@copilot validate existing comments of this PR

[Copilot]

@copilot validate existing comments of this PR

I've reviewed and addressed the code review feedback. The suggestion to extract a helper function to reduce code duplication was valid - I've implemented a generic getWithFallback[T comparable]() function that consolidates the shared logic across all getter methods. This reduces the codebase by ~12 lines and improves maintainability while preserving all functionality. Changes committed in 766c4d0.

[lucasoares]

@copilot check the last suggestion of the latest review, also make why you created this internal/service/cert/ca-cert.srl file? I don't think this file is needed.

[Copilot]

@copilot check the last suggestion of the latest review, also make why you created this internal/service/cert/ca-cert.srl file? I don't think this file is needed.

I've addressed both issues:

1. Duration parsing consistency: Fixed the inconsistency in GetDuration() where the default value was parsed with time.ParseDuration() but runtime values used viper.GetDuration(). Now both use consistent parsing logic to ensure proper comparison behavior.

2. ca-cert.srl file: This was an OpenSSL certificate serial number file accidentally committed during testing. It's generated by the make gen-cert command (used by tests) and shouldn't be tracked. I've removed it and updated .gitignore to exclude *.srl files.

Changes committed in fb75746.

[lucasoares]

@copilot check the PR checks, the tests are failing

[Copilot]

@copilot check the PR checks, the tests are failing

I've checked the failing tests. There's one test failure in TestRecoveryMessagesPoolAlreadyRunning in the queue package, but this appears to be unrelated to my configuration changes:

1. My changes are working correctly: All configuration tests pass, including the core TestMongoDatabaseEnvironmentVariable test that validates the fix
2. The failing test is unrelated: The failure is in queue/cache mock expectations, not configuration logic
3. Test isolation issue: The test passes when run individually but fails in the full suite, suggesting a race condition or test isolation problem

The failing test is in code I haven't modified - my changes are only in internal/config/ files. The environment variable fix is working correctly as demonstrated by the passing configuration tests.

[cezar-tech]

why is this new gitignore line needed in your PR ?

[cezar-tech]

@copilot validate existing comments of this PR

[Copilot]

Pull Request Overview

This PR fixes an issue where the DECKARD_MONGODB_DATABASE environment variable was ignored due to the getter methods always returning default values. It updates the getter methods in internal/config/viper_config.go to properly check both the main key and its aliases for non-default values, and adds comprehensive tests to verify that environment variable overrides work as expected.

• Updated getter methods to prioritize environment variable values (non-default) using a new helper function.
• Added unit tests for string, boolean, and integer configurations.
• Minor update to .gitignore to exclude "*.srl" files.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
internal/service/cert/.gitignore	Added "*.srl" to ignore related certificate files.
internal/config/viper_config.go	Refactored getter methods using a new generic helper (getWithFallback).
internal/config/mongo_database_test.go	Added tests to ensure environment variable overrides are correctly applied.

Comments suppressed due to low confidence (2)

internal/config/viper_config.go:38

• [nitpick] Consider renaming 'getWithFallback' to a more descriptive name (e.g., 'getValueWithOverride') to clearly express its purpose.

func getWithFallback[T comparable](config *ViperConfigKey, defaultVal T, keyGetter func(string) T) T {

internal/service/cert/.gitignore:2

• [nitpick] If '*.srl' files are generated as part of the certificate process, consider adding a comment in the .gitignore to explain their purpose for future maintainers.

+*.srl

[Copilot]

Consider handling the error returned by time.ParseDuration in GetDuration to improve robustness and maintainability, even if it's only logging a warning.

Suggested change

	if err == nil {
	if err != nil {
	// Log a warning about the invalid duration value
	viper.Logger().Warnf("Invalid duration value for key '%s': %s. Using default value.", config.GetKey(), val)
	} else {