From 38c121c78ce0a522ea7fa9a86f81cb708857ca66 Mon Sep 17 00:00:00 2001 From: Deluan Date: Wed, 14 Jan 2026 15:34:07 -0500 Subject: [PATCH] feat: implement IsOptionPattern method for better return type handling in Rust PDK generation Signed-off-by: Deluan --- plugins/cmd/ndpgen/integration_test.go | 3 + plugins/cmd/ndpgen/internal/generator_test.go | 192 ++++++++++++++++++ .../ndpgen/internal/templates/client.rs.tmpl | 30 ++- plugins/cmd/ndpgen/internal/types.go | 19 ++ .../testdata/config_client_expected.go.txt | 156 ++++++++++++++ .../ndpgen/testdata/config_client_expected.py | 126 ++++++++++++ .../ndpgen/testdata/config_client_expected.rs | 135 ++++++++++++ .../cmd/ndpgen/testdata/config_service.go.txt | 15 ++ .../discord-rich-presence-rs/src/lib.rs | 10 +- .../discord-rich-presence-rs/src/rpc.rs | 97 ++++----- .../pdk/rust/nd-pdk-host/src/nd_host_cache.rs | 40 ++-- .../rust/nd-pdk-host/src/nd_host_config.rs | 20 +- .../rust/nd-pdk-host/src/nd_host_kvstore.rs | 10 +- 13 files changed, 767 insertions(+), 86 deletions(-) create mode 100644 plugins/cmd/ndpgen/testdata/config_client_expected.go.txt create mode 100644 plugins/cmd/ndpgen/testdata/config_client_expected.py create mode 100644 plugins/cmd/ndpgen/testdata/config_client_expected.rs create mode 100644 plugins/cmd/ndpgen/testdata/config_service.go.txt diff --git a/plugins/cmd/ndpgen/integration_test.go b/plugins/cmd/ndpgen/integration_test.go index a491fdfc1..db500c1fc 100644 --- a/plugins/cmd/ndpgen/integration_test.go +++ b/plugins/cmd/ndpgen/integration_test.go @@ -279,6 +279,9 @@ type ServiceB interface { Entry("bytes", "codec_service.go.txt", "codec_client_expected.go.txt", "codec_client_expected.py", "codec_client_expected.rs"), + + Entry("option pattern (value, exists bool)", + "config_service.go.txt", "config_client_expected.go.txt", "config_client_expected.py", "config_client_expected.rs"), ) It("generates compilable client code for comprehensive service", func() { diff --git a/plugins/cmd/ndpgen/internal/generator_test.go b/plugins/cmd/ndpgen/internal/generator_test.go index 58ba64788..0fcd0da98 100644 --- a/plugins/cmd/ndpgen/internal/generator_test.go +++ b/plugins/cmd/ndpgen/internal/generator_test.go @@ -315,6 +315,109 @@ var _ = Describe("Generator", func() { }) }) + Describe("Method.IsOptionPattern", func() { + It("should return true for (value, exists bool) pattern", func() { + m := Method{ + Returns: []Param{ + {Name: "value", Type: "string"}, + {Name: "exists", Type: "bool"}, + }, + } + Expect(m.IsOptionPattern()).To(BeTrue()) + }) + + It("should return true for (value, ok bool) pattern", func() { + m := Method{ + Returns: []Param{ + {Name: "value", Type: "int64"}, + {Name: "ok", Type: "bool"}, + }, + } + Expect(m.IsOptionPattern()).To(BeTrue()) + }) + + It("should return true for (value, found bool) pattern", func() { + m := Method{ + Returns: []Param{ + {Name: "data", Type: "[]byte"}, + {Name: "found", Type: "bool"}, + }, + } + Expect(m.IsOptionPattern()).To(BeTrue()) + }) + + It("should be case insensitive for bool name", func() { + m := Method{ + Returns: []Param{ + {Name: "value", Type: "string"}, + {Name: "EXISTS", Type: "bool"}, + }, + } + Expect(m.IsOptionPattern()).To(BeTrue()) + }) + + It("should return false for single return", func() { + m := Method{ + Returns: []Param{ + {Name: "value", Type: "string"}, + }, + } + Expect(m.IsOptionPattern()).To(BeFalse()) + }) + + It("should return false for more than two returns", func() { + m := Method{ + Returns: []Param{ + {Name: "value", Type: "string"}, + {Name: "count", Type: "int"}, + {Name: "exists", Type: "bool"}, + }, + } + Expect(m.IsOptionPattern()).To(BeFalse()) + }) + + It("should return false when second return is not bool", func() { + m := Method{ + Returns: []Param{ + {Name: "value", Type: "string"}, + {Name: "count", Type: "int"}, + }, + } + Expect(m.IsOptionPattern()).To(BeFalse()) + }) + + It("should return false when bool is not named exists/ok/found", func() { + m := Method{ + Returns: []Param{ + {Name: "value", Type: "string"}, + {Name: "success", Type: "bool"}, + }, + } + Expect(m.IsOptionPattern()).To(BeFalse()) + }) + + It("should return false for Has() pattern where first return is bool", func() { + // Has(key) -> (exists bool) should NOT be treated as Option pattern + m := Method{ + Returns: []Param{ + {Name: "exists", Type: "bool"}, + }, + } + Expect(m.IsOptionPattern()).To(BeFalse()) + }) + + It("should return false when first return is bool (preserves Has-like methods)", func() { + // Even with two returns, if first is bool, don't convert to Option + m := Method{ + Returns: []Param{ + {Name: "result", Type: "bool"}, + {Name: "exists", Type: "bool"}, + }, + } + Expect(m.IsOptionPattern()).To(BeFalse()) + }) + }) + Describe("Python type and name helpers", func() { Describe("ToPythonType", func() { It("should map Go types to Python types", func() { @@ -1328,6 +1431,95 @@ var _ = Describe("Rust Generation", func() { Expect(codeStr).To(ContainSubstring("FnResult>")) }) }) + + Describe("GenerateClientRust", func() { + It("should generate Option for (value, exists bool) pattern", func() { + svc := Service{ + Name: "Config", + Permission: "config", + Interface: "ConfigService", + Methods: []Method{ + { + Name: "Get", + Params: []Param{ + {Name: "key", Type: "string", JSONName: "key"}, + }, + Returns: []Param{ + {Name: "value", Type: "string", JSONName: "value"}, + {Name: "exists", Type: "bool", JSONName: "exists"}, + }, + }, + }, + } + + code, err := GenerateClientRust(svc) + Expect(err).NotTo(HaveOccurred()) + + codeStr := string(code) + + // Should generate Option return type, not (String, bool) + Expect(codeStr).To(ContainSubstring("Result, Error>")) + Expect(codeStr).NotTo(ContainSubstring("Result<(String, bool), Error>")) + + // Should generate Some/None logic + Expect(codeStr).To(ContainSubstring("Ok(Some(")) + Expect(codeStr).To(ContainSubstring("Ok(None)")) + }) + + It("should generate tuple for non-option multi-return", func() { + svc := Service{ + Name: "Test", + Permission: "test", + Interface: "TestService", + Methods: []Method{ + { + Name: "GetStats", + Returns: []Param{ + {Name: "count", Type: "int64", JSONName: "count"}, + {Name: "size", Type: "int64", JSONName: "size"}, + }, + }, + }, + } + + code, err := GenerateClientRust(svc) + Expect(err).NotTo(HaveOccurred()) + + codeStr := string(code) + + // Should generate tuple return type + Expect(codeStr).To(ContainSubstring("Result<(i64, i64), Error>")) + Expect(codeStr).NotTo(ContainSubstring("Option<")) + }) + + It("should NOT generate Option for Has() pattern where first return is bool", func() { + svc := Service{ + Name: "Cache", + Permission: "cache", + Interface: "CacheService", + Methods: []Method{ + { + Name: "Has", + Params: []Param{ + {Name: "key", Type: "string", JSONName: "key"}, + }, + Returns: []Param{ + {Name: "exists", Type: "bool", JSONName: "exists"}, + }, + }, + }, + } + + code, err := GenerateClientRust(svc) + Expect(err).NotTo(HaveOccurred()) + + codeStr := string(code) + + // Should generate simple bool return, not Option + Expect(codeStr).To(ContainSubstring("Result")) + Expect(codeStr).NotTo(ContainSubstring("Option")) + }) + }) }) func writeFile(path, content string) error { diff --git a/plugins/cmd/ndpgen/internal/templates/client.rs.tmpl b/plugins/cmd/ndpgen/internal/templates/client.rs.tmpl index 39f5ea5ce..6dea8098e 100644 --- a/plugins/cmd/ndpgen/internal/templates/client.rs.tmpl +++ b/plugins/cmd/ndpgen/internal/templates/client.rs.tmpl @@ -69,7 +69,9 @@ extern "ExtismHost" { {{- if .HasReturns}} /// /// # Returns -{{- if eq (len .Returns) 1}} +{{- if .IsOptionPattern}} +/// `Some({{(index .Returns 0).RustName}})` if found, `None` otherwise. +{{- else if eq (len .Returns) 1}} /// The {{(index .Returns 0).RustName}} value. {{- else}} /// A tuple of ({{range $i, $r := .Returns}}{{if $i}}, {{end}}{{$r.RustName}}{{end}}). @@ -78,6 +80,31 @@ extern "ExtismHost" { /// /// # Errors /// Returns an error if the host function call fails. +{{- if .IsOptionPattern}} +pub fn {{rustFunc .}}({{range $i, $p := .Params}}{{if $i}}, {{end}}{{$p.RustName}}: {{rustParamType $p}}{{end}}) -> Result, Error> { + let response = unsafe { +{{- if .HasParams}} + {{exportName .}}(Json({{requestType .}} { +{{- range .Params}} + {{.RustName}}: {{.RustName}}{{if .NeedsToOwned}}.to_owned(){{end}}, +{{- end}} + }))? +{{- else}} + {{exportName .}}(Json(serde_json::json!({})))? +{{- end}} + }; +{{if .HasError}} + if let Some(err) = response.0.error { + return Err(Error::msg(err)); + } +{{end}} + if response.0.{{(index .Returns 1).RustName}} { + Ok(Some(response.0.{{(index .Returns 0).RustName}})) + } else { + Ok(None) + } +} +{{- else}} pub fn {{rustFunc .}}({{range $i, $p := .Params}}{{if $i}}, {{end}}{{$p.RustName}}: {{rustParamType $p}}{{end}}) -> Result<{{if eq (len .Returns) 0}}(){{else if eq (len .Returns) 1}}{{rustType (index .Returns 0)}}{{else}}({{range $i, $r := .Returns}}{{if $i}}, {{end}}{{rustType $r}}{{end}}){{end}}, Error> { let response = unsafe { {{- if .HasParams}} @@ -104,3 +131,4 @@ pub fn {{rustFunc .}}({{range $i, $p := .Params}}{{if $i}}, {{end}}{{$p.RustName {{- end}} } {{- end}} +{{- end}} diff --git a/plugins/cmd/ndpgen/internal/types.go b/plugins/cmd/ndpgen/internal/types.go index b46600d73..5fd7e892c 100644 --- a/plugins/cmd/ndpgen/internal/types.go +++ b/plugins/cmd/ndpgen/internal/types.go @@ -246,6 +246,25 @@ func (m Method) IsMultiReturn() bool { return len(m.Returns) > 1 } +// IsOptionPattern returns true if the method returns (value, bool) where the bool +// indicates existence (named "exists", "ok", or "found"). This pattern is used to +// generate Option in Rust instead of a tuple. +func (m Method) IsOptionPattern() bool { + if len(m.Returns) != 2 { + return false + } + if m.Returns[1].Type != "bool" { + return false + } + // Only treat as option pattern if the first return has a meaningful value type + // (not just a bool check like Has()) + if m.Returns[0].Type == "bool" { + return false + } + name := strings.ToLower(m.Returns[1].Name) + return name == "exists" || name == "ok" || name == "found" +} + // ReturnSignature returns the Go return type signature for the wrapper function. // For error-only: "error" // For single return with error: "(Type, error)" diff --git a/plugins/cmd/ndpgen/testdata/config_client_expected.go.txt b/plugins/cmd/ndpgen/testdata/config_client_expected.go.txt new file mode 100644 index 000000000..c88fb930b --- /dev/null +++ b/plugins/cmd/ndpgen/testdata/config_client_expected.go.txt @@ -0,0 +1,156 @@ +// Code generated by ndpgen. DO NOT EDIT. +// +// This file contains client wrappers for the Config host service. +// It is intended for use in Navidrome plugins built with TinyGo. +// +//go:build wasip1 + +package ndhost + +import ( + "encoding/json" + "errors" + + "github.com/navidrome/navidrome/plugins/pdk/go/pdk" +) + +// config_get is the host function provided by Navidrome. +// +//go:wasmimport extism:host/user config_get +func config_get(uint64) uint64 + +// config_set is the host function provided by Navidrome. +// +//go:wasmimport extism:host/user config_set +func config_set(uint64) uint64 + +// config_has is the host function provided by Navidrome. +// +//go:wasmimport extism:host/user config_has +func config_has(uint64) uint64 + +type configGetRequest struct { + Key string `json:"key"` +} + +type configGetResponse struct { + Value string `json:"value,omitempty"` + Exists bool `json:"exists,omitempty"` + Error string `json:"error,omitempty"` +} + +type configSetRequest struct { + Key string `json:"key"` + Value string `json:"value"` +} + +type configHasRequest struct { + Key string `json:"key"` +} + +type configHasResponse struct { + Exists bool `json:"exists,omitempty"` + Error string `json:"error,omitempty"` +} + +// ConfigGet calls the config_get host function. +func ConfigGet(key string) (string, bool, error) { + // Marshal request to JSON + req := configGetRequest{ + Key: key, + } + reqBytes, err := json.Marshal(req) + if err != nil { + return "", false, err + } + reqMem := pdk.AllocateBytes(reqBytes) + defer reqMem.Free() + + // Call the host function + responsePtr := config_get(reqMem.Offset()) + + // Read the response from memory + responseMem := pdk.FindMemory(responsePtr) + responseBytes := responseMem.ReadBytes() + + // Parse the response + var response configGetResponse + if err := json.Unmarshal(responseBytes, &response); err != nil { + return "", false, err + } + + // Convert Error field to Go error + if response.Error != "" { + return "", false, errors.New(response.Error) + } + + return response.Value, response.Exists, nil +} + +// ConfigSet calls the config_set host function. +func ConfigSet(key string, value string) error { + // Marshal request to JSON + req := configSetRequest{ + Key: key, + Value: value, + } + reqBytes, err := json.Marshal(req) + if err != nil { + return err + } + reqMem := pdk.AllocateBytes(reqBytes) + defer reqMem.Free() + + // Call the host function + responsePtr := config_set(reqMem.Offset()) + + // Read the response from memory + responseMem := pdk.FindMemory(responsePtr) + responseBytes := responseMem.ReadBytes() + + // Parse error-only response + var response struct { + Error string `json:"error,omitempty"` + } + if err := json.Unmarshal(responseBytes, &response); err != nil { + return err + } + if response.Error != "" { + return errors.New(response.Error) + } + return nil +} + +// ConfigHas calls the config_has host function. +func ConfigHas(key string) (bool, error) { + // Marshal request to JSON + req := configHasRequest{ + Key: key, + } + reqBytes, err := json.Marshal(req) + if err != nil { + return false, err + } + reqMem := pdk.AllocateBytes(reqBytes) + defer reqMem.Free() + + // Call the host function + responsePtr := config_has(reqMem.Offset()) + + // Read the response from memory + responseMem := pdk.FindMemory(responsePtr) + responseBytes := responseMem.ReadBytes() + + // Parse the response + var response configHasResponse + if err := json.Unmarshal(responseBytes, &response); err != nil { + return false, err + } + + // Convert Error field to Go error + if response.Error != "" { + return false, errors.New(response.Error) + } + + return response.Exists, nil +} diff --git a/plugins/cmd/ndpgen/testdata/config_client_expected.py b/plugins/cmd/ndpgen/testdata/config_client_expected.py new file mode 100644 index 000000000..370de6d10 --- /dev/null +++ b/plugins/cmd/ndpgen/testdata/config_client_expected.py @@ -0,0 +1,126 @@ +# Code generated by ndpgen. DO NOT EDIT. +# +# This file contains client wrappers for the Config host service. +# It is intended for use in Navidrome plugins built with extism-py. +# +# IMPORTANT: Due to a limitation in extism-py, you cannot import this file directly. +# The @extism.import_fn decorators are only detected when defined in the plugin's +# main __init__.py file. Copy the needed functions from this file into your plugin. + +from dataclasses import dataclass +from typing import Any + +import extism +import json + + +class HostFunctionError(Exception): + """Raised when a host function returns an error.""" + pass + + +@extism.import_fn("extism:host/user", "config_get") +def _config_get(offset: int) -> int: + """Raw host function - do not call directly.""" + ... + + +@extism.import_fn("extism:host/user", "config_set") +def _config_set(offset: int) -> int: + """Raw host function - do not call directly.""" + ... + + +@extism.import_fn("extism:host/user", "config_has") +def _config_has(offset: int) -> int: + """Raw host function - do not call directly.""" + ... + + +@dataclass +class ConfigGetResult: + """Result type for config_get.""" + value: str + exists: bool + + +def config_get(key: str) -> ConfigGetResult: + """Call the config_get host function. + + Args: + key: str parameter. + + Returns: + ConfigGetResult containing value, exists,. + + Raises: + HostFunctionError: If the host function returns an error. + """ + request = { + "key": key, + } + request_bytes = json.dumps(request).encode("utf-8") + request_mem = extism.memory.alloc(request_bytes) + response_offset = _config_get(request_mem.offset) + response_mem = extism.memory.find(response_offset) + response = json.loads(extism.memory.string(response_mem)) + + if response.get("error"): + raise HostFunctionError(response["error"]) + + return ConfigGetResult( + value=response.get("value", ""), + exists=response.get("exists", False), + ) + + +def config_set(key: str, value: str) -> None: + """Call the config_set host function. + + Args: + key: str parameter. + value: str parameter. + + Raises: + HostFunctionError: If the host function returns an error. + """ + request = { + "key": key, + "value": value, + } + request_bytes = json.dumps(request).encode("utf-8") + request_mem = extism.memory.alloc(request_bytes) + response_offset = _config_set(request_mem.offset) + response_mem = extism.memory.find(response_offset) + response = json.loads(extism.memory.string(response_mem)) + + if response.get("error"): + raise HostFunctionError(response["error"]) + + + +def config_has(key: str) -> bool: + """Call the config_has host function. + + Args: + key: str parameter. + + Returns: + bool: The result value. + + Raises: + HostFunctionError: If the host function returns an error. + """ + request = { + "key": key, + } + request_bytes = json.dumps(request).encode("utf-8") + request_mem = extism.memory.alloc(request_bytes) + response_offset = _config_has(request_mem.offset) + response_mem = extism.memory.find(response_offset) + response = json.loads(extism.memory.string(response_mem)) + + if response.get("error"): + raise HostFunctionError(response["error"]) + + return response.get("exists", False) diff --git a/plugins/cmd/ndpgen/testdata/config_client_expected.rs b/plugins/cmd/ndpgen/testdata/config_client_expected.rs new file mode 100644 index 000000000..154d01b0c --- /dev/null +++ b/plugins/cmd/ndpgen/testdata/config_client_expected.rs @@ -0,0 +1,135 @@ +// Code generated by ndpgen. DO NOT EDIT. +// +// This file contains client wrappers for the Config host service. +// It is intended for use in Navidrome plugins built with extism-pdk. + +use extism_pdk::*; +use serde::{Deserialize, Serialize}; + +#[derive(Debug, Clone, Serialize)] +#[serde(rename_all = "camelCase")] +struct ConfigGetRequest { + key: String, +} + +#[derive(Debug, Clone, Deserialize)] +#[serde(rename_all = "camelCase")] +struct ConfigGetResponse { + #[serde(default)] + value: String, + #[serde(default)] + exists: bool, + #[serde(default)] + error: Option, +} + +#[derive(Debug, Clone, Serialize)] +#[serde(rename_all = "camelCase")] +struct ConfigSetRequest { + key: String, + value: String, +} + +#[derive(Debug, Clone, Deserialize)] +#[serde(rename_all = "camelCase")] +struct ConfigSetResponse { + #[serde(default)] + error: Option, +} + +#[derive(Debug, Clone, Serialize)] +#[serde(rename_all = "camelCase")] +struct ConfigHasRequest { + key: String, +} + +#[derive(Debug, Clone, Deserialize)] +#[serde(rename_all = "camelCase")] +struct ConfigHasResponse { + #[serde(default)] + exists: bool, + #[serde(default)] + error: Option, +} + +#[host_fn] +extern "ExtismHost" { + fn config_get(input: Json) -> Json; + fn config_set(input: Json) -> Json; + fn config_has(input: Json) -> Json; +} + +/// Calls the config_get host function. +/// +/// # Arguments +/// * `key` - String parameter. +/// +/// # Returns +/// `Some(value)` if found, `None` otherwise. +/// +/// # Errors +/// Returns an error if the host function call fails. +pub fn get(key: &str) -> Result, Error> { + let response = unsafe { + config_get(Json(ConfigGetRequest { + key: key.to_owned(), + }))? + }; + + if let Some(err) = response.0.error { + return Err(Error::msg(err)); + } + + if response.0.exists { + Ok(Some(response.0.value)) + } else { + Ok(None) + } +} + +/// Calls the config_set host function. +/// +/// # Arguments +/// * `key` - String parameter. +/// * `value` - String parameter. +/// +/// # Errors +/// Returns an error if the host function call fails. +pub fn set(key: &str, value: &str) -> Result<(), Error> { + let response = unsafe { + config_set(Json(ConfigSetRequest { + key: key.to_owned(), + value: value.to_owned(), + }))? + }; + + if let Some(err) = response.0.error { + return Err(Error::msg(err)); + } + + Ok(()) +} + +/// Calls the config_has host function. +/// +/// # Arguments +/// * `key` - String parameter. +/// +/// # Returns +/// The exists value. +/// +/// # Errors +/// Returns an error if the host function call fails. +pub fn has(key: &str) -> Result { + let response = unsafe { + config_has(Json(ConfigHasRequest { + key: key.to_owned(), + }))? + }; + + if let Some(err) = response.0.error { + return Err(Error::msg(err)); + } + + Ok(response.0.exists) +} diff --git a/plugins/cmd/ndpgen/testdata/config_service.go.txt b/plugins/cmd/ndpgen/testdata/config_service.go.txt new file mode 100644 index 000000000..5def79302 --- /dev/null +++ b/plugins/cmd/ndpgen/testdata/config_service.go.txt @@ -0,0 +1,15 @@ +package testpkg + +import "context" + +//nd:hostservice name=Config permission=config +type ConfigService interface { + //nd:hostfunc + Get(ctx context.Context, key string) (value string, exists bool, err error) + + //nd:hostfunc + Set(ctx context.Context, key string, value string) error + + //nd:hostfunc + Has(ctx context.Context, key string) (exists bool, err error) +} diff --git a/plugins/examples/discord-rich-presence-rs/src/lib.rs b/plugins/examples/discord-rich-presence-rs/src/lib.rs index 90647ec5b..8bbcb8fb5 100644 --- a/plugins/examples/discord-rich-presence-rs/src/lib.rs +++ b/plugins/examples/discord-rich-presence-rs/src/lib.rs @@ -65,10 +65,9 @@ struct DiscordPlugin; // ============================================================================ fn get_config() -> Result<(String, std::collections::HashMap), Error> { - let (client_id, exists) = config::get(CLIENT_ID_KEY)?; - if !exists || client_id.is_empty() { - return Err(Error::msg("missing clientid in configuration")); - } + let client_id = config::get(CLIENT_ID_KEY)? + .filter(|s| !s.is_empty()) + .ok_or_else(|| Error::msg("missing clientid in configuration"))?; // Get all user keys with the "user." prefix let user_keys = config::keys(USER_KEY_PREFIX)?; @@ -76,8 +75,7 @@ fn get_config() -> Result<(String, std::collections::HashMap), E let mut users = std::collections::HashMap::new(); for key in user_keys { let username = key.strip_prefix(USER_KEY_PREFIX).unwrap_or(&key); - let (token, token_exists) = config::get(&key)?; - if token_exists && !token.is_empty() { + if let Some(token) = config::get(&key)?.filter(|s| !s.is_empty()) { users.insert(username.to_string(), token); } } diff --git a/plugins/examples/discord-rich-presence-rs/src/rpc.rs b/plugins/examples/discord-rich-presence-rs/src/rpc.rs index 2615ba3c2..3de9eff63 100644 --- a/plugins/examples/discord-rich-presence-rs/src/rpc.rs +++ b/plugins/examples/discord-rich-presence-rs/src/rpc.rs @@ -129,8 +129,8 @@ pub fn cleanup_connection(username: &str) { // Try to close the WebSocket connection let conn_key = connection_key(username); - if let Ok((conn_id, exists)) = cache::get_string(&conn_key) { - if exists && !conn_id.is_empty() { + if let Ok(Some(conn_id)) = cache::get_string(&conn_key) { + if !conn_id.is_empty() { if let Err(e) = websocket::close_connection(&conn_id, 1000, "Reconnecting") { trace!("Failed to close WebSocket for user {}: {:?}", username, e); } @@ -264,24 +264,22 @@ pub fn handle_clear_activity_callback(username: &str) -> Result<(), Error> { info!("Clearing activity for user {}", username); let conn_key = connection_key(username); - if let Ok((conn_id, exists)) = cache::get_string(&conn_key) { - if exists && !conn_id.is_empty() { - // Send empty presence to clear activity - let msg = GatewayMessage { - op: PRESENCE_OP_CODE, - d: PresencePayload { - activities: vec![], - since: 0, - status: "dnd".to_string(), - afk: false, - }, - }; + if let Some(conn_id) = cache::get_string(&conn_key)?.filter(|s| !s.is_empty()) { + // Send empty presence to clear activity + let msg = GatewayMessage { + op: PRESENCE_OP_CODE, + d: PresencePayload { + activities: vec![], + since: 0, + status: "dnd".to_string(), + afk: false, + }, + }; - let json = serde_json::to_string(&msg) - .map_err(|e| Error::msg(format!("Failed to serialize message: {}", e)))?; + let json = serde_json::to_string(&msg) + .map_err(|e| Error::msg(format!("Failed to serialize message: {}", e)))?; - websocket::send_text(&conn_id, &json)?; - } + websocket::send_text(&conn_id, &json)?; } Ok(()) @@ -298,15 +296,13 @@ pub fn disconnect(username: &str) -> Result<(), Error> { // Close the WebSocket connection let conn_key = connection_key(username); - if let Ok((conn_id, exists)) = cache::get_string(&conn_key) { - if exists && !conn_id.is_empty() { - if let Err(e) = websocket::close_connection(&conn_id, 1000, "Navidrome disconnect") { - warn!("Failed to close WebSocket connection: {:?}", e); - } - // Clean up reverse mapping - let reverse_key = format!("discord.reverse.{}", conn_id); - let _ = cache::remove(&reverse_key); + if let Some(conn_id) = cache::get_string(&conn_key)?.filter(|s| !s.is_empty()) { + if let Err(e) = websocket::close_connection(&conn_id, 1000, "Navidrome disconnect") { + warn!("Failed to close WebSocket connection: {:?}", e); } + // Clean up reverse mapping + let reverse_key = format!("discord.reverse.{}", conn_id); + let _ = cache::remove(&reverse_key); } // Clean up cache entries @@ -324,10 +320,9 @@ pub fn send_activity( mut activity: Activity, ) -> Result<(), Error> { let conn_key = connection_key(username); - let (conn_id, exists) = cache::get_string(&conn_key)?; - if !exists || conn_id.is_empty() { - return Err(Error::msg("Not connected to Discord")); - } + let conn_id = cache::get_string(&conn_key)? + .filter(|s| !s.is_empty()) + .ok_or_else(|| Error::msg("Not connected to Discord"))?; // Process image URL activity.assets.large_image = process_image(&activity.assets.large_image, client_id, token)?; @@ -361,12 +356,7 @@ fn find_username_for_connection(connection_id: &str) -> Result, E // The connection ID is stored as cache value, so we need to scan for it // Since we can't iterate cache, we'll use a workaround with a reverse mapping let reverse_key = format!("discord.reverse.{}", connection_id); - if let Ok((username, exists)) = cache::get_string(&reverse_key) { - if exists && !username.is_empty() { - return Ok(Some(username)); - } - } - Ok(None) + Ok(cache::get_string(&reverse_key)?.filter(|s| !s.is_empty())) } fn get_discord_gateway() -> Result { @@ -394,16 +384,14 @@ fn identify(username: &str) -> Result<(), Error> { info!("Identifying with Discord for user {}", username); let conn_key = connection_key(username); - let (conn_id, exists) = cache::get_string(&conn_key)?; - if !exists || conn_id.is_empty() { - return Err(Error::msg("No connection found")); - } + let conn_id = cache::get_string(&conn_key)? + .filter(|s| !s.is_empty()) + .ok_or_else(|| Error::msg("No connection found"))?; let token_k = token_key(username); - let (token, exists) = cache::get_string(&token_k)?; - if !exists || token.is_empty() { - return Err(Error::msg("No token found")); - } + let token = cache::get_string(&token_k)? + .filter(|s| !s.is_empty()) + .ok_or_else(|| Error::msg("No token found"))?; // Store reverse mapping for connection -> username let reverse_key = format!("discord.reverse.{}", conn_id); @@ -440,19 +428,14 @@ fn identify(username: &str) -> Result<(), Error> { fn send_heartbeat(username: &str) -> Result<(), Error> { let conn_key = connection_key(username); - let (conn_id, exists) = cache::get_string(&conn_key)?; - if !exists || conn_id.is_empty() { - return Err(Error::msg("No connection found")); - } + let conn_id = cache::get_string(&conn_key)? + .filter(|s| !s.is_empty()) + .ok_or_else(|| Error::msg("No connection found"))?; // Get sequence number let seq_key = sequence_key(username); - let (seq_str, exists) = cache::get_string(&seq_key)?; - let seq: Option = if exists && !seq_str.is_empty() { - seq_str.parse().ok() - } else { - None - }; + let seq: Option = cache::get_string(&seq_key)? + .and_then(|s| s.parse().ok()); // Send heartbeat let msg = GatewayMessage { @@ -493,10 +476,8 @@ fn process_image_inner( // Check cache let cache_key = format!("discord.image.{:x}", md5_hash(url)); - if let Ok((cached, exists)) = cache::get_string(&cache_key) { - if exists && !cached.is_empty() { - return Ok(cached); - } + if let Some(cached) = cache::get_string(&cache_key)?.filter(|s| !s.is_empty()) { + return Ok(cached); } // Process via Discord API diff --git a/plugins/pdk/rust/nd-pdk-host/src/nd_host_cache.rs b/plugins/pdk/rust/nd-pdk-host/src/nd_host_cache.rs index d9baa7c56..1f3d69295 100644 --- a/plugins/pdk/rust/nd-pdk-host/src/nd_host_cache.rs +++ b/plugins/pdk/rust/nd-pdk-host/src/nd_host_cache.rs @@ -220,11 +220,11 @@ pub fn set_string(key: &str, value: &str, ttl_seconds: i64) -> Result<(), Error> /// * `key` - String parameter. /// /// # Returns -/// A tuple of (value, exists). +/// `Some(value)` if found, `None` otherwise. /// /// # Errors /// Returns an error if the host function call fails. -pub fn get_string(key: &str) -> Result<(String, bool), Error> { +pub fn get_string(key: &str) -> Result, Error> { let response = unsafe { cache_getstring(Json(CacheGetStringRequest { key: key.to_owned(), @@ -235,7 +235,11 @@ pub fn get_string(key: &str) -> Result<(String, bool), Error> { return Err(Error::msg(err)); } - Ok((response.0.value, response.0.exists)) + if response.0.exists { + Ok(Some(response.0.value)) + } else { + Ok(None) + } } /// SetInt stores an integer value in the cache. @@ -282,11 +286,11 @@ pub fn set_int(key: &str, value: i64, ttl_seconds: i64) -> Result<(), Error> { /// * `key` - String parameter. /// /// # Returns -/// A tuple of (value, exists). +/// `Some(value)` if found, `None` otherwise. /// /// # Errors /// Returns an error if the host function call fails. -pub fn get_int(key: &str) -> Result<(i64, bool), Error> { +pub fn get_int(key: &str) -> Result, Error> { let response = unsafe { cache_getint(Json(CacheGetIntRequest { key: key.to_owned(), @@ -297,7 +301,11 @@ pub fn get_int(key: &str) -> Result<(i64, bool), Error> { return Err(Error::msg(err)); } - Ok((response.0.value, response.0.exists)) + if response.0.exists { + Ok(Some(response.0.value)) + } else { + Ok(None) + } } /// SetFloat stores a float value in the cache. @@ -344,11 +352,11 @@ pub fn set_float(key: &str, value: f64, ttl_seconds: i64) -> Result<(), Error> { /// * `key` - String parameter. /// /// # Returns -/// A tuple of (value, exists). +/// `Some(value)` if found, `None` otherwise. /// /// # Errors /// Returns an error if the host function call fails. -pub fn get_float(key: &str) -> Result<(f64, bool), Error> { +pub fn get_float(key: &str) -> Result, Error> { let response = unsafe { cache_getfloat(Json(CacheGetFloatRequest { key: key.to_owned(), @@ -359,7 +367,11 @@ pub fn get_float(key: &str) -> Result<(f64, bool), Error> { return Err(Error::msg(err)); } - Ok((response.0.value, response.0.exists)) + if response.0.exists { + Ok(Some(response.0.value)) + } else { + Ok(None) + } } /// SetBytes stores a byte slice in the cache. @@ -406,11 +418,11 @@ pub fn set_bytes(key: &str, value: Vec, ttl_seconds: i64) -> Result<(), Erro /// * `key` - String parameter. /// /// # Returns -/// A tuple of (value, exists). +/// `Some(value)` if found, `None` otherwise. /// /// # Errors /// Returns an error if the host function call fails. -pub fn get_bytes(key: &str) -> Result<(Vec, bool), Error> { +pub fn get_bytes(key: &str) -> Result>, Error> { let response = unsafe { cache_getbytes(Json(CacheGetBytesRequest { key: key.to_owned(), @@ -421,7 +433,11 @@ pub fn get_bytes(key: &str) -> Result<(Vec, bool), Error> { return Err(Error::msg(err)); } - Ok((response.0.value, response.0.exists)) + if response.0.exists { + Ok(Some(response.0.value)) + } else { + Ok(None) + } } /// Has checks if a key exists in the cache. diff --git a/plugins/pdk/rust/nd-pdk-host/src/nd_host_config.rs b/plugins/pdk/rust/nd-pdk-host/src/nd_host_config.rs index ebb4ffb8b..effd5923e 100644 --- a/plugins/pdk/rust/nd-pdk-host/src/nd_host_config.rs +++ b/plugins/pdk/rust/nd-pdk-host/src/nd_host_config.rs @@ -67,18 +67,22 @@ extern "ExtismHost" { /// * `key` - String parameter. /// /// # Returns -/// A tuple of (value, exists). +/// `Some(value)` if found, `None` otherwise. /// /// # Errors /// Returns an error if the host function call fails. -pub fn get(key: &str) -> Result<(String, bool), Error> { +pub fn get(key: &str) -> Result, Error> { let response = unsafe { config_get(Json(ConfigGetRequest { key: key.to_owned(), }))? }; - Ok((response.0.value, response.0.exists)) + if response.0.exists { + Ok(Some(response.0.value)) + } else { + Ok(None) + } } /// GetInt retrieves a configuration value as an integer. @@ -93,18 +97,22 @@ pub fn get(key: &str) -> Result<(String, bool), Error> { /// * `key` - String parameter. /// /// # Returns -/// A tuple of (value, exists). +/// `Some(value)` if found, `None` otherwise. /// /// # Errors /// Returns an error if the host function call fails. -pub fn get_int(key: &str) -> Result<(i64, bool), Error> { +pub fn get_int(key: &str) -> Result, Error> { let response = unsafe { config_getint(Json(ConfigGetIntRequest { key: key.to_owned(), }))? }; - Ok((response.0.value, response.0.exists)) + if response.0.exists { + Ok(Some(response.0.value)) + } else { + Ok(None) + } } /// Keys returns configuration keys matching the given prefix. diff --git a/plugins/pdk/rust/nd-pdk-host/src/nd_host_kvstore.rs b/plugins/pdk/rust/nd-pdk-host/src/nd_host_kvstore.rs index 6e5b79f2b..5048f369c 100644 --- a/plugins/pdk/rust/nd-pdk-host/src/nd_host_kvstore.rs +++ b/plugins/pdk/rust/nd-pdk-host/src/nd_host_kvstore.rs @@ -139,11 +139,11 @@ pub fn set(key: &str, value: Vec) -> Result<(), Error> { /// * `key` - String parameter. /// /// # Returns -/// A tuple of (value, exists). +/// `Some(value)` if found, `None` otherwise. /// /// # Errors /// Returns an error if the host function call fails. -pub fn get(key: &str) -> Result<(Vec, bool), Error> { +pub fn get(key: &str) -> Result>, Error> { let response = unsafe { kvstore_get(Json(KVStoreGetRequest { key: key.to_owned(), @@ -154,7 +154,11 @@ pub fn get(key: &str) -> Result<(Vec, bool), Error> { return Err(Error::msg(err)); } - Ok((response.0.value, response.0.exists)) + if response.0.exists { + Ok(Some(response.0.value)) + } else { + Ok(None) + } } /// Delete removes a value from storage.