updated app.RefreshSettings and added more tests

core/base.go

@@ -1,6 +1,7 @@
 package core
 
 import (
+	"bytes"
 	"context"
 	"database/sql"
 	"encoding/json"
@@ -358,8 +359,8 @@ func (app *BaseApp) RefreshSettings() error {
 		return err
 	}
 
+	// no settings were previously stored
 	if param == nil {
-		// no settings were previously stored
 		return app.Dao().SaveParam(models.ParamAppSettings, app.settings, encryptionKey)
 	}
 
@@ -390,12 +391,25 @@ func (app *BaseApp) RefreshSettings() error {
 		}
 	}
 
+	beforeMergeRaw, err := json.Marshal(newSettings)
+	if err != nil {
+		return err
+	}
+
 	if err := app.settings.Merge(newSettings); err != nil {
 		return err
 	}
 
-	if plainDecodeErr == nil && encryptionKey != "" {
-		// save because previously the settings weren't stored encrypted
+	afterMergeRaw, err := json.Marshal(app.settings)
+	if err != nil {
+		return err
+	}
+
+	if
+	// save because previously the settings weren't stored encrypted
+	(plainDecodeErr == nil && encryptionKey != "") ||
+		// or save because there are new fields after the merge
+		!bytes.Equal(beforeMergeRaw, afterMergeRaw) {
 		saveErr := app.Dao().SaveParam(models.ParamAppSettings, app.settings, encryptionKey)
 		if saveErr != nil {
 			return saveErr

core/settings_refresh_test.go

@@ -0,0 +1,76 @@
+package core_test
+
+import (
+	"bytes"
+	"testing"
+
+	"github.com/pocketbase/pocketbase/models"
+	"github.com/pocketbase/pocketbase/tests"
+	"github.com/pocketbase/pocketbase/tools/types"
+)
+
+func TestBaseAppRefreshSettings(t *testing.T) {
+	app, _ := tests.NewTestApp()
+	defer app.Cleanup()
+
+	// cleanup all stored settings
+	if _, err := app.DB().NewQuery("DELETE from _params;").Execute(); err != nil {
+		t.Fatalf("Failed to delete all test settings: %v", err)
+	}
+
+	// check if the new settings are saved in the db
+	app.ResetEventCalls()
+	if err := app.RefreshSettings(); err != nil {
+		t.Fatal("Failed to refresh the settings after delete")
+	}
+	testEvents(t, app, map[string]int{
+		"OnModelBeforeCreate": 1,
+		"OnModelAfterCreate":  1,
+	})
+	param, err := app.Dao().FindParamByKey(models.ParamAppSettings)
+	if err != nil {
+		t.Fatalf("Expected new settings to be persisted, got %v", err)
+	}
+
+	// change the db entry and refresh the app settings
+	param.Value = types.JsonRaw([]byte(`{"example": 123}`))
+	if err := app.Dao().SaveParam(param.Key, param.Value); err != nil {
+		t.Fatalf("Failed to update the test settings: %v", err)
+	}
+	app.ResetEventCalls()
+	if err := app.RefreshSettings(); err != nil {
+		t.Fatalf("Failed to refresh the app settings: %v", err)
+	}
+	testEvents(t, app, map[string]int{
+		"OnModelBeforeUpdate": 1,
+		"OnModelAfterUpdate":  1,
+	})
+
+	// make sure that the newly merged settings were actually saved
+	newParam, err := app.Dao().FindParamByKey(models.ParamAppSettings)
+	if err != nil {
+		t.Fatalf("Failed to fetch new settings param: %v", err)
+	}
+	if bytes.Equal(param.Value, newParam.Value) {
+		t.Fatalf("Expected the changed refreshed and merged settings to be persisted, got: \n%v", string(newParam.Value))
+	}
+
+	// try to refresh again and ensure that there was no db update
+	app.ResetEventCalls()
+	if err := app.RefreshSettings(); err != nil {
+		t.Fatalf("Failed to refresh the app settings without change: %v", err)
+	}
+	testEvents(t, app, nil)
+}
+
+func testEvents(t *testing.T, app *tests.TestApp, events map[string]int) {
+	if len(events) != len(app.EventCalls) {
+		t.Fatalf("Expected events doesn't match: \n%v, \ngot \n%v", events, app.EventCalls)
+	}
+
+	for name, total := range events {
+		if v, ok := app.EventCalls[name]; !ok || v != total {
+			t.Fatalf("Expected events doesn't exist or match: \n%v, \ngot \n%v", events, app.EventCalls)
+		}
+	}
+}