From 185e3b562ab078d74279f75f588207cc2e495802 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Wed, 11 Mar 2026 17:23:51 +0300 Subject: [PATCH] treewide: cleanup Signed-off-by: NotAShelf Change-Id: Ia01590cdeed872cc8ebd16f6ca95f3cc6a6a6964 --- crates/pinakes-core/src/export.rs | 4 +- crates/pinakes-core/src/metadata/mod.rs | 6 +- crates/pinakes-core/src/plugin/mod.rs | 42 +----- crates/pinakes-core/src/plugin/registry.rs | 2 +- crates/pinakes-core/src/storage/sqlite.rs | 10 +- crates/pinakes-core/tests/integration.rs | 20 ++- crates/pinakes-plugin-api/src/manifest.rs | 12 +- crates/pinakes-plugin-api/src/ui_schema.rs | 5 - crates/pinakes-plugin-api/src/validation.rs | 4 +- crates/pinakes-server/src/dto/media.rs | 13 +- crates/pinakes-server/src/routes/books.rs | 24 ++-- crates/pinakes-server/src/routes/plugins.rs | 10 +- crates/pinakes-ui/src/plugin_ui/actions.rs | 5 +- crates/pinakes-ui/src/plugin_ui/data.rs | 152 +++++++++++++------- crates/pinakes-ui/src/plugin_ui/registry.rs | 50 +++---- crates/pinakes-ui/src/plugin_ui/renderer.rs | 118 +++++++-------- 16 files changed, 258 insertions(+), 219 deletions(-) diff --git a/crates/pinakes-core/src/export.rs b/crates/pinakes-core/src/export.rs index c5f3ce5..f50ec38 100644 --- a/crates/pinakes-core/src/export.rs +++ b/crates/pinakes-core/src/export.rs @@ -42,7 +42,9 @@ pub async fn export_library( match format { ExportFormat::Json => { let json = serde_json::to_string_pretty(&items).map_err(|e| { - crate::error::PinakesError::Serialization(format!("json serialize: {e}")) + crate::error::PinakesError::Serialization(format!( + "json serialize: {e}" + )) })?; std::fs::write(destination, json)?; }, diff --git a/crates/pinakes-core/src/metadata/mod.rs b/crates/pinakes-core/src/metadata/mod.rs index 8fcc8b7..0ea4da3 100644 --- a/crates/pinakes-core/src/metadata/mod.rs +++ b/crates/pinakes-core/src/metadata/mod.rs @@ -6,11 +6,7 @@ pub mod video; use std::{collections::HashMap, path::Path}; -use crate::{ - error::Result, - media_type::MediaType, - model::BookMetadata, -}; +use crate::{error::Result, media_type::MediaType, model::BookMetadata}; #[derive(Debug, Clone, Default)] pub struct ExtractedMetadata { diff --git a/crates/pinakes-core/src/plugin/mod.rs b/crates/pinakes-core/src/plugin/mod.rs index fc77aed..e43e930 100644 --- a/crates/pinakes-core/src/plugin/mod.rs +++ b/crates/pinakes-core/src/plugin/mod.rs @@ -607,42 +607,12 @@ impl PluginManager { pub async fn list_ui_pages( &self, ) -> Vec<(String, pinakes_plugin_api::UiPage)> { - let registry = self.registry.read().await; - let mut pages = Vec::new(); - for plugin in registry.list_all() { - if !plugin.enabled { - continue; - } - let plugin_dir = plugin - .manifest_path - .as_ref() - .and_then(|p| p.parent()) - .map(std::path::Path::to_path_buf); - let Some(plugin_dir) = plugin_dir else { - // No manifest path; serve only inline pages. - for entry in &plugin.manifest.ui.pages { - if let pinakes_plugin_api::manifest::UiPageEntry::Inline(page) = entry - { - pages.push((plugin.id.clone(), (**page).clone())); - } - } - continue; - }; - match plugin.manifest.load_ui_pages(&plugin_dir) { - Ok(loaded) => { - for page in loaded { - pages.push((plugin.id.clone(), page)); - } - }, - Err(e) => { - tracing::warn!( - "Failed to load UI pages for plugin '{}': {e}", - plugin.id - ); - }, - } - } - pages + self + .list_ui_pages_with_endpoints() + .await + .into_iter() + .map(|(id, page, _)| (id, page)) + .collect() } /// List all UI pages provided by loaded plugins, including each plugin's diff --git a/crates/pinakes-core/src/plugin/registry.rs b/crates/pinakes-core/src/plugin/registry.rs index 6e9219e..a773164 100644 --- a/crates/pinakes-core/src/plugin/registry.rs +++ b/crates/pinakes-core/src/plugin/registry.rs @@ -131,7 +131,7 @@ impl PluginRegistry { self .plugins .values() - .filter(|p| p.manifest.plugin.kind.contains(&kind.to_string())) + .filter(|p| p.manifest.plugin.kind.iter().any(|k| k == kind)) .collect() } diff --git a/crates/pinakes-core/src/storage/sqlite.rs b/crates/pinakes-core/src/storage/sqlite.rs index cfe08c9..9bd117a 100644 --- a/crates/pinakes-core/src/storage/sqlite.rs +++ b/crates/pinakes-core/src/storage/sqlite.rs @@ -1888,10 +1888,12 @@ impl StorageBackend for SqliteBackend { .unchecked_transaction() .map_err(crate::error::db_ctx("batch_tag_media", &ctx))?; // Prepare statement once for reuse - let mut stmt = tx.prepare_cached( - "INSERT OR IGNORE INTO media_tags (media_id, tag_id) VALUES (?1, ?2)", - ) - .map_err(crate::error::db_ctx("batch_tag_media", &ctx))?; + let mut stmt = tx + .prepare_cached( + "INSERT OR IGNORE INTO media_tags (media_id, tag_id) VALUES (?1, \ + ?2)", + ) + .map_err(crate::error::db_ctx("batch_tag_media", &ctx))?; let mut count = 0u64; for mid in &media_ids { for tid in &tag_ids { diff --git a/crates/pinakes-core/tests/integration.rs b/crates/pinakes-core/tests/integration.rs index 8cc4d4d..927d012 100644 --- a/crates/pinakes-core/tests/integration.rs +++ b/crates/pinakes-core/tests/integration.rs @@ -962,7 +962,15 @@ async fn test_batch_update_media_single_field() { storage.insert_media(&item).await.unwrap(); let count = storage - .batch_update_media(&[item.id], Some("Bulk Title"), None, None, None, None, None) + .batch_update_media( + &[item.id], + Some("Bulk Title"), + None, + None, + None, + None, + None, + ) .await .unwrap(); assert_eq!(count, 1); @@ -1021,7 +1029,15 @@ async fn test_batch_update_media_subset_of_items() { // Only update item_a. let count = storage - .batch_update_media(&[item_a.id], Some("Only A"), None, None, None, None, None) + .batch_update_media( + &[item_a.id], + Some("Only A"), + None, + None, + None, + None, + None, + ) .await .unwrap(); assert_eq!(count, 1); diff --git a/crates/pinakes-plugin-api/src/manifest.rs b/crates/pinakes-plugin-api/src/manifest.rs index 340dc24..a7229c0 100644 --- a/crates/pinakes-plugin-api/src/manifest.rs +++ b/crates/pinakes-plugin-api/src/manifest.rs @@ -759,11 +759,19 @@ wasm = "plugin.wasm" let manifest = PluginManifest::parse_str(toml).unwrap(); assert_eq!( - manifest.ui.theme_extensions.get("--accent-color").map(String::as_str), + manifest + .ui + .theme_extensions + .get("--accent-color") + .map(String::as_str), Some("#ff6b6b") ); assert_eq!( - manifest.ui.theme_extensions.get("--sidebar-width").map(String::as_str), + manifest + .ui + .theme_extensions + .get("--sidebar-width") + .map(String::as_str), Some("280px") ); } diff --git a/crates/pinakes-plugin-api/src/ui_schema.rs b/crates/pinakes-plugin-api/src/ui_schema.rs index 02ec93d..f73a5ba 100644 --- a/crates/pinakes-plugin-api/src/ui_schema.rs +++ b/crates/pinakes-plugin-api/src/ui_schema.rs @@ -275,11 +275,6 @@ impl UiWidget { /// /// Returns `SchemaError::ValidationError` if validation fails pub fn validate(&self) -> SchemaResult<()> { - if self.id.is_empty() { - return Err(SchemaError::ValidationError( - "Widget id cannot be empty".to_string(), - )); - } if self.target.is_empty() { return Err(SchemaError::ValidationError( "Widget target cannot be empty".to_string(), diff --git a/crates/pinakes-plugin-api/src/validation.rs b/crates/pinakes-plugin-api/src/validation.rs index fc060f2..b7bb445 100644 --- a/crates/pinakes-plugin-api/src/validation.rs +++ b/crates/pinakes-plugin-api/src/validation.rs @@ -331,7 +331,9 @@ impl SchemaValidator { pub(crate) fn is_reserved_route(route: &str) -> bool { RESERVED_ROUTES.iter().any(|reserved| { - route == *reserved || route.starts_with(&format!("{reserved}/")) + route == *reserved + || (route.starts_with(reserved) + && route.as_bytes().get(reserved.len()) == Some(&b'/')) }) } } diff --git a/crates/pinakes-server/src/dto/media.rs b/crates/pinakes-server/src/dto/media.rs index 231bbb9..dc1a155 100644 --- a/crates/pinakes-server/src/dto/media.rs +++ b/crates/pinakes-server/src/dto/media.rs @@ -15,7 +15,8 @@ pub fn relativize_path(full_path: &Path, roots: &[PathBuf]) -> String { let mut best: Option<&PathBuf> = None; for root in roots { if full_path.starts_with(root) { - let is_longer = best.map_or(true, |b| root.components().count() > b.components().count()); + let is_longer = best + .map_or(true, |b| root.components().count() > b.components().count()); if is_longer { best = Some(root); } @@ -268,10 +269,7 @@ impl MediaResponse { /// matching root prefix from the path before serialization. Pass the /// configured root directories so that clients receive a relative path /// (e.g. `"Music/song.mp3"`) rather than a full server filesystem path. - pub fn new( - item: pinakes_core::model::MediaItem, - roots: &[PathBuf], - ) -> Self { + pub fn new(item: pinakes_core::model::MediaItem, roots: &[PathBuf]) -> Self { Self { id: item.id.0.to_string(), path: relativize_path(&item.path, roots), @@ -358,10 +356,7 @@ mod tests { #[test] fn relativize_path_empty_roots_returns_full() { let path = Path::new("/home/user/music/song.mp3"); - assert_eq!( - relativize_path(path, &[]), - "/home/user/music/song.mp3" - ); + assert_eq!(relativize_path(path, &[]), "/home/user/music/song.mp3"); } #[test] diff --git a/crates/pinakes-server/src/routes/books.rs b/crates/pinakes-server/src/routes/books.rs index 7ae042f..9e3a0bc 100644 --- a/crates/pinakes-server/src/routes/books.rs +++ b/crates/pinakes-server/src/routes/books.rs @@ -195,8 +195,10 @@ pub async fn list_books( .await?; let roots = state.config.read().await.directories.roots.clone(); - let response: Vec = - items.into_iter().map(|item| MediaResponse::new(item, &roots)).collect(); + let response: Vec = items + .into_iter() + .map(|item| MediaResponse::new(item, &roots)) + .collect(); Ok(Json(response)) } @@ -225,8 +227,10 @@ pub async fn get_series_books( ) -> Result { let items = state.storage.get_series_books(&series_name).await?; let roots = state.config.read().await.directories.roots.clone(); - let response: Vec = - items.into_iter().map(|item| MediaResponse::new(item, &roots)).collect(); + let response: Vec = items + .into_iter() + .map(|item| MediaResponse::new(item, &roots)) + .collect(); Ok(Json(response)) } @@ -261,8 +265,10 @@ pub async fn get_author_books( .await?; let roots = state.config.read().await.directories.roots.clone(); - let response: Vec = - items.into_iter().map(|item| MediaResponse::new(item, &roots)).collect(); + let response: Vec = items + .into_iter() + .map(|item| MediaResponse::new(item, &roots)) + .collect(); Ok(Json(response)) } @@ -321,8 +327,10 @@ pub async fn get_reading_list( .await?; let roots = state.config.read().await.directories.roots.clone(); - let response: Vec = - items.into_iter().map(|item| MediaResponse::new(item, &roots)).collect(); + let response: Vec = items + .into_iter() + .map(|item| MediaResponse::new(item, &roots)) + .collect(); Ok(Json(response)) } diff --git a/crates/pinakes-server/src/routes/plugins.rs b/crates/pinakes-server/src/routes/plugins.rs index 5653d23..6748282 100644 --- a/crates/pinakes-server/src/routes/plugins.rs +++ b/crates/pinakes-server/src/routes/plugins.rs @@ -153,10 +153,12 @@ pub async fn list_plugin_ui_pages( let pages = plugin_manager.list_ui_pages_with_endpoints().await; let entries = pages .into_iter() - .map(|(plugin_id, page, allowed_endpoints)| PluginUiPageEntry { - plugin_id, - page, - allowed_endpoints, + .map(|(plugin_id, page, allowed_endpoints)| { + PluginUiPageEntry { + plugin_id, + page, + allowed_endpoints, + } }) .collect(); Ok(Json(entries)) diff --git a/crates/pinakes-ui/src/plugin_ui/actions.rs b/crates/pinakes-ui/src/plugin_ui/actions.rs index 8c9eb64..1c6f553 100644 --- a/crates/pinakes-ui/src/plugin_ui/actions.rs +++ b/crates/pinakes-ui/src/plugin_ui/actions.rs @@ -96,14 +96,11 @@ async fn execute_inline_action( action: &ActionDefinition, form_data: Option<&serde_json::Value>, ) -> Result { - // Build URL from path - let url = action.path.clone(); - // Merge action params with form data into query string for GET, body for // others let method = to_reqwest_method(&action.method); - let mut request = client.raw_request(method.clone(), &url); + let mut request = client.raw_request(method.clone(), &action.path); // For GET, merge params into query string; for mutating methods, send as // JSON body diff --git a/crates/pinakes-ui/src/plugin_ui/data.rs b/crates/pinakes-ui/src/plugin_ui/data.rs index d3f42dc..2244fe6 100644 --- a/crates/pinakes-ui/src/plugin_ui/data.rs +++ b/crates/pinakes-ui/src/plugin_ui/data.rs @@ -2,7 +2,10 @@ //! //! Provides data fetching and caching for plugin data sources. -use std::{collections::HashMap, time::Duration}; +use std::{ + collections::{HashMap, HashSet}, + time::Duration, +}; use dioxus::prelude::*; use dioxus_core::Task; @@ -15,7 +18,7 @@ use crate::client::ApiClient; #[derive(Debug, Clone, Default, PartialEq, Eq)] pub struct PluginPageData { data: HashMap, - loading: HashMap, + loading: HashSet, errors: HashMap, } @@ -29,13 +32,13 @@ impl PluginPageData { /// Check if a source is currently loading #[must_use] pub fn is_loading(&self, source: &str) -> bool { - self.loading.get(source).copied().unwrap_or(false) + self.loading.contains(source) } /// Get error for a specific source #[must_use] - pub fn error(&self, source: &str) -> Option<&String> { - self.errors.get(source) + pub fn error(&self, source: &str) -> Option<&str> { + self.errors.get(source).map(String::as_str) } /// Check if there is data for a specific source @@ -52,7 +55,7 @@ impl PluginPageData { /// Set loading state for a source pub fn set_loading(&mut self, source: &str, loading: bool) { if loading { - self.loading.insert(source.to_string(), true); + self.loading.insert(source.to_string()); self.errors.remove(source); } else { self.loading.remove(source); @@ -161,9 +164,10 @@ async fn fetch_endpoint( /// /// Endpoint sources are deduplicated by `(path, method, params)`: if multiple /// sources share the same triplet, a single HTTP request is made and the raw -/// response is shared, with each source's own `transform` applied independently. -/// All unique Endpoint and Static sources are fetched concurrently. Transform -/// sources are applied after, in iteration order, against the full result set. +/// response is shared, with each source's own `transform` applied +/// independently. All unique Endpoint and Static sources are fetched +/// concurrently. Transform sources are applied after, in iteration order, +/// against the full result set. /// /// # Errors /// @@ -263,8 +267,15 @@ pub async fn fetch_page_data( .. } => { let empty_ctx = serde_json::json!({}); - fetch_endpoint(&client, path, method.clone(), params, &empty_ctx, &allowed) - .await? + fetch_endpoint( + &client, + path, + method.clone(), + params, + &empty_ctx, + &allowed, + ) + .await? }, DataSource::Static { value } => value.clone(), DataSource::Transform { .. } => unreachable!(), @@ -296,21 +307,60 @@ pub async fn fetch_page_data( } } - // Process Transform sources sequentially; they reference results above. - for (name, source) in data_sources { - if let DataSource::Transform { - source_name, - expression, - } = source - { - let ctx = serde_json::Value::Object( - results - .iter() - .map(|(k, v): (&String, &serde_json::Value)| (k.clone(), v.clone())) - .collect(), + // Process Transform sources in dependency order. HashMap iteration order is + // non-deterministic, so a Transform referencing another Transform could see + // null if the upstream was not yet resolved. The pending loop below defers + // any Transform whose upstream is not yet in results, making progress on + // each pass until all are resolved. UiPage::validate guarantees no cycles, + // so the loop always terminates. + let mut pending: Vec<(&String, &String, &Expression)> = data_sources + .iter() + .filter_map(|(name, source)| { + match source { + DataSource::Transform { + source_name, + expression, + } => Some((name, source_name, expression)), + _ => None, + } + }) + .collect(); + + while !pending.is_empty() { + let prev_len = pending.len(); + let mut i = 0; + while i < pending.len() { + let (name, source_name, expression) = pending[i]; + if results.contains_key(source_name.as_str()) { + let ctx = serde_json::Value::Object( + results + .iter() + .map(|(k, v)| (k.clone(), v.clone())) + .collect(), + ); + results.insert(name.clone(), evaluate_expression(expression, &ctx)); + pending.swap_remove(i); + } else { + i += 1; + } + } + if pending.len() == prev_len { + // No progress: upstream source is missing (should be caught by + // UiPage::validate, but handled defensively here). + tracing::warn!( + "plugin transform dependency unresolvable; processing remaining in \ + iteration order" ); - let _ = source_name; // accessible in ctx by its key - results.insert(name.clone(), evaluate_expression(expression, &ctx)); + for (name, _, expression) in pending { + let ctx = serde_json::Value::Object( + results + .iter() + .map(|(k, v)| (k.clone(), v.clone())) + .collect(), + ); + results.insert(name.clone(), evaluate_expression(expression, &ctx)); + } + break; } } @@ -446,7 +496,7 @@ mod tests { // Test error state data.set_error("error".to_string(), "oops".to_string()); - assert_eq!(data.error("error"), Some(&"oops".to_string())); + assert_eq!(data.error("error"), Some("oops")); } #[test] @@ -522,7 +572,9 @@ mod tests { value: serde_json::json!(true), }); - let results = super::fetch_page_data(&client, &sources, &[]).await.unwrap(); + let results = super::fetch_page_data(&client, &sources, &[]) + .await + .unwrap(); assert_eq!(results["nums"], serde_json::json!([1, 2, 3])); assert_eq!(results["flag"], serde_json::json!(true)); } @@ -544,7 +596,9 @@ mod tests { value: serde_json::json!({"ok": true}), }); - let results = super::fetch_page_data(&client, &sources, &[]).await.unwrap(); + let results = super::fetch_page_data(&client, &sources, &[]) + .await + .unwrap(); assert_eq!(results["raw"], serde_json::json!({"ok": true})); // derived should return the value of "raw" from context assert_eq!(results["derived"], serde_json::json!({"ok": true})); @@ -566,13 +620,13 @@ mod tests { expression: Expression::Literal(serde_json::json!("constant")), }); - let results = super::fetch_page_data(&client, &sources, &[]).await.unwrap(); + let results = super::fetch_page_data(&client, &sources, &[]) + .await + .unwrap(); // A Literal expression returns the literal value, not the source data assert_eq!(results["derived"], serde_json::json!("constant")); } - // Test: multiple Static sources with the same value each get their own - // result; dedup logic does not collapse distinct-named Static sources. #[tokio::test] async fn test_fetch_page_data_deduplicates_identical_endpoints() { use pinakes_plugin_api::DataSource; @@ -589,18 +643,18 @@ mod tests { sources.insert("b".to_string(), DataSource::Static { value: serde_json::json!(1), }); - let results = super::fetch_page_data(&client, &sources, &[]).await.unwrap(); + let results = super::fetch_page_data(&client, &sources, &[]) + .await + .unwrap(); assert_eq!(results["a"], serde_json::json!(1)); assert_eq!(results["b"], serde_json::json!(1)); assert_eq!(results.len(), 2); } - // Test: Endpoint sources with identical (path, method, params) but different - // transform expressions each get a correctly transformed result. Because the - // test runs without a real server the path is checked against the allowlist - // before any network call, so we verify the dedup key grouping through the - // allowlist rejection path: both names should see the same error message, - // proving they were grouped and the single rejection propagates to all names. + // Verifies that endpoint sources with identical (path, method, params) are + // deduplicated correctly. Because there is no real server, the allowlist + // rejection fires before any network call; both names seeing the same error + // proves they were grouped and that the single rejection propagated to all. #[tokio::test] async fn test_dedup_groups_endpoint_sources_with_same_key() { use pinakes_plugin_api::{DataSource, Expression, HttpMethod}; @@ -640,14 +694,12 @@ mod tests { ); } - // Test: multiple Transform sources referencing the same upstream Static source - // with different expressions each receive their independently transformed - // result. This exercises the transform fan-out behavior that mirrors what - // the Endpoint dedup group does after a single shared HTTP request completes: - // each member of a group applies its own transform to the shared raw value. + // Verifies the transform fan-out behavior: each member of a dedup group + // applies its own transform to the shared raw value independently. This + // mirrors what Endpoint dedup does after a single shared HTTP request. // - // Testing the Endpoint dedup success path with real per-member transforms - // requires a mock HTTP server and belongs in an integration test. + // Testing Endpoint dedup with real per-member transforms requires a mock HTTP + // server and belongs in an integration test. #[tokio::test] async fn test_dedup_transform_applied_per_source() { use pinakes_plugin_api::{DataSource, Expression}; @@ -670,8 +722,9 @@ mod tests { expression: Expression::Path("raw_data.name".to_string()), }); - let results = - super::fetch_page_data(&client, &sources, &[]).await.unwrap(); + let results = super::fetch_page_data(&client, &sources, &[]) + .await + .unwrap(); assert_eq!( results["raw_data"], serde_json::json!({"count": 42, "name": "test"}) @@ -681,8 +734,6 @@ mod tests { assert_eq!(results.len(), 3); } - // Test: fetch_page_data returns an error when the endpoint data source path is - // not listed in the allowed_endpoints slice. #[tokio::test] async fn test_endpoint_blocked_when_not_in_allowlist() { use pinakes_plugin_api::{DataSource, HttpMethod}; @@ -705,7 +756,8 @@ mod tests { assert!( result.is_err(), - "fetch_page_data must return Err when endpoint is not in allowed_endpoints" + "fetch_page_data must return Err when endpoint is not in \ + allowed_endpoints" ); let msg = result.unwrap_err(); assert!( diff --git a/crates/pinakes-ui/src/plugin_ui/registry.rs b/crates/pinakes-ui/src/plugin_ui/registry.rs index fb3d1b6..4ad2b5c 100644 --- a/crates/pinakes-ui/src/plugin_ui/registry.rs +++ b/crates/pinakes-ui/src/plugin_ui/registry.rs @@ -35,13 +35,6 @@ pub struct PluginPage { pub allowed_endpoints: Vec, } -impl PluginPage { - /// The canonical route for this page, taken directly from the page schema. - pub fn full_route(&self) -> String { - self.page.route.clone() - } -} - /// Registry of all plugin-provided UI pages and widgets /// /// This is typically stored as a signal in the Dioxus tree. @@ -109,14 +102,11 @@ impl PluginRegistry { ); return; } - self.pages.insert( - (plugin_id.clone(), page_id), - PluginPage { - plugin_id, - page, - allowed_endpoints, - }, - ); + self.pages.insert((plugin_id.clone(), page_id), PluginPage { + plugin_id, + page, + allowed_endpoints, + }); } /// Get a specific page by plugin ID and page ID @@ -179,7 +169,7 @@ impl PluginRegistry { self .pages .values() - .map(|p| (p.plugin_id.clone(), p.page.id.clone(), p.full_route())) + .map(|p| (p.plugin_id.clone(), p.page.id.clone(), p.page.route.clone())) .collect() } @@ -207,7 +197,9 @@ impl PluginRegistry { } match self.client.get_plugin_ui_theme_extensions().await { Ok(vars) => tmp.theme_vars = vars, - Err(e) => tracing::warn!("Failed to refresh plugin theme extensions: {e}"), + Err(e) => { + tracing::warn!("Failed to refresh plugin theme extensions: {e}") + }, } // Atomic swap: no window where the registry appears empty. @@ -367,7 +359,7 @@ mod tests { } #[test] - fn test_page_full_route() { + fn test_page_route() { let client = ApiClient::default(); let mut registry = PluginRegistry::new(client); registry.register_page( @@ -376,9 +368,7 @@ mod tests { vec![], ); let plugin_page = registry.get_page("my-plugin", "demo").unwrap(); - // full_route() returns page.route directly; create_test_page sets it as - // "/plugins/test/{id}" - assert_eq!(plugin_page.full_route(), "/plugins/test/demo"); + assert_eq!(plugin_page.page.route, "/plugins/test/demo"); } #[test] @@ -418,8 +408,16 @@ mod tests { fn test_all_pages_returns_references() { let client = ApiClient::default(); let mut registry = PluginRegistry::new(client); - registry.register_page("p1".to_string(), create_test_page("a", "A"), vec![]); - registry.register_page("p2".to_string(), create_test_page("b", "B"), vec![]); + registry.register_page( + "p1".to_string(), + create_test_page("a", "A"), + vec![], + ); + registry.register_page( + "p2".to_string(), + create_test_page("b", "B"), + vec![], + ); let pages = registry.all_pages(); assert_eq!(pages.len(), 2); @@ -536,7 +534,11 @@ mod tests { assert_eq!(registry.all_pages().len(), 0); // Valid page; should still register fine - registry.register_page("p".to_string(), create_test_page("good", "Good"), vec![]); + registry.register_page( + "p".to_string(), + create_test_page("good", "Good"), + vec![], + ); assert_eq!(registry.all_pages().len(), 1); } diff --git a/crates/pinakes-ui/src/plugin_ui/renderer.rs b/crates/pinakes-ui/src/plugin_ui/renderer.rs index e8372fd..0272e6b 100644 --- a/crates/pinakes-ui/src/plugin_ui/renderer.rs +++ b/crates/pinakes-ui/src/plugin_ui/renderer.rs @@ -110,8 +110,12 @@ pub fn PluginViewRenderer(props: PluginViewProps) -> Element { modal, local_state, }; - let page_data = - use_plugin_data(props.client, data_sources, refresh, props.allowed_endpoints); + let page_data = use_plugin_data( + props.client, + data_sources, + refresh, + props.allowed_endpoints, + ); // Consume pending navigation requests and forward to the parent use_effect(move || { @@ -151,7 +155,7 @@ pub fn PluginViewRenderer(props: PluginViewProps) -> Element { onclick: move |_| modal.set(None), "×" } - { render_element(&elem, &page_data.read(), &HashMap::new(), ctx) } + { render_element(&elem, &page_data.read(), &actions, ctx) } } } } @@ -318,44 +322,37 @@ fn PluginDataTable(props: PluginDataTableProps) -> Element { let row_val = row; rsx! { tr { - for col in props.columns.clone() { + for col in &props.columns { td { "{extract_cell(&row_val, &col.key)}" } } if !props.row_actions.is_empty() { td { class: "row-actions", - for act in props.row_actions.clone() { + for act in &props.row_actions { { let action = act.action.clone(); let row_data = row_val.clone(); let variant_class = button_variant_class(&act.variant); let page_actions = props.actions.clone(); - let success_msg: Option = - match &act.action { - ActionRef::Special(_) => None, - ActionRef::Name(name) => props - .actions - .get(name) - .and_then(|a| { - a.success_message.clone() - }), - ActionRef::Inline(a) => { - a.success_message.clone() - }, - }; - let error_msg: Option = - match &act.action { - ActionRef::Special(_) => None, - ActionRef::Name(name) => props - .actions - .get(name) - .and_then(|a| { - a.error_message.clone() - }), - ActionRef::Inline(a) => { - a.error_message.clone() - }, - }; + let (success_msg, error_msg): ( + Option, + Option, + ) = match &act.action { + ActionRef::Special(_) => (None, None), + ActionRef::Name(name) => props + .actions + .get(name) + .map_or((None, None), |a| { + ( + a.success_message.clone(), + a.error_message.clone(), + ) + }), + ActionRef::Inline(a) => ( + a.success_message.clone(), + a.error_message.clone(), + ), + }; let ctx = props.ctx; // Pre-compute data JSON at render time to // avoid moving props.data into closures. @@ -489,7 +486,8 @@ pub fn render_element( || "0".to_string(), |p| format!("{}px {}px {}px {}px", p[0], p[1], p[2], p[3]), ); - let style = format!("--plugin-gap:{gap}px;--plugin-padding:{padding_css};"); + let style = + format!("--plugin-gap:{gap}px;--plugin-padding:{padding_css};"); rsx! { div { class: "plugin-container", @@ -829,20 +827,18 @@ pub fn render_element( let variant_class = button_variant_class(variant); let action_ref = action.clone(); let page_actions = actions.clone(); - let success_msg: Option = match action { - ActionRef::Special(_) => None, - ActionRef::Name(name) => { - actions.get(name).and_then(|a| a.success_message.clone()) - }, - ActionRef::Inline(a) => a.success_message.clone(), - }; - let error_msg: Option = match action { - ActionRef::Special(_) => None, - ActionRef::Name(name) => { - actions.get(name).and_then(|a| a.error_message.clone()) - }, - ActionRef::Inline(a) => a.error_message.clone(), - }; + let (success_msg, error_msg): (Option, Option) = + match action { + ActionRef::Special(_) => (None, None), + ActionRef::Name(name) => { + actions.get(name).map_or((None, None), |a| { + (a.success_message.clone(), a.error_message.clone()) + }) + }, + ActionRef::Inline(a) => { + (a.success_message.clone(), a.error_message.clone()) + }, + }; let data_snapshot = build_ctx(data, &ctx.local_state.read()); rsx! { button { @@ -904,20 +900,18 @@ pub fn render_element( } => { let action_ref = submit_action.clone(); let page_actions = actions.clone(); - let success_msg: Option = match submit_action { - ActionRef::Special(_) => None, - ActionRef::Name(name) => { - actions.get(name).and_then(|a| a.success_message.clone()) - }, - ActionRef::Inline(a) => a.success_message.clone(), - }; - let error_msg: Option = match submit_action { - ActionRef::Special(_) => None, - ActionRef::Name(name) => { - actions.get(name).and_then(|a| a.error_message.clone()) - }, - ActionRef::Inline(a) => a.error_message.clone(), - }; + let (success_msg, error_msg): (Option, Option) = + match submit_action { + ActionRef::Special(_) => (None, None), + ActionRef::Name(name) => { + actions.get(name).map_or((None, None), |a| { + (a.success_message.clone(), a.error_message.clone()) + }) + }, + ActionRef::Inline(a) => { + (a.success_message.clone(), a.error_message.clone()) + }, + }; let data_snapshot = build_ctx(data, &ctx.local_state.read()); rsx! { form { @@ -1096,8 +1090,6 @@ pub fn render_element( } => { let chart_class = chart_type_class(chart_type); let chart_data = data.get(source_key).cloned(); - let x_label = x_axis_label.as_deref().unwrap_or("").to_string(); - let y_label = y_axis_label.as_deref().unwrap_or("").to_string(); rsx! { div { class: "plugin-chart {chart_class}", @@ -1111,7 +1103,7 @@ pub fn render_element( if let Some(x) = x_axis_label { div { class: "chart-x-label", "{x}" } } if let Some(y) = y_axis_label { div { class: "chart-y-label", "{y}" } } div { class: "chart-data-table", - { render_chart_data(chart_data.as_ref(), &x_label, &y_label) } + { render_chart_data(chart_data.as_ref(), x_axis_label.as_deref().unwrap_or(""), y_axis_label.as_deref().unwrap_or("")) } } } }