feat(plugins): add JSONForms-based plugin configuration UI (#4911)

* feat(plugins): add JSONForms schema for plugin configuration Signed-off-by: Deluan <deluan@navidrome.org> * feat: enhance error handling by formatting validation errors with field names Signed-off-by: Deluan <deluan@navidrome.org> * feat: enforce required fields in config validation and improve error handling Signed-off-by: Deluan <deluan@navidrome.org> * format JS code Signed-off-by: Deluan <deluan@navidrome.org> * feat: add config schema validation and enhance manifest structure Signed-off-by: Deluan <deluan@navidrome.org> * feat: refactor plugin config parsing and add unit tests Signed-off-by: Deluan <deluan@navidrome.org> * feat: add config validation error message in Portuguese * feat: enhance AlwaysExpandedArrayLayout with description support and improve array control testing Signed-off-by: Deluan <deluan@navidrome.org> * feat: update Discord Rust plugin configuration to use JSONForm for user tokens and enhance schema validation Signed-off-by: Deluan <deluan@navidrome.org> * fix: resolve React Hooks linting issues in plugin UI components * Apply suggestions from code review Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * format code Signed-off-by: Deluan <deluan@navidrome.org> * feat: migrate schema validation to use santhosh-tekuri/jsonschema and improve error formatting Signed-off-by: Deluan <deluan@navidrome.org> * address PR comments Signed-off-by: Deluan <deluan@navidrome.org> * fix flaky test Signed-off-by: Deluan <deluan@navidrome.org> * feat: enhance array layout and configuration handling with AJV defaults Signed-off-by: Deluan <deluan@navidrome.org> * feat: implement custom tester to exclude enum arrays from AlwaysExpandedArrayLayout Signed-off-by: Deluan <deluan@navidrome.org> * feat: add error boundary for schema rendering and improve error messages Signed-off-by: Deluan <deluan@navidrome.org> * feat: refine non-enum array control logic by utilizing JSONForms schema resolution Signed-off-by: Deluan <deluan@navidrome.org> * feat: add error styling to ToggleEnabledSwitch for disabled state Signed-off-by: Deluan <deluan@navidrome.org> * feat: adjust label positioning and styling in SchemaConfigEditor for improved layout Signed-off-by: Deluan <deluan@navidrome.org> * feat: implement outlined input controls renderers to replace custom fragile CSS Signed-off-by: Deluan <deluan@navidrome.org> * feat: remove margin from last form control inside array items for better spacing Signed-off-by: Deluan <deluan@navidrome.org> * feat: enhance AJV error handling to transform required errors for field-level validation Signed-off-by: Deluan <deluan@navidrome.org> * feat: set default value for User Tokens in manifest.json to improve user experience Signed-off-by: Deluan <deluan@navidrome.org> * format Signed-off-by: Deluan <deluan@navidrome.org> * feat: add margin to outlined input controls for improved spacing Signed-off-by: Deluan <deluan@navidrome.org> * feat: remove redundant margin rule for last form control in array items Signed-off-by: Deluan <deluan@navidrome.org> * feat: adjust font size of label elements in SchemaConfigEditor for improved readability Signed-off-by: Deluan <deluan@navidrome.org> --------- Signed-off-by: Deluan <deluan@navidrome.org> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
2026-01-19 20:51:00 -05:00
parent 66474fc9f4
commit f1e75c40dc
40 changed files with 5430 additions and 2007 deletions
@@ -20,6 +20,105 @@ import (
 	. "github.com/onsi/gomega"
 )

+// testConfigInput is the input for nd_test_config callback.
+type testConfigInput struct {
+	Operation string `json:"operation"`
+	Key       string `json:"key,omitempty"`
+	Prefix    string `json:"prefix,omitempty"`
+}
+
+// testConfigOutput is the output from nd_test_config callback.
+type testConfigOutput struct {
+	StringVal string   `json:"string_val,omitempty"`
+	IntVal    int64    `json:"int_val,omitempty"`
+	Keys      []string `json:"keys,omitempty"`
+	Exists    bool     `json:"exists,omitempty"`
+	Error     *string  `json:"error,omitempty"`
+}
+
+// setupTestConfigPlugin sets up a test environment with the test-config plugin loaded.
+// Returns a cleanup function and a helper to call the plugin's nd_test_config function.
+func setupTestConfigPlugin(configJSON string) (*Manager, func(context.Context, testConfigInput) (*testConfigOutput, error)) {
+	tmpDir, err := os.MkdirTemp("", "config-test-*")
+	Expect(err).ToNot(HaveOccurred())
+
+	// Copy the test-config plugin
+	srcPath := filepath.Join(testdataDir, "test-config"+PackageExtension)
+	destPath := filepath.Join(tmpDir, "test-config"+PackageExtension)
+	data, err := os.ReadFile(srcPath)
+	Expect(err).ToNot(HaveOccurred())
+	err = os.WriteFile(destPath, data, 0600)
+	Expect(err).ToNot(HaveOccurred())
+
+	// Compute SHA256 for the plugin
+	hash := sha256.Sum256(data)
+	hashHex := hex.EncodeToString(hash[:])
+
+	// Setup config
+	DeferCleanup(configtest.SetupConfig())
+	conf.Server.Plugins.Enabled = true
+	conf.Server.Plugins.Folder = tmpDir
+	conf.Server.Plugins.AutoReload = false
+	conf.Server.CacheFolder = filepath.Join(tmpDir, "cache")
+
+	// Setup mock DataStore
+	mockPluginRepo := tests.CreateMockPluginRepo()
+	mockPluginRepo.Permitted = true
+	mockPluginRepo.SetData(model.Plugins{{
+		ID:       "test-config",
+		Path:     destPath,
+		SHA256:   hashHex,
+		Enabled:  true,
+		AllUsers: true,
+		Config:   configJSON,
+	}})
+	dataStore := &tests.MockDataStore{MockedPlugin: mockPluginRepo}
+
+	// Create and start manager
+	manager := &Manager{
+		plugins:        make(map[string]*plugin),
+		ds:             dataStore,
+		subsonicRouter: http.NotFoundHandler(),
+	}
+	err = manager.Start(GinkgoT().Context())
+	Expect(err).ToNot(HaveOccurred())
+
+	DeferCleanup(func() {
+		_ = manager.Stop()
+		_ = os.RemoveAll(tmpDir)
+	})
+
+	// Helper to call test plugin's exported function
+	callTestConfig := func(ctx context.Context, input testConfigInput) (*testConfigOutput, error) {
+		manager.mu.RLock()
+		p := manager.plugins["test-config"]
+		manager.mu.RUnlock()
+
+		instance, err := p.instance(ctx)
+		if err != nil {
+			return nil, err
+		}
+		defer instance.Close(ctx)
+
+		inputBytes, _ := json.Marshal(input)
+		_, outputBytes, err := instance.Call("nd_test_config", inputBytes)
+		if err != nil {
+			return nil, err
+		}
+
+		var output testConfigOutput
+		if err := json.Unmarshal(outputBytes, &output); err != nil {
+			return nil, err
+		}
+		if output.Error != nil {
+			return nil, errors.New(*output.Error)
+		}
+		return &output, nil
+	}
+
+	return manager, callTestConfig
+}
+
 var _ = Describe("ConfigService", func() {
 	var service *configServiceImpl
 	var ctx context.Context
@@ -144,59 +243,12 @@ var _ = Describe("ConfigService", func() {

 var _ = Describe("ConfigService Integration", Ordered, func() {
 	var (
-		manager *Manager
-		tmpDir  string
+		manager        *Manager
+		callTestConfig func(context.Context, testConfigInput) (*testConfigOutput, error)
 	)

 	BeforeAll(func() {
-		var err error
-		tmpDir, err = os.MkdirTemp("", "config-test-*")
-		Expect(err).ToNot(HaveOccurred())
-
-		// Copy the test-config plugin
-		srcPath := filepath.Join(testdataDir, "test-config"+PackageExtension)
-		destPath := filepath.Join(tmpDir, "test-config"+PackageExtension)
-		data, err := os.ReadFile(srcPath)
-		Expect(err).ToNot(HaveOccurred())
-		err = os.WriteFile(destPath, data, 0600)
-		Expect(err).ToNot(HaveOccurred())
-
-		// Compute SHA256 for the plugin
-		hash := sha256.Sum256(data)
-		hashHex := hex.EncodeToString(hash[:])
-
-		// Setup config
-		DeferCleanup(configtest.SetupConfig())
-		conf.Server.Plugins.Enabled = true
-		conf.Server.Plugins.Folder = tmpDir
-		conf.Server.Plugins.AutoReload = false
-		conf.Server.CacheFolder = filepath.Join(tmpDir, "cache")
-
-		// Setup mock DataStore with pre-enabled plugin and config
-		mockPluginRepo := tests.CreateMockPluginRepo()
-		mockPluginRepo.Permitted = true
-		mockPluginRepo.SetData(model.Plugins{{
-			ID:      "test-config",
-			Path:    destPath,
-			SHA256:  hashHex,
-			Enabled: true,
-			Config:  `{"api_key":"test_secret","max_retries":"5","timeout":"30"}`,
-		}})
-		dataStore := &tests.MockDataStore{MockedPlugin: mockPluginRepo}
-
-		// Create and start manager
-		manager = &Manager{
-			plugins:        make(map[string]*plugin),
-			ds:             dataStore,
-			subsonicRouter: http.NotFoundHandler(),
-		}
-		err = manager.Start(GinkgoT().Context())
-		Expect(err).ToNot(HaveOccurred())
-
-		DeferCleanup(func() {
-			_ = manager.Stop()
-			_ = os.RemoveAll(tmpDir)
-		})
+		manager, callTestConfig = setupTestConfigPlugin(`{"api_key":"test_secret","max_retries":"5","timeout":"30"}`)
 	})

 	Describe("Plugin Loading", func() {
@@ -205,54 +257,11 @@ var _ = Describe("ConfigService Integration", Ordered, func() {
 			p, ok := manager.plugins["test-config"]
 			manager.mu.RUnlock()
 			Expect(ok).To(BeTrue())
-			// Config service doesn't require permission, so Permissions can be nil
-			// Just verify the plugin loaded
 			Expect(p.manifest.Name).To(Equal("Test Config Plugin"))
 		})
 	})

 	Describe("Config Operations via Plugin", func() {
-		type testConfigInput struct {
-			Operation string `json:"operation"`
-			Key       string `json:"key,omitempty"`
-			Prefix    string `json:"prefix,omitempty"`
-		}
-		type testConfigOutput struct {
-			StringVal string   `json:"string_val,omitempty"`
-			IntVal    int64    `json:"int_val,omitempty"`
-			Keys      []string `json:"keys,omitempty"`
-			Exists    bool     `json:"exists,omitempty"`
-			Error     *string  `json:"error,omitempty"`
-		}
-
-		// Helper to call test plugin's exported function
-		callTestConfig := func(ctx context.Context, input testConfigInput) (*testConfigOutput, error) {
-			manager.mu.RLock()
-			p := manager.plugins["test-config"]
-			manager.mu.RUnlock()
-
-			instance, err := p.instance(ctx)
-			if err != nil {
-				return nil, err
-			}
-			defer instance.Close(ctx)
-
-			inputBytes, _ := json.Marshal(input)
-			_, outputBytes, err := instance.Call("nd_test_config", inputBytes)
-			if err != nil {
-				return nil, err
-			}
-
-			var output testConfigOutput
-			if err := json.Unmarshal(outputBytes, &output); err != nil {
-				return nil, err
-			}
-			if output.Error != nil {
-				return nil, errors.New(*output.Error)
-			}
-			return &output, nil
-		}
-
 		It("should get string value", func() {
 			output, err := callTestConfig(GinkgoT().Context(), testConfigInput{
 				Operation: "get",
@@ -285,7 +294,7 @@ var _ = Describe("ConfigService Integration", Ordered, func() {
 		It("should return not exists for non-integer value", func() {
 			output, err := callTestConfig(GinkgoT().Context(), testConfigInput{
 				Operation: "get_int",
-				Key:       "api_key", // This is a string, not an integer
+				Key:       "api_key",
 			})
 			Expect(err).ToNot(HaveOccurred())
 			Expect(output.Exists).To(BeFalse())
@@ -310,3 +319,64 @@ var _ = Describe("ConfigService Integration", Ordered, func() {
 		})
 	})
 })
+
+var _ = Describe("Complex Config Values Integration", Ordered, func() {
+	var callTestConfig func(context.Context, testConfigInput) (*testConfigOutput, error)
+
+	BeforeAll(func() {
+		// Config with arrays and objects - these should be properly serialized as JSON strings
+		_, callTestConfig = setupTestConfigPlugin(`{"api_key":"secret123","users":[{"username":"admin","token":"tok1"},{"username":"user2","token":"tok2"}],"settings":{"enabled":true,"count":5}}`)
+	})
+
+	Describe("Config Serialization", func() {
+		It("should make simple string config values accessible to plugin", func() {
+			output, err := callTestConfig(GinkgoT().Context(), testConfigInput{
+				Operation: "get",
+				Key:       "api_key",
+			})
+			Expect(err).ToNot(HaveOccurred())
+			Expect(output.Exists).To(BeTrue())
+			Expect(output.StringVal).To(Equal("secret123"))
+		})
+
+		It("should serialize array config values as JSON strings", func() {
+			output, err := callTestConfig(GinkgoT().Context(), testConfigInput{
+				Operation: "get",
+				Key:       "users",
+			})
+			Expect(err).ToNot(HaveOccurred())
+			Expect(output.Exists).To(BeTrue())
+			// Array values are serialized as JSON strings - parse to verify structure
+			var users []map[string]string
+			Expect(json.Unmarshal([]byte(output.StringVal), &users)).To(Succeed())
+			Expect(users).To(HaveLen(2))
+			Expect(users[0]).To(HaveKeyWithValue("username", "admin"))
+			Expect(users[0]).To(HaveKeyWithValue("token", "tok1"))
+			Expect(users[1]).To(HaveKeyWithValue("username", "user2"))
+			Expect(users[1]).To(HaveKeyWithValue("token", "tok2"))
+		})
+
+		It("should serialize object config values as JSON strings", func() {
+			output, err := callTestConfig(GinkgoT().Context(), testConfigInput{
+				Operation: "get",
+				Key:       "settings",
+			})
+			Expect(err).ToNot(HaveOccurred())
+			Expect(output.Exists).To(BeTrue())
+			// Object values are serialized as JSON strings - parse to verify structure
+			var settings map[string]any
+			Expect(json.Unmarshal([]byte(output.StringVal), &settings)).To(Succeed())
+			Expect(settings).To(HaveKeyWithValue("enabled", true))
+			Expect(settings).To(HaveKeyWithValue("count", float64(5)))
+		})
+
+		It("should list all config keys including complex values", func() {
+			output, err := callTestConfig(GinkgoT().Context(), testConfigInput{
+				Operation: "list",
+				Prefix:    "",
+			})
+			Expect(err).ToNot(HaveOccurred())
+			Expect(output.Keys).To(ConsistOf("api_key", "users", "settings"))
+		})
+	})
+})