diff --git a/backend/internal/appconfig/appconfig_actor.go b/backend/internal/appconfig/appconfig_actor.go index d15b6a02..903da950 100644 --- a/backend/internal/appconfig/appconfig_actor.go +++ b/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 +} diff --git a/backend/internal/appconfig/model.go b/backend/internal/appconfig/model.go index c862ea85..798e26b6 100644 --- a/backend/internal/appconfig/model.go +++ b/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() { diff --git a/backend/internal/appconfig/model_test.go b/backend/internal/appconfig/model_test.go index 123de9b7..b992f230 100644 --- a/backend/internal/appconfig/model_test.go +++ b/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'") diff --git a/backend/internal/appconfig/service.go b/backend/internal/appconfig/service.go index 38a53822..7d58d716 100644 --- a/backend/internal/appconfig/service.go +++ b/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() diff --git a/backend/resources/migrations/postgres/20260709000000_freeze_config.down.sql b/backend/resources/migrations/postgres/20260718000000_freeze_config.down.sql similarity index 100% rename from backend/resources/migrations/postgres/20260709000000_freeze_config.down.sql rename to backend/resources/migrations/postgres/20260718000000_freeze_config.down.sql diff --git a/backend/resources/migrations/postgres/20260709000000_freeze_config.up.sql b/backend/resources/migrations/postgres/20260718000000_freeze_config.up.sql similarity index 100% rename from backend/resources/migrations/postgres/20260709000000_freeze_config.up.sql rename to backend/resources/migrations/postgres/20260718000000_freeze_config.up.sql diff --git a/backend/resources/migrations/sqlite/20260709000000_freeze_config.down.sql b/backend/resources/migrations/sqlite/20260718000000_freeze_config.down.sql similarity index 100% rename from backend/resources/migrations/sqlite/20260709000000_freeze_config.down.sql rename to backend/resources/migrations/sqlite/20260718000000_freeze_config.down.sql diff --git a/backend/resources/migrations/sqlite/20260709000000_freeze_config.up.sql b/backend/resources/migrations/sqlite/20260718000000_freeze_config.up.sql similarity index 100% rename from backend/resources/migrations/sqlite/20260709000000_freeze_config.up.sql rename to backend/resources/migrations/sqlite/20260718000000_freeze_config.up.sql