pkg/common: Update types

Signed-off-by: yxxhero <aiopsclub@163.com>

pkg/common/README.md

@@ -0,0 +1,108 @@
+# Common Package
+
+## Overview
+
+The `pkg/common` package provides common utilities, types, and interfaces used throughout the Helmfile application. This package contains shared functionality that doesn't belong to any specific domain but is used by multiple components.
+
+## Current File Structure
+
+```
+pkg/common/
+├── README.md                    # This documentation file
+├── bool_flag.go                 # Boolean flag implementation
+├── bool_flag_test.go            # Tests for boolean flag
+├── string_flag.go               # String flag implementation
+├── string_flag_test.go          # Tests for string flag
+├── string_array_flag.go         # String array flag implementation
+├── string_array_flag_test.go    # Tests for array flag
+├── map_flag.go                  # Map flag implementation
+├── map_flag_test.go             # Tests for map flag
+```
+
+## Planned Future Implementations
+
+The following files are planned for future development:
+```
+├── constants.go          # Common constants used throughout the application
+├── errors.go             # Common error types and error handling utilities
+├── errors_test.go        # Tests for error utilities
+├── logging.go            # Logging utilities and interfaces
+├── logging_test.go       # Tests for logging utilities
+├── types.go              # Common type definitions
+├── types_test.go         # Tests for common types
+├── utils.go              # General utility functions
+└── utils_test.go         # Tests for utility functions
+```
+
+## Components
+
+### Currently Implemented
+
+#### Flag Types
+- **BoolFlag**: A boolean flag implementation with value tracking
+- **StringFlag**: A string flag implementation with value tracking
+- **StringArrayFlag**: A string array flag implementation with value tracking
+- **MapFlag**: A map flag implementation with value tracking
+
+### Planned for Future Implementation
+
+#### Utilities
+- **Constants**: Common constants used throughout the application
+- **Error Handling**: Standardized error types and handling
+- **Logging**: Consistent logging interfaces and utilities
+- **Types**: Common type definitions
+- **Utilities**: General utility functions
+
+## Key Features
+
+### Currently Available
+- **Flag Implementations**: Reusable flag implementations with value tracking
+
+### Planned Features
+- **Error Handling**: Standardized error types and handling
+- **Logging**: Consistent logging interfaces and utilities
+- **Type Safety**: Common type definitions for consistent usage
+
+## Usage
+
+### Flag Usage
+
+```go
+// Create a boolean flag
+includeFlag := common.NewBoolFlag(false)
+
+// Set the flag value
+includeFlag.Set(true)
+
+// Get the flag value
+value := includeFlag.Value()
+```
+
+### Future Error Handling (Planned)
+
+```go
+// Create a common error
+err := common.NewError("operation failed")
+
+// Check if an error is of a specific type
+if common.IsNotFoundError(err) {
+    // Handle not found error
+}
+```
+
+### Future Logging (Planned)
+
+```go
+// Create a logger
+logger := common.NewLogger()
+
+// Log messages
+logger.Info("Operation started")
+logger.Error("Operation failed: %v", err)
+```
+
+## Related Packages
+
+- `pkg/config`: Uses common flag types for option configuration
+- `pkg/flags`: Integrates with common flag types for flag handling
+- `pkg/app`: Uses common utilities for application functionality

pkg/common/map_flag.go

@@ -0,0 +1,69 @@
+package common
+
+// MapFlag represents a string map flag that tracks whether it was explicitly set
+type MapFlag interface {
+	// Value returns a copy of the current map value
+	Value() map[string]string
+
+	// WasExplicitlySet returns whether the flag was explicitly set
+	WasExplicitlySet() bool
+
+	// Set sets the value and marks the flag as explicitly set
+	Set(value map[string]string)
+
+	// SetKey sets a specific key-value pair and marks the flag as explicitly set
+	SetKey(key, value string)
+}
+
+// mapFlag is the implementation of MapFlag
+type mapFlag struct {
+	value            map[string]string
+	wasExplicitlySet bool
+}
+
+// NewMapFlag creates a new MapFlag with default values
+func NewMapFlag(defaultValue map[string]string) MapFlag {
+	// Create a copy of the default value to avoid modifying the original
+	valueCopy := make(map[string]string, len(defaultValue))
+	for k, v := range defaultValue {
+		valueCopy[k] = v
+	}
+
+	return &mapFlag{
+		value:            valueCopy,
+		wasExplicitlySet: false,
+	}
+}
+
+// Value returns a copy of the current map value
+func (mf *mapFlag) Value() map[string]string {
+	// Create a copy to prevent external modification of internal state
+	result := make(map[string]string, len(mf.value))
+	for k, v := range mf.value {
+		result[k] = v
+	}
+	return result
+}
+
+// WasExplicitlySet returns whether the flag was explicitly set
+func (mf *mapFlag) WasExplicitlySet() bool {
+	return mf.wasExplicitlySet
+}
+
+// Set sets the value and marks the flag as explicitly set
+func (mf *mapFlag) Set(value map[string]string) {
+	// Create a copy of the value to avoid modifying the original
+	valueCopy := make(map[string]string, len(value))
+	for k, v := range value {
+		valueCopy[k] = v
+	}
+
+	mf.value = valueCopy
+	mf.wasExplicitlySet = true
+}
+
+// SetKey sets a specific key-value pair and marks the flag as explicitly set
+func (mf *mapFlag) SetKey(key, value string) {
+	mf.value[key] = value
+	mf.wasExplicitlySet = true
+}

pkg/common/map_flag_test.go

@@ -0,0 +1,159 @@
+package common
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestNewMapFlag(t *testing.T) {
+	tests := []struct {
+		name         string
+		defaultValue map[string]string
+		expected     map[string]string
+	}{
+		{
+			name:         "default empty",
+			defaultValue: map[string]string{},
+			expected:     map[string]string{},
+		},
+		{
+			name:         "default with values",
+			defaultValue: map[string]string{"key1": "value1", "key2": "value2"},
+			expected:     map[string]string{"key1": "value1", "key2": "value2"},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			flag := NewMapFlag(tt.defaultValue)
+
+			// Check initial state
+			assert.Equal(t, tt.expected, flag.Value(), "Value should match default")
+			assert.False(t, flag.WasExplicitlySet(), "New flag should not be marked as explicitly set")
+
+			// Ensure the default value is copied, not referenced
+			if len(tt.defaultValue) > 0 {
+				original := make(map[string]string)
+				for k, v := range tt.defaultValue {
+					original[k] = v
+				}
+
+				// Modify the original
+				for k := range tt.defaultValue {
+					tt.defaultValue[k] = "modified"
+					break
+				}
+
+				// Flag value should remain unchanged
+				assert.Equal(t, original, flag.Value(), "Flag value should be a copy, not a reference")
+			}
+		})
+	}
+}
+
+func TestMapFlag_ValueImmutability(t *testing.T) {
+	// Test that modifying the returned value doesn't affect the internal state
+	flag := NewMapFlag(map[string]string{"key1": "value1", "key2": "value2"})
+
+	// Get the value and modify it
+	value := flag.Value()
+	value["key1"] = "modified"
+
+	// Check that the flag's internal state is unchanged
+	expected := map[string]string{"key1": "value1", "key2": "value2"}
+	assert.Equal(t, expected, flag.Value(), "Modifying the returned value should not affect the flag's internal state")
+}
+
+func TestMapFlag_Set(t *testing.T) {
+	tests := []struct {
+		name         string
+		defaultValue map[string]string
+		setValue     map[string]string
+		expected     map[string]string
+	}{
+		{
+			name:         "default empty, set values",
+			defaultValue: map[string]string{},
+			setValue:     map[string]string{"key1": "value1", "key2": "value2"},
+			expected:     map[string]string{"key1": "value1", "key2": "value2"},
+		},
+		{
+			name:         "default with values, set new values",
+			defaultValue: map[string]string{"key1": "value1", "key2": "value2"},
+			setValue:     map[string]string{"key3": "value3", "key4": "value4"},
+			expected:     map[string]string{"key3": "value3", "key4": "value4"},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			flag := NewMapFlag(tt.defaultValue)
+
+			// Set the value
+			flag.Set(tt.setValue)
+
+			// Check state after setting
+			assert.Equal(t, tt.expected, flag.Value(), "Value should match set value")
+			assert.True(t, flag.WasExplicitlySet(), "Flag should be marked as explicitly set")
+
+			// Ensure the set value is copied, not referenced
+			if len(tt.setValue) > 0 {
+				original := make(map[string]string)
+				for k, v := range tt.setValue {
+					original[k] = v
+				}
+
+				// Modify the original
+				for k := range tt.setValue {
+					tt.setValue[k] = "modified"
+					break
+				}
+
+				// Flag value should remain unchanged
+				assert.Equal(t, original, flag.Value(), "Flag value should be a copy, not a reference")
+			}
+		})
+	}
+}
+
+func TestMapFlag_SetKey(t *testing.T) {
+	flag := NewMapFlag(map[string]string{"existing": "value"})
+
+	// Initial state
+	assert.Equal(t, map[string]string{"existing": "value"}, flag.Value())
+	assert.False(t, flag.WasExplicitlySet())
+
+	// Set a new key
+	flag.SetKey("new", "value")
+	assert.Equal(t, map[string]string{"existing": "value", "new": "value"}, flag.Value())
+	assert.True(t, flag.WasExplicitlySet())
+
+	// Override an existing key
+	flag.SetKey("existing", "updated")
+	assert.Equal(t, map[string]string{"existing": "updated", "new": "value"}, flag.Value())
+	assert.True(t, flag.WasExplicitlySet())
+}
+
+func TestMapFlag_MultipleSet(t *testing.T) {
+	flag := NewMapFlag(map[string]string{"initial": "value"})
+
+	// Initial state
+	assert.Equal(t, map[string]string{"initial": "value"}, flag.Value())
+	assert.False(t, flag.WasExplicitlySet())
+
+	// First set
+	flag.Set(map[string]string{"first": "set"})
+	assert.Equal(t, map[string]string{"first": "set"}, flag.Value())
+	assert.True(t, flag.WasExplicitlySet())
+
+	// Second set
+	flag.Set(map[string]string{"second": "set"})
+	assert.Equal(t, map[string]string{"second": "set"}, flag.Value())
+	assert.True(t, flag.WasExplicitlySet(), "Flag should remain explicitly set")
+}
+
+func TestMapFlag_Implementation(t *testing.T) {
+	// Test that mapFlag properly implements MapFlag interface
+	var _ MapFlag = &mapFlag{}
+}

pkg/common/string_array_flag.go

@@ -1,56 +1,63 @@
 package common
 
-// For array/slice flags
+// StringArrayFlag represents a string array flag that tracks whether it was explicitly set
 type StringArrayFlag interface {
-	Values() []string
+	// Value returns a copy of the current string array value
+	Value() []string
+
+	// WasExplicitlySet returns whether the flag was explicitly set
 	WasExplicitlySet() bool
-	Add(value string)
-	Set(values []string)
+
+	// Set sets the value and marks the flag as explicitly set
+	Set(value []string)
+
+	// Append adds a value to the array and marks the flag as explicitly set
+	Append(value string)
 }
 
+// stringArrayFlag is the implementation of ArrayFlag
 type stringArrayFlag struct {
-	values           []string
+	value            []string
 	wasExplicitlySet bool
 }
 
-// NewStringArrayFlag creates a new StringArrayFlag with the given default values
-// Create a defensive copy of the default values to prevent external modifications
-// and to ensure that the original values remain unchanged.
-func NewStringArrayFlag(defaultValues []string) StringArrayFlag {
-	valuesCopy := make([]string, len(defaultValues))
-	copy(valuesCopy, defaultValues)
+// NewStringArrayFlag creates a new ArrayFlag with default values
+func NewStringArrayFlag(defaultValue []string) StringArrayFlag {
+	// Create a copy of the default value to avoid modifying the original
+	valueCopy := make([]string, len(defaultValue))
+	copy(valueCopy, defaultValue)
 
 	return &stringArrayFlag{
-		values:           valuesCopy,
+		value:            valueCopy,
 		wasExplicitlySet: false,
 	}
 }
 
-// Values returns the values of the flag
-// It returns a copy of the values to prevent external modifications
-// and to ensure that the original values remain unchanged.
-// This is important for flags that may be modified by the user
-// or other parts of the program.
-func (f *stringArrayFlag) Values() []string {
-	// Return a copy to prevent external modifications
-	valuesCopy := make([]string, len(f.values))
-	copy(valuesCopy, f.values)
-	return valuesCopy
+// Value returns a copy of the current string array value
+func (af *stringArrayFlag) Value() []string {
+	// Create a copy to prevent external modification of internal state
+	result := make([]string, len(af.value))
+	copy(result, af.value)
+	return result
 }
 
 // WasExplicitlySet returns whether the flag was explicitly set
-func (f *stringArrayFlag) WasExplicitlySet() bool {
-	return f.wasExplicitlySet
+func (af *stringArrayFlag) WasExplicitlySet() bool {
+	return af.wasExplicitlySet
 }
 
-// Set sets the values and marks the flag as explicitly set
-func (f *stringArrayFlag) Set(values []string) {
-	f.values = values
-	f.wasExplicitlySet = true
+// Set sets the value and marks the flag as explicitly set
+func (af *stringArrayFlag) Set(value []string) {
+	// Create a copy of the value to avoid modifying the original
+	valueCopy := make([]string, len(value))
+	copy(valueCopy, value)
+
+	af.value = valueCopy
+	af.wasExplicitlySet = true
 }
 
-// Add sets the value and marks the flag as explicitly set
-func (f *stringArrayFlag) Add(value string) {
-	f.values = append(f.values, value)
-	f.wasExplicitlySet = true
+// Append adds a value to the array and marks the flag as explicitly set
+func (af *stringArrayFlag) Append(value string) {
+	af.value = append(af.value, value)
+	af.wasExplicitlySet = true
 }

pkg/common/string_array_flag_test.go

@@ -13,19 +13,14 @@ func TestNewStringArrayFlag(t *testing.T) {
 		expected     []string
 	}{
 		{
-			name:         "nil default",
-			defaultValue: nil,
-			expected:     []string{},
-		},
-		{
-			name:         "empty default",
+			name:         "default empty",
 			defaultValue: []string{},
 			expected:     []string{},
 		},
 		{
-			name:         "non-empty default",
-			defaultValue: []string{"value1", "value2"},
-			expected:     []string{"value1", "value2"},
+			name:         "default with values",
+			defaultValue: []string{"one", "two"},
+			expected:     []string{"one", "two"},
 		},
 	}
 
@@ -34,8 +29,20 @@ func TestNewStringArrayFlag(t *testing.T) {
 			flag := NewStringArrayFlag(tt.defaultValue)
 
 			// Check initial state
-			assert.Equal(t, tt.expected, flag.Values(), "Values should match default")
+			assert.Equal(t, tt.expected, flag.Value(), "Value should match default")
 			assert.False(t, flag.WasExplicitlySet(), "New flag should not be marked as explicitly set")
+
+			// Ensure the default value is copied, not referenced
+			if len(tt.defaultValue) > 0 {
+				original := make([]string, len(tt.defaultValue))
+				copy(original, tt.defaultValue)
+
+				// Modify the original
+				tt.defaultValue[0] = "modified"
+
+				// Flag value should remain unchanged
+				assert.Equal(t, original, flag.Value(), "Flag value should be a copy, not a reference")
+			}
 		})
 	}
 }
@@ -48,22 +55,16 @@ func TestStringArrayFlag_Set(t *testing.T) {
 		expected     []string
 	}{
 		{
-			name:         "nil default, set values",
-			defaultValue: nil,
-			setValue:     []string{"new1", "new2"},
-			expected:     []string{"new1", "new2"},
+			name:         "default empty, set values",
+			defaultValue: []string{},
+			setValue:     []string{"one", "two"},
+			expected:     []string{"one", "two"},
 		},
 		{
-			name:         "non-empty default, set empty",
-			defaultValue: []string{"default1", "default2"},
-			setValue:     []string{},
-			expected:     []string{},
-		},
-		{
-			name:         "non-empty default, set new values",
-			defaultValue: []string{"default1", "default2"},
-			setValue:     []string{"new1", "new2"},
-			expected:     []string{"new1", "new2"},
+			name:         "default with values, set new values",
+			defaultValue: []string{"one", "two"},
+			setValue:     []string{"three", "four"},
+			expected:     []string{"three", "four"},
 		},
 	}
 
@@ -71,125 +72,77 @@ func TestStringArrayFlag_Set(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			flag := NewStringArrayFlag(tt.defaultValue)
 
-			// Set the values
+			// Set the value
 			flag.Set(tt.setValue)
 
 			// Check state after setting
-			assert.Equal(t, tt.expected, flag.Values(), "Values should match set values")
+			assert.Equal(t, tt.expected, flag.Value(), "Value should match set value")
 			assert.True(t, flag.WasExplicitlySet(), "Flag should be marked as explicitly set")
+
+			// Ensure the set value is copied, not referenced
+			if len(tt.setValue) > 0 {
+				original := make([]string, len(tt.setValue))
+				copy(original, tt.setValue)
+
+				// Modify the original
+				tt.setValue[0] = "modified"
+
+				// Flag value should remain unchanged
+				assert.Equal(t, original, flag.Value(), "Flag value should be a copy, not a reference")
+			}
 		})
 	}
 }
 
-func TestStringArrayFlag_Add(t *testing.T) {
-	tests := []struct {
-		name         string
-		defaultValue []string
-		addValue     string
-		expected     []string
-	}{
-		{
-			name:         "nil default, add value",
-			defaultValue: nil,
-			addValue:     "new",
-			expected:     []string{"new"},
-		},
-		{
-			name:         "empty default, add value",
-			defaultValue: []string{},
-			addValue:     "new",
-			expected:     []string{"new"},
-		},
-		{
-			name:         "non-empty default, add value",
-			defaultValue: []string{"default1", "default2"},
-			addValue:     "new",
-			expected:     []string{"default1", "default2", "new"},
-		},
-	}
+func TestStringArrayFlag_ValueImmutability(t *testing.T) {
+	// Test that modifying the returned value doesn't affect the internal state
+	flag := NewStringArrayFlag([]string{"one", "two"})
 
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			flag := NewStringArrayFlag(tt.defaultValue)
+	// Get the value and modify it
+	value := flag.Value()
+	value[0] = "modified"
 
-			// Add the value
-			flag.Add(tt.addValue)
-
-			// Check state after adding
-			assert.Equal(t, tt.expected, flag.Values(), "Values should include added value")
-			assert.True(t, flag.WasExplicitlySet(), "Flag should be marked as explicitly set")
-		})
-	}
+	// Check that the flag's internal state is unchanged
+	assert.Equal(t, []string{"one", "two"}, flag.Value(), "Modifying the returned value should not affect the flag's internal state")
 }
 
-func TestStringArrayFlag_MultipleOperations(t *testing.T) {
+func TestStringArrayFlag_Append(t *testing.T) {
+	flag := NewStringArrayFlag([]string{"one"})
+
+	// Initial state
+	assert.Equal(t, []string{"one"}, flag.Value())
+	assert.False(t, flag.WasExplicitlySet())
+
+	// Append a value
+	flag.Append("two")
+	assert.Equal(t, []string{"one", "two"}, flag.Value())
+	assert.True(t, flag.WasExplicitlySet())
+
+	// Append another value
+	flag.Append("three")
+	assert.Equal(t, []string{"one", "two", "three"}, flag.Value())
+	assert.True(t, flag.WasExplicitlySet())
+}
+
+func TestStringArrayFlag_MultipleSet(t *testing.T) {
 	flag := NewStringArrayFlag([]string{"initial"})
 
 	// Initial state
-	assert.Equal(t, []string{"initial"}, flag.Values())
+	assert.Equal(t, []string{"initial"}, flag.Value())
 	assert.False(t, flag.WasExplicitlySet())
 
-	// Add a value
-	flag.Add("added")
-	assert.Equal(t, []string{"initial", "added"}, flag.Values())
+	// First set
+	flag.Set([]string{"first", "set"})
+	assert.Equal(t, []string{"first", "set"}, flag.Value())
 	assert.True(t, flag.WasExplicitlySet())
 
-	// Set completely new values
-	flag.Set([]string{"new1", "new2"})
-	assert.Equal(t, []string{"new1", "new2"}, flag.Values())
+	// Second set
+	flag.Set([]string{"second", "set"})
+	assert.Equal(t, []string{"second", "set"}, flag.Value())
 	assert.True(t, flag.WasExplicitlySet(), "Flag should remain explicitly set")
-
-	// Add another value after Set
-	flag.Add("added2")
-	assert.Equal(t, []string{"new1", "new2", "added2"}, flag.Values())
-	assert.True(t, flag.WasExplicitlySet())
 }
 
-func TestStringArrayFlag_Implementation(t *testing.T) {
+func TestArrayStringFlag_Implementation(t *testing.T) {
 	// Test that stringArrayFlag properly implements StringArrayFlag interface
 	var _ StringArrayFlag = &stringArrayFlag{}
 }
-
-func TestStringArrayFlag_DefensiveCopy(t *testing.T) {
-	// Test that modifying the original slice doesn't affect the flag
-	original := []string{"value1", "value2"}
-	flag := NewStringArrayFlag(original)
-
-	// Verify initial state
-	assert.Equal(t, []string{"value1", "value2"}, flag.Values())
-
-	// Modify the original slice - should NOT affect the flag's internal state
-	// because NewStringArrayFlag creates a defensive copy
-	original[0] = "modified"
-	original = append(original, "added")
-
-	// Flag values should remain unchanged
-	assert.Equal(t, []string{"value1", "value2"}, flag.Values())
-
-	// Test that modifying the returned slice doesn't affect the flag
-	values := flag.Values()
-	values[0] = "modified"
-	values = append(values, "added")
-
-	// Flag values should remain unchanged because Values() returns a copy
-	assert.Equal(t, []string{"value1", "value2"}, flag.Values())
-}
-
-func TestStringArrayFlag_SetDefensiveCopy(t *testing.T) {
-	// Test that Set doesn't create a defensive copy (current implementation)
-	flag := NewStringArrayFlag([]string{})
-
-	// Create a slice to set
-	setValues := []string{"value1", "value2"}
-	flag.Set(setValues)
-
-	// Verify state after setting
-	assert.Equal(t, []string{"value1", "value2"}, flag.Values())
-
-	// Modify the original slice - this WILL affect the flag's internal state
-	// because Set doesn't create a defensive copy in the current implementation
-	setValues[0] = "modified"
-
-	// Flag values will reflect the modification
-	assert.Equal(t, []string{"modified", "value2"}, flag.Values())
-}