diff --git a/android/app/src/androidTest/kotlin/com/zarz/spotiflac/ExtensionMetadataBridgeTest.kt b/android/app/src/androidTest/kotlin/com/zarz/spotiflac/ExtensionMetadataBridgeTest.kt new file mode 100644 index 00000000..1938b3ec --- /dev/null +++ b/android/app/src/androidTest/kotlin/com/zarz/spotiflac/ExtensionMetadataBridgeTest.kt @@ -0,0 +1,63 @@ +package com.zarz.spotiflac + +import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.test.platform.app.InstrumentationRegistry +import com.spotiflac.backend.CancellationDomain +import com.spotiflac.backend.CancellationRegistry +import com.spotiflac.backend.ExtensionManager +import java.io.File +import org.json.JSONObject +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test +import org.junit.runner.RunWith + +@RunWith(AndroidJUnit4::class) +class ExtensionMetadataBridgeTest { + @Test + fun applicationMetadataRoutePreservesProviderStampingAndCancellation() { + val context = InstrumentationRegistry.getInstrumentation().targetContext + val root = File(context.cacheDir, "extension-bridge-${System.nanoTime()}").apply { mkdirs() } + val id = "example.metadata" + try { + val sources = File(root, "sources").apply { mkdirs() } + val extension = File(sources, id).apply { mkdirs() } + File(extension, "manifest.json").writeText( + """{"name":"$id","displayName":"Example Metadata","version":"1","description":"Generic bridge fixture","type":["metadata_provider"]}""", + ) + File(extension, "index.js").writeText(""" + function collection(id) { + if (id === "missing") return null; + return {id, name:"Album 音楽 🎵", artists:"Artist Café", total_tracks:128, + tracks:Array.from({length:128}, (_, i) => ({id:"track-"+i, + name:"歌 🎵 "+i, artists:"Artist Café", album_name:"Album 音楽 🎵", + provider_id:"supplied", duration_ms:123456, track_number:i+1}))}; + } + registerExtension({getAlbum:collection,getPlaylist:collection}); + """.trimIndent()) + ExtensionManager(sources.path, File(root, "data").path, "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=", "1", 10000uL).use { manager -> + manager.loadAll() + manager.setEnabled(id, true) + for ((kind, method) in listOf("album" to "getAlbum", "playlist" to "getPlaylist")) { + val legacy = JSONObject(manager.providerCall(id, method, "[\"fixture\"]", null, 10000uL)) + val result = JSONObject(manager.getProviderMetadataJson(id, kind, "fixture", null)) + val tracks = result.getJSONArray("track_list") + assertEquals(128, tracks.length()) + assertEquals(id, tracks.getJSONObject(127).getString("provider_id")) + assertEquals(legacy.getJSONArray("tracks").getJSONObject(127).getString("name"), tracks.getJSONObject(127).getString("name")) + assertEquals("Album 音楽 🎵", result.getJSONObject("${kind}_info").getString("name")) + assertTrue(runCatching { manager.getProviderMetadataJson(id, kind, "missing", null) }.isFailure) + } + CancellationRegistry(CancellationDomain.EXTENSION_REQUEST).use { registry -> + registry.acquire("bridge-cancel").use { lease -> + registry.cancel("bridge-cancel") + assertTrue(runCatching { manager.getProviderMetadataJson(id, "album", "fixture", lease) }.isFailure) + } + } + assertEquals(128, JSONObject(manager.getProviderMetadataJson(id, "album", "fixture", null)).getJSONArray("track_list").length()) + } + } finally { + root.deleteRecursively() + } + } +} diff --git a/rust_backend/crates/extensions/src/backend/provider_metadata.rs b/rust_backend/crates/extensions/src/backend/provider_metadata.rs index 5098903f..3d437829 100644 --- a/rust_backend/crates/extensions/src/backend/provider_metadata.rs +++ b/rust_backend/crates/extensions/src/backend/provider_metadata.rs @@ -6,6 +6,9 @@ use spotiflac_providers::resolver::{Check, ResolverError}; use std::sync::{Arc, mpsc}; use std::time::Duration; +#[cfg(test)] +mod value_tests; + impl Backend { pub fn enrich_track_json(&self, id: &str, track_json: &str) -> Result { let _operation = self.enter()?; @@ -186,7 +189,10 @@ impl Backend { } }; let arguments = json!([resource_id]).to_string(); - let result = self.provider_metadata_call(id, method, &arguments, check)?; + let result = self.metadata_provider_work_result(check, |lease| { + self.manager + .provider_call_value(id, method, &arguments, Some(lease), 30_000) + })?; let result = response(kind, &result, check)?; serde_json::to_string(&result).map_err(|error| ResolverError::Failed(error.to_string())) }) @@ -210,6 +216,15 @@ impl Backend { check: &Check<'_>, work: impl FnOnce(Arc) -> Result + Send, ) -> Result { + let result = self.metadata_provider_work_result(check, work)?; + serde_json::from_str(&result).map_err(|error| ResolverError::Failed(error.to_string())) + } + + fn metadata_provider_work_result( + &self, + check: &Check<'_>, + work: impl FnOnce(Arc) -> Result + Send, + ) -> Result { let cancellation = CancellationRegistry::new(CancellationDomain::ExtensionRequest); let lease = Arc::new( cancellation @@ -248,8 +263,7 @@ impl Backend { return Err(ResolverError::Cancelled(message)); } check().map_err(ResolverError::Cancelled)?; - let result = result.map_err(|error| ResolverError::Failed(error.to_string()))?; - serde_json::from_str(&result).map_err(|error| ResolverError::Failed(error.to_string())) + result.map_err(|error| ResolverError::Failed(error.to_string())) }) } } diff --git a/rust_backend/crates/extensions/src/backend/provider_metadata/value_tests.rs b/rust_backend/crates/extensions/src/backend/provider_metadata/value_tests.rs new file mode 100644 index 00000000..ac166882 --- /dev/null +++ b/rust_backend/crates/extensions/src/backend/provider_metadata/value_tests.rs @@ -0,0 +1,191 @@ +use super::*; +use crate::RuntimeLimits; +use std::sync::atomic::{AtomicBool, Ordering}; + +const ID: &str = "example.metadata"; +const SOURCE: &str = r#" +function collection(id) { + if (id === "null") return null; + if (id === "bad") return {id: "bad", tracks: {}}; + if (id === "scalar") return {id: "scalar", tracks: "not an array"}; + if (id === "throw") throw new Error("fixture failure"); + const count = Number(id); + return {id, name: "Album 音楽 🎵", artists: "Artist Café", provider_id: "supplied", + cover_url: "https://example.invalid/cover.jpg", total_tracks: count, + tracks: Array.from({length: count}, (_, i) => ({id: "track-" + i, + name: "歌 🎵 " + i, artists: "Artist Café", album_name: "Album 音楽 🎵", + provider_id: "supplied-track", duration_ms: 123456, track_number: i + 1, + explicit: i % 2 === 0, external_links: {example: "https://example.invalid/track/" + i}}))}; +} +registerExtension({getAlbum: collection, getPlaylist: collection}); +"#; + +fn fixture() -> (tempfile::TempDir, Backend) { + let root = tempfile::tempdir().unwrap(); + let source = root.path().join("sources").join(ID); + std::fs::create_dir_all(&source).unwrap(); + std::fs::write( + source.join("manifest.json"), + json!({"name":ID,"displayName":"Example Metadata","version":"1", + "description":"Generic metadata fixture","type":["metadata_provider"]}) + .to_string(), + ) + .unwrap(); + std::fs::write(source.join("index.js"), SOURCE).unwrap(); + let backend = Backend::new( + &root.path().join("sources"), + &root.path().join("data"), + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=", + "1", + RuntimeLimits::default(), + ) + .unwrap(); + backend.load_all().unwrap(); + backend.set_enabled(ID, true).unwrap(); + (root, backend) +} + +#[test] +fn metadata_value_bridge_preserves_album_playlist_and_public_json() { + let (_root, backend) = fixture(); + for (kind, method) in [("album", "getAlbum"), ("playlist", "getPlaylist")] { + let arguments = r#"["100"]"#; + let legacy = backend + .manager + .provider_call(ID, method, arguments, None, 30_000) + .unwrap(); + let legacy_value: Value = serde_json::from_str(&legacy).unwrap(); + let value = backend + .manager + .provider_call_value(ID, method, arguments, None, 30_000) + .unwrap(); + assert_eq!(value, legacy_value); + assert_eq!(value.to_string(), legacy); + assert_eq!(value["provider_id"], ID); + assert_eq!(value["tracks"][99]["provider_id"], ID); + assert_eq!(value["tracks"][99]["name"], "歌 🎵 99"); + let expected = response(kind, &legacy_value, &|| Ok(())).unwrap(); + let actual = backend + .get_provider_metadata_json(ID, kind, "100", &|| Ok(())) + .unwrap(); + assert_eq!(actual, expected.to_string()); + // Legacy array-like coercion accepts a string as one default track per + // character. Keep that compatibility rather than tightening decoding. + let scalar = backend + .manager + .provider_call(ID, method, r#"["scalar"]"#, None, 30_000) + .unwrap(); + let scalar_value = backend + .manager + .provider_call_value(ID, method, r#"["scalar"]"#, None, 30_000) + .unwrap(); + assert_eq!(scalar_value.to_string(), scalar); + assert_eq!(scalar_value["tracks"].as_array().unwrap().len(), 12); + } + if std::env::var_os("SPOTIFLAC_METADATA_BRIDGE_BENCH").is_some() { + measure_removed_roundtrip(&backend); + } + backend.shutdown(); +} + +#[test] +fn metadata_value_bridge_preserves_provider_errors_and_cancelled_leases() { + let (_root, backend) = fixture(); + for (kind, method) in [("album", "getAlbum"), ("playlist", "getPlaylist")] { + for id in ["null", "bad", "throw"] { + let arguments = json!([id]).to_string(); + let legacy = backend + .manager + .provider_call(ID, method, &arguments, None, 30_000) + .unwrap_err(); + let typed = backend + .manager + .provider_call_value(ID, method, &arguments, None, 30_000) + .unwrap_err(); + assert_eq!(typed.to_string(), legacy.to_string()); + let facade = backend + .get_provider_metadata_json(ID, kind, id, &|| Ok(())) + .unwrap_err(); + assert!(facade.contains(&legacy.to_string()), "{facade}"); + } + } + let registry = CancellationRegistry::new(CancellationDomain::ExtensionRequest); + let lease = Arc::new(registry.acquire("metadata-fixture").unwrap()); + lease.release(); + let legacy = backend + .manager + .provider_call(ID, "getAlbum", r#"["1"]"#, Some(lease.clone()), 30_000) + .unwrap_err(); + let typed = backend + .manager + .provider_call_value(ID, "getAlbum", r#"["1"]"#, Some(lease), 30_000) + .unwrap_err(); + assert_eq!(typed.to_string(), legacy.to_string()); + backend.shutdown(); +} + +#[test] +fn metadata_value_worker_joins_cancellation_and_reports_panic() { + let (_root, backend) = fixture(); + let started = AtomicBool::new(false); + let joined = AtomicBool::new(false); + let result = backend.metadata_provider_work_result( + &|| { + if started.load(Ordering::Acquire) { + Err("cancelled typed result".into()) + } else { + Ok(()) + } + }, + |lease| { + started.store(true, Ordering::Release); + assert!(lease.wait_cancelled(60_000).is_err()); + joined.store(true, Ordering::Release); + Ok(json!({"tracks":[]})) + }, + ); + assert_eq!( + result, + Err(ResolverError::Cancelled("cancelled typed result".into())) + ); + assert!(joined.load(Ordering::Acquire)); + let panic = backend + .metadata_provider_work_result(&|| Ok(()), |_| -> Result { + panic!("typed metadata worker failed") + }); + assert_eq!( + panic, + Err(ResolverError::Failed("metadata provider panicked".into())) + ); + backend.shutdown(); +} + +fn measure_removed_roundtrip(backend: &Backend) { + use std::hint::black_box; + use std::time::Instant; + + for count in [1, 100, 1_000, 10_000] { + let value = backend + .manager + .provider_call_value( + ID, + "getPlaylist", + &json!([count.to_string()]).to_string(), + None, + 30_000, + ) + .unwrap(); + let bytes = value.to_string().len(); + let iterations = if count >= 1_000 { 25 } else { 100 }; + let start = Instant::now(); + for _ in 0..iterations { + let encoded = black_box(&value).to_string(); + let decoded: Value = serde_json::from_str(black_box(&encoded)).unwrap(); + black_box(decoded); + } + let micros = start.elapsed().as_secs_f64() * 1_000_000.0 / f64::from(iterations); + eprintln!( + "metadata bridge isolated removed serialize+parse: tracks={count} bytes={bytes} iterations={iterations} mean_us={micros:.2}" + ); + } +} diff --git a/rust_backend/crates/extensions/src/manager/providers.rs b/rust_backend/crates/extensions/src/manager/providers.rs index 190d4771..9c4d7f6a 100644 --- a/rust_backend/crates/extensions/src/manager/providers.rs +++ b/rust_backend/crates/extensions/src/manager/providers.rs @@ -175,6 +175,19 @@ impl ExtensionManager { self.provider_operation(id, method, arguments, lease, timeout_ms, "") } + /// Internal native callers keep the already validated provider value; + /// only public string boundaries need to serialize it again. + pub(crate) fn provider_call_value( + &self, + id: &str, + method: &str, + arguments: &str, + lease: Option>, + timeout_ms: u64, + ) -> Result { + self.provider_operation_value(id, method, arguments, lease, timeout_ms, "") + } + pub(super) fn provider_operation( &self, id: &str, @@ -184,6 +197,19 @@ impl ExtensionManager { timeout_ms: u64, item_id: &str, ) -> Result { + self.provider_operation_value(id, method, arguments, lease, timeout_ms, item_id) + .map(|value| value.to_string()) + } + + fn provider_operation_value( + &self, + id: &str, + method: &str, + arguments: &str, + lease: Option>, + timeout_ms: u64, + item_id: &str, + ) -> Result { let entry = self.get(id)?; let manifest = &entry.manifest; let requirement = @@ -300,12 +326,12 @@ impl ExtensionManager { return Err(verification_error(id)); } if value.is_null() { - return Ok(json!({"available":false,"reason":"not implemented"}).to_string()); + return Ok(json!({"available":false,"reason":"not implemented"})); } } if value.is_null() { return if method == "customSearch" { - Ok("[]".into()) + Ok(json!([])) } else if method == "handleUrl" { Err(error("handleUrl returned null - URL not recognized")) } else { @@ -374,7 +400,7 @@ impl ExtensionManager { } _ => {} } - Ok(value.to_string()) + Ok(value) } }