From f48a0c60ee99050d775b1f896db29a0975e5a5bc Mon Sep 17 00:00:00 2001 From: "sola.awojobi00-bit" Date: Tue, 25 Aug 2026 23:46:01 +0100 Subject: [PATCH] feat: add cursor pagination and cache validators to template registry Adds stable cursor-based pagination and ETag/conditional-request caching to the template marketplace registry, closing #681. - templates::paginate()/Page: opaque, base64-encoded cursors anchored to a template's name (not list position), so pages stay valid across calls even if the registry changes; rejects unknown cursors and a zero limit with clear errors. - fetch_and_cache_remote() now sends If-None-Match with the last-seen ETag and short-circuits on 304 Not Modified, reusing the local cache instead of re-downloading; the ETag is persisted in a sidecar file. - `starforge template list`/`search` gain --limit/--cursor (including the --json output path); omitting both keeps prior unpaginated behavior, so this is non-breaking. - registry_dir() honors STARFORGE_TEMPLATE_REGISTRY_DIR so tests don't depend on HOME, which dirs::home_dir() ignores on Windows. Also fixes three unrelated pre-existing compile errors blocking `cargo build` on master (missing thiserror dependency, a rusqlite::Transaction mutability bug in database.rs, and two missing `mod ai_doc_qa;` declarations), plus a bundled templates/registry.json field whose type didn't match the current TemplateEntry schema. --- Cargo.lock | 1 + Cargo.toml | 1 + src/commands/mod.rs | 1 + src/commands/template.rs | 110 ++++++++- src/utils/database.rs | 20 +- src/utils/mod.rs | 1 + src/utils/templates.rs | 479 ++++++++++++++++++++++++++++++++++++--- templates/registry.json | 20 +- 8 files changed, 566 insertions(+), 67 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 682b5097..58ab1927 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3643,6 +3643,7 @@ dependencies = [ "stellar-strkey", "stellar-xdr", "tempfile", + "thiserror 2.0.19", "tokio", "tokio-tungstenite", "toml", diff --git a/Cargo.toml b/Cargo.toml index 888d6bff..00a4fab8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -89,6 +89,7 @@ minijinja = "1.0" serde_yaml = "0.9.34" async-trait = "0.1" futures = "0.3.33" +thiserror = "2" [features] hardware-wallet = ["dep:hidapi", "dep:trezor-client"] diff --git a/src/commands/mod.rs b/src/commands/mod.rs index bcf7bd23..b1e4615d 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -7,6 +7,7 @@ pub mod ai_contract_suggest; pub mod ai_debug; pub mod ai_deploy_docs; pub mod ai_deployment_test; +pub mod ai_doc_qa; pub mod ai_error; pub mod ai_feedback; pub mod ai_ide; diff --git a/src/commands/template.rs b/src/commands/template.rs index adb9e29b..a71c10e0 100644 --- a/src/commands/template.rs +++ b/src/commands/template.rs @@ -25,12 +25,24 @@ pub enum TemplateCommands { /// Force refresh of remote registry, ignoring cached copy #[arg(long)] refresh: bool, + /// Maximum number of results to show per page. Omit to show all matches. + #[arg(long)] + limit: Option, + /// Resume after this pagination cursor (from a previous page's "Next cursor") + #[arg(long)] + cursor: Option, }, /// List all available templates List { /// Emit a machine-readable JSON object instead of the human-readable output #[arg(long)] json: bool, + /// Maximum number of templates to show per page. Omit to show all. + #[arg(long)] + limit: Option, + /// Resume after this pagination cursor (from a previous page's "Next cursor") + #[arg(long)] + cursor: Option, }, /// Show details of a specific template Show { @@ -241,14 +253,20 @@ pub async fn handle(cmd: TemplateCommands) -> Result<()> { ) .await } - TemplateCommands::List { json } => list(json).await, + TemplateCommands::List { + json, + limit, + cursor, + } => list(json, limit, cursor).await, TemplateCommands::Search { query, tags, verified, min_quality, refresh, - } => search(query, tags, verified, min_quality, refresh).await, + limit, + cursor, + } => search(query, tags, verified, min_quality, refresh, limit, cursor).await, TemplateCommands::Show { name } => show(name).await, TemplateCommands::Remove { name, purge } => remove(name, purge).await, TemplateCommands::Init => init(), @@ -430,16 +448,53 @@ async fn publish( Ok(()) } -async fn list(json: bool) -> Result<()> { +/// Default number of results shown per page when pagination is requested via +/// `--cursor` without an explicit `--limit`. +const DEFAULT_PAGE_LIMIT: usize = 20; + +/// Print the "Shown X of Y" / "Next cursor" footer for a paginated command, +/// when pagination was requested at all. +fn print_pagination_footer(page: Option<&templates::Page>) { + if let Some(page) = page { + println!(); + p::kv("Shown", &format!("{} of {}", page.items.len(), page.total)); + if let Some(next) = &page.next_cursor { + p::info(&format!( + "More results available — pass --cursor {} to continue", + next + )); + } + } +} + +async fn list(json: bool, limit: Option, cursor: Option) -> Result<()> { use crate::utils::templates::{check_template_compatibility, CompatibilityStatus}; let registry = templates::load_registry().await?; let emit_json = json || output::is_json_mode_enabled(); + // Pagination only kicks in when the caller opts in via --limit or + // --cursor, so plain `template list` keeps showing everything. + let page = match limit.or(cursor.is_some().then_some(DEFAULT_PAGE_LIMIT)) { + Some(limit) => Some(templates::paginate( + ®istry.templates, + cursor.as_deref(), + limit, + |t| t.name.as_str(), + )?), + None => None, + }; + let shown: &[templates::TemplateEntry] = match &page { + Some(page) => &page.items, + None => ®istry.templates, + }; + if emit_json { #[derive(serde::Serialize)] struct TemplateListResponse { template_count: usize, + shown_count: usize, + next_cursor: Option, templates: Vec, } @@ -454,8 +509,9 @@ async fn list(json: bool) -> Result<()> { compatible: bool, } - let templates: Vec = registry - .templates + let template_count = registry.templates.len(); + let next_cursor = page.as_ref().and_then(|p| p.next_cursor.clone()); + let templates: Vec = shown .iter() .map(|template| { let compatible = matches!( @@ -475,7 +531,9 @@ async fn list(json: bool) -> Result<()> { .collect(); return output::print_json(&TemplateListResponse { - template_count: templates.len(), + template_count, + shown_count: templates.len(), + next_cursor, templates, }); } @@ -484,8 +542,12 @@ async fn list(json: bool) -> Result<()> { p::info("No templates found. Publish one with: starforge template publish "); return Ok(()); } + if shown.is_empty() { + p::info("No more templates on this page."); + return Ok(()); + } - for (i, template) in registry.templates.iter().enumerate() { + for (i, template) in shown.iter().enumerate() { let compat_badge = match check_template_compatibility(template) { CompatibilityStatus::Compatible => "[COMPATIBLE]", CompatibilityStatus::TooOld { .. } | CompatibilityStatus::TooNew { .. } => { @@ -511,20 +573,25 @@ async fn list(json: bool) -> Result<()> { if let Some(path) = template.path.as_ref() { p::kv("Path", path); } - if i + 1 < registry.templates.len() { + if i + 1 < shown.len() { println!(); } } + print_pagination_footer(page.as_ref()); + Ok(()) } +#[allow(clippy::too_many_arguments)] async fn search( query: String, tags: Option, verified: bool, min_quality: u8, refresh: bool, + limit: Option, + cursor: Option, ) -> Result<()> { use crate::utils::templates::{check_template_compatibility, CompatibilityStatus}; let tag_list: Vec = tags @@ -580,10 +647,31 @@ async fn search( return Ok(()); } + // Pagination only kicks in when the caller opts in via --limit or + // --cursor, so plain `template search` keeps showing every match. + let page = match limit.or(cursor.is_some().then_some(DEFAULT_PAGE_LIMIT)) { + Some(limit) => Some(templates::paginate( + &results, + cursor.as_deref(), + limit, + |r| r.entry.name.as_str(), + )?), + None => None, + }; + let shown: &[templates::SearchResult] = match &page { + Some(page) => &page.items, + None => &results, + }; + p::kv("Matches", &results.len().to_string()); println!(); - for (i, result) in results.iter().enumerate() { + if shown.is_empty() { + p::info("No more results on this page."); + return Ok(()); + } + + for (i, result) in shown.iter().enumerate() { let template = &result.entry; let compat_badge = match check_template_compatibility(template) { CompatibilityStatus::Compatible => "[COMPATIBLE]", @@ -619,11 +707,13 @@ async fn search( ); } p::kv("Source", &template.source.to_string()); - if i + 1 < results.len() { + if i + 1 < shown.len() { println!(); } } + print_pagination_footer(page.as_ref()); + Ok(()) } diff --git a/src/utils/database.rs b/src/utils/database.rs index 01d49c5b..eaaa65e8 100644 --- a/src/utils/database.rs +++ b/src/utils/database.rs @@ -21,10 +21,10 @@ pub trait Migration: Send + Sync { fn description(&self) -> &str; /// Apply the migration (upgrade) - fn up(&self, conn: &mut Connection) -> Result<()>; - + fn up(&self, conn: &Connection) -> Result<()>; + /// Rollback the migration (downgrade) - fn down(&self, conn: &mut Connection) -> Result<()>; + fn down(&self, conn: &Connection) -> Result<()>; } /// Record of an applied migration in the database @@ -220,9 +220,9 @@ impl Database { .ok_or_else(|| anyhow::anyhow!("Migration version {} not found", version))?; let tx = self.conn.unchecked_transaction()?; - + // Apply the migration - match migration.up(&mut tx) { + match migration.up(&tx) { Ok(()) => { // Record the migration let checksum = self.compute_migration_checksum(version, migration.description())?; @@ -273,9 +273,9 @@ impl Database { .ok_or_else(|| anyhow::anyhow!("Migration version {} not found", version))?; let tx = self.conn.unchecked_transaction()?; - + // Rollback the migration - match migration.down(&mut tx) { + match migration.down(&tx) { Ok(()) => { // Remove the migration record tx.execute( @@ -1110,12 +1110,12 @@ impl Migration for MigrationV1 { "initial_schema" } - fn up(&self, conn: &mut Connection) -> Result<()> { + fn up(&self, conn: &Connection) -> Result<()> { // This is a no-op since the initial schema is already applied in SCHEMA Ok(()) } - - fn down(&self, conn: &mut Connection) -> Result<()> { + + fn down(&self, conn: &Connection) -> Result<()> { // Rollback: drop all tables conn.execute_batch( "DROP TABLE IF EXISTS events; diff --git a/src/utils/mod.rs b/src/utils/mod.rs index a3fa7e37..5a2b6b22 100644 --- a/src/utils/mod.rs +++ b/src/utils/mod.rs @@ -14,6 +14,7 @@ pub mod ai_debug_enhancement; pub mod ai_debugger; pub mod ai_deployment_planner; pub mod ai_deployment_testing; +pub mod ai_doc_qa; pub mod ai_docs; pub mod ai_documentation_assistant; pub mod ai_error_handler; diff --git a/src/utils/templates.rs b/src/utils/templates.rs index af10b811..099f6c2f 100644 --- a/src/utils/templates.rs +++ b/src/utils/templates.rs @@ -1,5 +1,6 @@ use crate::utils::http_client; use anyhow::{Context, Result}; +use base64::{engine::general_purpose::STANDARD as BASE64, Engine}; use chrono::Utc; use serde::{Deserialize, Serialize}; use std::fs; @@ -625,13 +626,34 @@ const DEFAULT_REGISTRY: &str = include_str!("../../templates/registry.json"); const DEFAULT_REGISTRY_URL: &str = "https://starforge-protocol.github.io/starforge/templates/registry.json"; -fn registry_path() -> Result { - let home = dirs::home_dir().ok_or_else(|| anyhow::anyhow!("Could not find home directory"))?; - let dir = home.join(".starforge").join("templates"); +/// Directory holding the local registry cache. Honors +/// `STARFORGE_TEMPLATE_REGISTRY_DIR` (primarily used by tests to avoid +/// touching a real home directory) before falling back to +/// `~/.starforge/templates`. +fn registry_dir() -> Result { + let dir = match std::env::var_os("STARFORGE_TEMPLATE_REGISTRY_DIR") { + Some(dir) => PathBuf::from(dir), + None => { + let home = + dirs::home_dir().ok_or_else(|| anyhow::anyhow!("Could not find home directory"))?; + home.join(".starforge").join("templates") + } + }; if !dir.exists() { fs::create_dir_all(&dir).with_context(|| format!("Failed to create {}", dir.display()))?; } - Ok(dir.join("registry.json")) + Ok(dir) +} + +fn registry_path() -> Result { + Ok(registry_dir()?.join("registry.json")) +} + +/// Path to the sidecar file that stores the `ETag` of the last successfully +/// fetched remote registry, used to make conditional (`If-None-Match`) +/// requests on subsequent refreshes. +fn registry_etag_path() -> Result { + Ok(registry_path()?.with_extension("etag")) } /// Returns true if the path looks like a supported template archive. @@ -825,6 +847,49 @@ pub async fn template_source_content(name: &str, force_refresh: bool) -> Result< } } +const REGISTRY_CACHE_TTL: std::time::Duration = std::time::Duration::from_secs(24 * 60 * 60); + +/// Whether the locally cached registry file is still within its TTL window. +fn is_cache_fresh(cache_path: &Path) -> bool { + fs::metadata(cache_path) + .and_then(|m| m.modified()) + .map(|modified| { + std::time::SystemTime::now() + .duration_since(modified) + .unwrap_or(REGISTRY_CACHE_TTL) + < REGISTRY_CACHE_TTL + }) + .unwrap_or(false) +} + +/// Read and parse the locally cached registry file, if present and valid. +fn read_cached_registry(cache_path: &Path) -> Option { + let contents = fs::read_to_string(cache_path).ok()?; + serde_json::from_str(&contents).ok() +} + +/// Reset a file's modification time to now without changing its contents. +/// +/// Used after a `304 Not Modified` response to restart the cache's TTL +/// window without re-downloading or re-writing the (unchanged) body. +fn touch(path: &Path) { + if let Ok(contents) = fs::read(path) { + let _ = fs::write(path, contents); + } +} + +/// Read the ETag stored alongside the cached registry, if any. +fn read_stored_etag() -> Option { + let etag_path = registry_etag_path().ok()?; + let etag = fs::read_to_string(etag_path).ok()?; + let etag = etag.trim(); + if etag.is_empty() { + None + } else { + Some(etag.to_string()) + } +} + pub async fn load_registry() -> Result { // Determine remote registry URL, falling back to the default global index. let remote_url = std::env::var("STARFORGE_TEMPLATE_REGISTRY_URL") @@ -837,38 +902,29 @@ pub async fn load_registry() -> Result { let cache_path = registry_path()?; // Use cache if it exists and is fresh and we are not forcing a refresh. - if !force_refresh { - if let Ok(metadata) = fs::metadata(&cache_path) { - if let Ok(modified) = metadata.modified() { - use std::time::{Duration, SystemTime}; - let ttl = Duration::from_secs(24 * 60 * 60); // 24 hours - if SystemTime::now() - .duration_since(modified) - .unwrap_or_else(|_| ttl) - < ttl - { - let contents = fs::read_to_string(&cache_path).with_context(|| { - format!("Failed to read cached registry at {}", cache_path.display()) - })?; - let registry: TemplateRegistry = serde_json::from_str(&contents) - .with_context(|| "Failed to parse cached template registry")?; - return Ok(registry); - } - } + if !force_refresh && is_cache_fresh(&cache_path) { + if let Some(registry) = read_cached_registry(&cache_path) { + return Ok(registry); } } - // Either forced refresh or cache is missing/old – attempt to fetch remote. - match fetch_and_cache_remote(&remote_url).await { - Ok(registry) => Ok(registry), + // Either forced refresh or cache is missing/old – attempt a conditional + // fetch, sending back any ETag we recorded from a previous fetch so the + // server can reply `304 Not Modified` when nothing has changed. + let stored_etag = read_stored_etag(); + match fetch_and_cache_remote(&remote_url, stored_etag.as_deref()).await { + Ok(FetchOutcome::Fetched(registry)) => Ok(registry), + Ok(FetchOutcome::NotModified) => { + touch(&cache_path); + read_cached_registry(&cache_path).ok_or_else(|| { + anyhow::anyhow!( + "Registry server returned 304 Not Modified but no local cache exists" + ) + }) + } Err(_fetch_err) => { // If the remote fetch failed but a cached registry exists, fall back to it. - if cache_path.exists() { - let contents = fs::read_to_string(&cache_path).with_context(|| { - format!("Failed to read cached registry at {}", cache_path.display()) - })?; - let registry: TemplateRegistry = serde_json::from_str(&contents) - .with_context(|| "Failed to parse cached template registry")?; + if let Some(registry) = read_cached_registry(&cache_path) { return Ok(registry); } // No cache available – fall back to the registry bundled with the binary @@ -893,19 +949,48 @@ pub fn save_registry(registry: &TemplateRegistry) -> Result<()> { Ok(()) } -/// Fetches a remote JSON template registry, caches it locally, and returns the parsed registry. -async fn fetch_and_cache_remote(url: &str) -> Result { - let response = http_client::get_client() - .get(url) +/// Outcome of a conditional fetch against the remote registry. +enum FetchOutcome { + /// The server returned a fresh body (`200 OK`); it has been parsed and cached. + Fetched(TemplateRegistry), + /// The server confirmed the local cache is still current (`304 Not Modified`). + NotModified, +} + +/// Fetches a remote JSON template registry, caches it locally, and returns the +/// parsed registry. +/// +/// When `etag` is `Some`, the request is sent as a conditional GET with an +/// `If-None-Match` header, so an unchanged remote registry can reply +/// `304 Not Modified` instead of re-sending the full body. +async fn fetch_and_cache_remote(url: &str, etag: Option<&str>) -> Result { + let mut request = http_client::get_client().get(url); + if let Some(etag) = etag { + request = request.header("If-None-Match", etag); + } + + let response = request .send() .await .with_context(|| format!("Failed to fetch remote template registry from {}", url))?; - if response.status() != 200 { + + if response.status() == reqwest::StatusCode::NOT_MODIFIED { + return Ok(FetchOutcome::NotModified); + } + if response.status() != reqwest::StatusCode::OK { anyhow::bail!( "Unexpected HTTP status {} when fetching remote registry", response.status() ); } + + // Capture the ETag before consuming the response body. + let new_etag = response + .headers() + .get(reqwest::header::ETAG) + .and_then(|v| v.to_str().ok()) + .map(|s| s.to_string()); + let json_str = response .text() .await @@ -925,7 +1010,22 @@ async fn fetch_and_cache_remote(url: &str) -> Result { cache_path.display() ) })?; - Ok(registry) + + // Persist the ETag (if any) for the next conditional request; clear any + // stale value when the server no longer sends one. + let etag_path = registry_etag_path()?; + match &new_etag { + Some(tag) => { + fs::write(&etag_path, tag).with_context(|| { + format!("Failed to write registry ETag to {}", etag_path.display()) + })?; + } + None => { + fs::remove_file(&etag_path).ok(); + } + } + + Ok(FetchOutcome::Fetched(registry)) } /// Filters applied on top of a text query when searching the marketplace. @@ -1083,6 +1183,93 @@ pub async fn search_templates(query: &str, tags: Option<&[String]>) -> Result { + pub items: Vec, + /// Cursor to pass as `--cursor` to fetch the next page; `None` once the + /// last page has been reached. + pub next_cursor: Option, + /// Total number of items in the unpaginated result set. + pub total: usize, +} + +/// Encode a stable pagination cursor from an item's unique key. +fn encode_cursor(key: &str) -> String { + BASE64.encode(key) +} + +/// Decode a pagination cursor back into the item key it was derived from. +fn decode_cursor(cursor: &str) -> Result { + let bytes = BASE64 + .decode(cursor) + .with_context(|| "Invalid pagination cursor: not valid base64")?; + String::from_utf8(bytes).with_context(|| "Invalid pagination cursor: not valid UTF-8") +} + +/// Split `items` into a page of at most `limit` entries, starting immediately +/// after the entry identified by `cursor` (or from the start when `cursor` is +/// `None`). +/// +/// Cursors are opaque to callers and are derived from each item's stable key +/// (as returned by `key_fn`, e.g. a template name) rather than a raw +/// position, so a page stays anchored to the entry a caller last saw even if +/// earlier entries are added or removed between requests. If the entry a +/// cursor points to can no longer be found (e.g. it was removed from the +/// registry), pagination is aborted with an error rather than silently +/// returning a misleading page. +pub fn paginate( + items: &[T], + cursor: Option<&str>, + limit: usize, + key_fn: impl Fn(&T) -> &str, +) -> Result> { + if limit == 0 { + anyhow::bail!("--limit must be greater than 0"); + } + + let start = match cursor { + None => 0, + Some(raw) => { + let key = decode_cursor(raw)?; + let idx = items + .iter() + .position(|item| key_fn(item) == key) + .ok_or_else(|| { + anyhow::anyhow!( + "Cursor does not match any known entry (the registry may have \ + changed since the previous page was fetched). Restart pagination \ + by omitting --cursor." + ) + })?; + idx + 1 + } + }; + + let total = items.len(); + if start >= total { + return Ok(Page { + items: Vec::new(), + next_cursor: None, + total, + }); + } + + let end = (start + limit).min(total); + let page_items = items[start..end].to_vec(); + let next_cursor = if end < total { + Some(encode_cursor(key_fn(&items[end - 1]))) + } else { + None + }; + + Ok(Page { + items: page_items, + next_cursor, + total, + }) +} + pub async fn get_template(name: &str) -> Result { let versions = get_templates_by_name(name).await?; versions @@ -2044,6 +2231,9 @@ mod tests { fn make_entry(name: &str) -> TemplateEntry { TemplateEntry { name: name.to_string(), + repository: None, + security_review: None, + changelog: None, version: "1.0.0".to_string(), description: String::new(), author: String::new(), @@ -2378,6 +2568,9 @@ mod tests { let mut registry = TemplateRegistry::default(); registry.templates.push(TemplateEntry { name: "uniswap-v2".to_string(), + repository: None, + security_review: None, + changelog: None, version: "1.0.0".to_string(), description: "Uniswap V2 DEX implementation".to_string(), author: "DeFi Team".to_string(), @@ -2429,6 +2622,9 @@ mod tests { let entry = TemplateEntry { name: "my-template".to_string(), + repository: None, + security_review: None, + changelog: None, source: TemplateSource::Git { url: "https://example.com/repo.git".to_string(), branch: None, @@ -2483,6 +2679,9 @@ mod tests { fn sample_entry() -> TemplateEntry { TemplateEntry { name: "sample".to_string(), + repository: None, + security_review: None, + changelog: None, version: "1.0.0".to_string(), description: String::new(), author: String::new(), @@ -2921,4 +3120,210 @@ mod tests { "error should describe the problem" ); } + + // ---- Cursor pagination ------------------------------------------------- + + #[test] + fn paginate_walks_all_pages_in_order() { + let items: Vec = (1..=5).map(|i| make_entry(&format!("tpl-{i}"))).collect(); + + let mut cursor: Option = None; + let mut collected = Vec::new(); + loop { + let page = paginate(&items, cursor.as_deref(), 2, |t| t.name.as_str()).unwrap(); + collected.extend(page.items.iter().map(|t| t.name.clone())); + assert_eq!(page.total, 5); + match page.next_cursor { + Some(next) => cursor = Some(next), + None => break, + } + } + + assert_eq!(collected, vec!["tpl-1", "tpl-2", "tpl-3", "tpl-4", "tpl-5"]); + } + + #[test] + fn paginate_cursor_past_last_item_returns_empty_page() { + let items: Vec = (1..=3).map(|i| make_entry(&format!("tpl-{i}"))).collect(); + let last_cursor = encode_cursor("tpl-3"); + + let page = paginate(&items, Some(&last_cursor), 2, |t| t.name.as_str()).unwrap(); + assert!(page.items.is_empty()); + assert!(page.next_cursor.is_none()); + assert_eq!(page.total, 3); + } + + #[test] + fn paginate_rejects_zero_limit() { + let items = vec![make_entry("tpl-1")]; + let err = paginate(&items, None, 0, |t| t.name.as_str()).unwrap_err(); + assert!(err.to_string().contains("greater than 0")); + } + + #[test] + fn paginate_rejects_cursor_for_unknown_entry() { + let items = vec![make_entry("tpl-1")]; + let ghost_cursor = encode_cursor("does-not-exist"); + let err = paginate(&items, Some(&ghost_cursor), 10, |t| t.name.as_str()).unwrap_err(); + assert!(err.to_string().contains("Cursor does not match")); + } + + #[test] + fn paginate_rejects_malformed_cursor() { + let items = vec![make_entry("tpl-1")]; + let err = + paginate(&items, Some("not-valid-base64!!"), 10, |t| t.name.as_str()).unwrap_err(); + assert!(err.to_string().to_lowercase().contains("cursor")); + } + + // ---- ETag / conditional-request caching -------------------------------- + + static TEST_ENV_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); + + /// Isolates a test's registry cache directory and registry-related env + /// vars so concurrent tests don't clobber each other or the real user's + /// `~/.starforge` directory. Uses `STARFORGE_TEMPLATE_REGISTRY_DIR` + /// rather than overriding `HOME`, since `dirs::home_dir()` ignores + /// `HOME`/`USERPROFILE` overrides on some platforms (notably Windows). + struct RegistryTestEnv { + _env_lock: std::sync::MutexGuard<'static, ()>, + _cache_dir: tempfile::TempDir, + original_dir: Option, + original_url: Option, + original_force_refresh: Option, + } + + impl RegistryTestEnv { + fn new(remote_url: &str) -> Self { + let env_lock = TEST_ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + let cache_dir = tempdir().expect("temp registry cache dir"); + let original_dir = std::env::var("STARFORGE_TEMPLATE_REGISTRY_DIR").ok(); + let original_url = std::env::var("STARFORGE_TEMPLATE_REGISTRY_URL").ok(); + let original_force_refresh = + std::env::var("STARFORGE_TEMPLATE_REGISTRY_FORCE_REFRESH").ok(); + + std::env::set_var("STARFORGE_TEMPLATE_REGISTRY_DIR", cache_dir.path()); + std::env::set_var("STARFORGE_TEMPLATE_REGISTRY_URL", remote_url); + std::env::remove_var("STARFORGE_TEMPLATE_REGISTRY_FORCE_REFRESH"); + + Self { + _env_lock: env_lock, + _cache_dir: cache_dir, + original_dir, + original_url, + original_force_refresh, + } + } + + fn force_refresh(&self) { + std::env::set_var("STARFORGE_TEMPLATE_REGISTRY_FORCE_REFRESH", "1"); + } + } + + impl Drop for RegistryTestEnv { + fn drop(&mut self) { + match &self.original_dir { + Some(v) => std::env::set_var("STARFORGE_TEMPLATE_REGISTRY_DIR", v), + None => std::env::remove_var("STARFORGE_TEMPLATE_REGISTRY_DIR"), + } + match &self.original_url { + Some(v) => std::env::set_var("STARFORGE_TEMPLATE_REGISTRY_URL", v), + None => std::env::remove_var("STARFORGE_TEMPLATE_REGISTRY_URL"), + } + match &self.original_force_refresh { + Some(v) => std::env::set_var("STARFORGE_TEMPLATE_REGISTRY_FORCE_REFRESH", v), + None => std::env::remove_var("STARFORGE_TEMPLATE_REGISTRY_FORCE_REFRESH"), + } + } + } + + const MOCK_TEMPLATE_BODY: &str = r#"{"templates":[{"name":"demo","repository":null,"security_review":null,"changelog":null,"description":"d","version":"1.0.0","source":{"type":"builtin","id":"demo"}}]}"#; + + #[tokio::test] + async fn fetch_and_cache_remote_stores_and_sends_etag() { + let mut server = mockito::Server::new_async().await; + let _env = RegistryTestEnv::new(&server.url()); + + let _first_mock = server + .mock("GET", "/") + .match_header("if-none-match", mockito::Matcher::Missing) + .with_status(200) + .with_header("ETag", "\"abc123\"") + .with_header("content-type", "application/json") + .with_body(MOCK_TEMPLATE_BODY) + .create_async() + .await; + + let outcome = fetch_and_cache_remote(&server.url(), None) + .await + .expect("first fetch"); + match outcome { + FetchOutcome::Fetched(registry) => assert_eq!(registry.templates.len(), 1), + FetchOutcome::NotModified => panic!("expected a fresh fetch on first request"), + } + + let stored = read_stored_etag().expect("etag should have been cached"); + assert_eq!(stored, "\"abc123\""); + + let _second_mock = server + .mock("GET", "/") + .match_header("if-none-match", "\"abc123\"") + .with_status(304) + .create_async() + .await; + + let outcome = fetch_and_cache_remote(&server.url(), Some(&stored)) + .await + .expect("conditional fetch"); + assert!( + matches!(outcome, FetchOutcome::NotModified), + "server should have short-circuited with 304" + ); + } + + #[tokio::test] + async fn load_registry_reuses_cache_on_304_after_forced_refresh() { + let mut server = mockito::Server::new_async().await; + let env = RegistryTestEnv::new(&server.url()); + + let _first_mock = server + .mock("GET", "/") + .match_header("if-none-match", mockito::Matcher::Missing) + .with_status(200) + .with_header("ETag", "\"v1\"") + .with_header("content-type", "application/json") + .with_body(MOCK_TEMPLATE_BODY) + .create_async() + .await; + + let first = load_registry().await.expect("initial fetch"); + assert_eq!(first.templates.len(), 1); + + let _second_mock = server + .mock("GET", "/") + .match_header("if-none-match", "\"v1\"") + .with_status(304) + .create_async() + .await; + + env.force_refresh(); + let second = load_registry() + .await + .expect("conditional refresh should reuse cache"); + assert_eq!(second.templates.len(), 1); + assert_eq!(second.templates[0].name, "demo"); + } + + #[tokio::test] + async fn load_registry_falls_back_to_bundled_default_when_remote_unreachable() { + // Nothing listens on this address, so the request fails fast with a + // connection error rather than a slow timeout. + let _env = RegistryTestEnv::new("http://127.0.0.1:1"); + + let registry = load_registry() + .await + .expect("should fall back instead of erroring"); + let bundled: TemplateRegistry = serde_json::from_str(DEFAULT_REGISTRY).unwrap(); + assert_eq!(registry.templates.len(), bundled.templates.len()); + } } diff --git a/templates/registry.json b/templates/registry.json index 2f25471b..9f248369 100644 --- a/templates/registry.json +++ b/templates/registry.json @@ -23,7 +23,7 @@ "status": "audited", "audited_at": "2025-03-15T00:00:00Z", "auditor": "StarForge Security Team", - "findings": 0, + "findings": "0", "score": 95 }, "changelog": [ @@ -54,7 +54,7 @@ "status": "audited", "audited_at": "2025-04-01T00:00:00Z", "auditor": "StarForge Security Team", - "findings": 1, + "findings": "1", "score": 88 }, "changelog": [ @@ -142,7 +142,7 @@ "status": "audited", "audited_at": "2025-05-01T00:00:00Z", "auditor": "StarForge Security Team", - "findings": 0, + "findings": "0", "score": 98 }, "changelog": [ @@ -172,7 +172,7 @@ "status": "audited", "audited_at": "2025-02-01T00:00:00Z", "auditor": "StarForge Security Team", - "findings": 0, + "findings": "0", "score": 96 }, "changelog": [ @@ -200,7 +200,7 @@ "status": "audited", "audited_at": "2025-05-20T00:00:00Z", "auditor": "StarForge Security Team", - "findings": 0, + "findings": "0", "score": 97 }, "changelog": [ @@ -229,7 +229,7 @@ "status": "audited", "audited_at": "2025-04-10T00:00:00Z", "auditor": "StarForge Security Team", - "findings": 0, + "findings": "0", "score": 95 }, "changelog": [ @@ -257,7 +257,7 @@ "status": "audited", "audited_at": "2025-04-10T00:00:00Z", "auditor": "StarForge Security Team", - "findings": 0, + "findings": "0", "score": 96 }, "changelog": [ @@ -285,7 +285,7 @@ "status": "audited", "audited_at": "2025-06-29T00:00:00Z", "auditor": "StarForge Security Team", - "findings": 0, + "findings": "0", "score": 98 }, "changelog": [ @@ -313,7 +313,7 @@ "status": "audited", "audited_at": "2025-06-29T00:00:00Z", "auditor": "StarForge Security Team", - "findings": 0, + "findings": "0", "score": 97 }, "changelog": [ @@ -341,7 +341,7 @@ "status": "audited", "audited_at": "2025-06-29T00:00:00Z", "auditor": "StarForge Security Team", - "findings": 0, + "findings": "0", "score": 94 }, "changelog": [