Config client

[jcrichlake]

Description

Adding a config parsing utility that will allow us to receive the texts of any config file and load it into a struct.

Issue

Issue

Checklist

• I have added tests to cover my changes
• I have added logging where useful (with appropriate log level)
• I have updated the documentation accordingly

Note: You may remove items that are not applicable

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Error Handling The error handling in populatePartnerSettings might suppress the underlying JSON unmarshalling error which could make debugging difficult. Consider reworking how errors are handled to provide clearer insights into issues. Logging Details The logging done in the populatePartnerSettings method might not provide enough context in the logs for debugging purposes, especially in production environments. Consider including more contextual information.

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Possible issue	Improve error handling and messaging for JSON unmarshalling failures Handle the case where json.Unmarshal might fail due to invalid JSON format by adding a more descriptive error message or a recovery mechanism. src/utils/config.go [37-41] err := json.Unmarshal(jsonData, &partnerSettings) if err != nil { - slog.Error("Unable unmarshall to partner settings", slog.Any(ErrorKey, err)) + slog.Error("Failed to unmarshal JSON data to partner settings", slog.Any(ErrorKey, err)) return err } Suggestion importance[1-10]: 5 Why: The suggestion improves the error message clarity, which is beneficial for debugging and understanding the context of the error. It's a moderate improvement as it enhances the readability and usefulness of the log messages.	5
	Add validation for PartnerSettings struct fields to ensure they are not null or default Ensure that the PartnerSettings struct fields are properly validated for null or default values to prevent any runtime errors or misconfigurations. src/utils/config.go [17-22] type PartnerSettings struct { DisplayName string `json:"displayName"` // full name if we need pretty names IsActive bool `json:"isActive"` IsExternalSftpConnection bool `json:"isExternalSftpConnection"` HasZipPassword bool `json:"hasZipPassword"` DefaultEncoding string `json:"defaultEncoding"` + // Add validation checks here } Suggestion importance[1-10]: 3 Why: The suggestion to add validation checks is valid but not critical as the fields are already being validated indirectly through JSON unmarshalling and specific encoding checks. The suggestion does not directly modify the code but prompts for additional validation which might be implemented elsewhere.	3
General	Ensure DisplayName is neither empty nor contains only whitespace Consider adding a check for empty or whitespace-only strings for DisplayName in PartnerSettings to ensure data integrity. src/utils/config.go [18] DisplayName string `json:"displayName"` // full name if we need pretty names +// Add check for empty or whitespace-only strings Suggestion importance[1-10]: 4 Why: Adding a check for empty or whitespace-only strings in DisplayName is a good practice to ensure data integrity. However, the impact is moderate as it depends on the broader context of how DisplayName is used throughout the application.	4
	Add success logging to enhance traceability of operations Add logging for successful operations within the populatePartnerSettings function to provide better traceability and debugging capabilities. src/utils/config.go [50-51] config.partnerSettings = partnerSettings +slog.Info("Successfully populated partner settings", slog.String("Partner ID", config.PartnerId)) return nil Suggestion importance[1-10]: 4 Why: Adding success logging is beneficial for monitoring and debugging, especially in production environments. It provides better traceability of the function's successful operations. The impact is moderate as it primarily aids in debugging and operational transparency.	4

[halprin]

Mostly some clarifying questions.

[pluckyswan]

I believe two or so tests do not follow the testing naming convention set by the ADR, which should follow either Test_TheMethodBeingTested_TheCoveredScenario_TheExpectedResult or Test_TheMethodBeingTested_TheExpectedResult

[jcrichlake]

This is updated

[pluckyswan]

Is validateDefaultEncoding missing a happy path or is populateConfig covering it enough that we don't need it

[jcrichlake]

The latter

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud