-
Notifications
You must be signed in to change notification settings - Fork 3
Description
🔍 Duplicate Code Pattern: Parallel Config Struct Hierarchies
Part of duplicate code analysis: #581
Summary
The configuration system maintains two parallel struct hierarchies: one for TOML files (config_core.go) and one for JSON stdin (config_stdin.go). These structs define nearly identical fields with different JSON/TOML tags and naming conventions, creating a significant maintenance burden.
Duplication Details
Pattern: Parallel Struct Definitions
- Severity: High
- Occurrences: 3 struct pairs (6 total structs)
- Locations:
internal/config/config_core.go(lines 23-84):Config(line 23)GatewayConfig(line 39)ServerConfig(line 60)
internal/config/config_stdin.go(lines 14-67):StdinConfig(line 14)StdinGatewayConfig(line 27)StdinServerConfig(line 37)
Code Sample: Gateway Config Duplication
TOML Version (config_core.go:39-57):
type GatewayConfig struct {
Port int `toml:"port" json:"port,omitempty"`
APIKey string `toml:"api_key" json:"api_key,omitempty"`
Domain string `toml:"domain" json:"domain,omitempty"`
StartupTimeout int `toml:"startup_timeout" json:"startup_timeout,omitempty"`
ToolTimeout int `toml:"tool_timeout" json:"tool_timeout,omitempty"`
PayloadDir string `toml:"payload_dir" json:"payload_dir,omitempty"`
}JSON Stdin Version (config_stdin.go:27-34):
type StdinGatewayConfig struct {
Port *int `json:"port,omitempty"`
APIKey string `json:"apiKey,omitempty"` // camelCase
Domain string `json:"domain,omitempty"`
StartupTimeout *int `json:"startupTimeout,omitempty"` // camelCase
ToolTimeout *int `json:"toolTimeout,omitempty"` // camelCase
PayloadDir string `json:"payloadDir,omitempty"` // camelCase
}Key Differences:
- Struct names:
GatewayConfigvsStdinGatewayConfig - JSON field naming: snake_case vs camelCase
- Pointer types:
intvs*int(for optional field detection) - Tags: Both TOML and JSON vs JSON only
Impact Analysis
- Maintainability: Adding a new config field requires updating 6 locations (2 struct definitions + normalization logic)
- Bug Risk: High - fields can easily get out of sync, causing subtle bugs
- Code Bloat: ~100 lines of nearly identical struct definitions
- Testing: Parallel test cases needed for TOML and JSON parsing
- Documentation: Must maintain separate examples for TOML and JSON formats
Real-world scenario:
Adding a new gateway config field like MaxConnections requires:
- Add to
GatewayConfigwith TOML/JSON tags - Add to
StdinGatewayConfigwith JSON tags - Update normalization logic in
normalizeStdinToInternalConfig() - Update validation logic
- Update documentation examples
- Risk: Forgetting any step causes format-specific bugs
Refactoring Recommendations
Option 1: Unified Struct with Dual Tags (Recommended)
Use a single struct with both TOML and JSON tags:
// internal/config/unified_config.go
package config
// Config represents gateway configuration (supports TOML and JSON).
type Config struct {
Servers map[string]*ServerConfig `toml:"servers" json:"mcpServers"` // Different key names
Gateway *GatewayConfig `toml:"gateway" json:"gateway,omitempty"`
EnableDIFC bool `toml:"enable_difc" json:"enable_difc,omitempty"`
SequentialLaunch bool `toml:"sequential_launch" json:"sequential_launch,omitempty"`
}
// GatewayConfig holds global gateway settings.
type GatewayConfig struct {
Port int `toml:"port" json:"port,omitempty"`
APIKey string `toml:"api_key" json:"apiKey,omitempty"` // Dual tags
Domain string `toml:"domain" json:"domain,omitempty"`
StartupTimeout int `toml:"startup_timeout" json:"startupTimeout,omitempty"` // Dual tags
ToolTimeout int `toml:"tool_timeout" json:"toolTimeout,omitempty"` // Dual tags
PayloadDir string `toml:"payload_dir" json:"payloadDir,omitempty"` // Dual tags
}
// ServerConfig represents an individual MCP server configuration.
type ServerConfig struct {
Type string `toml:"type" json:"type,omitempty"`
Command string `toml:"command" json:"command,omitempty"`
Container string `toml:"container" json:"container,omitempty"`
Args []string `toml:"args" json:"args,omitempty"`
Env map[string]string `toml:"env" json:"env,omitempty"`
WorkingDirectory string `toml:"working_directory" json:"workingDirectory,omitempty"` // Dual tags
URL string `toml:"url" json:"url,omitempty"`
Headers map[string]string `toml:"headers" json:"headers,omitempty"`
Tools []string `toml:"tools" json:"tools,omitempty"`
}Handle optional fields with custom unmarshaling:
// For fields that need nil detection in JSON (like *int), use a custom type:
type OptionalInt struct {
Value int
Set bool
}
func (o *OptionalInt) UnmarshalJSON(data []byte) error {
if string(data) == "null" {
o.Set = false
return nil
}
o.Set = true
return json.Unmarshal(data, &o.Value)
}Benefits:
- Eliminates 3 duplicate structs (~50 lines)
- Single source of truth for config schema
- Reduces normalization code complexity
- New fields only added once
- Both formats stay in sync automatically
Challenges:
- Different field name conventions (snake_case vs camelCase) require dual tags
- Optional field handling (JSON
*intvs TOMLint) needs custom types or post-load processing - Requires careful testing of both TOML and JSON parsing
Option 2: Shared Interface with Format-Specific Implementations
Define interfaces and use format-specific implementations:
type ConfigLoader interface {
LoadGatewayConfig() GatewayConfig
LoadServers() map[string]ServerConfig
}
type TOMLConfigLoader struct { /* TOML-specific fields */ }
type JSONConfigLoader struct { /* JSON-specific fields */ }Benefits:
- Clean separation of concerns
- Format-specific validation logic
Drawbacks:
- More complex than Option 1
- Still requires maintaining parallel field definitions
- Adds abstraction overhead
Option 3: Code Generation
Generate struct definitions from a single schema definition:
Benefits:
- Ultimate DRY - single schema source
- Can generate validation, docs, etc.
Drawbacks:
- Adds build complexity
- Overkill for this project size
- Makes code less readable
Recommendation
Implement Option 1 (Unified Struct with Dual Tags) for the following reasons:
- Simplicity: No code generation or complex interfaces
- Maintainability: Single struct definition per concept
- Backwards Compatibility: Both TOML and JSON parsing continue to work
- Testing: Easier to test - one struct, two unmarshalers
Implementation Checklist
- Review approach with team
- Create
internal/config/unified.gowith merged struct definitions - Add dual tags (TOML snake_case + JSON camelCase) to all fields
- Handle optional field detection (replace
*intwith custom type or post-processing) - Update
LoadFromFile()to use unified structs - Update
LoadFromStdin()to use unified structs - Remove old
StdinConfig,StdinGatewayConfig,StdinServerConfigdefinitions - Remove or simplify
normalizeStdinToInternalConfig()function - Update all tests to use unified structs
- Verify both TOML and JSON parsing still work correctly
- Update documentation and examples
Estimated effort: 3-4 hours (struct merge + normalization removal + testing)
Parent Issue
See parent analysis report: #581
Related to #581
AI generated by Duplicate Code Detector