More WIP

backend/internal/appconfig/appconfig_actor.go

@@ -7,7 +7,9 @@ import (
 	"time"
 
 	"github.com/italypaleale/francis/actor"
+
 	"github.com/pocket-id/pocket-id/backend/internal/common"
+	"github.com/pocket-id/pocket-id/backend/internal/dto"
 )
 
 // The AppConfig singleton actor maintains the dynamic configuration for the Pocket ID cluster
@@ -110,3 +112,81 @@ func (a *appConfigActor) Peek(parentCtx context.Context, method string, data act
 	// Return the state
 	return state, nil
 }
+
+func (a *appConfigActor) Invoke(parentCtx context.Context, method string, data actor.Envelope) (any, error) {
+	// Check the method first
+	switch method {
+	case "get", "update", "replace":
+		// All good
+		// Note: we support "get" also via Invoke and not just Peek
+	default:
+		return nil, common.ErrUnsupportedActorMethod{Method: method}
+	}
+
+	// Load the actor state
+	ctx, cancel := context.WithTimeout(parentCtx, 10*time.Second)
+	defer cancel()
+	state, err := a.client.GetState(ctx)
+	if err != nil {
+		return nil, fmt.Errorf("error retrieving actor state: %w", err)
+	}
+
+	switch method {
+	case "get":
+		// If the method is "get", just return the actor state, we're done
+		// This switch case is a no-op
+
+	case "replace":
+		// Replace the entire config
+		// The input data must be a dto.AppConfigUpdateDto
+		payload := dto.AppConfigUpdateDto{}
+		if data == nil {
+			return nil, fmt.Errorf("request body is empty for method 'replace': %w", err)
+		}
+		err = data.Decode(&payload)
+		if err != nil {
+			return nil, fmt.Errorf("request body is not valid for method 'replace': %w", err)
+		}
+
+		// Update the in-memory data
+		// Work on a clone to avoid touching the cached object in case of errors (we'll update after we've committed the state)
+		newState := state.Clone()
+		newState.Replace(payload)
+
+		// Save the updated state
+		ctx, cancel = context.WithTimeout(parentCtx, 10*time.Second)
+		defer cancel()
+		err = a.client.SetState(ctx, state, nil)
+
+		// Update the cached state too
+		*state = *newState
+
+	case "update":
+		// Update the config
+		// The input data must be a map[string]string
+		var payload map[string]string
+		if data == nil {
+			return nil, fmt.Errorf("request body is empty for method 'update': %w", err)
+		}
+		err = data.Decode(&payload)
+		if err != nil {
+			return nil, fmt.Errorf("request body is not valid for method 'update': %w", err)
+		}
+
+		// Update the in-memory data
+		// Work on a clone to avoid touching the cached object in case of errors (we'll update after we've committed the state)
+		newState := state.Clone()
+		newState.Update(payload)
+
+		// Save the updated state
+		ctx, cancel = context.WithTimeout(parentCtx, 10*time.Second)
+		defer cancel()
+		err = a.client.SetState(ctx, state, nil)
+
+		// Update the cached state too
+		*state = *newState
+	}
+
+	// Return the state
+	return state, nil
+}

backend/internal/appconfig/model.go

@@ -63,6 +63,17 @@ type AppConfigModel struct {
 	LdapSoftDeleteUsers                AppConfigValue `json:"ldapSoftDeleteUsers" type:"bool"`
 }
 
+// Clone returns a deep copy of the AppConfigModel.
+func (m *AppConfigModel) Clone() *AppConfigModel {
+	if m == nil {
+		return nil
+	}
+
+	// All fields are value types (AppConfigValue is a string), so copying the struct is sufficient for a deep copy.
+	clone := *m
+	return &clone
+}
+
 // AppConfigValue holds a value
 type AppConfigValue string
 
@@ -182,7 +193,7 @@ func fromLegacyConfig(legacyCfg map[string]string) (*AppConfigModel, error) {
 
 // Replace updates every configuration property with the values from the input DTO
 // An empty string value resets the corresponding property to its default value
-func (m *AppConfigModel) Replace(input dto.AppConfigUpdateDto) error {
+func (m *AppConfigModel) Replace(input dto.AppConfigUpdateDto) {
 	// Collect the values from the input DTO into a map, keyed by the "json" tag
 	inRv := reflect.ValueOf(input)
 	inRt := inRv.Type()
@@ -208,29 +219,18 @@ func (m *AppConfigModel) Replace(input dto.AppConfigUpdateDto) error {
 
 		rv.Field(i).SetString(value)
 	}
-
-	return nil
 }
 
 // Update sets configuration properties from the provided key-value pairs
 // Keys correspond to the "json" tags on the model
 // An empty string value resets the property to its default value
-func (m *AppConfigModel) Update(keysAndValues ...string) error {
-	// Count of keysAndValues must be even
-	if len(keysAndValues)%2 != 0 {
-		return errors.New("invalid number of arguments received")
-	}
-
+func (m *AppConfigModel) Update(values map[string]string) error {
 	rv := reflect.ValueOf(m).Elem()
 	rt := rv.Type()
 	defaults := reflect.ValueOf(getDefaultConfig()).Elem()
 
 	// Iterate through the key-value pairs
-	// (Note the += 2, as we are iterating through key-value pairs)
-	for i := 1; i < len(keysAndValues); i += 2 {
-		key := keysAndValues[i-1]
-		value := keysAndValues[i]
-
+	for key, value := range values {
 		// Find the field in the struct whose "json" tag matches
 		fieldIdx := -1
 		for j := range rt.NumField() {

backend/internal/appconfig/model_test.go

@@ -29,8 +29,7 @@ func TestAppConfigModel_Replace(t *testing.T) {
 		input := dtoWithMarkerValues()
 
 		var m AppConfigModel
-		err := m.Replace(input)
-		require.NoError(t, err)
+		m.Replace(input)
 
 		// Each model property must hold the marker built from its own "json" key.
 		// This also asserts that the model and the DTO share the same set of keys.
@@ -53,8 +52,7 @@ func TestAppConfigModel_Replace(t *testing.T) {
 		input.LdapUserSearchFilter = ""
 
 		var m AppConfigModel
-		err := m.Replace(input)
-		require.NoError(t, err)
+		m.Replace(input)
 
 		// Blanked properties are reset to their default
 		assert.Equal(t, defaults.AppName, m.AppName)
@@ -74,8 +72,7 @@ func TestAppConfigModel_Replace(t *testing.T) {
 			SmtpHost:    "smtp.example.com",
 		}
 
-		err := m.Replace(dto.AppConfigUpdateDto{})
-		require.NoError(t, err)
+		m.Replace(dto.AppConfigUpdateDto{})
 
 		assert.Equal(t, *getDefaultConfig(), m)
 	})
@@ -88,8 +85,7 @@ func TestAppConfigModel_Replace(t *testing.T) {
 		input := dto.AppConfigUpdateDto{}
 		input.AppName = "New Name"
 
-		err := m.Replace(input)
-		require.NoError(t, err)
+		m.Replace(input)
 
 		// Explicitly provided value wins
 		assert.Equal(t, "New Name", m.AppName)
@@ -103,19 +99,68 @@ func TestAppConfigModel_Replace(t *testing.T) {
 		input.LdapEnabled = "true"    // bool-tagged property
 
 		var m AppConfigModel
-		err := m.Replace(input)
-		require.NoError(t, err)
+		m.Replace(input)
 
 		assert.Equal(t, "120", m.SessionDuration)
 		assert.Equal(t, "true", m.LdapEnabled)
 	})
 }
 
+func TestAppConfigModel_Clone(t *testing.T) {
+	t.Run("clones every property", func(t *testing.T) {
+		// Populate every property with a unique marker so we can assert each one is copied
+		var original AppConfigModel
+		rv := reflect.ValueOf(&original).Elem()
+		rt := rv.Type()
+		for i := range rt.NumField() {
+			key, _, _ := strings.Cut(rt.Field(i).Tag.Get("json"), ",")
+			rv.Field(i).SetString("marker-" + key)
+		}
+
+		clone := original.Clone()
+
+		require.NotNil(t, clone)
+		// The clone must be a distinct object with equal contents
+		assert.NotSame(t, &original, clone)
+		assert.Equal(t, original, *clone)
+	})
+
+	t.Run("mutating the clone does not affect the original", func(t *testing.T) {
+		original := getDefaultConfig()
+
+		clone := original.Clone()
+		clone.AppName = "Changed"
+		clone.LdapEnabled = "true"
+
+		// The original keeps its values
+		assert.Equal(t, getDefaultConfig().AppName, original.AppName)
+		assert.Equal(t, getDefaultConfig().LdapEnabled, original.LdapEnabled)
+
+		// The clone holds the new values
+		assert.Equal(t, AppConfigValue("Changed"), clone.AppName)
+		assert.Equal(t, AppConfigValue("true"), clone.LdapEnabled)
+	})
+
+	t.Run("mutating the original does not affect the clone", func(t *testing.T) {
+		original := getDefaultConfig()
+
+		clone := original.Clone()
+		original.AppName = "Changed"
+
+		assert.Equal(t, getDefaultConfig().AppName, clone.AppName)
+	})
+
+	t.Run("cloning a nil receiver returns nil", func(t *testing.T) {
+		var m *AppConfigModel
+		assert.Nil(t, m.Clone())
+	})
+}
+
 func TestAppConfigModel_Update(t *testing.T) {
 	t.Run("updates a single property", func(t *testing.T) {
 		m := getDefaultConfig()
 
-		err := m.Update("appName", "My App")
+		err := m.Update(map[string]string{"appName": "My App"})
 		require.NoError(t, err)
 
 		assert.Equal(t, "My App", m.AppName)
@@ -124,7 +169,7 @@ func TestAppConfigModel_Update(t *testing.T) {
 	t.Run("updates multiple properties and leaves others untouched", func(t *testing.T) {
 		m := getDefaultConfig()
 
-		err := m.Update("appName", "My App", "homePageUrl", "/home", "ldapEnabled", "true")
+		err := m.Update(map[string]string{"appName": "My App", "homePageUrl": "/home", "ldapEnabled": "true"})
 		require.NoError(t, err)
 
 		assert.Equal(t, "My App", m.AppName)
@@ -139,7 +184,7 @@ func TestAppConfigModel_Update(t *testing.T) {
 		m.SmtpTls = "tls"         // default is "none"
 		m.SessionDuration = "120" // default is "60"
 
-		err := m.Update("smtpTls", "", "sessionDuration", "")
+		err := m.Update(map[string]string{"smtpTls": "", "sessionDuration": ""})
 		require.NoError(t, err)
 
 		assert.Equal(t, getDefaultConfig().SmtpTls, m.SmtpTls)
@@ -149,48 +194,27 @@ func TestAppConfigModel_Update(t *testing.T) {
 	t.Run("stores raw string values without type coercion", func(t *testing.T) {
 		m := getDefaultConfig()
 
-		err := m.Update("sessionDuration", "120", "disableAnimations", "true")
+		err := m.Update(map[string]string{"sessionDuration": "120", "disableAnimations": "true"})
 		require.NoError(t, err)
 
 		assert.Equal(t, "120", m.SessionDuration)
 		assert.Equal(t, "true", m.DisableAnimations)
 	})
 
-	t.Run("later value wins for a repeated key", func(t *testing.T) {
-		m := getDefaultConfig()
-
-		err := m.Update("appName", "First", "appName", "Second")
-		require.NoError(t, err)
-
-		assert.Equal(t, "Second", m.AppName)
-	})
-
-	t.Run("no arguments is a no-op", func(t *testing.T) {
+	t.Run("an empty map is a no-op", func(t *testing.T) {
 		m := getDefaultConfig()
 		before := *m
 
-		err := m.Update()
+		err := m.Update(nil)
 		require.NoError(t, err)
 
 		assert.Equal(t, before, *m)
 	})
 
-	t.Run("an odd number of arguments returns an error", func(t *testing.T) {
-		m := getDefaultConfig()
-		before := *m
-
-		err := m.Update("appName")
-		require.Error(t, err)
-		assert.EqualError(t, err, "invalid number of arguments received")
-
-		// The config must not have been modified
-		assert.Equal(t, before, *m)
-	})
-
 	t.Run("an unknown key returns AppConfigKeyNotFoundError", func(t *testing.T) {
 		m := getDefaultConfig()
 
-		err := m.Update("thisKeyDoesNotExist", "value")
+		err := m.Update(map[string]string{"thisKeyDoesNotExist": "value"})
 		require.Error(t, err)
 		assert.EqualError(t, err, "cannot find config key 'thisKeyDoesNotExist'")
 

backend/internal/appconfig/service.go

@@ -12,7 +12,6 @@ import (
 	"github.com/italypaleale/francis/actor"
 	"github.com/italypaleale/francis/host/local"
 	"gorm.io/gorm"
-	"gorm.io/gorm/clause"
 
 	"github.com/pocket-id/pocket-id/backend/internal/common"
 	"github.com/pocket-id/pocket-id/backend/internal/dto"
@@ -96,24 +95,8 @@ func (s *AppConfigService) GetConfig(parentCtx context.Context) (*AppConfigModel
 	return &cfg, nil
 }
 
-func (s *AppConfigService) updateAppConfigUpdateDatabase(ctx context.Context, tx *gorm.DB, dbUpdate *[]model.AppConfigVariable) error {
-	err := tx.
-		WithContext(ctx).
-		Clauses(clause.OnConflict{
-			// Perform an "upsert" if the key already exists, replacing the value
-			Columns:   []clause.Column{{Name: "key"}},
-			DoUpdates: clause.AssignmentColumns([]string{"value"}),
-		}).
-		Create(&dbUpdate).
-		Error
-	if err != nil {
-		return fmt.Errorf("failed to update config in database: %w", err)
-	}
-
-	return nil
-}
-
 func (s *AppConfigService) UpdateAppConfig(ctx context.Context, input dto.AppConfigUpdateDto) ([]model.AppConfigVariable, error) {
+	// If the UI config is disabled, we cannot continue
 	if common.EnvConfig.UiConfigDisabled {
 		return nil, &common.UiConfigDisabledError{}
 	}
@@ -185,6 +168,7 @@ func (s *AppConfigService) UpdateAppConfigValues(ctx context.Context, keysAndVal
 		return errors.New("invalid number of arguments received")
 	}
 
+	// If the UI config is disabled, we cannot continue
 	if common.EnvConfig.UiConfigDisabled {
 		return &common.UiConfigDisabledError{}
 	}
@@ -256,10 +240,6 @@ func (s *AppConfigService) UpdateAppConfigValues(ctx context.Context, keysAndVal
 	return nil
 }
 
-func (s *AppConfigService) ListAppConfig(showAll bool) []model.AppConfigVariable {
-	return s.GetDbConfig().ToAppConfigVariableSlice(showAll, true)
-}
-
 func (s *AppConfigService) loadDbConfigFromEnv() (*AppConfigModel, error) {
 	// First, start from the default configuration
 	dest := getDefaultConfig()